diff --git a/src/main/browser/agent-browser-bridge-execution.ts b/src/main/browser/agent-browser-bridge-execution.ts index 31f5a191ede4..9c09d1f5fc19 100644 --- a/src/main/browser/agent-browser-bridge-execution.ts +++ b/src/main/browser/agent-browser-bridge-execution.ts @@ -11,6 +11,7 @@ import { } from './agent-browser-bridge-process' import { translateResult } from './agent-browser-bridge-result' import { AgentBrowserBridgeTabs } from './agent-browser-bridge-tabs' +import { assertRemoteCliBrowserAccess } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' import { ORCA_TAB_SESSION_PREFIX } from './agent-browser-orphan-sweep' import { canSkipAgentBrowserSessionReset } from './agent-browser-session-reset' import { @@ -83,6 +84,8 @@ export abstract class AgentBrowserBridgeExecution extends AgentBrowserBridgeTabs throw this.createPageUnavailableError(sessionName) } + // Why: a command can spawn several helpers in turn; none may start after a host's revocation. + assertRemoteCliBrowserAccess(session.webContentsId) this.reinitializeIfDaemonIdledOut(sessionName, session) session.lastCommandAt = Date.now() diff --git a/src/main/browser/agent-browser-bridge-host-revocation.test.ts b/src/main/browser/agent-browser-bridge-host-revocation.test.ts new file mode 100644 index 000000000000..f75234734848 --- /dev/null +++ b/src/main/browser/agent-browser-bridge-host-revocation.test.ts @@ -0,0 +1,290 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +const { execFileMock, webContentsFromIdMock, existsSyncMock, readFileSyncMock, stdinWrites } = + vi.hoisted(() => ({ + execFileMock: vi.fn(), + webContentsFromIdMock: vi.fn(), + existsSyncMock: vi.fn(() => false), + readFileSyncMock: vi.fn(() => Buffer.from('')), + stdinWrites: new Array() + })) + +vi.mock('child_process', () => ({ execFile: execFileMock })) +vi.mock('fs', () => ({ + existsSync: existsSyncMock, + readFileSync: readFileSyncMock, + accessSync: vi.fn(), + chmodSync: vi.fn(), + constants: { X_OK: 1 } +})) +vi.mock('os', () => ({ platform: () => 'darwin', arch: () => 'arm64' })) +vi.mock('electron', () => ({ + app: { getPath: vi.fn(() => '/app'), getAppPath: vi.fn(() => '/project'), isPackaged: false }, + webContents: { fromId: webContentsFromIdMock } +})) +const { CdpWsProxyMock } = vi.hoisted(() => { + const instances: unknown[] = [] + const MockClass = vi.fn().mockImplementation(function ( + this: Record, + _wc: unknown + ) { + this._wc = _wc + this.start = vi.fn(async () => 'ws://127.0.0.1:9222') + this.stop = vi.fn(async () => {}) + this.getPort = vi.fn(() => 9222) + instances.push(this) + }) + return { CdpWsProxyMock: Object.assign(MockClass, { instances }) } +}) +vi.mock('./cdp-ws-proxy', () => ({ CdpWsProxy: CdpWsProxyMock })) + +import { AgentBrowserBridge } from './agent-browser-bridge' +import { + captureRemoteCliBrowserScope, + runInRemoteCliBrowserScope +} from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' +import { assertRemoteCliPageAdmitted } from './remote-cli-page-admission' +import { + createFakeAgentBrowserChild, + mockBrowserManager, + mockWebContents, + overrideBridgeWebContentsLookup, + resetAgentBrowserBridgeMocks, + type ExecFileCallback +} from './agent-browser-bridge-test-harness' + +overrideBridgeWebContentsLookup(AgentBrowserBridge.prototype, webContentsFromIdMock) + +const WORKSPACE = 'repo-host::/srv/app' + +function dispatchedMouseEvents(wc: ReturnType): unknown[] { + return wc.debugger.sendCommand.mock.calls.filter((call) => call[0] === 'Input.dispatchMouseEvent') +} + +describe('AgentBrowserBridge under a host revocation', () => { + let bridge: AgentBrowserBridge + + beforeEach(() => { + resetAgentBrowserBridgeMocks({ + webContentsFromIdMock, + existsSyncMock, + readFileSyncMock, + stdinWrites, + cdpWsProxyInstances: CdpWsProxyMock.instances + }) + bridge = new AgentBrowserBridge(mockBrowserManager()) + bridge.setActiveTab(100) + }) + + it('never presses the mouse once the host is revoked during a running click', async () => { + const wc = mockWebContents(100) + let answerHitTest = (): void => {} + wc.debugger.sendCommand.mockImplementation(async (method: string) => { + if (method === 'Runtime.evaluate') { + await new Promise((resolve) => { + answerHitTest = resolve + }) + return { result: { value: { x: 10, y: 20, adjusted: false, handled: false } } } + } + return {} + }) + webContentsFromIdMock.mockReturnValue(wc) + const revoked = new AbortController() + + const click = runInRemoteCliBrowserScope( + WORKSPACE, + () => bridge.mouseClick(10, 20, 'left', undefined, 'tab-1', 18), + revoked.signal + ) + await vi.waitFor(() => + expect(wc.debugger.sendCommand).toHaveBeenCalledWith('Runtime.evaluate', expect.anything()) + ) + revoked.abort() + answerHitTest() + + await expect(click).rejects.toMatchObject({ code: 'desktop_browser_bridge_unavailable' }) + expect(dispatchedMouseEvents(wc)).toHaveLength(0) + }) + + it('holds the page against a running helper click once the host is revoked', async () => { + const helperCallbacks: ExecFileCallback[] = [] + execFileMock.mockImplementation( + (_bin: string, args: string[], _opts: unknown, cb: ExecFileCallback) => { + if (args.includes('click')) { + helperCallbacks.push(cb) + } else { + cb(null, JSON.stringify({ success: true, data: null }), '') + } + return createFakeAgentBrowserChild({ stdin: { on: vi.fn(), end: vi.fn() } }) + } + ) + const revoked = new AbortController() + + const click = runInRemoteCliBrowserScope( + WORKSPACE, + () => bridge.click('@e1', undefined, 'tab-1'), + revoked.signal + ) + await vi.waitFor(() => expect(helperCallbacks).toHaveLength(1)) + expect(() => assertRemoteCliPageAdmitted(100)).not.toThrow() + + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: every instance the hoisted mock constructs has a vi.fn stop. + const proxy = CdpWsProxyMock.instances.at(-1) as { stop: ReturnType } + let heldWhenRetired = false + proxy.stop.mockImplementation(async () => { + heldWhenRetired = (() => { + try { + assertRemoteCliPageAdmitted(100) + return false + } catch { + return true + } + })() + }) + revoked.abort() + // The helper's next CDP step reaches the page's proxy and is refused there. + expect(() => assertRemoteCliPageAdmitted(100)).toThrow('stopped sharing') + // Retired on the revocation itself, not when the helper's client times out. + await vi.waitFor(() => expect(proxy.stop).toHaveBeenCalled()) + helperCallbacks[0]!( + null, + JSON.stringify({ success: false, error: 'The Orca desktop stopped sharing its browser' }), + '' + ) + + await expect(click).rejects.toMatchObject({ code: 'desktop_browser_bridge_unavailable' }) + expect(() => assertRemoteCliPageAdmitted(100)).not.toThrow() + // The helper's daemon outlives its client, so it is retired before the page is let go. + expect(heldWhenRetired).toBe(true) + expect(execFileMock).toHaveBeenCalledWith( + expect.anything(), + expect.arrayContaining(['close']), + expect.anything(), + expect.anything() + ) + }) + + it('still runs a desktop-local command queued behind a host command that was revoked', async () => { + const wc = mockWebContents(100) + let answerHitTest = (): void => {} + wc.debugger.sendCommand.mockImplementation(async (method: string) => { + if (method === 'Runtime.evaluate') { + await new Promise((resolve) => { + answerHitTest = resolve + }) + return { result: { value: { x: 10, y: 20, adjusted: false, handled: false } } } + } + return {} + }) + webContentsFromIdMock.mockReturnValue(wc) + const revoked = new AbortController() + const hostClick = runInRemoteCliBrowserScope( + WORKSPACE, + () => bridge.mouseClick(10, 20, 'left', undefined, 'tab-1', 18), + revoked.signal + ) + hostClick.catch(() => {}) + await vi.waitFor(() => + expect(wc.debugger.sendCommand).toHaveBeenCalledWith('Runtime.evaluate', expect.anything()) + ) + + const localMove = bridge.mouseMove(5, 6, undefined, 'tab-1') + revoked.abort() + answerHitTest() + + await expect(hostClick).rejects.toMatchObject({ code: 'desktop_browser_bridge_unavailable' }) + await expect(localMove).resolves.toBeDefined() + expect(dispatchedMouseEvents(wc)).toEqual([ + ['Input.dispatchMouseEvent', expect.objectContaining({ type: 'mouseMoved', x: 5, y: 6 })] + ]) + }) + + it('never hit-tests (which can activate the element) once revoked during the focus wait', async () => { + let grantFocus = (): void => {} + const release = vi.fn() + const guardUserFocusForAgentInput = vi.fn( + () => + new Promise<() => void>((resolve) => { + grantFocus = () => resolve(release) + }) + ) + bridge = new AgentBrowserBridge( + mockBrowserManager(undefined, undefined, { guardUserFocusForAgentInput }) + ) + bridge.setActiveTab(100) + const wc = mockWebContents(100) + wc.getType.mockReturnValue('webview') + wc.debugger.sendCommand.mockResolvedValue({}) + webContentsFromIdMock.mockReturnValue(wc) + const revoked = new AbortController() + + const click = runInRemoteCliBrowserScope( + WORKSPACE, + () => bridge.mouseClick(10, 20, 'left', undefined, 'tab-1', 18), + revoked.signal + ) + await vi.waitFor(() => expect(guardUserFocusForAgentInput).toHaveBeenCalled()) + revoked.abort() + grantFocus() + + await expect(click).rejects.toMatchObject({ code: 'desktop_browser_bridge_unavailable' }) + expect(wc.debugger.sendCommand).not.toHaveBeenCalledWith('Runtime.evaluate', expect.anything()) + expect(dispatchedMouseEvents(wc)).toHaveLength(0) + expect(release).toHaveBeenCalled() + }) + + it('lets go off the page of a press that landed just before the host was revoked', async () => { + const wc = mockWebContents(100) + const revoked = new AbortController() + wc.debugger.sendCommand.mockImplementation(async (method: string, params?: unknown) => { + if (method === 'Runtime.evaluate') { + return { result: { value: { x: 10, y: 20, adjusted: false, handled: false } } } + } + if ( + typeof params === 'object' && + params !== null && + 'type' in params && + params.type === 'mousePressed' + ) { + revoked.abort() + } + return {} + }) + webContentsFromIdMock.mockReturnValue(wc) + + const click = runInRemoteCliBrowserScope( + WORKSPACE, + () => bridge.mouseClick(10, 20, 'left', undefined, 'tab-1', 18), + revoked.signal + ) + + await expect(click).rejects.toMatchObject({ code: 'desktop_browser_bridge_unavailable' }) + expect(dispatchedMouseEvents(wc)).toEqual([ + ['Input.dispatchMouseEvent', expect.objectContaining({ type: 'mousePressed', x: 10 })], + ['Input.dispatchMouseEvent', expect.objectContaining({ type: 'mouseReleased', x: -1, y: -1 })] + ]) + }) + + it("starts the page's CDP proxy outside the host's scope, so it outlives that host's request", async () => { + let startedInHostScope: boolean | undefined + CdpWsProxyMock.mockImplementationOnce(function (this: Record) { + this.start = vi.fn(async () => { + startedInHostScope = captureRemoteCliBrowserScope().bridged + return 'ws://127.0.0.1:9222' + }) + this.stop = vi.fn(async () => {}) + this.getPort = vi.fn(() => 9222) + CdpWsProxyMock.instances.push(this) + }) + execFileMock.mockImplementation( + (_bin: string, _args: string[], _opts: unknown, cb: ExecFileCallback) => { + cb(null, JSON.stringify({ success: true, data: null }), '') + return createFakeAgentBrowserChild({ stdin: { on: vi.fn(), end: vi.fn() } }) + } + ) + + await runInRemoteCliBrowserScope(WORKSPACE, () => bridge.click('@e1', undefined, 'tab-1')) + + expect(startedInHostScope).toBe(false) + }) +}) diff --git a/src/main/browser/agent-browser-bridge-lifecycle.ts b/src/main/browser/agent-browser-bridge-lifecycle.ts index fdd1d387017a..1900c5767ef5 100644 --- a/src/main/browser/agent-browser-bridge-lifecycle.ts +++ b/src/main/browser/agent-browser-bridge-lifecycle.ts @@ -4,6 +4,7 @@ import { ORCA_TAB_SESSION_PREFIX } from './agent-browser-orphan-sweep' import { AgentBrowserBridgeRawProcess } from './agent-browser-bridge-raw-process' import type { AgentBrowserCleanupOptions } from './agent-browser-bridge-types' import { AGENT_BROWSER_CLEANUP_TIMEOUT_MS } from './agent-browser-bridge-types' +import { runOutsideRemoteCliBrowserScope } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' export abstract class AgentBrowserBridgeLifecycle extends AgentBrowserBridgeRawProcess { async onTabClosed(webContentsId: number): Promise { @@ -126,7 +127,9 @@ export abstract class AgentBrowserBridgeLifecycle extends AgentBrowserBridgeRawP }) } - const promise = createSession() + // Why: the proxy's sockets keep the async scope they were created in; a host's would refuse + // every later command on this page, the desktop's included, after that host's request ended. + const promise = runOutsideRemoteCliBrowserScope(createSession) this.pendingSessionCreation.set(sessionName, promise) try { await promise @@ -157,15 +160,23 @@ export abstract class AgentBrowserBridgeLifecycle extends AgentBrowserBridgeRawP const session = this.sessions.get(sessionName) if (!session) { - this.rejectQueuedCommandsForClosedSession(sessionName) + if (!options.keepQueuedCommands) { + this.rejectQueuedCommandsForClosedSession(sessionName) + } return } this.sessions.delete(sessionName) this.pendingSessionCreation.delete(sessionName) + // Why: a retired session's page stays open, so the fresh one must keep its interception. + if (options.keepQueuedCommands && session.activeInterceptPatterns.length > 0) { + this.pendingInterceptRestore.set(sessionName, [...session.activeInterceptPatterns]) + } // Why: queued commands would hang forever if we just delete the queue — drain and reject them. - this.rejectQueuedCommandsForClosedSession(sessionName) + if (!options.keepQueuedCommands) { + this.rejectQueuedCommandsForClosedSession(sessionName) + } if (session.activeProcess) { // Why: rejecting the queue isn't enough for an in-flight command — kill the process so callers don't wait out the exec timeout. diff --git a/src/main/browser/agent-browser-bridge-mouse-commands.ts b/src/main/browser/agent-browser-bridge-mouse-commands.ts index 1aaef42a16cb..d20b73e194a3 100644 --- a/src/main/browser/agent-browser-bridge-mouse-commands.ts +++ b/src/main/browser/agent-browser-bridge-mouse-commands.ts @@ -7,6 +7,8 @@ import { resolveMobileTouchClickPoint } from './agent-browser-bridge-mouse' import { acquireElectronDebugger } from './electron-debugger-lease' +import { sendGuestCdpCommand } from './guest-cdp-command' +import { assertRemoteCliBrowserAccess } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' import { AgentBrowserBridgePointerCommands } from './agent-browser-bridge-pointer-commands' export abstract class AgentBrowserBridgeMouseCommands extends AgentBrowserBridgePointerCommands { @@ -39,6 +41,8 @@ export abstract class AgentBrowserBridgeMouseCommands extends AgentBrowserBridge let releaseFocusGuard = (): void => {} try { releaseFocusGuard = await this.focusPageForPress(wc, target.webContentsId) + // Why: the hit test below can activate the element, and access may be revoked meanwhile. + assertRemoteCliBrowserAccess(target.webContentsId) const point = cdpButton === 'left' ? // Why: DOM activation can't carry Cmd/Ctrl/Alt/Shift, so modifier clicks use the adjusted point and let CDP dispatch the event. @@ -47,7 +51,7 @@ export abstract class AgentBrowserBridgeMouseCommands extends AgentBrowserBridge // Why: land the tap as one atomic op — separate move/down/up CLI calls visibly hover and can miss small controls. // Why: mobile-emulated BrowserViews can ignore CDP mouse clicks, so the runtime may already have activated DOM controls. if (!point.handled) { - await wc.debugger.sendCommand('Input.dispatchMouseEvent', { + await sendGuestCdpCommand(wc, 'Input.dispatchMouseEvent', { type: 'mousePressed', x: point.x, y: point.y, @@ -56,7 +60,7 @@ export abstract class AgentBrowserBridgeMouseCommands extends AgentBrowserBridge modifiers: cdpModifiers, clickCount: 1 }) - await wc.debugger.sendCommand('Input.dispatchMouseEvent', { + await sendGuestCdpCommand(wc, 'Input.dispatchMouseEvent', { type: 'mouseReleased', x: point.x, y: point.y, diff --git a/src/main/browser/agent-browser-bridge-pointer-commands.ts b/src/main/browser/agent-browser-bridge-pointer-commands.ts index d79e0a919a0c..1377fd843696 100644 --- a/src/main/browser/agent-browser-bridge-pointer-commands.ts +++ b/src/main/browser/agent-browser-bridge-pointer-commands.ts @@ -10,6 +10,7 @@ import { type CdpPointerState } from './cdp-pointer-input' import { acquireElectronDebugger } from './electron-debugger-lease' +import { sendGuestCdpCommand } from './guest-cdp-command' import { AgentBrowserBridgeInputCommands } from './agent-browser-bridge-input-commands' import { isEmbeddedGuest } from './guest-direct-key-input' import { enablePageFocusEmulation } from './agent-input-focus-guard' @@ -64,7 +65,7 @@ export abstract class AgentBrowserBridgePointerCommands extends AgentBrowserBrid if (press) { releaseFocusGuard = await this.focusPageForPress(wc, target.webContentsId) } - await wc.debugger.sendCommand('Input.dispatchMouseEvent', params) + await sendGuestCdpCommand(wc, 'Input.dispatchMouseEvent', params) return result } catch (error) { Object.assign(state, preDispatch) diff --git a/src/main/browser/agent-browser-bridge-queue.ts b/src/main/browser/agent-browser-bridge-queue.ts index bf9826119691..e63b2842e446 100644 --- a/src/main/browser/agent-browser-bridge-queue.ts +++ b/src/main/browser/agent-browser-bridge-queue.ts @@ -6,7 +6,15 @@ import type { EnqueueTargetedCommandOptions, ResolvedBrowserCommandTarget } from './agent-browser-bridge-types' -import { captureRemoteCliBrowserRevocation } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' +import { isAdmissionRevoked, runWithRemoteCliPageAdmission } from './remote-cli-page-admission' +import { AGENT_BROWSER_CLEANUP_TIMEOUT_MS } from './agent-browser-bridge-types' +import { releaseHeldPageInput } from './guest-direct-key-input' +import { + captureRemoteCliBrowserRevocation, + captureRemoteCliBrowserRevokedSignal, + captureRemoteCliBrowserScope, + runOutsideRemoteCliBrowserScope +} from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown { // Why: route tab switch through the command queue so it can't race in-flight commands targeting the old tab. @@ -61,6 +69,7 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown ): Promise { this.assertCommandAdmission() const assertNotRevoked = captureRemoteCliBrowserRevocation() + const origin = captureRemoteCliBrowserScope() assertNotRevoked() const { browserPageId: pageId } = this.resolveCommandTarget( worktreeId, @@ -78,15 +87,22 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown queue.push({ execute: () => { assertNotRevoked() - return this.executeQueuedCommand( - worktreeId, - pageId, - (session, target) => { - assertNotRevoked(target.webContentsId) - return execute(session, target) - }, - options - ) + const run = (): Promise => + this.executeQueuedCommand( + worktreeId, + pageId, + (session, target) => { + const gate = (): void => assertNotRevoked(target.webContentsId) + gate() + return origin.bridged + ? this.runAdmitted(session, target.webContentsId, gate, () => + execute(session, target) + ) + : execute(session, target) + }, + options + ) + return origin.run(run) }, resolve: resolve as (value: unknown) => void, reject @@ -111,6 +127,55 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown return execute(sessionName, target) } + /** + * Holds the page for a host's command: every later page-affecting step, including the helper's + * through the CDP proxy, is refused once the host is revoked, and the command reports that. + */ + private async runAdmitted( + sessionName: string, + webContentsId: number, + gate: () => void, + execute: () => Promise + ): Promise { + // Why: the helper's daemon outlives its killed client and keeps retrying; retire it while the + // page is still held, or its next step lands once the hold is gone. Retiring on the signal also + // frees the page's queue now instead of at the helper's exec timeout. + let retiring: Promise | null = null + const retire = (): Promise => + (retiring ??= runOutsideRemoteCliBrowserScope(() => + this.destroySession(sessionName, { + closeTimeoutMs: AGENT_BROWSER_CLEANUP_TIMEOUT_MS, + keepQueuedCommands: true + }) + )) + const signal = captureRemoteCliBrowserRevokedSignal() + const onRevoked = (): void => void retire() + signal?.addEventListener('abort', onRevoked, { once: true }) + const held = async (): Promise => { + try { + return await execute() + } finally { + signal?.removeEventListener('abort', onRevoked) + if (retiring || isAdmissionRevoked(gate)) { + await retire() + } + } + } + try { + return await runWithRemoteCliPageAdmission(webContentsId, gate, held, (release) => { + const wc = this.getWebContents(webContentsId) + return wc + ? releaseHeldPageInput(wc, release, () => + this.browserManager.guardUserFocusForAgentInput(webContentsId) + ) + : Promise.resolve() + }) + } catch (error) { + gate() + throw error + } + } + protected async processQueue(sessionName: string): Promise { if (this.processingQueues.has(sessionName)) { return diff --git a/src/main/browser/agent-browser-bridge-types.ts b/src/main/browser/agent-browser-bridge-types.ts index 2d4236afdbd8..ad5d1747a6c4 100644 --- a/src/main/browser/agent-browser-bridge-types.ts +++ b/src/main/browser/agent-browser-bridge-types.ts @@ -43,6 +43,8 @@ export type ResolvedBrowserCommandTarget = { export type AgentBrowserCleanupOptions = { closeTimeoutMs?: number + /** Retire only the helper and proxy; commands waiting on the page still run, on a fresh session. */ + keepQueuedCommands?: boolean } export type BrowserMouseModifier = 'cmd' | 'ctrl' | 'alt' | 'shift' diff --git a/src/main/browser/agent-input-injection.ts b/src/main/browser/agent-input-injection.ts index e5ec1cc6b4a9..658d06bc7f27 100644 --- a/src/main/browser/agent-input-injection.ts +++ b/src/main/browser/agent-input-injection.ts @@ -1,4 +1,10 @@ import type { WebContents } from 'electron' +import { + assertRemoteCliPageAdmitted, + isInputRelease, + noteRemoteCliInput +} from './remote-cli-page-admission' +import { assertRemoteCliBrowserAccess } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' // Why: Orca's guest shortcut and grab handlers act on keys the page receives, and an unhandled key // can reach the app menu. Keys an agent injects must do neither (#20595), whichever path sends them. @@ -8,18 +14,38 @@ export function isAgentInjectingInput(webContents: WebContents): boolean { return (injectionDepth.get(webContents) ?? 0) > 0 } +/** The CDP input event being delivered, so a revocation can release what a host pressed. */ +export type AgentInputEvent = { method: string; params: Record } + +/** Refuses agent input that a revoked SSH host's command sends, directly or through its helper. */ +export function assertAgentInputAdmitted(webContents: WebContents): void { + assertRemoteCliBrowserAccess(webContents.id) + assertRemoteCliPageAdmitted(webContents.id) +} + /** Marks input sent during `run` as the agent's: Orca shortcuts and app menu items ignore it. */ export async function withAgentInputInjection( webContents: WebContents, - run: () => Promise + run: () => Promise, + input?: AgentInputEvent ): Promise { + assertAgentInputAdmitted(webContents) + // Why: a press counts as held once it may have landed; a release only once it surely did. + const releasing = input !== undefined && isInputRelease(input.params) + if (input && !releasing) { + noteRemoteCliInput(webContents.id, input.method, input.params) + } const depth = injectionDepth.get(webContents) ?? 0 injectionDepth.set(webContents, depth + 1) if (depth === 0) { webContents.setIgnoreMenuShortcuts(true) } try { - return await run() + const result = await run() + if (input && releasing) { + noteRemoteCliInput(webContents.id, input.method, input.params) + } + return result } finally { const remaining = (injectionDepth.get(webContents) ?? 1) - 1 if (remaining > 0) { diff --git a/src/main/browser/cdp-debugger-channel.ts b/src/main/browser/cdp-debugger-channel.ts index 18620aa6c6d3..82da834ca1db 100644 --- a/src/main/browser/cdp-debugger-channel.ts +++ b/src/main/browser/cdp-debugger-channel.ts @@ -1,6 +1,7 @@ import { WebSocket } from 'ws' import type { WebContents } from 'electron' import { acquireElectronDebugger, type ElectronDebuggerLease } from './electron-debugger-lease' +import { assertRemoteCliPageAdmitted } from './remote-cli-page-admission' import type { CdpClientResponseWriter } from './cdp-client-response-writer' import type { CdpSyntheticSessionRegistry } from './cdp-synthetic-session-registry' import { sendGuestCdpCommand } from './guest-cdp-command' @@ -93,6 +94,12 @@ export class CdpDebuggerChannel { params: Record, sessionId?: string ): Promise { + // Why: the proxy's queued input outlives the message's admission; a revoked host's must not land. + try { + assertRemoteCliPageAdmitted(this.webContents.id) + } catch (error) { + return Promise.reject(error) + } return sessionId ? sendGuestCdpCommand(this.webContents, method, params, sessionId) : sendGuestCdpCommand(this.webContents, method, params) diff --git a/src/main/browser/cdp-page-input-commands.ts b/src/main/browser/cdp-page-input-commands.ts index 89da2e803fd1..57e0bf15fe5d 100644 --- a/src/main/browser/cdp-page-input-commands.ts +++ b/src/main/browser/cdp-page-input-commands.ts @@ -85,8 +85,10 @@ export class CdpPageInputCommands { return await this.debuggerChannel.sendDebuggerCommand(method, params, sessionId) } const send = (): Promise => - withAgentInputInjection(this.webContents, () => - this.debuggerChannel.sendDebuggerCommand(method, params, sessionId) + withAgentInputInjection( + this.webContents, + () => this.debuggerChannel.sendDebuggerCommand(method, params, sessionId), + { method, params } ) const routedByFocus = !sessionId && isFocusRoutedCdpInput(method) if (!routedByFocus && !isFocusTakingCdpInput(method, params)) { diff --git a/src/main/browser/cdp-ws-proxy-host-admission.test.ts b/src/main/browser/cdp-ws-proxy-host-admission.test.ts new file mode 100644 index 000000000000..ef43cdc249ef --- /dev/null +++ b/src/main/browser/cdp-ws-proxy-host-admission.test.ts @@ -0,0 +1,308 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { CdpWsProxy } from './cdp-ws-proxy' +import { runWithRemoteCliPageAdmission } from './remote-cli-page-admission' +import { sendGuestCdpCommand } from './guest-cdp-command' +import { releaseHeldPageInput } from './guest-direct-key-input' +import { + connect, + createMockWebContents, + getSendCommandMethods, + sendAndReceive, + type MockWebContents +} from './cdp-ws-proxy-test-harness' + +vi.mock('electron', () => ({ + webContents: { fromId: vi.fn() } +})) + +const PAGE_ID = 7 +const REVOKED = 'The Orca desktop stopped sharing its browser before this command ran.' + +function keyUps(mock: MockWebContents): string[] { + return [ + ...mock.webContents.sendInputEvent.mock.calls.map((call) => call[0].type), + ...mock.webContents.debugger.sendCommand.mock.calls + .filter((call) => call[0] === 'Input.dispatchKeyEvent') + .map((call) => String(call[1]?.type)) + ].filter((type) => type === 'keyUp') +} + +function mouseEvents(mock: MockWebContents): string[] { + return mock.webContents.debugger.sendCommand.mock.calls + .filter((call) => call[0] === 'Input.dispatchMouseEvent') + .map((call) => String(call[1]?.type)) +} + +describe('CdpWsProxy under a host command', () => { + let mock: MockWebContents + let proxy: CdpWsProxy + let endpoint: string + let guardFocus: () => Promise<() => void> + + beforeEach(async () => { + mock = createMockWebContents() + guardFocus = async () => () => {} + const page = { ...mock.webContents, id: PAGE_ID } + proxy = new CdpWsProxy( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mock implements every WebContents member the proxy calls. + page as never, + () => () => {}, + () => guardFocus() + ) + endpoint = await proxy.start() + }) + + afterEach(async () => { + await proxy.stop() + }) + + it('refuses every helper step after the host is revoked, without sending it to the page', async () => { + let revoked = false + let finish = (): void => {} + const held = runWithRemoteCliPageAdmission( + PAGE_ID, + () => { + if (revoked) { + throw new Error(REVOKED) + } + }, + () => new Promise((resolve) => (finish = resolve)) + ) + const client = await connect(endpoint) + const press = { type: 'mouseMoved', x: 1, y: 1 } + + const allowed = await sendAndReceive(client, { + id: 1, + method: 'Input.dispatchMouseEvent', + params: press + }) + revoked = true + const refused = await sendAndReceive(client, { + id: 2, + method: 'Input.dispatchMouseEvent', + params: press + }) + + expect(allowed.error).toBeUndefined() + expect(refused).toMatchObject({ id: 2, error: { message: REVOKED } }) + expect(mouseEvents(mock)).toEqual(['mouseMoved']) + finish() + await held + client.close() + }) + + function holdPage(): { revoke: () => void; release: () => Promise } { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mock implements every WebContents member the release path calls. + const page = { ...mock.webContents, id: PAGE_ID } as never + let revoked = false + let finish = (): void => {} + const held = runWithRemoteCliPageAdmission( + PAGE_ID, + () => { + if (revoked) { + throw new Error(REVOKED) + } + }, + () => new Promise((resolve) => (finish = resolve)), + (release) => releaseHeldPageInput(page, release, () => guardFocus()) + ) + return { + revoke: () => (revoked = true), + release: async () => { + finish() + await held + } + } + } + + const MOUSE = 'Input.dispatchMouseEvent' + const KEY = 'Input.dispatchKeyEvent' + const pressed = { type: 'mousePressed', x: 1, y: 1, button: 'left', clickCount: 1 } + const released = { type: 'mouseReleased', x: 1, y: 1, button: 'left', clickCount: 1 } + const shiftDown = { type: 'rawKeyDown', key: 'Shift', code: 'ShiftLeft', modifiers: 8 } + const shiftUp = { type: 'keyUp', key: 'Shift', code: 'ShiftLeft' } + + it.each(['window', 'webview'])( + 'refuses a revoked release, then lets go of the button and key itself (%s page)', + async (pageType) => { + mock.webContents.getType.mockReturnValue(pageType) + const page = holdPage() + const client = await connect(endpoint) + + await sendAndReceive(client, { id: 5, method: MOUSE, params: pressed }) + await sendAndReceive(client, { id: 6, method: KEY, params: shiftDown }) + page.revoke() + const mouseUp = await sendAndReceive(client, { id: 7, method: MOUSE, params: released }) + const keyUp = await sendAndReceive(client, { id: 8, method: KEY, params: shiftUp }) + + expect(mouseUp).toMatchObject({ error: { message: REVOKED } }) + expect(keyUp).toMatchObject({ error: { message: REVOKED } }) + expect(mouseEvents(mock)).toEqual(['mousePressed']) + expect(keyUps(mock)).toHaveLength(0) + await page.release() + // Off the page, so letting go completes no click the host had started. + expect(mock.webContents.debugger.sendCommand).toHaveBeenCalledWith( + MOUSE, + expect.objectContaining({ type: 'mouseReleased', x: -1, y: -1 }) + ) + expect(mouseEvents(mock)).toEqual(['mousePressed', 'mouseReleased']) + expect(keyUps(mock)).toHaveLength(1) + client.close() + } + ) + + it('lets go of nothing when the command released what it pressed before the revocation', async () => { + const page = holdPage() + const client = await connect(endpoint) + + await sendAndReceive(client, { id: 11, method: MOUSE, params: pressed }) + await sendAndReceive(client, { id: 12, method: MOUSE, params: released }) + page.revoke() + await page.release() + + expect(mouseEvents(mock)).toEqual(['mousePressed', 'mouseReleased']) + client.close() + }) + + it('refuses a release of something the revoked command never pressed', async () => { + const page = holdPage() + const client = await connect(endpoint) + page.revoke() + + const mouseUp = await sendAndReceive(client, { id: 9, method: MOUSE, params: released }) + const keyUp = await sendAndReceive(client, { id: 10, method: KEY, params: shiftUp }) + + expect(mouseUp).toMatchObject({ error: { message: REVOKED } }) + expect(keyUp).toMatchObject({ error: { message: REVOKED } }) + expect(mouseEvents(mock)).toEqual([]) + await page.release() + client.close() + }) + + it('drops a press it accepted before the revocation but had not delivered yet', async () => { + mock.webContents.getType.mockReturnValue('webview') + let grantFocus = (): void => {} + guardFocus = () => new Promise((resolve) => (grantFocus = () => resolve(() => {}))) + let revoked = false + let finish = (): void => {} + const held = runWithRemoteCliPageAdmission( + PAGE_ID, + () => { + if (revoked) { + throw new Error(REVOKED) + } + }, + () => new Promise((resolve) => (finish = resolve)) + ) + const client = await connect(endpoint) + + const reply = sendAndReceive(client, { + id: 3, + method: 'Input.dispatchMouseEvent', + params: { type: 'mousePressed', x: 1, y: 1, button: 'left', clickCount: 1 } + }) + await vi.waitFor(() => + expect(getSendCommandMethods(mock)).toContain('Emulation.setFocusEmulationEnabled') + ) + revoked = true + grantFocus() + + await expect(reply).resolves.toMatchObject({ id: 3, error: { message: REVOKED } }) + expect(mouseEvents(mock)).toEqual([]) + finish() + await held + client.close() + }) + + it('still refuses a deferred press after the revoked command let go of the page', async () => { + mock.webContents.getType.mockReturnValue('webview') + let grantFocus = (): void => {} + guardFocus = () => new Promise((resolve) => (grantFocus = () => resolve(() => {}))) + const page = holdPage() + const client = await connect(endpoint) + + const reply = sendAndReceive(client, { id: 13, method: MOUSE, params: pressed }) + await vi.waitFor(() => + expect(getSendCommandMethods(mock)).toContain('Emulation.setFocusEmulationEnabled') + ) + page.revoke() + await page.release() + grantFocus() + + await expect(reply).resolves.toMatchObject({ id: 13, error: { message: REVOKED } }) + expect(mouseEvents(mock)).toEqual([]) + client.close() + }) + + it('lets go of a button whose own release failed to land', async () => { + const page = holdPage() + const client = await connect(endpoint) + const send = mock.webContents.debugger.sendCommand.getMockImplementation() + mock.webContents.debugger.sendCommand.mockImplementationOnce(async () => ({})) + mock.webContents.debugger.sendCommand.mockImplementationOnce(async () => { + throw new Error('target closed') + }) + + await sendAndReceive(client, { id: 14, method: MOUSE, params: pressed }) + const failed = await sendAndReceive(client, { id: 15, method: MOUSE, params: released }) + if (send) { + mock.webContents.debugger.sendCommand.mockImplementation(send) + } + page.revoke() + await page.release() + + expect(failed.error).toBeDefined() + expect(mock.webContents.debugger.sendCommand).toHaveBeenCalledWith( + MOUSE, + expect.objectContaining({ type: 'mouseReleased', x: -1, y: -1 }) + ) + client.close() + }) + + it('forwards commands again once the host command ends', async () => { + await runWithRemoteCliPageAdmission( + PAGE_ID, + () => { + throw new Error(REVOKED) + }, + async () => {} + ) + const client = await connect(endpoint) + + const response = await sendAndReceive(client, { + id: 4, + method: 'Accessibility.getFullAXTree', + params: {} + }) + + expect(response.error).toBeUndefined() + expect(getSendCommandMethods(mock)).toContain('Accessibility.getFullAXTree') + client.close() + }) + + it('never refuses the desktop its own page commands while a revoked host command unwinds', async () => { + let finish = (): void => {} + const held = runWithRemoteCliPageAdmission( + PAGE_ID, + () => { + throw new Error(REVOKED) + }, + () => new Promise((resolve) => (finish = resolve)) + ) + + await expect( + sendGuestCdpCommand( + { ...mock.webContents, id: PAGE_ID }, + 'Emulation.setDeviceMetricsOverride', + { + width: 400, + height: 300, + deviceScaleFactor: 1, + mobile: false + } + ) + ).resolves.toEqual({}) + finish() + await held + }) +}) diff --git a/src/main/browser/cdp-ws-proxy.ts b/src/main/browser/cdp-ws-proxy.ts index b074a61eae3c..163e857c01d0 100644 --- a/src/main/browser/cdp-ws-proxy.ts +++ b/src/main/browser/cdp-ws-proxy.ts @@ -12,6 +12,10 @@ import type { CapturePaintHold } from './cdp-screenshot' import type { AgentInputFocusGuard } from './agent-input-focus-guard' import { CdpPageInputCommands } from './cdp-page-input-commands' import { isTrustedCdpLoopbackRequest } from './cdp-loopback-request-guard' +import { + assertRemoteCliPageAdmitted, + runUnderRemoteCliPageAdmission +} from './remote-cli-page-admission' export class CdpWsProxy { private httpServer: Server | null = null @@ -96,7 +100,9 @@ export class CdpWsProxy { this.closeClient() this.client = ws const onMessage = (data: WebSocket.RawData): void => { - this.handleClientMessage(ws, data.toString()) + runUnderRemoteCliPageAdmission(this.webContents.id, () => + this.handleClientMessage(ws, data.toString()) + ) } const onClose = (): void => { detach() @@ -178,6 +184,17 @@ export class CdpWsProxy { } const clientId = msg.id this.responder.recordRequestSessionId(client, clientId, msg) + // Why here too: a revoked host's helper is refused even its read-only steps, so it stops at once. + try { + assertRemoteCliPageAdmitted(this.webContents.id) + } catch (error) { + this.responder.sendError( + clientId, + error instanceof Error ? error.message : String(error), + client + ) + return + } if (this.discovery.handleCommand(client, clientId, msg)) { return diff --git a/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-connection.ts b/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-connection.ts index 391c96216fb5..12de9ee580f1 100644 --- a/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-connection.ts +++ b/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-connection.ts @@ -28,6 +28,7 @@ export class DesktopBrowserBridgeConnection { private attachmentId: string | null = null private closed = false private readonly revoked = new AbortController() + private readonly cancellations = new Map() constructor( private readonly options: { @@ -102,15 +103,29 @@ export class DesktopBrowserBridgeConnection { this.lose(new Error('desktop_browser_bridge_request_before_ready')) return } + if (parsed.data.type === 'cancel') { + this.cancellations.get(parsed.data.requestId)?.abort() + return + } void this.answer(this.attachmentId, parsed.data) } private async answer(attachmentId: string, request: DesktopBrowserBridgeRequest): Promise { // Why: once orcad has stopped waiting, a command still queued here must not run late. - const signal = request.timeoutMs - ? AbortSignal.any([this.revoked.signal, AbortSignal.timeout(request.timeoutMs)]) - : this.revoked.signal - const outcome = await this.options.execute(request, signal) + const cancelled = new AbortController() + this.cancellations.set(request.requestId, cancelled) + const signals = [this.revoked.signal, cancelled.signal] + if (request.timeoutMs) { + signals.push(AbortSignal.timeout(request.timeoutMs)) + } + let outcome: DesktopBrowserBridgeOutcome + try { + outcome = await this.options.execute(request, AbortSignal.any(signals)) + } finally { + if (this.cancellations.get(request.requestId) === cancelled) { + this.cancellations.delete(request.requestId) + } + } const sendRequest = this.subscription?.sendRequest if (this.closed || this.attachmentId !== attachmentId || !sendRequest) { return diff --git a/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-manager.test.ts b/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-manager.test.ts index 2af864680d8d..cac10e444169 100644 --- a/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-manager.test.ts +++ b/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-manager.test.ts @@ -315,4 +315,44 @@ describe('DesktopBrowserBridgeManager', () => { expect(seen!.aborted).toBe(false) await vi.waitFor(() => expect(seen!.aborted).toBe(true)) }) + + it('stops a queued or running command when orcad cancels it, and stays attached', async () => { + const h = harness() + let seen: AbortSignal | null = null + let finish = (): void => {} + h.execute.mockImplementationOnce(async (_candidate, _request, signal) => { + seen = signal + await new Promise((resolve) => { + finish = resolve + }) + return { ok: true, result: {} } + }) + h.manager.reconcile() + await vi.waitFor(() => expect(h.subscriptions).toHaveLength(1)) + const socket = h.subscriptions[0]! + socket.callbacks.onResponse(event({ type: 'ready', attachmentId: 'att-1' })) + socket.callbacks.onResponse( + event({ type: 'request', requestId: 'r', method: 'browser.click', params: {} }) + ) + await vi.waitFor(() => expect(seen).not.toBeNull()) + expect(seen!.aborted).toBe(false) + + socket.callbacks.onResponse(event({ type: 'cancel', requestId: 'r' })) + + expect(seen!.aborted).toBe(true) + expect(h.manager.isAttached('env-a')).toBe(true) + finish() + }) + + it('ignores a cancel for a request it is not running', async () => { + const h = harness() + h.manager.reconcile() + await vi.waitFor(() => expect(h.subscriptions).toHaveLength(1)) + const socket = h.subscriptions[0]! + socket.callbacks.onResponse(event({ type: 'ready', attachmentId: 'att-1' })) + + socket.callbacks.onResponse(event({ type: 'cancel', requestId: 'unknown' })) + + expect(h.manager.isAttached('env-a')).toBe(true) + }) }) diff --git a/src/main/browser/guest-cdp-command.ts b/src/main/browser/guest-cdp-command.ts index ed7735df2d87..f3845e552a93 100644 --- a/src/main/browser/guest-cdp-command.ts +++ b/src/main/browser/guest-cdp-command.ts @@ -1,5 +1,7 @@ import type { WebContents } from 'electron' import { BrowserError } from './browser-error' +import { assertRemoteCliBrowserAccess } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' +import { isInputRelease, noteRemoteCliInput } from './remote-cli-page-admission' // Why: Chromium resizes the page's view for these without checking the view still exists // (WebContentsImpl::SetDeviceEmulationSize). A crashed renderer takes its view with it until the @@ -10,6 +12,7 @@ const VIEW_RESIZING_CDP_METHODS: ReadonlySet = new Set([ ]) type GuestCdpTarget = Pick & { + id?: number debugger: Pick } @@ -24,6 +27,12 @@ export function sendGuestCdpCommand( params?: Record, sessionId?: string ): Promise { + try { + // Why: only a host's own command runs in its scope; desktop-owned sends are never refused here. + assertRemoteCliBrowserAccess(guest.id) + } catch (error) { + return Promise.reject(error) + } // Why same task as the send: isCrashed() flips in the same Chromium task that drops the view, // so checking right before sendCommand leaves no gap for the renderer to die in between. if (VIEW_RESIZING_CDP_METHODS.has(method) && (guest.isDestroyed() || guest.isCrashed())) { @@ -34,9 +43,20 @@ export function sendGuestCdpCommand( ) ) } - return Promise.resolve( + // Why: a press counts as held once it may have landed; a release only once it surely did. + const releasing = params !== undefined && isInputRelease(params) + if (!releasing) { + noteRemoteCliInput(guest.id, method, params) + } + const sent = Promise.resolve( sessionId === undefined ? guest.debugger.sendCommand(method, params) : guest.debugger.sendCommand(method, params, sessionId) ) + return releasing + ? sent.then((result: unknown) => { + noteRemoteCliInput(guest.id, method, params) + return result + }) + : sent } diff --git a/src/main/browser/guest-direct-key-input.test.ts b/src/main/browser/guest-direct-key-input.test.ts index 1c14b08905cf..6fdf71ebd871 100644 --- a/src/main/browser/guest-direct-key-input.test.ts +++ b/src/main/browser/guest-direct-key-input.test.ts @@ -2,11 +2,10 @@ import { describe, expect, it, vi } from 'vitest' import type { WebContents } from 'electron' import { dispatchPageKeyEvent, - planCdpKeyEvent, - planInsertText, routePageTextInsertion, type GuestCdpSender } from './guest-direct-key-input' +import { planCdpKeyEvent, planInsertText } from './guest-key-event-plan' function fakePage(type: string) { const page = { diff --git a/src/main/browser/guest-direct-key-input.ts b/src/main/browser/guest-direct-key-input.ts index dd2d559a6755..08e9d2649abf 100644 --- a/src/main/browser/guest-direct-key-input.ts +++ b/src/main/browser/guest-direct-key-input.ts @@ -1,185 +1,32 @@ -import type { InputEvent, KeyboardInputEvent, WebContents } from 'electron' +import type { WebContents } from 'electron' import { enablePageFocusEmulation, type AgentInputFocusGuard } from './agent-input-focus-guard' -import { withAgentInputInjection } from './agent-input-injection' +import { assertAgentInputAdmitted, withAgentInputInjection } from './agent-input-injection' import { resolveFocusedFrameRoute, settleWithin, type GuestSessionCdpSender } from './guest-focused-frame-route' import { sendFrameSessionInput } from './guest-frame-input-sessions' +import { planCdpKeyEvent, planInsertText, type GuestEditCommand } from './guest-key-event-plan' +import { sendGuestCdpCommand } from './guest-cdp-command' +import { acquireElectronDebugger } from './electron-debugger-lease' +import { normalizeCdpPointerButton } from './agent-browser-bridge-mouse' +import { cdpPointerStateFor, releaseCdpPointerButton } from './cdp-pointer-input' +import type { HeldInputRelease } from './remote-cli-page-admission' // Why: Chromium routes main-session CDP key and text input on a guest by the // window's focus, so it lands in whatever the user is typing into (#20595). Events sent to // the guest's own widget reach its page without moving focus. // See docs/reference/agent-browser-focus-isolation.md. -type ElectronModifier = NonNullable[number] - -// Why: Electron honours this NativeWebKeyboardEvent flag although its typings omit it; an -// unhandled agent key must not be redispatched to Orca's window or application menu. -type GuestKeyboardEvent = KeyboardInputEvent & { skipIfUnhandled: true } - -type GuestEditCommand = 'selectAll' | 'copy' | 'cut' | 'paste' | 'undo' | 'redo' | 'delete' - -type GuestKeyPlan = { - events: GuestKeyboardEvent[] - // Why: macOS editing shortcuts arrive as CDP `commands`; widget key events skip the - // browser-side key bindings that would run them. - commands: GuestEditCommand[] -} - export type GuestCdpSender = GuestSessionCdpSender -const CDP_MODIFIER_NAMES: [number, ElectronModifier][] = [ - [1, 'alt'], - [2, 'control'], - [4, 'meta'], - [8, 'shift'] -] - -// CDP `key` -> Electron accelerator key code, for keys whose names differ or are not single characters. -const ELECTRON_KEY_CODES: Record = { - Enter: 'Enter', - Tab: 'Tab', - Backspace: 'Backspace', - Delete: 'Delete', - Escape: 'Escape', - ' ': 'Space', - ArrowUp: 'Up', - ArrowDown: 'Down', - ArrowLeft: 'Left', - ArrowRight: 'Right', - Home: 'Home', - End: 'End', - PageUp: 'PageUp', - PageDown: 'PageDown', - Insert: 'Insert', - CapsLock: 'CapsLock', - NumLock: 'NumLock', - ScrollLock: 'ScrollLock', - PrintScreen: 'PrintScreen', - Shift: 'Shift', - Control: 'Control', - Alt: 'Alt', - Meta: 'Meta' -} - -const GUEST_EDIT_COMMANDS: ReadonlySet = new Set([ - 'selectAll', - 'copy', - 'cut', - 'paste', - 'undo', - 'redo', - 'delete' -]) - -function isGuestEditCommand(command: unknown): command is GuestEditCommand { - return typeof command === 'string' && GUEST_EDIT_COMMANDS.has(command) -} - // Why: getType() is 'webview' only for guests hosted in an Orca window; offscreen and // headless pages have no host frame tree, so CDP input already reaches them. export function isEmbeddedGuest(webContents: WebContents): boolean { return webContents.getType() === 'webview' } -function electronKeyCode(key: unknown): string | null { - if (typeof key !== 'string') { - return null - } - if (Object.hasOwn(ELECTRON_KEY_CODES, key)) { - return ELECTRON_KEY_CODES[key] - } - if (/^F([1-9]|1\d|2[0-4])$/.test(key)) { - return key - } - // Why: Electron derives key/code from a US layout, which covers printable ASCII only. - return key.length === 1 && key >= '!' && key <= '~' ? key : null -} - -function electronModifiers(mask: unknown): ElectronModifier[] { - const bits = typeof mask === 'number' ? mask : 0 - return CDP_MODIFIER_NAMES.filter(([bit]) => (bits & bit) !== 0).map(([, name]) => name) -} - -// Why: Blink drops control characters from char events ("a\nb" types "ab"), so text that -// carries them cannot be typed this way and must take the fallback. -function typeableText(text: unknown): text is string { - return ( - typeof text === 'string' && - text.length > 0 && - Array.from(text).every((char) => { - const code = char.charCodeAt(0) - return code >= 0x20 && code !== 0x7f - }) - ) -} - -function charEvents(text: string, modifiers: ElectronModifier[]): GuestKeyboardEvent[] { - return Array.from(text, (char) => ({ - type: 'char', - keyCode: char, - modifiers, - skipIfUnhandled: true - })) -} - -/** Translates one CDP Input.dispatchKeyEvent into guest widget events, or null when it cannot. */ -export function planCdpKeyEvent(params: Record): GuestKeyPlan | null { - const commandsParam = params.commands ?? [] - if (!Array.isArray(commandsParam) || !commandsParam.every(isGuestEditCommand)) { - return null - } - const commands = commandsParam - const modifiers = electronModifiers(params.modifiers) - const { type, text } = params - if (type === 'char') { - return typeableText(text) ? { events: charEvents(text, modifiers), commands } : null - } - if (type !== 'keyDown' && type !== 'rawKeyDown' && type !== 'keyUp') { - return null - } - const keyCode = electronKeyCode(params.key) - // Why: a named key's text is a control character (Tab "\t", Backspace "\b") that the key - // event already acts on; only Enter's "\r" char is needed, to submit forms and insert newlines. - const isNamedKey = - typeof params.key === 'string' && - params.key !== ' ' && - Object.hasOwn(ELECTRON_KEY_CODES, params.key) - if (type === 'keyDown' && !isNamedKey && typeof text === 'string' && text !== '') { - if (!typeableText(text)) { - return null - } - // Why: a non-US-layout character has no Electron key code; the char event alone types it. - const down: GuestKeyboardEvent[] = - keyCode === null ? [] : [{ type: 'keyDown', keyCode, modifiers, skipIfUnhandled: true }] - return { events: [...down, ...charEvents(text, modifiers)], commands } - } - if (keyCode === null) { - // Why: a character outside the US layout went down as a char event alone; its keyUp has no - // widget form, and the fallback would focus the guest just to release a key never pressed. - const isCharacterKey = typeof params.key === 'string' && Array.from(params.key).length === 1 - return type === 'keyUp' && isCharacterKey ? { events: [], commands } : null - } - const event: GuestKeyboardEvent = { - type: type === 'keyUp' ? 'keyUp' : 'keyDown', - keyCode, - modifiers, - skipIfUnhandled: true - } - const enterChar: GuestKeyboardEvent[] = - type === 'keyDown' && params.key === 'Enter' && (text === '\r' || text === '\n') - ? [{ type: 'char', keyCode: '\r', modifiers, skipIfUnhandled: true }] - : [] - return { events: [event, ...enterChar], commands } -} - -/** Translates CDP Input.insertText into guest widget char events, or null when it cannot. */ -export function planInsertText(text: unknown): GuestKeyboardEvent[] | null { - return typeableText(text) ? charEvents(text, []) : null -} - // Why: sendInputEvent returns before the page handles the event, but CDP input replies only after. // A task-queue round trip in the page lets callers read the result of what they just typed. async function waitForPageToHandleInput(send: GuestCdpSender): Promise { @@ -195,6 +42,8 @@ async function waitForPageToHandleInput(send: GuestCdpSender): Promise { } function runEditCommand(webContents: WebContents, command: GuestEditCommand): void { + // Why: it runs after awaiting the page, so a host's access may have been revoked meanwhile. + assertAgentInputAdmitted(webContents) switch (command) { case 'selectAll': webContents.selectAll() @@ -242,8 +91,10 @@ async function sendToFrameSession( method: string, params: Record ): Promise { - return await withAgentInputInjection(webContents, () => - sendFrameSessionInput(webContents, send, sessionId, method, params) + return await withAgentInputInjection( + webContents, + () => sendFrameSessionInput(webContents, send, sessionId, method, params), + { method, params } ) } @@ -260,6 +111,7 @@ export async function dispatchPageKeyEvent( if (!isEmbeddedGuest(webContents)) { return await send('Input.dispatchKeyEvent', params) } + const keyInput = { method: 'Input.dispatchKeyEvent', params } const route = await resolveFocusedFrameRoute(webContents, send) if (route.kind === 'session') { // Why: Tab from a frame's last field can move focus out of the guest into Orca's window. @@ -285,17 +137,21 @@ export async function dispatchPageKeyEvent( ? await guardFocus() : () => {} try { - await withAgentInputInjection(webContents, async () => { - for (const event of plan.events) { - webContents.sendInputEvent(event) - } - if (plan.commands.length > 0 || plan.events.length > 0) { - await waitForPageToHandleInput(send) - } - for (const command of plan.commands) { - runEditCommand(webContents, command) - } - }) + await withAgentInputInjection( + webContents, + async () => { + for (const event of plan.events) { + webContents.sendInputEvent(event) + } + if (plan.commands.length > 0 || plan.events.length > 0) { + await waitForPageToHandleInput(send) + } + for (const command of plan.commands) { + runEditCommand(webContents, command) + } + }, + keyInput + ) } finally { release() } @@ -303,12 +159,41 @@ export async function dispatchPageKeyEvent( } const release = await focusGuestForCdpInput(webContents, send, guardFocus) try { - return await withAgentInputInjection(webContents, () => send('Input.dispatchKeyEvent', params)) + return await withAgentInputInjection( + webContents, + () => send('Input.dispatchKeyEvent', params), + keyInput + ) } finally { release() } } +/** Lets go of what a revoked host's command left pressed; a key goes where the focus routes it. */ +export async function releaseHeldPageInput( + webContents: WebContents, + release: HeldInputRelease, + guardFocus: AgentInputFocusGuard +): Promise { + const send: GuestCdpSender = (method, params, sessionId) => + sendGuestCdpCommand(webContents, method, params, sessionId) + // Why: the command's own debugger lease may already be gone, detaching the debugger. + const lease = acquireElectronDebugger(webContents) + try { + if (release.method === 'Input.dispatchKeyEvent') { + return await dispatchPageKeyEvent(webContents, release.params, send, guardFocus) + } + const result = await send(release.method, release.params) + if (release.method === 'Input.dispatchMouseEvent') { + const button = normalizeCdpPointerButton(String(release.params.button)) + releaseCdpPointerButton(cdpPointerStateFor(webContents), button) + } + return result + } finally { + lease.release() + } +} + export type TextInsertionRoute = { deliver: (params: Record) => Promise // Why: the fallback holds borrowed focus from routing until delivery; an insert abandoned in diff --git a/src/main/browser/guest-key-event-plan.ts b/src/main/browser/guest-key-event-plan.ts new file mode 100644 index 000000000000..32fe6f2036a3 --- /dev/null +++ b/src/main/browser/guest-key-event-plan.ts @@ -0,0 +1,162 @@ +import type { InputEvent, KeyboardInputEvent } from 'electron' + +/** Translates CDP key and text input into Electron guest widget events (#20595). */ + +type ElectronModifier = NonNullable[number] + +// Why: Electron honours this NativeWebKeyboardEvent flag although its typings omit it; an +// unhandled agent key must not be redispatched to Orca's window or application menu. +export type GuestKeyboardEvent = KeyboardInputEvent & { skipIfUnhandled: true } + +export type GuestEditCommand = 'selectAll' | 'copy' | 'cut' | 'paste' | 'undo' | 'redo' | 'delete' + +type GuestKeyPlan = { + events: GuestKeyboardEvent[] + // Why: macOS editing shortcuts arrive as CDP `commands`; widget key events skip the + // browser-side key bindings that would run them. + commands: GuestEditCommand[] +} + +const CDP_MODIFIER_NAMES: [number, ElectronModifier][] = [ + [1, 'alt'], + [2, 'control'], + [4, 'meta'], + [8, 'shift'] +] + +// CDP `key` -> Electron accelerator key code, for keys whose names differ or are not single characters. +const ELECTRON_KEY_CODES: Record = { + Enter: 'Enter', + Tab: 'Tab', + Backspace: 'Backspace', + Delete: 'Delete', + Escape: 'Escape', + ' ': 'Space', + ArrowUp: 'Up', + ArrowDown: 'Down', + ArrowLeft: 'Left', + ArrowRight: 'Right', + Home: 'Home', + End: 'End', + PageUp: 'PageUp', + PageDown: 'PageDown', + Insert: 'Insert', + CapsLock: 'CapsLock', + NumLock: 'NumLock', + ScrollLock: 'ScrollLock', + PrintScreen: 'PrintScreen', + Shift: 'Shift', + Control: 'Control', + Alt: 'Alt', + Meta: 'Meta' +} + +const GUEST_EDIT_COMMANDS: ReadonlySet = new Set([ + 'selectAll', + 'copy', + 'cut', + 'paste', + 'undo', + 'redo', + 'delete' +]) + +function isGuestEditCommand(command: unknown): command is GuestEditCommand { + return typeof command === 'string' && GUEST_EDIT_COMMANDS.has(command) +} + +function electronKeyCode(key: unknown): string | null { + if (typeof key !== 'string') { + return null + } + if (Object.hasOwn(ELECTRON_KEY_CODES, key)) { + return ELECTRON_KEY_CODES[key] + } + if (/^F([1-9]|1\d|2[0-4])$/.test(key)) { + return key + } + // Why: Electron derives key/code from a US layout, which covers printable ASCII only. + return key.length === 1 && key >= '!' && key <= '~' ? key : null +} + +function electronModifiers(mask: unknown): ElectronModifier[] { + const bits = typeof mask === 'number' ? mask : 0 + return CDP_MODIFIER_NAMES.filter(([bit]) => (bits & bit) !== 0).map(([, name]) => name) +} + +// Why: Blink drops control characters from char events ("a\nb" types "ab"), so text that +// carries them cannot be typed this way and must take the fallback. +function typeableText(text: unknown): text is string { + return ( + typeof text === 'string' && + text.length > 0 && + Array.from(text).every((char) => { + const code = char.charCodeAt(0) + return code >= 0x20 && code !== 0x7f + }) + ) +} + +function charEvents(text: string, modifiers: ElectronModifier[]): GuestKeyboardEvent[] { + return Array.from(text, (char) => ({ + type: 'char', + keyCode: char, + modifiers, + skipIfUnhandled: true + })) +} + +/** Translates one CDP Input.dispatchKeyEvent into guest widget events, or null when it cannot. */ +export function planCdpKeyEvent(params: Record): GuestKeyPlan | null { + const commandsParam = params.commands ?? [] + if (!Array.isArray(commandsParam) || !commandsParam.every(isGuestEditCommand)) { + return null + } + const commands = commandsParam + const modifiers = electronModifiers(params.modifiers) + const { type, text } = params + if (type === 'char') { + return typeableText(text) ? { events: charEvents(text, modifiers), commands } : null + } + if (type !== 'keyDown' && type !== 'rawKeyDown' && type !== 'keyUp') { + return null + } + const keyCode = electronKeyCode(params.key) + // Why: a named key's text is a control character (Tab "\t", Backspace "\b") that the key + // event already acts on; only Enter's "\r" char is needed, to submit forms and insert newlines. + const isNamedKey = + typeof params.key === 'string' && + params.key !== ' ' && + Object.hasOwn(ELECTRON_KEY_CODES, params.key) + if (type === 'keyDown' && !isNamedKey && typeof text === 'string' && text !== '') { + if (!typeableText(text)) { + return null + } + // Why: a non-US-layout character has no Electron key code; the char event alone types it. + const down: GuestKeyboardEvent[] = + keyCode === null ? [] : [{ type: 'keyDown', keyCode, modifiers, skipIfUnhandled: true }] + return { events: [...down, ...charEvents(text, modifiers)], commands } + } + if (keyCode === null) { + // Why: a character outside the US layout went down as a char event alone; its keyUp has no + // widget form, and the fallback would focus the guest just to release a key never pressed. + const isCharacterKey = typeof params.key === 'string' && Array.from(params.key).length === 1 + return type === 'keyUp' && isCharacterKey ? { events: [], commands } : null + } + const event: GuestKeyboardEvent = { + type: type === 'keyUp' ? 'keyUp' : 'keyDown', + keyCode, + modifiers, + skipIfUnhandled: true + } + const enterChar: GuestKeyboardEvent[] = + type === 'keyDown' && params.key === 'Enter' && (text === '\r' || text === '\n') + ? [{ type: 'char', keyCode: '\r', modifiers, skipIfUnhandled: true }] + : [] + return { events: [event, ...enterChar], commands } +} + +/** Translates CDP Input.insertText into guest widget char events, or null when it cannot. */ +export function planInsertText(text: unknown): GuestKeyboardEvent[] | null { + return typeableText(text) ? charEvents(text, []) : null +} diff --git a/src/main/browser/remote-cli-page-admission.ts b/src/main/browser/remote-cli-page-admission.ts new file mode 100644 index 000000000000..c80979150835 --- /dev/null +++ b/src/main/browser/remote-cli-page-admission.ts @@ -0,0 +1,146 @@ +/** + * The pages an SSH host's command is driving right now, each with the check that refuses its next + * step once that host is revoked. Every page-affecting sink (guest CDP sends, native input events, + * edit commands) consults it, so no queued or deferred step of a running command outlives the + * revocation. + */ +import { AsyncLocalStorage } from 'node:async_hooks' +import { runOutsideRemoteCliBrowserScope } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope' + +/** The CDP input event that lets go of something a command pressed. */ +export type HeldInputRelease = { method: string; params: Record } + +type Admission = { + admit: () => void + /** What this command pressed and has not released, keyed by button, key or touch. */ + held: Map +} + +const admissions = new Map() +/** The admission a proxy message arrived under; its deferred steps keep it after the hold ends. */ +const messageAdmission = new AsyncLocalStorage<() => void>() + +/** + * Runs a host's command holding the page. If the host is revoked mid-press, the command's own + * release is refused like every other step, so the desktop lets go of what it left held once the + * command ends: from outside the host's scope, with routing probes no longer refused. + */ +export async function runWithRemoteCliPageAdmission( + webContentsId: number, + admit: () => void, + run: () => Promise, + releaseHeld?: (release: HeldInputRelease) => Promise +): Promise { + const admission: Admission = { admit, held: new Map() } + admissions.set(webContentsId, admission) + try { + return await run() + } finally { + if (admissions.get(webContentsId) === admission) { + admissions.delete(webContentsId) + } + if (releaseHeld && admission.held.size > 0 && isAdmissionRevoked(admit)) { + await runOutsideRemoteCliBrowserScope(() => releaseAll(admission.held, releaseHeld)) + } + } +} + +/** Throws while a revoked host's command still holds this page; a no-op otherwise. */ +export function assertRemoteCliPageAdmitted(webContentsId: number | undefined): void { + if (webContentsId !== undefined) { + admissions.get(webContentsId)?.admit() + } + messageAdmission.getStore()?.() +} + +/** Runs a proxy message's handling, deferred steps included, under the page's current hold. */ +export function runUnderRemoteCliPageAdmission(webContentsId: number, handle: () => T): T { + const admission = admissions.get(webContentsId) + return admission ? messageAdmission.run(admission.admit, handle) : handle() +} + +/** Records input that reached a page a host's command holds, so a revocation can release it. */ +export function noteRemoteCliInput( + webContentsId: number | undefined, + method: string, + params?: Record +): void { + const admission = webContentsId === undefined ? undefined : admissions.get(webContentsId) + if (admission) { + trackInput(admission, method, params) + } +} + +/** True once the host behind this admission check was revoked. */ +export function isAdmissionRevoked(admit: () => void): boolean { + try { + admit() + return false + } catch { + return true + } +} + +async function releaseAll( + held: Map, + releaseHeld: (release: HeldInputRelease) => Promise +): Promise { + for (const release of held.values()) { + try { + await releaseHeld(release) + } catch { + // Best effort: the page may be gone, and nothing else is waiting on this release. + } + } +} + +export function isInputRelease(params: Record): boolean { + return ['mouseReleased', 'keyUp', 'touchEnd', 'touchCancel'].includes(String(params.type)) +} + +function trackInput( + admission: Admission, + method: string | undefined, + params: Record | undefined +): void { + const type = params?.type + if (!params || typeof type !== 'string') { + return + } + if ( + method === 'Input.dispatchMouseEvent' && + (type === 'mousePressed' || type === 'mouseReleased') + ) { + const id = `mouse:${String(params.button ?? 'left')}` + if (type === 'mouseReleased') { + admission.held.delete(id) + return + } + // Why off the page: letting go there completes no click or drop the host had started. + admission.held.set(id, { + method, + params: { type: 'mouseReleased', x: -1, y: -1, button: params.button ?? 'left', buttons: 0 } + }) + } else if ( + method === 'Input.dispatchKeyEvent' && + ['keyDown', 'rawKeyDown', 'keyUp'].includes(type) + ) { + const id = `key:${String(params.code ?? params.key ?? '')}` + if (type === 'keyUp') { + admission.held.delete(id) + return + } + const { key, code, windowsVirtualKeyCode, nativeVirtualKeyCode } = params + admission.held.set(id, { + method, + params: { type: 'keyUp', key, code, windowsVirtualKeyCode, nativeVirtualKeyCode } + }) + } else if (method === 'Input.dispatchTouchEvent' && type.startsWith('touch')) { + if (type === 'touchStart') { + // Why cancel: a cancelled touch completes no tap the host had started. + admission.held.set('touch', { method, params: { type: 'touchCancel', touchPoints: [] } }) + } else if (type === 'touchEnd' || type === 'touchCancel') { + admission.held.delete('touch') + } + } +} diff --git a/src/main/ipc/desktop-browser-bridge-wiring.ts b/src/main/ipc/desktop-browser-bridge-wiring.ts index d7537233422c..e5c10f3a8c1e 100644 --- a/src/main/ipc/desktop-browser-bridge-wiring.ts +++ b/src/main/ipc/desktop-browser-bridge-wiring.ts @@ -5,6 +5,7 @@ import { randomUUID } from 'node:crypto' import { webContents, type BrowserWindow } from 'electron' import { + DESKTOP_BROWSER_BRIDGE_CANCEL_CAPABILITY, DESKTOP_BROWSER_BRIDGE_RUNTIME_CAPABILITY, isDesktopBrowserBridgeMethod } from '../../shared/desktop-browser-bridge-protocol' @@ -135,7 +136,8 @@ export function installDesktopBrowserBridge(options: { { clientCapabilities: [ ...ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES, - DESKTOP_BROWSER_BRIDGE_RUNTIME_CAPABILITY + DESKTOP_BROWSER_BRIDGE_RUNTIME_CAPABILITY, + DESKTOP_BROWSER_BRIDGE_CANCEL_CAPABILITY ] } ) diff --git a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-dispatch.test.ts b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-dispatch.test.ts index 79725b7dcbe5..00d4ac308bbd 100644 --- a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-dispatch.test.ts +++ b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-dispatch.test.ts @@ -2,6 +2,7 @@ import '../rpc/unused-default-rpc-methods.test-fixture' import { afterEach, describe, expect, it, vi } from 'vitest' import { z } from 'zod' import { + DESKTOP_BROWSER_BRIDGE_CANCEL_CAPABILITY, DESKTOP_BROWSER_BRIDGE_RUNTIME_CAPABILITY, DESKTOP_BROWSER_BRIDGE_SETTING_LABEL } from '../../../shared/desktop-browser-bridge-protocol' @@ -54,6 +55,7 @@ function orcadRuntime(options: { localBrowser?: boolean } = {}) { method, params, caller: context.caller, + signal: context.signal, hostEnabled: true, registry: getDesktopBrowserBridgeRegistry(runtime), localBrowserAvailable: options.localBrowser === true, @@ -73,7 +75,12 @@ const DESKTOP = { clientCapabilities: [DESKTOP_BROWSER_BRIDGE_RUNTIME_CAPABILITY] } -async function attachDesktop(dispatcher: RpcDispatcher, options = DESKTOP) { +async function attachDesktop( + dispatcher: RpcDispatcher, + options: + | typeof DESKTOP + | (Omit & { clientCapabilities: string[] }) = DESKTOP +) { const events: Record[] = [] const done = dispatcher.dispatchStreaming( { @@ -287,4 +294,40 @@ describe('desktop browser bridge on orcad', () => { expect(response).toMatchObject({ ok: true, result: { url: 'local' } }) expect(events).toHaveLength(1) }) + + it.each([ + { + name: 'a cancel-capable desktop hears', + capabilities: [DESKTOP_BROWSER_BRIDGE_CANCEL_CAPABILITY], + cancels: true + }, + { name: 'an older desktop never hears', capabilities: [], cancels: false } + ])('$name that the CLI cancelled a forwarded command', async ({ capabilities, cancels }) => { + setDesktopBrowserBridgeHostEnabled(true) + const { runtime } = orcadRuntime() + const dispatcher = new RpcDispatcher({ runtime, methods: METHODS }) + const { events } = await attachDesktop(dispatcher, { + ...DESKTOP, + clientCapabilities: [DESKTOP_BROWSER_BRIDGE_RUNTIME_CAPABILITY, ...capabilities] + }) + const cli = new AbortController() + + const call = dispatcher.dispatch( + { + id: 'cli-goto', + authToken: 'owner', + method: 'browser.goto', + params: { url: 'https://example.com', worktree: 'path:/srv/app' } + }, + { signal: cli.signal } + ) + await vi.waitFor(() => expect(events).toHaveLength(2)) + const forwarded = ForwardedEvent.parse(events[1]) + cli.abort() + + await expect(call).resolves.toMatchObject({ ok: false }) + expect(events.slice(2)).toEqual( + cancels ? [{ type: 'cancel', requestId: forwarded.requestId }] : [] + ) + }) }) diff --git a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-registry.test.ts b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-registry.test.ts index 53cf377a205d..a0e98c890d35 100644 --- a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-registry.test.ts +++ b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-registry.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { DesktopBrowserBridgeEvent } from '../../../shared/desktop-browser-bridge-protocol' import { DesktopBrowserBridgeRegistry, @@ -31,4 +31,72 @@ describe('DesktopBrowserBridgeRegistry', () => { expect(events[0]).toMatchObject({ type: 'request', timeoutMs: 50_000 }) }) + + it('tells a cancel-capable desktop to drop a command the CLI cancelled', async () => { + const registry = new DesktopBrowserBridgeRegistry() + const events: DesktopBrowserBridgeEvent[] = [] + registry.attach({ + connectionId: 'c', + pairedDeviceId: 'd', + acceptsCancel: true, + emit: (event) => events.push(event) + }) + const cli = new AbortController() + + const pending = registry.forward( + { method: 'browser.click', params: {} }, + { timeoutMs: 60_000, signal: cli.signal } + ) + cli.abort() + + await expect(pending).rejects.toMatchObject({ code: 'browser_aborted' }) + const sent = events[0] + expect(sent?.type).toBe('request') + expect(events[1]).toEqual({ + type: 'cancel', + requestId: sent?.type === 'request' ? sent.requestId : '' + }) + }) + + it('never sends a cancel to an older desktop, which would drop its attachment on it', async () => { + const registry = new DesktopBrowserBridgeRegistry() + const events: DesktopBrowserBridgeEvent[] = [] + registry.attach({ connectionId: 'c', pairedDeviceId: 'd', emit: (event) => events.push(event) }) + const cli = new AbortController() + + const pending = registry.forward( + { method: 'browser.click', params: {} }, + { timeoutMs: 60_000, signal: cli.signal } + ) + cli.abort() + + await expect(pending).rejects.toMatchObject({ code: 'browser_aborted' }) + expect(events.map((event) => event.type)).toEqual(['request']) + }) + + it('cancels on the desktop when orcad stops waiting at its deadline', async () => { + vi.useFakeTimers() + try { + const registry = new DesktopBrowserBridgeRegistry() + const events: DesktopBrowserBridgeEvent[] = [] + registry.attach({ + connectionId: 'c', + pairedDeviceId: 'd', + acceptsCancel: true, + emit: (event) => events.push(event) + }) + const pending = registry.forward( + { method: 'browser.click', params: {} }, + { timeoutMs: 60_000 } + ) + pending.catch(() => {}) + + await vi.advanceTimersByTimeAsync(60_000) + + await expect(pending).rejects.toMatchObject({ code: 'browser_timeout' }) + expect(events.map((event) => event.type)).toEqual(['request', 'cancel']) + } finally { + vi.useRealTimers() + } + }) }) diff --git a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-registry.ts b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-registry.ts index 0f10f2423cf1..3434827d0d75 100644 --- a/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-registry.ts +++ b/src/main/runtime/desktop-browser-bridge/desktop-browser-bridge-registry.ts @@ -22,6 +22,8 @@ type Attachment = { attachmentId: string connectionId: string pairedDeviceId: string + /** True when the desktop declared it understands `cancel`; older desktops never receive one. */ + acceptsCancel?: boolean emit: (event: DesktopBrowserBridgeEvent) => void } @@ -75,12 +77,26 @@ export class DesktopBrowserBridgeRegistry { this.pending.delete(requestId) options.signal?.removeEventListener('abort', onAbort) } + let emitted = false + // Why: without this a command still queued on the desktop runs after the CLI gave up on it. + const cancelOnDesktop = (): void => { + if (!emitted || !attachment.acceptsCancel || !this.attachments.includes(attachment)) { + return + } + try { + attachment.emit({ type: 'cancel', requestId }) + } catch { + // The socket is gone, so the desktop's own detach revokes the command. + } + } const onAbort = (): void => { settle() + cancelOnDesktop() reject(new BrowserError('browser_aborted', 'The browser command was cancelled.')) } const timer = setTimeout(() => { settle() + cancelOnDesktop() reject( new BrowserError('browser_timeout', 'The desktop did not answer the browser command.') ) @@ -115,6 +131,7 @@ export class DesktopBrowserBridgeRegistry { ), ...(request.workspaceId ? { workspaceId: request.workspaceId } : {}) }) + emitted = true } catch { this.pending.get(requestId)?.reject(unavailableError()) } diff --git a/src/main/runtime/desktop-browser-bridge/remote-cli-browser-scope.ts b/src/main/runtime/desktop-browser-bridge/remote-cli-browser-scope.ts index 2a1ad031dde1..469f27d07e7a 100644 --- a/src/main/runtime/desktop-browser-bridge/remote-cli-browser-scope.ts +++ b/src/main/runtime/desktop-browser-bridge/remote-cli-browser-scope.ts @@ -55,6 +55,36 @@ export function captureRemoteCliBrowserRevocation(): (webContentsId?: number) => } } +/** + * Captured where a browser command is queued: `run` puts the queued work back in exactly that + * scope (none for a desktop-local command), never the queue runner's, which may be another host's. + */ +export function captureRemoteCliBrowserScope(): { + bridged: boolean + run(work: () => T): T +} { + const active = scope.getStore() + return { + bridged: active !== undefined, + run: (work) => (active ? scope.run(active, work) : scope.exit(work)) + } +} + +/** The signal that fires when the host behind the current command is revoked, if any. */ +export function captureRemoteCliBrowserRevokedSignal(): AbortSignal | undefined { + return scope.getStore()?.revoked +} + +/** Runs `work` in no host's scope, so long-lived resources it creates never inherit one. */ +export function runOutsideRemoteCliBrowserScope(work: () => T): T { + return scope.exit(work) +} + +/** Throws inside a bridged command once it was revoked or `webContentsId` stopped being admitted. */ +export function assertRemoteCliBrowserAccess(webContentsId?: number): void { + captureRemoteCliBrowserRevocation()(webContentsId) +} + /** Null outside a bridged command; inside one, the scoped workspace or a refusal. */ export function resolveRemoteCliBrowserScopeSelector( selector: string diff --git a/src/main/runtime/rpc/methods/desktop-browser-bridge.ts b/src/main/runtime/rpc/methods/desktop-browser-bridge.ts index 0014cc3ef33b..468bab72c5fa 100644 --- a/src/main/runtime/rpc/methods/desktop-browser-bridge.ts +++ b/src/main/runtime/rpc/methods/desktop-browser-bridge.ts @@ -1,4 +1,5 @@ import { + DESKTOP_BROWSER_BRIDGE_CANCEL_CAPABILITY, DESKTOP_BROWSER_BRIDGE_RUNTIME_CAPABILITY, DesktopBrowserBridgeAttachParams, DesktopBrowserBridgeResultParams @@ -35,7 +36,13 @@ export const DESKTOP_BROWSER_BRIDGE_RPC_METHODS = [ handler: async (params, context, emit) => { const caller = requireBridgeCaller(context) const { runtime, signal } = context - const handle = getDesktopBrowserBridgeRegistry(runtime).attach({ ...caller, emit }) + const acceptsCancel = + context.clientCapabilities?.includes(DESKTOP_BROWSER_BRIDGE_CANCEL_CAPABILITY) ?? false + const handle = getDesktopBrowserBridgeRegistry(runtime).attach({ + ...caller, + acceptsCancel, + emit + }) let release = (): void => {} const released = new Promise((resolve) => { release = resolve diff --git a/src/shared/desktop-browser-bridge-protocol.ts b/src/shared/desktop-browser-bridge-protocol.ts index 17425962c6b6..4b739f7d420c 100644 --- a/src/shared/desktop-browser-bridge-protocol.ts +++ b/src/shared/desktop-browser-bridge-protocol.ts @@ -9,6 +9,11 @@ import { z } from 'zod' export const DESKTOP_BROWSER_BRIDGE_RUNTIME_CAPABILITY = 'browser.desktopBridge.v1' +/** + * Declared by a desktop that understands a `cancel` event. orcad never sends one to a desktop + * without it: an older desktop drops its whole attachment on an event type it cannot parse. + */ +export const DESKTOP_BROWSER_BRIDGE_CANCEL_CAPABILITY = 'browser.desktopBridge.cancel.v1' export const DESKTOP_BROWSER_BRIDGE_UNAVAILABLE_CODE = 'desktop_browser_bridge_unavailable' export const DESKTOP_BROWSER_BRIDGE_NOT_FORWARDED_CODE = 'desktop_browser_bridge_not_forwarded' @@ -120,7 +125,9 @@ export type DesktopBrowserBridgeRequest = z.infer