Skip to content

Commit ffa7fcf

Browse files
TeigenZhangTeigenArk0Nclaude
authored
fix(tmux-manager): use '|' separator in reconcileSessions (#71)
* fix(tmux-manager): use '|' separator in reconcileSessions Under non-tty execution contexts (launchd on macOS, systemd without TTY), tmux emits '\t' in FORMAT strings as the literal two characters `\` + `t` rather than as a tab. The parser's `line.indexOf('\t')` (a real tab char) therefore never matches, `activeSessions` stays empty, `reconcileSessions` returns `alive: []` / `discovered: []`, and `cleanupStaleSessions()` wipes every entry in `state.json` — even though the underlying tmux sessions are still alive. On the next startup the user sees an empty session list. The bug reproduces reliably when codeman is launched via a user LaunchAgent or a systemd unit without `TTYPath`. Interactive `npm run dev` hides it because tmux's format parser does interpret `\t` when stdout is a TTY. Fix: use `|` as the separator. tmux passes it through verbatim in every environment, and `|` is not a valid tmux session-name character so it cannot collide with the codeman-<uuid> / claudeman-<uuid> naming scheme. * test(tmux-manager): cover parsePaneList separator contract Extract the inline pane-list parser from `reconcileSessions` into an exported `parsePaneList()` helper plus `PANE_LIST_SEP` / `PANE_LIST_FORMAT` constants, so the '|' separator contract can be unit-tested directly. The new tests lock in: - Well-formed parsing into name -> pid Map - Empty / blank-line / missing-separator handling - Non-numeric pid and empty-name rejection - A literal `\t` (backslash + t) in the input is NOT treated as a delimiter — guards against the launchd/systemd regression that motivated PR #71. - Splitting on the first separator only. No behavior change in `reconcileSessions`; the body now delegates to the helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Teigen <teigen@TeigendeMac-mini.local> Co-authored-by: arkon <arkon.85@hotmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6280998 commit ffa7fcf

2 files changed

Lines changed: 125 additions & 18 deletions

File tree

src/tmux-manager.ts

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,44 @@ const LEGACY_MUX_NAME_PATTERN = /^claudeman-[a-f0-9-]+$/;
9898
/** Regex to validate tmux pane targets (e.g., "%0", "%1", "0", "1") */
9999
const SAFE_PANE_TARGET_PATTERN = /^(%\d+|\d+)$/;
100100

101+
/**
102+
* Separator used in `tmux list-panes -F` output between session name and pid.
103+
*
104+
* Must NOT be a backslash-escape (e.g. `\t`, `\n`): under non-tty execution
105+
* contexts (launchd on macOS, systemd without TTYPath) tmux can emit such
106+
* escapes as the literal two characters `\` + letter rather than the control
107+
* byte, breaking the parser and causing every tracked session to be classified
108+
* as dead — which wipes state.json on restart. '|' is passed through verbatim
109+
* in every environment and is rejected by tmux's own session-name validation,
110+
* so it cannot appear inside `#{session_name}` and cause a false split.
111+
*/
112+
const PANE_LIST_SEP = '|';
113+
114+
/** Format string for `tmux list-panes -F`. Keep in sync with {@link parsePaneList}. */
115+
const PANE_LIST_FORMAT = `#{session_name}${PANE_LIST_SEP}#{pane_pid}`;
116+
117+
/**
118+
* Parse the output of `tmux list-panes -a -F '#{session_name}|#{pane_pid}'`
119+
* into a Map of session-name → pane pid. Exported for unit testing.
120+
*
121+
* - Skips empty lines and lines without the separator.
122+
* - Skips entries with a non-numeric pid or empty name.
123+
*/
124+
export function parsePaneList(output: string): Map<string, number> {
125+
const result = new Map<string, number>();
126+
for (const line of output.split('\n')) {
127+
if (!line) continue;
128+
const sep = line.indexOf(PANE_LIST_SEP);
129+
if (sep === -1) continue;
130+
const name = line.slice(0, sep);
131+
const pid = parseInt(line.slice(sep + 1), 10);
132+
if (name && !Number.isNaN(pid)) {
133+
result.set(name, pid);
134+
}
135+
}
136+
return result;
137+
}
138+
101139
/** Characters unsafe in paths — shell metacharacters, quotes, and control chars */
102140
const UNSAFE_PATH_CHARS = /[;&|$`(){}<>'"\n\r]/;
103141

@@ -902,23 +940,13 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
902940
const discovered: string[] = [];
903941

904942
// Batch: single tmux call to get all session names + pane PIDs (replaces N per-session subprocess calls)
905-
const activeSessions = new Map<string, number>();
943+
let activeSessions = new Map<string, number>();
906944
try {
907-
const output = execSync("tmux list-panes -a -F '#{session_name}\t#{pane_pid}' 2>/dev/null || true", {
945+
const output = execSync(`tmux list-panes -a -F '${PANE_LIST_FORMAT}' 2>/dev/null || true`, {
908946
encoding: 'utf-8',
909947
timeout: EXEC_TIMEOUT_MS,
910948
}).trim();
911-
912-
for (const line of output.split('\n')) {
913-
if (!line) continue;
914-
const sep = line.indexOf('\t');
915-
if (sep === -1) continue;
916-
const name = line.slice(0, sep);
917-
const pid = parseInt(line.slice(sep + 1), 10);
918-
if (name && !Number.isNaN(pid)) {
919-
activeSessions.set(name, pid);
920-
}
921-
}
949+
activeSessions = parsePaneList(output);
922950
} catch (err) {
923951
console.error('[TmuxManager] Failed to list tmux panes:', err);
924952
}

test/tmux-manager.test.ts

Lines changed: 84 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99

1010
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
11-
import { TmuxManager } from '../src/tmux-manager.js';
11+
import { TmuxManager, parsePaneList } from '../src/tmux-manager.js';
1212
import { execSync } from 'node:child_process';
1313

1414
// ============================================================================
@@ -287,13 +287,27 @@ describe('TmuxManager (unit)', () => {
287287
});
288288

289289
it('should update respawn config', () => {
290-
const config = { enabled: true, idleTimeoutMs: 5000, updatePrompt: 'test', interStepDelayMs: 1000, sendClear: true, sendInit: true };
290+
const config = {
291+
enabled: true,
292+
idleTimeoutMs: 5000,
293+
updatePrompt: 'test',
294+
interStepDelayMs: 1000,
295+
sendClear: true,
296+
sendInit: true,
297+
};
291298
manager.updateRespawnConfig('meta-test', config);
292299
expect(manager.getSession('meta-test')?.respawnConfig).toEqual(config);
293300
});
294301

295302
it('should clear respawn config', () => {
296-
manager.updateRespawnConfig('meta-test', { enabled: true, idleTimeoutMs: 5000, updatePrompt: 'test', interStepDelayMs: 1000, sendClear: true, sendInit: true });
303+
manager.updateRespawnConfig('meta-test', {
304+
enabled: true,
305+
idleTimeoutMs: 5000,
306+
updatePrompt: 'test',
307+
interStepDelayMs: 1000,
308+
sendClear: true,
309+
sendInit: true,
310+
});
297311
manager.clearRespawnConfig('meta-test');
298312
expect(manager.getSession('meta-test')?.respawnConfig).toBeUndefined();
299313
});
@@ -327,8 +341,8 @@ describe('TmuxManager (unit)', () => {
327341

328342
const sessions = manager.getSessions();
329343
expect(sessions).toHaveLength(2);
330-
expect(sessions.map(s => s.sessionId)).toContain('s1');
331-
expect(sessions.map(s => s.sessionId)).toContain('s2');
344+
expect(sessions.map((s) => s.sessionId)).toContain('s1');
345+
expect(sessions.map((s) => s.sessionId)).toContain('s2');
332346
});
333347
});
334348

@@ -342,3 +356,68 @@ describe('TmuxManager (unit)', () => {
342356
});
343357
});
344358

359+
// ============================================================================
360+
// Parser Tests — locks in the '|' separator contract for `tmux list-panes -F`
361+
// output, guarding against regressions in non-tty execution contexts where
362+
// `\t` in tmux FORMAT strings can be emitted as the literal two characters
363+
// `\` + `t` instead of a tab byte (launchd, systemd without TTYPath, docker
364+
// exec without TTY). See PR #71.
365+
// ============================================================================
366+
367+
describe('parsePaneList', () => {
368+
it('parses well-formed output into name → pid', () => {
369+
const out = 'codeman-aaaa|1234\ncodeman-bbbb|5678\nclaudeman-cccc|9999';
370+
const result = parsePaneList(out);
371+
expect(result.size).toBe(3);
372+
expect(result.get('codeman-aaaa')).toBe(1234);
373+
expect(result.get('codeman-bbbb')).toBe(5678);
374+
expect(result.get('claudeman-cccc')).toBe(9999);
375+
});
376+
377+
it('returns an empty map for empty output', () => {
378+
expect(parsePaneList('').size).toBe(0);
379+
});
380+
381+
it('skips blank lines', () => {
382+
const result = parsePaneList('\ncodeman-aaaa|100\n\n\ncodeman-bbbb|200\n');
383+
expect(result.size).toBe(2);
384+
expect(result.get('codeman-aaaa')).toBe(100);
385+
expect(result.get('codeman-bbbb')).toBe(200);
386+
});
387+
388+
it('skips lines without the separator', () => {
389+
const result = parsePaneList('codeman-aaaa 1234\ncodeman-bbbb|5678');
390+
expect(result.size).toBe(1);
391+
expect(result.get('codeman-bbbb')).toBe(5678);
392+
});
393+
394+
it('skips lines with a non-numeric pid', () => {
395+
const result = parsePaneList('codeman-aaaa|notapid\ncodeman-bbbb|5678');
396+
expect(result.size).toBe(1);
397+
expect(result.get('codeman-bbbb')).toBe(5678);
398+
});
399+
400+
it('skips lines with an empty session name', () => {
401+
const result = parsePaneList('|1234\ncodeman-bbbb|5678');
402+
expect(result.size).toBe(1);
403+
expect(result.get('codeman-bbbb')).toBe(5678);
404+
});
405+
406+
it('treats a literal backslash-t in input as part of the session name, not a delimiter', () => {
407+
// Reproduces the launchd/systemd regression: under non-tty contexts tmux
408+
// was emitting FORMAT '\t' as the two characters `\` + `t` rather than a
409+
// tab byte. With the '|' separator, such literals must not be silently
410+
// treated as a delimiter — the line is discarded because there is no '|'.
411+
const literalBackslashT = 'codeman-aaaa\\t1234';
412+
const result = parsePaneList(literalBackslashT);
413+
expect(result.size).toBe(0);
414+
});
415+
416+
it('splits on the first separator only', () => {
417+
// Numeric trailing junk after the pid is tolerated by parseInt — proves
418+
// that splitting on the first '|' leaves the pid extractable even if a
419+
// future tmux ever appended extra fields.
420+
const result = parsePaneList('codeman-aaaa|1234|extra-field');
421+
expect(result.get('codeman-aaaa')).toBe(1234);
422+
});
423+
});

0 commit comments

Comments
 (0)