refactor(browser): one page gate and one owner for a host command's hold

src/main/browser/agent-browser-bridge-host-revocation.test.ts

@@ -43,7 +43,10 @@ import {
   captureRemoteCliBrowserScope,
   runInRemoteCliBrowserScope
 } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope'
-import { assertRemoteCliPageAdmitted } from './remote-cli-page-admission'
+import {
+  assertRemoteCliPageAccess,
+  runUnderRemoteCliPageAdmission
+} from './remote-cli-page-admission'
 import {
   createFakeAgentBrowserChild,
   mockBrowserManager,
@@ -57,6 +60,11 @@ overrideBridgeWebContentsLookup(AgentBrowserBridge.prototype, webContentsFromIdM
 
 const WORKSPACE = 'repo-host::/srv/app'
 
+/** A helper step reaching the page's CDP proxy now. */
+function assertHelperStepAdmitted(): void {
+  runUnderRemoteCliPageAdmission(100, () => assertRemoteCliPageAccess(100))
+}
+
 function dispatchedMouseEvents(wc: ReturnType<typeof mockWebContents>): unknown[] {
   return wc.debugger.sendCommand.mock.calls.filter((call) => call[0] === 'Input.dispatchMouseEvent')
 }
@@ -126,24 +134,21 @@ describe('AgentBrowserBridge under a host revocation', () => {
       revoked.signal
     )
     await vi.waitFor(() => expect(helperCallbacks).toHaveLength(1))
-    expect(() => assertRemoteCliPageAdmitted(100)).not.toThrow()
+    expect(() => assertHelperStepAdmitted()).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<typeof vi.fn> }
     let heldWhenRetired = false
     proxy.stop.mockImplementation(async () => {
-      heldWhenRetired = (() => {
-        try {
-          assertRemoteCliPageAdmitted(100)
-          return false
-        } catch {
-          return true
-        }
-      })()
+      try {
+        assertHelperStepAdmitted()
+      } catch {
+        heldWhenRetired = true
+      }
     })
     revoked.abort()
     // The helper's next CDP step reaches the page's proxy and is refused there.
-    expect(() => assertRemoteCliPageAdmitted(100)).toThrow('stopped sharing')
+    expect(() => assertHelperStepAdmitted()).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]!(
@@ -153,7 +158,7 @@ describe('AgentBrowserBridge under a host revocation', () => {
     )
 
     await expect(click).rejects.toMatchObject({ code: 'desktop_browser_bridge_unavailable' })
-    expect(() => assertRemoteCliPageAdmitted(100)).not.toThrow()
+    expect(() => assertHelperStepAdmitted()).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(

src/main/browser/agent-browser-bridge-queue.ts

@@ -6,14 +6,12 @@ import type {
   EnqueueTargetedCommandOptions,
   ResolvedBrowserCommandTarget
 } from './agent-browser-bridge-types'
-import { isAdmissionRevoked, runWithRemoteCliPageAdmission } from './remote-cli-page-admission'
+import { 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
+  captureRemoteCliBrowserScope
 } from '../runtime/desktop-browser-bridge/remote-cli-browser-scope'
 
 export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown {
@@ -95,7 +93,7 @@ export abstract class AgentBrowserBridgeQueue extends AgentBrowserBridgeShutdown
                 const gate = (): void => assertNotRevoked(target.webContentsId)
                 gate()
                 return origin.bridged
-                  ? this.runAdmitted(session, target.webContentsId, gate, () =>
+                  ? this.runHostCommand(session, target.webContentsId, gate, origin.revoked, () =>
                       execute(session, target)
                     )
                   : execute(session, target)
@@ -127,53 +125,31 @@ 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<T>(
+  /** Holds the page for a host's command, so a revocation stops every later step of it. */
+  private runHostCommand<T>(
     sessionName: string,
     webContentsId: number,
     gate: () => void,
+    revoked: AbortSignal | undefined,
     execute: () => Promise<T>
   ): Promise<T> {
-    // 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<void> | null = null
-    const retire = (): Promise<void> =>
-      (retiring ??= runOutsideRemoteCliBrowserScope(() =>
+    return runWithRemoteCliPageAdmission(webContentsId, gate, execute, {
+      revoked,
+      // Why keep the queue: commands waiting on the page still run, on a fresh helper session.
+      retire: () =>
         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<T> => {
-      try {
-        return await execute()
-      } finally {
-        signal?.removeEventListener('abort', onRevoked)
-        if (retiring || isAdmissionRevoked(gate)) {
-          await retire()
-        }
-      }
-    }
-    try {
-      return await runWithRemoteCliPageAdmission(webContentsId, gate, held, (release) => {
+        }),
+      releaseHeld: (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<void> {

src/main/browser/agent-input-injection.ts

@@ -1,10 +1,5 @@
 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'
+import { assertRemoteCliPageAccess } from './remote-cli-page-admission'
 
 // 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.
@@ -14,38 +9,19 @@ 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<string, unknown> }
-
-/** 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<T>(
   webContents: WebContents,
-  run: () => Promise<T>,
-  input?: AgentInputEvent
+  run: () => Promise<T>
 ): Promise<T> {
-  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)
-  }
+  assertRemoteCliPageAccess(webContents.id)
   const depth = injectionDepth.get(webContents) ?? 0
   injectionDepth.set(webContents, depth + 1)
   if (depth === 0) {
     webContents.setIgnoreMenuShortcuts(true)
   }
   try {
-    const result = await run()
-    if (input && releasing) {
-      noteRemoteCliInput(webContents.id, input.method, input.params)
-    }
-    return result
+    return await run()
   } finally {
     const remaining = (injectionDepth.get(webContents) ?? 1) - 1
     if (remaining > 0) {

src/main/browser/cdp-debugger-channel.ts

@@ -1,7 +1,6 @@
 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'
@@ -94,12 +93,6 @@ export class CdpDebuggerChannel {
     params: Record<string, unknown>,
     sessionId?: string
   ): Promise<unknown> {
-    // 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)

src/main/browser/cdp-page-input-commands.ts

@@ -85,10 +85,8 @@ export class CdpPageInputCommands {
       return await this.debuggerChannel.sendDebuggerCommand(method, params, sessionId)
     }
     const send = (): Promise<unknown> =>
-      withAgentInputInjection(
-        this.webContents,
-        () => this.debuggerChannel.sendDebuggerCommand(method, params, sessionId),
-        { method, params }
+      withAgentInputInjection(this.webContents, () =>
+        this.debuggerChannel.sendDebuggerCommand(method, params, sessionId)
       )
     const routedByFocus = !sessionId && isFocusRoutedCdpInput(method)
     if (!routedByFocus && !isFocusTakingCdpInput(method, params)) {

src/main/browser/cdp-ws-proxy-host-admission.test.ts

@@ -104,7 +104,7 @@ describe('CdpWsProxy under a host command', () => {
         }
       },
       () => new Promise<void>((resolve) => (finish = resolve)),
-      (release) => releaseHeldPageInput(page, release, () => guardFocus())
+      { releaseHeld: (release) => releaseHeldPageInput(page, release, () => guardFocus()) }
     )
     return {
       revoke: () => (revoked = true),

src/main/browser/cdp-ws-proxy.ts

@@ -13,7 +13,7 @@ import type { AgentInputFocusGuard } from './agent-input-focus-guard'
 import { CdpPageInputCommands } from './cdp-page-input-commands'
 import { isTrustedCdpLoopbackRequest } from './cdp-loopback-request-guard'
 import {
-  assertRemoteCliPageAdmitted,
+  assertRemoteCliPageAccess,
   runUnderRemoteCliPageAdmission
 } from './remote-cli-page-admission'
 
@@ -186,7 +186,7 @@ export class CdpWsProxy {
     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)
+      assertRemoteCliPageAccess(this.webContents.id)
     } catch (error) {
       this.responder.sendError(
         clientId,

src/main/browser/guest-cdp-command.ts

@@ -1,7 +1,6 @@
 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'
+import { assertRemoteCliPageAccess, trackRemoteCliInput } 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
@@ -28,8 +27,8 @@ export function sendGuestCdpCommand(
   sessionId?: string
 ): Promise<unknown> {
   try {
-    // Why: only a host's own command runs in its scope; desktop-owned sends are never refused here.
-    assertRemoteCliBrowserAccess(guest.id)
+    // Why: only a host's own work carries its scope; desktop-owned sends are never refused here.
+    assertRemoteCliPageAccess(guest.id)
   } catch (error) {
     return Promise.reject(error)
   }
@@ -43,20 +42,11 @@ export function sendGuestCdpCommand(
       )
     )
   }
-  // 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 trackRemoteCliInput(guest.id, method, params, () =>
+    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
 }