Skip to content

Commit b752e11

Browse files
committed
fix(desktop): read tmux format output with a separator no tmux version escapes
tmux 3.4 and 3.5 print control characters in -F output as octal escapes, so the 0x1f field separator arrived as the text \037 and no line split: Sim never found the tmux client in a terminal, treated it as a plain shell, and the panes operation came back empty. The separator is now printable text that no tmux escapes; a field that happened to contain it changes the line's field count, so that line is dropped rather than misread. The fake tmux in the unit tests now escapes its output the way 3.4 and 3.5 do, so this cannot pass unnoticed again.
1 parent 18ad8b8 commit b752e11

2 files changed

Lines changed: 53 additions & 5 deletions

File tree

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

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,15 @@ import {
88
isDescendantOf,
99
parseFormatLines,
1010
pollRun,
11+
resolveAttachment,
1112
runPaneState,
1213
startRun,
1314
stopRun,
1415
type TmuxRunHandle,
1516
} from '@/main/terminal/tmux'
1617

17-
/** The separator the format strings use; no tmux field can contain it. */
18-
const F = '\u001f'
18+
/** The separator the format strings use. */
19+
const F = '|~sim~|'
1920

2021
describe('parseFormatLines', () => {
2122
it('drops lines with the wrong field count rather than mis-assigning them', () => {
@@ -104,6 +105,8 @@ interface FakeTmuxState {
104105
log: string[]
105106
/** Commands the fake fails, with the error tmux would print. */
106107
fail?: Record<string, string>
108+
/** Attached clients, as `list-clients` reports them. */
109+
clients?: Array<{ pid: string; tty: string; session: string }>
107110
}
108111

109112
const FAKE_TMUX = `
@@ -114,6 +117,12 @@ const args = process.argv.slice(2)
114117
const save = () => fs.writeFileSync(file, JSON.stringify(state))
115118
const target = () => args[args.indexOf('-t') + 1]
116119
const fail = (message) => { process.stderr.write(message); process.exit(1) }
120+
// Prints a format's output as tmux 3.4 and 3.5 do: a backslash doubled, and every other control
121+
// character as its octal escape, so a control-character separator would arrive as text.
122+
const escaped = (text) =>
123+
text
124+
.replace(/\\\\/g, '\\\\\\\\')
125+
.replace(/[\\x00-\\x1f]/g, (c) => '\\\\' + c.charCodeAt(0).toString(8).padStart(3, '0'))
117126
if (state.fail && state.fail[args[0]]) fail(state.fail[args[0]])
118127
switch (args[0]) {
119128
case 'new-window': {
@@ -144,6 +153,17 @@ switch (args[0]) {
144153
process.stdout.write((pane.options[name] ?? '') + '\\n')
145154
break
146155
}
156+
case 'list-clients': {
157+
const format = args[args.indexOf('-F') + 1]
158+
for (const client of state.clients ?? []) {
159+
const line = format
160+
.replace('#{client_pid}', client.pid)
161+
.replace('#{client_tty}', client.tty)
162+
.replace('#{client_session}', client.session)
163+
process.stdout.write(escaped(line) + '\\n')
164+
}
165+
break
166+
}
147167
case 'send-keys':
148168
case 'kill-pane': {
149169
if (!state.panes[target()]) fail("can't find pane")
@@ -191,6 +211,29 @@ function fakeTmux(options: { exec?: boolean } = {}) {
191211
}
192212
}
193213

214+
describe('finding the tmux session a shell runs', () => {
215+
const dirs: string[] = []
216+
217+
afterEach(() => {
218+
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true })
219+
})
220+
221+
it('reads the clients tmux 3.4 and 3.5 list, which escape control characters', async () => {
222+
const tmux = fakeTmux()
223+
dirs.push(tmux.dir)
224+
// A client of this very process: its parent stands in for the shell the client runs in.
225+
tmux.write({
226+
...tmux.read(),
227+
clients: [{ pid: String(process.pid), tty: '/dev/pts/3', session: 'work' }],
228+
})
229+
230+
expect(await resolveAttachment(process.ppid, tmux.env)).toEqual({
231+
session: 'work',
232+
clientTty: '/dev/pts/3',
233+
})
234+
})
235+
})
236+
194237
describe('stopping a tmux run touches only its own pane', () => {
195238
const dirs: string[] = []
196239

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,13 @@ const TMUX_TIMEOUT_MS = 5_000
3838
/** How often the status file is checked while a tmux-run command is going. */
3939
const RUN_POLL_INTERVAL_MS = 250
4040

41-
/** Field separator for `-F` output. Chosen because no tmux field contains it. */
42-
const FIELD = '\u001f'
41+
/**
42+
* Field separator for `-F` output. Printable on purpose: tmux 3.4 and 3.5 print a control
43+
* 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.
46+
*/
47+
const FIELD = '|~sim~|'
4348

4449
export interface TmuxCommandResult {
4550
ok: boolean
@@ -125,7 +130,7 @@ export function runTmux(args: string[], env: NodeJS.ProcessEnv): Promise<TmuxCom
125130
/**
126131
* Parses `list-clients`/`list-panes` output into records.
127132
*
128-
* Split on a control character rather than whitespace: window names and
133+
* Split on a dedicated separator rather than whitespace: window names and
129134
* working directories contain spaces, and a path with a space would otherwise
130135
* shift every later field by one.
131136
*/

0 commit comments

Comments
 (0)