diff --git a/packages/kernel/src/snapshot-attach-refs.test.ts b/packages/kernel/src/snapshot-attach-refs.test.ts new file mode 100644 index 000000000..b92bb8e54 --- /dev/null +++ b/packages/kernel/src/snapshot-attach-refs.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, test } from 'vitest'; +import { attachRefs, type RawSnapshotNode } from './snapshot.ts'; + +const node = (index: number, extra: Partial = {}): RawSnapshotNode => ({ + index, + ...extra, +}); + +describe('attachRefs', () => { + test('preserves a backend-minted ref instead of re-minting a dense positional one', () => { + // The web/agent-browser backend mints refs in tree order (non-dense — non-interactive + // nodes are skipped). The first node's backend ref is e2 because e1 went to a node + // that did not appear in the projected tree. Preserving it keeps the ref the agent + // reads off the snapshot identical to the ref the backend resolves on the next action. + const attached = attachRefs([node(0, { ref: 'e2' }), node(1, { ref: 'e3' })]); + expect(attached.map((n) => n.ref)).toEqual(['e2', 'e3']); + // index is preserved untouched — the positional identity is orthogonal to the ref. + expect(attached.map((n) => n.index)).toEqual([0, 1]); + }); + + test('re-mints dense positional refs for nodes without a backend ref', () => { + const attached = attachRefs([node(0), node(1, { ref: 'e7' }), node(2)]); + expect(attached.map((n) => n.ref)).toEqual(['e1', 'e7', 'e3']); + }); +}); diff --git a/packages/kernel/src/snapshot.ts b/packages/kernel/src/snapshot.ts index 4fc96bcfd..06e0e5a71 100644 --- a/packages/kernel/src/snapshot.ts +++ b/packages/kernel/src/snapshot.ts @@ -120,6 +120,15 @@ export type RawSnapshotNode = { hiddenContentBelow?: boolean; interactionBlocked?: 'covered'; presentationHints?: string[]; + /** + * Backend-minted ref for this node, when the capture backend already assigns a + * stable, actionable ref (e.g. the web/agent-browser backend resolves actions + * against its own `@eN` refs). `attachRefs` preserves this instead of re-minting + * a dense positional ref, so the ref an agent sees in the snapshot is the same + * ref the backend can resolve on the next action. Absent for backends that do + * not mint refs — those fall back to dense `e${index}` numbering. + */ + ref?: string; /** * Accessibility custom actions the element exposes (iOS * `UIAccessibilityCustomAction`, React Native `accessibilityActions`). Merged @@ -273,8 +282,15 @@ export type ScreenshotOverlayRef = { center: Point; }; +/** + * Assign a display ref to every node. A node that already carries a backend-minted + * `ref` keeps it (see `RawSnapshotNode.ref`) — the web/agent-browser backend resolves + * actions against its own refs, so re-minting a dense positional ref here would make + * the snapshot show one ref while actions act on a different element. Backends that do + * not mint refs get dense `e${index}` numbering, matching the historical behavior. + */ export function attachRefs(nodes: RawSnapshotNode[]): SnapshotNode[] { - return nodes.map((node, idx) => ({ ...node, ref: `e${idx + 1}` })); + return nodes.map((node, idx) => ({ ...node, ref: node.ref ?? `e${idx + 1}` })); } /** diff --git a/packages/platform-web/src/__tests__/agent-browser-snapshot.test.ts b/packages/platform-web/src/__tests__/agent-browser-snapshot.test.ts index 8664e487d..c94661662 100644 --- a/packages/platform-web/src/__tests__/agent-browser-snapshot.test.ts +++ b/packages/platform-web/src/__tests__/agent-browser-snapshot.test.ts @@ -59,4 +59,34 @@ describe('normalizeAgentBrowserSnapshot', () => { expect(result.nodes[0]?.enabled).toBe(true); }); + + test('preserves the backend ref so the displayed ref is the actionable one', async () => { + // agent-browser mints refs in tree order and skips non-interactive nodes + // (the leading `generic` container never gets a ref), so the refs are NOT dense: + // e1 = generic (skipped here, no ref on it) + // e2 = username textbox + // e3 = passcode textbox <-- agent presses @e3 expecting username + // e4 = sign-in button + // The username textbox is the 2nd NODE in the snapshot but its backend ref is e2. + // If agent-device re-mints dense positional refs, the snapshot would show the + // username as @e2 while the passcode (node 3) shows @e3 — and an agent that + // reads "@e3 = passcode" would land in the passcode field when it meant the + // username, because the backend's own e3 resolves to a different element. + // Preserving the backend ref keeps display ref == actionable ref. + const result = await normalizeAgentBrowserSnapshot({ + snapshot: [ + '- textbox "Username" [ref=e2]', + '- textbox "Passcode" [ref=e3]', + '- button "Sign in" [ref=e4]', + ].join('\n'), + refs: { + e2: { role: 'textbox', name: 'Username' }, + e3: { role: 'textbox', name: 'Passcode' }, + e4: { role: 'button', name: 'Sign in' }, + }, + }); + + expect(result.nodes.map((node) => node.label)).toEqual(['Username', 'Passcode', 'Sign in']); + expect(result.nodes.map((node) => node.ref)).toEqual(['e2', 'e3', 'e4']); + }); }); diff --git a/packages/platform-web/src/agent-browser-provider.test.ts b/packages/platform-web/src/agent-browser-provider.test.ts index 43635685f..d2629b2a1 100644 --- a/packages/platform-web/src/agent-browser-provider.test.ts +++ b/packages/platform-web/src/agent-browser-provider.test.ts @@ -666,6 +666,9 @@ function expectedNode( ) { return { index, + // The web normalizer now preserves the backend ref on each node. This fixture's + // refs happen to be dense in tree order, so the ref matches the positional `e{index}`. + ref: `e${index + 1}`, type, role: type, label, diff --git a/packages/platform-web/src/agent-browser-snapshot.ts b/packages/platform-web/src/agent-browser-snapshot.ts index 4f0211eab..9994570ba 100644 --- a/packages/platform-web/src/agent-browser-snapshot.ts +++ b/packages/platform-web/src/agent-browser-snapshot.ts @@ -31,7 +31,13 @@ export async function normalizeAgentBrowserSnapshot( if (fetchBox) await attachDraftRects(drafts, fetchBox); return { - nodes: drafts.map((draft, index) => ({ ...draft.node, index })), + // Preserve each draft's backend ref as the node's `ref`. agent-browser resolves + // actions (click/fill/hover) against its own `@eN` refs, which are minted in tree + // order and are NOT dense — so the ref an agent reads off the snapshot must be the + // backend ref, not a re-minted dense positional ref. `attachRefs` (downstream) keeps + // it. Without this the snapshot could show `@e3` = username while the action on + // `@e3` lands on the backend's `@e3` (a different element, e.g. the passcode field). + nodes: drafts.map((draft, index) => ({ ...draft.node, index, ref: draft.ref })), truncated: readBooleanProperty(data, 'truncated'), }; } diff --git a/test/integration/provider-scenarios/web-provider.test.ts b/test/integration/provider-scenarios/web-provider.test.ts index 10edfccca..67d784491 100644 --- a/test/integration/provider-scenarios/web-provider.test.ts +++ b/test/integration/provider-scenarios/web-provider.test.ts @@ -1,7 +1,13 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; import { WEB_DESKTOP_DEVICE } from '../../../src/__tests__/test-utils/device-fixtures.ts'; -import type { WebProvider } from '@agent-device/platform-web'; +import { createAgentBrowserWebProvider, type WebProvider } from '@agent-device/platform-web'; +import { withCommandExecutorOverride } from '@agent-device/host-kit/command'; +import { mkdtempForTestSync } from '../../../src/__tests__/test-utils/tmp-dir.ts'; +import { + installFakeManagedAgentBrowser, + withNodeRuntime, +} from '../../../src/__tests__/test-utils/web-managed-agent-browser.ts'; import { createProviderScenarioHarness } from './harness.ts'; test('web provider is scoped through the request router and dispatch path', async () => { @@ -200,3 +206,85 @@ test('web provider is scoped through the request router and dispatch path', asyn await harness.close(); } }); + +test('non-dense browser refs survive snapshot storage and routed fill and click', async () => { + const calls: string[][] = []; + const values: Record = { '@e2': '', '@e3': '' }; + let signedIn = false; + const stateDir = mkdtempForTestSync('web-ref-route-'); + installFakeManagedAgentBrowser(stateDir); + await withNodeRuntime({ version: '24.0.0' }, async () => { + const provider = await createAgentBrowserWebProvider({ stateDir }); + await withCommandExecutorOverride( + async (_command, args) => { + const [command, ref, text] = args.slice(1); + let data: unknown = {}; + if (command === 'snapshot') { + data = { + snapshot: [ + '- textbox "Username" [ref=e2]', + '- textbox "Passcode" [ref=e3]', + '- button "Sign in" [ref=e4]', + ].join('\n'), + refs: { + e2: { role: 'textbox', name: 'Username' }, + e3: { role: 'textbox', name: 'Passcode' }, + e4: { role: 'button', name: 'Sign in' }, + }, + }; + } else if (command === 'fill') { + calls.push(['fill', ref!, text!]); + values[ref!] = text!; + } else if (command === 'click') { + calls.push(['click', ref!]); + signedIn = ref === '@e4'; + } else { + assert.ok(command === 'open' || command === 'close', `Unexpected command: ${args}`); + } + return { stdout: JSON.stringify({ success: true, data }), stderr: '', exitCode: 0 }; + }, + async () => { + const harness = await createProviderScenarioHarness({ + deviceInventoryProvider: async () => [WEB_DESKTOP_DEVICE], + platformRuntime: true, + webProvider: () => provider, + }); + try { + const open = await harness.callCommand('open', ['https://example.test/login'], { + platform: 'web', + }); + assert.equal(open.json.error, undefined); + const snapshot = await harness.callCommand('snapshot', [], { platform: 'web' }); + assert.equal(snapshot.json.error, undefined); + const nodes = snapshot.json.result.data.nodes; + assert.deepEqual( + nodes.map((node: { label: string; ref: string }) => [node.label, node.ref]), + [ + ['Username', 'e2'], + ['Passcode', 'e3'], + ['Sign in', 'e4'], + ], + ); + const fill = await harness.callCommand('fill', [`@${nodes[0].ref}`, 'Ada']); + assert.equal(fill.json.error, undefined); + assert.deepEqual(values, { '@e2': 'Ada', '@e3': '' }); + + const refreshed = await harness.callCommand('snapshot'); + assert.equal(refreshed.json.error, undefined); + const button = refreshed.json.result.data.nodes.find( + (node: { label: string }) => node.label === 'Sign in', + ); + const click = await harness.callCommand('click', [`@${button.ref}`]); + assert.equal(click.json.error, undefined); + assert.equal(signedIn, true); + assert.deepEqual(calls, [ + ['fill', '@e2', 'Ada'], + ['click', '@e4'], + ]); + } finally { + await harness.close(); + } + }, + ); + }); +});