Skip to content

Commit 8cc81f4

Browse files
committed
fix(desktop): run an agent command on tmux that cannot tag its pane, untracked
Agent `run` inside tmux tagged its pane with a pane option (`set-option -p`, tmux 3.0+) and refused the command when that failed, so on older tmux every agent run failed, with the background executor off too. A run whose pane cannot be tagged now goes ahead untracked: Stop, sign-out and switching Terminal off leave it alone, since nothing could tell its pane from one of the user's. Tagged runs are unchanged. The gate still holds every command until the tagging call has finished.
1 parent dfe8ecf commit 8cc81f4

4 files changed

Lines changed: 64 additions & 29 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,8 @@ export class TerminalService {
521521
const pending = this.pendingRuns.get(terminalId)
522522
if (!pending) return
523523
for (const handle of pending) {
524-
if (env && !isRunComplete(handle)) this.orphanedRuns.set(handle, env)
524+
// An untracked run is never stopped, so there is nothing to keep it for.
525+
if (env && handle.runId !== null && !isRunComplete(handle)) this.orphanedRuns.set(handle, env)
525526
this.releaseRun(handle)
526527
}
527528
this.pendingRuns.delete(terminalId)

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

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ interface FakeTmuxState {
107107
fail?: Record<string, string>
108108
/** Attached clients, as `list-clients` reports them. */
109109
clients?: Array<{ pid: string; tty: string; session: string }>
110+
111+
/** Commands the fake answers only after this many milliseconds, like a busy tmux server. */
112+
delay?: Record<string, number>
110113
}
111114

112115
const FAKE_TMUX = `
@@ -123,6 +126,10 @@ const escaped = (text) =>
123126
text
124127
.replace(/\\\\/g, '\\\\\\\\')
125128
.replace(/[\\x00-\\x1f]/g, (c) => '\\\\' + c.charCodeAt(0).toString(8).padStart(3, '0'))
129+
130+
if (state.delay && state.delay[args[0]]) {
131+
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, state.delay[args[0]])
132+
}
126133
if (state.fail && state.fail[args[0]]) fail(state.fail[args[0]])
127134
switch (args[0]) {
128135
case 'new-window': {
@@ -287,16 +294,22 @@ describe('stopping a tmux run touches only its own pane', () => {
287294
expect(tmux.read().log).toEqual([])
288295
})
289296

290-
it('never lets a run it could not tag start, and closes no pane by id to stop it', async () => {
297+
it('starts a run it could not tag, untracked, and never stops it by a pane id', async () => {
291298
const tmux = fakeTmux()
292299
dirs.push(tmux.dir)
300+
// tmux before 3.0 has no pane options.
293301
tmux.write({ ...tmux.read(), fail: { 'set-option': 'invalid option: @sim-run-id' } })
294302

295-
const result = await startRun('agent', 'sleep 600', null, tmux.env)
303+
const run = await startRun('agent', 'sleep 600', null, tmux.env)
304+
if ('error' in run) throw new Error(run.error)
296305

297-
expect(result).toMatchObject({ error: expect.stringContaining('was not run') })
298-
// No pane is closed by an id that a restarted server might have handed to the user.
306+
expect(run.runId).toBeNull()
307+
expect(existsSync(join(run.statusPath, '..', 'go'))).toBe(true)
308+
expect(await runPaneState(run, tmux.env)).toBe('unknown')
309+
await stopRun(run, tmux.env, 0)
310+
// No pane is touched by an id that a restarted server might have handed to the user.
299311
expect(tmux.read().log).toEqual([])
312+
expect(Object.keys(tmux.read().panes)).toEqual([run.pane])
300313
})
301314

302315
it('lets a tagged run start only once its pane is tagged', async () => {
@@ -318,7 +331,7 @@ describe('stopping a tmux run touches only its own pane', () => {
318331
expect(tmux.read().log).toEqual([])
319332
})
320333

321-
it('runs a tagged command for real, and never runs one it could not tag', async () => {
334+
it('runs a command for real whether or not tmux could tag its pane', async () => {
322335
const tagged = fakeTmux({ exec: true })
323336
dirs.push(tagged.dir)
324337
const run = await startRun('agent', 'echo ran', null, tagged.env)
@@ -333,10 +346,27 @@ describe('stopping a tmux run touches only its own pane', () => {
333346
const untagged = fakeTmux({ exec: true })
334347
dirs.push(untagged.dir)
335348
untagged.write({ ...untagged.read(), fail: { 'set-option': 'invalid option' } })
336-
const marker = join(untagged.dir, 'ran')
337-
await startRun('agent', `touch ${JSON.stringify(marker)}`, null, untagged.env)
338-
await sleep(6_000)
349+
const untracked = await startRun('agent', 'echo ran', null, untagged.env)
350+
if ('error' in untracked) throw new Error(untracked.error)
351+
await expect
352+
.poll(() => pollRun(untracked), { timeout: 10_000 })
353+
.toMatchObject({ done: true, exitCode: 0, output: 'ran\n' })
354+
}, 20_000)
339355

356+
it('holds a command until tmux has finished tagging its pane', async () => {
357+
const tmux = fakeTmux({ exec: true })
358+
dirs.push(tmux.dir)
359+
tmux.write({ ...tmux.read(), delay: { 'set-option': 2_000 } })
360+
const marker = join(tmux.dir, 'ran')
361+
362+
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)
340366
expect(existsSync(marker)).toBe(false)
367+
368+
const run = await starting
369+
if ('error' in run) throw new Error(run.error)
370+
await expect.poll(() => existsSync(marker), { timeout: 10_000 }).toBe(true)
341371
}, 20_000)
342372
})

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

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,11 @@ export interface TmuxRunHandle {
311311
pane: string
312312
/**
313313
* Tagged on the pane as the `@sim-run-id` user option. Window and pane ids restart from zero
314-
* with the tmux server, so only the tag proves a pane is still this run's.
314+
* with the tmux server, so only the tag proves a pane is still this run's. Null when tmux could
315+
* not tag the pane (tmux before 3.0 has no pane options): the run goes ahead untracked, and
316+
* nothing ever stops it, since nothing could tell its pane from one of the user's.
315317
*/
316-
runId: string
318+
runId: string | null
317319
outPath: string
318320
statusPath: string
319321
dispose(): void
@@ -368,10 +370,10 @@ export async function startRun(
368370
// into the pipeline and print `No such file or directory` into the user's own
369371
// tmux window, minutes after they closed the tab.
370372
//
371-
// The command waits for its pane to be tagged as this run's (the go file), so nothing runs that
372-
// a later stop could not recognize. Untagged, the script gives up once the tagging call has
373-
// surely failed, and its pane closes on its own; no one has to close a pane whose id might no
374-
// longer be its own.
373+
// The command waits for the tagging call to finish (the go file), so a tagged run's command
374+
// never runs before a later stop could recognize its pane. If Sim never releases it (it quit
375+
// mid-start), the script gives up once the tagging call has surely ended and its pane closes on
376+
// its own; no one has to close a pane whose id might no longer be its own.
375377
//
376378
// The script is a file rather than a `bash -c` string: tmux hands its command to `sh -c`, which
377379
// would expand `$` references meant for bash (the gate's counter, PIPESTATUS) before bash ran.
@@ -410,18 +412,16 @@ export async function startRun(
410412
return { error: created.stderr.trim() || 'tmux could not open a window for the command.' }
411413
}
412414
const [window = '', pane = ''] = created.stdout.trim().split(' ')
413-
const runId = generateId()
415+
const tag = generateId()
414416
// An untagged pane is never treated as the run's: without the tag a stop could not tell it from
415-
// a pane the user opened later under the same id, so it sends nothing at all.
416-
const tagged = await runTmux(['set-option', '-p', '-t', pane, RUN_ID_OPTION, runId], env)
417+
// a pane the user opened later under the same id, so the run is left untracked.
418+
const tagged = await runTmux(['set-option', '-p', '-t', pane, RUN_ID_OPTION, tag], env)
417419
if (!tagged.ok) {
418-
// Untagged, nothing could stop it safely later, so it never starts: without the go file the
419-
// wrapper exits by itself.
420-
dispose()
421-
return {
422-
error: `tmux could not mark the command's pane (${tagged.stderr.trim() || 'no detail'}), so the command was not run.`,
423-
}
420+
logger.warn('tmux could not tag a run pane; the run goes ahead untracked', {
421+
error: tagged.stderr.trim(),
422+
})
424423
}
424+
const runId = tagged.ok ? tag : null
425425
try {
426426
writeFileSync(goPath, '')
427427
} catch (error) {
@@ -435,14 +435,15 @@ 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: such a
439-
* 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 the
439+
* run is untracked: such a pane is neither touched nor given up on.
440440
*/
441441
export async function runPaneState(
442442
handle: TmuxRunHandle,
443443
env: NodeJS.ProcessEnv
444444
): Promise<'ours' | 'gone' | 'unknown'> {
445445
if (!handle.pane) return 'gone'
446+
if (handle.runId === null) return 'unknown'
446447
const shown = await runTmux(
447448
['display-message', '-p', '-t', handle.pane, `#{${RUN_ID_OPTION}}`],
448449
env

‎packages/terminal-protocol/src/index.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,8 @@ export interface TerminalToolArgs {
177177
*/
178178
terminalId?: string
179179
/**
180-
* Which tmux pane to act on, as a tmux target (`session:window.pane`), for
181-
* a terminal that has tmux attached. Omitting it uses that session's active
180+
* Which tmux pane to act on, as a tmux target (`session:window.pane`, or a
181+
* run's pane id `%N`), for a terminal that has tmux attached. Omitting it uses that session's active
182182
* pane. Ignored when the terminal is a plain shell.
183183
*/
184184
pane?: string
@@ -214,7 +214,10 @@ export interface TerminalRunResult {
214214
durationMs: number
215215
cwd: string | null
216216
terminalId: string
217-
/** Set when the command ran in tmux: the target it ran under. */
217+
/**
218+
* Set when the command ran in tmux: its own pane's id (`%N`), a tmux target that `read`,
219+
* `input`, `kill` and `close` accept as `pane`.
220+
*/
218221
pane?: string
219222
/** True when output was elided to fit {@link MAX_TOOL_OUTPUT_CHARS}. */
220223
truncated: boolean

0 commit comments

Comments
 (0)