Skip to content

Commit b0fe9d4

Browse files
committed
fix(desktop): reclaim tmux run temp directories that outlive their wait window
`startRun` makes a temp directory per run holding the tee target and the status file, and only its handle can remove it. `runInTmux` disposed the handle on the `outcome.done` branch only — on the still-running path the handle was a local that went out of scope, and nothing polls that run again (`read` captures the pane instead). With a 30s default wait, every longer command leaked its `/tmp/sim-tmux-run-*` directory for the life of the process while `tee` kept appending the full output to it. Handles for still-running commands are now held per terminal and reclaimed by the terminal's own lifecycle: `retire`/`dispose` release them all, and a new run on the same terminal first reaps any whose status file has since appeared. No reaper timer — the new-run path is already doing run bookkeeping. Live runs are left alone, since their tee is still writing there. Not a security issue: the directories are 0700 in the per-user tmpdir and tmux reaps the window itself. It is unbounded disk and inode growth.
1 parent a99223d commit b0fe9d4

3 files changed

Lines changed: 183 additions & 1 deletion

File tree

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
awaitRun,
4141
capturePane,
4242
closeRunWindow,
43+
isRunComplete,
4344
isTmuxUnavailable,
4445
killPane,
4546
listPanes,
@@ -49,6 +50,7 @@ import {
4950
startRun,
5051
TMUX_KEY_NAMES,
5152
type TmuxAttachment,
53+
type TmuxRunHandle,
5254
} from '@/main/terminal/tmux'
5355

5456
const logger = createLogger('DesktopTerminal')
@@ -176,6 +178,14 @@ export class TerminalService {
176178
private readonly handoffs = new Map<string, boolean>()
177179
/** Recently resolved tmux attachments, by terminal id, to avoid re-spawning. */
178180
private readonly tmuxCache = new Map<string, { at: number; attachment: TmuxAttachment | null }>()
181+
/**
182+
* Run handles for commands that outlived their wait window, keyed by
183+
* terminal. `startRun` makes a temp directory per run and only its handle can
184+
* remove it, so a handle dropped on the still-running path leaks that
185+
* directory for the life of the process while `tee` keeps appending to it.
186+
* Held here so the terminal's own lifecycle can reclaim them.
187+
*/
188+
private readonly pendingRuns = new Map<string, TmuxRunHandle[]>()
179189

180190
constructor(private readonly options: TerminalServiceOptions = {}) {}
181191

@@ -294,6 +304,37 @@ export class TerminalService {
294304
return this.retire(terminalId)
295305
}
296306

307+
/**
308+
* Removes the temp directories of tracked runs that have since finished.
309+
*
310+
* Called when a new run starts on the same terminal, which is the one moment
311+
* the service is already doing run bookkeeping — a dedicated reaper timer
312+
* would be a subsystem to own for something this cheap. A run still going is
313+
* left alone: its `tee` is still appending to that directory.
314+
*/
315+
private reapFinishedRuns(terminalId: string): void {
316+
const pending = this.pendingRuns.get(terminalId)
317+
if (!pending) return
318+
const stillRunning = pending.filter((handle) => {
319+
if (!isRunComplete(handle)) return true
320+
handle.dispose()
321+
return false
322+
})
323+
if (stillRunning.length === 0) this.pendingRuns.delete(terminalId)
324+
else this.pendingRuns.set(terminalId, stillRunning)
325+
}
326+
327+
/**
328+
* Releases every tracked run for a terminal, finished or not. The terminal is
329+
* going away, so nothing will ever read these files again.
330+
*/
331+
private releasePendingRuns(terminalId: string): void {
332+
const pending = this.pendingRuns.get(terminalId)
333+
if (!pending) return
334+
for (const handle of pending) handle.dispose()
335+
this.pendingRuns.delete(terminalId)
336+
}
337+
297338
/**
298339
* Drops a terminal and decides what replaces it. Closing and exiting share
299340
* this so the two cannot drift into different answers for "what happens to
@@ -310,6 +351,7 @@ export class TerminalService {
310351
session.dispose()
311352
this.sessions.delete(terminalId)
312353
this.tmuxCache.delete(terminalId)
354+
this.releasePendingRuns(terminalId)
313355

314356
if (this.sessions.size === 0) {
315357
this.spawn(this.resolveCwd(closedCwd), cols, rows)
@@ -459,6 +501,7 @@ export class TerminalService {
459501
}
460502
this.sessions.clear()
461503
this.tmuxCache.clear()
504+
for (const terminalId of [...this.pendingRuns.keys()]) this.releasePendingRuns(terminalId)
462505
this.activeId = null
463506
// A stale claim here is what let Cmd-W close a shell that no longer exists.
464507
this.setPanelFocused(false)
@@ -784,6 +827,7 @@ export class TerminalService {
784827
if (!command) throw new TerminalError('INVALID_REQUEST', 'run needs a `command`.')
785828

786829
const started = Date.now()
830+
this.reapFinishedRuns(terminal.terminalId)
787831
const handle = await startRun(session, command, terminal.currentCwd, terminal.env)
788832
if ('error' in handle) throw new TerminalError('SPAWN_FAILED', handle.error)
789833

@@ -792,6 +836,12 @@ export class TerminalService {
792836
if (outcome.done) {
793837
await closeRunWindow(handle, terminal.env)
794838
handle.dispose()
839+
} else {
840+
// Still going, and nothing polls the status file again — `read` captures
841+
// the pane instead. Without this the handle would go out of scope here.
842+
const pending = this.pendingRuns.get(terminal.terminalId)
843+
if (pending) pending.push(handle)
844+
else this.pendingRuns.set(terminal.terminalId, [handle])
795845
}
796846

797847
const { text, truncated } = elideOutput(outcome.output)
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const { tmuxStub } = vi.hoisted(() => ({
7+
tmuxStub: {
8+
/** Handles handed out by `startRun`, newest last, with their dispose spies. */
9+
handles: [] as Array<{ window: string; dispose: ReturnType<typeof vi.fn> }>,
10+
/** Whether a run's status file is considered present yet. */
11+
complete: new Set<string>(),
12+
/** What `awaitRun` reports — the leak is on the `done: false` path. */
13+
done: false,
14+
},
15+
}))
16+
17+
vi.mock('@/main/terminal/tmux', () => ({
18+
TMUX_KEY_NAMES: {},
19+
activePane: vi.fn(async () => 'sess:0.0'),
20+
awaitRun: vi.fn(async () => ({
21+
done: tmuxStub.done,
22+
output: 'out',
23+
exitCode: tmuxStub.done ? 0 : null,
24+
})),
25+
capturePane: vi.fn(async () => ({ ok: true, stdout: '', stderr: '' })),
26+
closeRunWindow: vi.fn(async () => undefined),
27+
isRunComplete: vi.fn((handle: { window: string }) => tmuxStub.complete.has(handle.window)),
28+
isTmuxUnavailable: vi.fn(() => false),
29+
killPane: vi.fn(async () => ({ ok: true, stdout: '', stderr: '' })),
30+
listPanes: vi.fn(async () => []),
31+
resolveAttachment: vi.fn(async () => ({ session: 'sess' })),
32+
sendKey: vi.fn(async () => ({ ok: true, stdout: '', stderr: '' })),
33+
sendText: vi.fn(async () => ({ ok: true, stdout: '', stderr: '' })),
34+
startRun: vi.fn(async () => {
35+
const handle = {
36+
window: `sess:${tmuxStub.handles.length}`,
37+
outPath: '/tmp/fake/out',
38+
statusPath: '/tmp/fake/status',
39+
dispose: vi.fn(),
40+
}
41+
tmuxStub.handles.push(handle)
42+
return handle
43+
}),
44+
}))
45+
46+
vi.mock('@/main/terminal/session', () => ({
47+
elide: (text: string) => ({ text, truncated: false }),
48+
TerminalSession: {
49+
create: ({ terminalId, cwd }: { terminalId: string; cwd: string }) => ({
50+
terminalId,
51+
cols: 80,
52+
rows: 24,
53+
pid: 1234,
54+
env: {},
55+
shell: 'zsh',
56+
alive: true,
57+
currentCwd: cwd,
58+
foreground: null,
59+
isBusy: false,
60+
hasShellIntegration: true,
61+
dispose: vi.fn(),
62+
write: vi.fn(),
63+
resize: vi.fn(),
64+
setBusy: vi.fn(),
65+
refreshCwd: async () => {},
66+
takeReplaySnapshot: () => '',
67+
tabState: (active: boolean) => ({
68+
terminalId,
69+
title: 'zsh',
70+
cwd,
71+
running: null,
72+
interactive: false,
73+
active,
74+
}),
75+
}),
76+
},
77+
}))
78+
79+
import { TerminalService } from '@/main/terminal'
80+
81+
/**
82+
* A run whose command outlives the wait window leaves a temp directory behind
83+
* that only its handle can remove, and nothing polls that run again — `read`
84+
* captures the tmux pane instead. These cover who eventually disposes it.
85+
*/
86+
describe('pending tmux runs', () => {
87+
let service: TerminalService
88+
89+
beforeEach(() => {
90+
tmuxStub.handles.length = 0
91+
tmuxStub.complete.clear()
92+
tmuxStub.done = false
93+
service = new TerminalService({ loadCwd: () => '/tmp', saveCwd: () => {} })
94+
})
95+
96+
it('reclaims a still-running run when the terminal is closed', async () => {
97+
await service.executeTool('call-1', 'run', { command: 'sleep 600' })
98+
99+
const [handle] = tmuxStub.handles
100+
expect(handle.dispose).not.toHaveBeenCalled()
101+
102+
service.dispose()
103+
104+
expect(handle.dispose).toHaveBeenCalledTimes(1)
105+
})
106+
107+
it('reaps a finished run when the next run starts, and leaves live ones alone', async () => {
108+
await service.executeTool('call-1', 'run', { command: 'sleep 600' })
109+
const [first] = tmuxStub.handles
110+
111+
await service.executeTool('call-2', 'run', { command: 'sleep 600' })
112+
expect(first.dispose).not.toHaveBeenCalled()
113+
114+
tmuxStub.complete.add(first.window)
115+
await service.executeTool('call-3', 'run', { command: 'sleep 600' })
116+
117+
expect(first.dispose).toHaveBeenCalledTimes(1)
118+
expect(tmuxStub.handles[1].dispose).not.toHaveBeenCalled()
119+
120+
service.dispose()
121+
})
122+
123+
it('disposes a run inline when it finishes inside the wait window', async () => {
124+
tmuxStub.done = true
125+
await service.executeTool('call-1', 'run', { command: 'true' })
126+
127+
expect(tmuxStub.handles[0].dispose).toHaveBeenCalledTimes(1)
128+
129+
service.dispose()
130+
expect(tmuxStub.handles[0].dispose).toHaveBeenCalledTimes(1)
131+
})
132+
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ export function pollRun(handle: TmuxRunHandle): TmuxRunOutcome {
381381
* second, quadratic in output size. Liveness only needs the status file, which
382382
* is a few bytes; the output is read once, when the run is settled.
383383
*/
384-
function isRunComplete(handle: TmuxRunHandle): boolean {
384+
export function isRunComplete(handle: TmuxRunHandle): boolean {
385385
return readIfPresent(handle.statusPath) !== null
386386
}
387387

0 commit comments

Comments
 (0)