fix(desktop): harden browser and terminal tab activity

Author: Sg312

apps/desktop/src/main/browser-agent/driver.test.ts

@@ -176,6 +176,150 @@ describe('executeTool', () => {
     }
   })
 
+  it('cancels the exact takeover and clears its attention state immediately', async () => {
+    await driver.executeTool('chat-test', 'browser_open_tab', {})
+    vi.useFakeTimers()
+    try {
+      const takeover = driver.executeTool(
+        'chat-test',
+        'browser_request_takeover',
+        { reason: 'Please finish in the browser' },
+        'tool-takeover'
+      )
+      await vi.advanceTimersByTimeAsync(0)
+      expect(session.getTabsState().automationNeedsAttention).toBe(true)
+
+      expect(driver.cancelTool('chat-test', 'tool-takeover')).toBe(true)
+      expect(session.getTabsState().automationNeedsAttention).toBe(false)
+      await vi.advanceTimersByTimeAsync(1_500)
+
+      await expect(takeover).resolves.toMatchObject({
+        ok: false,
+        error: expect.stringContaining('cancelled'),
+      })
+      await expect(
+        driver.executeTool('chat-test', 'browser_list_tabs', {}, 'tool-takeover')
+      ).resolves.toMatchObject({
+        ok: false,
+        error: expect.stringContaining('cancelled before it started'),
+      })
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('does not let cancelled takeover cleanup clear a newer takeover', async () => {
+    await driver.executeTool('chat-test', 'browser_open_tab', {})
+    vi.useFakeTimers()
+    try {
+      const firstTakeover = driver.executeTool(
+        'chat-test',
+        'browser_request_takeover',
+        { reason: 'First handoff' },
+        'tool-takeover-first'
+      )
+      await vi.advanceTimersByTimeAsync(0)
+
+      expect(driver.cancelTool('chat-test', 'tool-takeover-first')).toBe(true)
+      await expect(firstTakeover).resolves.toMatchObject({
+        ok: false,
+        error: expect.stringContaining('cancelled'),
+      })
+
+      const secondTakeover = driver.executeTool(
+        'chat-test',
+        'browser_request_takeover',
+        { reason: 'Second handoff' },
+        'tool-takeover-second'
+      )
+      await vi.advanceTimersByTimeAsync(0)
+      expect(session.getTabsState().automationNeedsAttention).toBe(true)
+
+      // Let the detached first takeover observe cancellation and run finally.
+      await vi.advanceTimersByTimeAsync(1_500)
+      expect(session.getTabsState().automationNeedsAttention).toBe(true)
+
+      await driver.handlePanelAction('chat-test', { action: 'takeover-done' })
+      await vi.advanceTimersByTimeAsync(1_500)
+      await expect(secondTakeover).resolves.toMatchObject({
+        ok: true,
+        result: { completed: true },
+      })
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('honors cancellation that arrives before the authorized tool invocation', async () => {
+    expect(driver.cancelTool('chat-test', 'tool-before-authorization')).toBe(true)
+
+    await expect(
+      driver.executeTool('chat-test', 'browser_list_tabs', {}, 'tool-before-authorization')
+    ).resolves.toMatchObject({
+      ok: false,
+      error: expect.stringContaining('cancelled before it started'),
+    })
+  })
+
+  it('settles native automation activity immediately when an active tool is cancelled', async () => {
+    await driver.executeTool('chat-test', 'browser_open_tab', {})
+    vi.useFakeTimers()
+    try {
+      const waiting = driver.executeTool(
+        'chat-test',
+        'browser_wait_for',
+        { timeoutMs: 120_000 },
+        'tool-waiting'
+      )
+      await vi.advanceTimersByTimeAsync(0)
+      expect(session.getTabsState().automationActive).toBe(true)
+
+      expect(driver.cancelActiveTool('chat-test')).toBe(true)
+      await vi.advanceTimersByTimeAsync(0)
+      expect(session.getTabsState().automationActive).toBe(false)
+      await expect(waiting).resolves.toMatchObject({
+        ok: false,
+        error: expect.stringContaining('cancelled'),
+      })
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
+  it('cancels queued pre-boundary tools while allowing later browser work', async () => {
+    await driver.executeTool('chat-test', 'browser_open_tab', {})
+    vi.useFakeTimers()
+    try {
+      const waiting = driver.executeTool(
+        'chat-test',
+        'browser_wait_for',
+        { timeoutMs: 120_000 },
+        'tool-active'
+      )
+      await vi.advanceTimersByTimeAsync(0)
+      const queuedOpen = driver.executeTool('chat-test', 'browser_open_tab', {}, 'tool-queued')
+
+      expect(driver.cancelActiveTool('chat-test')).toBe(true)
+
+      await expect(waiting).resolves.toMatchObject({
+        ok: false,
+        error: expect.stringContaining('cancelled'),
+      })
+      await expect(queuedOpen).resolves.toMatchObject({
+        ok: false,
+        error: expect.stringContaining('cancelled before it started'),
+      })
+      expect(session.getTabsState().tabs).toHaveLength(1)
+
+      await expect(
+        driver.executeTool('chat-test', 'browser_open_tab', {}, 'tool-after-boundary')
+      ).resolves.toMatchObject({ ok: true })
+      expect(session.getTabsState().tabs).toHaveLength(2)
+    } finally {
+      vi.useRealTimers()
+    }
+  })
+
   it('releases an abandoned takeover when a newer browser action arrives', async () => {
     await driver.executeTool('chat-test', 'browser_open_tab', {})
     vi.useFakeTimers()

apps/desktop/src/main/browser-agent/driver.ts

@@ -114,8 +114,16 @@ interface DriverScopeState {
   takeoverActive: boolean
   takeoverDone: boolean
   takeoverResponse: string | null
+  /** Invocation that currently owns the shared takeover response state. */
+  takeoverInvocationEpoch: number | null
   /** Monotonic browser-tool invocation id used to supersede an abandoned takeover. */
   toolInvocationEpoch: number
+  /** Exact client tool currently at the head of this scope's serialized queue. */
+  activeToolCallId: string | null
+  /** Rejects the active queue entry while its underlying Chromium work winds down. */
+  activeToolCancel: (() => void) | null
+  /** Invalidates every invocation already queued when a scope-level cancel establishes a boundary. */
+  toolQueueCancellationEpoch: number
   lastTabsStateFingerprint: string | null
   toolQueue: Promise<unknown>
   /** True while activation is the only operation that has touched this scope. */
@@ -132,13 +140,23 @@ interface DriverScopeState {
   toolExecutionEpoch: number
 }
 
+/** Captures the native queue boundary before an async authorization round trip. */
+export interface BrowserToolQueueBoundary {
+  scopeId: string
+  cancellationEpoch: number
+}
+
 function createDriverScopeState(): DriverScopeState {
   return {
     pendingNotices: [],
     takeoverActive: false,
     takeoverDone: false,
     takeoverResponse: null,
+    takeoverInvocationEpoch: null,
     toolInvocationEpoch: 0,
+    activeToolCallId: null,
+    activeToolCancel: null,
+    toolQueueCancellationEpoch: 0,
     lastTabsStateFingerprint: null,
     toolQueue: Promise.resolve(),
     activationOnly: true,
@@ -158,6 +176,22 @@ function invalidateSnapshot(state = driverScopeState()): void {
 
 const driverScopeStates = new Map<string, DriverScopeState>()
 const driverScopeAliases = new Map<string, string>()
+const CANCELLED_TOOL_TTL_MS = 5 * 60_000
+const MAX_CANCELLED_TOOL_TOMBSTONES = 256
+const cancelledToolCallIds = new Map<string, number>()
+
+function pruneCancelledToolCallIds(now = Date.now()): void {
+  for (const [toolCallId, expiresAt] of cancelledToolCallIds) {
+    if (expiresAt > now && cancelledToolCallIds.size <= MAX_CANCELLED_TOOL_TOMBSTONES) break
+    cancelledToolCallIds.delete(toolCallId)
+  }
+}
+
+function isToolCallCancelled(toolCallId: string | undefined): boolean {
+  if (!toolCallId) return false
+  pruneCancelledToolCallIds()
+  return cancelledToolCallIds.has(toolCallId)
+}
 
 function resolveDriverScopeId(scopeId: string): string {
   let resolved = scopeId
@@ -179,6 +213,19 @@ function driverScopeState(scopeId = session.getBrowserScopeId()): DriverScopeSta
   return state
 }
 
+export function captureBrowserToolQueueBoundary(scopeId: string): BrowserToolQueueBoundary {
+  const resolvedScopeId = resolveDriverScopeId(scopeId)
+  return {
+    scopeId: resolvedScopeId,
+    cancellationEpoch: driverScopeState(resolvedScopeId).toolQueueCancellationEpoch,
+  }
+}
+
+function isBrowserToolQueueBoundaryCurrent(boundary: BrowserToolQueueBoundary): boolean {
+  const state = driverScopeStates.get(resolveDriverScopeId(boundary.scopeId))
+  return state?.toolQueueCancellationEpoch === boundary.cancellationEpoch
+}
+
 function recordNotice(notice: string): void {
   const state = driverScopeState()
   state.activationOnly = false
@@ -313,6 +360,7 @@ export function initDriver(
   // first tab push as a duplicate.
   driverScopeStates.clear()
   driverScopeAliases.clear()
+  cancelledToolCallIds.clear()
   // The serialization chain, too. A takeover from the previous session can sit
   // unresolved indefinitely, and its `takeoverDone` flag is reset above — so
   // leaving the old chain head in place would queue the new session's first
@@ -1644,6 +1692,7 @@ async function runTakeover(purpose: string | undefined, invocationEpoch: number)
   state.takeoverActive = true
   state.takeoverDone = false
   state.takeoverResponse = null
+  state.takeoverInvocationEpoch = invocationEpoch
   session.setAutomationNeedsAttention(true)
 
   const startedAt = Date.now()
@@ -1673,10 +1722,16 @@ async function runTakeover(purpose: string | undefined, invocationEpoch: number)
       }
     }
   } finally {
-    session.setAutomationNeedsAttention(false)
-    state.takeoverActive = false
-    state.takeoverDone = false
-    state.takeoverResponse = null
+    // Cancellation releases the serialized tool queue before this polling
+    // loop observes its superseding epoch. Never let that delayed cleanup
+    // erase a newer takeover that has already claimed the same scope state.
+    if (state.takeoverInvocationEpoch === invocationEpoch) {
+      session.setAutomationNeedsAttention(false)
+      state.takeoverActive = false
+      state.takeoverDone = false
+      state.takeoverResponse = null
+      state.takeoverInvocationEpoch = null
+    }
   }
 }
 
@@ -2987,7 +3042,9 @@ function withNotices(result: unknown): unknown {
 export async function executeTool(
   scopeId: string,
   tool: BrowserToolName,
-  params: Record<string, unknown>
+  params: Record<string, unknown>,
+  toolCallId?: string,
+  authorizationBoundary?: BrowserToolQueueBoundary
 ): Promise<{ ok: boolean; result?: unknown; error?: string }> {
   const resolvedScopeId = resolveDriverScopeId(scopeId)
   if (session.isBrowserScopeSuspended(resolvedScopeId)) {
@@ -2999,7 +3056,21 @@ export async function executeTool(
   const state = driverScopeState(resolvedScopeId)
   state.activationOnly = false
   const invocationEpoch = ++state.toolInvocationEpoch
+  const queueCancellationEpoch = state.toolQueueCancellationEpoch
   const run = async () => {
+    if (
+      (authorizationBoundary && !isBrowserToolQueueBoundaryCurrent(authorizationBoundary)) ||
+      queueCancellationEpoch !== state.toolQueueCancellationEpoch ||
+      isToolCallCancelled(toolCallId)
+    ) {
+      throw new ToolError('This browser action was cancelled before it started.')
+    }
+    state.activeToolCallId = toolCallId ?? null
+    let cancelActiveExecution: () => void = () => {}
+    const cancellation = new Promise<never>((_resolve, reject) => {
+      cancelActiveExecution = () => reject(new ToolError('This browser action was cancelled.'))
+    })
+    state.activeToolCancel = cancelActiveExecution
     return await session.withBrowserScope(resolvedScopeId, async () => {
       logger.info('Executing browser tool', { tool, scopeId: resolvedScopeId })
       const keepHiddenPageActive = tool !== 'browser_request_takeover'
@@ -3022,20 +3093,24 @@ export async function executeTool(
           executionDeadline,
           invocationEpoch
         )
-        return withNotices(
-          await (watchdogMs === null
+        const guardedExecution =
+          watchdogMs === null
             ? execution
             : raceAgainstWatchdog(execution, watchdogMs, () => {
                 if (state.toolExecutionEpoch === executionEpoch) state.toolExecutionEpoch++
                 if (tool === 'browser_snapshot' || tool === 'browser_open_url') {
                   invalidateSnapshot(state)
                 }
-              }))
-        )
+              })
+        return withNotices(await Promise.race([guardedExecution, cancellation]))
       } finally {
         if (keepHiddenPageActive) {
           session.setAutomationActive(false)
         }
+        if (state.activeToolCancel === cancelActiveExecution) {
+          state.activeToolCallId = null
+          state.activeToolCancel = null
+        }
       }
     })
   }
@@ -3057,6 +3132,40 @@ export async function executeTool(
   }
 }
 
+/**
+ * Cancels one exact browser tool, including a cancellation that arrives while
+ * its authorization IPC is still in flight. The bounded tombstone lets that
+ * later invocation observe the stop without retaining call ids indefinitely.
+ */
+export function cancelTool(scopeId: string, toolCallId: string): boolean {
+  pruneCancelledToolCallIds()
+  cancelledToolCallIds.set(toolCallId, Date.now() + CANCELLED_TOOL_TTL_MS)
+  pruneCancelledToolCallIds()
+
+  const resolvedScopeId = resolveDriverScopeId(scopeId)
+  const state = driverScopeStates.get(resolvedScopeId)
+  if (!state || state.activeToolCallId !== toolCallId) return true
+
+  state.toolInvocationEpoch++
+  state.toolExecutionEpoch++
+  state.activeToolCancel?.()
+  void session.withBrowserScope(resolvedScopeId, () => {
+    session.setAutomationActive(false)
+    session.setAutomationNeedsAttention(false)
+  })
+  return true
+}
+
+/** Cancels the active tool and every older invocation already queued for this scope. */
+export function cancelActiveTool(scopeId: string): boolean {
+  const resolvedScopeId = resolveDriverScopeId(scopeId)
+  const state = driverScopeStates.get(resolvedScopeId)
+  if (!state) return false
+  state.toolQueueCancellationEpoch++
+  const toolCallId = state.activeToolCallId
+  return toolCallId ? cancelTool(resolvedScopeId, toolCallId) : false
+}
+
 /** Browser-chrome commands from the panel header; fire-and-forget. */
 export async function handlePanelAction(
   scopeId: string,