From 8cc81f4e3da2c6036bb51e74435049c63e7d07ac Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 17:04:09 -0700 Subject: [PATCH 1/5] fix(desktop): run an agent command on tmux that cannot tag its pane, untracked Agent `run` inside tmux tagged its pane with a pane option (`set-option -p`, tmux 3.0+) and refused the command when that failed, so on older tmux every agent run failed, with the background executor off too. A run whose pane cannot be tagged now goes ahead untracked: Stop, sign-out and switching Terminal off leave it alone, since nothing could tell its pane from one of the user's. Tagged runs are unchanged. The gate still holds every command until the tagging call has finished. --- apps/desktop/src/main/terminal/index.ts | 3 +- apps/desktop/src/main/terminal/tmux.test.ts | 46 +++++++++++++++++---- apps/desktop/src/main/terminal/tmux.ts | 35 ++++++++-------- packages/terminal-protocol/src/index.ts | 9 ++-- 4 files changed, 64 insertions(+), 29 deletions(-) diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index 56dfe8b8348..9a994dfa767 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/apps/desktop/src/main/terminal/index.ts @@ -521,7 +521,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) diff --git a/apps/desktop/src/main/terminal/tmux.test.ts b/apps/desktop/src/main/terminal/tmux.test.ts index b160a552505..59edc6d77bd 100644 --- a/apps/desktop/src/main/terminal/tmux.test.ts +++ b/apps/desktop/src/main/terminal/tmux.test.ts @@ -107,6 +107,9 @@ interface FakeTmuxState { fail?: Record /** Attached clients, as `list-clients` reports them. */ clients?: Array<{ pid: string; tty: string; session: string }> + + /** Commands the fake answers only after this many milliseconds, like a busy tmux server. */ + delay?: Record } const FAKE_TMUX = ` @@ -123,6 +126,10 @@ const escaped = (text) => text .replace(/\\\\/g, '\\\\\\\\') .replace(/[\\x00-\\x1f]/g, (c) => '\\\\' + c.charCodeAt(0).toString(8).padStart(3, '0')) + +if (state.delay && state.delay[args[0]]) { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, state.delay[args[0]]) +} if (state.fail && state.fail[args[0]]) fail(state.fail[args[0]]) switch (args[0]) { case 'new-window': { @@ -287,16 +294,22 @@ 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('starts a run it could not tag, untracked, and never stops it by a pane id', async () => { const tmux = fakeTmux() dirs.push(tmux.dir) + // tmux before 3.0 has no pane options. tmux.write({ ...tmux.read(), fail: { 'set-option': 'invalid option: @sim-run-id' } }) - const result = await startRun('agent', 'sleep 600', null, tmux.env) + const run = await startRun('agent', 'sleep 600', null, tmux.env) + if ('error' in run) throw new Error(run.error) - 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(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]) }) it('lets a tagged run start only once its pane is tagged', async () => { @@ -318,7 +331,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) @@ -333,10 +346,27 @@ 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) + 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) + tmux.write({ ...tmux.read(), delay: { 'set-option': 2_000 } }) + const marker = join(tmux.dir, 'ran') + + const starting = startRun('agent', `touch ${JSON.stringify(marker)}`, null, tmux.env) + await sleep(1_200) + // The pane exists and its script is running, but the tag is not on it yet. + expect(Object.keys(tmux.read().panes)).toHaveLength(1) expect(existsSync(marker)).toBe(false) + + 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) }) diff --git a/apps/desktop/src/main/terminal/tmux.ts b/apps/desktop/src/main/terminal/tmux.ts index 8896ae3536c..ce4f848d8a2 100644 --- a/apps/desktop/src/main/terminal/tmux.ts +++ b/apps/desktop/src/main/terminal/tmux.ts @@ -311,9 +311,11 @@ export interface TmuxRunHandle { pane: string /** * Tagged on the pane as the `@sim-run-id` user option. Window and pane ids restart from zero - * with the tmux server, so only the tag proves a pane is still this run's. + * with the tmux server, so only the tag proves a pane is still this run's. Null when tmux could + * not tag the pane (tmux before 3.0 has no pane options): the run goes ahead untracked, and + * nothing ever stops it, since nothing could tell its pane from one of the user's. */ - runId: string + runId: string | null outPath: string statusPath: string dispose(): void @@ -368,10 +370,10 @@ export async function startRun( // into the pipeline and print `No such file or directory` into the user's own // tmux window, minutes after they closed the tab. // - // 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 once the tagging call has - // surely failed, and its pane closes on its own; no one has to close a pane whose id might no - // longer be its own. + // The command waits for the tagging call to finish (the go file), so a tagged run's command + // never runs before a later stop could recognize its pane. If Sim never releases it (it quit + // mid-start), the script gives up once the tagging call has surely ended 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. @@ -410,18 +412,16 @@ export async function startRun( return { error: created.stderr.trim() || 'tmux could not open a window for the command.' } } const [window = '', pane = ''] = created.stdout.trim().split(' ') - const runId = generateId() + const tag = generateId() // An untagged pane is never treated as the run's: without the tag a stop could not tell it from - // 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) + // a pane the user opened later under the same id, so the run is left untracked. + const tagged = await runTmux(['set-option', '-p', '-t', pane, RUN_ID_OPTION, tag], env) if (!tagged.ok) { - // 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 (${tagged.stderr.trim() || 'no detail'}), so the command was not run.`, - } + logger.warn('tmux could not tag a run pane; the run goes ahead untracked', { + error: tagged.stderr.trim(), + }) } + const runId = tagged.ok ? tag : null try { writeFileSync(goPath, '') } catch (error) { @@ -435,14 +435,15 @@ export async function startRun( /** * Whether the run's pane is still the run's: `ours`, or `gone` when tmux has no such pane or the * pane under that id is not tagged as this run's (the user closed it, or a restarted tmux server - * handed the id to one of the user's own panes). `unknown` when tmux could not be asked: such a - * pane is neither touched nor given up on. + * handed the id to one of the user's own panes). `unknown` when tmux could not be asked, or the + * run is untracked: such a pane is neither touched nor given up on. */ export async function runPaneState( handle: TmuxRunHandle, env: NodeJS.ProcessEnv ): Promise<'ours' | 'gone' | 'unknown'> { if (!handle.pane) return 'gone' + if (handle.runId === null) return 'unknown' const shown = await runTmux( ['display-message', '-p', '-t', handle.pane, `#{${RUN_ID_OPTION}}`], env diff --git a/packages/terminal-protocol/src/index.ts b/packages/terminal-protocol/src/index.ts index 7356750ca7c..78c9817a2ec 100644 --- a/packages/terminal-protocol/src/index.ts +++ b/packages/terminal-protocol/src/index.ts @@ -177,8 +177,8 @@ export interface TerminalToolArgs { */ terminalId?: string /** - * Which tmux pane to act on, as a tmux target (`session:window.pane`), for - * a terminal that has tmux attached. Omitting it uses that session's active + * Which tmux pane to act on, as a tmux target (`session:window.pane`, or a + * run's pane id `%N`), for a terminal that has tmux attached. Omitting it uses that session's active * pane. Ignored when the terminal is a plain shell. */ pane?: string @@ -214,7 +214,10 @@ export interface TerminalRunResult { durationMs: number cwd: string | null terminalId: string - /** Set when the command ran in tmux: the target it ran under. */ + /** + * Set when the command ran in tmux: its own pane's id (`%N`), a tmux target that `read`, + * `input`, `kill` and `close` accept as `pane`. + */ pane?: string /** True when output was elided to fit {@link MAX_TOOL_OUTPUT_CHARS}. */ truncated: boolean From 67b544b448520e47d912c068031abd91a0fc54b2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 17:32:49 -0700 Subject: [PATCH 2/5] fix(desktop): report an untracked run a Stop could not end as still running, and reap it once its pane is gone --- apps/desktop/src/main/terminal/index.ts | 6 ++- .../desktop/src/main/terminal/service.test.ts | 38 +++++++++++++++++-- apps/desktop/src/main/terminal/tmux.test.ts | 30 +++++++++++++-- apps/desktop/src/main/terminal/tmux.ts | 14 +++---- 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index 9a994dfa767..62a7be318a8 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/apps/desktop/src/main/terminal/index.ts @@ -1301,7 +1301,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() diff --git a/apps/desktop/src/main/terminal/service.test.ts b/apps/desktop/src/main/terminal/service.test.ts index 24256d6b30f..8c822de00fc 100644 --- a/apps/desktop/src/main/terminal/service.test.ts +++ b/apps/desktop/src/main/terminal/service.test.ts @@ -16,6 +16,8 @@ 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(), + /** Runs start untracked, as on a tmux too old to tag their panes. */ + untracked: false, statusPaths: new Map(), })) @@ -40,7 +42,7 @@ vi.mock('@/main/terminal/tmux', async () => { 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 }), @@ -48,12 +50,13 @@ vi.mock('@/main/terminal/tmux', async () => { }, runPaneState: async (...args: Parameters) => { 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) => { 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') }, @@ -587,6 +590,35 @@ 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("keeps a run's output readable for its call when the terminal goes away mid-wait", async () => { tmuxFake.on = true tmuxFake.statusPaths.clear() diff --git a/apps/desktop/src/main/terminal/tmux.test.ts b/apps/desktop/src/main/terminal/tmux.test.ts index 59edc6d77bd..44011279b67 100644 --- a/apps/desktop/src/main/terminal/tmux.test.ts +++ b/apps/desktop/src/main/terminal/tmux.test.ts @@ -110,6 +110,11 @@ interface FakeTmuxState { /** Commands the fake answers only after this many milliseconds, like a busy tmux server. */ delay?: Record + + /** Commands the fake holds until the file named here exists, like a busy tmux server. */ + hold?: Record + /** Commands the fake is holding right now. */ + held?: string[] } const FAKE_TMUX = ` @@ -129,6 +134,14 @@ const escaped = (text) => if (state.delay && state.delay[args[0]]) { Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, state.delay[args[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]) { @@ -310,6 +323,12 @@ describe('stopping a tmux run touches only its own pane', () => { // 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('lets a tagged run start only once its pane is tagged', async () => { @@ -356,15 +375,18 @@ describe('stopping a tmux run touches only its own pane', () => { it('holds a command until tmux has finished tagging its pane', async () => { const tmux = fakeTmux({ exec: true }) dirs.push(tmux.dir) - tmux.write({ ...tmux.read(), delay: { 'set-option': 2_000 } }) + 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) - await sleep(1_200) - // The pane exists and its script is running, but the tag is not on it yet. - expect(Object.keys(tmux.read().panes)).toHaveLength(1) + // 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) diff --git a/apps/desktop/src/main/terminal/tmux.ts b/apps/desktop/src/main/terminal/tmux.ts index ce4f848d8a2..22741d86789 100644 --- a/apps/desktop/src/main/terminal/tmux.ts +++ b/apps/desktop/src/main/terminal/tmux.ts @@ -435,19 +435,19 @@ export async function startRun( /** * Whether the run's pane is still the run's: `ours`, or `gone` when tmux has no such pane or the * pane under that id is not tagged as this run's (the user closed it, or a restarted tmux server - * handed the id to one of the user's own panes). `unknown` when tmux could not be asked, or the - * run is untracked: such a pane is neither touched nor given up on. + * handed the id to one of the user's own panes). `unknown` when tmux could not be asked, or an + * untracked run's pane still exists, since nothing proves whose it is: such a pane is neither + * touched nor given up on. */ export async function runPaneState( handle: TmuxRunHandle, env: NodeJS.ProcessEnv ): Promise<'ours' | 'gone' | 'unknown'> { if (!handle.pane) return 'gone' - if (handle.runId === null) return 'unknown' - const shown = await runTmux( - ['display-message', '-p', '-t', handle.pane, `#{${RUN_ID_OPTION}}`], - env - ) + // An untracked run's pane can still be found missing, with a format every tmux knows. + const format = handle.runId === null ? '#{pane_id}' : `#{${RUN_ID_OPTION}}` + const shown = await runTmux(['display-message', '-p', '-t', handle.pane, format], env) + if (shown.ok && handle.runId === null) return 'unknown' if (shown.ok) return shown.stdout.trim() === handle.runId ? 'ours' : 'gone' return /can't find|no server running/i.test(shown.stderr) ? 'gone' : 'unknown' } From 1ce44c8c9536246df48f1cdfb7f77a291a793cd2 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 18:44:25 -0700 Subject: [PATCH 3/5] fix(desktop): run untracked only on tmux without pane options, and reap and close finished untracked panes - Only tmux before 3.0, which refuses `set-option -p` as an unknown flag or invalid option, runs a command untracked. A tmux that can tag panes but did not (it timed out, or failed otherwise) gets the run refused, as before, so no command starts that Stop and sign-out could never end. - tmux 3.x answers `display-message` for a pane that is gone with an empty line rather than an error, so an untracked run's pane is gone unless tmux echoes its id back. - A finished untracked run's pane is closed, and a run that finishes after its call returned has its pane closed when it is reaped, so dead panes kept by `remain-on-exit` do not pile up. --- apps/desktop/src/main/terminal/index.ts | 5 +- .../desktop/src/main/terminal/service.test.ts | 25 +++++++ apps/desktop/src/main/terminal/tmux.test.ts | 70 ++++++++++++++----- apps/desktop/src/main/terminal/tmux.ts | 28 ++++++-- 4 files changed, 105 insertions(+), 23 deletions(-) diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index 62a7be318a8..c8de703d177 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/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() } diff --git a/apps/desktop/src/main/terminal/service.test.ts b/apps/desktop/src/main/terminal/service.test.ts index 8c822de00fc..55204d4ce8e 100644 --- a/apps/desktop/src/main/terminal/service.test.ts +++ b/apps/desktop/src/main/terminal/service.test.ts @@ -18,6 +18,8 @@ const tmuxFake = vi.hoisted(() => ({ gone: new Set(), /** 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(), statusPaths: new Map(), })) @@ -39,6 +41,7 @@ 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, @@ -62,6 +65,7 @@ vi.mock('@/main/terminal/tmux', async () => { }, closeRunPane: async (...args: Parameters) => { if (!tmuxFake.on) return actual.closeRunPane(...args) + tmuxFake.open.delete(args[0].pane) }, } }) @@ -619,6 +623,27 @@ describe('agent commands in tmux', () => { } }) + 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() diff --git a/apps/desktop/src/main/terminal/tmux.test.ts b/apps/desktop/src/main/terminal/tmux.test.ts index 44011279b67..58f4e9355b1 100644 --- a/apps/desktop/src/main/terminal/tmux.test.ts +++ b/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, @@ -167,10 +168,11 @@ 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() : (pane.options[name] ?? '') + process.stdout.write(value + '\\n') break } case 'list-clients': { @@ -307,28 +309,62 @@ describe('stopping a tmux run touches only its own pane', () => { expect(tmux.read().log).toEqual([]) }) - it('starts a run it could not tag, untracked, and never stops it by a pane id', async () => { + it.each(['invalid option: @sim-run-id', 'unknown flag -p'])( + 'starts a run untracked on a tmux without pane options (%s), and never stops it by a pane id', + async (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 before 3.0 has no pane options. tmux.write({ ...tmux.read(), fail: { 'set-option': 'invalid option: @sim-run-id' } }) - - const run = await startRun('agent', 'sleep 600', null, tmux.env) + const run = await startRun('agent', 'make build', 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([]) + await closeRunPane(run, tmux.env) 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') + // 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('lets a tagged run start only once its pane is tagged', async () => { diff --git a/apps/desktop/src/main/terminal/tmux.ts b/apps/desktop/src/main/terminal/tmux.ts index 22741d86789..9233748c341 100644 --- a/apps/desktop/src/main/terminal/tmux.ts +++ b/apps/desktop/src/main/terminal/tmux.ts @@ -330,6 +330,9 @@ const RUN_GATE_POLLS = Math.ceil((2 * TMUX_TIMEOUT_MS + 5_000) / 50) /** The tmux user option that marks a pane as one run's own. */ const RUN_ID_OPTION = '@sim-run-id' +/** How tmux before 3.0, which has no pane options, refuses `set-option -p`. */ +const NO_PANE_OPTIONS = /unknown flag|invalid option/i + /** * Starts a command in a dedicated tmux window. * @@ -414,10 +417,18 @@ export async function startRun( const [window = '', pane = ''] = created.stdout.trim().split(' ') const tag = generateId() // An untagged pane is never treated as the run's: without the tag a stop could not tell it from - // a pane the user opened later under the same id, so the run is left untracked. + // a pane the user opened later under the same id. const tagged = await runTmux(['set-option', '-p', '-t', pane, RUN_ID_OPTION, tag], env) + if (!tagged.ok && !NO_PANE_OPTIONS.test(tagged.stderr)) { + // A tmux that can tag panes but did not (it timed out, or failed otherwise) gets no command + // that nothing could stop: without the go file the wrapper exits by itself. + dispose() + return { + error: `tmux could not mark the command's pane (${tagged.stderr.trim() || 'no detail'}), so the command was not run.`, + } + } if (!tagged.ok) { - logger.warn('tmux could not tag a run pane; the run goes ahead untracked', { + logger.warn('This tmux cannot tag a run pane; the run goes ahead untracked', { error: tagged.stderr.trim(), }) } @@ -447,7 +458,10 @@ export async function runPaneState( // An untracked run's pane can still be found missing, with a format every tmux knows. const format = handle.runId === null ? '#{pane_id}' : `#{${RUN_ID_OPTION}}` const shown = await runTmux(['display-message', '-p', '-t', handle.pane, format], env) - if (shown.ok && handle.runId === null) return 'unknown' + // tmux 3.x answers for a missing pane with an empty line rather than an error. + if (shown.ok && handle.runId === null) { + return shown.stdout.trim() === handle.pane ? 'unknown' : 'gone' + } if (shown.ok) return shown.stdout.trim() === handle.runId ? 'ours' : 'gone' return /can't find|no server running/i.test(shown.stderr) ? 'gone' : 'unknown' } @@ -531,10 +545,14 @@ export async function killPane(target: string, env: NodeJS.ProcessEnv): Promise< /** * Closes the pane opened by {@link startRun}, and with it the window once that pane is the last - * one in it. Only the run's own pane, and only while it is still the run's. + * one in it. Only the run's own pane, and only while it is still the run's. An untracked run's + * pane is closed only once the run has written its exit status: its command has just ended in + * that pane, so the id is still the one the run opened. */ export async function closeRunPane(handle: TmuxRunHandle, env: NodeJS.ProcessEnv): Promise { - if ((await runPaneState(handle, env)) !== 'ours') return + const state = await runPaneState(handle, env) + const finishedUntracked = handle.runId === null && state === 'unknown' && isRunComplete(handle) + if (state !== 'ours' && !finishedUntracked) return const killed = await runTmux(['kill-pane', '-t', handle.pane], env) if (!killed.ok) { logger.warn('Could not close the tmux run pane', { error: killed.stderr.trim() }) From 26c99a133e4ddfeb6ae624fe2653dcbdda1eb7c6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 19:06:44 -0700 Subject: [PATCH 4/5] fix(desktop): close a finished untracked run's pane only when its start command proves it is the run's --- apps/desktop/src/main/terminal/tmux.test.ts | 31 +++++++++++++++++++-- apps/desktop/src/main/terminal/tmux.ts | 25 ++++++++++++++--- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/main/terminal/tmux.test.ts b/apps/desktop/src/main/terminal/tmux.test.ts index 58f4e9355b1..07e3e6a0def 100644 --- a/apps/desktop/src/main/terminal/tmux.test.ts +++ b/apps/desktop/src/main/terminal/tmux.test.ts @@ -101,7 +101,7 @@ describe('run status files', () => { interface FakeTmuxState { nextWindow: number nextPane: number - panes: Record }> + panes: Record; 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. */ @@ -149,7 +149,7 @@ 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) { @@ -171,7 +171,13 @@ switch (args[0]) { // Like tmux 3.x, a pane that is gone answers with an empty line rather than an error. const pane = state.panes[target()] const name = args[args.length - 1].slice(2, -1) - const value = !pane ? '' : name === 'pane_id' ? target() : (pane.options[name] ?? '') + const value = !pane + ? '' + : name === 'pane_id' + ? target() + : name === 'pane_start_command' + ? (pane.command ?? '') + : (pane.options[name] ?? '') process.stdout.write(value + '\\n') break } @@ -367,6 +373,25 @@ describe('stopping a tmux run touches only its own pane', () => { 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': 'invalid option: @sim-run-id' } }) + 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(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 () => { const tmux = fakeTmux() const run = await started(tmux) diff --git a/apps/desktop/src/main/terminal/tmux.ts b/apps/desktop/src/main/terminal/tmux.ts index 9233748c341..c3e368370ea 100644 --- a/apps/desktop/src/main/terminal/tmux.ts +++ b/apps/desktop/src/main/terminal/tmux.ts @@ -19,7 +19,7 @@ import { spawn } from 'node:child_process' import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join } from 'node:path' import { createLogger } from '@sim/logger' import type { TerminalPaneState } from '@sim/terminal-protocol' import { getErrorMessage } from '@sim/utils/errors' @@ -543,15 +543,32 @@ export async function killPane(target: string, env: NodeJS.ProcessEnv): Promise< return runTmux(['kill-pane', '-t', target], env) } +/** + * Whether the pane under an untracked run's id was started with that run's own script, whose path + * is unique to the run. Every tmux reports a pane's start command, so this holds where tags do not. + */ +async function startedByRun(handle: TmuxRunHandle, env: NodeJS.ProcessEnv): Promise { + const script = join(dirname(handle.statusPath), 'run.sh') + const shown = await runTmux( + ['display-message', '-p', '-t', handle.pane, '#{pane_start_command}'], + env + ) + return shown.ok && shown.stdout.includes(script) +} + /** * Closes the pane opened by {@link startRun}, and with it the window once that pane is the last * one in it. Only the run's own pane, and only while it is still the run's. An untracked run's - * pane is closed only once the run has written its exit status: its command has just ended in - * that pane, so the id is still the one the run opened. + * pane is closed only once the run has written its exit status, and only if the pane was started + * by the run's own script: a restarted tmux may have handed the id to one of the user's panes. */ export async function closeRunPane(handle: TmuxRunHandle, env: NodeJS.ProcessEnv): Promise { const state = await runPaneState(handle, env) - const finishedUntracked = handle.runId === null && state === 'unknown' && isRunComplete(handle) + const finishedUntracked = + handle.runId === null && + state === 'unknown' && + isRunComplete(handle) && + (await startedByRun(handle, env)) if (state !== 'ours' && !finishedUntracked) return const killed = await runTmux(['kill-pane', '-t', handle.pane], env) if (!killed.ok) { From 8dddbd7c18d25f360167caba62ace3b51132c499 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 6 Oct 2026 21:17:14 -0700 Subject: [PATCH 5/5] fix(desktop): recognise how real tmux before 3.0 refuses pane options, and use a separator no field can straddle - Real tmux 2.9a refuses `set-option -p` with its own getopt's `unknown option -- p` and set-option's usage line (BSD getopt on macOS: `illegal option -- p`). The untracked fallback only matched later wordings, so on real pre-3.0 tmux every run was refused. The fake tmux and the tests now use the text captured from real 2.9a. - The `-F` separator `|~sim~|` began and ended with the same character, so a field ending in `|~sim~` was misread rather than dropped. `<~sim~>` has no proper prefix that is also a suffix, so it is only found where it was written or wholly inside a field, whose line is then dropped. --- apps/desktop/src/main/terminal/tmux.test.ts | 43 ++++++++++++++------- apps/desktop/src/main/terminal/tmux.ts | 16 +++++--- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/apps/desktop/src/main/terminal/tmux.test.ts b/apps/desktop/src/main/terminal/tmux.test.ts index 07e3e6a0def..88c1ccb1942 100644 --- a/apps/desktop/src/main/terminal/tmux.test.ts +++ b/apps/desktop/src/main/terminal/tmux.test.ts @@ -16,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', () => { @@ -108,10 +128,6 @@ interface FakeTmuxState { fail?: Record /** Attached clients, as `list-clients` reports them. */ clients?: Array<{ pid: string; tty: string; session: string }> - - /** Commands the fake answers only after this many milliseconds, like a busy tmux server. */ - delay?: Record - /** Commands the fake holds until the file named here exists, like a busy tmux server. */ hold?: Record /** Commands the fake is holding right now. */ @@ -132,10 +148,6 @@ const escaped = (text) => text .replace(/\\\\/g, '\\\\\\\\') .replace(/[\\x00-\\x1f]/g, (c) => '\\\\' + c.charCodeAt(0).toString(8).padStart(3, '0')) - -if (state.delay && state.delay[args[0]]) { - Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, state.delay[args[0]]) - if (state.hold && state.hold[args[0]]) { state.held = [...(state.held ?? []), args[0]] save() @@ -315,9 +327,12 @@ describe('stopping a tmux run touches only its own pane', () => { expect(tmux.read().log).toEqual([]) }) - it.each(['invalid option: @sim-run-id', 'unknown flag -p'])( + 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 (refusal) => { + async (_build, refusal) => { const tmux = fakeTmux() dirs.push(tmux.dir) // tmux before 3.0 has no pane options. @@ -360,7 +375,7 @@ describe('stopping a tmux run touches only its own pane', () => { 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) @@ -376,7 +391,7 @@ describe('stopping a tmux run touches only its own pane', () => { 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': '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) writeFileSync(run.statusPath, '0') @@ -425,7 +440,7 @@ 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' } }) + 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 diff --git a/apps/desktop/src/main/terminal/tmux.ts b/apps/desktop/src/main/terminal/tmux.ts index c3e368370ea..6fb40d82ba3 100644 --- a/apps/desktop/src/main/terminal/tmux.ts +++ b/apps/desktop/src/main/terminal/tmux.ts @@ -41,10 +41,11 @@ const RUN_POLL_INTERVAL_MS = 250 /** * Field separator for `-F` output. Printable on purpose: tmux 3.4 and 3.5 print a control * character as its octal escape, so a control-character separator arrived as the text `\037` and - * no line split. No tmux escapes these characters, and a field that happened to contain the - * separator would change the line's field count, so that line is dropped rather than misread. + * no line split. No tmux escapes these characters. No proper prefix of the separator is also a + * suffix of it, so it can only be found where it was written or wholly inside a field: a field + * holding it changes the line's field count, and that line is dropped rather than misread. */ -const FIELD = '|~sim~|' +const FIELD = '<~sim~>' export interface TmuxCommandResult { ok: boolean @@ -330,8 +331,13 @@ const RUN_GATE_POLLS = Math.ceil((2 * TMUX_TIMEOUT_MS + 5_000) / 50) /** The tmux user option that marks a pane as one run's own. */ const RUN_ID_OPTION = '@sim-run-id' -/** How tmux before 3.0, which has no pane options, refuses `set-option -p`. */ -const NO_PANE_OPTIONS = /unknown flag|invalid option/i +/** + * How tmux before 3.0, which has no pane options, refuses `set-option -p`. Its own getopt prints + * `unknown option -- p` (BSD getopt on macOS: `illegal option -- p`) followed by set-option's usage + * line; later wordings are kept for any build that phrases it so. + */ +const NO_PANE_OPTIONS = + /unknown option -- p|illegal option -- p|usage: set-option|unknown flag|invalid option/i /** * Starts a command in a dedicated tmux window.