diff --git a/config/scripts/ci-shard-timings.json b/config/scripts/ci-shard-timings.json index 5cc632315a7..c9139da2205 100644 --- a/config/scripts/ci-shard-timings.json +++ b/config/scripts/ci-shard-timings.json @@ -5773,7 +5773,7 @@ "src/main/runtime/runtime-rpc-device-key-dispatch.test.ts": 3022, "src/main/runtime/runtime-rpc-device-revocation.test.ts": 4229, "src/main/runtime/runtime-rpc-dispatch-inject-long-poll.test.ts": 2899, - "src/main/runtime/runtime-rpc-linear-comment-cancellation.test.ts": 2559, + "src/main/runtime/runtime-rpc-local-disconnect-forwarding.test.ts": 2559, "src/main/runtime/runtime-rpc-long-poll-transport.test.ts": 23796, "src/main/runtime/runtime-rpc-metadata-lifecycle.test.ts": 2618, "src/main/runtime/runtime-rpc-mobile-legacy-socket-relay-refusal.test.ts": 2701, diff --git a/src/main/browser/agent-browser-bridge-capture-commands.ts b/src/main/browser/agent-browser-bridge-capture-commands.ts index 4c7708c20f2..7e8f55ee8ff 100644 --- a/src/main/browser/agent-browser-bridge-capture-commands.ts +++ b/src/main/browser/agent-browser-bridge-capture-commands.ts @@ -7,6 +7,7 @@ import { sendGuestCdpCommand } from './guest-cdp-command' import { AgentBrowserBridgeUtilityCommands } from './agent-browser-bridge-utility-commands' import { ORCA_TAB_SESSION_PREFIX } from './agent-browser-orphan-sweep' import { captureRemoteCliBrowserWorkBound } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' +import { throwIfSignalAborted, waitForPromiseWithSignal } from '../../shared/abort-signal-reason' export abstract class AgentBrowserBridgeCaptureCommands extends AgentBrowserBridgeUtilityCommands { async screenshot( @@ -59,7 +60,8 @@ export abstract class AgentBrowserBridgeCaptureCommands extends AgentBrowserBrid async evaluate( expression: string, worktreeId?: string, - browserPageId?: string + browserPageId?: string, + signal?: AbortSignal ): Promise { // Capture the host's scope before the queue can run this command elsewhere. const bound = captureRemoteCliBrowserWorkBound() @@ -67,18 +69,23 @@ export abstract class AgentBrowserBridgeCaptureCommands extends AgentBrowserBrid worktreeId, browserPageId, async (_sessionName, target) => { + throwIfSignalAborted(signal) const wc = this.requireTargetWebContents(target) let releaseDebugger = (): void => {} try { releaseDebugger = acquireElectronDebugger(wc).release + // A caller leaving frees the page queue but does not terminate its script. // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Runtime.evaluate with returnByValue always answers this CDP shape; unchanged from before the bound. - const { result, exceptionDetails } = (await bound( - sendGuestCdpCommand(wc, 'Runtime.evaluate', { - expression, - returnByValue: true, - awaitPromise: true - }), - () => void wc.debugger.sendCommand('Runtime.terminateExecution').catch(() => {}) + const { result, exceptionDetails } = (await waitForPromiseWithSignal( + bound( + sendGuestCdpCommand(wc, 'Runtime.evaluate', { + expression, + returnByValue: true, + awaitPromise: true + }), + () => void wc.debugger.sendCommand('Runtime.terminateExecution').catch(() => {}) + ), + signal )) as { result: { value?: unknown; description?: string } exceptionDetails?: { text: string; exception?: { description?: string } } diff --git a/src/main/browser/agent-browser-bridge-navigation.test.ts b/src/main/browser/agent-browser-bridge-navigation.test.ts index 93b88097792..f71dafa6afb 100644 --- a/src/main/browser/agent-browser-bridge-navigation.test.ts +++ b/src/main/browser/agent-browser-bridge-navigation.test.ts @@ -416,6 +416,30 @@ describe('AgentBrowserBridge', () => { expect(execFileMock).not.toHaveBeenCalled() }) + it('frees the page queue when callers leave a never-settling evaluation', async () => { + const wc = mockWebContents(100) + wc.debugger.sendCommand + .mockReturnValueOnce(new Promise(() => {})) + .mockResolvedValue({ result: { value: 'example.com' } }) + webContentsFromIdMock.mockReturnValue(wc) + const hungCaller = new AbortController() + const queuedCaller = new AbortController() + + const hung = bridge.evaluate('new Promise(() => {})', undefined, undefined, hungCaller.signal) + const queued = bridge.evaluate('sideEffect()', undefined, undefined, queuedCaller.signal) + const next = bridge.evaluate('location.host') + queuedCaller.abort() + hungCaller.abort() + + await expect(hung).rejects.toThrow() + await expect(queued).rejects.toThrow() + await expect(next).resolves.toMatchObject({ result: 'example.com' }) + expect(wc.debugger.sendCommand).not.toHaveBeenCalledWith( + 'Runtime.evaluate', + expect.objectContaining({ expression: 'sideEffect()' }) + ) + }) + it('fails closed when the registered webContents debugger is stale', async () => { const wc = mockWebContents(100) wc.debugger.sendCommand.mockRejectedValue(new Error('Debugger is detached')) diff --git a/src/main/runtime/browser/browser-capture.ts b/src/main/runtime/browser/browser-capture.ts index 6b93a994d1b..b05cbbae929 100644 --- a/src/main/runtime/browser/browser-capture.ts +++ b/src/main/runtime/browser/browser-capture.ts @@ -70,11 +70,12 @@ export class RuntimeBrowserCapture { } async browserEval( - params: { expression: string } & BrowserCommandTargetParams + params: { expression: string } & BrowserCommandTargetParams, + signal?: AbortSignal ): Promise { const target = await this.targets.resolveBrowserCommandTarget(params) return this.targets .requireAgentBrowserBridge() - .evaluate(params.expression, target.worktreeId, target.browserPageId) + .evaluate(params.expression, target.worktreeId, target.browserPageId, signal) } } diff --git a/src/main/runtime/rpc/methods/browser-core.ts b/src/main/runtime/rpc/methods/browser-core.ts index c354a55b1b7..cc3c4e17ec9 100644 --- a/src/main/runtime/rpc/methods/browser-core.ts +++ b/src/main/runtime/rpc/methods/browser-core.ts @@ -94,7 +94,7 @@ export const BROWSER_CORE_METHODS = [ name: 'browser.eval', permission: 'workspace', params: Eval, - handler: async (params, { runtime }) => runtime.browserEval(params) + handler: async (params, { runtime, signal }) => runtime.browserEval(params, signal) }), defineMethod({ name: 'browser.tabList', diff --git a/src/main/runtime/runtime-rpc-linear-comment-cancellation.test.ts b/src/main/runtime/runtime-rpc-local-disconnect-forwarding.test.ts similarity index 91% rename from src/main/runtime/runtime-rpc-linear-comment-cancellation.test.ts rename to src/main/runtime/runtime-rpc-local-disconnect-forwarding.test.ts index b5939429e13..504b51c6201 100644 --- a/src/main/runtime/runtime-rpc-linear-comment-cancellation.test.ts +++ b/src/main/runtime/runtime-rpc-local-disconnect-forwarding.test.ts @@ -7,10 +7,10 @@ import { OrcaRuntimeService } from './orca-runtime' import { defineMethod } from './rpc/core' import { OrcaRuntimeRpcServer } from './runtime-rpc' -it.each(['linear.commentEdit', 'linear.commentDelete'] as const)( +it.each(['browser.eval', 'linear.commentEdit', 'linear.commentDelete'] as const)( 'forwards local disconnect to %s without long-poll keepalives', async (method) => { - const userDataPath = mkdtempSync(join(tmpdir(), 'pr-30003-cancellation-')) + const userDataPath = mkdtempSync(join(tmpdir(), 'local-disconnect-forwarding-')) const entered = vi.fn() const server = new OrcaRuntimeRpcServer({ runtime: new OrcaRuntimeService(), diff --git a/src/main/runtime/runtime-rpc/runtime-rpc-request-admission.ts b/src/main/runtime/runtime-rpc/runtime-rpc-request-admission.ts index ebfa391e32c..e4c7999a4d0 100644 --- a/src/main/runtime/runtime-rpc/runtime-rpc-request-admission.ts +++ b/src/main/runtime/runtime-rpc/runtime-rpc-request-admission.ts @@ -7,6 +7,13 @@ import { RuntimeRpcBinaryRouting } from './runtime-rpc-binary-routing' import { classifyRuntimeLongPoll, type RuntimeLongPollClass } from './runtime-rpc-long-poll' import { OWNER_RPC_CALLER_SCOPE, type RpcCallerScope } from '../rpc/rpc-caller-scope' +// Short calls that stop on disconnect without long-poll keepalives or admission slots. +const DISCONNECT_FORWARDED_METHODS = new Set([ + 'browser.eval', + 'linear.commentEdit', + 'linear.commentDelete' +]) + export class RuntimeRpcRequestAdmission extends RuntimeRpcBinaryRouting { // One-shot Unix requests use shared tokens from mode-0600 metadata (design ยง3.1). protected async handleMessage( @@ -38,9 +45,7 @@ export class RuntimeRpcRequestAdmission extends RuntimeRpcBinaryRouting { try { return await this.dispatcher.dispatch(request, { signal: - longPoll || - request.method === 'linear.commentEdit' || - request.method === 'linear.commentDelete' + longPoll || DISCONNECT_FORWARDED_METHODS.has(request.method) ? context?.signal : undefined, callerScope