From f1bef4b3a2fbd050766f8fd3fc7837d59aae2e62 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 26 Aug 2026 20:09:49 -0700 Subject: [PATCH] fix(browser): preserve snapshot names and native refs (#130623) --- docs/tools/browser-control.md | 13 +- .../browser/pw-role-snapshot.chromium.test.ts | 96 +++++++++++++ .../src/browser/pw-role-snapshot.test.ts | 53 ++++++++ .../browser/src/browser/pw-role-snapshot.ts | 126 ++++++++---------- 4 files changed, 213 insertions(+), 75 deletions(-) create mode 100644 extensions/browser/src/browser/pw-role-snapshot.chromium.test.ts diff --git a/docs/tools/browser-control.md b/docs/tools/browser-control.md index bcd5868b0089..40019ced3b58 100644 --- a/docs/tools/browser-control.md +++ b/docs/tools/browser-control.md @@ -265,7 +265,7 @@ Notes: interception is available for managed Playwright profiles; existing-session profiles return an unsupported-operation error. - Prefer atomic chooser uploads: pass the trigger `--ref` with the upload so OpenClaw arms and clicks in one request. Paths-only `upload` remains supported when a later trigger is intentional. Use `--input-ref` or `--element` to set a file input directly. `dialog` is an arming call; run it before the click/press that triggers the dialog. If an action opens a modal, the action response includes `blockedByDialog` and `browserState.dialogs.pending`; pass that `dialogId` to respond directly. Dialogs handled outside OpenClaw appear under `browserState.dialogs.recent`. -- `click`/`type`/etc require a `ref` from `snapshot` (numeric `12`, role ref `e12`, or actionable ARIA ref `ax12`). CSS selectors are intentionally not supported for actions. Use `click-coords` when the visible viewport position is the only reliable target. +- `click`/`type`/etc require a `ref` from `snapshot` (for example, Playwright ref `f1e12`, role ref `e12`, or actionable ARIA ref `ax12`). Copy the returned ref unchanged, including any frame prefix. CSS selectors are intentionally not supported for actions. Use `click-coords` when the visible viewport position is the only reliable target. - Download and trace paths are constrained to OpenClaw temp roots: `/tmp/openclaw{,/downloads}` (fallback: `${os.tmpdir()}/openclaw/...`). - `upload` accepts files from the OpenClaw temp uploads root and OpenClaw-managed inbound media. Managed inbound media can be referenced as @@ -282,7 +282,7 @@ volatile; prefer `suggestedTargetId` from `tabs` in scripts. Snapshot flags at a glance: -- `--format ai` (default with Playwright): AI snapshot with numeric refs (`aria-ref=""`). +- `--format ai` (default with Playwright): AI snapshot with native Playwright refs, including frame-qualified refs such as `f1e12`. - `--format aria`: accessibility tree with `axN` refs. When Playwright is available, OpenClaw binds refs with backend DOM ids to the live page so follow-up actions can use them; otherwise treat the output as inspection-only. - `--efficient` (or `--mode efficient`): compact role snapshot preset. Set `browser.snapshotDefaults.mode: "efficient"` to make this the default (see [Gateway configuration](/gateway/configuration-reference#browser)). - `--interactive`, `--compact`, `--depth`, `--selector` force a role snapshot with `ref=e12` refs. `--frame "'); + const buttonName = 'Save: "owner\'s" C:\\draft 🦞'; + const inputName = 'Project "path"'; + await page.evaluate( + ({ buttonName: label, inputName: inputLabel }) => { + for (const id of ["first", "second"]) { + const button = document.createElement("button"); + button.textContent = label; + button.addEventListener("click", () => { + document.querySelector("output")!.textContent = id; + }); + document.querySelector("main")!.append(button); + } + const input = document.createElement("input"); + input.setAttribute("aria-label", inputLabel); + document.querySelector("main")!.append(input); + document.querySelector("iframe")!.srcdoc = + ""; + }, + { buttonName, inputName }, + ); + await page.frameLocator("iframe").getByRole("button").waitFor(); + const session = await context.newCDPSession(page); + const { targetInfo } = await session.send("Target.getTargetInfo"); + await session.detach(); + const target = { cdpUrl, targetId: targetInfo.targetId }; + for (const mode of ["role", "interactive", "ai", "interactive-aria"] as const) { + const snapshot = + mode === "ai" + ? await snapshotAiViaPlaywright(target) + : await snapshotRoleViaPlaywright({ + ...target, + refsMode: mode === "interactive-aria" ? "aria" : "role", + options: { interactive: mode !== "role" }, + }); + const buttons = Object.entries(snapshot.refs).filter( + ([, value]) => value.role === "button" && value.name === buttonName, + ); + expect(buttons, mode).toHaveLength(2); + for (const [index, [ref]] of buttons.entries()) { + await clickViaPlaywright({ ...target, ref, timeoutMs: 1_000 }); + expect(await page.locator("output").textContent(), mode).toBe( + index ? "second" : "first", + ); + } + const input = Object.entries(snapshot.refs).find( + ([, value]) => value.role === "textbox" && value.name === inputName, + ); + expect(input, mode).toBeDefined(); + await typeViaPlaywright({ ...target, ref: input![0], text: mode, timeoutMs: 1_000 }); + expect(await page.getByRole("textbox").inputValue()).toBe(mode); + } + const snapshot = await snapshotAiViaPlaywright(target); + const nested = Object.entries(snapshot.refs).find( + ([, value]) => value.name === "Frame: action", + ); + expect(nested).toBeDefined(); + await clickViaPlaywright({ ...target, ref: nested![0], timeoutMs: 1_000 }); + expect(await page.frameLocator("iframe").getByRole("button").textContent()).toBe( + "Frame clicked", + ); + } finally { + await closePlaywrightBrowserConnection({ cdpUrl }); + await context.close(); + } + }, 30_000); + }, +); diff --git a/extensions/browser/src/browser/pw-role-snapshot.test.ts b/extensions/browser/src/browser/pw-role-snapshot.test.ts index 9a272a1d721f..d47d635d0e34 100644 --- a/extensions/browser/src/browser/pw-role-snapshot.test.ts +++ b/extensions/browser/src/browser/pw-role-snapshot.test.ts @@ -9,6 +9,38 @@ import { } from "./pw-role-snapshot.js"; describe("pw-role-snapshot", () => { + describe.each([false, true])("encoded names (interactive=%s)", (interactive) => { + it.each([ + ['button "Save \\"draft\\""', 'Save "draft"'], + ['button "Open C:\\\\draft"', "Open C:\\draft"], + [`'button "Save: owner''s draft"'`, "Save: owner's draft"], + [`'button "Issue #123"'`, "Issue #123"], + [`'button "Save {draft}"'`, "Save {draft}"], + ['button "保存 🦞 résumé"', "保存 🦞 résumé"], + ["button /api/v1/", "/api/v1/"], + ])("preserves %s through actionable refs", (key, name) => { + const built = buildRoleSnapshotFromAriaSnapshot(`- ${key}`, { interactive }); + const result = finalizeRoleSnapshot(built); + expect(result.refs).toEqual({ e1: { role: "button", name } }); + expect(result.stats.refs).toBe(1); + expect(result.snapshot).toContain(JSON.stringify(name)); + }); + + it("preserves native frame refs without reading refs inside names or values", () => { + const snapshot = [ + `- 'button "Save: owner''s draft" [ref=f2e3]': text [ref=e99]`, + '- button "Save \\"draft\\" [ref=e98]" [ref=f2e4]', + ].join("\n"); + const built = buildRoleSnapshotFromAiSnapshot(snapshot, { interactive }); + const result = finalizeRoleSnapshot(built); + expect(result.refs).toEqual({ + f2e3: { role: "button", name: "Save: owner's draft" }, + f2e4: { role: "button", name: 'Save "draft" [ref=e98]' }, + }); + expect(result.stats.refs).toBe(2); + }); + }); + it("adds refs for interactive elements", () => { const aria = [ '- heading "Example" [level=1]', @@ -104,6 +136,27 @@ describe("pw-role-snapshot", () => { expect(result.stats.refs).toBe(1); }); + it("finalizes MCP text without requiring JSON-encoded control characters", () => { + const name = "Edit\titem\b"; + const result = finalizeRoleSnapshot({ + snapshot: `- button "${name}" [ref=mcp-ref:session:3]`, + refs: { "mcp-ref:session:3": { role: "button", name } }, + }); + expect(result.refs).toEqual({ "mcp-ref:session:3": { role: "button", name } }); + }); + + it.each(["\u2028", "\u2029"])("preserves MCP refs around Unicode separator %j", (separator) => { + for (const field of ["name", "value", "description"] as const) { + const name = field === "name" ? `Edit${separator}item` : "Edit item"; + const suffix = field === "name" ? "" : ` ${field}="first${separator}second"`; + const result = finalizeRoleSnapshot({ + snapshot: `- textbox "${name}" [ref=mcp-ref:session:3]${suffix}`, + refs: { "mcp-ref:session:3": { role: "textbox", name } }, + }); + expect(result.refs, field).toEqual({ "mcp-ref:session:3": { role: "textbox", name } }); + } + }); + it("uses a bounded marker for budgets too small for a snapshot line", () => { const result = finalizeRoleSnapshot({ snapshot: '- button "Visible" [ref=e1]', diff --git a/extensions/browser/src/browser/pw-role-snapshot.ts b/extensions/browser/src/browser/pw-role-snapshot.ts index 7972e0436822..484c56a53aff 100644 --- a/extensions/browser/src/browser/pw-role-snapshot.ts +++ b/extensions/browser/src/browser/pw-role-snapshot.ts @@ -29,8 +29,6 @@ type RoleSnapshotStats = { }; const ROLE_SNAPSHOT_TRUNCATION_MARKER = "[...TRUNCATED - page too large]"; -// A formatter ref precedes any YAML scalar delimiter; ref-looking scalar text is hostile page content. -const ROLE_SNAPSHOT_LINE_REF_RE = /^\s*-\s+\w+(?:\s+"(?:\\.|[^"\\])*")?[^:]*?\[ref=([^\]]+)\]/; /** Options for filtering and compacting role snapshots. */ export type RoleSnapshotOptions = { @@ -43,7 +41,8 @@ export type RoleSnapshotOptions = { }; function findSnapshotLineRef(line: string): string | undefined { - return ROLE_SNAPSHOT_LINE_REF_RE.exec(line)?.[1]; + // Names and scalar values are page content, never formatter-owned refs. + return parseSnapshotLine(line)?.suffix.match(/^[^:]*?\[ref=([^\]]+)\]/)?.[1]; } function getRoleSnapshotIdentityKey( @@ -193,36 +192,47 @@ function getIndentLevel(line: string): number { return indent === undefined ? 0 : Math.floor(indent.length / 2); } -function matchInteractiveSnapshotLine( - line: string, - options: RoleSnapshotOptions, -): { roleRaw: string; role: string; name?: string; suffix: string } | null { - const depth = getIndentLevel(line); - if (options.maxDepth !== undefined && depth > options.maxDepth) { +function parseSnapshotLine(line: string) { + const entry = line.match(/^(\s*-\s+)(.*)$/s); + if (!entry) { return null; } - const match = line.match(/^(\s*-\s*)(\w+)(?:\s+"([^"]*)")?(.*)$/); + const prefix = entry[1]!; + const content = entry[2]!; + // Playwright JSON-encodes names, then single-quotes YAML keys when required. + // Keep the token lexical here: the shared finalizer also consumes MCP text. + const quoted = content.match(/^'((?:[^']|'')*)'(.*)$/s); + const key = quoted ? quoted[1]!.replaceAll("''", "'") : content; + const match = key.match(/^(\w+)(?:\s+("(?:\\.|[^"\\])*"))?(.*)$/s); if (!match) { return null; } - const roleRaw = match[2]; - const name = match[3]; - const suffix = match[4]; - if (roleRaw === undefined || suffix === undefined) { - return null; + const roleRaw = match[1]!; + let nameToken = match[2]; + let suffix = match[3]!; + // Slash-delimited names are emitted literally outside codegen mode. An + // unquoted YAML value cannot be part of that name (even if it contains '/'). + if (nameToken === undefined && suffix.startsWith(" /")) { + const header = quoted ? suffix : suffix.split(/:(?=\s|$)/, 1)[0]!; + const literal = header.match(/^ (\/.*\/)/s); + if (literal) { + nameToken = literal[1]!; + suffix = suffix.slice(literal[0].length); + } } - if (roleRaw.startsWith("/")) { - return null; - } - const role = normalizeLowercaseStringOrEmpty(roleRaw); return { + prefix, roleRaw, - role, - ...(name ? { name } : {}), - suffix, + role: normalizeLowercaseStringOrEmpty(roleRaw), + nameToken, + suffix: suffix + (quoted?.[2] ?? ""), }; } +function decodeSnapshotName(nameToken: string | undefined): string | undefined { + return nameToken?.startsWith('"') ? JSON.parse(nameToken) : nameToken; +} + type RoleNameTracker = { counts: Map; refsByKey: Map; @@ -334,23 +344,12 @@ function processLine( return null; } - const match = line.match(/^(\s*-\s*)(\w+)(?:\s+"([^"]*)")?(.*)$/); - if (!match) { + const parsed = parseSnapshotLine(line); + if (!parsed) { return options.interactive ? null : line; } - - const prefix = match[1]; - const roleRaw = match[2]; - const name = match[3]; - const suffix = match[4]; - if (prefix === undefined || roleRaw === undefined || suffix === undefined) { - return options.interactive ? null : line; - } - if (roleRaw.startsWith("/")) { - return options.interactive ? null : line; - } - - const role = normalizeLowercaseStringOrEmpty(roleRaw); + const { prefix, roleRaw, role, suffix } = parsed; + const name = decodeSnapshotName(parsed.nameToken); const isInteractive = INTERACTIVE_ROLES.has(role); const isContent = CONTENT_ROLES.has(role); const isStructural = STRUCTURAL_ROLES.has(role); @@ -378,7 +377,7 @@ function processLine( let enhanced = `${prefix}${roleRaw}`; if (name) { - enhanced += ` "${name}"`; + enhanced += ` ${JSON.stringify(name)}`; } enhanced += ` [ref=${ref}]`; if (nth > 0) { @@ -390,21 +389,27 @@ function processLine( return enhanced; } -type InteractiveSnapshotLine = NonNullable>; +type InteractiveSnapshotLine = NonNullable> & { + name?: string; +}; function buildInteractiveSnapshotLines(params: { lines: string[]; options: RoleSnapshotOptions; resolveRef: (parsed: InteractiveSnapshotLine) => { ref: string; nth?: number } | null; recordRef: (parsed: InteractiveSnapshotLine, ref: string, nth?: number) => void; - includeSuffix: (suffix: string) => boolean; + formatSuffix: (suffix: string, ref: string) => string; }): string[] { const out: string[] = []; for (const line of params.lines) { - const parsed = matchInteractiveSnapshotLine(line, params.options); - if (!parsed) { + if (params.options.maxDepth !== undefined && getIndentLevel(line) > params.options.maxDepth) { continue; } + const entry = parseSnapshotLine(line); + if (!entry) { + continue; + } + const parsed = { ...entry, name: decodeSnapshotName(entry.nameToken) }; if (!INTERACTIVE_ROLES.has(parsed.role)) { continue; } @@ -416,15 +421,13 @@ function buildInteractiveSnapshotLines(params: { let enhanced = `- ${parsed.roleRaw}`; if (parsed.name) { - enhanced += ` "${parsed.name}"`; + enhanced += ` ${JSON.stringify(parsed.name)}`; } enhanced += ` [ref=${resolved.ref}]`; if ((resolved.nth ?? 0) > 0) { enhanced += ` [nth=${resolved.nth}]`; } - if (params.includeSuffix(parsed.suffix)) { - enhanced += parsed.suffix; - } + enhanced += params.formatSuffix(parsed.suffix, resolved.ref); out.push(enhanced); } return out; @@ -482,7 +485,7 @@ export function buildRoleSnapshotFromAriaSnapshot( nth, }; }, - includeSuffix: (suffix) => suffix.includes("["), + formatSuffix: (suffix) => (suffix.includes("[") ? suffix : ""), }); removeNthFromNonDuplicates(refs, tracker); @@ -511,12 +514,8 @@ export function buildRoleSnapshotFromAriaSnapshot( } function parseAiSnapshotRef(suffix: string): string | null { - const eMatch = suffix.match(/\[ref=(e\d+)\]/i); - if (eMatch) { - return eMatch[1] ?? null; - } - const numMatch = suffix.match(/\[ref=(\d{1,9})\]/); - return numMatch?.[1] ?? null; + // Playwright's page-wide AI snapshots qualify element refs with a frame seq. + return suffix.match(/^[^:]*?\[ref=((?:f\d+)?e\d+|\d{1,9})\]/i)?.[1] ?? null; } /** @@ -542,7 +541,7 @@ export function buildRoleSnapshotFromAiSnapshot( recordRef: ({ role, name }, ref) => { refs[ref] = { role, ...(name ? { name } : {}) }; }, - includeSuffix: () => true, + formatSuffix: (suffix, ref) => suffix.replace(` [ref=${ref}]`, ""), }); return { snapshot: out.join("\n") || "(no interactive elements)", @@ -557,24 +556,13 @@ export function buildRoleSnapshotFromAiSnapshot( continue; } - const match = line.match(/^(\s*-\s*)(\w+)(?:\s+"([^"]*)")?(.*)$/); - if (!match) { + const parsed = parseSnapshotLine(line); + if (!parsed) { out.push(line); continue; } - const roleRaw = match[2]; - const name = match[3]; - const suffix = match[4]; - if (roleRaw === undefined || suffix === undefined) { - out.push(line); - continue; - } - if (roleRaw.startsWith("/")) { - out.push(line); - continue; - } - - const role = normalizeLowercaseStringOrEmpty(roleRaw); + const { role, suffix } = parsed; + const name = decodeSnapshotName(parsed.nameToken); const isStructural = STRUCTURAL_ROLES.has(role); if (options.compact && isStructural && !name) {