Skip to content

Commit 26c99a1

Browse files
committed
fix(desktop): close a finished untracked run's pane only when its start command proves it is the run's
1 parent 1ce44c8 commit 26c99a1

2 files changed

Lines changed: 49 additions & 7 deletions

File tree

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ describe('run status files', () => {
101101
interface FakeTmuxState {
102102
nextWindow: number
103103
nextPane: number
104-
panes: Record<string, { window: string; options: Record<string, string> }>
104+
panes: Record<string, { window: string; options: Record<string, string>; command?: string }>
105105
/** Every command that reached a pane: `send-keys %1 C-c`, `kill-pane %1`. */
106106
log: string[]
107107
/** Commands the fake fails, with the error tmux would print. */
@@ -149,7 +149,7 @@ switch (args[0]) {
149149
case 'new-window': {
150150
const window = '@' + state.nextWindow++
151151
const pane = '%' + state.nextPane++
152-
state.panes[pane] = { window, options: {} }
152+
state.panes[pane] = { window, options: {}, command: args[args.length - 1] }
153153
save()
154154
// Runs the pane's command for real, the way tmux would, when a test asks for it.
155155
if (process.env.FAKE_TMUX_EXEC) {
@@ -171,7 +171,13 @@ switch (args[0]) {
171171
// Like tmux 3.x, a pane that is gone answers with an empty line rather than an error.
172172
const pane = state.panes[target()]
173173
const name = args[args.length - 1].slice(2, -1)
174-
const value = !pane ? '' : name === 'pane_id' ? target() : (pane.options[name] ?? '')
174+
const value = !pane
175+
? ''
176+
: name === 'pane_id'
177+
? target()
178+
: name === 'pane_start_command'
179+
? (pane.command ?? '')
180+
: (pane.options[name] ?? '')
175181
process.stdout.write(value + '\\n')
176182
break
177183
}
@@ -367,6 +373,25 @@ describe('stopping a tmux run touches only its own pane', () => {
367373
expect(Object.keys(tmux.read().panes)).toEqual([])
368374
})
369375

376+
it("never closes a pane that took a finished untracked run's id after tmux restarted", async () => {
377+
const tmux = fakeTmux()
378+
dirs.push(tmux.dir)
379+
tmux.write({ ...tmux.read(), fail: { 'set-option': 'invalid option: @sim-run-id' } })
380+
const run = await startRun('agent', 'make build', null, tmux.env)
381+
if ('error' in run) throw new Error(run.error)
382+
writeFileSync(run.statusPath, '0')
383+
tmux.restart()
384+
// The user's own shell gets the ids the run's pane had.
385+
const state = tmux.read()
386+
state.panes[run.pane] = { window: run.window, options: {}, command: 'zsh' }
387+
tmux.write(state)
388+
389+
await closeRunPane(run, tmux.env)
390+
391+
expect(tmux.read().log).toEqual([])
392+
expect(Object.keys(tmux.read().panes)).toEqual([run.pane])
393+
})
394+
370395
it('lets a tagged run start only once its pane is tagged', async () => {
371396
const tmux = fakeTmux()
372397
const run = await started(tmux)

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

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import { spawn } from 'node:child_process'
2020
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
2121
import { tmpdir } from 'node:os'
22-
import { join } from 'node:path'
22+
import { dirname, join } from 'node:path'
2323
import { createLogger } from '@sim/logger'
2424
import type { TerminalPaneState } from '@sim/terminal-protocol'
2525
import { getErrorMessage } from '@sim/utils/errors'
@@ -543,15 +543,32 @@ export async function killPane(target: string, env: NodeJS.ProcessEnv): Promise<
543543
return runTmux(['kill-pane', '-t', target], env)
544544
}
545545

546+
/**
547+
* Whether the pane under an untracked run's id was started with that run's own script, whose path
548+
* is unique to the run. Every tmux reports a pane's start command, so this holds where tags do not.
549+
*/
550+
async function startedByRun(handle: TmuxRunHandle, env: NodeJS.ProcessEnv): Promise<boolean> {
551+
const script = join(dirname(handle.statusPath), 'run.sh')
552+
const shown = await runTmux(
553+
['display-message', '-p', '-t', handle.pane, '#{pane_start_command}'],
554+
env
555+
)
556+
return shown.ok && shown.stdout.includes(script)
557+
}
558+
546559
/**
547560
* Closes the pane opened by {@link startRun}, and with it the window once that pane is the last
548561
* one in it. Only the run's own pane, and only while it is still the run's. An untracked run's
549-
* pane is closed only once the run has written its exit status: its command has just ended in
550-
* that pane, so the id is still the one the run opened.
562+
* pane is closed only once the run has written its exit status, and only if the pane was started
563+
* by the run's own script: a restarted tmux may have handed the id to one of the user's panes.
551564
*/
552565
export async function closeRunPane(handle: TmuxRunHandle, env: NodeJS.ProcessEnv): Promise<void> {
553566
const state = await runPaneState(handle, env)
554-
const finishedUntracked = handle.runId === null && state === 'unknown' && isRunComplete(handle)
567+
const finishedUntracked =
568+
handle.runId === null &&
569+
state === 'unknown' &&
570+
isRunComplete(handle) &&
571+
(await startedByRun(handle, env))
555572
if (state !== 'ours' && !finishedUntracked) return
556573
const killed = await runTmux(['kill-pane', '-t', handle.pane], env)
557574
if (!killed.ok) {

0 commit comments

Comments
 (0)