From 5e02f63d5c6f882bc2be672dff3b046e174f6a33 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 14 Sep 2026 08:02:45 +0200 Subject: [PATCH 1/5] (remote): forward the CLI's title through tmux on attach so an attached remote row is not mute Since the row-ownership rule (#273) an attached remote row is driven by the OSC title only, and tmux forwards it to the outer terminal only with set-titles on; the default is off and the default format wraps the pane title, so the classifier never saw the glyph. The attach command now sets set-titles on and set-titles-string '#T' in solo and shared mode alike, the probe records the previous values and the solo detach restores them. Closes #290. --- .ai/contexts/session-cache.md | 75 ++++++++++- remote-attach.js | 66 +++++++-- test/remote-attach.test.js | 244 +++++++++++++++++++++++++++------- 3 files changed, 326 insertions(+), 59 deletions(-) diff --git a/.ai/contexts/session-cache.md b/.ai/contexts/session-cache.md index 8301ba18..0fe99412 100644 --- a/.ai/contexts/session-cache.md +++ b/.ai/contexts/session-cache.md @@ -797,7 +797,8 @@ Launching a new remote session (#222) and injection over the messaging socket prefixes the attach with three session-scoped (never `-g`, never `-w`) `tmux ... \; ...` sets — `status off`, `mouse on`, `window-size latest` — when `solo` is true, and emits the unchanged pre-#253 command when it - isn't (shared attach never touches another client's view). The probe + isn't (shared attach never touches another client's view — qualified by + issue #290 below: title forwarding is the one exception). The probe (`buildProbeCommand`) now also reads `mouse` and `window-size` alongside `status`, and `parseProbeOutput` returns their raw pre-attach values as `pre: { status, mouse, windowSize }` (`null` when an option is absent @@ -820,6 +821,78 @@ Launching a new remote session (#222) and injection over the messaging socket killed. No shared-attach restore is ever sent, because a shared attach never applied the options in the first place. +- **Title forwarding, issue #290.** An attached remote row went mute + overnight (v0.0.79 field trace, `.work-files/switchboard/trace-2026-09-13- + nuit.md`): two VPS sessions attached 9+ hours, transcripts still being + written, zero busy signal on either row. Cause: since #273, an attached + row is driven only by the OSC title sequences the CLI writes to its own + terminal — and inside tmux those sequences update the *pane* title, which + tmux forwards to the outer terminal (what an ssh client actually sees) + only when the session option `set-titles` is on, formatted by + `set-titles-string`. Measured on the target host: tmux 3.6, `set-titles` + off, `set-titles-string` at its built-in default, + `` #S:#I:#W - "#T" #{session_alerts} `` — which additionally wraps the pane + title in surrounding text, defeating `classifyTitleActivity`'s reliance on + the *first code point* of the title even if forwarding were on. Fix: + `buildAttachCommand` now always prefixes the attach with `set -t + set-titles on \; set -t set-titles-string '#T'` — `#T` alone, + no wrapper — **in both solo and shared mode**, unlike `status`/`mouse`/ + `window-size`. This is deliberately not solo-gated: those three change + what a shared session's screen or input behavior looks like to every + attached human, which is exactly the case #253 refuses to disturb; title + forwarding only changes the *outer terminal's own title* for whichever + client asks for it (an ssh/tmux-client-local rendering choice, not a + property of the shared pane content), so a second human already attached + never sees anything change on their screen because of it. + - **`#T` must be quoted.** The attach command executes over `ssh -tt + ` with the whole `buildAttachCommand(...)` return value + passed as a single argv element — ssh joins it back into one string and + hands it to the remote's login shell (`$SHELL -c command`), it is never + exec'd directly. In POSIX shell, an unquoted `#` at the start of a word + starts a comment that swallows the rest of the line — `set-titles-string + #T` would silently delete `\; attach -t ` and every attach would + hang with no pty ever spawned. Wrapping it as `'#T'` keeps the `#` + inside a quoted context, where the comment rule never applies, and tmux + still receives the literal two-character format string `#T` (single + quotes are removed by the shell before tmux ever sees the argument). + - **The probe reads back both options the same way as `status`/`mouse`/ + `window-size`.** `buildProbeCommand` appends two more `show-options -A` + segments (`set-titles`, `set-titles-string`) ahead of the existing + `list-clients`/cmdline-check tail, and `parseProbeOutput` adds + `pre.setTitles`/`pre.setTitlesString` to its return value, following the + same starred/inherited-is-null rule as the other three options. + `parseDiscoveryProbeOutput`'s slice widened from `parts.slice(1, 5)` to + `parts.slice(1, 7)` to carry the two extra fields through to + `parseProbeOutput`; `clientCount`/`cmdlineHasClaude` shifted two + positions later (now `parts[7]`/`parts[8]`) — this is regenerated and + consumed by the same call, never persisted, so there is no wire-format + compatibility concern across a version boundary the way there would be + for a value the CLI itself writes to disk. + - **`set-titles-string` is a string option, not a bare word like the + other three — a dedicated parser and a dedicated restore quoting path + exist because of it.** `tmux show-options -A` quotes a string-valued + option in double quotes whenever it contains spaces, backslash-escaping + any embedded `"` or `\` (this is why the measured default prints as + `` "#S:#I:#W - \"#T\" #{session_alerts}" `` on the wire) — `bare \S+` + parsing (used for `status`/`mouse`/`window-size`) cannot capture a value + with spaces at all. `parseTitleStringToken` matches to the end of the + segment instead, strips one layer of surrounding `"..."` when present, + and unescapes `\"`/`\\`. On restore, the value must be **shell-quoted** + (single-quote-wrapped, with any embedded `'` escaped as `'\''`) because + it goes back through the same remote-shell-then-tmux path as the attach + command — an unquoted restore of the default value would both split on + its spaces and hit the same `#`-starts-a-comment problem as `#T` above. + Round-trip proven byte-for-byte in `test/remote-attach.test.js` against + the measured default value (spaces and embedded quotes) and against a + value containing a literal single quote. + - **Restore stays solo-gated, same as `status`/`mouse`/`window-size`.** On + detach, `buildRestoreCommand` puts `set-titles`/`set-titles-string` back + to their probed values (or `set -u` when they were `null`) only when the + attach was solo — a shared session keeps forwarding titles after this + app's client detaches, which is harmless (the CLI's own title-writing + behavior is unaffected either way) and avoids the asymmetry of + restoring only some of what a shared attach turned on. + - **This is the first thing to populate the session-handle seam from issue #220** (see `.ai/contexts/trigger-watcher.md`, "Session handle"): a remote-attach entry sets `host: alias`, `kind: 'remote-attach'`, and diff --git a/remote-attach.js b/remote-attach.js index b0bf4ad9..ea20d0ef 100644 --- a/remote-attach.js +++ b/remote-attach.js @@ -40,6 +40,19 @@ function parseOptionToken(part, name) { return { value: m[2], inherited: m[1] === '*' }; } +// see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) +function parseTitleStringToken(part, name) { + const re = new RegExp(`${escapeRegExpLiteral(name)}(\\*?)\\s+([\\s\\S]*)`); + const m = re.exec(part || ''); + if (!m) return { value: null, inherited: false }; + const inherited = m[1] === '*'; + const raw = m[2].replace(/\r?\n+$/, ''); + const value = raw.length >= 2 && raw[0] === '"' && raw[raw.length - 1] === '"' + ? raw.slice(1, -1).replace(/\\(["\\])/g, '$1') + : raw; + return { value, inherited }; +} + function parseProbeOutput(stdout) { const text = typeof stdout === 'string' ? stdout : ''; const parts = text.split(PROBE_SEP); @@ -47,6 +60,8 @@ function parseProbeOutput(stdout) { const statusPart = parts[1] || ''; const mousePart = parts[2] || ''; const windowSizePart = parts[3] || ''; + const setTitlesPart = parts[4] || ''; + const setTitlesStringPart = parts[5] || ''; const sizeMatch = /(\d+)x(\d+)/.exec(sizePart); if (!sizeMatch) return null; @@ -73,6 +88,12 @@ function parseProbeOutput(stdout) { const windowSizeParsed = parseOptionToken(windowSizePart, 'window-size'); if (['latest', 'largest', 'smallest', 'manual'].includes(windowSizeParsed.value)) windowSize = windowSizeParsed.value; + let setTitles = null; + const setTitlesParsed = parseOptionToken(setTitlesPart, 'set-titles'); + if (setTitlesParsed.value === 'on' || setTitlesParsed.value === 'off') setTitles = setTitlesParsed.value; + + const setTitlesStringParsed = parseTitleStringToken(setTitlesStringPart, 'set-titles-string'); + // pre. non-null only for a session-scoped override; null means restore by `set -u` return { cols: width, @@ -81,6 +102,8 @@ function parseProbeOutput(stdout) { status: statusParsed.inherited ? null : status, mouse: mouseParsed.inherited ? null : mouse, windowSize: windowSizeParsed.inherited ? null : windowSize, + setTitles: setTitlesParsed.inherited ? null : setTitles, + setTitlesString: setTitlesStringParsed.inherited ? null : setTitlesStringParsed.value, }, }; } @@ -99,24 +122,40 @@ function buildProbeCommand(pid, target) { `; printf '${PROBE_SEP}'; tmux -S "$sock" show-options -A -t ${target} status 2>/dev/null` + `; printf '${PROBE_SEP}'; tmux -S "$sock" show-options -A -t ${target} mouse 2>/dev/null` + `; printf '${PROBE_SEP}'; tmux -S "$sock" show-options -A -t ${target} window-size 2>/dev/null` + + `; printf '${PROBE_SEP}'; tmux -S "$sock" show-options -A -t ${target} set-titles 2>/dev/null` + + `; printf '${PROBE_SEP}'; tmux -S "$sock" show-options -A -t ${target} set-titles-string 2>/dev/null` + `; printf '${PROBE_SEP}'; tmux -S "$sock" list-clients -t ${target} 2>/dev/null | wc -l` + `; printf '${PROBE_SEP}'; ${buildProcCmdlineCheck(pid)}`; } -// see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", solo attach parity, issue #253) +// see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) +function shellSingleQuote(value) { + return `'${String(value).replace(/'/g, `'\\''`)}'`; +} + +// see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) function buildAttachCommand(socket, target, opts = {}) { + const titleSegments = [ + `set -t ${target} set-titles on`, + `set -t ${target} set-titles-string '#T'`, + ]; if (!opts.solo) { - return `tmux -S '${socket}' attach -t ${target}`; + return `tmux -S '${socket}' ${[...titleSegments, `attach -t ${target}`].join(' \\; ')}`; } - return `tmux -S '${socket}' set -t ${target} status off \\; ` + - `set -t ${target} mouse on \\; ` + - `set -t ${target} window-size latest \\; ` + - `attach -t ${target}`; + const segments = [ + `set -t ${target} status off`, + `set -t ${target} mouse on`, + `set -t ${target} window-size latest`, + ...titleSegments, + `attach -t ${target}`, + ]; + return `tmux -S '${socket}' ${segments.join(' \\; ')}`; } // see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", solo attach parity, issue #253) -function buildRestoreOptionSegment(target, name, value) { - return value == null ? `set -u -t ${target} ${name}` : `set -t ${target} ${name} ${value}`; +function buildRestoreOptionSegment(target, name, value, opts = {}) { + if (value == null) return `set -u -t ${target} ${name}`; + return `set -t ${target} ${name} ${opts.quote ? shellSingleQuote(value) : value}`; } function buildRestoreCommand(socket, target, pre) { @@ -125,6 +164,8 @@ function buildRestoreCommand(socket, target, pre) { buildRestoreOptionSegment(target, 'status', p.status), buildRestoreOptionSegment(target, 'mouse', p.mouse), buildRestoreOptionSegment(target, 'window-size', p.windowSize), + buildRestoreOptionSegment(target, 'set-titles', p.setTitles), + buildRestoreOptionSegment(target, 'set-titles-string', p.setTitlesString, { quote: true }), ]; return `tmux -S '${socket}' ${segments.join(' \\; ')}`; } @@ -138,16 +179,17 @@ function parseClientCount(text) { } // see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", socket discovery) -// parts: [socket, size, status, mouse, window-size, clientCount, cmdlineHasClaude]; trailing ones optional +// parts: [socket, size, status, mouse, window-size, set-titles, set-titles-string, +// clientCount, cmdlineHasClaude]; trailing ones optional function parseDiscoveryProbeOutput(stdout) { const text = typeof stdout === 'string' ? stdout : ''; const parts = text.split(PROBE_SEP); const socket = parts[0] || ''; if (parts.length < 3 || !isSafeSocketPath(socket)) return null; - const probed = parseProbeOutput(parts.slice(1, 5).join(PROBE_SEP)); + const probed = parseProbeOutput(parts.slice(1, 7).join(PROBE_SEP)); if (!probed) return null; - const clientCount = parseClientCount(parts[5]); - const cmdlineHasClaude = parts[6] === '1' ? true : parts[6] === '0' ? false : null; + const clientCount = parseClientCount(parts[7]); + const cmdlineHasClaude = parts[8] === '1' ? true : parts[8] === '0' ? false : null; return { socket, cols: probed.cols, rows: probed.rows, pre: probed.pre, clientCount, cmdlineHasClaude }; } diff --git a/test/remote-attach.test.js b/test/remote-attach.test.js index 1fe12bcd..b16c1639 100644 --- a/test/remote-attach.test.js +++ b/test/remote-attach.test.js @@ -23,6 +23,7 @@ const { buildRestoreCommand, buildRemoteCommandArgs, } = require('../remote-attach'); +const { classifyTitleActivity } = require('../classify-title-activity'); const PROBE_SEP = ''; const silentLog = { info() {}, warn() {}, error() {} }; @@ -74,21 +75,21 @@ test('parseTmuxField accepts the CLI-written format and rejects the rest', () => test('parseProbeOutput sizes rows as height plus status lines (status on)', () => { assert.deepEqual( parseProbeOutput('200x51' + PROBE_SEP + 'status on'), - { cols: 200, rows: 52, pre: { status: 'on', mouse: null, windowSize: null } }, + { cols: 200, rows: 52, pre: { status: 'on', mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, ); }); test('parseProbeOutput sizes rows as height plus 0 when status is off', () => { assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status off'), - { cols: 200, rows: 50, pre: { status: 'off', mouse: null, windowSize: null } }, + { cols: 200, rows: 50, pre: { status: 'off', mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, ); }); test('parseProbeOutput honors a rendered status line count beyond on/off', () => { assert.deepEqual( parseProbeOutput('200x51' + PROBE_SEP + 'status 2'), - { cols: 200, rows: 53, pre: { status: 2, mouse: null, windowSize: null } }, + { cols: 200, rows: 53, pre: { status: 2, mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, ); }); @@ -100,11 +101,11 @@ test('parseProbeOutput returns null when the size cannot be parsed', () => { test('parseProbeOutput parses pre-attach mouse and window-size when present', () => { assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status off' + PROBE_SEP + 'mouse on' + PROBE_SEP + 'window-size latest'), - { cols: 200, rows: 50, pre: { status: 'off', mouse: 'on', windowSize: 'latest' } }, + { cols: 200, rows: 50, pre: { status: 'off', mouse: 'on', windowSize: 'latest', setTitles: null, setTitlesString: null } }, ); assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual'), - { cols: 200, rows: 51, pre: { status: 'on', mouse: 'off', windowSize: 'manual' } }, + { cols: 200, rows: 51, pre: { status: 'on', mouse: 'off', windowSize: 'manual', setTitles: null, setTitlesString: null } }, ); }); @@ -114,15 +115,58 @@ test('parseProbeOutput parses pre-attach mouse and window-size when present', () test('parseProbeOutput reports null for mouse and window-size when absent from the probe output', () => { assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + ''), - { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: null } }, + { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, ); // No separators at all beyond size+status -- same as the pre-#253 wire format. assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status off'), - { cols: 200, rows: 50, pre: { status: 'off', mouse: null, windowSize: null } }, + { cols: 200, rows: 50, pre: { status: 'off', mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, ); }); +// --- set-titles / set-titles-string probe parsing (issue #290) ------------ + +test('parseProbeOutput parses a session-scoped set-titles override', () => { + assert.deepEqual( + parseProbeOutput('200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + 'set-titles on'), + { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: null, setTitles: 'on', setTitlesString: null } }, + ); +}); + +test('parseProbeOutput reports setTitles null when absent or inherited (starred)', () => { + assert.deepEqual( + parseProbeOutput('200x50' + PROBE_SEP + 'status on'), + { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, + ); + assert.deepEqual( + parseProbeOutput('200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + 'set-titles* off'), + { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, + ); +}); + +// Host measurement (tmux 3.6): the default set-titles-string is +// `#S:#I:#W - "#T" #{session_alerts}`, which `show-options -A` quotes and +// backslash-escapes because it contains spaces and embedded double quotes -- +// see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles). +const DEFAULT_SET_TITLES_STRING = '#S:#I:#W - "#T" #{session_alerts}'; +const QUOTED_DEFAULT_SET_TITLES_STRING = '"#S:#I:#W - \\"#T\\" #{session_alerts}"'; + +test('parseProbeOutput unescapes a quoted set-titles-string value with embedded quotes and spaces', () => { + const probed = parseProbeOutput( + '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '' + + PROBE_SEP + `set-titles-string ${QUOTED_DEFAULT_SET_TITLES_STRING}`, + ); + assert.equal(probed.pre.setTitlesString, DEFAULT_SET_TITLES_STRING); +}); + +test('parseProbeOutput reports setTitlesString null when starred (inherited)', () => { + const probed = parseProbeOutput( + '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '' + + PROBE_SEP + `set-titles-string* ${QUOTED_DEFAULT_SET_TITLES_STRING}`, + ); + assert.equal(probed.pre.setTitlesString, null); +}); + // Property 1, through the adapter: the size handed to spawnPty must already // carry the status-line correction, not the bare tmux window height. test('attach() spawns the pty at window height plus status lines, never the bare height', async () => { @@ -295,7 +339,7 @@ test('attach() opens at the local size and forwards resize to the ssh pty when n const raw = fakeRawPty(); const spawnCalls = []; const adapter = makeAdapter({ - probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '0', + probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '0', spawnCalls, rawPtyFactory: () => raw.pty, }); @@ -322,7 +366,7 @@ test('attach() keeps the fixed remote size and ignores resize when another clien const logLines = []; const log = { info: (msg) => logLines.push(msg), warn() {}, error() {} }; const adapter = makeAdapter({ - probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '1', + probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '1', spawnCalls, rawPtyFactory: () => raw.pty, log, @@ -348,7 +392,7 @@ test('attach() keeps the fixed remote size and ignores resize when another clien test('attach() fails closed to the fixed remote size when the client count cannot be parsed', async () => { const spawnCalls = []; const adapter = makeAdapter({ - probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + 'garbage', + probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + 'garbage', spawnCalls, }); const result = await adapter.attach( @@ -368,7 +412,7 @@ test('the attached-client count rides the existing probe connection, never a sec const spawnCalls = []; const probeCalls = []; const adapter = makeAdapter({ - probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '0', + probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '0', spawnCalls, probeCalls, }); @@ -390,15 +434,15 @@ test('the attached-client count rides the existing probe connection, never a sec test('parseProbeOutput sizes a starred (inherited) status option exactly like the unstarred form', () => { assert.deepEqual( parseProbeOutput('200x51' + PROBE_SEP + 'status* on'), - { cols: 200, rows: 52, pre: { status: null, mouse: null, windowSize: null } }, + { cols: 200, rows: 52, pre: { status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, ); assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status* off'), - { cols: 200, rows: 50, pre: { status: null, mouse: null, windowSize: null } }, + { cols: 200, rows: 50, pre: { status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, ); assert.deepEqual( parseProbeOutput('200x51' + PROBE_SEP + 'status* 2'), - { cols: 200, rows: 53, pre: { status: null, mouse: null, windowSize: null } }, + { cols: 200, rows: 53, pre: { status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, ); }); @@ -407,12 +451,12 @@ test('parseProbeOutput sizes a starred (inherited) status option exactly like th test('parseProbeOutput: pre.mouse is the value for a session-scoped override, null for an inherited one', () => { assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + ''), - { cols: 200, rows: 51, pre: { status: 'on', mouse: 'off', windowSize: null } }, + { cols: 200, rows: 51, pre: { status: 'on', mouse: 'off', windowSize: null, setTitles: null, setTitlesString: null } }, 'unstarred "mouse off" is a real session override -- must be restored via set -t', ); assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse* on' + PROBE_SEP + ''), - { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: null } }, + { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, 'starred "mouse* on" is inherited -- no session override exists, restore must set -u', ); }); @@ -420,11 +464,11 @@ test('parseProbeOutput: pre.mouse is the value for a session-scoped override, nu test('parseProbeOutput: pre.windowSize follows the same starred/unstarred rule as status and mouse', () => { assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + 'window-size manual'), - { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: 'manual' } }, + { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: 'manual', setTitles: null, setTitlesString: null } }, ); assert.deepEqual( parseProbeOutput('200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + 'window-size* latest'), - { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: null } }, + { cols: 200, rows: 51, pre: { status: 'on', mouse: null, windowSize: null, setTitles: null, setTitlesString: null } }, ); }); @@ -437,30 +481,52 @@ test('buildAttachCommand: solo prefixes session-scoped option sets before attach "tmux -S '/tmp/tmux-0/main' set -t main:@0.%0 status off \\; " + 'set -t main:@0.%0 mouse on \\; ' + 'set -t main:@0.%0 window-size latest \\; ' + + "set -t main:@0.%0 set-titles on \\; " + + "set -t main:@0.%0 set-titles-string '#T' \\; " + 'attach -t main:@0.%0', ); const statusIdx = cmd.indexOf('status off'); const mouseIdx = cmd.indexOf('mouse on'); const windowSizeIdx = cmd.indexOf('window-size latest'); + const setTitlesIdx = cmd.indexOf('set-titles on'); + const setTitlesStringIdx = cmd.indexOf("set-titles-string '#T'"); const attachIdx = cmd.indexOf('attach -t'); - assert.ok(statusIdx < mouseIdx && mouseIdx < windowSizeIdx && windowSizeIdx < attachIdx, 'sets must precede attach, in order'); + assert.ok( + statusIdx < mouseIdx && mouseIdx < windowSizeIdx && windowSizeIdx < setTitlesIdx && setTitlesIdx < setTitlesStringIdx && setTitlesStringIdx < attachIdx, + 'sets must precede attach, in order', + ); assert.ok(!cmd.includes('-g'), 'solo attach must never touch the global option scope'); assert.ok(!cmd.includes('-w'), 'solo attach must never touch window-scoped options'); }); -test('buildAttachCommand: shared (solo false or omitted) emits the byte-identical unchanged command', () => { - const unchanged = "tmux -S '/tmp/tmux-0/main' attach -t main:@0.%0"; - assert.equal(buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0', { solo: false }), unchanged); - assert.equal(buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0'), unchanged); +// issue #290 -- title forwarding is applied in shared mode too (it only +// changes each client's own outer-terminal title, unlike status/mouse/ +// window-size, which stay untouched for a shared attach). +test('buildAttachCommand: shared still turns on title forwarding, quoting #T so the shell does not treat it as a comment', () => { + const cmd = buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0', { solo: false }); + assert.equal( + cmd, + "tmux -S '/tmp/tmux-0/main' set -t main:@0.%0 set-titles on \\; " + + "set -t main:@0.%0 set-titles-string '#T' \\; " + + 'attach -t main:@0.%0', + ); + assert.equal(buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0'), cmd, 'solo omitted behaves like solo: false'); + assert.ok(!cmd.includes('status'), 'shared attach must never touch status'); + assert.ok(!cmd.includes('mouse'), 'shared attach must never touch mouse'); + assert.ok(!cmd.includes('window-size'), 'shared attach must never touch window-size'); }); test('buildRestoreCommand: restores each probed value when non-null', () => { - const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { status: 'on', mouse: 'off', windowSize: 'manual' }); + const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { + status: 'on', mouse: 'off', windowSize: 'manual', setTitles: 'off', setTitlesString: 'plain', + }); assert.equal( cmd, "tmux -S '/tmp/tmux-0/main' set -t main:@0.%0 status on \\; " + 'set -t main:@0.%0 mouse off \\; ' + - 'set -t main:@0.%0 window-size manual', + 'set -t main:@0.%0 window-size manual \\; ' + + 'set -t main:@0.%0 set-titles off \\; ' + + "set -t main:@0.%0 set-titles-string 'plain'", ); }); @@ -470,25 +536,84 @@ test('buildRestoreCommand: restores a numeric status value', () => { }); test('buildRestoreCommand: uses "set -u" for each probed value that was null', () => { - const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { status: null, mouse: null, windowSize: null }); + const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { + status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: null, + }); assert.equal( cmd, "tmux -S '/tmp/tmux-0/main' set -u -t main:@0.%0 status \\; " + 'set -u -t main:@0.%0 mouse \\; ' + - 'set -u -t main:@0.%0 window-size', + 'set -u -t main:@0.%0 window-size \\; ' + + 'set -u -t main:@0.%0 set-titles \\; ' + + 'set -u -t main:@0.%0 set-titles-string', + ); +}); + +test('buildRestoreCommand: defaults every option to "set -u" when pre is empty/absent', () => { + assert.equal( + buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', {}), + buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { + status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: null, + }), ); }); test('buildRestoreCommand: mixes "set" and "set -u" per option independently', () => { - const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { status: 'off', mouse: null, windowSize: 'latest' }); + const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { + status: 'off', mouse: null, windowSize: 'latest', setTitles: 'on', setTitlesString: null, + }); assert.equal( cmd, "tmux -S '/tmp/tmux-0/main' set -t main:@0.%0 status off \\; " + 'set -u -t main:@0.%0 mouse \\; ' + - 'set -t main:@0.%0 window-size latest', + 'set -t main:@0.%0 window-size latest \\; ' + + 'set -t main:@0.%0 set-titles on \\; ' + + 'set -u -t main:@0.%0 set-titles-string', ); }); +// issue #290 -- the round trip must reproduce the default set-titles-string +// (spaces and embedded double quotes) byte for byte. No real shell runs here, +// so this test undoes the single-quoting itself (bash/sh single-quote +// removal is: strip the wrapping quotes, "'\''" becomes "'") to prove the +// segment, once shell-unquoted, is exactly the probed value. +function unshellSingleQuote(word) { + assert.equal(word[0], "'", `expected a single-quoted word: ${word}`); + assert.equal(word[word.length - 1], "'", `expected a single-quoted word: ${word}`); + return word.slice(1, -1).replace(/'\\''/g, "'"); +} + +test('buildRestoreCommand: set-titles-string quoting round-trips the default tmux value byte for byte', () => { + const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { + status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: DEFAULT_SET_TITLES_STRING, + }); + const m = /set -t main:@0\.%0 set-titles-string (.+)$/.exec(cmd); + assert.ok(m, `restore command must set-titles-string: ${cmd}`); + assert.equal(unshellSingleQuote(m[1]), DEFAULT_SET_TITLES_STRING); +}); + +test('buildRestoreCommand: set-titles-string quoting round-trips a value containing a literal single quote', () => { + const withQuote = `it's "#T"`; + const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { + status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: withQuote, + }); + const m = /set -t main:@0\.%0 set-titles-string (.+)$/.exec(cmd); + assert.ok(m, `restore command must set-titles-string: ${cmd}`); + assert.equal(unshellSingleQuote(m[1]), withQuote); +}); + +// issue #290 -- the probe must read back both new options so a restore has +// something to put back. +test('buildProbeCommand reads both set-titles and set-titles-string', () => { + const cmd = buildProbeCommand(4242, 'main:@0.%0'); + assert.match(cmd, /show-options -A -t main:@0\.%0 set-titles 2>\/dev\/null/); + assert.match(cmd, /show-options -A -t main:@0\.%0 set-titles-string 2>\/dev\/null/); + const setTitlesIdx = cmd.indexOf('show-options -A -t main:@0.%0 set-titles 2'); + const setTitlesStringIdx = cmd.indexOf('show-options -A -t main:@0.%0 set-titles-string 2'); + const listClientsIdx = cmd.indexOf('list-clients'); + assert.ok(setTitlesIdx < setTitlesStringIdx && setTitlesStringIdx < listClientsIdx, 'both option probes must precede the trailing client-count/cmdline segments'); +}); + // No remote command string may ever contain a backtick -- these run over ssh, // where a backtick executes (issue #253 acceptance criterion). test('no builder ever emits a backtick', () => { @@ -497,8 +622,12 @@ test('no builder ever emits a backtick', () => { buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0'), buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0', { solo: true }), buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0', { solo: false }), - buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { status: 'on', mouse: 'off', windowSize: 'manual' }), - buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { status: null, mouse: null, windowSize: null }), + buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { + status: 'on', mouse: 'off', windowSize: 'manual', setTitles: 'on', setTitlesString: DEFAULT_SET_TITLES_STRING, + }), + buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { + status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: null, + }), buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', {}), buildRemoteCommandArgs('vps', 'echo hi').join(' '), ]; @@ -519,7 +648,7 @@ test('buildRemoteCommandArgs adds ConnectTimeout=5 alongside BatchMode, alias la test('parseDiscoveryProbeOutput reads the trailing cmdline-check segment as cmdlineHasClaude', () => { const fields = (cmdline) => { - const base = [FAKE_SOCKET, '200x50', 'status on', '', '', '0']; + const base = [FAKE_SOCKET, '200x50', 'status on', '', '', '', '', '0']; return (cmdline == null ? base : [...base, cmdline]).join(PROBE_SEP); }; assert.equal(parseDiscoveryProbeOutput(fields('1')).cmdlineHasClaude, true); @@ -544,7 +673,7 @@ test('parseDiscoveryProbeOutput reads the trailing cmdline-check segment as cmdl test('attach() proceeds when the probed cmdline still says claude', async () => { const spawnCalls = []; const adapter = makeAdapter({ - probeStdout: ['200x50', 'status on', '', '', '0', '1'].join(PROBE_SEP), + probeStdout: ['200x50', 'status on', '', '', '', '', '0', '1'].join(PROBE_SEP), spawnCalls, }); const result = await adapter.attach('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0', procStart: '123456' }); @@ -555,7 +684,7 @@ test('attach() proceeds when the probed cmdline still says claude', async () => test('attach() refuses before spawnPty when the probed cmdline no longer says claude', async () => { const spawnCalls = []; const adapter = makeAdapter({ - probeStdout: ['200x50', 'status on', '', '', '0', '0'].join(PROBE_SEP), + probeStdout: ['200x50', 'status on', '', '', '', '', '0', '0'].join(PROBE_SEP), spawnCalls, }); const result = await adapter.attach('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0', procStart: '123456' }); @@ -567,7 +696,7 @@ test('attach() refuses before spawnPty when the probed cmdline no longer says cl test('attach() proceeds unverified when the probe carries no cmdline segment (older probe output)', async () => { const spawnCalls = []; const adapter = makeAdapter({ - probeStdout: ['200x50', 'status on', '', '', '0'].join(PROBE_SEP), + probeStdout: ['200x50', 'status on', '', '', '', '', '0'].join(PROBE_SEP), spawnCalls, }); const result = await adapter.attach('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0', procStart: '123456' }); @@ -580,7 +709,7 @@ test('attach() proceeds unverified when the probe carries no cmdline segment (ol test('attach() applies the session-scoped option sets in the real ssh argv when solo', async () => { const spawnCalls = []; const adapter = makeAdapter({ - probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '0', + probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '0', spawnCalls, }); const result = await adapter.attach( @@ -588,15 +717,18 @@ test('attach() applies the session-scoped option sets in the real ssh argv when ); assert.equal(result.ok, true); const attachCommand = spawnCalls[0].args[spawnCalls[0].args.length - 1]; - assert.match(attachCommand, /set -t main:@0\.%0 status off \\; set -t main:@0\.%0 mouse on \\; set -t main:@0\.%0 window-size latest \\; attach -t main:@0\.%0/); + assert.match( + attachCommand, + /set -t main:@0\.%0 status off \\; set -t main:@0\.%0 mouse on \\; set -t main:@0\.%0 window-size latest \\; set -t main:@0\.%0 set-titles on \\; set -t main:@0\.%0 set-titles-string '#T' \\; attach -t main:@0\.%0/, + ); }); -// Shared attach() must emit the byte-identical unchanged attach command -- -// never touch another attached client's view. -test('attach() emits the unchanged attach command in the real ssh argv when shared', async () => { +// issue #290 -- title forwarding is turned on in shared mode too, unlike +// status/mouse/window-size which stay untouched for a shared attach. +test('attach() turns on title forwarding in the real ssh argv when shared, leaving everything else unchanged', async () => { const spawnCalls = []; const adapter = makeAdapter({ - probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '1', + probeStdout: '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + 'mouse off' + PROBE_SEP + 'window-size manual' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '1', spawnCalls, }); const result = await adapter.attach( @@ -604,7 +736,10 @@ test('attach() emits the unchanged attach command in the real ssh argv when shar ); assert.equal(result.ok, true); const attachCommand = spawnCalls[0].args[spawnCalls[0].args.length - 1]; - assert.equal(attachCommand, "tmux -S '/tmp/tmux-0/test' attach -t main:@0.%0"); + assert.equal( + attachCommand, + "tmux -S '/tmp/tmux-0/test' set -t main:@0.%0 set-titles on \\; set -t main:@0.%0 set-titles-string '#T' \\; attach -t main:@0.%0", + ); }); // Detach must run a best-effort restore ssh call using the probed pre-attach @@ -619,7 +754,7 @@ test('detach() runs a best-effort restore call with the probed pre-attach values } return { code: 0, - stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}0`, + stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}set-titles on${PROBE_SEP}set-titles-string ${QUOTED_DEFAULT_SET_TITLES_STRING}${PROBE_SEP}0`, stderr: '', }; }; @@ -641,7 +776,9 @@ test('detach() runs a best-effort restore call with the probed pre-attach values assert.equal(restoreCalls.length, 1, 'exactly one restore call must be sent on detach when solo'); assert.equal( restoreCalls[0], - "tmux -S '/tmp/tmux-0/test' set -t main:@0.%0 status on \\; set -t main:@0.%0 mouse off \\; set -t main:@0.%0 window-size manual", + "tmux -S '/tmp/tmux-0/test' set -t main:@0.%0 status on \\; set -t main:@0.%0 mouse off \\; set -t main:@0.%0 window-size manual \\; " + + `set -t main:@0.%0 set-titles on \\; set -t main:@0.%0 set-titles-string '${DEFAULT_SET_TITLES_STRING}'`, + 'the set-titles-string round trip must reproduce the original value byte for byte', ); }); @@ -657,7 +794,7 @@ test('detach() sends no restore call when shared', async () => { } return { code: 0, - stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}1`, + stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}${PROBE_SEP}${PROBE_SEP}1`, stderr: '', }; }; @@ -688,7 +825,7 @@ test('detach() swallows a failing restore call without throwing', async () => { } return { code: 0, - stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}0`, + stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}${PROBE_SEP}${PROBE_SEP}0`, stderr: '', }; }; @@ -707,3 +844,18 @@ test('detach() swallows a failing restore call without throwing', async () => { await Promise.resolve(); assert.equal(raw.killedCount(), 1, 'the local ssh client must still be killed even if the restore call rejects'); }); + +// issue #290 -- pins that '#T' is the right choice: once set-titles-string is +// '#T', the pane title tmux forwards through the OSC 0 sequence IS the CLI's +// own title, unwrapped by any surrounding format (no "#S:#I:#W - ..." around +// it) -- so the same OSC-extraction regex wireSessionPty() uses on a local +// PTY (main.js, see .ai/contexts/session-cache.md) classifies it identically. +test('a tmux-forwarded #T title reaches classifyTitleActivity as busy, same as a local OSC 0 title', () => { + const data = '\x1b]0;⠋ Claude Code\x07'; + const oscMatches = [...data.matchAll(/\x1b\](\d+);([^\x07\x1b]*)(?:\x07|\x1b\\)/g)]; + assert.equal(oscMatches.length, 1); + assert.equal(oscMatches[0][1], '0'); + const payload = oscMatches[0][2]; + assert.equal(payload, '⠋ Claude Code'); + assert.deepEqual(classifyTitleActivity(payload), { busy: true, idle: false, via: 'glyph' }); +}); From 9554100c294f065ce14b77f95599ebe5c0991a8f Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 14 Sep 2026 08:21:42 +0200 Subject: [PATCH 2/5] (remote): restore the title options on every detach and hand tmux back its own quoted token Shared attaches also set the two title options, so they are restored on a shared detach too (the other three stay solo-only), which removes the baseline ratchet after a first shared attach. set-titles-string is kept as the raw token tmux prints and restored inside shell single quotes only, letting tmux re-parse its own quoting. --- .ai/contexts/session-cache.md | 76 +++++++++++++++++++++++------------ remote-attach.js | 39 +++++++++--------- test/remote-attach.test.js | 75 ++++++++++++++++++++-------------- 3 files changed, 115 insertions(+), 75 deletions(-) diff --git a/.ai/contexts/session-cache.md b/.ai/contexts/session-cache.md index 0fe99412..939644d5 100644 --- a/.ai/contexts/session-cache.md +++ b/.ai/contexts/session-cache.md @@ -818,8 +818,10 @@ Launching a new remote session (#222) and injection over the messaging socket override (`set -u -t `) so the host's own global option applies again. The restore call's failure is only logged — it never throws out of `detach()` and never blocks the local ssh client from being - killed. No shared-attach restore is ever sent, because a shared attach - never applied the options in the first place. + killed. No shared-attach restore is ever sent for these three options, + because a shared attach never applied them in the first place — issue + #290 below adds a restore that shared detach DOES send, but only for the + two title options a shared attach does touch. - **Title forwarding, issue #290.** An attached remote row went mute overnight (v0.0.79 field trace, `.work-files/switchboard/trace-2026-09-13- @@ -837,13 +839,23 @@ Launching a new remote session (#222) and injection over the messaging socket `buildAttachCommand` now always prefixes the attach with `set -t set-titles on \; set -t set-titles-string '#T'` — `#T` alone, no wrapper — **in both solo and shared mode**, unlike `status`/`mouse`/ - `window-size`. This is deliberately not solo-gated: those three change - what a shared session's screen or input behavior looks like to every - attached human, which is exactly the case #253 refuses to disturb; title - forwarding only changes the *outer terminal's own title* for whichever - client asks for it (an ssh/tmux-client-local rendering choice, not a - property of the shared pane content), so a second human already attached - never sees anything change on their screen because of it. + `window-size`. + - **This is a deliberate, session-wide side effect on another human's + terminal — not a no-op for a shared attach.** `set-titles` and + `set-titles-string` are *session* options: turning them on changes what + every client currently attached to that session sees as its own outer + terminal's title (each ssh/tmux client renders the forwarded pane title + into its own window/tab title — session-wide because the option lives on + the session, client-local only in *where* each client happens to render + it), unlike a purely local rendering choice. This is unlike `status`/ + `mouse`/`window-size`, which change the shared *screen or input* another + human is looking at or typing into — the harm case #253 refuses. A title + change is comparatively minor (a tab/window title, not the pane content + or input behavior) and is reverted on detach (below), so the decision + made here is: **forward titles in shared mode anyway** — a permanently + mute attached row is the worse defect (the #290 field trace may well have + been a shared attach), and the side effect is both small and temporary. + State this plainly rather than claiming no visible effect. - **`#T` must be quoted.** The attach command executes over `ssh -tt ` with the whole `buildAttachCommand(...)` return value passed as a single argv element — ssh joins it back into one string and @@ -876,22 +888,36 @@ Launching a new remote session (#222) and injection over the messaging socket `` "#S:#I:#W - \"#T\" #{session_alerts}" `` on the wire) — `bare \S+` parsing (used for `status`/`mouse`/`window-size`) cannot capture a value with spaces at all. `parseTitleStringToken` matches to the end of the - segment instead, strips one layer of surrounding `"..."` when present, - and unescapes `\"`/`\\`. On restore, the value must be **shell-quoted** - (single-quote-wrapped, with any embedded `'` escaped as `'\''`) because - it goes back through the same remote-shell-then-tmux path as the attach - command — an unquoted restore of the default value would both split on - its spaces and hit the same `#`-starts-a-comment problem as `#T` above. - Round-trip proven byte-for-byte in `test/remote-attach.test.js` against - the measured default value (spaces and embedded quotes) and against a - value containing a literal single quote. - - **Restore stays solo-gated, same as `status`/`mouse`/`window-size`.** On - detach, `buildRestoreCommand` puts `set-titles`/`set-titles-string` back - to their probed values (or `set -u` when they were `null`) only when the - attach was solo — a shared session keeps forwarding titles after this - app's client detaches, which is harmless (the CLI's own title-writing - behavior is unaffected either way) and avoids the asymmetry of - restoring only some of what a shared attach turned on. + segment instead and keeps **the raw printed token exactly as tmux wrote + it** (`"#S:#I:#W - \"#T\" #{session_alerts}"`, quote marks and backslash + escapes included as literal characters) — it does not unquote or + unescape it; starred (inherited) still reads as `null`, same rule as the + other three. On restore, `buildRestoreCommand` puts that raw token back + with **shell single-quoting only** (`''`, any embedded `'` + escaped as `'\''`) and lets **tmux's own command-line parser** undo + tmux's own quoting when `set -t set-titles-string ` + receives it — the same division of labor as `#T` above (the shell only + protects the argument from word-splitting and the `#`-comment rule, + tmux does the rest). Attempting to unescape and re-serialize the value + in this adapter would risk a mismatch against tmux's actual quoting + rules; forwarding the untouched raw token sidesteps that risk entirely. + Proven in `test/remote-attach.test.js`: a probed raw token reappears + byte for byte inside the restore command's single quotes. + - **Restore now runs on every detach, shared included — solo restores all + five options, shared restores only the two title options.** Earlier this + fix gated the whole restore on `solo`, matching `status`/`mouse`/ + `window-size` — but since the `set` for titles now also fires in shared + mode, that left a **baseline ratchet**: after one shared attach the + session stayed at `set-titles on` / `set-titles-string '#T'` forever, + and every later probe would read that back as the pre-existing baseline + to restore *to*, permanently losing whatever the session had before its + first shared attach. `buildRestoreCommand(socket, target, pre, { + titlesOnly })` now takes a `titlesOnly` option; `detach()` always fires + a restore call, passing `titlesOnly: !solo` — a solo detach restores + `status`/`mouse`/`window-size`/`set-titles`/`set-titles-string` exactly + as before, a shared detach restores only `set-titles`/`set-titles-string` + (never `status`/`mouse`/`window-size`, which a shared attach never + touched in the first place). - **This is the first thing to populate the session-handle seam from issue #220** (see `.ai/contexts/trigger-watcher.md`, "Session handle"): a diff --git a/remote-attach.js b/remote-attach.js index ea20d0ef..a50f79c3 100644 --- a/remote-attach.js +++ b/remote-attach.js @@ -45,12 +45,7 @@ function parseTitleStringToken(part, name) { const re = new RegExp(`${escapeRegExpLiteral(name)}(\\*?)\\s+([\\s\\S]*)`); const m = re.exec(part || ''); if (!m) return { value: null, inherited: false }; - const inherited = m[1] === '*'; - const raw = m[2].replace(/\r?\n+$/, ''); - const value = raw.length >= 2 && raw[0] === '"' && raw[raw.length - 1] === '"' - ? raw.slice(1, -1).replace(/\\(["\\])/g, '$1') - : raw; - return { value, inherited }; + return { value: m[2].replace(/\r?\n+$/, ''), inherited: m[1] === '*' }; } function parseProbeOutput(stdout) { @@ -158,15 +153,21 @@ function buildRestoreOptionSegment(target, name, value, opts = {}) { return `set -t ${target} ${name} ${opts.quote ? shellSingleQuote(value) : value}`; } -function buildRestoreCommand(socket, target, pre) { +// titlesOnly — see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) +function buildRestoreCommand(socket, target, pre, opts = {}) { const p = pre || {}; - const segments = [ - buildRestoreOptionSegment(target, 'status', p.status), - buildRestoreOptionSegment(target, 'mouse', p.mouse), - buildRestoreOptionSegment(target, 'window-size', p.windowSize), + const titleSegments = [ buildRestoreOptionSegment(target, 'set-titles', p.setTitles), buildRestoreOptionSegment(target, 'set-titles-string', p.setTitlesString, { quote: true }), ]; + const segments = opts.titlesOnly + ? titleSegments + : [ + buildRestoreOptionSegment(target, 'status', p.status), + buildRestoreOptionSegment(target, 'mouse', p.mouse), + buildRestoreOptionSegment(target, 'window-size', p.windowSize), + ...titleSegments, + ]; return `tmux -S '${socket}' ${segments.join(' \\; ')}`; } @@ -362,15 +363,13 @@ function createTmuxAttachAdapter(opts = {}) { if (detaching || !alive) return; detaching = true; try { raw.kill(); } catch {} - if (solo) { - // best-effort restore — see .ai/contexts/session-cache.md ("solo attach parity, issue #253") - try { - const restoreCmd = buildRestoreCommand(discovery.socket, parsed.target, discovery.pre); - Promise.resolve(runRemoteCommand(alias, restoreCmd, { timeoutMs: DEFAULT_PROBE_TIMEOUT_MS })) - .catch((err) => log.warn(`[remote-attach:${alias}] restore-on-detach failed: ${err && err.message}`)); - } catch (err) { - log.warn(`[remote-attach:${alias}] restore-on-detach failed: ${err && err.message}`); - } + // best-effort restore, every detach — see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) + try { + const restoreCmd = buildRestoreCommand(discovery.socket, parsed.target, discovery.pre, { titlesOnly: !solo }); + Promise.resolve(runRemoteCommand(alias, restoreCmd, { timeoutMs: DEFAULT_PROBE_TIMEOUT_MS })) + .catch((err) => log.warn(`[remote-attach:${alias}] restore-on-detach failed: ${err && err.message}`)); + } catch (err) { + log.warn(`[remote-attach:${alias}] restore-on-detach failed: ${err && err.message}`); } } diff --git a/test/remote-attach.test.js b/test/remote-attach.test.js index b16c1639..c22d6468 100644 --- a/test/remote-attach.test.js +++ b/test/remote-attach.test.js @@ -145,24 +145,26 @@ test('parseProbeOutput reports setTitles null when absent or inherited (starred) }); // Host measurement (tmux 3.6): the default set-titles-string is -// `#S:#I:#W - "#T" #{session_alerts}`, which `show-options -A` quotes and -// backslash-escapes because it contains spaces and embedded double quotes -- -// see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles). -const DEFAULT_SET_TITLES_STRING = '#S:#I:#W - "#T" #{session_alerts}'; -const QUOTED_DEFAULT_SET_TITLES_STRING = '"#S:#I:#W - \\"#T\\" #{session_alerts}"'; - -test('parseProbeOutput unescapes a quoted set-titles-string value with embedded quotes and spaces', () => { +// `#S:#I:#W - "#T" #{session_alerts}`, and `show-options -A` prints it in +// tmux's own re-parsable quoting: `"#S:#I:#W - \"#T\" #{session_alerts}"`. +// `pre.setTitlesString` is kept as this RAW printed token, unmodified -- +// restoring it is shell-single-quoting only, leaving tmux's own parser to +// unquote it back on `set` -- see .ai/contexts/session-cache.md ("Remote +// hosts — tmux attach", set-titles). +const RAW_DEFAULT_SET_TITLES_STRING = '"#S:#I:#W - \\"#T\\" #{session_alerts}"'; + +test('parseProbeOutput keeps set-titles-string as the raw printed token, unmodified', () => { const probed = parseProbeOutput( '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '' + - PROBE_SEP + `set-titles-string ${QUOTED_DEFAULT_SET_TITLES_STRING}`, + PROBE_SEP + `set-titles-string ${RAW_DEFAULT_SET_TITLES_STRING}`, ); - assert.equal(probed.pre.setTitlesString, DEFAULT_SET_TITLES_STRING); + assert.equal(probed.pre.setTitlesString, RAW_DEFAULT_SET_TITLES_STRING, 'no unescaping -- the raw token is kept as printed'); }); test('parseProbeOutput reports setTitlesString null when starred (inherited)', () => { const probed = parseProbeOutput( '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '' + - PROBE_SEP + `set-titles-string* ${QUOTED_DEFAULT_SET_TITLES_STRING}`, + PROBE_SEP + `set-titles-string* ${RAW_DEFAULT_SET_TITLES_STRING}`, ); assert.equal(probed.pre.setTitlesString, null); }); @@ -572,27 +574,30 @@ test('buildRestoreCommand: mixes "set" and "set -u" per option independently', ( ); }); -// issue #290 -- the round trip must reproduce the default set-titles-string -// (spaces and embedded double quotes) byte for byte. No real shell runs here, -// so this test undoes the single-quoting itself (bash/sh single-quote -// removal is: strip the wrapping quotes, "'\''" becomes "'") to prove the -// segment, once shell-unquoted, is exactly the probed value. +// issue #290 -- restore only shell-single-quotes the raw probed token; it +// never re-interprets tmux's own quoting (that unquoting is left to tmux's +// own parser when `set` receives it). No real shell runs here, so this test +// undoes the single-quoting itself (bash/sh single-quote removal is: strip +// the wrapping quotes, "'\''" becomes "'") to prove the segment, once +// shell-unquoted, carries the exact same byte sequence the probe produced. function unshellSingleQuote(word) { assert.equal(word[0], "'", `expected a single-quoted word: ${word}`); assert.equal(word[word.length - 1], "'", `expected a single-quoted word: ${word}`); return word.slice(1, -1).replace(/'\\''/g, "'"); } -test('buildRestoreCommand: set-titles-string quoting round-trips the default tmux value byte for byte', () => { - const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { - status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: DEFAULT_SET_TITLES_STRING, - }); +test('buildRestoreCommand: set-titles-string round-trips the raw probed token byte for byte, quoting only', () => { + const probed = parseProbeOutput( + '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '' + + PROBE_SEP + `set-titles-string ${RAW_DEFAULT_SET_TITLES_STRING}`, + ); + const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', probed.pre); const m = /set -t main:@0\.%0 set-titles-string (.+)$/.exec(cmd); assert.ok(m, `restore command must set-titles-string: ${cmd}`); - assert.equal(unshellSingleQuote(m[1]), DEFAULT_SET_TITLES_STRING); + assert.equal(unshellSingleQuote(m[1]), RAW_DEFAULT_SET_TITLES_STRING, 'the probe output token must reappear byte for byte inside single quotes'); }); -test('buildRestoreCommand: set-titles-string quoting round-trips a value containing a literal single quote', () => { +test('buildRestoreCommand: set-titles-string quoting handles a raw token containing a literal single quote', () => { const withQuote = `it's "#T"`; const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: withQuote, @@ -623,12 +628,13 @@ test('no builder ever emits a backtick', () => { buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0', { solo: true }), buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0', { solo: false }), buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { - status: 'on', mouse: 'off', windowSize: 'manual', setTitles: 'on', setTitlesString: DEFAULT_SET_TITLES_STRING, + status: 'on', mouse: 'off', windowSize: 'manual', setTitles: 'on', setTitlesString: RAW_DEFAULT_SET_TITLES_STRING, }), buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: null, }), buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', {}), + buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { setTitles: 'on', setTitlesString: RAW_DEFAULT_SET_TITLES_STRING }, { titlesOnly: true }), buildRemoteCommandArgs('vps', 'echo hi').join(' '), ]; for (const cmd of commands) { @@ -754,7 +760,7 @@ test('detach() runs a best-effort restore call with the probed pre-attach values } return { code: 0, - stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}set-titles on${PROBE_SEP}set-titles-string ${QUOTED_DEFAULT_SET_TITLES_STRING}${PROBE_SEP}0`, + stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}set-titles on${PROBE_SEP}set-titles-string ${RAW_DEFAULT_SET_TITLES_STRING}${PROBE_SEP}0`, stderr: '', }; }; @@ -777,14 +783,18 @@ test('detach() runs a best-effort restore call with the probed pre-attach values assert.equal( restoreCalls[0], "tmux -S '/tmp/tmux-0/test' set -t main:@0.%0 status on \\; set -t main:@0.%0 mouse off \\; set -t main:@0.%0 window-size manual \\; " + - `set -t main:@0.%0 set-titles on \\; set -t main:@0.%0 set-titles-string '${DEFAULT_SET_TITLES_STRING}'`, - 'the set-titles-string round trip must reproduce the original value byte for byte', + `set -t main:@0.%0 set-titles on \\; set -t main:@0.%0 set-titles-string '${RAW_DEFAULT_SET_TITLES_STRING}'`, + 'solo detach must restore all five options, the raw set-titles-string token reappearing byte for byte', ); }); -// Shared attach must never restore anything on detach -- it never changed -// anything in the first place, and another client's view must not move. -test('detach() sends no restore call when shared', async () => { +// issue #290 (MAJOR) -- a shared attach still turns title forwarding on +// (see buildAttachCommand), so leaving the session at `set-titles on` / +// `'#T'` forever after the first shared attach would silently ratchet the +// baseline every later probe restores against. A shared detach must restore +// the two title options -- and only those two, since status/mouse/window-size +// were never touched in shared mode and must stay untouched. +test('detach() restores only set-titles/set-titles-string on a shared detach, leaving status/mouse/window-size untouched', async () => { const raw = fakeRawPty(); const restoreCalls = []; const runRemoteCommand = async (alias, command) => { @@ -794,7 +804,7 @@ test('detach() sends no restore call when shared', async () => { } return { code: 0, - stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}${PROBE_SEP}${PROBE_SEP}1`, + stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}set-titles* off${PROBE_SEP}${PROBE_SEP}1`, stderr: '', }; }; @@ -812,7 +822,12 @@ test('detach() sends no restore call when shared', async () => { await Promise.resolve(); await Promise.resolve(); - assert.equal(restoreCalls.length, 0, 'a shared attach must never send a restore call on detach'); + assert.equal(restoreCalls.length, 1, 'a shared detach must still restore the title options'); + assert.equal( + restoreCalls[0], + "tmux -S '/tmp/tmux-0/test' set -u -t main:@0.%0 set-titles \\; set -u -t main:@0.%0 set-titles-string", + 'shared detach restores only the two title options, using set -u since both were inherited (starred) before attach', + ); }); // A restore-on-detach failure must never throw out of kill()/detach(), and From 0fa43e52759ccee3ee9ac64881137b5f58714c16 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 14 Sep 2026 08:35:01 +0200 Subject: [PATCH 3/5] (remote): parse tmux's escaped option output instead of feeding the token back tmux never re-parses an argv value, so the printed token cannot be handed back as-is; the parser now reverses the escapes tmux 3.6 actually emits (outer quote pair, \n, \t, backslash-any), pinned by a table of twenty values measured on the host. --- .ai/contexts/session-cache.md | 70 +++++++++++++++------- remote-attach.js | 24 +++++++- test/remote-attach.test.js | 106 +++++++++++++++++++--------------- 3 files changed, 133 insertions(+), 67 deletions(-) diff --git a/.ai/contexts/session-cache.md b/.ai/contexts/session-cache.md index 939644d5..5e92e722 100644 --- a/.ai/contexts/session-cache.md +++ b/.ai/contexts/session-cache.md @@ -882,27 +882,55 @@ Launching a new remote session (#222) and injection over the messaging socket for a value the CLI itself writes to disk. - **`set-titles-string` is a string option, not a bare word like the other three — a dedicated parser and a dedicated restore quoting path - exist because of it.** `tmux show-options -A` quotes a string-valued - option in double quotes whenever it contains spaces, backslash-escaping - any embedded `"` or `\` (this is why the measured default prints as - `` "#S:#I:#W - \"#T\" #{session_alerts}" `` on the wire) — `bare \S+` - parsing (used for `status`/`mouse`/`window-size`) cannot capture a value - with spaces at all. `parseTitleStringToken` matches to the end of the - segment instead and keeps **the raw printed token exactly as tmux wrote - it** (`"#S:#I:#W - \"#T\" #{session_alerts}"`, quote marks and backslash - escapes included as literal characters) — it does not unquote or - unescape it; starred (inherited) still reads as `null`, same rule as the - other three. On restore, `buildRestoreCommand` puts that raw token back - with **shell single-quoting only** (`''`, any embedded `'` - escaped as `'\''`) and lets **tmux's own command-line parser** undo - tmux's own quoting when `set -t set-titles-string ` - receives it — the same division of labor as `#T` above (the shell only - protects the argument from word-splitting and the `#`-comment rule, - tmux does the rest). Attempting to unescape and re-serialize the value - in this adapter would risk a mismatch against tmux's actual quoting - rules; forwarding the untouched raw token sidesteps that risk entirely. - Proven in `test/remote-attach.test.js`: a probed raw token reappears - byte for byte inside the restore command's single quotes. + exist because of it.** `tmux show-options -A` prints a string-valued + option in tmux's own escaped form whenever it contains characters that + need it — quoting with `'` or `"` and backslash-escaping — which `bare + \S+` parsing (used for `status`/`mouse`/`window-size`) cannot capture at + all once the value contains a space. `parseTitleStringToken` matches to + the end of the segment, then **unescapes it into the real value** (the + quote marks and backslashes are tmux's printing artifact, not part of + the option's actual content); starred (inherited) still reads as `null`, + same rule as the other three. + - **Corrected 2026-09-14 — the first version of this fix was wrong, + caught by live measurement on tmux 3.6 (a throwaway server).** It kept + the raw printed token untouched and restored it as `set -t + set-titles-string ''`, + reasoning that tmux's own command-line parser would undo tmux's own + quoting the way it does for a `.tmux.conf` line. Measured instead: + `tmux set -t t set-titles-string '"#S:#I:#W - \"#T\" #{session_alerts}"'` + stores the value **literally**, quote marks, backslashes and all — + `show-options` then prints it back double-escaped + (`` "\"#S:#I:#W - \\"#T\\" #{session_alerts}\"" ``). **An argv value + tmux receives on its own command line is never re-parsed through + tmux's config-file/command-prompt quoting** — only the text `tmux + show-options` *prints* goes through that quoting, to make it + re-typeable at the `:` prompt or in a `.tmux.conf` line, not to be + re-quoted proof against a shell-passed argv. The fix is to do the + unescaping ourselves: `parseTitleStringToken` reverses tmux's printed + form back to the real value, and `buildRestoreCommand` sends that real + value back with **shell single-quoting only** (`''`, any + embedded `'` escaped as `'\''`), which tmux then stores literally — + confirmed on the host (`set -t t set-titles-string '#T'` then + `show-options` prints `"#T"`; `set -u` brings back the inherited + default). + - **The unescaping rule, reverse-engineered from a 20-case table + measured live on tmux 3.6, 2026-09-14** (`SET_TITLES_STRING_CASES` in + `test/remote-attach.test.js`, one input value per row, mapped to + exactly what `show-options` printed for it): if the token starts and + ends with the same quote character (`'` or `"`, length ≥ 2), strip + that outer pair; then scan left to right unescaping `\n`→LF, `\t`→TAB, + and `\`→that char (drop the backslash). This one rule + reproduces every measured case without needing to model *why* tmux + picked a given wrapper quote or which characters it decided to escape + (space/`;`/`$`-before-a-name-char/quotes/non-ASCII trigger quoting; + `~`/bare backslash/bare LF/TAB do not; the wrapper quote is whichever + of `'`/`"` avoids escaping an embedded quote of that kind, defaulting + to `"` when both or neither are present) — the strip-then-unescape + algorithm is symmetric to whichever choice tmux made. Proven in + `test/remote-attach.test.js`, table-driven over the 20 measured pairs: + `parseProbeOutput` on tmux's printed form recovers the original input, + and `buildRestoreCommand`'s `set-titles-string` segment is exactly + `set -t set-titles-string ` + `shellSingleQuote(input)`. - **Restore now runs on every detach, shared included — solo restores all five options, shared restores only the two title options.** Earlier this fix gated the whole restore on `solo`, matching `status`/`mouse`/ diff --git a/remote-attach.js b/remote-attach.js index a50f79c3..81f2c3b0 100644 --- a/remote-attach.js +++ b/remote-attach.js @@ -40,12 +40,33 @@ function parseOptionToken(part, name) { return { value: m[2], inherited: m[1] === '*' }; } +// see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) +function unescapeTmuxOptionString(raw) { + let body = raw; + if (body.length >= 2 && (body[0] === '"' || body[0] === "'") && body[body.length - 1] === body[0]) { + body = body.slice(1, -1); + } + let out = ''; + for (let i = 0; i < body.length; i++) { + const c = body[i]; + if (c === '\\' && i + 1 < body.length) { + const next = body[i + 1]; + out += next === 'n' ? '\n' : next === 't' ? '\t' : next; + i++; + } else { + out += c; + } + } + return out; +} + // see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) function parseTitleStringToken(part, name) { const re = new RegExp(`${escapeRegExpLiteral(name)}(\\*?)\\s+([\\s\\S]*)`); const m = re.exec(part || ''); if (!m) return { value: null, inherited: false }; - return { value: m[2].replace(/\r?\n+$/, ''), inherited: m[1] === '*' }; + const raw = m[2].replace(/\r?\n+$/, ''); + return { value: unescapeTmuxOptionString(raw), inherited: m[1] === '*' }; } function parseProbeOutput(stdout) { @@ -411,6 +432,7 @@ module.exports = { buildAttachCommand, buildRestoreCommand, buildRemoteCommandArgs, + shellSingleQuote, isValidPid, buildProcCmdlineCheck, defaultRunRemoteCommand, diff --git a/test/remote-attach.test.js b/test/remote-attach.test.js index c22d6468..80ec9887 100644 --- a/test/remote-attach.test.js +++ b/test/remote-attach.test.js @@ -22,6 +22,7 @@ const { buildAttachCommand, buildRestoreCommand, buildRemoteCommandArgs, + shellSingleQuote, } = require('../remote-attach'); const { classifyTitleActivity } = require('../classify-title-activity'); @@ -147,24 +148,24 @@ test('parseProbeOutput reports setTitles null when absent or inherited (starred) // Host measurement (tmux 3.6): the default set-titles-string is // `#S:#I:#W - "#T" #{session_alerts}`, and `show-options -A` prints it in // tmux's own re-parsable quoting: `"#S:#I:#W - \"#T\" #{session_alerts}"`. -// `pre.setTitlesString` is kept as this RAW printed token, unmodified -- -// restoring it is shell-single-quoting only, leaving tmux's own parser to -// unquote it back on `set` -- see .ai/contexts/session-cache.md ("Remote -// hosts — tmux attach", set-titles). -const RAW_DEFAULT_SET_TITLES_STRING = '"#S:#I:#W - \\"#T\\" #{session_alerts}"'; +// `parseTitleStringToken` reverses that quoting (see the full escaping table +// below) -- see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", +// set-titles). +const DEFAULT_SET_TITLES_STRING = '#S:#I:#W - "#T" #{session_alerts}'; +const PRINTED_DEFAULT_SET_TITLES_STRING = '"#S:#I:#W - \\"#T\\" #{session_alerts}"'; -test('parseProbeOutput keeps set-titles-string as the raw printed token, unmodified', () => { +test('parseProbeOutput unescapes the default set-titles-string back to its real value', () => { const probed = parseProbeOutput( '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '' + - PROBE_SEP + `set-titles-string ${RAW_DEFAULT_SET_TITLES_STRING}`, + PROBE_SEP + `set-titles-string ${PRINTED_DEFAULT_SET_TITLES_STRING}`, ); - assert.equal(probed.pre.setTitlesString, RAW_DEFAULT_SET_TITLES_STRING, 'no unescaping -- the raw token is kept as printed'); + assert.equal(probed.pre.setTitlesString, DEFAULT_SET_TITLES_STRING); }); test('parseProbeOutput reports setTitlesString null when starred (inherited)', () => { const probed = parseProbeOutput( '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '' + - PROBE_SEP + `set-titles-string* ${RAW_DEFAULT_SET_TITLES_STRING}`, + PROBE_SEP + `set-titles-string* ${PRINTED_DEFAULT_SET_TITLES_STRING}`, ); assert.equal(probed.pre.setTitlesString, null); }); @@ -574,38 +575,53 @@ test('buildRestoreCommand: mixes "set" and "set -u" per option independently', ( ); }); -// issue #290 -- restore only shell-single-quotes the raw probed token; it -// never re-interprets tmux's own quoting (that unquoting is left to tmux's -// own parser when `set` receives it). No real shell runs here, so this test -// undoes the single-quoting itself (bash/sh single-quote removal is: strip -// the wrapping quotes, "'\''" becomes "'") to prove the segment, once -// shell-unquoted, carries the exact same byte sequence the probe produced. -function unshellSingleQuote(word) { - assert.equal(word[0], "'", `expected a single-quoted word: ${word}`); - assert.equal(word[word.length - 1], "'", `expected a single-quoted word: ${word}`); - return word.slice(1, -1).replace(/'\\''/g, "'"); -} - -test('buildRestoreCommand: set-titles-string round-trips the raw probed token byte for byte, quoting only', () => { - const probed = parseProbeOutput( - '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '' + - PROBE_SEP + `set-titles-string ${RAW_DEFAULT_SET_TITLES_STRING}`, - ); - const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', probed.pre); - const m = /set -t main:@0\.%0 set-titles-string (.+)$/.exec(cmd); - assert.ok(m, `restore command must set-titles-string: ${cmd}`); - assert.equal(unshellSingleQuote(m[1]), RAW_DEFAULT_SET_TITLES_STRING, 'the probe output token must reappear byte for byte inside single quotes'); -}); - -test('buildRestoreCommand: set-titles-string quoting handles a raw token containing a literal single quote', () => { - const withQuote = `it's "#T"`; - const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { - status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: withQuote, +// issue #290, corrected 2026-09-14 -- live measurement on tmux 3.6 (a +// throwaway server) proved the earlier "keep the raw token, let tmux +// re-parse it" restore approach wrong: `set -t t set-titles-string +// '""'` stores the quotes and backslashes LITERALLY -- +// tmux's argv is never re-parsed by tmux's own quoting rules. Each `printed` +// value below is exactly what `show-options -t t set-titles-string` printed +// after the option name for that `input` value on that host. See +// .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles). +const SET_TITLES_STRING_CASES = [ + { name: 'plain', input: `plain`, printed: `plain` }, + { name: 'space', input: `has space`, printed: `"has space"` }, + { name: 'double quote', input: `dq"in`, printed: `'dq"in'` }, + { name: 'backslash', input: `bs\\in`, printed: `bs\\\\in` }, + { name: 'dollar before name char', input: `dollar$x`, printed: `"dollar\\$x"` }, + { name: 'semicolon', input: `semi;colon`, printed: `"semi;colon"` }, + { name: 'tilde', input: `tilde~x`, printed: `tilde~x` }, + { name: 'single quote', input: `sq'in`, printed: `"sq'in"` }, + { name: 'hash', input: `hash#T`, printed: `"hash#T"` }, + { name: 'unicode', input: `uni é⠋`, printed: `"uni é⠋"` }, + { name: 'newline', input: `nl\nx`, printed: `nl\\nx` }, + { name: 'tab', input: `tab\tx`, printed: `tab\\tx` }, + { name: 'trailing backslash', input: `trail\\`, printed: `trail\\\\` }, + { name: 'single and double quote', input: `both'and"q`, printed: `"both'and\\"q"` }, + { name: 'double quote then trailing dollar (unescaped)', input: `dq"and$`, printed: `"dq\\"and$"` }, + { name: 'single quote then backslash', input: `sq'and\\bs`, printed: `"sq'and\\\\bs"` }, + { name: 'double quote then backslash', input: `dq"bs\\x`, printed: `'dq"bs\\\\x'` }, + { name: 'double quote then newline', input: `dq"nl\nx`, printed: `'dq"nl\\nx'` }, + { name: 'double quote then dollar before name char', input: `dq"dollar$x`, printed: `"dq\\"dollar\\$x"` }, + { name: 'newline then double quote', input: `nl\nx"dq`, printed: `'nl\\nx"dq'` }, +]; + +for (const { name, input, printed } of SET_TITLES_STRING_CASES) { + test(`set-titles-string escaping (${name}): parse(printed) recovers the input, and restore re-quotes the input`, () => { + const probed = parseProbeOutput( + '200x50' + PROBE_SEP + 'status on' + PROBE_SEP + '' + PROBE_SEP + '' + PROBE_SEP + '' + + PROBE_SEP + `set-titles-string ${printed}`, + ); + assert.equal(probed.pre.setTitlesString, input, `parse(${JSON.stringify(printed)}) must recover the original input`); + + const cmd = buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { + status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: input, + }); + const m = /set -t main:@0\.%0 set-titles-string ([\s\S]+)$/.exec(cmd); + assert.ok(m, `restore command must set set-titles-string: ${cmd}`); + assert.equal(m[1], shellSingleQuote(input), 'the restore segment must be exactly set-titles-string + shellSingleQuote(input)'); }); - const m = /set -t main:@0\.%0 set-titles-string (.+)$/.exec(cmd); - assert.ok(m, `restore command must set-titles-string: ${cmd}`); - assert.equal(unshellSingleQuote(m[1]), withQuote); -}); +} // issue #290 -- the probe must read back both new options so a restore has // something to put back. @@ -628,13 +644,13 @@ test('no builder ever emits a backtick', () => { buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0', { solo: true }), buildAttachCommand('/tmp/tmux-0/main', 'main:@0.%0', { solo: false }), buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { - status: 'on', mouse: 'off', windowSize: 'manual', setTitles: 'on', setTitlesString: RAW_DEFAULT_SET_TITLES_STRING, + status: 'on', mouse: 'off', windowSize: 'manual', setTitles: 'on', setTitlesString: DEFAULT_SET_TITLES_STRING, }), buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: null, }), buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', {}), - buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { setTitles: 'on', setTitlesString: RAW_DEFAULT_SET_TITLES_STRING }, { titlesOnly: true }), + buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { setTitles: 'on', setTitlesString: DEFAULT_SET_TITLES_STRING }, { titlesOnly: true }), buildRemoteCommandArgs('vps', 'echo hi').join(' '), ]; for (const cmd of commands) { @@ -760,7 +776,7 @@ test('detach() runs a best-effort restore call with the probed pre-attach values } return { code: 0, - stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}set-titles on${PROBE_SEP}set-titles-string ${RAW_DEFAULT_SET_TITLES_STRING}${PROBE_SEP}0`, + stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}set-titles on${PROBE_SEP}set-titles-string ${PRINTED_DEFAULT_SET_TITLES_STRING}${PROBE_SEP}0`, stderr: '', }; }; @@ -783,8 +799,8 @@ test('detach() runs a best-effort restore call with the probed pre-attach values assert.equal( restoreCalls[0], "tmux -S '/tmp/tmux-0/test' set -t main:@0.%0 status on \\; set -t main:@0.%0 mouse off \\; set -t main:@0.%0 window-size manual \\; " + - `set -t main:@0.%0 set-titles on \\; set -t main:@0.%0 set-titles-string '${RAW_DEFAULT_SET_TITLES_STRING}'`, - 'solo detach must restore all five options, the raw set-titles-string token reappearing byte for byte', + `set -t main:@0.%0 set-titles on \\; set -t main:@0.%0 set-titles-string ${shellSingleQuote(DEFAULT_SET_TITLES_STRING)}`, + 'solo detach must restore all five options, set-titles-string unescaped from the probe and re-quoted for the shell', ); }); From d954b5b90c82eb7d70b11020d0ea4ddb5794291c Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 14 Sep 2026 08:56:04 +0200 Subject: [PATCH 4/5] (remote): restore the title options on detach only when no other client remains Two clients attached to the same tmux session would otherwise switch title forwarding off under each other. Detach probes the live client count and keeps the title options when another client is still there; the base options keep the solo rule. Three more measured quoting rows in the table. --- .ai/contexts/session-cache.md | 114 +++++++++++++++++++++++++------- remote-attach.js | 78 +++++++++++++++------- test/remote-attach.test.js | 120 +++++++++++++++++++++++++++++++++- 3 files changed, 266 insertions(+), 46 deletions(-) diff --git a/.ai/contexts/session-cache.md b/.ai/contexts/session-cache.md index 5e92e722..3fd592b7 100644 --- a/.ai/contexts/session-cache.md +++ b/.ai/contexts/session-cache.md @@ -913,11 +913,13 @@ Launching a new remote session (#222) and injection over the messaging socket confirmed on the host (`set -t t set-titles-string '#T'` then `show-options` prints `"#T"`; `set -u` brings back the inherited default). - - **The unescaping rule, reverse-engineered from a 20-case table + - **The unescaping rule, reverse-engineered from a 23-case table measured live on tmux 3.6, 2026-09-14** (`SET_TITLES_STRING_CASES` in `test/remote-attach.test.js`, one input value per row, mapped to - exactly what `show-options` printed for it): if the token starts and - ends with the same quote character (`'` or `"`, length ≥ 2), strip + exactly what `show-options` printed for it — 20 rows from the first + measurement pass, 3 more added later: empty string, and a value that + is itself entirely wrapped in `'...'` or `"..."`): if the token starts + and ends with the same quote character (`'` or `"`, length ≥ 2), strip that outer pair; then scan left to right unescaping `\n`→LF, `\t`→TAB, and `\`→that char (drop the backslash). This one rule reproduces every measured case without needing to model *why* tmux @@ -926,26 +928,94 @@ Launching a new remote session (#222) and injection over the messaging socket `~`/bare backslash/bare LF/TAB do not; the wrapper quote is whichever of `'`/`"` avoids escaping an embedded quote of that kind, defaulting to `"` when both or neither are present) — the strip-then-unescape - algorithm is symmetric to whichever choice tmux made. Proven in - `test/remote-attach.test.js`, table-driven over the 20 measured pairs: - `parseProbeOutput` on tmux's printed form recovers the original input, - and `buildRestoreCommand`'s `set-titles-string` segment is exactly - `set -t set-titles-string ` + `shellSingleQuote(input)`. + algorithm is symmetric to whichever choice tmux made, which is also + why the 3 later rows needed no rule change: a value that already + looks quoted on the outside is still just one more case of "matching + outer pair, strip it." Proven in `test/remote-attach.test.js`, + table-driven over the 23 measured pairs: `parseProbeOutput` on tmux's + printed form recovers the original input; `buildRestoreCommand`'s + `set-titles-string` segment is exactly `set -t + set-titles-string ` + `shellSingleQuote(input)`; and, independent of + the production encoder, the segment is structurally checked to start + and end with `'` and to carry no bare (unescaped) `'` once every + `'\''` escape is removed. - **Restore now runs on every detach, shared included — solo restores all - five options, shared restores only the two title options.** Earlier this - fix gated the whole restore on `solo`, matching `status`/`mouse`/ - `window-size` — but since the `set` for titles now also fires in shared - mode, that left a **baseline ratchet**: after one shared attach the - session stayed at `set-titles on` / `set-titles-string '#T'` forever, - and every later probe would read that back as the pre-existing baseline - to restore *to*, permanently losing whatever the session had before its - first shared attach. `buildRestoreCommand(socket, target, pre, { - titlesOnly })` now takes a `titlesOnly` option; `detach()` always fires - a restore call, passing `titlesOnly: !solo` — a solo detach restores - `status`/`mouse`/`window-size`/`set-titles`/`set-titles-string` exactly - as before, a shared detach restores only `set-titles`/`set-titles-string` - (never `status`/`mouse`/`window-size`, which a shared attach never - touched in the first place). + five options, shared restores only the two title options, subject to the + live-client-count gate below.** Earlier this fix gated the whole restore + on `solo`, matching `status`/`mouse`/`window-size` — but since the `set` + for titles now also fires in shared mode, that left a **baseline + ratchet**: after one shared attach the session stayed at `set-titles on` + / `set-titles-string '#T'` forever, and every later probe would read + that back as the pre-existing baseline to restore *to*, permanently + losing whatever the session had before its first shared attach. + `buildRestoreCommand(socket, target, pre, { includeBase, includeTitles })` + takes two independent flags (each defaults `true`) instead of one + `titlesOnly` switch — three of the four combinations are real: + both true (today's solo full restore), titles-only (today's shared + restore), and base-only (new, see below); all-false returns `null` + (nothing to send) rather than an empty `tmux -S '' ` command. + `detach()` always calls it, computing `includeBase: solo` (unchanged -- + still exactly the pre-#290 solo rule) and `includeTitles` from the + live-client-count probe immediately below. + - **Multi-client race, follow-up fix.** Two Switchboard clients attached to + the same remote tmux session (two machines, or two windows) raced each + other's title restore: client A detaching restored `set-titles`/ + `set-titles-string` to A's own probed pre-attach baseline, turning + forwarding off (or back to A's idea of "before") **while client B was + still attached and relying on it** — B's row would go mute mid-session + with no detach of its own. Symmetrically, if B detached afterwards, B's + own restore (based on B's probed baseline, captured while A's forwarding + was already on) could re-apply `on`/`'#T'` *after* A had genuinely + restored the pristine original, leaving the session's title-forwarding + state permanently wrong relative to what it was before either client + ever attached. Fix, bounded to the title options only: right before + building the restore command, `detach()` runs one more small, + non-interactive probe — `buildClientCountProbeCommand(socket, target)` + (`tmux -S '' list-clients -t 2>/dev/null | wc -l`, + the same `list-clients` query and `parseClientCount()` the attach-time + probe already uses, just against the socket/target this call already + has rather than rediscovering them) — and sets `includeTitles: count <= + 1` (our own about-to-close client may still show up in the count, hence + `<= 1` and not `=== 0`). `count > 1` means at least one other real + client is still attached: the title restore is skipped entirely and + logged at debug (`log.debug`, falling back to `log.info` if the + injected `log` carries no `debug()`), never at warn — this is an + expected, routine outcome, not a failure. **`status`/`mouse`/ + `window-size` are untouched by this fix** — `includeBase` stays exactly + `solo`, per the instruction that started this fix: those three already + had their own correct-by-construction rule (a shared attach never sets + them, so a shared detach has nothing to put back), and reopening that + rule was out of scope. **If the client-count probe itself fails or + returns something `parseClientCount` can't parse, `includeTitles` falls + back to `true`** — restoring is the pre-existing (safe-by-comparison) + behavior, and a probe failure is not treated as evidence that someone + else is still attached. + - **What this coordinates, and what it deliberately does not.** This + makes "the last client out restores the title options" hold in the + common case (a probe running right before the restore catches almost + every real multi-client overlap). It does **not** make the two + options fully consistent across an overlapping multi-client episode + in every case — **no cross-instance state exists**: two Switchboard + processes (or two hand-run `ssh`/`tmux attach` sessions) never + coordinate with each other directly, only through what the live + `list-clients` count happens to read at the moment of each one's own + detach, which is inherently racy between concurrent detaches. The + accepted residual gap: a **solo** attach that happens *after* a + multi-client episode probes whatever `set-titles`/`set-titles-string` + were left at (typically `on`/`'#T'`, since some client's shared + attach turned them on and no detach happened to be the qualifying + "last one out") as *its own* pre-attach baseline, and will faithfully + restore back to that value on its own later detach — carrying forward + a value that was arguably never the session's true original. This + residual is bounded to the two title options (never `status`/`mouse`/ + `window-size`) and is accepted rather than solved here: solving it + fully would need either a shared external ledger of "what was here + before anyone touched it" or a lock across attach attempts, both out + of scope for a fix whose brief was "bounded and simple." Proven in + `test/remote-attach.test.js`: detach-time count 0 or 1 still restores + the titles; count 2 restores only `status`/`mouse`/`window-size` (the + solo case) and skips the title segment, logging why; a failing + client-count probe falls back to restoring the titles. - **This is the first thing to populate the session-handle seam from issue #220** (see `.ai/contexts/trigger-watcher.md`, "Session handle"): a diff --git a/remote-attach.js b/remote-attach.js index 81f2c3b0..09a776a2 100644 --- a/remote-attach.js +++ b/remote-attach.js @@ -174,22 +174,26 @@ function buildRestoreOptionSegment(target, name, value, opts = {}) { return `set -t ${target} ${name} ${opts.quote ? shellSingleQuote(value) : value}`; } -// titlesOnly — see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) +// includeBase/includeTitles — see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) function buildRestoreCommand(socket, target, pre, opts = {}) { const p = pre || {}; - const titleSegments = [ - buildRestoreOptionSegment(target, 'set-titles', p.setTitles), - buildRestoreOptionSegment(target, 'set-titles-string', p.setTitlesString, { quote: true }), - ]; - const segments = opts.titlesOnly - ? titleSegments - : [ - buildRestoreOptionSegment(target, 'status', p.status), - buildRestoreOptionSegment(target, 'mouse', p.mouse), - buildRestoreOptionSegment(target, 'window-size', p.windowSize), - ...titleSegments, - ]; - return `tmux -S '${socket}' ${segments.join(' \\; ')}`; + const includeBase = opts.includeBase !== false; + const includeTitles = opts.includeTitles !== false; + const segments = []; + if (includeBase) { + segments.push( + buildRestoreOptionSegment(target, 'status', p.status), + buildRestoreOptionSegment(target, 'mouse', p.mouse), + buildRestoreOptionSegment(target, 'window-size', p.windowSize), + ); + } + if (includeTitles) { + segments.push( + buildRestoreOptionSegment(target, 'set-titles', p.setTitles), + buildRestoreOptionSegment(target, 'set-titles-string', p.setTitlesString, { quote: true }), + ); + } + return segments.length ? `tmux -S '${socket}' ${segments.join(' \\; ')}` : null; } // see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", solo vs shared) @@ -200,6 +204,11 @@ function parseClientCount(text) { return Number.parseInt(trimmed, 10); } +// see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) +function buildClientCountProbeCommand(socket, target) { + return `tmux -S '${socket}' list-clients -t ${target} 2>/dev/null | wc -l`; +} + // see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", socket discovery) // parts: [socket, size, status, mouse, window-size, set-titles, set-titles-string, // clientCount, cmdlineHasClaude]; trailing ones optional @@ -308,7 +317,12 @@ function createTmuxAttachAdapter(opts = {}) { } const runRemoteCommand = opts.runRemoteCommand || defaultRunRemoteCommand; const resolveSshPath = opts.resolveSshPath || defaultResolveSshPath; - const log = opts.log || { info() {}, warn() {}, error() {} }; + const log = opts.log || { info() {}, warn() {}, error() {}, debug() {} }; + // see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) + function logDebug(msg) { + if (typeof log.debug === 'function') log.debug(msg); + else if (typeof log.info === 'function') log.info(msg); + } /** Whether this descriptor names a multiplexer this adapter can attach to. */ function supports(descriptor) { @@ -379,19 +393,36 @@ function createTmuxAttachAdapter(opts = {}) { // loses its pty and tmux drops it, leaving the session running. Sending a // prefix keystroke instead would assume this host's prefix, and land as // literal text in the remote session on any host that remapped it. - // see .ai/contexts/session-cache.md ("Remote hosts — tmux attach") + // see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) + async function restoreOnDetach() { + let includeTitles = true; + try { + const clientProbe = await runRemoteCommand(alias, buildClientCountProbeCommand(discovery.socket, parsed.target), { timeoutMs: DEFAULT_PROBE_TIMEOUT_MS }); + if (clientProbe && clientProbe.code === 0) { + const count = parseClientCount(clientProbe.stdout); + if (count != null) includeTitles = count <= 1; + } + } catch { + // see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) + } + if (!includeTitles) { + logDebug(`[remote-attach:${alias}] skipping title restore on detach — another client is still attached to ${parsed.target}`); + } + const restoreCmd = buildRestoreCommand(discovery.socket, parsed.target, discovery.pre, { includeBase: solo, includeTitles }); + if (!restoreCmd) return; + const result = await runRemoteCommand(alias, restoreCmd, { timeoutMs: DEFAULT_PROBE_TIMEOUT_MS }); + if (!result || result.code !== 0) { + const reason = result ? `exit ${result.code}: ${(result.stderr || '').trim() || 'no stderr'}` : 'no response'; + log.warn(`[remote-attach:${alias}] restore-on-detach failed (${reason})`); + } + } + function detach() { if (detaching || !alive) return; detaching = true; try { raw.kill(); } catch {} // best-effort restore, every detach — see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) - try { - const restoreCmd = buildRestoreCommand(discovery.socket, parsed.target, discovery.pre, { titlesOnly: !solo }); - Promise.resolve(runRemoteCommand(alias, restoreCmd, { timeoutMs: DEFAULT_PROBE_TIMEOUT_MS })) - .catch((err) => log.warn(`[remote-attach:${alias}] restore-on-detach failed: ${err && err.message}`)); - } catch (err) { - log.warn(`[remote-attach:${alias}] restore-on-detach failed: ${err && err.message}`); - } + restoreOnDetach().catch((err) => log.warn(`[remote-attach:${alias}] restore-on-detach failed: ${err && err.message}`)); } const ptyProcess = { @@ -432,6 +463,7 @@ module.exports = { buildAttachCommand, buildRestoreCommand, buildRemoteCommandArgs, + buildClientCountProbeCommand, shellSingleQuote, isValidPid, buildProcCmdlineCheck, diff --git a/test/remote-attach.test.js b/test/remote-attach.test.js index 80ec9887..d0f5d7bf 100644 --- a/test/remote-attach.test.js +++ b/test/remote-attach.test.js @@ -22,6 +22,7 @@ const { buildAttachCommand, buildRestoreCommand, buildRemoteCommandArgs, + buildClientCountProbeCommand, shellSingleQuote, } = require('../remote-attach'); const { classifyTitleActivity } = require('../classify-title-activity'); @@ -604,6 +605,9 @@ const SET_TITLES_STRING_CASES = [ { name: 'double quote then newline', input: `dq"nl\nx`, printed: `'dq"nl\\nx'` }, { name: 'double quote then dollar before name char', input: `dq"dollar$x`, printed: `"dq\\"dollar\\$x"` }, { name: 'newline then double quote', input: `nl\nx"dq`, printed: `'nl\\nx"dq'` }, + { name: 'empty', input: ``, printed: `''` }, + { name: 'entirely single-quoted content', input: `'abc'`, printed: `"'abc'"` }, + { name: 'entirely double-quoted content', input: `"abc"`, printed: `'"abc"'` }, ]; for (const { name, input, printed } of SET_TITLES_STRING_CASES) { @@ -620,6 +624,15 @@ for (const { name, input, printed } of SET_TITLES_STRING_CASES) { const m = /set -t main:@0\.%0 set-titles-string ([\s\S]+)$/.exec(cmd); assert.ok(m, `restore command must set set-titles-string: ${cmd}`); assert.equal(m[1], shellSingleQuote(input), 'the restore segment must be exactly set-titles-string + shellSingleQuote(input)'); + + // Structural check independent of the production encoder: a valid shell + // single-quoted word starts and ends with `'`, and once every `'\''` + // escape is removed, no bare `'` remains inside. + const quoted = m[1]; + assert.equal(quoted[0], "'", `restore segment must start with a single quote: ${quoted}`); + assert.equal(quoted[quoted.length - 1], "'", `restore segment must end with a single quote: ${quoted}`); + const withoutEscapes = quoted.slice(1, -1).split(`'\\''`).join(''); + assert.ok(!withoutEscapes.includes("'"), `no unescaped single quote may remain inside the quoted segment: ${quoted}`); }); } @@ -635,6 +648,15 @@ test('buildProbeCommand reads both set-titles and set-titles-string', () => { assert.ok(setTitlesIdx < setTitlesStringIdx && setTitlesStringIdx < listClientsIdx, 'both option probes must precede the trailing client-count/cmdline segments'); }); +// issue #290 (follow-up) -- the small, standalone detach-time probe reuses +// the same list-clients command and the same parseClientCount() the +// attach-time probe already uses, just against the already-known socket/ +// target instead of rediscovering them. +test('buildClientCountProbeCommand builds the same list-clients query the attach-time probe uses', () => { + const cmd = buildClientCountProbeCommand('/tmp/tmux-0/main', 'main:@0.%0'); + assert.equal(cmd, "tmux -S '/tmp/tmux-0/main' list-clients -t main:@0.%0 2>/dev/null | wc -l"); +}); + // No remote command string may ever contain a backtick -- these run over ssh, // where a backtick executes (issue #253 acceptance criterion). test('no builder ever emits a backtick', () => { @@ -650,7 +672,7 @@ test('no builder ever emits a backtick', () => { status: null, mouse: null, windowSize: null, setTitles: null, setTitlesString: null, }), buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', {}), - buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { setTitles: 'on', setTitlesString: DEFAULT_SET_TITLES_STRING }, { titlesOnly: true }), + buildRestoreCommand('/tmp/tmux-0/main', 'main:@0.%0', { setTitles: 'on', setTitlesString: DEFAULT_SET_TITLES_STRING }, { includeBase: false }), buildRemoteCommandArgs('vps', 'echo hi').join(' '), ]; for (const cmd of commands) { @@ -846,6 +868,102 @@ test('detach() restores only set-titles/set-titles-string on a shared detach, le ); }); +// --- issue #290 (follow-up): two Switchboard clients attached to the same +// remote session must not race each other's title restore -- see +// .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles). + +const flushAsync = () => new Promise((resolve) => setImmediate(resolve)); + +// Builds a fake runRemoteCommand that tells apart the three kinds of calls +// detach() can now make: the detach-time client-count probe (list-clients, +// answered with a bare count), the restore call itself, and (falling through +// to the default) the original attach-time discovery probe. +function makeDetachClientCountFake({ clientCountAtDetach, clientCountProbeFails = false } = {}) { + const restoreCalls = []; + const clientCountProbeCalls = []; + const runRemoteCommand = async (alias, command) => { + if (/^tmux -S '.*' list-clients -t /.test(command)) { + clientCountProbeCalls.push(command); + if (clientCountProbeFails) return { code: 1, stdout: '', stderr: 'ssh: connection refused' }; + return { code: 0, stdout: `${clientCountAtDetach}\n`, stderr: '' }; + } + if (/^tmux -S /.test(command) && !command.includes('attach') && !/display-message|show-options|list-clients/.test(command)) { + restoreCalls.push(command); + return { code: 0, stdout: '', stderr: '' }; + } + return { + code: 0, + // discoveryClientCount 0 + a supplied localSize -> solo attach, so the + // "other three still restored" half of the fix is exercised too. + stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}set-titles on${PROBE_SEP}set-titles-string ${PRINTED_DEFAULT_SET_TITLES_STRING}${PROBE_SEP}0`, + stderr: '', + }; + }; + return { runRemoteCommand, restoreCalls, clientCountProbeCalls }; +} + +for (const count of [0, 1]) { + test(`detach() still restores the title options when the detach-time client count is ${count} ("ours may still be counted")`, async () => { + const raw = fakeRawPty(); + const { runRemoteCommand, restoreCalls, clientCountProbeCalls } = makeDetachClientCountFake({ clientCountAtDetach: count }); + const adapter = createTmuxAttachAdapter({ spawnPty: () => raw.pty, runRemoteCommand, log: silentLog }); + const result = await adapter.attach('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0' }, { cols: 100, rows: 40 }); + assert.equal(result.ok, true); + + result.ptyProcess.kill(); + await flushAsync(); + + assert.equal(clientCountProbeCalls.length, 1, 'detach must probe the live client count exactly once'); + assert.equal(restoreCalls.length, 1); + assert.match( + restoreCalls[0], + /set -t main:@0\.%0 set-titles on \\; set -t main:@0\.%0 set-titles-string/, + `title options must be restored when the detach-time count is ${count}`, + ); + }); +} + +test('detach() skips the title restore, but still restores status/mouse/window-size, when another client is attached at detach time (count 2)', async () => { + const raw = fakeRawPty(); + const logLines = []; + const log = { info() {}, warn() {}, error() {}, debug: (msg) => logLines.push(msg) }; + const { runRemoteCommand, restoreCalls, clientCountProbeCalls } = makeDetachClientCountFake({ clientCountAtDetach: 2 }); + const adapter = createTmuxAttachAdapter({ spawnPty: () => raw.pty, runRemoteCommand, log }); + const result = await adapter.attach('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0' }, { cols: 100, rows: 40 }); + assert.equal(result.ok, true); + + result.ptyProcess.kill(); + await flushAsync(); + + assert.equal(clientCountProbeCalls.length, 1); + assert.equal(restoreCalls.length, 1, 'status/mouse/window-size must still be restored -- their solo rule is unchanged by this fix'); + assert.equal( + restoreCalls[0], + "tmux -S '/tmp/tmux-0/test' set -t main:@0.%0 status on \\; set -t main:@0.%0 mouse off \\; set -t main:@0.%0 window-size manual", + 'no set-titles/set-titles-string segment when another client is still attached at detach time', + ); + assert.ok(logLines.some((l) => /skipping title restore/i.test(l)), 'must log at debug why the title restore was skipped'); +}); + +test('detach() falls back to restoring the title options when the detach-time client-count probe fails', async () => { + const raw = fakeRawPty(); + const { runRemoteCommand, restoreCalls, clientCountProbeCalls } = makeDetachClientCountFake({ clientCountAtDetach: 0, clientCountProbeFails: true }); + const adapter = createTmuxAttachAdapter({ spawnPty: () => raw.pty, runRemoteCommand, log: silentLog }); + const result = await adapter.attach('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0' }, { cols: 100, rows: 40 }); + assert.equal(result.ok, true); + + result.ptyProcess.kill(); + await flushAsync(); + + assert.equal(clientCountProbeCalls.length, 1); + assert.equal(restoreCalls.length, 1); + assert.match( + restoreCalls[0], + /set -t main:@0\.%0 set-titles on \\; set -t main:@0\.%0 set-titles-string/, + 'a failed client-count probe must fall back to restoring the titles, same as before this fix', + ); +}); + // A restore-on-detach failure must never throw out of kill()/detach(), and // must not prevent the local ssh client from being killed. test('detach() swallows a failing restore call without throwing', async () => { From 908e2b93e875ecfccfbc283bbe8b11a9453f4fc5 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Mon, 14 Sep 2026 09:13:09 +0200 Subject: [PATCH 5/5] (remote): count clients before killing the local pty so the last-client rule holds The detach-time client count ran after the local ssh client was killed and on a fresh connection, so our own client had usually vanished and a single remaining peer read as 'nobody but us'. The probe now runs first, bounded by a dedicated 5 s timeout, then the kill, then the restore. --- .ai/contexts/session-cache.md | 96 ++++++++++++++++++++++++++--------- remote-attach.js | 6 +-- test/remote-attach.test.js | 90 ++++++++++++++++++++++++++------ 3 files changed, 149 insertions(+), 43 deletions(-) diff --git a/.ai/contexts/session-cache.md b/.ai/contexts/session-cache.md index 3fd592b7..53d28371 100644 --- a/.ai/contexts/session-cache.md +++ b/.ai/contexts/session-cache.md @@ -968,28 +968,67 @@ Launching a new remote session (#222) and injection over the messaging socket was already on) could re-apply `on`/`'#T'` *after* A had genuinely restored the pristine original, leaving the session's title-forwarding state permanently wrong relative to what it was before either client - ever attached. Fix, bounded to the title options only: right before - building the restore command, `detach()` runs one more small, - non-interactive probe — `buildClientCountProbeCommand(socket, target)` - (`tmux -S '' list-clients -t 2>/dev/null | wc -l`, - the same `list-clients` query and `parseClientCount()` the attach-time - probe already uses, just against the socket/target this call already - has rather than rediscovering them) — and sets `includeTitles: count <= - 1` (our own about-to-close client may still show up in the count, hence - `<= 1` and not `=== 0`). `count > 1` means at least one other real - client is still attached: the title restore is skipped entirely and - logged at debug (`log.debug`, falling back to `log.info` if the - injected `log` carries no `debug()`), never at warn — this is an - expected, routine outcome, not a failure. **`status`/`mouse`/ - `window-size` are untouched by this fix** — `includeBase` stays exactly - `solo`, per the instruction that started this fix: those three already - had their own correct-by-construction rule (a shared attach never sets - them, so a shared detach has nothing to put back), and reopening that - rule was out of scope. **If the client-count probe itself fails or - returns something `parseClientCount` can't parse, `includeTitles` falls - back to `true`** — restoring is the pre-existing (safe-by-comparison) - behavior, and a probe failure is not treated as evidence that someone - else is still attached. + ever attached. Fix, bounded to the title options only: `detach()` runs + one more small, non-interactive probe — `buildClientCountProbeCommand + (socket, target)` (`tmux -S '' list-clients -t + 2>/dev/null | wc -l`, the same `list-clients` query and + `parseClientCount()` the attach-time probe already uses, just against + the socket/target this call already has rather than rediscovering + them) — and sets `includeTitles: count <= 1`. + - **Ordering, corrected 2026-09-14 — the probe must run BEFORE + `raw.kill()`, not after.** The first cut killed the local ssh client + first and only then ran the client-count probe, reasoning that "our + own about-to-close client may still show up in the count" as a + possibility to tolerate with `<=` instead of `<`. That reasoning was + backwards: on a fast network the local ssh process is very likely + already gone (or its tmux client already dropped) by the time the + probe's own ssh round trip lands, so with exactly one real peer still + attached the probe would read back `1`, `1 <= 1` would restore, and + forwarding would be switched off under that peer — **the exact + failure this fix exists to prevent, masked in easy conditions and + live in exactly the conditions (slower networks, slower tmux) where + it would matter most.** `restoreOnDetach()` now runs the client-count + probe **first**, while our own client is unconditionally still + attached, and only calls `raw.kill()` afterward (still guarded by its + own try/catch, same as before). With that ordering, "our own client + counts as one of the attached clients" is not a possibility to + tolerate — it is guaranteed, every time, by construction: `count <= + 1` now means "nobody but us," not "maybe just us, maybe we already + left." `count >= 2` means at least one other real client was attached + at the moment we checked; the title restore is skipped and logged at + debug (`log.debug`, falling back to `log.info` if the injected `log` + carries no `debug()`), never at warn — this is an expected, routine + outcome, not a failure. + - **Accepted trade-off: detach now waits for this probe, bounded by + `DETACH_CLIENT_COUNT_TIMEOUT_MS` (5 s, shorter than the 15 s attach + probe because the user is watching the tab close), before the local + ssh client is killed.** `ptyProcess.kill()` no longer ends the local client + synchronously; it starts `restoreOnDetach()`, which is awaited by + nothing (still fire-and-forget from the caller's perspective) but now + performs the probe, then the kill, then the restore call, in that + order. The alternative — kill first, then require the live count to + read exactly `0` (proof our own client is provably gone) before + trusting a "last one out" restore — was rejected: it does not avoid a + wait, it relocates and lengthens it (now needing the *post-kill* + count to visibly drop, which depends on tmux noticing the disconnect + *after* the ssh teardown completes, on top of the same probe round + trip), and it adds a genuinely new failure mode (poll until `0`, or + guess a single retry, either more code or a coin flip) for no + correctness gain over probing first. Probing first costs one ssh + round trip of added latency before the local terminal visibly closes + — the same order of magnitude as the attach-time probe already + accepted, and worst-case bounded by a 5 s kill timer; a probe that + times out falls back to restoring. + - **`status`/`mouse`/`window-size` are untouched by this fix** — + `includeBase` stays exactly `solo`, per the instruction that started + this fix: those three already had their own correct-by-construction + rule (a shared attach never sets them, so a shared detach has nothing + to put back), and reopening that rule was out of scope. **If the + client-count probe itself fails or returns something + `parseClientCount` can't parse, `includeTitles` falls back to + `true`** — restoring is the pre-existing (safe-by-comparison) + behavior, and a probe failure is not treated as evidence that someone + else is still attached. - **What this coordinates, and what it deliberately does not.** This makes "the last client out restores the title options" hold in the common case (a probe running right before the restore catches almost @@ -1013,9 +1052,16 @@ Launching a new remote session (#222) and injection over the messaging socket before anyone touched it" or a lock across attach attempts, both out of scope for a fix whose brief was "bounded and simple." Proven in `test/remote-attach.test.js`: detach-time count 0 or 1 still restores - the titles; count 2 restores only `status`/`mouse`/`window-size` (the - solo case) and skips the title segment, logging why; a failing - client-count probe falls back to restoring the titles. + the titles (solo case, all five options); count 2 restores only + `status`/`mouse`/`window-size` (solo) and skips the title segment, + logging why; the same count-2 case on a **shared** attach sends no + restore call at all (`buildRestoreCommand` returns `null` — nothing + was ever eligible); a failing client-count probe falls back to + restoring the titles. Every one of those cases also asserts the + ordering directly — a fake `runRemoteCommand` snapshots the local + pty's `killedCount()` at the moment the client-count probe runs and + the test checks it is still `0` there, then `1` once the whole detach + has settled. - **This is the first thing to populate the session-handle seam from issue #220** (see `.ai/contexts/trigger-watcher.md`, "Session handle"): a diff --git a/remote-attach.js b/remote-attach.js index 09a776a2..9982cba4 100644 --- a/remote-attach.js +++ b/remote-attach.js @@ -6,6 +6,7 @@ const TMUX_FIELD_RE = /^([A-Za-z0-9._-]{1,64}):(@?\d{1,10}(?:\.%?\d{1,10})?)$/; const PROBE_SEP = '\u0001'; const DEFAULT_PROBE_TIMEOUT_MS = 15000; +const DETACH_CLIENT_COUNT_TIMEOUT_MS = 5000; const DEFAULT_STATUS_LINES = 1; const NO_TMUX_ENV_EXIT_CODE = 3; const NO_TMUX_ENV_MARKER = 'NO_TMUX_ENV'; @@ -397,7 +398,7 @@ function createTmuxAttachAdapter(opts = {}) { async function restoreOnDetach() { let includeTitles = true; try { - const clientProbe = await runRemoteCommand(alias, buildClientCountProbeCommand(discovery.socket, parsed.target), { timeoutMs: DEFAULT_PROBE_TIMEOUT_MS }); + const clientProbe = await runRemoteCommand(alias, buildClientCountProbeCommand(discovery.socket, parsed.target), { timeoutMs: DETACH_CLIENT_COUNT_TIMEOUT_MS }); if (clientProbe && clientProbe.code === 0) { const count = parseClientCount(clientProbe.stdout); if (count != null) includeTitles = count <= 1; @@ -408,6 +409,7 @@ function createTmuxAttachAdapter(opts = {}) { if (!includeTitles) { logDebug(`[remote-attach:${alias}] skipping title restore on detach — another client is still attached to ${parsed.target}`); } + try { raw.kill(); } catch {} const restoreCmd = buildRestoreCommand(discovery.socket, parsed.target, discovery.pre, { includeBase: solo, includeTitles }); if (!restoreCmd) return; const result = await runRemoteCommand(alias, restoreCmd, { timeoutMs: DEFAULT_PROBE_TIMEOUT_MS }); @@ -420,8 +422,6 @@ function createTmuxAttachAdapter(opts = {}) { function detach() { if (detaching || !alive) return; detaching = true; - try { raw.kill(); } catch {} - // best-effort restore, every detach — see .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) restoreOnDetach().catch((err) => log.warn(`[remote-attach:${alias}] restore-on-detach failed: ${err && err.message}`)); } diff --git a/test/remote-attach.test.js b/test/remote-attach.test.js index d0f5d7bf..b4869058 100644 --- a/test/remote-attach.test.js +++ b/test/remote-attach.test.js @@ -29,6 +29,10 @@ const { classifyTitleActivity } = require('../classify-title-activity'); const PROBE_SEP = ''; const silentLog = { info() {}, warn() {}, error() {} }; +// Flushes every pending microtask (any depth of chained awaits), unlike a +// fixed count of `await Promise.resolve()` calls -- see +// .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles, issue #290) +const flushAsync = () => new Promise((resolve) => setImmediate(resolve)); /** A minimal IPty-like double: onData/onExit/write/resize/kill/pid. */ function fakeRawPty() { @@ -311,6 +315,10 @@ test('the returned ptyProcess pilots the fake remote pty through write() and kil assert.equal(ptyProcess.pid, 4242); ptyProcess.kill(); + // The local ssh client is now only killed once the detach-time + // client-count probe settles -- see .ai/contexts/session-cache.md + // ("Remote hosts — tmux attach", set-titles, issue #290). + await flushAsync(); // Detaching means ending the local ssh client and nothing else: any prefix // keystroke would assume this host's tmux prefix and land as literal text // in the remote session on a host that remapped it. Measured on the live @@ -813,9 +821,8 @@ test('detach() runs a best-effort restore call with the probed pre-attach values assert.equal(result.ok, true); result.ptyProcess.kill(); - // The restore call is fire-and-forget from inside kill(); let its microtask run. - await Promise.resolve(); - await Promise.resolve(); + // The restore call is fire-and-forget from inside kill(); let it settle. + await flushAsync(); assert.equal(restoreCalls.length, 1, 'exactly one restore call must be sent on detach when solo'); assert.equal( @@ -857,8 +864,7 @@ test('detach() restores only set-titles/set-titles-string on a shared detach, le assert.equal(result.ok, true); result.ptyProcess.kill(); - await Promise.resolve(); - await Promise.resolve(); + await flushAsync(); assert.equal(restoreCalls.length, 1, 'a shared detach must still restore the title options'); assert.equal( @@ -872,18 +878,23 @@ test('detach() restores only set-titles/set-titles-string on a shared detach, le // remote session must not race each other's title restore -- see // .ai/contexts/session-cache.md ("Remote hosts — tmux attach", set-titles). -const flushAsync = () => new Promise((resolve) => setImmediate(resolve)); - // Builds a fake runRemoteCommand that tells apart the three kinds of calls // detach() can now make: the detach-time client-count probe (list-clients, // answered with a bare count), the restore call itself, and (falling through -// to the default) the original attach-time discovery probe. -function makeDetachClientCountFake({ clientCountAtDetach, clientCountProbeFails = false } = {}) { +// to the default) the original attach-time discovery probe. `raw` is the +// fake local pty -- the probe handler snapshots raw.killedCount() at the +// moment it runs, proving the probe is sent while our own client is still +// unconditionally attached, before raw.kill() ever runs (issue #290 +// follow-up: a probe taken after killing our own client would undercount by +// one and misread "one real peer left" as "we were the last client out"). +function makeDetachClientCountFake({ raw, clientCountAtDetach, clientCountProbeFails = false } = {}) { const restoreCalls = []; const clientCountProbeCalls = []; + let killedCountAtProbeTime = null; const runRemoteCommand = async (alias, command) => { if (/^tmux -S '.*' list-clients -t /.test(command)) { clientCountProbeCalls.push(command); + killedCountAtProbeTime = raw.killedCount(); if (clientCountProbeFails) return { code: 1, stdout: '', stderr: 'ssh: connection refused' }; return { code: 0, stdout: `${clientCountAtDetach}\n`, stderr: '' }; } @@ -899,13 +910,13 @@ function makeDetachClientCountFake({ clientCountAtDetach, clientCountProbeFails stderr: '', }; }; - return { runRemoteCommand, restoreCalls, clientCountProbeCalls }; + return { runRemoteCommand, restoreCalls, clientCountProbeCalls, killedCountAtProbeTime: () => killedCountAtProbeTime }; } for (const count of [0, 1]) { test(`detach() still restores the title options when the detach-time client count is ${count} ("ours may still be counted")`, async () => { const raw = fakeRawPty(); - const { runRemoteCommand, restoreCalls, clientCountProbeCalls } = makeDetachClientCountFake({ clientCountAtDetach: count }); + const { runRemoteCommand, restoreCalls, clientCountProbeCalls, killedCountAtProbeTime } = makeDetachClientCountFake({ raw, clientCountAtDetach: count }); const adapter = createTmuxAttachAdapter({ spawnPty: () => raw.pty, runRemoteCommand, log: silentLog }); const result = await adapter.attach('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0' }, { cols: 100, rows: 40 }); assert.equal(result.ok, true); @@ -914,6 +925,8 @@ for (const count of [0, 1]) { await flushAsync(); assert.equal(clientCountProbeCalls.length, 1, 'detach must probe the live client count exactly once'); + assert.equal(killedCountAtProbeTime(), 0, 'the client-count probe must be sent before the local ssh client is killed'); + assert.equal(raw.killedCount(), 1, 'the local ssh client must be killed once the probe has settled'); assert.equal(restoreCalls.length, 1); assert.match( restoreCalls[0], @@ -927,7 +940,7 @@ test('detach() skips the title restore, but still restores status/mouse/window-s const raw = fakeRawPty(); const logLines = []; const log = { info() {}, warn() {}, error() {}, debug: (msg) => logLines.push(msg) }; - const { runRemoteCommand, restoreCalls, clientCountProbeCalls } = makeDetachClientCountFake({ clientCountAtDetach: 2 }); + const { runRemoteCommand, restoreCalls, clientCountProbeCalls, killedCountAtProbeTime } = makeDetachClientCountFake({ raw, clientCountAtDetach: 2 }); const adapter = createTmuxAttachAdapter({ spawnPty: () => raw.pty, runRemoteCommand, log }); const result = await adapter.attach('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0' }, { cols: 100, rows: 40 }); assert.equal(result.ok, true); @@ -936,6 +949,8 @@ test('detach() skips the title restore, but still restores status/mouse/window-s await flushAsync(); assert.equal(clientCountProbeCalls.length, 1); + assert.equal(killedCountAtProbeTime(), 0, 'the client-count probe must be sent before the local ssh client is killed'); + assert.equal(raw.killedCount(), 1, 'the local ssh client must still be killed once the probe has settled'); assert.equal(restoreCalls.length, 1, 'status/mouse/window-size must still be restored -- their solo rule is unchanged by this fix'); assert.equal( restoreCalls[0], @@ -945,9 +960,53 @@ test('detach() skips the title restore, but still restores status/mouse/window-s assert.ok(logLines.some((l) => /skipping title restore/i.test(l)), 'must log at debug why the title restore was skipped'); }); +// Test gap: a shared attach (never touches status/mouse/window-size) whose +// detach-time client count is >= 2 must send no restore call at all -- +// includeBase is false (shared) and includeTitles is false (another client +// still attached), so buildRestoreCommand returns null. +test('detach() sends no restore call at all on a shared detach when another client is attached at detach time (count 2)', async () => { + const raw = fakeRawPty(); + const restoreCalls = []; + const clientCountProbeCalls = []; + let killedCountAtProbeTime = null; + const runRemoteCommand = async (alias, command) => { + if (/^tmux -S '.*' list-clients -t /.test(command)) { + clientCountProbeCalls.push(command); + killedCountAtProbeTime = raw.killedCount(); + return { code: 0, stdout: '2\n', stderr: '' }; + } + if (/^tmux -S /.test(command) && !command.includes('attach') && !/display-message|show-options|list-clients/.test(command)) { + restoreCalls.push(command); + return { code: 0, stdout: '', stderr: '' }; + } + return { + code: 0, + // discovery-time clientCount 1 -> shared (non-solo) attach. + stdout: `${FAKE_SOCKET}${PROBE_SEP}200x50${PROBE_SEP}status on${PROBE_SEP}mouse off${PROBE_SEP}window-size manual${PROBE_SEP}set-titles on${PROBE_SEP}set-titles-string ${PRINTED_DEFAULT_SET_TITLES_STRING}${PROBE_SEP}1`, + stderr: '', + }; + }; + const adapter = createTmuxAttachAdapter({ spawnPty: () => raw.pty, runRemoteCommand, log: silentLog }); + const result = await adapter.attach( + 'vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0' }, { cols: 100, rows: 40 }, + ); + assert.equal(result.ok, true); + + result.ptyProcess.kill(); + await flushAsync(); + + assert.equal(clientCountProbeCalls.length, 1, 'detach must still probe the live client count on a shared attach'); + assert.equal(killedCountAtProbeTime, 0, 'the client-count probe must be sent before the local ssh client is killed'); + assert.equal(raw.killedCount(), 1, 'the local ssh client must still be killed'); + assert.equal( + restoreCalls.length, 0, + 'a shared detach with another client still attached must send no restore call at all -- base was never touched and titles are skipped', + ); +}); + test('detach() falls back to restoring the title options when the detach-time client-count probe fails', async () => { const raw = fakeRawPty(); - const { runRemoteCommand, restoreCalls, clientCountProbeCalls } = makeDetachClientCountFake({ clientCountAtDetach: 0, clientCountProbeFails: true }); + const { runRemoteCommand, restoreCalls, clientCountProbeCalls, killedCountAtProbeTime } = makeDetachClientCountFake({ raw, clientCountAtDetach: 0, clientCountProbeFails: true }); const adapter = createTmuxAttachAdapter({ spawnPty: () => raw.pty, runRemoteCommand, log: silentLog }); const result = await adapter.attach('vps', { sessionId: 's1', pid: 4242, tmux: 'main:@0.%0' }, { cols: 100, rows: 40 }); assert.equal(result.ok, true); @@ -956,6 +1015,8 @@ test('detach() falls back to restoring the title options when the detach-time cl await flushAsync(); assert.equal(clientCountProbeCalls.length, 1); + assert.equal(killedCountAtProbeTime(), 0, 'the client-count probe must be sent before the local ssh client is killed, even when the probe fails'); + assert.equal(raw.killedCount(), 1, 'the local ssh client must still be killed after a failed probe'); assert.equal(restoreCalls.length, 1); assert.match( restoreCalls[0], @@ -989,8 +1050,7 @@ test('detach() swallows a failing restore call without throwing', async () => { assert.equal(result.ok, true); assert.doesNotThrow(() => result.ptyProcess.kill()); - await Promise.resolve(); - await Promise.resolve(); + await flushAsync(); assert.equal(raw.killedCount(), 1, 'the local ssh client must still be killed even if the restore call rejects'); });