fix(desktop): run an agent command on tmux that cannot tag its pane, untracked

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

@@ -494,7 +494,10 @@ export class TerminalService {
     }
     for (const handle of this.pendingRuns.get(terminalId) ?? []) {
       if (this.awaitedRuns.has(handle)) continue
-      if (isRunComplete(handle) || (await runPaneState(handle, env)) === 'gone') {
+      const complete = isRunComplete(handle)
+      if (complete || (await runPaneState(handle, env)) === 'gone') {
+        // A pane kept open after its command ended (`remain-on-exit`) closes with its run.
+        if (complete) await closeRunPane(handle, env)
         this.untrackRun(terminalId, handle)
         handle.dispose()
       }
@@ -521,7 +524,8 @@ export class TerminalService {
     const pending = this.pendingRuns.get(terminalId)
     if (!pending) return
     for (const handle of pending) {
-      if (env && !isRunComplete(handle)) this.orphanedRuns.set(handle, env)
+      // An untracked run is never stopped, so there is nothing to keep it for.
+      if (env && handle.runId !== null && !isRunComplete(handle)) this.orphanedRuns.set(handle, env)
       this.releaseRun(handle)
     }
     this.pendingRuns.delete(terminalId)
@@ -1300,7 +1304,11 @@ export class TerminalService {
     if (latch.signal.aborted) void latch.stopRunning()
     const outcome = await Promise.race([
       awaitRun(handle, waitMs),
-      stopped.then(() => ({ ...pollRun(handle), done: true })),
+      // A stopped run's closed pane never writes its status. An untracked run is never stopped,
+      // so it is still going unless its status says otherwise.
+      stopped.then(() =>
+        handle.runId === null ? pollRun(handle) : { ...pollRun(handle), done: true }
+      ),
     ]).finally(() => {
       this.awaitedRuns.delete(handle)
       if (this.releasedAwaitedRuns.delete(handle)) handle.dispose()

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

@@ -16,6 +16,10 @@ const tmuxFake = vi.hoisted(() => ({
   stopped: [] as string[],
   /** Panes no longer the run's (closed by the user, or reused after a tmux restart). */
   gone: new Set<string>(),
+  /** Runs start untracked, as on a tmux too old to tag their panes. */
+  untracked: false,
+  /** Run panes tmux still shows, kept open after their command ends (`remain-on-exit`). */
+  open: new Set<string>(),
   statusPaths: new Map<string, string>(),
 }))
 
@@ -37,28 +41,31 @@ vi.mock('@/main/terminal/tmux', async () => {
       const statusPath = join(dir, 'status')
       writeFileSync(join(dir, 'out'), 'partial output')
       tmuxFake.statusPaths.set(pane, statusPath)
+      tmuxFake.open.add(pane)
       return {
         window: `@${pane.slice(1)}`,
         pane,
-        runId: `run-${pane}`,
+        runId: tmuxFake.untracked ? null : `run-${pane}`,
         outPath: join(dir, 'out'),
         statusPath,
         dispose: () => rmSync(dir, { recursive: true, force: true }),
       }
     },
     runPaneState: async (...args: Parameters<typeof actual.runPaneState>) => {
       if (!tmuxFake.on) return actual.runPaneState(...args)
-      return tmuxFake.gone.has(args[0].pane) ? 'gone' : 'ours'
+      if (tmuxFake.gone.has(args[0].pane)) return 'gone'
+      return args[0].runId === null ? 'unknown' : 'ours'
     },
     stopRun: async (...args: Parameters<typeof actual.stopRun>) => {
       if (!tmuxFake.on) return actual.stopRun(...args)
       const [handle] = args
-      if (tmuxFake.gone.has(handle.pane)) return
+      if (tmuxFake.gone.has(handle.pane) || handle.runId === null) return
       tmuxFake.stopped.push(handle.pane)
       writeFileSync(handle.statusPath, '130')
     },
     closeRunPane: async (...args: Parameters<typeof actual.closeRunPane>) => {
       if (!tmuxFake.on) return actual.closeRunPane(...args)
+      tmuxFake.open.delete(args[0].pane)
     },
   }
 })
@@ -587,6 +594,56 @@ describe('agent commands in tmux', () => {
     }
   })
 
+  it('reports an untracked run it could not stop as still running, and keeps tracking it', async () => {
+    tmuxFake.on = true
+    tmuxFake.untracked = true
+    tmuxFake.statusPaths.clear()
+    try {
+      const terminal = new TerminalService({ loadCwd: () => '/tmp' })
+      terminal.start({ cols: 80, rows: 24 })
+      const running = terminal.executeTool('call-untracked', 'run', {
+        command: 'sleep 600',
+        waitSeconds: 60,
+      })
+      await vi.waitFor(() => expect(tmuxFake.statusPaths.size).toBe(1))
+      const [, statusPath = ''] = [...tmuxFake.statusPaths][0] ?? []
+
+      await terminal.cancelTool('call-untracked')
+
+      await expect(running).resolves.toMatchObject({ ok: true, result: { status: 'running' } })
+      expect(existsSync(join(statusPath, '..'))).toBe(true)
+
+      // Once it does finish, the next run's bookkeeping reaps it.
+      writeFileSync(statusPath, '0')
+      await terminal.executeTool('call-next', 'run', { command: 'ls', waitSeconds: 1 })
+      expect(existsSync(join(statusPath, '..'))).toBe(false)
+    } finally {
+      tmuxFake.on = false
+      tmuxFake.untracked = false
+    }
+  })
+
+  it("closes a run's pane when a later run reaps it after it finished", async () => {
+    tmuxFake.on = true
+    tmuxFake.statusPaths.clear()
+    tmuxFake.open.clear()
+    try {
+      const terminal = new TerminalService({ loadCwd: () => '/tmp' })
+      terminal.start({ cols: 80, rows: 24 })
+      await terminal.executeTool('call-long', 'run', { command: 'make build', waitSeconds: 1 })
+      const [[pane = '', statusPath = ''] = []] = [...tmuxFake.statusPaths]
+      // It finishes after its call returned, and its dead pane stays open.
+      writeFileSync(statusPath, '0')
+      expect(tmuxFake.open.has(pane)).toBe(true)
+
+      await terminal.executeTool('call-next', 'run', { command: 'ls', waitSeconds: 1 })
+
+      expect(tmuxFake.open.has(pane)).toBe(false)
+    } finally {
+      tmuxFake.on = false
+    }
+  })
+
   it("keeps a run's output readable for its call when the terminal goes away mid-wait", async () => {
     tmuxFake.on = true
     tmuxFake.statusPaths.clear()

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

@@ -5,6 +5,7 @@ import { sleep } from '@sim/utils/helpers'
 import { afterEach, describe, expect, it } from 'vitest'
 import {
   awaitRun,
+  closeRunPane,
   isDescendantOf,
   parseFormatLines,
   pollRun,
@@ -15,13 +16,33 @@ import {
   type TmuxRunHandle,
 } from '@/main/terminal/tmux'
 
+/** What real tmux 2.9a writes for `set-option -p`, captured from the binary. */
+const TMUX_29_NO_PANE_OPTIONS =
+  'tmux: unknown option -- p\nusage: set-option [-aFgosquw] [-t target-window] option [value]\n'
+/** The same refusal from a tmux built against BSD getopt, as on macOS. */
+const TMUX_29_BSD_NO_PANE_OPTIONS =
+  'tmux: illegal option -- p\nusage: set-option [-aFgosquw] [-t target-window] option [value]\n'
+
 /** The separator the format strings use. */
-const F = '|~sim~|'
+const F = '<~sim~>'
 
 describe('parseFormatLines', () => {
   it('drops lines with the wrong field count rather than mis-assigning them', () => {
     expect(parseFormatLines(`a${F}b\nonly-one\n`, 2)).toEqual([['a', 'b']])
   })
+
+  it('reads a field that ends with part of the separator as it is', () => {
+    // A cwd or window name may end with any text, including all but the separator's last character.
+    const partial = F.slice(0, -1)
+    expect(parseFormatLines(`/tmp/x/p${partial}${F}1\n`, 2)).toEqual([[`/tmp/x/p${partial}`, '1']])
+    expect(parseFormatLines(`tail${partial}${F}%3${F}zsh\n`, 3)).toEqual([
+      [`tail${partial}`, '%3', 'zsh'],
+    ])
+  })
+
+  it('drops a line whose field holds the whole separator rather than misread it', () => {
+    expect(parseFormatLines(`a${F}b${F}c\n`, 2)).toEqual([])
+  })
 })
 
 describe('isDescendantOf', () => {
@@ -100,13 +121,17 @@ describe('run status files', () => {
 interface FakeTmuxState {
   nextWindow: number
   nextPane: number
-  panes: Record<string, { window: string; options: Record<string, string> }>
+  panes: Record<string, { window: string; options: Record<string, string>; command?: string }>
   /** Every command that reached a pane: `send-keys %1 C-c`, `kill-pane %1`. */
   log: string[]
   /** Commands the fake fails, with the error tmux would print. */
   fail?: Record<string, string>
   /** Attached clients, as `list-clients` reports them. */
   clients?: Array<{ pid: string; tty: string; session: string }>
+  /** Commands the fake holds until the file named here exists, like a busy tmux server. */
+  hold?: Record<string, string>
+  /** Commands the fake is holding right now. */
+  held?: string[]
 }
 
 const FAKE_TMUX = `
@@ -123,12 +148,20 @@ const escaped = (text) =>
   text
     .replace(/\\\\/g, '\\\\\\\\')
     .replace(/[\\x00-\\x1f]/g, (c) => '\\\\' + c.charCodeAt(0).toString(8).padStart(3, '0'))
+if (state.hold && state.hold[args[0]]) {
+  state.held = [...(state.held ?? []), args[0]]
+  save()
+  while (!fs.existsSync(state.hold[args[0]])) {
+    Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 20)
+  }
+  state.held = state.held.filter((command) => command !== args[0])
+}
 if (state.fail && state.fail[args[0]]) fail(state.fail[args[0]])
 switch (args[0]) {
   case 'new-window': {
     const window = '@' + state.nextWindow++
     const pane = '%' + state.nextPane++
-    state.panes[pane] = { window, options: {} }
+    state.panes[pane] = { window, options: {}, command: args[args.length - 1] }
     save()
     // Runs the pane's command for real, the way tmux would, when a test asks for it.
     if (process.env.FAKE_TMUX_EXEC) {
@@ -147,10 +180,17 @@ switch (args[0]) {
     break
   }
   case 'display-message': {
+    // Like tmux 3.x, a pane that is gone answers with an empty line rather than an error.
     const pane = state.panes[target()]
-    if (!pane) fail("can't find pane")
     const name = args[args.length - 1].slice(2, -1)
-    process.stdout.write((pane.options[name] ?? '') + '\\n')
+    const value = !pane
+      ? ''
+      : name === 'pane_id'
+        ? target()
+        : name === 'pane_start_command'
+          ? (pane.command ?? '')
+          : (pane.options[name] ?? '')
+    process.stdout.write(value + '\\n')
     break
   }
   case 'list-clients': {
@@ -287,16 +327,84 @@ describe('stopping a tmux run touches only its own pane', () => {
     expect(tmux.read().log).toEqual([])
   })
 
-  it('never lets a run it could not tag start, and closes no pane by id to stop it', async () => {
+  it.each([
+    ['tmux 2.9a', TMUX_29_NO_PANE_OPTIONS],
+    ['BSD getopt', TMUX_29_BSD_NO_PANE_OPTIONS],
+  ])(
+    'starts a run untracked on a tmux without pane options (%s), and never stops it by a pane id',
+    async (_build, refusal) => {
+      const tmux = fakeTmux()
+      dirs.push(tmux.dir)
+      // tmux before 3.0 has no pane options.
+      tmux.write({ ...tmux.read(), fail: { 'set-option': refusal } })
+
+      const run = await startRun('agent', 'sleep 600', null, tmux.env)
+      if ('error' in run) throw new Error(run.error)
+
+      expect(run.runId).toBeNull()
+      expect(existsSync(join(run.statusPath, '..', 'go'))).toBe(true)
+      expect(await runPaneState(run, tmux.env)).toBe('unknown')
+      await stopRun(run, tmux.env, 0)
+      // No pane is touched by an id that a restarted server might have handed to the user.
+      expect(tmux.read().log).toEqual([])
+      expect(Object.keys(tmux.read().panes)).toEqual([run.pane])
+
+      // Once its pane is gone, it can be let go.
+      const state = tmux.read()
+      delete state.panes[run.pane]
+      tmux.write(state)
+      expect(await runPaneState(run, tmux.env)).toBe('gone')
+    }
+  )
+
+  it.each(['tmux did not respond', 'server exited unexpectedly'])(
+    'refuses a run that a tmux able to tag panes did not tag (%s)',
+    async (failure) => {
+      const tmux = fakeTmux()
+      dirs.push(tmux.dir)
+      tmux.write({ ...tmux.read(), fail: { 'set-option': failure } })
+
+      const result = await startRun('agent', 'sleep 600', null, tmux.env)
+
+      // Untagged on a tmux that tags, nothing could stop it later, so it never starts.
+      expect(result).toMatchObject({ error: expect.stringContaining('was not run') })
+      expect(tmux.read().log).toEqual([])
+    }
+  )
+
+  it("closes a finished untracked run's pane, and only once it has finished", async () => {
     const tmux = fakeTmux()
     dirs.push(tmux.dir)
-    tmux.write({ ...tmux.read(), fail: { 'set-option': 'invalid option: @sim-run-id' } })
+    tmux.write({ ...tmux.read(), fail: { 'set-option': TMUX_29_NO_PANE_OPTIONS } })
+    const run = await startRun('agent', 'make build', null, tmux.env)
+    if ('error' in run) throw new Error(run.error)
 
-    const result = await startRun('agent', 'sleep 600', null, tmux.env)
+    await closeRunPane(run, tmux.env)
+    expect(Object.keys(tmux.read().panes)).toEqual([run.pane])
+
+    // Its command ended; with `remain-on-exit` its dead pane would otherwise stay open.
+    writeFileSync(run.statusPath, '0')
+    await closeRunPane(run, tmux.env)
+    expect(Object.keys(tmux.read().panes)).toEqual([])
+  })
+
+  it("never closes a pane that took a finished untracked run's id after tmux restarted", async () => {
+    const tmux = fakeTmux()
+    dirs.push(tmux.dir)
+    tmux.write({ ...tmux.read(), fail: { 'set-option': TMUX_29_NO_PANE_OPTIONS } })
+    const run = await startRun('agent', 'make build', null, tmux.env)
+    if ('error' in run) throw new Error(run.error)
+    writeFileSync(run.statusPath, '0')
+    tmux.restart()
+    // The user's own shell gets the ids the run's pane had.
+    const state = tmux.read()
+    state.panes[run.pane] = { window: run.window, options: {}, command: 'zsh' }
+    tmux.write(state)
+
+    await closeRunPane(run, 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([])
+    expect(Object.keys(tmux.read().panes)).toEqual([run.pane])
   })
 
   it('lets a tagged run start only once its pane is tagged', async () => {
@@ -318,7 +426,7 @@ 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 () => {
+  it('runs a command for real whether or not tmux could tag its pane', async () => {
     const tagged = fakeTmux({ exec: true })
     dirs.push(tagged.dir)
     const run = await startRun('agent', 'echo ran', null, tagged.env)
@@ -332,11 +440,31 @@ describe('stopping a tmux run touches only its own pane', () => {
 
     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)
+    untagged.write({ ...untagged.read(), fail: { 'set-option': TMUX_29_NO_PANE_OPTIONS } })
+    const untracked = await startRun('agent', 'echo ran', null, untagged.env)
+    if ('error' in untracked) throw new Error(untracked.error)
+    await expect
+      .poll(() => pollRun(untracked), { timeout: 10_000 })
+      .toMatchObject({ done: true, exitCode: 0, output: 'ran\n' })
+  }, 20_000)
 
+  it('holds a command until tmux has finished tagging its pane', async () => {
+    const tmux = fakeTmux({ exec: true })
+    dirs.push(tmux.dir)
+    const release = join(tmux.dir, 'release')
+    tmux.write({ ...tmux.read(), hold: { 'set-option': release } })
+    const marker = join(tmux.dir, 'ran')
+
+    const starting = startRun('agent', `touch ${JSON.stringify(marker)}`, null, tmux.env)
+    // The pane is open and the tagging call is in flight, held by tmux.
+    await expect.poll(() => tmux.read().held ?? [], { timeout: 10_000 }).toEqual(['set-option'])
+    // Time enough for an ungated command to have run.
+    await sleep(1_000)
     expect(existsSync(marker)).toBe(false)
+
+    writeFileSync(release, '')
+    const run = await starting
+    if ('error' in run) throw new Error(run.error)
+    await expect.poll(() => existsSync(marker), { timeout: 10_000 }).toBe(true)
   }, 20_000)
 })