Skip to content

Commit e48dede

Browse files
committed
fix(desktop): stop the agent's tmux runs a previous process left going
A tmux run outlives the app, but only the process that started it knew about it: after a crash, a quit, or a crash mid-sign-out, the next launch had nothing to stop, and a chat put away took its runs out of reach of sign-out too. - Each tagged run is recorded in userData (outside account data): its tag, its pane and its tmux server's socket, nothing else. The record goes once the run has ended, its pane is gone, or Sim has stopped it. - Sign-out, switching Terminal off and the launch-time account recovery stop every recorded run; every launch stops the runs a previous process left going. A pane is only acted on, on the run's own server, while it still carries the run's tag. - Restart semantics follow the executor's journal: a call claimed before the crash is settled as outcome unknown and nothing reattaches to its command, so a run still going belongs to a call no one can collect; a run that finished is only cleaned up.
1 parent 8dddbd7 commit e48dede

9 files changed

Lines changed: 480 additions & 15 deletions

File tree

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ import {
101101
import { setShellTheme } from '@/main/shell-theme'
102102
import { attachTelemetryPolicy } from '@/main/telemetry-policy'
103103
import { TerminalRegistry } from '@/main/terminal/registry'
104+
import { createRunLedger } from '@/main/terminal/run-ledger'
104105
import { installTray, type TrayHandle } from '@/main/tray'
105106
import { checkForUpdatesInteractive, initUpdater, type UpdaterHandle } from '@/main/updater'
106107
import { installBrowserUserAgent } from '@/main/user-agent'
@@ -167,15 +168,20 @@ function main(): void {
167168
),
168169
})
169170
const scopeEvents = new ScopedEventRouter()
170-
const terminal = new TerminalRegistry({
171-
load: (scopeId) => desktopChatSessions.getTerminal(processOrigin, scopeId) ?? undefined,
172-
save: (scopeId, snapshot) => desktopChatSessions.setTerminal(processOrigin, scopeId, snapshot),
173-
migrate: (fromScopeId, toScopeId) =>
174-
desktopChatSessions.migrateTerminal(processOrigin, fromScopeId, toScopeId),
175-
disposeScope: (scopeId) => {
176-
desktopChatSessions.deleteScope(processOrigin, scopeId)
171+
const terminal = new TerminalRegistry(
172+
{
173+
load: (scopeId) => desktopChatSessions.getTerminal(processOrigin, scopeId) ?? undefined,
174+
save: (scopeId, snapshot) =>
175+
desktopChatSessions.setTerminal(processOrigin, scopeId, snapshot),
176+
migrate: (fromScopeId, toScopeId) =>
177+
desktopChatSessions.migrateTerminal(processOrigin, fromScopeId, toScopeId),
178+
disposeScope: (scopeId) => {
179+
desktopChatSessions.deleteScope(processOrigin, scopeId)
180+
},
177181
},
178-
})
182+
undefined,
183+
createRunLedger(join(userDataPath, 'terminal-runs'))
184+
)
179185
const preloadPath = join(__dirname, 'preload.cjs')
180186

181187
const windows = new Set<BrowserWindow>()
@@ -804,6 +810,11 @@ function main(): void {
804810
}
805811
}
806812

813+
// A tmux run the previous process left going belongs to a call it can no longer report (its
814+
// journal settles it as outcome unknown) or to a chat view that is gone: nothing will collect
815+
// what it does, so it is stopped, while its pane still carries its tag.
816+
void terminal.stopRecordedRuns({ excludeLive: true })
817+
807818
if (!accountDataAvailable()) {
808819
logger.warn(
809820
'Account-bearing browser, terminal, and local filesystem APIs are unavailable until local recovery succeeds'

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
resourceTabTargetIndex,
3939
} from '@/main/resource-shortcuts'
4040
import { readForegroundProcessGroup, signalProcessGroup } from '@/main/terminal/process-group'
41+
import type { RunLedger } from '@/main/terminal/run-ledger'
4142
import { elide, TerminalSession } from '@/main/terminal/session'
4243
import {
4344
activePane,
@@ -156,6 +157,8 @@ export interface TerminalServiceOptions {
156157
canSpawn?(): boolean
157158
/** Reads and signals a terminal's foreground process group; the OS's by default. */
158159
processGroups?: TerminalProcessGroups
160+
/** Where tagged tmux runs are recorded, so a later process can still stop them. */
161+
runLedger?: RunLedger
159162
}
160163

161164
interface TerminalProcessGroups {
@@ -490,7 +493,10 @@ export class TerminalService {
490493
private async reapFinishedRuns(terminalId: string, env: NodeJS.ProcessEnv): Promise<void> {
491494
// A closed tab's run whose pane has since gone (its command ended) needs no stopping.
492495
for (const [handle, orphanEnv] of this.orphanedRuns) {
493-
if ((await runPaneState(handle, orphanEnv)) === 'gone') this.orphanedRuns.delete(handle)
496+
if ((await runPaneState(handle, orphanEnv)) === 'gone') {
497+
this.orphanedRuns.delete(handle)
498+
this.forgetRun(handle)
499+
}
494500
}
495501
for (const handle of this.pendingRuns.get(terminalId) ?? []) {
496502
if (this.awaitedRuns.has(handle)) continue
@@ -499,11 +505,17 @@ export class TerminalService {
499505
// A pane kept open after its command ended (`remain-on-exit`) closes with its run.
500506
if (complete) await closeRunPane(handle, env)
501507
this.untrackRun(terminalId, handle)
508+
this.forgetRun(handle)
502509
handle.dispose()
503510
}
504511
}
505512
}
506513

514+
/** Drops a run's record once nothing of it is left for any process to stop. */
515+
private forgetRun(handle: TmuxRunHandle): void {
516+
if (handle.runId) this.options.runLedger?.forget(handle.runId)
517+
}
518+
507519
/** Removes a run's files now, or once the call still reading them is done with them. */
508520
private releaseRun(handle: TmuxRunHandle): void {
509521
if (this.awaitedRuns.has(handle)) this.releasedAwaitedRuns.add(handle)
@@ -1282,6 +1294,13 @@ export class TerminalService {
12821294
await this.reapFinishedRuns(terminal.terminalId, terminal.env)
12831295
const handle = await startRun(session, command, terminal.currentCwd, terminal.env)
12841296
if ('error' in handle) throw new TerminalError('SPAWN_FAILED', handle.error)
1297+
if (handle.runId && handle.socket) {
1298+
this.options.runLedger?.record({
1299+
runId: handle.runId,
1300+
pane: handle.pane,
1301+
socket: handle.socket,
1302+
})
1303+
}
12851304
// Tracked from the moment its window exists, so sign-out can stop it even mid-wait.
12861305
const pending = this.pendingRuns.get(terminal.terminalId)
12871306
if (pending) pending.push(handle)
@@ -1316,6 +1335,7 @@ export class TerminalService {
13161335
if (outcome.done) {
13171336
await closeRunPane(handle, terminal.env)
13181337
this.untrackRun(terminal.terminalId, handle)
1338+
this.forgetRun(handle)
13191339
handle.dispose()
13201340
}
13211341
// Still going, it stays tracked, and nothing polls the status file again: `read` captures
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { mkdtempSync, rmSync } from 'node:fs'
2+
import { tmpdir } from 'node:os'
3+
import { join } from 'node:path'
4+
import { afterEach, describe, expect, it, vi } from 'vitest'
5+
6+
vi.mock('electron', () => import('@/test/electron-mock'))
7+
8+
/** How each recorded run's pane answers when stopped: still tracked by tmux or not. */
9+
const { panes } = vi.hoisted(() => ({
10+
panes: new Map<string, 'gone' | 'unknown'>(),
11+
}))
12+
13+
vi.mock('@/main/terminal/tmux', async () => {
14+
const actual =
15+
await vi.importActual<typeof import('@/main/terminal/tmux')>('@/main/terminal/tmux')
16+
return {
17+
...actual,
18+
stopRecordedRun: async (run: { runId: string }) => panes.get(run.runId) ?? 'gone',
19+
}
20+
})
21+
22+
import { TerminalRegistry } from '@/main/terminal/registry'
23+
import { createRunLedger } from '@/main/terminal/run-ledger'
24+
25+
const dirs: string[] = []
26+
27+
function ledgerDir(): string {
28+
const dir = mkdtempSync(join(tmpdir(), 'sim-registry-ledger-'))
29+
dirs.push(dir)
30+
return join(dir, 'terminal-runs')
31+
}
32+
33+
function run(runId: string, pane: string) {
34+
return { runId, pane, socket: '/tmp/tmux-501/default' }
35+
}
36+
37+
afterEach(() => {
38+
panes.clear()
39+
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true })
40+
})
41+
42+
describe('stopping recorded tmux runs', () => {
43+
it('forgets each run once nothing of it is left, and keeps one tmux could not answer for', async () => {
44+
const dir = ledgerDir()
45+
const previous = createRunLedger(dir)
46+
previous.record(run('stopped', '%1'))
47+
previous.record(run('unanswered', '%2'))
48+
panes.set('unanswered', 'unknown')
49+
const ledger = createRunLedger(dir)
50+
51+
await new TerminalRegistry(undefined, undefined, ledger).stopAgentCommands()
52+
53+
expect(ledger.list().map((record) => record.runId)).toEqual(['unanswered'])
54+
})
55+
56+
it("at launch, stops the previous process's runs and none this one has started", async () => {
57+
const dir = ledgerDir()
58+
createRunLedger(dir).record(run('previous', '%1'))
59+
const ledger = createRunLedger(dir)
60+
ledger.record(run('current', '%2'))
61+
62+
await new TerminalRegistry(undefined, undefined, ledger).stopRecordedRuns({ excludeLive: true })
63+
64+
expect(ledger.list().map((record) => record.runId)).toEqual(['current'])
65+
})
66+
})

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

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ import {
1919
type TerminalServiceOptions,
2020
type TerminalSink,
2121
} from '@/main/terminal'
22+
import type { RunLedger } from '@/main/terminal/run-ledger'
23+
import { stopRecordedRun } from '@/main/terminal/tmux'
24+
25+
/** How long a recorded run gets to end on Ctrl-C before its pane is closed. */
26+
const RECORDED_RUN_GRACE_MS = 2_000
2227

2328
/** Native PTYs and their headless xterm buffers are process-wide resources. */
2429
export const MAX_TERMINALS_PER_PROCESS = 48
@@ -106,7 +111,8 @@ export class TerminalRegistry {
106111

107112
constructor(
108113
private readonly persistence?: TerminalScopePersistence,
109-
private readonly serviceFactory: TerminalServiceFactory = createTerminalService
114+
private readonly serviceFactory: TerminalServiceFactory = createTerminalService,
115+
private readonly runLedger?: RunLedger
110116
) {}
111117

112118
setSink(sink: ScopedTerminalSink | null): void {
@@ -369,11 +375,33 @@ export class TerminalRegistry {
369375
return true
370376
}
371377

372-
/** Stops every command the agent started in any chat's terminals; the user's own are untouched. */
378+
/**
379+
* Stops every command the agent started in any chat's terminals; the user's own are untouched.
380+
* That includes tmux runs no live terminal holds any more: a chat put away, or a previous
381+
* process that quit or crashed while they ran.
382+
*/
373383
async stopAgentCommands(): Promise<void> {
374384
await Promise.allSettled(
375385
[...this.entries.values()].map((entry) => entry.service.stopAgentCommands())
376386
)
387+
await this.stopRecordedRuns()
388+
}
389+
390+
/**
391+
* Stops the recorded tmux runs, each only while its pane still carries its tag, and drops the
392+
* records with nothing left to stop. At launch it skips the runs this process has started since:
393+
* every other run belongs to a call the previous process can no longer report, which its
394+
* journal settles as outcome unknown, so nothing is left to collect what it does.
395+
*/
396+
async stopRecordedRuns(options: { excludeLive?: boolean } = {}): Promise<void> {
397+
const ledger = this.runLedger
398+
if (!ledger) return
399+
await Promise.allSettled(
400+
ledger.list(options).map(async (run) => {
401+
const state = await stopRecordedRun(run, process.env, RECORDED_RUN_GRACE_MS)
402+
if (state === 'gone') ledger.forget(run.runId)
403+
})
404+
)
377405
}
378406

379407
/** Tears down every shell owned by every chat scope. */
@@ -405,6 +433,7 @@ export class TerminalRegistry {
405433
service: this.serviceFactory(scope, {
406434
loadCwd: () => this.entries.get(scope)?.persisted?.tabs[0]?.cwd,
407435
canSpawn: () => this.liveTerminalCount() < MAX_TERMINALS_PER_PROCESS,
436+
runLedger: this.runLedger,
408437
}),
409438
persisted,
410439
restoreApplied: false,
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs'
2+
import { tmpdir } from 'node:os'
3+
import { join } from 'node:path'
4+
import { afterEach, describe, expect, it } from 'vitest'
5+
import { createRunLedger } from '@/main/terminal/run-ledger'
6+
7+
const dirs: string[] = []
8+
9+
function scratch(): string {
10+
const dir = mkdtempSync(join(tmpdir(), 'sim-run-ledger-'))
11+
dirs.push(dir)
12+
return join(dir, 'terminal-runs')
13+
}
14+
15+
const RUN = { runId: 'run-1', pane: '%3', socket: '/tmp/tmux-501/default' }
16+
17+
afterEach(() => {
18+
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true })
19+
})
20+
21+
describe('the tmux run ledger', () => {
22+
it('keeps a run recorded until it is forgotten, for the next process too', () => {
23+
const dir = scratch()
24+
createRunLedger(dir).record(RUN)
25+
26+
const nextProcess = createRunLedger(dir)
27+
expect(nextProcess.list()).toEqual([RUN])
28+
29+
nextProcess.forget(RUN.runId)
30+
expect(createRunLedger(dir).list()).toEqual([])
31+
})
32+
33+
it("leaves out this process's own runs when asked", () => {
34+
const dir = scratch()
35+
createRunLedger(dir).record(RUN)
36+
const ledger = createRunLedger(dir)
37+
ledger.record({ ...RUN, runId: 'run-2', pane: '%4' })
38+
39+
expect(ledger.list({ excludeLive: true })).toEqual([RUN])
40+
expect(ledger.list()).toHaveLength(2)
41+
})
42+
43+
it('drops files nothing could act on safely', () => {
44+
const dir = scratch()
45+
const ledger = createRunLedger(dir)
46+
ledger.record(RUN)
47+
writeFileSync(join(dir, 'garbled.json'), '{not json')
48+
writeFileSync(join(dir, 'run-9.json'), JSON.stringify({ ...RUN, socket: 'relative.sock' }))
49+
// A record under another run's name could stop the wrong run.
50+
writeFileSync(join(dir, 'run-8.json'), JSON.stringify({ ...RUN, runId: 'run-7' }))
51+
52+
expect(ledger.list()).toEqual([RUN])
53+
expect(readdirSync(dir)).toEqual(['run-1.json'])
54+
})
55+
56+
it('records nothing for a run tag that is not a plain id', () => {
57+
const dir = scratch()
58+
const ledger = createRunLedger(dir)
59+
ledger.record({ ...RUN, runId: '../escape' })
60+
61+
expect(ledger.list()).toEqual([])
62+
})
63+
})

0 commit comments

Comments
 (0)