mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(browser): stop an abandoned orca eval from blocking later commands on its page (#35284)
* fix(browser): free the page queue when an eval's caller disconnects A local orca eval whose caller gave up (CLI timeout, Ctrl-C) kept the page's serial command queue until its script settled, so a never-settling script blocked every later browser command on that page. The unix socket now forwards its disconnect signal to browser.eval, and the eval stops waiting on abort; the abandoned script keeps running. * test(runtime): pin local disconnect forwarding for browser.eval * refactor(runtime): name the methods that get the local disconnect signal
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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<BrowserEvalResult> {
|
||||
// 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 } }
|
||||
|
||||
@@ -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'))
|
||||
|
||||
@@ -70,11 +70,12 @@ export class RuntimeBrowserCapture {
|
||||
}
|
||||
|
||||
async browserEval(
|
||||
params: { expression: string } & BrowserCommandTargetParams
|
||||
params: { expression: string } & BrowserCommandTargetParams,
|
||||
signal?: AbortSignal
|
||||
): Promise<BrowserEvalResult> {
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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',
|
||||
|
||||
+2
-2
@@ -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(),
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user