Skip to content

Commit 67b544b

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 8cc81f4 commit 67b544b

4 files changed

Lines changed: 73 additions & 15 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: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ interface FakeTmuxState {
110110

111111
/** Commands the fake answers only after this many milliseconds, like a busy tmux server. */
112112
delay?: Record<string, number>
113+
114+
/** Commands the fake holds until the file named here exists, like a busy tmux server. */
115+
hold?: Record<string, string>
116+
/** Commands the fake is holding right now. */
117+
held?: string[]
113118
}
114119

115120
const FAKE_TMUX = `
@@ -129,6 +134,14 @@ const escaped = (text) =>
129134
130135
if (state.delay && state.delay[args[0]]) {
131136
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, state.delay[args[0]])
137+
138+
if (state.hold && state.hold[args[0]]) {
139+
state.held = [...(state.held ?? []), args[0]]
140+
save()
141+
while (!fs.existsSync(state.hold[args[0]])) {
142+
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 20)
143+
}
144+
state.held = state.held.filter((command) => command !== args[0])
132145
}
133146
if (state.fail && state.fail[args[0]]) fail(state.fail[args[0]])
134147
switch (args[0]) {
@@ -310,6 +323,12 @@ describe('stopping a tmux run touches only its own pane', () => {
310323
// No pane is touched by an id that a restarted server might have handed to the user.
311324
expect(tmux.read().log).toEqual([])
312325
expect(Object.keys(tmux.read().panes)).toEqual([run.pane])
326+
327+
// Once its pane is gone, it can be let go.
328+
const state = tmux.read()
329+
delete state.panes[run.pane]
330+
tmux.write(state)
331+
expect(await runPaneState(run, tmux.env)).toBe('gone')
313332
})
314333

315334
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', () => {
356375
it('holds a command until tmux has finished tagging its pane', async () => {
357376
const tmux = fakeTmux({ exec: true })
358377
dirs.push(tmux.dir)
359-
tmux.write({ ...tmux.read(), delay: { 'set-option': 2_000 } })
378+
const release = join(tmux.dir, 'release')
379+
tmux.write({ ...tmux.read(), hold: { 'set-option': release } })
360380
const marker = join(tmux.dir, 'ran')
361381

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

389+
writeFileSync(release, '')
368390
const run = await starting
369391
if ('error' in run) throw new Error(run.error)
370392
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
@@ -435,19 +435,19 @@ export async function startRun(
435435
/**
436436
* Whether the run's pane is still the run's: `ours`, or `gone` when tmux has no such pane or the
437437
* pane under that id is not tagged as this run's (the user closed it, or a restarted tmux server
438-
* handed the id to one of the user's own panes). `unknown` when tmux could not be asked, or the
439-
* run is untracked: such a pane is neither touched nor given up on.
438+
* handed the id to one of the user's own panes). `unknown` when tmux could not be asked, or an
439+
* untracked run's pane still exists, since nothing proves whose it is: such a pane is neither
440+
* touched nor given up on.
440441
*/
441442
export async function runPaneState(
442443
handle: TmuxRunHandle,
443444
env: NodeJS.ProcessEnv
444445
): Promise<'ours' | 'gone' | 'unknown'> {
445446
if (!handle.pane) return 'gone'
446-
if (handle.runId === null) return 'unknown'
447-
const shown = await runTmux(
448-
['display-message', '-p', '-t', handle.pane, `#{${RUN_ID_OPTION}}`],
449-
env
450-
)
447+
// An untracked run's pane can still be found missing, with a format every tmux knows.
448+
const format = handle.runId === null ? '#{pane_id}' : `#{${RUN_ID_OPTION}}`
449+
const shown = await runTmux(['display-message', '-p', '-t', handle.pane, format], env)
450+
if (shown.ok && handle.runId === null) return 'unknown'
451451
if (shown.ok) return shown.stdout.trim() === handle.runId ? 'ours' : 'gone'
452452
return /can't find|no server running/i.test(shown.stderr) ? 'gone' : 'unknown'
453453
}

0 commit comments

Comments
 (0)