fix(desktop): a tmux run starts only once its pane is tagged, and sign-out reaches runs of closed tabs

- A run's command now waits for its pane to be tagged as the run's before
  it starts. Untagged, it never starts and its pane closes by itself, so no
  pane is ever closed by an id a restarted tmux server might have handed to
  the user. A short command can no longer finish before the tag exists.
- The run script is a file instead of a bash -c string. tmux hands its
  command to sh -c, which expanded the counter and PIPESTATUS meant for
  bash first, so tmux runs reported no exit code.
- A run whose Sim terminal closed while it kept going in tmux is still
  stopped at sign-out and when Terminal is switched off.
- Retiring an install id either replaces it or removes it from disk, so a
  retired id is never picked up again. A 409 whose new id cannot be saved
  backs off instead of retrying at once.
- A failing approval notification no longer keeps the inbox read from
  claiming its calls.

Author: waleedlatif1

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

@@ -248,9 +248,14 @@ export class DesktopExecutor {
     for (const item of items) {
       if (item.kind === 'cancel') void this.stop(item.toolCallId, 'Stopped by the user.')
     }
-    this.options.onApprovals?.(
-      items.filter((item): item is DesktopApprovalItem => item.kind === 'approval_needed')
-    )
+    try {
+      this.options.onApprovals?.(
+        items.filter((item): item is DesktopApprovalItem => item.kind === 'approval_needed')
+      )
+    } catch (error) {
+      // Notifications are a courtesy; the calls in this read still get claimed.
+      logger.warn('Could not notify about desktop approvals', { error: getErrorMessage(error) })
+    }
     for (const item of items) {
       if (item.kind !== 'call' || this.held.has(item.toolCallId)) continue
       if (this.paused || this.disposed) return

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

@@ -16,7 +16,11 @@ import { backoffWithJitter } from '@sim/utils/retry'
 import { truncate } from '@sim/utils/string'
 import type { Session } from 'electron'
 import { app, net, powerMonitor } from 'electron'
-import { readFileWithinLimit, writeJsonFileAtomically } from '@/main/atomic-json-file'
+import {
+  readFileWithinLimit,
+  removeFileIfPresent,
+  writeJsonFileAtomically,
+} from '@/main/atomic-json-file'
 import {
   createDesktopExecutorClient,
   type DesktopExecutorClient,
@@ -136,11 +140,27 @@ export function createDesktopExecutorService(
     return next
   }
 
-  /** Rotates where a failure must not stop the caller; the next registration tries again. */
-  async function retireInstallId(): Promise<void> {
-    await rotateInstallId().catch((error) =>
+  /**
+   * Retires the current install id for good: a new one replaces it, or, if that cannot be saved,
+   * the old one is removed so no later launch or sign-in can pick it up again. False when neither
+   * worked and the old id is still on disk.
+   */
+  async function retireInstallId(): Promise<boolean> {
+    try {
+      await rotateInstallId()
+      return true
+    } catch (error) {
       logger.warn('Could not save a new desktop install id', { error: getErrorMessage(error) })
-    )
+    }
+    try {
+      await removeFileIfPresent(identityPath)
+      return true
+    } catch (error) {
+      logger.warn('Could not remove the retired desktop install id', {
+        error: getErrorMessage(error),
+      })
+      return false
+    }
   }
 
   function fetchWithAppSession(url: string, init: RequestInit): Promise<Response> {
@@ -288,8 +308,18 @@ export function createDesktopExecutorService(
         // The id belongs to another account (a copied profile); this install takes a new one.
         logger.warn('Desktop install id is registered to another account; minting a new one')
         await resetExecutor()
-        await retireInstallId()
-        scheduleRegistration(0)
+        if (await retireInstallId()) {
+          scheduleRegistration(0)
+        } else {
+          // The conflicting id is still on disk: retrying at once would only conflict again.
+          registrationAttempt += 1
+          scheduleRegistration(
+            backoffWithJitter(registrationAttempt, null, {
+              baseMs: 2_000,
+              maxMs: REGISTRATION_RETRY_MAX_MS,
+            })
+          )
+        }
         return
       }
       if (error instanceof DeviceRequestError && error.unregistered) {

apps/desktop/src/main/terminal/index.ts

@@ -248,6 +248,11 @@ export class TerminalService {
    * Held here so the terminal's own lifecycle can reclaim them.
    */
   private readonly pendingRuns = new Map<string, TmuxRunHandle[]>()
+  /**
+   * Agent runs still going in tmux after their Sim terminal closed: the tmux session outlives the
+   * tab, but sign-out must still stop them.
+   */
+  private readonly orphanedRuns = new Map<TmuxRunHandle, NodeJS.ProcessEnv>()
   /** Runs a `run` call is still waiting on; their files are read when the wait ends. */
   private readonly awaitedRuns = new Set<TmuxRunHandle>()
   /** Awaited runs released meanwhile (their terminal closed); their files go once the wait ends. */
@@ -508,10 +513,13 @@ export class TerminalService {
    * Releases every tracked run for a terminal, finished or not. The terminal is
    * going away, so nothing will ever read these files again.
    */
-  private releasePendingRuns(terminalId: string): void {
+  private releasePendingRuns(terminalId: string, env?: NodeJS.ProcessEnv): void {
     const pending = this.pendingRuns.get(terminalId)
     if (!pending) return
-    for (const handle of pending) this.releaseRun(handle)
+    for (const handle of pending) {
+      if (env && !isRunComplete(handle)) this.orphanedRuns.set(handle, env)
+      this.releaseRun(handle)
+    }
     this.pendingRuns.delete(terminalId)
   }
 
@@ -525,10 +533,11 @@ export class TerminalService {
     const closedCwd = session.currentCwd
     const order = [...this.sessions.keys()]
     const index = order.indexOf(terminalId)
+    const env = session.env
     session.dispose()
     this.sessions.delete(terminalId)
     this.tmuxCache.delete(terminalId)
-    this.releasePendingRuns(terminalId)
+    this.releasePendingRuns(terminalId, env)
 
     this.rememberClosed(closedCwd)
     // Nothing is left for the user to hold on to; the next shell the agent
@@ -890,6 +899,10 @@ export class TerminalService {
         if (!isRunComplete(handle)) stops.push(stopRun(handle, session.env, STOP_ESCALATION_MS))
       }
     }
+    for (const [handle, env] of this.orphanedRuns) {
+      stops.push(stopRun(handle, env, STOP_ESCALATION_MS))
+    }
+    this.orphanedRuns.clear()
     await Promise.allSettled(stops)
   }
 

apps/desktop/src/main/terminal/tmux.test.ts

@@ -1,6 +1,7 @@
-import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
+import { sleep } from '@sim/utils/helpers'
 import { afterEach, describe, expect, it } from 'vitest'
 import {
   awaitRun,
@@ -120,6 +121,12 @@ switch (args[0]) {
     const pane = '%' + state.nextPane++
     state.panes[pane] = { window, options: {} }
     save()
+    // Runs the pane's command for real, the way tmux would, when a test asks for it.
+    if (process.env.FAKE_TMUX_EXEC) {
+      require('node:child_process')
+        .spawn('sh', ['-c', args[args.length - 1]], { detached: true, stdio: 'ignore' })
+        .unref()
+    }
     process.stdout.write(window + ' ' + pane + '\\n')
     break
   }
@@ -150,7 +157,7 @@ switch (args[0]) {
 }
 `
 
-function fakeTmux() {
+function fakeTmux(options: { exec?: boolean } = {}) {
   const dir = mkdtempSync(join(tmpdir(), 'fake-tmux-'))
   const stateFile = join(dir, 'state.json')
   const binary = join(dir, 'tmux')
@@ -161,7 +168,12 @@ function fakeTmux() {
   const read = (): FakeTmuxState => JSON.parse(readFileSync(stateFile, 'utf8'))
   return {
     dir,
-    env: { ...process.env, PATH: `${dir}:${process.env.PATH ?? ''}`, FAKE_TMUX_STATE: stateFile },
+    env: {
+      ...process.env,
+      PATH: `${dir}:${process.env.PATH ?? ''}`,
+      FAKE_TMUX_STATE: stateFile,
+      ...(options.exec ? { FAKE_TMUX_EXEC: '1' } : {}),
+    },
     read,
     write,
     /** The user splits a run's window: a new pane of theirs beside the run's. */
@@ -232,16 +244,24 @@ describe('stopping a tmux run touches only its own pane', () => {
     expect(tmux.read().log).toEqual([])
   })
 
-  it('closes a run it could not tag instead of leaving an unstoppable command', async () => {
+  it('never lets a run it could not tag start, and closes no pane by id to stop it', async () => {
     const tmux = fakeTmux()
     dirs.push(tmux.dir)
     tmux.write({ ...tmux.read(), fail: { 'set-option': 'invalid option: @sim-run-id' } })
 
-    const started = await startRun('agent', 'sleep 600', null, tmux.env)
+    const result = await startRun('agent', 'sleep 600', null, tmux.env)
+
+    expect(result).toMatchObject({ error: expect.stringContaining('was not run') })
+    // No pane is closed by an id that a restarted server might have handed to the user.
+    expect(tmux.read().log).toEqual([])
+  })
+
+  it('lets a tagged run start only once its pane is tagged', async () => {
+    const tmux = fakeTmux()
+    const run = await started(tmux)
 
-    expect('error' in started).toBe(true)
-    expect(tmux.read().log).toEqual(['kill-pane %0'])
-    expect(tmux.read().panes).toEqual({})
+    expect(tmux.read().panes[run.pane]?.options['@sim-run-id']).toBe(run.runId)
+    expect(existsSync(join(run.statusPath, '..', 'go'))).toBe(true)
   })
 
   it('neither stops nor gives up on a run while tmux cannot be asked', async () => {
@@ -254,4 +274,26 @@ describe('stopping a tmux run touches only its own pane', () => {
 
     expect(tmux.read().log).toEqual([])
   })
+
+  it('runs a tagged command for real, and never runs one it could not tag', async () => {
+    const tagged = fakeTmux({ exec: true })
+    dirs.push(tagged.dir)
+    const run = await startRun('agent', 'echo ran', null, tagged.env)
+    if ('error' in run) throw new Error(run.error)
+    await expect
+      .poll(() => pollRun(run), { timeout: 10_000 })
+      .toMatchObject({
+        done: true,
+        exitCode: 0,
+      })
+
+    const untagged = fakeTmux({ exec: true })
+    dirs.push(untagged.dir)
+    untagged.write({ ...untagged.read(), fail: { 'set-option': 'invalid option' } })
+    const marker = join(untagged.dir, 'ran')
+    await startRun('agent', `touch ${JSON.stringify(marker)}`, null, untagged.env)
+    await sleep(6_000)
+
+    expect(existsSync(marker)).toBe(false)
+  }, 20_000)
 })

apps/desktop/src/main/terminal/tmux.ts

@@ -17,11 +17,12 @@
  * of the shell that launched it.
  */
 import { spawn } from 'node:child_process'
-import { mkdtempSync, readFileSync, rmSync } from 'node:fs'
+import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { createLogger } from '@sim/logger'
 import type { TerminalPaneState } from '@sim/terminal-protocol'
+import { getErrorMessage } from '@sim/utils/errors'
 import { sleep } from '@sim/utils/helpers'
 import { generateId } from '@sim/utils/id'
 
@@ -337,6 +338,7 @@ export async function startRun(
   const dir = mkdtempSync(join(tmpdir(), 'sim-tmux-run-'))
   const outPath = join(dir, 'out')
   const statusPath = join(dir, 'status')
+  const goPath = join(dir, 'go')
   const dispose = () => {
     try {
       rmSync(dir, { recursive: true, force: true })
@@ -354,8 +356,27 @@ export async function startRun(
   // writing to the unlinked inode — but an unredirected `printf` would fail
   // into the pipeline and print `No such file or directory` into the user's own
   // tmux window, minutes after they closed the tab.
-  const script = `${command}\nprintf %s "\${PIPESTATUS[0]}" > ${JSON.stringify(statusPath)} 2>/dev/null`
-  const wrapper = `bash -lc ${JSON.stringify(`{ ${script}; } 2>&1 | tee ${JSON.stringify(outPath)}`)}`
+  //
+  // The command waits for its pane to be tagged as this run's (the go file), so nothing runs that
+  // a later stop could not recognize. Untagged, the script gives up after five seconds and its
+  // pane closes on its own; no one has to close a pane whose id might no longer be its own.
+  //
+  // The script is a file rather than a `bash -c` string: tmux hands its command to `sh -c`, which
+  // would expand `$` references meant for bash (the gate's counter, PIPESTATUS) before bash ran.
+  const scriptPath = join(dir, 'run.sh')
+  writeFileSync(
+    scriptPath,
+    [
+      'i=0',
+      `while [ ! -e ${JSON.stringify(goPath)} ] && [ "$i" -lt 100 ]; do sleep 0.05; i=$((i + 1)); done`,
+      `[ -e ${JSON.stringify(goPath)} ] || exit 0`,
+      `{ ${command}`,
+      `printf %s "\${PIPESTATUS[0]}" > ${JSON.stringify(statusPath)} 2>/dev/null; } 2>&1 | tee ${JSON.stringify(outPath)}`,
+      '',
+    ].join('\n'),
+    { mode: 0o600 }
+  )
+  const wrapper = `bash -l ${JSON.stringify(scriptPath)}`
 
   const args = [
     'new-window',
@@ -382,14 +403,19 @@ export async function startRun(
   // a pane the user opened later under the same id, so it sends nothing at all.
   const tagged = await runTmux(['set-option', '-p', '-t', pane, RUN_ID_OPTION, runId], env)
   if (!tagged.ok) {
-    // Untagged, nothing could ever stop it safely later, so it does not get to run. The pane was
-    // created a moment ago by this call, so its id is still ours to close.
-    await runTmux(['kill-pane', '-t', pane], env)
+    // Untagged, nothing could stop it safely later, so it never starts: without the go file the
+    // wrapper exits by itself.
     dispose()
     return {
-      error: `tmux could not mark the command's pane, so it was closed straight away (${tagged.stderr.trim() || 'no detail'}). It may have started; check before running it again.`,
+      error: `tmux could not mark the command's pane (${tagged.stderr.trim() || 'no detail'}), so the command was not run.`,
     }
   }
+  try {
+    writeFileSync(goPath, '')
+  } catch (error) {
+    dispose()
+    return { error: `The command could not be started: ${getErrorMessage(error)}` }
+  }
 
   return { window, pane, runId, outPath, statusPath, dispose }
 }