Skip to content

Commit f55a277

Browse files
committed
fix(desktop): report an untracked run a Stop could not end as still running, and reap it once its pane is gone
1 parent 62f4787 commit f55a277

4 files changed

Lines changed: 71 additions & 19 deletions

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1301,7 +1301,11 @@ export class TerminalService {
13011301
if (latch.signal.aborted) void latch.stopRunning()
13021302
const outcome = await Promise.race([
13031303
awaitRun(handle, waitMs),
1304-
stopped.then(() => ({ ...pollRun(handle), done: true })),
1304+
// A stopped run's closed pane never writes its status. An untracked run is never stopped,
1305+
// so it is still going unless its status says otherwise.
1306+
stopped.then(() =>
1307+
handle.runId === null ? pollRun(handle) : { ...pollRun(handle), done: true }
1308+
),
13051309
]).finally(() => {
13061310
this.awaitedRuns.delete(handle)
13071311
if (this.releasedAwaitedRuns.delete(handle)) handle.dispose()

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

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const tmuxFake = vi.hoisted(() => ({
1616
stopped: [] as string[],
1717
/** Panes no longer the run's (closed by the user, or reused after a tmux restart). */
1818
gone: new Set<string>(),
19+
/** Runs start untracked, as on a tmux too old to tag their panes. */
20+
untracked: false,
1921
statusPaths: new Map<string, string>(),
2022
}))
2123

@@ -40,20 +42,21 @@ vi.mock('@/main/terminal/tmux', async () => {
4042
return {
4143
window: `@${pane.slice(1)}`,
4244
pane,
43-
runId: `run-${pane}`,
45+
runId: tmuxFake.untracked ? null : `run-${pane}`,
4446
outPath: join(dir, 'out'),
4547
statusPath,
4648
dispose: () => rmSync(dir, { recursive: true, force: true }),
4749
}
4850
},
4951
runPaneState: async (...args: Parameters<typeof actual.runPaneState>) => {
5052
if (!tmuxFake.on) return actual.runPaneState(...args)
51-
return tmuxFake.gone.has(args[0].pane) ? 'gone' : 'ours'
53+
if (tmuxFake.gone.has(args[0].pane)) return 'gone'
54+
return args[0].runId === null ? 'unknown' : 'ours'
5255
},
5356
stopRun: async (...args: Parameters<typeof actual.stopRun>) => {
5457
if (!tmuxFake.on) return actual.stopRun(...args)
5558
const [handle] = args
56-
if (tmuxFake.gone.has(handle.pane)) return
59+
if (tmuxFake.gone.has(handle.pane) || handle.runId === null) return
5760
tmuxFake.stopped.push(handle.pane)
5861
writeFileSync(handle.statusPath, '130')
5962
},
@@ -587,6 +590,35 @@ describe('agent commands in tmux', () => {
587590
}
588591
})
589592

593+
it('reports an untracked run it could not stop as still running, and keeps tracking it', async () => {
594+
tmuxFake.on = true
595+
tmuxFake.untracked = true
596+
tmuxFake.statusPaths.clear()
597+
try {
598+
const terminal = new TerminalService({ loadCwd: () => '/tmp' })
599+
terminal.start({ cols: 80, rows: 24 })
600+
const running = terminal.executeTool('call-untracked', 'run', {
601+
command: 'sleep 600',
602+
waitSeconds: 60,
603+
})
604+
await vi.waitFor(() => expect(tmuxFake.statusPaths.size).toBe(1))
605+
const [, statusPath = ''] = [...tmuxFake.statusPaths][0] ?? []
606+
607+
await terminal.cancelTool('call-untracked')
608+
609+
await expect(running).resolves.toMatchObject({ ok: true, result: { status: 'running' } })
610+
expect(existsSync(join(statusPath, '..'))).toBe(true)
611+
612+
// Once it does finish, the next run's bookkeeping reaps it.
613+
writeFileSync(statusPath, '0')
614+
await terminal.executeTool('call-next', 'run', { command: 'ls', waitSeconds: 1 })
615+
expect(existsSync(join(statusPath, '..'))).toBe(false)
616+
} finally {
617+
tmuxFake.on = false
618+
tmuxFake.untracked = false
619+
}
620+
})
621+
590622
it("keeps a run's output readable for its call when the terminal goes away mid-wait", async () => {
591623
tmuxFake.on = true
592624
tmuxFake.statusPaths.clear()

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

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,10 @@ interface FakeTmuxState {
104104
log: string[]
105105
/** Commands the fake fails, with the error tmux would print. */
106106
fail?: Record<string, string>
107-
/** Commands the fake answers only after this many milliseconds, like a busy tmux server. */
108-
delay?: Record<string, number>
107+
/** Commands the fake holds until the file named here exists, like a busy tmux server. */
108+
hold?: Record<string, string>
109+
/** Commands the fake is holding right now. */
110+
held?: string[]
109111
}
110112

111113
const FAKE_TMUX = `
@@ -116,8 +118,13 @@ const args = process.argv.slice(2)
116118
const save = () => fs.writeFileSync(file, JSON.stringify(state))
117119
const target = () => args[args.indexOf('-t') + 1]
118120
const fail = (message) => { process.stderr.write(message); process.exit(1) }
119-
if (state.delay && state.delay[args[0]]) {
120-
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, state.delay[args[0]])
121+
if (state.hold && state.hold[args[0]]) {
122+
state.held = [...(state.held ?? []), args[0]]
123+
save()
124+
while (!fs.existsSync(state.hold[args[0]])) {
125+
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 20)
126+
}
127+
state.held = state.held.filter((command) => command !== args[0])
121128
}
122129
if (state.fail && state.fail[args[0]]) fail(state.fail[args[0]])
123130
switch (args[0]) {
@@ -265,6 +272,12 @@ describe('stopping a tmux run touches only its own pane', () => {
265272
// No pane is touched by an id that a restarted server might have handed to the user.
266273
expect(tmux.read().log).toEqual([])
267274
expect(Object.keys(tmux.read().panes)).toEqual([run.pane])
275+
276+
// Once its pane is gone, it can be let go.
277+
const state = tmux.read()
278+
delete state.panes[run.pane]
279+
tmux.write(state)
280+
expect(await runPaneState(run, tmux.env)).toBe('gone')
268281
})
269282

270283
it('lets a tagged run start only once its pane is tagged', async () => {
@@ -311,15 +324,18 @@ describe('stopping a tmux run touches only its own pane', () => {
311324
it('holds a command until tmux has finished tagging its pane', async () => {
312325
const tmux = fakeTmux({ exec: true })
313326
dirs.push(tmux.dir)
314-
tmux.write({ ...tmux.read(), delay: { 'set-option': 2_000 } })
327+
const release = join(tmux.dir, 'release')
328+
tmux.write({ ...tmux.read(), hold: { 'set-option': release } })
315329
const marker = join(tmux.dir, 'ran')
316330

317331
const starting = startRun('agent', `touch ${JSON.stringify(marker)}`, null, tmux.env)
318-
await sleep(1_200)
319-
// The pane exists and its script is running, but the tag is not on it yet.
320-
expect(Object.keys(tmux.read().panes)).toHaveLength(1)
332+
// The pane is open and the tagging call is in flight, held by tmux.
333+
await expect.poll(() => tmux.read().held ?? [], { timeout: 10_000 }).toEqual(['set-option'])
334+
// Time enough for an ungated command to have run.
335+
await sleep(1_000)
321336
expect(existsSync(marker)).toBe(false)
322337

338+
writeFileSync(release, '')
323339
const run = await starting
324340
if ('error' in run) throw new Error(run.error)
325341
await expect.poll(() => existsSync(marker), { timeout: 10_000 }).toBe(true)

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -430,19 +430,19 @@ export async function startRun(
430430
/**
431431
* Whether the run's pane is still the run's: `ours`, or `gone` when tmux has no such pane or the
432432
* pane under that id is not tagged as this run's (the user closed it, or a restarted tmux server
433-
* handed the id to one of the user's own panes). `unknown` when tmux could not be asked, or the
434-
* run is untracked: such a pane is neither touched nor given up on.
433+
* handed the id to one of the user's own panes). `unknown` when tmux could not be asked, or an
434+
* untracked run's pane still exists, since nothing proves whose it is: such a pane is neither
435+
* touched nor given up on.
435436
*/
436437
export async function runPaneState(
437438
handle: TmuxRunHandle,
438439
env: NodeJS.ProcessEnv
439440
): Promise<'ours' | 'gone' | 'unknown'> {
440441
if (!handle.pane) return 'gone'
441-
if (handle.runId === null) return 'unknown'
442-
const shown = await runTmux(
443-
['display-message', '-p', '-t', handle.pane, `#{${RUN_ID_OPTION}}`],
444-
env
445-
)
442+
// An untracked run's pane can still be found missing, with a format every tmux knows.
443+
const format = handle.runId === null ? '#{pane_id}' : `#{${RUN_ID_OPTION}}`
444+
const shown = await runTmux(['display-message', '-p', '-t', handle.pane, format], env)
445+
if (shown.ok && handle.runId === null) return 'unknown'
446446
if (shown.ok) return shown.stdout.trim() === handle.runId ? 'ours' : 'gone'
447447
return /can't find|no server running/i.test(shown.stderr) ? 'gone' : 'unknown'
448448
}

0 commit comments

Comments
 (0)