Skip to content

Commit 8dddbd7

Browse files
committed
fix(desktop): recognise how real tmux before 3.0 refuses pane options, and use a separator no field can straddle
- Real tmux 2.9a refuses `set-option -p` with its own getopt's `unknown option -- p` and set-option's usage line (BSD getopt on macOS: `illegal option -- p`). The untracked fallback only matched later wordings, so on real pre-3.0 tmux every run was refused. The fake tmux and the tests now use the text captured from real 2.9a. - The `-F` separator `|~sim~|` began and ended with the same character, so a field ending in `|~sim~` was misread rather than dropped. `<~sim~>` has no proper prefix that is also a suffix, so it is only found where it was written or wholly inside a field, whose line is then dropped.
1 parent 26c99a1 commit 8dddbd7

2 files changed

Lines changed: 40 additions & 19 deletions

File tree

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

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,33 @@ import {
1616
type TmuxRunHandle,
1717
} from '@/main/terminal/tmux'
1818

19+
/** What real tmux 2.9a writes for `set-option -p`, captured from the binary. */
20+
const TMUX_29_NO_PANE_OPTIONS =
21+
'tmux: unknown option -- p\nusage: set-option [-aFgosquw] [-t target-window] option [value]\n'
22+
/** The same refusal from a tmux built against BSD getopt, as on macOS. */
23+
const TMUX_29_BSD_NO_PANE_OPTIONS =
24+
'tmux: illegal option -- p\nusage: set-option [-aFgosquw] [-t target-window] option [value]\n'
25+
1926
/** The separator the format strings use. */
20-
const F = '|~sim~|'
27+
const F = '<~sim~>'
2128

2229
describe('parseFormatLines', () => {
2330
it('drops lines with the wrong field count rather than mis-assigning them', () => {
2431
expect(parseFormatLines(`a${F}b\nonly-one\n`, 2)).toEqual([['a', 'b']])
2532
})
33+
34+
it('reads a field that ends with part of the separator as it is', () => {
35+
// A cwd or window name may end with any text, including all but the separator's last character.
36+
const partial = F.slice(0, -1)
37+
expect(parseFormatLines(`/tmp/x/p${partial}${F}1\n`, 2)).toEqual([[`/tmp/x/p${partial}`, '1']])
38+
expect(parseFormatLines(`tail${partial}${F}%3${F}zsh\n`, 3)).toEqual([
39+
[`tail${partial}`, '%3', 'zsh'],
40+
])
41+
})
42+
43+
it('drops a line whose field holds the whole separator rather than misread it', () => {
44+
expect(parseFormatLines(`a${F}b${F}c\n`, 2)).toEqual([])
45+
})
2646
})
2747

2848
describe('isDescendantOf', () => {
@@ -108,10 +128,6 @@ interface FakeTmuxState {
108128
fail?: Record<string, string>
109129
/** Attached clients, as `list-clients` reports them. */
110130
clients?: Array<{ pid: string; tty: string; session: string }>
111-
112-
/** Commands the fake answers only after this many milliseconds, like a busy tmux server. */
113-
delay?: Record<string, number>
114-
115131
/** Commands the fake holds until the file named here exists, like a busy tmux server. */
116132
hold?: Record<string, string>
117133
/** Commands the fake is holding right now. */
@@ -132,10 +148,6 @@ const escaped = (text) =>
132148
text
133149
.replace(/\\\\/g, '\\\\\\\\')
134150
.replace(/[\\x00-\\x1f]/g, (c) => '\\\\' + c.charCodeAt(0).toString(8).padStart(3, '0'))
135-
136-
if (state.delay && state.delay[args[0]]) {
137-
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, state.delay[args[0]])
138-
139151
if (state.hold && state.hold[args[0]]) {
140152
state.held = [...(state.held ?? []), args[0]]
141153
save()
@@ -315,9 +327,12 @@ describe('stopping a tmux run touches only its own pane', () => {
315327
expect(tmux.read().log).toEqual([])
316328
})
317329

318-
it.each(['invalid option: @sim-run-id', 'unknown flag -p'])(
330+
it.each([
331+
['tmux 2.9a', TMUX_29_NO_PANE_OPTIONS],
332+
['BSD getopt', TMUX_29_BSD_NO_PANE_OPTIONS],
333+
])(
319334
'starts a run untracked on a tmux without pane options (%s), and never stops it by a pane id',
320-
async (refusal) => {
335+
async (_build, refusal) => {
321336
const tmux = fakeTmux()
322337
dirs.push(tmux.dir)
323338
// tmux before 3.0 has no pane options.
@@ -360,7 +375,7 @@ describe('stopping a tmux run touches only its own pane', () => {
360375
it("closes a finished untracked run's pane, and only once it has finished", async () => {
361376
const tmux = fakeTmux()
362377
dirs.push(tmux.dir)
363-
tmux.write({ ...tmux.read(), fail: { 'set-option': 'invalid option: @sim-run-id' } })
378+
tmux.write({ ...tmux.read(), fail: { 'set-option': TMUX_29_NO_PANE_OPTIONS } })
364379
const run = await startRun('agent', 'make build', null, tmux.env)
365380
if ('error' in run) throw new Error(run.error)
366381

@@ -376,7 +391,7 @@ describe('stopping a tmux run touches only its own pane', () => {
376391
it("never closes a pane that took a finished untracked run's id after tmux restarted", async () => {
377392
const tmux = fakeTmux()
378393
dirs.push(tmux.dir)
379-
tmux.write({ ...tmux.read(), fail: { 'set-option': 'invalid option: @sim-run-id' } })
394+
tmux.write({ ...tmux.read(), fail: { 'set-option': TMUX_29_NO_PANE_OPTIONS } })
380395
const run = await startRun('agent', 'make build', null, tmux.env)
381396
if ('error' in run) throw new Error(run.error)
382397
writeFileSync(run.statusPath, '0')
@@ -425,7 +440,7 @@ describe('stopping a tmux run touches only its own pane', () => {
425440

426441
const untagged = fakeTmux({ exec: true })
427442
dirs.push(untagged.dir)
428-
untagged.write({ ...untagged.read(), fail: { 'set-option': 'invalid option' } })
443+
untagged.write({ ...untagged.read(), fail: { 'set-option': TMUX_29_NO_PANE_OPTIONS } })
429444
const untracked = await startRun('agent', 'echo ran', null, untagged.env)
430445
if ('error' in untracked) throw new Error(untracked.error)
431446
await expect

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,11 @@ const RUN_POLL_INTERVAL_MS = 250
4141
/**
4242
* Field separator for `-F` output. Printable on purpose: tmux 3.4 and 3.5 print a control
4343
* character as its octal escape, so a control-character separator arrived as the text `\037` and
44-
* no line split. No tmux escapes these characters, and a field that happened to contain the
45-
* separator would change the line's field count, so that line is dropped rather than misread.
44+
* no line split. No tmux escapes these characters. No proper prefix of the separator is also a
45+
* suffix of it, so it can only be found where it was written or wholly inside a field: a field
46+
* holding it changes the line's field count, and that line is dropped rather than misread.
4647
*/
47-
const FIELD = '|~sim~|'
48+
const FIELD = '<~sim~>'
4849

4950
export interface TmuxCommandResult {
5051
ok: boolean
@@ -330,8 +331,13 @@ const RUN_GATE_POLLS = Math.ceil((2 * TMUX_TIMEOUT_MS + 5_000) / 50)
330331
/** The tmux user option that marks a pane as one run's own. */
331332
const RUN_ID_OPTION = '@sim-run-id'
332333

333-
/** How tmux before 3.0, which has no pane options, refuses `set-option -p`. */
334-
const NO_PANE_OPTIONS = /unknown flag|invalid option/i
334+
/**
335+
* How tmux before 3.0, which has no pane options, refuses `set-option -p`. Its own getopt prints
336+
* `unknown option -- p` (BSD getopt on macOS: `illegal option -- p`) followed by set-option's usage
337+
* line; later wordings are kept for any build that phrases it so.
338+
*/
339+
const NO_PANE_OPTIONS =
340+
/unknown option -- p|illegal option -- p|usage: set-option|unknown flag|invalid option/i
335341

336342
/**
337343
* Starts a command in a dedicated tmux window.

0 commit comments

Comments
 (0)