Skip to content

Commit 6cf16e3

Browse files
committed
fix(desktop): stop agent tmux runs from the moment they start, and when Terminal is switched off
- A tmux run is tracked as soon as its window exists, not once its wait ends. Sign-out now reaches a command the chat view started that is still inside its wait window. Reaping skips runs whose call is still reading their files. - Switching Terminal off stops the agent's commands before the shells go, as sign-out does. - The runner's deadline is built on the shared interruptible sleep. - A hostname longer than Sim's 128-character limit is shortened, so registration does not fail on it.
1 parent ff70743 commit 6cf16e3

6 files changed

Lines changed: 117 additions & 21 deletions

File tree

‎apps/desktop/src/main/desktop-executor/runner.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
type TerminalToolResponse,
3737
} from '@sim/terminal-protocol'
3838
import { getErrorMessage } from '@sim/utils/errors'
39+
import { interruptibleSleep } from '@sim/utils/helpers'
3940
import { isRecordLike } from '@sim/utils/object'
4041
import type { DesktopToolRunner } from '@/main/desktop-executor/executor'
4142
import type { ClaimedDesktopCall } from '@/main/desktop-executor/protocol'
@@ -101,14 +102,15 @@ async function withDeadline<T>(
101102
onTimeout: () => T
102103
): Promise<T> {
103104
if (timeoutMs === null) return work
104-
let timer: ReturnType<typeof setTimeout> | undefined
105-
const timeout = new Promise<T>((resolve) => {
106-
timer = setTimeout(() => resolve(onTimeout()), timeoutMs)
107-
})
105+
// Aborted once the race settles, which cancels the sleep; a cancelled sleep never times out.
106+
const settled = new AbortController()
107+
const timeout = interruptibleSleep(timeoutMs, settled.signal).then(() =>
108+
settled.signal.aborted ? new Promise<T>(() => {}) : onTimeout()
109+
)
108110
try {
109111
return await Promise.race([work, timeout])
110112
} finally {
111-
clearTimeout(timer)
113+
settled.abort()
112114
}
113115
}
114116

‎apps/desktop/src/main/desktop-executor/service.test.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { describe, expect, it, vi } from 'vitest'
77

88
vi.mock('electron', () => import('@/test/electron-mock'))
99

10-
import { createDesktopExecutorService } from '@/main/desktop-executor/service'
10+
import { createDesktopExecutorService, deviceName } from '@/main/desktop-executor/service'
1111

1212
/** Sim's device routes, with registration answers held until the test releases them. */
1313
function fakeSim(protocolVersion = 1) {
@@ -84,3 +84,10 @@ describe('desktop executor registration', () => {
8484
expect(desktopExecutor.getDevice()).toBeNull()
8585
})
8686
})
87+
88+
describe('device name', () => {
89+
it('fits a long hostname within what Sim accepts at registration', () => {
90+
expect(deviceName(`${'studio-'.repeat(40)}.local`).length).toBeLessThanOrEqual(128)
91+
expect(deviceName('Studio-Mac.local')).toBe('Studio-Mac')
92+
})
93+
})

‎apps/desktop/src/main/desktop-executor/service.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { generateId } from '@sim/utils/id'
1313
import { isRecordLike } from '@sim/utils/object'
1414
import { randomFloat } from '@sim/utils/random'
1515
import { backoffWithJitter } from '@sim/utils/retry'
16+
import { truncate } from '@sim/utils/string'
1617
import type { Session } from 'electron'
1718
import { app, net, powerMonitor } from 'electron'
1819
import { readFileWithinLimit, writeJsonFileAtomically } from '@/main/atomic-json-file'
@@ -75,12 +76,13 @@ async function readInstallId(filePath: string): Promise<string | null> {
7576
}
7677
}
7778

78-
function deviceName(): string {
79-
return (
80-
hostname()
81-
.replace(/\.local$/, '')
82-
.trim() || 'Sim desktop'
83-
)
79+
/** Sim refuses a longer device name at registration. */
80+
const DEVICE_NAME_MAX_CHARS = 128
81+
82+
/** The machine's name as the user knows it, within what Sim accepts. */
83+
export function deviceName(host = hostname()): string {
84+
const name = host.replace(/\.local$/, '').trim() || 'Sim desktop'
85+
return truncate(name, DEVICE_NAME_MAX_CHARS - 3)
8486
}
8587

8688
export function createDesktopExecutorService(

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,8 @@ function main(): void {
537537
desktopExecutor.refreshRegistration()
538538
},
539539
setTerminalEnabled: (enabled) => {
540-
if (!enabled) terminal.dispose()
540+
// Agent commands are stopped by their process groups first; a tmux run outlives its shell.
541+
if (!enabled) void terminal.stopAgentCommands().then(() => terminal.dispose())
541542
desktopExecutor.refreshRegistration()
542543
},
543544
setBrowserTheme: setAgentBrowserTheme,

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

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,8 @@ export class TerminalService {
246246
* Held here so the terminal's own lifecycle can reclaim them.
247247
*/
248248
private readonly pendingRuns = new Map<string, TmuxRunHandle[]>()
249+
/** Runs a `run` call is still waiting on; their files are read when the wait ends. */
250+
private readonly awaitedRuns = new Set<TmuxRunHandle>()
249251
/** How to stop each tool call still in flight, so Stop interrupts exactly what it started. */
250252
private readonly toolStops = new Map<string, () => Promise<void>>()
251253

@@ -479,13 +481,19 @@ export class TerminalService {
479481
if (!pending) return
480482
const stillRunning: TmuxRunHandle[] = []
481483
for (const handle of pending) {
482-
if (isRunComplete(handle)) handle.dispose()
484+
if (isRunComplete(handle) && !this.awaitedRuns.has(handle)) handle.dispose()
483485
else stillRunning.push(handle)
484486
}
485487
if (stillRunning.length === 0) this.pendingRuns.delete(terminalId)
486488
else this.pendingRuns.set(terminalId, stillRunning)
487489
}
488490

491+
private untrackRun(terminalId: string, handle: TmuxRunHandle): void {
492+
const remaining = (this.pendingRuns.get(terminalId) ?? []).filter((entry) => entry !== handle)
493+
if (remaining.length === 0) this.pendingRuns.delete(terminalId)
494+
else this.pendingRuns.set(terminalId, remaining)
495+
}
496+
489497
/**
490498
* Releases every tracked run for a terminal, finished or not. The terminal is
491499
* going away, so nothing will ever read these files again.
@@ -1251,6 +1259,11 @@ export class TerminalService {
12511259
this.reapFinishedRuns(terminal.terminalId)
12521260
const handle = await startRun(session, command, terminal.currentCwd, terminal.env)
12531261
if ('error' in handle) throw new TerminalError('SPAWN_FAILED', handle.error)
1262+
// Tracked from the moment its window exists, so sign-out can stop it even mid-wait.
1263+
const pending = this.pendingRuns.get(terminal.terminalId)
1264+
if (pending) pending.push(handle)
1265+
else this.pendingRuns.set(terminal.terminalId, [handle])
1266+
this.awaitedRuns.add(handle)
12541267

12551268
const waitMs = resolveRunWaitMs(args.waitSeconds)
12561269
// Inside tmux a stop arrives as Ctrl-C in the run's own window; closing that window hangs up
@@ -1269,17 +1282,14 @@ export class TerminalService {
12691282
const outcome = await Promise.race([
12701283
awaitRun(handle, waitMs),
12711284
stopped.then(() => ({ ...pollRun(handle), done: true })),
1272-
])
1285+
]).finally(() => this.awaitedRuns.delete(handle))
12731286
if (outcome.done) {
12741287
await closeRunWindow(handle, terminal.env)
1288+
this.untrackRun(terminal.terminalId, handle)
12751289
handle.dispose()
1276-
} else {
1277-
// Still going, and nothing polls the status file again — `read` captures
1278-
// the pane instead.
1279-
const pending = this.pendingRuns.get(terminal.terminalId)
1280-
if (pending) pending.push(handle)
1281-
else this.pendingRuns.set(terminal.terminalId, [handle])
12821290
}
1291+
// Still going, it stays tracked, and nothing polls the status file again: `read` captures
1292+
// the pane instead.
12831293

12841294
const { text, truncated } = elideOutput(outcome.output)
12851295
return {

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,55 @@
1+
import { mkdtempSync, writeFileSync } from 'node:fs'
2+
import { tmpdir } from 'node:os'
3+
import { join } from 'node:path'
14
import { describe, expect, it, vi } from 'vitest'
25
import { TerminalService } from '@/main/terminal'
36

7+
/**
8+
* A tmux attachment the service sees only when a test turns it on: run windows get real status
9+
* files, and Ctrl-C in a run's window ends its command the way tmux would.
10+
*/
11+
const tmuxFake = vi.hoisted(() => ({
12+
on: false,
13+
keys: [] as Array<{ target: string; key: string }>,
14+
closed: [] as string[],
15+
statusPaths: new Map<string, string>(),
16+
}))
17+
18+
vi.mock('@/main/terminal/tmux', async () => {
19+
const actual =
20+
await vi.importActual<typeof import('@/main/terminal/tmux')>('@/main/terminal/tmux')
21+
let nextWindow = 1
22+
return {
23+
...actual,
24+
isTmuxUnavailable: () => (tmuxFake.on ? false : actual.isTmuxUnavailable()),
25+
resolveAttachment: async (pid: number, env: NodeJS.ProcessEnv) =>
26+
tmuxFake.on
27+
? { session: 'agent', clientTty: '/dev/ttys001' }
28+
: actual.resolveAttachment(pid, env),
29+
startRun: async (...args: Parameters<typeof actual.startRun>) => {
30+
if (!tmuxFake.on) return actual.startRun(...args)
31+
const dir = mkdtempSync(join(tmpdir(), 'sim-tmux-fake-'))
32+
const window = `@${nextWindow++}`
33+
const statusPath = join(dir, 'status')
34+
writeFileSync(join(dir, 'out'), '')
35+
tmuxFake.statusPaths.set(window, statusPath)
36+
return { window, outPath: join(dir, 'out'), statusPath, dispose: () => {} }
37+
},
38+
sendKey: async (target: string, key: string, env: NodeJS.ProcessEnv) => {
39+
if (!tmuxFake.on) return actual.sendKey(target, key, env)
40+
tmuxFake.keys.push({ target, key })
41+
const statusPath = tmuxFake.statusPaths.get(target)
42+
if (key === 'C-c' && statusPath) writeFileSync(statusPath, '130')
43+
return { ok: true, stdout: '', stderr: '' }
44+
},
45+
closeRunWindow: async (...args: Parameters<typeof actual.closeRunWindow>) => {
46+
const [handle] = args
47+
if (!tmuxFake.on) return actual.closeRunWindow(...args)
48+
tmuxFake.closed.push(handle.window)
49+
},
50+
}
51+
})
52+
453
/** Stub sessions by terminal id, populated by the mock below. */
554
const { stubSessions } = vi.hoisted(() => ({
655
stubSessions: new Map<
@@ -500,3 +549,28 @@ describe('stopping a tool call', () => {
500549
await running
501550
})
502551
})
552+
553+
describe('agent commands in tmux', () => {
554+
it('stops a tmux run at sign-out while its call still waits on it', async () => {
555+
tmuxFake.on = true
556+
try {
557+
const terminal = new TerminalService({ loadCwd: () => '/tmp' })
558+
terminal.start({ cols: 80, rows: 24 })
559+
const running = terminal.executeTool('call-tmux', 'run', {
560+
command: 'sleep 600',
561+
waitSeconds: 60,
562+
})
563+
await vi.waitFor(() => expect(tmuxFake.statusPaths.size).toBe(1))
564+
565+
await terminal.stopAgentCommands()
566+
567+
expect(tmuxFake.keys).toEqual([{ target: '@1', key: 'C-c' }])
568+
await expect(running).resolves.toMatchObject({
569+
ok: true,
570+
result: { status: 'completed', exitCode: 130 },
571+
})
572+
} finally {
573+
tmuxFake.on = false
574+
}
575+
})
576+
})

0 commit comments

Comments
 (0)