Skip to content

Commit 07697ce

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 aa07d1a commit 07697ce

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
@@ -100,7 +100,7 @@ describe('run status files', () => {
100100
interface FakeTmuxState {
101101
nextWindow: number
102102
nextPane: number
103-
panes: Record<string, { window: string; options: Record<string, string> }>
103+
panes: Record<string, { window: string; options: Record<string, string>; command?: string }>
104104
/** Every command that reached a pane: `send-keys %1 C-c`, `kill-pane %1`. */
105105
log: string[]
106106
/** Commands the fake fails, with the error tmux would print. */
@@ -132,7 +132,7 @@ switch (args[0]) {
132132
case 'new-window': {
133133
const window = '@' + state.nextWindow++
134134
const pane = '%' + state.nextPane++
135-
state.panes[pane] = { window, options: {} }
135+
state.panes[pane] = { window, options: {}, command: args[args.length - 1] }
136136
save()
137137
// Runs the pane's command for real, the way tmux would, when a test asks for it.
138138
if (process.env.FAKE_TMUX_EXEC) {
@@ -154,7 +154,13 @@ switch (args[0]) {
154154
// Like tmux 3.x, a pane that is gone answers with an empty line rather than an error.
155155
const pane = state.panes[target()]
156156
const name = args[args.length - 1].slice(2, -1)
157-
const value = !pane ? '' : name === 'pane_id' ? target() : (pane.options[name] ?? '')
157+
const value = !pane
158+
? ''
159+
: name === 'pane_id'
160+
? target()
161+
: name === 'pane_start_command'
162+
? (pane.command ?? '')
163+
: (pane.options[name] ?? '')
158164
process.stdout.write(value + '\\n')
159165
break
160166
}
@@ -316,6 +322,25 @@ describe('stopping a tmux run touches only its own pane', () => {
316322
expect(Object.keys(tmux.read().panes)).toEqual([])
317323
})
318324

325+
it("never closes a pane that took a finished untracked run's id after tmux restarted", async () => {
326+
const tmux = fakeTmux()
327+
dirs.push(tmux.dir)
328+
tmux.write({ ...tmux.read(), fail: { 'set-option': 'invalid option: @sim-run-id' } })
329+
const run = await startRun('agent', 'make build', null, tmux.env)
330+
if ('error' in run) throw new Error(run.error)
331+
writeFileSync(run.statusPath, '0')
332+
tmux.restart()
333+
// The user's own shell gets the ids the run's pane had.
334+
const state = tmux.read()
335+
state.panes[run.pane] = { window: run.window, options: {}, command: 'zsh' }
336+
tmux.write(state)
337+
338+
await closeRunPane(run, tmux.env)
339+
340+
expect(tmux.read().log).toEqual([])
341+
expect(Object.keys(tmux.read().panes)).toEqual([run.pane])
342+
})
343+
319344
it('lets a tagged run start only once its pane is tagged', async () => {
320345
const tmux = fakeTmux()
321346
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'
@@ -538,15 +538,32 @@ export async function killPane(target: string, env: NodeJS.ProcessEnv): Promise<
538538
return runTmux(['kill-pane', '-t', target], env)
539539
}
540540

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

0 commit comments

Comments
 (0)