From 5834a3dcfdebe33a04ded1d5ba9541dcfd22b910 Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:15:04 +0000 Subject: [PATCH] fix(browser): agent clicks keep their target when a page changes, by upgrading agent-browser to 0.39.4 (#36007) * chore(browser): upgrade bundled agent-browser to 0.39.4 0.39.4 keeps refs stable across snapshots and refuses stale or hidden click targets. Its replies now carry daemon lifecycle bookkeeping, which is dropped where the JSON is parsed, and its tarball binaries lose the executable bit pnpm's skipped postinstall used to restore, so the orcad dev resolver now restores it like the desktop one. * fix(browser): keep orcad's Chromium unbounded and read 0.39's stale-ref error * fix(browser): import the shared stale-ref rule in orcad and keep the parser typed * fix(browser): keep workers and frames running after the agent-browser helper leaves agent-browser 0.39 asks auto-attached targets to wait for it before starting. The proxy forwards that onto the guest's shared debugger, which outlives the helper, so after the helper's idle exit nothing resumed new workers or frames. --- package.json | 2 +- pnpm-lock.yaml | 25 +++++++++++-------- .../browser/agent-browser-batch-command.ts | 23 +++++++++++++++-- ...t-browser-bridge-command-transport.test.ts | 16 ++++++++++++ .../browser/agent-browser-bridge-process.ts | 14 ++++------- .../agent-browser-dependency-binary.ts | 13 ++++++++++ src/main/browser/cdp-ws-proxy.test.ts | 20 +++++++++++++++ src/main/browser/cdp-ws-proxy.ts | 6 +++++ .../external-chromium-browser-session.test.ts | 22 +++++++++++++++- .../external-chromium-browser-session.ts | 6 ++++- src/main/orcad/orcad-agent-browser-binary.ts | 10 +++++--- 11 files changed, 129 insertions(+), 28 deletions(-) create mode 100644 src/main/browser/agent-browser-dependency-binary.ts diff --git a/package.json b/package.json index a0c63fadf3f..7cf074f2ec5 100644 --- a/package.json +++ b/package.json @@ -227,7 +227,7 @@ "@parcel/watcher": "^2.5.6", "@xterm/addon-serialize": "0.15.0-beta.301", "@xterm/headless": "6.1.0-beta.303", - "agent-browser": "~0.27.3", + "agent-browser": "~0.39.4", "electron-updater": "^6.8.10", "fflate": "0.8.3", "i18next": "^26.5.11", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9f3762426fb..a54699ff06d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -216,8 +216,8 @@ importers: specifier: 6.1.0-beta.303 version: 6.1.0-beta.303 agent-browser: - specifier: ~0.27.3 - version: 0.27.3 + specifier: ~0.39.4 + version: 0.39.4 electron-updater: specifier: ^6.8.10 version: 6.8.10(electron@43.8.1(supports-color@7.2.0))(supports-color@7.2.0) @@ -480,12 +480,12 @@ importers: hast-util-from-html-isomorphic: specifier: 2.0.0 version: 2.0.0 - hast-util-to-text: - specifier: 4.0.2 - version: 4.0.2 hast-util-to-jsx-runtime: specifier: 2.3.6 version: 2.3.6(supports-color@7.2.0) + hast-util-to-text: + specifier: 4.0.2 + version: 4.0.2 html-to-image: specifier: ^1.11.13 version: 1.11.13 @@ -4423,8 +4423,8 @@ packages: resolution: {integrity: sha512-MnA+YT8fwfJPgBx3m60MNqakm30XOkyIoH1y6huTQvC0PwZG7ki8NacLBcrPbNoo8vEZy7Jpuk7+jMO+CUovTQ==} engines: {node: '>= 14'} - agent-browser@0.27.3: - resolution: {integrity: sha512-isQ1xWoYOnfR8BigYRlYmkFOrqw71tKEmUEMmFJGStQFOyCrLSsKoOAZEu8TjPZdmjD1nPQ/hLLYlxTwdHnA0Q==} + agent-browser@0.39.4: + resolution: {integrity: sha512-R0ue5CWlpkGtbcXeq3ZKM96zLrCXkNKEGyttgWhi0eSNKoNSvxglvv1/D9NhKxe+MI+djiCFepPd4PYOM0H2Rg==} engines: {node: '>=24.0.0', pnpm: '>=11.0.0'} hasBin: true @@ -4432,9 +4432,12 @@ packages: resolution: {integrity: sha512-8iUql50EUR+uUcdRQ3HDqa6EVyo3docL8g5WJ3FNcWmu62IbkGUue/pEyLBW8VGKKucTPgqeks4fIU1DA4yowQ==} peerDependencies: '@types/ajv': '*' + ajv: ^8.0.0 peerDependenciesMeta: '@types/ajv': optional: true + ajv: + optional: true ajv@6.15.0: resolution: {integrity: sha512-fgFx7Hfoq60ytK2c7DhnF8jIvzYgOMxfugjLOSMHjLIPgenqa7S7oaagATUq99mV6IYvN2tRmC0wnTYX6iPbMw==} @@ -8839,7 +8842,7 @@ snapshots: dependencies: '@hono/node-server': 2.1.0(hono@4.13.11) ajv: 8.20.0 - ajv-formats: 3.0.1 + ajv-formats: 3.0.1(ajv@8.20.0) content-type: 1.0.5 cors: 2.8.6 cross-spawn: 7.0.6 @@ -11137,10 +11140,10 @@ snapshots: agent-base@7.1.4: {} - agent-browser@0.27.3: {} + agent-browser@0.39.4: {} - ajv-formats@3.0.1: - dependencies: + ajv-formats@3.0.1(ajv@8.20.0): + optionalDependencies: ajv: 8.20.0 ajv@6.15.0: diff --git a/src/main/browser/agent-browser-batch-command.ts b/src/main/browser/agent-browser-batch-command.ts index 64bc8a6c5c0..b5f745bf0ee 100644 --- a/src/main/browser/agent-browser-batch-command.ts +++ b/src/main/browser/agent-browser-batch-command.ts @@ -14,8 +14,27 @@ export function parseAgentBrowserJsonOutput(stdout: string): AgentBrowserEnvelop const parsed = JSON.parse(stdout) // Orca batches one command; a longer array is a passthrough `exec batch` reply. if (!Array.isArray(parsed) || parsed.length !== 1) { - return parsed + return withoutDaemonLifecycle(parsed) } const [entry] = parsed - return { success: entry?.success, data: entry?.result, error: entry?.error } + return withoutDaemonLifecycle({ + success: entry?.success, + data: entry?.result, + error: entry?.error + }) +} + +// agent-browser 0.31+ adds its daemon's launch bookkeeping to every reply; it is not page data. +function withoutDaemonLifecycle(envelope: AgentBrowserEnvelope): AgentBrowserEnvelope { + const data = envelope?.data + if (!data || typeof data !== 'object' || Array.isArray(data) || !('lifecycle' in data)) { + return envelope + } + const { lifecycle: _lifecycle, ...rest } = data + return { ...envelope, data: rest } +} + +/** "Unknown ref" for a ref never issued; 0.39+ also says "Stale ref for role=…" once its element is gone. */ +export function isAgentBrowserStaleRefError(message: string): boolean { + return /unknown ref|stale ref|ref not found|element not found: @e/i.test(message) } diff --git a/src/main/browser/agent-browser-bridge-command-transport.test.ts b/src/main/browser/agent-browser-bridge-command-transport.test.ts index 111f8e3a1d1..3e838bdaf11 100644 --- a/src/main/browser/agent-browser-bridge-command-transport.test.ts +++ b/src/main/browser/agent-browser-bridge-command-transport.test.ts @@ -202,6 +202,22 @@ describe('AgentBrowserBridge', () => { await expect(bridge.snapshot()).rejects.toThrow('Command failed') }) + it('drops the daemon lifecycle agent-browser adds to every result', async () => { + // Real agent-browser 0.39.4 batch reply. + const reply = + '[{"command":["click","@e2"],"error":null,"result":{"clicked":"@e2","lifecycle":{"effectiveLaunch":{"browserLaunched":true,"engine":"chrome","launchHash":520248349195465926},"launched":false,"relaunchedBrowser":false,"restartedBackground":false,"restoreStatus":"not_configured","reused":true,"saveStatus":"not_attempted"}},"success":true}]' + answerAgentBrowserCommands(execFileMock, stdinWrites, (_command, cb) => cb(null, reply, '')) + await expect(bridge.click('@e2')).resolves.toEqual({ clicked: '@e2' }) + }) + + it('reports a ref whose element is gone as stale', async () => { + // agent-browser 0.39.2+ wording when it cannot re-resolve a ref by role and name. + failWith( + 'Stale ref for role=button name=Approve cannot be recovered: several elements now match. Run snapshot again and use a fresh ref.' + ) + await expect(bridge.click('@e2')).rejects.toMatchObject({ code: 'browser_stale_ref' }) + }) + it('returns browser_error with truncated output for malformed JSON', async () => { execFileMock.mockImplementation( (_bin: string, _args: string[], _opts: unknown, cb: ExecFileCallback) => { diff --git a/src/main/browser/agent-browser-bridge-process.ts b/src/main/browser/agent-browser-bridge-process.ts index 70c097244d7..275c3f8a9ed 100644 --- a/src/main/browser/agent-browser-bridge-process.ts +++ b/src/main/browser/agent-browser-bridge-process.ts @@ -1,11 +1,13 @@ import { app } from 'electron' -import { existsSync, accessSync, chmodSync, constants } from 'node:fs' +import { existsSync } from 'node:fs' import { join } from 'node:path' import { platform, arch } from 'node:os' import type { WebContents } from 'electron' import { BrowserError } from './cdp-bridge' import { ORCA_TAB_SESSION_PREFIX } from './agent-browser-orphan-sweep' import { EMBEDDED_NAVIGATION_TIMEOUT_MS } from './agent-browser-bridge-types' +import { ensureDependencyBinaryExecutable } from './agent-browser-dependency-binary' +import { isAgentBrowserStaleRefError } from './agent-browser-batch-command' export function agentBrowserNativeName(): string { const ext = process.platform === 'win32' ? '.exe' : '' @@ -33,13 +35,7 @@ export function resolveAgentBrowserBinary(): string { agentBrowserNativeName() ) if (existsSync(nmBin)) { - if (process.platform !== 'win32') { - try { - accessSync(nmBin, constants.X_OK) - } catch { - chmodSync(nmBin, 0o755) - } - } + ensureDependencyBinaryExecutable(nmBin) return nmBin } @@ -91,7 +87,7 @@ export function stripAgentBrowserTargetArgs(args: string[]): string[] { // Stale refs need a distinct code so agents can retry with a fresh snapshot. export function classifyErrorCode(message: string): string { - if (/unknown ref|ref not found|element not found: @e/i.test(message)) { + if (isAgentBrowserStaleRefError(message)) { return 'browser_stale_ref' } return 'browser_error' diff --git a/src/main/browser/agent-browser-dependency-binary.ts b/src/main/browser/agent-browser-dependency-binary.ts new file mode 100644 index 00000000000..18bce0e27f1 --- /dev/null +++ b/src/main/browser/agent-browser-dependency-binary.ts @@ -0,0 +1,13 @@ +import { accessSync, chmodSync, constants } from 'node:fs' + +/** pnpm skips agent-browser's postinstall, which is what marks its tarball binaries executable. */ +export function ensureDependencyBinaryExecutable(path: string): void { + if (process.platform === 'win32') { + return + } + try { + accessSync(path, constants.X_OK) + } catch { + chmodSync(path, 0o755) + } +} diff --git a/src/main/browser/cdp-ws-proxy.test.ts b/src/main/browser/cdp-ws-proxy.test.ts index 4d16c97ac4a..f94e9f8fdb4 100644 --- a/src/main/browser/cdp-ws-proxy.test.ts +++ b/src/main/browser/cdp-ws-proxy.test.ts @@ -340,6 +340,26 @@ describe('CdpWsProxy', () => { client.close() }) + it('never leaves auto-attached workers and frames waiting on a client that may leave', async () => { + mock.webContents.debugger.sendCommand.mockResolvedValueOnce({}) + + const client = await connect(endpoint) + // agent-browser 0.39 asks child targets to wait for it; the shared guest debugger outlives it. + await sendAndReceive(client, { + id: 1, + method: 'Target.setAutoAttach', + params: { autoAttach: true, waitForDebuggerOnStart: true, flatten: true }, + sessionId: 'oopif-session-123' + }) + + expect(mock.webContents.debugger.sendCommand).toHaveBeenCalledWith( + 'Target.setAutoAttach', + { autoAttach: true, waitForDebuggerOnStart: false, flatten: true }, + 'oopif-session-123' + ) + client.close() + }) + it('forwards CDP events from debugger to client', async () => { const client = await connect(endpoint) diff --git a/src/main/browser/cdp-ws-proxy.ts b/src/main/browser/cdp-ws-proxy.ts index fa36662412c..0d3934d09e0 100644 --- a/src/main/browser/cdp-ws-proxy.ts +++ b/src/main/browser/cdp-ws-proxy.ts @@ -297,6 +297,12 @@ export class CdpWsProxy { void this.navigation.reloadWithLifecycle(client, clientId, msg.params ?? {}, msg.sessionId) return } + // The guest debugger outlives this client, so nothing would resume workers or frames left waiting. + if (msg.method === 'Target.setAutoAttach') { + const params = { ...msg.params, waitForDebuggerOnStart: false } + this.debuggerChannel.forwardCommand(client, clientId, msg.method, params, msg.sessionId) + return + } this.debuggerChannel.forwardCommand( client, clientId, diff --git a/src/main/orcad/external-chromium-browser-session.test.ts b/src/main/orcad/external-chromium-browser-session.test.ts index 0dd02cabdc5..300874fb7de 100644 --- a/src/main/orcad/external-chromium-browser-session.test.ts +++ b/src/main/orcad/external-chromium-browser-session.test.ts @@ -93,10 +93,30 @@ describe('orcad external-chromium agent-browser environment', () => { } ) + it('reports a ref whose element is gone as stale', async () => { + runProcessMock.mockResolvedValue({ + code: 1, + signal: null, + stdout: JSON.stringify({ + success: false, + error: + 'Stale ref for role=button name=Approve cannot be recovered: several elements now match.' + }), + stderr: '', + timedOut: false + }) + const session = new ExternalChromiumBrowserSession( + '/opt/orca/agent-browser', + { executablePath: BASE.executablePath, provider: 'chromium' }, + '/state' + ) + await expect(session.run(['click', '@e2'])).rejects.toMatchObject({ code: 'browser_stale_ref' }) + }) + it('never bounds the daemon that owns the Chromium tree', () => { const env = externalChromiumAgentBrowserEnvironment({ inheritedEnv: {}, ...BASE }) - expect(env.AGENT_BROWSER_IDLE_TIMEOUT_MS).toBeUndefined() + expect(env.AGENT_BROWSER_IDLE_TIMEOUT_MS).toBe('0') expect(env.AGENT_BROWSER_EXECUTABLE_PATH).toBe(BASE.executablePath) expect(env.AGENT_BROWSER_SESSION).toBe(BASE.sessionName) }) diff --git a/src/main/orcad/external-chromium-browser-session.ts b/src/main/orcad/external-chromium-browser-session.ts index c5f89db6d60..0df599dc033 100644 --- a/src/main/orcad/external-chromium-browser-session.ts +++ b/src/main/orcad/external-chromium-browser-session.ts @@ -6,6 +6,7 @@ import { BrowserError } from '../browser/browser-error' import { AGENT_BROWSER_BATCH_ARGS, agentBrowserBatchInput, + isAgentBrowserStaleRefError, parseAgentBrowserJsonOutput } from '../browser/agent-browser-batch-command' import { BROWSER_UNAVAILABLE_ERROR_CODE } from '../../shared/runtime-types' @@ -38,7 +39,7 @@ const AgentBrowserEnvelope = z.object({ export type AgentBrowserTab = z.infer function classifyAgentBrowserError(message: string): string { - if (/unknown ref|ref not found|element not found: @e/i.test(message)) { + if (isAgentBrowserStaleRefError(message)) { return 'browser_stale_ref' } if (/evaluation error/i.test(message)) { @@ -57,6 +58,7 @@ function classifyAgentBrowserError(message: string): string { } // No idle timeout: this daemon owns the user’s remote Chromium and all its live tabs. +// agent-browser 0.33+ reads an unset timeout as one hour, so only "0" keeps it unbounded. export function externalChromiumAgentBrowserEnvironment(options: { inheritedEnv: NodeJS.ProcessEnv executablePath: string @@ -66,6 +68,8 @@ export function externalChromiumAgentBrowserEnvironment(options: { }): NodeJS.ProcessEnv { return { ...options.inheritedEnv, + AGENT_BROWSER_IDLE_TIMEOUT_MS: + options.inheritedEnv.AGENT_BROWSER_IDLE_TIMEOUT_MS?.trim() || '0', AGENT_BROWSER_EXECUTABLE_PATH: options.executablePath, AGENT_BROWSER_PROFILE: options.profilePath, AGENT_BROWSER_SESSION: options.sessionName, diff --git a/src/main/orcad/orcad-agent-browser-binary.ts b/src/main/orcad/orcad-agent-browser-binary.ts index 932578c4596..833329b1181 100644 --- a/src/main/orcad/orcad-agent-browser-binary.ts +++ b/src/main/orcad/orcad-agent-browser-binary.ts @@ -2,6 +2,7 @@ import { accessSync, constants, existsSync } from 'node:fs' import { arch, platform } from 'node:os' import { dirname, join } from 'node:path' import { orcadAgentBrowserNativeName } from '../../shared/orcad-agent-browser-name' +import { ensureDependencyBinaryExecutable } from '../browser/agent-browser-dependency-binary' /** A slot carries only its own target's driver, so trying both libc names needs no libc probe. */ function hostDriverNames(): string[] { @@ -14,14 +15,17 @@ function hostDriverNames(): string[] { export function resolveOrcadAgentBrowserBinary(): string | null { const candidates = hostDriverNames().flatMap((name) => [ - join(dirname(process.argv[1] ?? __filename), name), - join(process.cwd(), 'node_modules', 'agent-browser', 'bin', name) + { path: join(dirname(process.argv[1] ?? __filename), name), dependency: false }, + { path: join(process.cwd(), 'node_modules', 'agent-browser', 'bin', name), dependency: true } ]) - for (const candidate of candidates) { + for (const { path: candidate, dependency } of candidates) { if (!existsSync(candidate)) { continue } try { + if (dependency) { + ensureDependencyBinaryExecutable(candidate) + } accessSync(candidate, process.platform === 'win32' ? constants.F_OK : constants.X_OK) return candidate } catch {