fix(desktop): stop every terminal operation, answer only from granted folders, and bound the outbox in bytes

- Stop reaches `input`, `kill` and pane `close`, not just `run` and `handoff`.
  A Stop that lands while the session resolves means none of them starts.
  A batch of keys or lines stops between keystrokes and says some input may
  already have arrived.
- A terminal call stopped while it waited for the chat's previous operation
  never reaches the terminal.
- Sign-out stops running actions before it waits on a reconcile in flight.
- The outbox budgets result data in UTF-8 bytes, so a journal of non-ASCII
  results still reads back after a restart.
- A local read, grep, glob, list or stat answers only while its folder is
  still granted when it finishes. This covers the chat view's path as well.
- Grep says when its results are truncated, as glob does.
- A doorbell stream that keeps closing as soon as it opens backs off instead
  of reconnecting every second.
- The not-started results follow #8666's wording: what happened, that nothing
  ran, and what the model should do instead of retrying.

Author: waleedlatif1

apps/desktop/src/main/desktop-executor/approval-notifier.ts

@@ -25,6 +25,11 @@ export interface ApprovalNotifierDeps {
   }) => ApprovalNotification | null
 }
 
+/**
+ * Creates the notifier the executor feeds each inbox read's approval items to. `update` notifies
+ * once per newly waiting call and closes notifications for calls decided since; `clear` closes
+ * them all, for sign-out.
+ */
 export function createApprovalNotifier(deps: ApprovalNotifierDeps) {
   /** Calls already brought to the user's attention, by notification or by being on screen. */
   const shown = new Map<string, ApprovalNotification | null>()

apps/desktop/src/main/desktop-executor/client.ts

@@ -67,6 +67,11 @@ async function errorMessage(response: Response): Promise<string> {
   return typeof body?.error === 'string' ? body.error : `HTTP ${response.status}`
 }
 
+/**
+ * Creates the client for one install id. Each request times out on its own and fails with a
+ * {@link DeviceRequestError}, so the executor decides what to retry; responses are parsed before
+ * they are returned, and a malformed one is a 502.
+ */
 export function createDesktopExecutorClient(
   options: DesktopExecutorClientOptions
 ): DesktopExecutorClient {

apps/desktop/src/main/desktop-executor/doorbell.test.ts

@@ -1,3 +1,4 @@
+import { sleep } from '@sim/utils/helpers'
 import { describe, expect, it, vi } from 'vitest'
 import { DeviceRequestError } from '@/main/desktop-executor/client'
 import { InboxDoorbell, parseServerSentEvents } from '@/main/desktop-executor/doorbell'
@@ -90,6 +91,28 @@ describe('InboxDoorbell', () => {
     doorbell.stop()
   })
 
+  it('backs off when every stream ends as soon as it opens', async () => {
+    const opened: number[] = []
+    const doorbell = new InboxDoorbell({
+      client: {
+        openInboxStream: async () => {
+          opened.push(Date.now())
+          return new ReadableStream<Uint8Array>({ start: (controller) => controller.close() })
+        },
+      },
+      onRing: vi.fn(),
+      onUnregistered: vi.fn(),
+      retryBaseMs: 5,
+    })
+    doorbell.start()
+    await sleep(400)
+    doorbell.stop()
+
+    // A fixed 5 ms retry would open dozens; doubling from 5 ms opens a handful.
+    expect(opened.length).toBeGreaterThan(2)
+    expect(opened.length).toBeLessThan(15)
+  })
+
   it('replaces a connection that goes silent past the heartbeat', async () => {
     const { doorbell, connections } = harness({ staleAfterMs: 30 })
     doorbell.start()

apps/desktop/src/main/desktop-executor/doorbell.ts

@@ -16,6 +16,8 @@ const logger = createLogger('DesktopExecutorDoorbell')
 /** Sim heartbeats every 30 s; two missed ones mean the connection is gone. */
 const STALE_STREAM_MS = 75_000
 const RECONNECT_MAX_MS = 30_000
+/** A stream that stayed open this long was healthy; its clean end starts backoff afresh. */
+const HEALTHY_STREAM_MS = 60_000
 const HANDSHAKE_TIMEOUT_MS = 15_000
 
 interface DoorbellOptions {
@@ -96,10 +98,16 @@ export class InboxDoorbell {
     let attempt = 0
     const current = () => this.loopGeneration === generation
     while (current()) {
+      const openedAt = Date.now()
       const rotated = await this.connectOnce().then(
         (result) => {
-          attempt = 0
-          return result === 'rotated'
+          if (result === 'rotated') {
+            attempt = 0
+            return true
+          }
+          // A stream something keeps closing right away is a failing connection, not a healthy one.
+          attempt = Date.now() - openedAt >= HEALTHY_STREAM_MS ? 0 : attempt + 1
+          return false
         },
         (error: unknown) => {
           if (this.reconnectNow) {

apps/desktop/src/main/desktop-executor/executor.test.ts

@@ -455,6 +455,31 @@ describe('registration', () => {
     expect(executor.heldCallCount()).toBe(0)
   })
 
+  it('stops running actions at sign-out without waiting on a claim still in flight', async () => {
+    const { sim, journal, runner, executor } = setup()
+    sim.inbox = [callItem('call-1', 'chat-a')]
+    await executor.reconcile()
+    await vi.waitFor(() => expect(runner.started).toEqual(['call-1']))
+    const answer = deferred<void>()
+    const claim = sim.client.claim
+    sim.client.claim = async (toolCallId) => {
+      await answer.promise
+      return claim(toolCallId)
+    }
+    sim.inbox = [callItem('call-2', 'chat-b')]
+    const reading = executor.reconcile()
+    await vi.waitFor(() => expect(journal.entries.get('call-2')?.state).toBe('claiming'))
+
+    const signingOut = executor.dispose()
+    await vi.waitFor(() => expect(runner.cancelled).toEqual(['call-1']))
+    answer.resolve()
+    await Promise.all([reading, signingOut])
+
+    expect(runner.started).toEqual(['call-1'])
+    expect(executor.heldCallCount()).toBe(0)
+    expect(journal.entries.size).toBe(0)
+  })
+
   it('does not keep a claim that Sim answers after sign-out', async () => {
     const { sim, journal, runner, executor } = setup({ leaseRenewMs: 10 })
     const answer = deferred<void>()

apps/desktop/src/main/desktop-executor/executor.ts

@@ -26,13 +26,13 @@ const DEFAULT_MAX_HELD_CALLS = 32
 const DELIVERY_RETRY_MAX_MS = 30_000
 
 const NOT_STARTED_AFTER_RESTART =
-  'Not run: the Sim desktop app restarted before this action started, so nothing happened on the user’s computer.'
+  'Not run: this action never started, because the Sim desktop app restarted before it began. Nothing happened on the user’s computer. Do not retry it in this turn; tell the user, who can ask again.'
 const OUTCOME_UNKNOWN_AFTER_RESTART =
   'The Sim desktop app restarted while this action was running, so its result was lost. It may already have taken effect: inspect the current state before repeating it, and do not retry it automatically.'
 const STOPPED_BEFORE_START = 'Stopped before the Sim desktop app started this action.'
 const STOPPED_WHILE_RUNNING = 'Stopped while the Sim desktop app was running this action.'
 const NOT_RECORDED =
-  'Not run: the Sim desktop app could not record this action on the user’s computer before starting it, so it did not start it.'
+  'Not run: this action never started, because the Sim desktop app could not record it on the user’s computer first. Nothing happened on the user’s computer. Do not retry it in this turn; tell the user, who can ask again later.'
 const RESULT_TOO_LARGE =
   'The action finished, but its result was too large to send back. Do not repeat a side-effecting action; inspect the current state instead.'
 
@@ -161,8 +161,16 @@ export class DesktopExecutor {
   /** Sign-out: stops every action and forgets every call; the session that owned them is gone. */
   async dispose(): Promise<void> {
     this.disposed = true
-    // An inbox read in flight may still be claiming; let it see `disposed` before clearing.
+    const stopping = this.dropHeld()
+    // A claim still in flight sees `disposed` once Sim answers and is never held; the journal is
+    // cleared only after it, so its `claiming` record does not outlive the session.
     await this.reconciling?.catch(() => {})
+    await Promise.all([stopping, this.dropHeld()])
+    await this.options.journal.clear()
+  }
+
+  /** Releases every held call and stops the actions already running. */
+  private async dropHeld(): Promise<void> {
     const held = [...this.held.values()]
     for (const entry of held) this.release(entry)
     await Promise.allSettled(
@@ -173,7 +181,6 @@ export class DesktopExecutor {
           return this.options.runner.cancel(entry.call)
         })
     )
-    await this.options.journal.clear()
   }
 
   private async reconcileOnce(): Promise<void> {
@@ -234,6 +241,7 @@ export class DesktopExecutor {
       this.noteRequestFailure('Desktop call was not claimed', error, { toolCallId })
       return
     }
+    if (this.disposed) return
     // Best effort: an unrecorded token leaves `claiming`, which recovery treats conservatively.
     await this.record({ toolCallId, state: 'claimed', executionToken: call.executionToken })
     const entry: HeldCall = {

apps/desktop/src/main/desktop-executor/journal.test.ts

@@ -102,6 +102,29 @@ describe('executor journal', () => {
     expect(JSON.stringify(restored)).toContain('resultOmitted')
   })
 
+  it('budgets results by their encoded size, not their length in characters', async () => {
+    const filePath = await journalPath()
+    const encryption = testEncryption()
+    const journal = createExecutorJournal(filePath, encryption)
+    // Three bytes per character: within the budget by length, three times over it in bytes.
+    const pageText = '€'.repeat(6 * 1024 * 1024)
+    for (const toolCallId of ['call-1', 'call-2', 'call-3']) {
+      await journal.put({
+        toolCallId,
+        state: 'result',
+        executionToken: `token-${toolCallId}`,
+        completion: { status: 'success', message: 'done', data: { pageText } },
+      })
+    }
+
+    const restored = await createExecutorJournal(filePath, encryption).load()
+    expect(restored).toHaveLength(3)
+    const kept = restored.filter(
+      (entry) => entry.state === 'result' && entry.completion.data?.pageText === pageText
+    )
+    expect(kept).toHaveLength(1)
+  })
+
   it('starts empty from a corrupt or foreign file instead of failing', async () => {
     const filePath = await journalPath()
     await writeFile(filePath, '{"version":1,"ciphertext":"not-base64-json"}')

apps/desktop/src/main/desktop-executor/journal.ts

@@ -29,10 +29,10 @@ const JOURNAL_VERSION = 1
 /** Results can carry a screenshot, so the bound is the size of a few of them. */
 const MAX_JOURNAL_BYTES = 64 * 1024 * 1024
 /**
- * Result data kept on disk across all entries. Encryption and base64 grow it by about a third,
- * so a journal within this budget always reads back under {@link MAX_JOURNAL_BYTES}.
+ * Result data kept on disk across all entries, in UTF-8 bytes. Encryption and base64 grow it by
+ * about a third, so a journal within this budget always reads back under {@link MAX_JOURNAL_BYTES}.
  */
-const MAX_PERSISTED_RESULT_CHARS = 32 * 1024 * 1024
+const MAX_PERSISTED_RESULT_BYTES = 32 * 1024 * 1024
 const RESULT_NOT_KEPT =
   'The action finished, but its result was too large to keep on this computer. Do not repeat a side-effecting action; inspect the current state instead.'
 
@@ -41,10 +41,10 @@ const RESULT_NOT_KEPT =
  * kept as finished without its data, so the file never grows past what a restart can read.
  */
 function boundedEntries(entries: Map<string, JournalEntry>): JournalEntry[] {
-  let budget = MAX_PERSISTED_RESULT_CHARS
+  let budget = MAX_PERSISTED_RESULT_BYTES
   return [...entries.values()].map((entry) => {
     if (entry.state !== 'result' || entry.completion.data === undefined) return entry
-    const size = JSON.stringify(entry.completion.data).length
+    const size = Buffer.byteLength(JSON.stringify(entry.completion.data), 'utf8')
     if (size <= budget) {
       budget -= size
       return entry

apps/desktop/src/main/desktop-executor/runner.test.ts

@@ -63,4 +63,32 @@ describe('background terminal calls', () => {
     expect((await second).status).toBe('success')
     expect(started).toEqual(['wedged', 'next'])
   })
+
+  it('never starts a terminal operation stopped while it waited for the chat terminal', async () => {
+    vi.useFakeTimers()
+    let releaseWedged: (response: TerminalToolResponse) => void = () => {}
+    const started: string[] = []
+    const runner = runnerWithTerminal((toolCallId) => {
+      started.push(toolCallId)
+      if (toolCallId === 'wedged')
+        return new Promise((resolve) => {
+          releaseWedged = resolve
+        })
+      return Promise.resolve({ ok: true, result: { output: '' } })
+    })
+    const first = runner.run(terminalCall('wedged', 'input'), new AbortController().signal)
+    await vi.advanceTimersByTimeAsync(15_000)
+    await first
+
+    const stop = new AbortController()
+    const second = runner.run(terminalCall('stopped', 'run'), stop.signal)
+    await vi.advanceTimersByTimeAsync(0)
+    stop.abort()
+    const completion = await second
+    releaseWedged({ ok: true, result: {} })
+    await vi.advanceTimersByTimeAsync(0)
+
+    expect(completion.status).toBe('error')
+    expect(started).toEqual(['wedged'])
+  })
 })

apps/desktop/src/main/desktop-executor/runner.ts

@@ -46,12 +46,12 @@ const USER_LOCAL_TOOLS: ReadonlySet<string> = new Set(['read', 'grep', 'glob'])
 
 /** The model learns a call never ran because a surface is switched off on this machine. */
 function surfaceOff(surface: string): DesktopToolCompletion {
-  const message = `Not run: ${surface} is switched off in the Sim desktop app's settings, so nothing happened on the user's computer. Continue without it, or ask the user to switch it on.`
+  const message = `Not run: this action never started, because ${surface} is switched off in the Sim desktop app’s settings. Nothing happened on the user’s computer. Do not retry it in this turn; continue without it, or ask the user to switch it on.`
   return { status: 'error', message, data: { error: message, notStarted: true } }
 }
 
 function unsupported(toolName: string): DesktopToolCompletion {
-  const message = `Not run: this version of the Sim desktop app cannot run ${toolName} in the background, so nothing happened on the user's computer.`
+  const message = `Not run: this action never started, because this version of the Sim desktop app cannot run ${toolName} in the background. Nothing happened on the user’s computer. Do not retry it in this turn; tell the user to update the Sim desktop app.`
   return { status: 'error', message, data: { error: message, notStarted: true } }
 }
 
@@ -88,6 +88,12 @@ export interface DesktopToolRunnerDeps {
   }
 }
 
+/** Resolves once the signal aborts, which may be never. */
+function untilAborted(signal: AbortSignal): Promise<void> {
+  if (signal.aborted) return Promise.resolve()
+  return new Promise((resolve) => signal.addEventListener('abort', () => resolve(), { once: true }))
+}
+
 /** Resolves with the work's result, or with `onTimeout`'s once `timeoutMs` passes first. */
 async function withDeadline<T>(
   work: Promise<T>,
@@ -146,7 +152,10 @@ export function createDesktopToolRunner(deps: DesktopToolRunnerDeps): DesktopToo
     )
   }
 
-  async function runTerminal(call: ClaimedDesktopCall): Promise<DesktopToolCompletion> {
+  async function runTerminal(
+    call: ClaimedDesktopCall,
+    signal: AbortSignal
+  ): Promise<DesktopToolCompletion> {
     if (!deps.preferences().terminalEnabled) return surfaceOff('the terminal')
     const { operation } = call.args
     if (!isTerminalOperation(operation)) {
@@ -159,7 +168,12 @@ export function createDesktopToolRunner(deps: DesktopToolRunnerDeps): DesktopToo
     const timeoutMs = terminalOperationTimeoutMs(operation)
     // An operation reported as unresponsive may still land; the chat's next one waits for it, so
     // two never act on the same terminals at once.
-    await unsettledTerminalWork.get(call.chatId)
+    const previous = unsettledTerminalWork.get(call.chatId)
+    if (previous) await Promise.race([previous, untilAborted(signal)])
+    // Stopped while it waited: the terminal never heard of it, so its own cancel cannot reach it.
+    if (signal.aborted) {
+      return terminalToolFailure('The terminal action was stopped before it started.', 'CANCELLED')
+    }
     const operationDone = deps.terminal.executeTool(call.chatId, call.toolCallId, operation, args)
     const settled = operationDone.then(
       () => undefined,
@@ -195,7 +209,7 @@ export function createDesktopToolRunner(deps: DesktopToolRunnerDeps): DesktopToo
     async run(call, signal) {
       try {
         if (isCurrentBrowserToolName(call.toolName)) return await runBrowser(call, call.toolName)
-        if (call.toolName === 'terminal') return await runTerminal(call)
+        if (call.toolName === 'terminal') return await runTerminal(call, signal)
         if (!deps.accountDataAvailable()) return surfaceOff('local file access')
         if (call.toolName === 'read_local_file') {
           return localFileReadCompletion(await deps.localFiles.read(call))