From c068047b19c9d99c24eac1c2921326f459668853 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 12 Oct 2026 18:30:07 +0200 Subject: [PATCH 1/3] perf(web): fetch snapshot geometry in one batch instead of one spawn per ref `is visible`/`is hidden` and every interaction that aims by ref ask the web backend for the bounds of every ref of a capture. The provider asked per ref, and each `agent-browser get box` is a fresh Node process: measured on a 408-ref page, 381 spawns for one check. The budget they spent on interpreter startup is why `is visible` on such a page died at the request envelope (`ready timed out after 4500ms`) - the command answers in 90s against a 45s deadline. The `--timeout` that would have raised the deadline is not even accepted by `is`/`wait`, so on a big page those commands could not be made to work at all. `batch` is the backend's own answer to that: it reads a JSON array of commands from stdin and runs them inside ONE process, so the whole geometry pass is a bounded number of spawns - 200 refs measure in 0.28s, all 408 of the bench page in 0.33s, against a 30s per-command budget. Getting `stdin` to the spawn is the whole plumbing cost - `ExecOptions` already supported it, so `runManagedAgentBrowser` forwards it and `runAgentBrowserCommand` passes it through. Grouping lives with the grouping decision: the projection asks for the bounds of every ref it published and the provider decides how many refs to send per call, because the refusal below is a claim about the whole capture and only one owner sees all of it. A ref the batch could not measure is an ordinary answer about that ref, and the entry's own `success` decides that - never the error's wording. `isIgnorableBoxError` is gone with it: it was a table of phrases the backend is free to reword, and a reworded one turned "this ref has no box" into "geometry is broken", failing an interaction over a ref `is visible` never aimed at. What the old code called a fatal error now has to be a fault of the batch as a whole, which is what it actually reports: a whole-batch fault, or an answer that cannot be paired back to the refs, is refused typed as `unusable batch result` with the reason named - the entry count, an entry that carries no verdict (an object with neither `success` nor the command it answers would read as "this element has no box" and drop a node silently), or an entry whose echoed command is not the command that was sent. Position is trusted only as the order of the echoed command, never as the pairing itself, because a mispaired box reads as a working capture with wrong geometry. And a capture whose refs ALL went unmeasurable is refused rather than published: a hidden element still measures as a box, so that shape means the backend answered to refs this snapshot does not publish, and the tree would carry no addressable target with usable bounds. It is decided over the capture rather than per group - a page whose first group went stale still answers for the rest. The refusal reports `reason: snapshot-refs-unmeasurable` with `requestedRefs` and `measuredRects`. Tests: the geometry seam is its own file, `agent-browser-provider-rects.test.ts`, mirroring the seam the provider source grew (the provider file was past the 1,000-line test tripwire and the ratchet named this split as its fix); the provider file keeps the capture half. The old per-ref tests became the batch contract: one call for the capture, groups bounded per fetch, a group that answered nothing while a later one measured, entries refused by echoed command or missing verdict, and each fault shape above. Measured live (managed agent-browser 0.27.1, same page, base/head interleaved in one loop iteration because this host sat at load 125-223, n=5 medians): | page (refs) | command | this branch | before | | --- | --- | --- | --- | | index.html (408) | `is visible` | **5.9s** | **90.1s, and 3 of 5 runs refused outright** | | index.html (408) | `fill` | **10.1s** | **90.2s** | | light.html (8) | `is visible` | **2.5s** | 3.9s | | light.html (8) | `fill` | 9.7s | 9.0s (noise; light pages were already fine) | Backend spawns for one capture + geometry pass: **4 instead of 385** (408-ref page), 4 instead of 11 (8-ref page). Verdicts are unchanged: on a page with an on-screen button and one at `top: -9000px`, both builds answer identically. Closes #3516 --- .../agent-browser-snapshot-refless.test.ts | 10 +- .../src/agent-browser-provider-rects.test.ts | 480 ++++++++++++++++++ .../src/agent-browser-provider.test.ts | 82 +-- .../src/agent-browser-provider.ts | 149 +++++- .../src/agent-browser-snapshot.ts | 32 +- .../platform-web/src/agent-browser-tool.ts | 3 + 6 files changed, 640 insertions(+), 116 deletions(-) create mode 100644 packages/platform-web/src/agent-browser-provider-rects.test.ts diff --git a/packages/platform-web/src/__tests__/agent-browser-snapshot-refless.test.ts b/packages/platform-web/src/__tests__/agent-browser-snapshot-refless.test.ts index 3f427124cd..2499af926d 100644 --- a/packages/platform-web/src/__tests__/agent-browser-snapshot-refless.test.ts +++ b/packages/platform-web/src/__tests__/agent-browser-snapshot-refless.test.ts @@ -282,8 +282,14 @@ describe('normalizeAgentBrowserSnapshot ref-less content', () => { }); test('attaches geometry only to the nodes the backend can measure', async () => { - const result = await normalizeAgentBrowserSnapshot(FORM_AFTER_SUBMIT, async (ref) => - ref === 'e2' ? { x: 1, y: 2, width: 10, height: 20 } : undefined, + const result = await normalizeAgentBrowserSnapshot( + FORM_AFTER_SUBMIT, + async (refs) => + new Map( + refs.flatMap((ref) => + ref === 'e2' ? [[ref, { x: 1, y: 2, width: 10, height: 20 }] as const] : [], + ), + ), ); expect(result.nodes.find((node) => node.ref === 'e2')?.rect).toEqual({ diff --git a/packages/platform-web/src/agent-browser-provider-rects.test.ts b/packages/platform-web/src/agent-browser-provider-rects.test.ts new file mode 100644 index 0000000000..1aa3f2c94a --- /dev/null +++ b/packages/platform-web/src/agent-browser-provider-rects.test.ts @@ -0,0 +1,480 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import { test, vi } from 'vitest'; +import { installFakeManagedAgentBrowser, mkdtempForTestSync } from './__tests__/test-utils.ts'; + +const { providerStartupCleanupMock } = vi.hoisted(() => ({ + providerStartupCleanupMock: vi.fn(), +})); + +// Startup cleanup walks the host process table, which would read as an extra backend call in the +// exact call lists these tests assert. +vi.mock('./agent-browser-lifecycle.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + cleanupManagedAgentBrowserOrphansForProviderStartup: providerStartupCleanupMock, + }; +}); + +import { createAgentBrowserWebProvider } from './agent-browser-provider.ts'; +import type { WebSnapshotResult } from './provider.ts'; +import { withCommandExecutorOverride, type ExecResult } from '@agent-device/host-kit/command'; +import { AppError } from '@agent-device/kernel/errors'; + +/** + * One backend call, with the text the provider wrote to its stdin — which is how `batch` receives + * the work it runs inside a single process. + */ +type AgentBrowserCall = { + cmd: string; + args: readonly string[]; + stdin?: string; +}; + +test('agent-browser provider fetches snapshot geometry in one batch for the whole capture', async () => { + await withWebProvider(async (provider) => { + const calls: AgentBrowserCall[] = []; + const snapshot = await withCommandExecutorOverride( + geometryExecutor(calls), + async () => await provider.snapshot({ includeRects: true }), + ); + + assert.deepEqual( + calls.map((call) => call.args.slice(0, 3)), + [ + ['snapshot', '--json', '--session'], + ['batch', '--json', '--session'], + ], + ); + // One round trip for every ref of the capture: each backend call is a process spawn, and a page + // can hold hundreds of refs. + assert.deepEqual(readBatchCommands(calls[1]?.stdin), [ + ['get', 'box', '@e1'], + ['get', 'box', '@e2'], + ['get', 'box', '@e3'], + ['get', 'box', '@e4'], + ]); + assertGeometryRects(snapshot); + }); +}); + +test('agent-browser provider keeps the geometry a batch did measure', async () => { + await withWebProvider(async (provider) => { + const calls: AgentBrowserCall[] = []; + const snapshot = await withCommandExecutorOverride( + async (cmd, args, options) => { + const cliArgs = recordCall(calls, cmd, args, options.stdin); + if (cliArgs[0] !== 'batch') return snapshotPayload(); + const commands = readBatchCommands(options.stdin); + return jsonResult( + commands.map((command, index) => + index === 0 + ? { command, success: false, error: 'Unknown ref: e1', result: null } + : { command, success: true, error: null, result: boxRect(command[2]) }, + ), + 1, + ); + }, + async () => await provider.snapshot({ includeRects: true }), + ); + + // A ref the backend could not measure is an ordinary answer about that ref: the rest of the + // tree keeps its geometry, and the entry's own `success` is what decided that — never the + // error's wording, which the backend has no reason to keep stable. + assert.deepEqual( + snapshot.nodes.map((node) => node.rect), + [ + undefined, + { x: 11, y: 12, width: 100, height: 20 }, + { x: 31, y: 32, width: 100, height: 20 }, + { x: 31, y: 32, width: 100, height: 20 }, + ], + ); + }); +}); + +test('agent-browser provider splits snapshot geometry into bounded batches', async () => { + await withWebProvider(async (provider) => { + const calls: AgentBrowserCall[] = []; + const snapshot = await withCommandExecutorOverride( + async (cmd, args, options) => { + const cliArgs = recordCall(calls, cmd, args, options.stdin); + if (cliArgs[0] !== 'batch') return manyRefSnapshotPayload(250); + const commands = readBatchCommands(options.stdin); + return jsonResult( + commands.map((command) => ({ + command, + success: true, + error: null, + result: { x: 1, y: 2, width: 10, height: 20 }, + })), + 1, + ); + }, + async () => await provider.snapshot({ includeRects: true }), + ); + + // The backend reads a batch from stdin as one JSON array, so refs are fetched in bounded groups + // rather than as one unbounded payload. + const groups = calls + .filter((call) => call.args[0] === 'batch') + .map((call) => readBatchCommands(call.stdin)); + assert.deepEqual( + groups.map((group) => group.length), + [200, 50], + ); + assert.equal( + snapshot.nodes.filter((node) => node.rect !== undefined).length, + 250, + 'every ref of both groups keeps the box its entry reported', + ); + }); +}); + +test('agent-browser provider keeps a capture whose later groups measured after an early one failed', async () => { + // The refusal below is a statement about the capture, so it has to be decided over all of it: a + // page whose first group of refs went stale still has geometry for the rest, and answering that + // capture is what the caller can act on. + await withWebProvider(async (provider) => { + const calls: AgentBrowserCall[] = []; + const snapshot = await withCommandExecutorOverride( + async (cmd, args, options) => { + const cliArgs = recordCall(calls, cmd, args, options.stdin); + if (cliArgs[0] !== 'batch') return manyRefSnapshotPayload(250); + const commands = readBatchCommands(options.stdin); + const firstGroup = commands.length === 200; + return jsonResult( + commands.map((command) => + firstGroup + ? { command, success: false, error: 'Unknown ref', result: null } + : { + command, + success: true, + error: null, + result: { x: 1, y: 2, width: 10, height: 20 }, + }, + ), + 1, + ); + }, + async () => await provider.snapshot({ includeRects: true }), + ); + + assert.equal( + snapshot.nodes.filter((node) => node.rect !== undefined).length, + 50, + 'the group that answered keeps its geometry', + ); + assert.equal( + calls.filter((call) => call.args[0] === 'batch').length, + 2, + 'both groups were asked', + ); + }); +}); + +test('agent-browser provider refuses snapshot geometry when the batch resolves no refs', async () => { + // An element the backend knows measures as a box even when hidden, so a capture whose every ref + // went unmeasurable answered to refs this snapshot does not publish. Returning no geometry at all + // would hand the caller a tree of targets with no usable bounds. + await withWebProvider(async (provider) => { + await assert.rejects( + () => + withCommandExecutorOverride( + async (_cmd, args, options) => { + if (agentBrowserCliArgs(args)[0] !== 'batch') return snapshotPayload(); + const commands = readBatchCommands(options.stdin); + return jsonResult( + commands.map((command) => ({ + command, + success: false, + error: 'Unknown ref', + result: null, + })), + 1, + ); + }, + async () => await provider.snapshot({ includeRects: true }), + ), + (error: unknown) => + error instanceof AppError && + error.code === 'COMMAND_FAILED' && + error.details?.reason === 'snapshot-refs-unmeasurable' && + error.details?.measuredRects === 0, + ); + }); +}); + +test('agent-browser provider refuses geometry when the batch answers with too few entries', async () => { + // Entries pair back to refs by position, so a short answer cannot be trusted: pairing four refs + // against the entries that exist would hand each node some other node's box. + await withWebProvider(async (provider) => { + await assert.rejects( + () => + withCommandExecutorOverride( + async (_cmd, args, options) => { + if (agentBrowserCliArgs(args)[0] !== 'batch') return snapshotPayload(); + const commands = readBatchCommands(options.stdin); + return jsonResult( + [{ command: commands[0], success: true, error: null, result: boxRect('@e1') }], + 1, + ); + }, + async () => await provider.snapshot({ includeRects: true }), + ), + (error: unknown) => + error instanceof AppError && + error.code === 'COMMAND_FAILED' && + /unusable batch result/.test(error.message), + ); + }); +}); + +test('agent-browser provider refuses geometry when the batch answers with an object', async () => { + // An answer that is not a list of entries cannot be paired to refs at all, so the geometry is + // refused rather than guessed at. + await withWebProvider(async (provider) => { + await assert.rejects( + () => + withCommandExecutorOverride( + async (_cmd, args) => + agentBrowserCliArgs(args)[0] === 'batch' + ? jsonResult({ success: true, data: { measured: 4 } }) + : snapshotPayload(), + async () => await provider.snapshot({ includeRects: true }), + ), + (error: unknown) => + error instanceof AppError && + error.code === 'COMMAND_FAILED' && + /unusable batch result/.test(error.message), + ); + }); +}); + +test('agent-browser provider refuses geometry when a batch entry is not an entry', async () => { + // Right number of answers, one of them not a result object — an empty object carrying neither the + // command it answers nor a verdict: the positions no longer describe the refs they measured. + await withWebProvider(async (provider) => { + await assert.rejects( + () => + withCommandExecutorOverride( + async (_cmd, args, options) => { + if (agentBrowserCliArgs(args)[0] !== 'batch') return snapshotPayload(); + const commands = readBatchCommands(options.stdin); + return jsonResult( + commands.map((command, index) => + index === 0 + ? {} + : { command, success: true, error: null, result: boxRect(command[2]) }, + ), + 1, + ); + }, + async () => await provider.snapshot({ includeRects: true }), + ), + (error: unknown) => + error instanceof AppError && + error.code === 'COMMAND_FAILED' && + /unusable batch result/.test(error.message), + ); + }); +}); + +test('agent-browser provider refuses geometry when an entry echoes a different command', async () => { + // The entries arrive in the order the commands were sent, so position alone would pair them. The + // echo is the proof that an entry really describes the ref it is paired with: a backend that + // reorders or answers a command it did not receive would otherwise hand each node another node's + // box, which reads as a working capture with wrong geometry. + await withWebProvider(async (provider) => { + await assert.rejects( + () => + withCommandExecutorOverride( + async (_cmd, args, options) => { + if (agentBrowserCliArgs(args)[0] !== 'batch') return snapshotPayload(); + const commands = readBatchCommands(options.stdin); + return jsonResult( + commands.map((command, index) => ({ + command: commands[(index + 1) % commands.length], + success: true, + error: null, + result: boxRect(command[2]), + })), + 1, + ); + }, + async () => await provider.snapshot({ includeRects: true }), + ), + (error: unknown) => + error instanceof AppError && + error.code === 'COMMAND_FAILED' && + /unusable batch result/.test(error.message), + ); + }); +}); + +test('agent-browser provider refuses geometry when the batch itself faults', async () => { + // A whole-batch fault comes back as one envelope rather than one entry per command, so nothing can + // be paired back to a ref; the refusal is the typed one the envelope carries. + await withWebProvider(async (provider) => { + await assert.rejects( + () => + withCommandExecutorOverride( + async (_cmd, args) => + agentBrowserCliArgs(args)[0] === 'batch' + ? jsonResult({ success: false, error: 'Invalid JSON input' }, 1) + : snapshotPayload(), + async () => await provider.snapshot({ includeRects: true }), + ), + (error: unknown) => error instanceof AppError && error.code === 'COMMAND_FAILED', + ); + }); +}); + +test('agent-browser provider surfaces a stale ref as a refusal of the geometry it was asked for', async () => { + await withWebProvider(async (provider) => { + await assert.rejects( + () => + withCommandExecutorOverride( + async (_cmd, args) => + agentBrowserCliArgs(args)[0] === 'batch' + ? jsonResult({ success: false, error: 'Stale ref @e1' }, 1) + : jsonResult({ + success: true, + data: { + refs: { e1: { role: 'button', name: 'Save' } }, + snapshot: 'button "Save" [ref=e1]', + }, + }), + async () => await provider.snapshot({ includeRects: true }), + ), + (error: unknown) => + error instanceof AppError && + error.code === 'COMMAND_FAILED' && + error.message === 'Stale ref @e1', + ); + }); +}); + +async function withWebProvider( + testFn: (provider: ReturnType) => void | Promise, +): Promise { + const stateDir = mkdtempForTestSync('agent-device-web-provider-rects-'); + try { + installFakeManagedAgentBrowser(stateDir); + await testFn(createAgentBrowserWebProvider({ stateDir, session: 'web-session' })); + } finally { + fs.rmSync(stateDir, { recursive: true, force: true }); + } +} + +/** + * The backend answers a geometry batch with one entry per command, and a ref it cannot measure comes + * back as a failed entry rather than a failed batch: `@e3` of this fixture is the element with no + * box. `@e1`/`@e2`/`@e4` report the boxes `boxRect` describes. + */ +function geometryExecutor(calls: AgentBrowserCall[]) { + return async ( + cmd: string, + args: readonly string[], + options: { allowFailure?: boolean; stdin?: string | Buffer }, + ): Promise => { + const cliArgs = recordCall(calls, cmd, args, options.stdin); + if (cliArgs[0] === 'snapshot') return snapshotPayload(); + if (cliArgs[0] === 'batch') { + const commands = readBatchCommands(options.stdin); + assert.equal(options.allowFailure, true); + assert.ok(!cliArgs.includes('--bail'), 'a bailed batch drops the refs after a failed one'); + return jsonResult( + commands.map((command) => + command.slice(0, 3).join(' ') === 'get box @e3' + ? { command, success: false, error: 'No box for element', result: null } + : { command, success: true, error: null, result: boxRect(command[2]) }, + ), + 1, + ); + } + return jsonResult({ success: true, data: {} }); + }; +} + +function assertGeometryRects(snapshot: WebSnapshotResult): void { + assert.deepEqual( + snapshot.nodes.map((node) => node.rect), + [ + { x: 1, y: 2, width: 100, height: 20 }, + { x: 11, y: 12, width: 100, height: 20 }, + undefined, + { x: 31, y: 32, width: 100, height: 20 }, + ], + ); +} + +/** The commands the provider wrote for one `batch`, which is the shape this file is about. */ +function readBatchCommands(stdin: string | Buffer | undefined): string[][] { + assert.equal(typeof stdin, 'string', 'batch reads its commands from stdin'); + return JSON.parse(String(stdin)) as string[][]; +} + +function recordCall( + calls: AgentBrowserCall[], + cmd: string, + args: readonly string[], + stdin?: string | Buffer, +): string[] { + const cliArgs = agentBrowserCliArgs(args); + const written = typeof stdin === 'string' ? stdin : undefined; + calls.push({ cmd, args: cliArgs, ...(written === undefined ? {} : { stdin: written }) }); + return cliArgs; +} + +/** + * The backend runs as `node `; that shape is owned and asserted by + * agent-browser-tool. Here it is only stripped, so these tests read as the CLI arguments they are + * about. + */ +function agentBrowserCliArgs(args: readonly string[]): string[] { + return args.slice(1); +} + +/** A capture of `count` addressable nodes, to exercise a page bigger than one batch group. */ +function manyRefSnapshotPayload(count: number): ExecResult { + const refs: Record = {}; + const lines: string[] = []; + for (let index = 1; index <= count; index += 1) { + refs[`e${index}`] = { role: 'button', name: `Item ${index}` }; + lines.push(`- button "Item ${index}" [ref=e${index}]`); + } + return jsonResult({ success: true, data: { refs, snapshot: lines.join('\n') } }); +} + +function snapshotPayload(): ExecResult { + return jsonResult({ + success: true, + data: { + refs: { + e1: { role: 'heading', name: 'Welcome' }, + e2: { role: 'textbox', name: 'Name' }, + e3: { role: 'button', name: 'Save' }, + e4: { role: 'link', name: 'Docs' }, + }, + snapshot: [ + '- heading "Welcome" [ref=e1]', + ' - textbox "Name" [ref=e2]: Ada', + ' - button "Save" [ref=e3]', + ' - link "Docs" [ref=e4]', + ].join('\n'), + truncated: false, + }, + }); +} + +/** The box the backend reports for a ref, as a batch entry carries it inline. */ +function boxRect(ref: string | undefined): { x: number; y: number; width: number; height: number } { + const offset = ref === '@e1' ? 0 : ref === '@e2' ? 10 : 30; + return { x: offset + 1, y: offset + 2, width: 100, height: 20 }; +} + +function jsonResult(value: unknown, exitCode = 0): ExecResult { + return { stdout: JSON.stringify(value), stderr: '', exitCode }; +} diff --git a/packages/platform-web/src/agent-browser-provider.test.ts b/packages/platform-web/src/agent-browser-provider.test.ts index 3a49b3277e..7c2ddfa9f9 100644 --- a/packages/platform-web/src/agent-browser-provider.test.ts +++ b/packages/platform-web/src/agent-browser-provider.test.ts @@ -196,30 +196,6 @@ test('agent-browser provider passes snapshot cancellation to the CLI process', a }); }); -test('agent-browser provider fetches snapshot rects only when requested', async () => { - await withManagedAgentBrowserProvider({ session: 'web-session' }, async (provider) => { - const calls: AgentBrowserCall[] = []; - const snapshot = await withCommandExecutorOverride( - snapshotExecutor(calls), - async () => await provider.snapshot({ includeRects: true }), - ); - - assert.deepEqual( - calls.map((call) => call.args.slice(0, 3)), - [ - ['snapshot', '--json', '--session'], - ['get', 'box', '@e1'], - ['get', 'box', '@e2'], - ['get', 'box', '@e3'], - ['get', 'box', '@e4'], - ], - ); - assertNormalizedSnapshot(snapshot); - assertSnapshotRects(snapshot); - assertRoleSelectorResolves(snapshot); - }); -}); - test('agent-browser provider dumps session network requests', async () => { await withManagedAgentBrowserProvider({ session: 'web-session' }, async (provider) => { const calls: AgentBrowserCall[] = []; @@ -405,33 +381,6 @@ test('agent-browser provider generated audio probe script samples streams discov }); }); -test('agent-browser provider surfaces stale ref failures during requested snapshot geometry lookup', async () => { - await withManagedAgentBrowserProvider({ session: 'web-session' }, async (provider) => { - await assert.rejects( - () => - withCommandExecutorOverride( - async (_cmd, args) => { - if (managedAgentBrowserCliArgs(args)[0] === 'snapshot') { - return agentBrowserJsonResult({ - success: true, - data: { - refs: { e1: { role: 'button', name: 'Save' } }, - snapshot: 'button "Save" [ref=e1]', - }, - }); - } - return agentBrowserJsonResult({ success: false, error: 'Stale ref @e1' }); - }, - async () => await provider.snapshot({ includeRects: true }), - ), - (error: unknown) => - error instanceof AppError && - error.code === 'COMMAND_FAILED' && - error.message === 'Stale ref @e1', - ); - }); -}); - test('agent-browser provider adds doctor guidance for missing binary and invalid JSON', async () => { // Pin a web-supported Node version so the missing-binary path yields the // setup hint instead of the Node upgrade hint on Node <24 hosts. @@ -505,15 +454,14 @@ function recordAgentBrowserCall( return cliArgs; } +/** + * The capture-only half of the backend's answers. The geometry the capture asks for is answered by + * `agent-browser-provider-rects.test.ts`, which owns the batch contract. + */ function snapshotExecutor(calls: AgentBrowserCall[]) { - return async (cmd: string, args: readonly string[], options: { allowFailure?: boolean }) => { + return async (cmd: string, args: readonly string[]): Promise => { const cliArgs = recordAgentBrowserCall(calls, cmd, args); if (cliArgs[0] === 'snapshot') return snapshotPayload(); - if (cliArgs.slice(0, 3).join(' ') === 'get box @e3') { - assert.equal(options.allowFailure, true); - return agentBrowserJsonResult({ success: false, error: 'No box for element' }, 1); - } - if (cliArgs[0] === 'get' && cliArgs[1] === 'box') return boxPayload(cliArgs[2]); return agentBrowserJsonResult({ success: true, data: {} }); }; } @@ -528,18 +476,6 @@ function assertNormalizedSnapshot(snapshot: WebSnapshotResult): void { ]); } -function assertSnapshotRects(snapshot: WebSnapshotResult): void { - assert.deepEqual( - snapshot.nodes.map((node) => node.rect), - [ - { x: 1, y: 2, width: 100, height: 20 }, - { x: 11, y: 12, width: 100, height: 20 }, - undefined, - { x: 31, y: 32, width: 100, height: 20 }, - ], - ); -} - function assertRoleSelectorResolves(snapshot: WebSnapshotResult): void { const nodesWithRefs = attachRefs(snapshot.nodes); const selectorChain = buildSelectorChainForNode(nodesWithRefs[2]!, 'web'); @@ -599,14 +535,6 @@ function snapshotPayload(): ExecResult { }); } -function boxPayload(ref: string | undefined): ExecResult { - const offset = ref === '@e1' ? 0 : ref === '@e2' ? 10 : 30; - return agentBrowserJsonResult({ - success: true, - data: { x: offset + 1, y: offset + 2, width: 100, height: 20 }, - }); -} - function expectedSelectAllShortcut(): string { return process.platform === 'darwin' ? 'Meta+a' : 'Control+a'; } diff --git a/packages/platform-web/src/agent-browser-provider.ts b/packages/platform-web/src/agent-browser-provider.ts index 490e9f92d4..a4c39090b1 100644 --- a/packages/platform-web/src/agent-browser-provider.ts +++ b/packages/platform-web/src/agent-browser-provider.ts @@ -46,12 +46,18 @@ type AgentBrowserProviderOptions = { ownedProcessRecords?: OwnedProcessRecordStore; }; +/** How one backend call is executed: cancellation, plus the work it reads from stdin (`batch`). */ +type AgentBrowserRunOptions = { + signal?: AbortSignal; + stdin?: string; +}; + export function createAgentBrowserWebProvider( options: AgentBrowserProviderOptions = {}, ): WebProvider { const session = options.session?.trim(); - const runJson = async (args: string[], signal?: AbortSignal): Promise => - await runAgentBrowserJson(args, { session, options, signal }); + const runJson = async (args: string[], run?: AgentBrowserRunOptions): Promise => + await runAgentBrowserJson(args, { session, options, ...run }); return { async open(target) { @@ -68,7 +74,7 @@ export function createAgentBrowserWebProvider( }, async snapshot(snapshotOptions) { return await captureAgentBrowserSnapshot( - (args) => runJson(args, snapshotOptions?.signal), + (args, run) => runJson(args, { ...run, signal: snapshotOptions?.signal }), snapshotOptions, ); }, @@ -258,13 +264,13 @@ async function movePointer(send: AgentBrowserSend, x: number, y: number): Promis } async function captureAgentBrowserSnapshot( - runJson: (args: string[]) => Promise, + runJson: (args: string[], run?: AgentBrowserRunOptions) => Promise, options: WebSnapshotOptions | undefined, ): Promise { const data = await runJson(buildSnapshotArgs(options)); return await normalizeAgentBrowserSnapshot( data, - options?.includeRects ? async (ref) => await fetchRefRect(runJson, ref) : undefined, + options?.includeRects ? (refs) => fetchRefRects(runJson, refs) : undefined, ); } @@ -283,21 +289,118 @@ function buildSnapshotArgs(options: WebSnapshotOptions | undefined): string[] { ]; } -async function fetchRefRect( - runJson: (args: string[]) => Promise, - ref: string, -): Promise { - try { - return parseRect(await runJson(['get', 'box', browserRefSelector(ref)])); - } catch (error) { - if (isIgnorableBoxError(error)) return undefined; - throw error; +/** + * Refs per backend `batch` call. The backend reads a batch from stdin as one JSON array, so the + * group is bounded by that payload rather than by a round-trip budget: measured on a 408-ref page, + * 200 refs measured in 0.28 s inside one process, so a group finishes far inside the command timeout + * the per-ref path also had — and the whole capture now costs 3 spawns instead of 381. + */ +const MAX_REFS_PER_RECT_BATCH = 200; + +/** + * The bounds of every ref of the capture, in `batch` round trips: the backend measures them inside + * one process, where the per-ref alternative pays one spawn per ref (measured on a 408-ref page: 90 s + * of spawns against 0.3 s of batching). + * + * A ref the backend could not measure comes back as a failed entry and simply gets no rect — an + * element that is absent or has no box is an ordinary answer, and the entry's own `success` field is + * what distinguishes it, never the error wording. `--bail` is deliberately not requested: it stops + * at the first failed entry, which would silently drop the geometry of every ref after it. + */ +async function fetchRefRects( + runJson: (args: string[], run?: AgentBrowserRunOptions) => Promise, + refs: readonly string[], +): Promise> { + const rects = new Map(); + for (let start = 0; start < refs.length; start += MAX_REFS_PER_RECT_BATCH) { + const group = refs.slice(start, start + MAX_REFS_PER_RECT_BATCH); + const commands = group.map((ref) => ['get', 'box', browserRefSelector(ref)]); + const entries = parseBatchEntries( + await runJson(['batch'], { stdin: JSON.stringify(commands) }), + commands, + ); + for (const [index, entry] of entries.entries()) { + const ref = group[index]; + const rect = entry.success === true ? parseRect(entry.result) : undefined; + if (rect && ref !== undefined) rects.set(ref, rect); + } } + if (refs.length > 0 && rects.size === 0) { + // An element the backend CAN see measures as a box even when it is hidden or zero-sized, so a + // capture whose every ref failed answered to a ref namespace the snapshot does not publish — + // typically refs that went stale between the snapshot and this fetch. Surfacing no geometry at + // all would leave the caller with a tree of unresolvable targets; the pre-batch code surfaced + // the failure the same way, through the per-ref error. + throw new AppError( + 'COMMAND_FAILED', + 'agent-browser resolved none of the snapshot refs while measuring snapshot geometry', + { + cmd: AGENT_BROWSER, + args: ['batch'], + reason: 'snapshot-refs-unmeasurable', + requestedRefs: refs.length, + measuredRects: 0, + hint: AGENT_BROWSER_DOCTOR_HINT, + }, + ); + } + return rects; +} + +type AgentBrowserBatchEntry = { + command?: unknown; + success?: unknown; + result?: unknown; +}; + +/** + * The backend answers a batch with one entry per command, echoing each command back, and reports a + * whole-batch fault as a single object instead of an array. The echo is what pairs an entry to the + * ref it measured: a count that disagrees, an entry that is not an object, or an entry describing + * some other command means the answer cannot be matched back, so the geometry is refused rather than + * guessed at. Position is trusted only as the order of the echo, never as the pairing itself. + */ +function parseBatchEntries( + data: unknown, + commands: readonly (readonly string[])[], +): AgentBrowserBatchEntry[] { + const unusable = (reason: string): AppError => + new AppError( + 'COMMAND_FAILED', + `agent-browser returned an unusable batch result for snapshot geometry (${reason})`, + { + cmd: AGENT_BROWSER, + args: ['batch'], + requestedRefs: commands.length, + hint: AGENT_BROWSER_DOCTOR_HINT, + }, + ); + if (!Array.isArray(data) || data.length !== commands.length) throw unusable('entry count'); + if (!data.every(isBatchEntry)) throw unusable('entry shape'); + const entries = data; + for (const [index, entry] of entries.entries()) { + if (!sameBatchCommand(entry.command, commands[index])) throw unusable('entry command'); + } + return entries; +} + +/** The echo as the backend printed it, which for these commands is exactly the array that was sent. */ +function sameBatchCommand( + echoed: unknown, + command: readonly string[] | undefined, +): echoed is readonly string[] { + return ( + command !== undefined && + Array.isArray(echoed) && + echoed.length === command.length && + echoed.every((token, index) => token === command[index]) + ); } -function isIgnorableBoxError(error: unknown): boolean { - const message = error instanceof Error ? error.message : String(error); - return !/\bstale\b/i.test(message) && /\bbox\b|not visible|not found|no element/i.test(message); +function isBatchEntry(value: unknown): value is AgentBrowserBatchEntry { + // An object with no verdict is not an answer about a ref: treated as a failed entry it would + // silently drop that node's geometry, which reads as "this element has no box". + return isJsonObject(value) && typeof value.success === 'boolean'; } async function runAgentBrowserJson( @@ -305,10 +408,9 @@ async function runAgentBrowserJson( params: { session: string | undefined; options: AgentBrowserProviderOptions; - signal?: AbortSignal; - }, + } & AgentBrowserRunOptions, ): Promise { - const { session, options, signal } = params; + const { session, options, signal, stdin } = params; const cliArgs = [...args, '--json', ...(session ? ['--session', session] : [])]; return await runAgentBrowserCommand( cliArgs, @@ -317,7 +419,7 @@ async function runAgentBrowserJson( const parsed = parseAgentBrowserJson(result.stdout, result.stderr, cliArgs, result.exitCode); return unwrapAgentBrowserJson(parsed, result, cliArgs); }, - signal, + { signal, stdin }, ); } @@ -325,7 +427,7 @@ async function runAgentBrowserCommand( cliArgs: string[], options: AgentBrowserProviderOptions, interpret: (result: { stdout: string; stderr: string; exitCode: number }) => unknown, - signal?: AbortSignal, + run: AgentBrowserRunOptions, ): Promise { let stdout = ''; let stderr = ''; @@ -339,7 +441,8 @@ async function runAgentBrowserCommand( stateDir: options.stateDir, allowFailure: true, timeoutMs: AGENT_BROWSER_TIMEOUT_MS, - signal, + signal: run.signal, + stdin: run.stdin, }); stdout = result.stdout; stderr = result.stderr; diff --git a/packages/platform-web/src/agent-browser-snapshot.ts b/packages/platform-web/src/agent-browser-snapshot.ts index 9b49271975..246c516152 100644 --- a/packages/platform-web/src/agent-browser-snapshot.ts +++ b/packages/platform-web/src/agent-browser-snapshot.ts @@ -37,17 +37,24 @@ const PRESENTATIONAL_REFLESS_ROLES = new Set(['listmarker']); */ const LABEL_ELEMENT_ROLES = new Set(['labeltext']); -const MAX_CONCURRENT_BOX_FETCHES = 8; +/** + * The bounds of every ref the capture published, in one call. + * + * Grouping the refs into backend round trips is the fetcher's decision, not the projection's: what + * the capture owes this seam is the question, and only the fetcher can weigh how much of it to send + * at once. A ref the backend could not measure is simply absent from the returned map. + */ +export type SnapshotRectFetch = (refs: readonly string[]) => Promise>; export async function normalizeAgentBrowserSnapshot( data: unknown, - fetchBox?: (ref: string) => Promise, + fetchRects?: SnapshotRectFetch, ): Promise { const snapshotText = readStringProperty(data, 'snapshot') ?? ''; const refs = collectSnapshotRefs(readProperty(data, 'refs')); const drafts = parseSnapshotDraftNodes(snapshotText, refs); - if (fetchBox) await attachDraftRects(drafts, fetchBox); + if (fetchRects) await attachDraftRects(drafts, fetchRects); return { // Preserve each draft's backend ref as the node's `ref`. agent-browser resolves @@ -70,20 +77,17 @@ export async function normalizeAgentBrowserSnapshot( async function attachDraftRects( drafts: SnapshotDraftNode[], - fetchBox: (ref: string) => Promise, + fetchRects: SnapshotRectFetch, ): Promise { // Only a node the backend issued a ref for has a box to ask for: a ref-less line is text the // backend never measured, and `get box` has no handle for it. - const addressable = drafts.flatMap((draft) => - draft.ref === undefined ? [] : [{ draft, ref: draft.ref }], - ); - for (let index = 0; index < addressable.length; index += MAX_CONCURRENT_BOX_FETCHES) { - const chunk = addressable.slice(index, index + MAX_CONCURRENT_BOX_FETCHES); - const rects = await Promise.all(chunk.map((entry) => fetchBox(entry.ref))); - for (const [chunkIndex, rect] of rects.entries()) { - const entry = chunk[chunkIndex]; - if (rect && entry) entry.draft.node.rect = rect; - } + const addressable = drafts.flatMap((draft) => (draft.ref === undefined ? [] : [draft.ref])); + if (addressable.length === 0) return; + const rects = await fetchRects(addressable); + for (const draft of drafts) { + const ref = draft.ref; + const rect = ref === undefined ? undefined : rects.get(ref); + if (rect) draft.node.rect = rect; } } diff --git a/packages/platform-web/src/agent-browser-tool.ts b/packages/platform-web/src/agent-browser-tool.ts index ec9dcb9e07..8b7744b13b 100644 --- a/packages/platform-web/src/agent-browser-tool.ts +++ b/packages/platform-web/src/agent-browser-tool.ts @@ -47,6 +47,8 @@ export type ManagedAgentBrowserRunOptions = { timeoutMs: number; allowFailure?: boolean; signal?: AbortSignal; + /** Written to the backend's stdin, for the command that reads its work from it (`batch`). */ + stdin?: string; }; /** @@ -165,6 +167,7 @@ async function spawnManagedAgentBrowser( env: managedAgentBrowserEnv(status, hostEnvironment()), timeoutMs: options.timeoutMs, signal: options.signal, + stdin: options.stdin, }); } From a4b5c3751eb6ad6c190c3e26a278a5080f70b630 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 12 Oct 2026 23:08:40 +0200 Subject: [PATCH 2/3] fix(web): key the batch refusals by what broke, and pin each one Second round on #3533. The refusals all shared one message prefix, which meant no test could tell them apart and an operator could not branch on them: each now carries a `reason` identifier (`batch-result-not-a-list`, `entry-count`, `entry-shape`, `entry-command`) beside the sentence, and the whole-batch fault is no longer reported as an `entry count` on an answer that was never a list - naming nothing counted. `requestedRefs` is the capture's ref count rather than the current group's, with `groupRefs` beside it, because the answer reads as a statement about the capture and one capture is measured in several groups. The empty-capture decision was encoded twice - an early return in the projection and `refs.length > 0` in the provider - with nothing pinning either. The provider keeps it (the loop over groups already does nothing) and a test states the consequence the projection depends on: a capture of only ref-less text sends no batch, and so cannot trip the all-unmeasurable refusal. Tests, which is where most of this lives: - The shape guard had no test that exercised it. Its fixture's bad member was `{}`, which is an object and therefore fell through to the echo check - deleting the guard kept all tests green. The member is now a non-object and the test names `entry-shape`. Mutation-checked: removing the shape guard or the list guard fails exactly one test each. - Every refusal test now asserts its own `reason` instead of the shared prefix. - The batch is the call that carries up to 200 commands and it had no cancellation assertion - a dropped signal there costs a full 30 s after the caller gave up. Pinned against the recorded `options.signal`; mutation-checked against the signal threading in `snapshot`. - `MAX_REFS_PER_RECT_BATCH`'s comment justified 200 by the stdin payload, which is not the constraint (23 bytes per ref). It is now bounded by the measurement that does decide it: a group has to finish inside `AGENT_BROWSER_TIMEOUT_MS`, and 200 refs measured in 0.28 s against 30 s. The spawn count in the same comment said 3 where the capture is 4. --- .../src/agent-browser-provider-rects.test.ts | 101 ++++++++++++++---- .../src/agent-browser-provider.ts | 41 ++++--- .../src/agent-browser-snapshot.ts | 10 +- 3 files changed, 113 insertions(+), 39 deletions(-) diff --git a/packages/platform-web/src/agent-browser-provider-rects.test.ts b/packages/platform-web/src/agent-browser-provider-rects.test.ts index 1aa3f2c94a..b378f835e9 100644 --- a/packages/platform-web/src/agent-browser-provider-rects.test.ts +++ b/packages/platform-web/src/agent-browser-provider-rects.test.ts @@ -207,8 +207,9 @@ test('agent-browser provider refuses snapshot geometry when the batch resolves n }); test('agent-browser provider refuses geometry when the batch answers with too few entries', async () => { - // Entries pair back to refs by position, so a short answer cannot be trusted: pairing four refs - // against the entries that exist would hand each node some other node's box. + // Each entry has to name the command it answers, so an answer with fewer entries than commands + // cannot be paired at all: filling four refs from the entries that exist would hand each node some + // other node's box. await withWebProvider(async (provider) => { await assert.rejects( () => @@ -223,10 +224,7 @@ test('agent-browser provider refuses geometry when the batch answers with too fe }, async () => await provider.snapshot({ includeRects: true }), ), - (error: unknown) => - error instanceof AppError && - error.code === 'COMMAND_FAILED' && - /unusable batch result/.test(error.message), + refusedFor('entry-count'), ); }); }); @@ -244,17 +242,15 @@ test('agent-browser provider refuses geometry when the batch answers with an obj : snapshotPayload(), async () => await provider.snapshot({ includeRects: true }), ), - (error: unknown) => - error instanceof AppError && - error.code === 'COMMAND_FAILED' && - /unusable batch result/.test(error.message), + refusedFor('batch-result-not-a-list'), ); }); }); test('agent-browser provider refuses geometry when a batch entry is not an entry', async () => { - // Right number of answers, one of them not a result object — an empty object carrying neither the - // command it answers nor a verdict: the positions no longer describe the refs they measured. + // Right number of answers, one of them not an object at all. The point of this test is the shape + // guard, so the member has to be one the guard alone rejects: an object would be refused further + // down by the echo check and the branch under test would go unpinned. await withWebProvider(async (provider) => { await assert.rejects( () => @@ -265,7 +261,7 @@ test('agent-browser provider refuses geometry when a batch entry is not an entry return jsonResult( commands.map((command, index) => index === 0 - ? {} + ? null : { command, success: true, error: null, result: boxRect(command[2]) }, ), 1, @@ -273,10 +269,7 @@ test('agent-browser provider refuses geometry when a batch entry is not an entry }, async () => await provider.snapshot({ includeRects: true }), ), - (error: unknown) => - error instanceof AppError && - error.code === 'COMMAND_FAILED' && - /unusable batch result/.test(error.message), + refusedFor('entry-shape'), ); }); }); @@ -305,10 +298,7 @@ test('agent-browser provider refuses geometry when an entry echoes a different c }, async () => await provider.snapshot({ includeRects: true }), ), - (error: unknown) => - error instanceof AppError && - error.code === 'COMMAND_FAILED' && - /unusable batch result/.test(error.message), + refusedFor('entry-command'), ); }); }); @@ -356,6 +346,64 @@ test('agent-browser provider surfaces a stale ref as a refusal of the geometry i }); }); +test('agent-browser provider passes snapshot cancellation to the geometry batch', async () => { + // The batch is the call that now carries up to 200 commands, so it is the one where a dropped + // signal costs a whole 30 s after the caller gave up. The capture call has its own test; this is + // the geometry half. + const controller = new AbortController(); + await withWebProvider(async (provider) => { + const signals: Array = []; + await withCommandExecutorOverride( + async (_cmd, args, options) => { + signals.push(options.signal); + if (agentBrowserCliArgs(args)[0] !== 'batch') return snapshotPayload(); + const commands = readBatchCommands(options.stdin); + return jsonResult( + commands.map((command) => ({ + command, + success: true, + error: null, + result: boxRect(command[2]), + })), + ); + }, + async () => await provider.snapshot({ includeRects: true, signal: controller.signal }), + ); + assert.equal(signals.length, 2, 'one capture call and one geometry batch'); + assert.equal( + signals[1], + controller.signal, + 'the geometry batch is cancellable by the request that asked for it', + ); + }); +}); + +test('agent-browser provider asks the backend nothing for a capture of only ref-less text', async () => { + // A capture of only ref-less text has no box to ask for. Declining is the fetcher's decision, and + // pinning it here is what keeps the all-unmeasurable refusal from firing on an empty tree. + await withWebProvider(async (provider) => { + const calls: AgentBrowserCall[] = []; + const snapshot = await withCommandExecutorOverride( + async (cmd, args) => { + recordCall(calls, cmd, args); + return jsonResult({ + success: true, + data: { refs: {}, snapshot: '- status "Hello Zed"', truncated: false }, + }); + }, + async () => await provider.snapshot({ includeRects: true }), + ); + + assert.deepEqual( + calls.map((call) => call.args[0]), + ['snapshot'], + 'no batch is sent when nothing in the capture has a ref', + ); + assert.equal(snapshot.nodes.length, 1); + assert.equal(snapshot.nodes[0]?.ref, undefined); + }); +}); + async function withWebProvider( testFn: (provider: ReturnType) => void | Promise, ): Promise { @@ -410,6 +458,17 @@ function assertGeometryRects(snapshot: WebSnapshotResult): void { ); } +/** + * The refusal this file is about, pinned to the invariant that broke. Every branch below shares the + * message prefix, so matching the prefix alone would let any of them satisfy any of these tests. + */ +function refusedFor(reason: string) { + return (error: unknown) => + error instanceof AppError && + error.code === 'COMMAND_FAILED' && + error.details?.reason === reason; +} + /** The commands the provider wrote for one `batch`, which is the shape this file is about. */ function readBatchCommands(stdin: string | Buffer | undefined): string[][] { assert.equal(typeof stdin, 'string', 'batch reads its commands from stdin'); diff --git a/packages/platform-web/src/agent-browser-provider.ts b/packages/platform-web/src/agent-browser-provider.ts index a4c39090b1..ab1f03f7e1 100644 --- a/packages/platform-web/src/agent-browser-provider.ts +++ b/packages/platform-web/src/agent-browser-provider.ts @@ -290,17 +290,18 @@ function buildSnapshotArgs(options: WebSnapshotOptions | undefined): string[] { } /** - * Refs per backend `batch` call. The backend reads a batch from stdin as one JSON array, so the - * group is bounded by that payload rather than by a round-trip budget: measured on a 408-ref page, - * 200 refs measured in 0.28 s inside one process, so a group finishes far inside the command timeout - * the per-ref path also had — and the whole capture now costs 3 spawns instead of 381. + * Refs per backend `batch` call. The bound is the group that finishes inside the command timeout it + * runs under (`AGENT_BROWSER_TIMEOUT_MS`, 30 s) with room to spare: measured on a 408-ref page, 200 + * refs measured in 0.28 s and all 408 in 0.33 s inside one process, because a command inside a batch + * is a DOM read of an already-loaded page rather than an interpreter boot. Raising the number is safe + * only while that ratio holds — the stdin payload itself (~23 bytes per ref) is not the constraint. */ const MAX_REFS_PER_RECT_BATCH = 200; /** * The bounds of every ref of the capture, in `batch` round trips: the backend measures them inside - * one process, where the per-ref alternative pays one spawn per ref (measured on a 408-ref page: 90 s - * of spawns against 0.3 s of batching). + * one process, where the per-ref alternative pays one spawn per ref (measured on a 408-ref page: one + * capture cost 385 spawns and 90 s, and 4 spawns and 0.3 s batched). * * A ref the backend could not measure comes back as a failed entry and simply gets no rect — an * element that is absent or has no box is an ordinary answer, and the entry's own `success` field is @@ -312,12 +313,16 @@ async function fetchRefRects( refs: readonly string[], ): Promise> { const rects = new Map(); + // A capture with no addressable node is an empty group list, so the loop below never runs and the + // backend is never called. That is the one place this seam declines to ask, and it is also what + // keeps such a capture from tripping the refusal: nothing failed, there was nothing to measure. for (let start = 0; start < refs.length; start += MAX_REFS_PER_RECT_BATCH) { const group = refs.slice(start, start + MAX_REFS_PER_RECT_BATCH); const commands = group.map((ref) => ['get', 'box', browserRefSelector(ref)]); const entries = parseBatchEntries( await runJson(['batch'], { stdin: JSON.stringify(commands) }), commands, + refs.length, ); for (const [index, entry] of entries.entries()) { const ref = group[index]; @@ -356,13 +361,20 @@ type AgentBrowserBatchEntry = { /** * The backend answers a batch with one entry per command, echoing each command back, and reports a * whole-batch fault as a single object instead of an array. The echo is what pairs an entry to the - * ref it measured: a count that disagrees, an entry that is not an object, or an entry describing - * some other command means the answer cannot be matched back, so the geometry is refused rather than - * guessed at. Position is trusted only as the order of the echo, never as the pairing itself. + * ref it measured, so position is trusted only as the order of the echo, never as the pairing + * itself. Anything that breaks that pairing — an answer that is not a list of entries, a list that + * does not cover the commands sent, an entry with no verdict, an entry describing some other command + * — means the geometry cannot be matched back and is refused, naming which invariant broke. + * + * Each refusal is keyed by a stable `reason` identifier rather than by its own sentence, so a caller + * branches on the failure without reading prose. `captureRefs` is the capture's whole ref count + * instead of this group's, because the answer reads as a statement about the capture and one capture + * can be measured in several groups. */ function parseBatchEntries( data: unknown, commands: readonly (readonly string[])[], + captureRefs: number, ): AgentBrowserBatchEntry[] { const unusable = (reason: string): AppError => new AppError( @@ -371,15 +383,18 @@ function parseBatchEntries( { cmd: AGENT_BROWSER, args: ['batch'], - requestedRefs: commands.length, + reason, + requestedRefs: captureRefs, + groupRefs: commands.length, hint: AGENT_BROWSER_DOCTOR_HINT, }, ); - if (!Array.isArray(data) || data.length !== commands.length) throw unusable('entry count'); - if (!data.every(isBatchEntry)) throw unusable('entry shape'); + if (!Array.isArray(data)) throw unusable('batch-result-not-a-list'); + if (data.length !== commands.length) throw unusable('entry-count'); + if (!data.every(isBatchEntry)) throw unusable('entry-shape'); const entries = data; for (const [index, entry] of entries.entries()) { - if (!sameBatchCommand(entry.command, commands[index])) throw unusable('entry command'); + if (!sameBatchCommand(entry.command, commands[index])) throw unusable('entry-command'); } return entries; } diff --git a/packages/platform-web/src/agent-browser-snapshot.ts b/packages/platform-web/src/agent-browser-snapshot.ts index 246c516152..d6f6ff2cfc 100644 --- a/packages/platform-web/src/agent-browser-snapshot.ts +++ b/packages/platform-web/src/agent-browser-snapshot.ts @@ -79,11 +79,11 @@ async function attachDraftRects( drafts: SnapshotDraftNode[], fetchRects: SnapshotRectFetch, ): Promise { - // Only a node the backend issued a ref for has a box to ask for: a ref-less line is text the - // backend never measured, and `get box` has no handle for it. - const addressable = drafts.flatMap((draft) => (draft.ref === undefined ? [] : [draft.ref])); - if (addressable.length === 0) return; - const rects = await fetchRects(addressable); + // One fetch for the whole capture. A capture of nothing addressable still comes through, and the + // fetcher decides not to call the backend for it: that decision belongs with the refusal below. + const rects = await fetchRects( + drafts.flatMap((draft) => (draft.ref === undefined ? [] : [draft.ref])), + ); for (const draft of drafts) { const ref = draft.ref; const rect = ref === undefined ? undefined : rects.get(ref); From 72386de6e0a41a890b28623035dd250926312f9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 13 Oct 2026 11:38:33 +0200 Subject: [PATCH 3/3] test(web): pin the entry-shape refusal on an entry that states no verdict The only entry-shape case sent a null member, which plain isJsonObject already rejected, so isBatchEntry's boolean-success clause had no test that needed it: revert the clause and an entry echoing the right command with a box but no verdict passes both the shape check and the echo check, then drops that node's rect in silence - the read being 'this element has no box'. The new member echoes commands[0] on purpose, because that is the one shape only the success clause refuses. Mutation-checked: isBatchEntry back to isJsonObject fails this case alone and leaves the other 13 green. The old comment claimed such an object would be refused further down by the echo check, which is what the new case disproves. --- .../src/agent-browser-provider-rects.test.ts | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/platform-web/src/agent-browser-provider-rects.test.ts b/packages/platform-web/src/agent-browser-provider-rects.test.ts index b378f835e9..68f63bdca0 100644 --- a/packages/platform-web/src/agent-browser-provider-rects.test.ts +++ b/packages/platform-web/src/agent-browser-provider-rects.test.ts @@ -247,10 +247,9 @@ test('agent-browser provider refuses geometry when the batch answers with an obj }); }); -test('agent-browser provider refuses geometry when a batch entry is not an entry', async () => { - // Right number of answers, one of them not an object at all. The point of this test is the shape - // guard, so the member has to be one the guard alone rejects: an object would be refused further - // down by the echo check and the branch under test would go unpinned. +test('agent-browser provider refuses geometry when a batch entry is not an object', async () => { + // Right number of answers, one of them not an object at all. A member that is not an object never + // reaches the echo check, so only the shape guard can refuse it. await withWebProvider(async (provider) => { await assert.rejects( () => @@ -274,6 +273,36 @@ test('agent-browser provider refuses geometry when a batch entry is not an entry }); }); +test('agent-browser provider refuses geometry when a batch entry carries no verdict', async () => { + // An entry that echoes the right command and carries a box but states no `success` is the shape + // the echo check cannot catch: it passes the echo and would be read as an ordinary failed ref, + // dropping that node's geometry with no error so it reads as "this element has no box". Only the + // shape guard's boolean-`success` clause refuses it. + await withWebProvider(async (provider) => { + await assert.rejects( + () => + withCommandExecutorOverride( + async (_cmd, args, options) => { + if (agentBrowserCliArgs(args)[0] !== 'batch') return snapshotPayload(); + const commands = readBatchCommands(options.stdin); + return jsonResult( + commands.map((command, index) => + // index 0 echoes the command it answers (`command` IS `commands[0]` there) and + // carries a plausible box, so only the missing `success` distinguishes it. + index === 0 + ? { command, result: boxRect(command[2]) } + : { command, success: true, error: null, result: boxRect(command[2]) }, + ), + 1, + ); + }, + async () => await provider.snapshot({ includeRects: true }), + ), + refusedFor('entry-shape'), + ); + }); +}); + test('agent-browser provider refuses geometry when an entry echoes a different command', async () => { // The entries arrive in the order the commands were sent, so position alone would pair them. The // echo is the proof that an entry really describes the ref it is paired with: a backend that