fix: address the release-prep review of #688

Give a client that fails the M-1..M-9 guard its key back (send-keys <key> as the if-shell else argument), so a browser tab's Esc then digit reaches its pane instead of being eaten, also after a TUI killed before restore(); only M-1..M-9 are accepted since the key is now interpolated into that command
Test it: the unit test pins the else argument and the key check, and the real-tmux test runs cat -v in alpha and waits for the other client's Esc 2 to echo there, both before and after the TUI dies
Say so in docs/architecture-invariants.md and docs/tui.md, and reword the killed-TUI and failed-arm notes that said the keys do nothing

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-10-24 14:45:34 +02:00
1 parent 311d01c52b
commit 46774b71f9
5 files changed
+39 -9

No files matched your search

+9 -2
View File
@@ -642,23 +642,30 @@ describe('TuiClient.bindSwitchKey', () => {
expect(bind).toBeDefined();
expect(bind).not.toContain(';');
expect(bind).not.toContain('\\;');
const command = bind?.at(-1) ?? '';
const command = bind?.at(-2) ?? '';
expect(command).toContain(
'switch-client -t codeman-aaaa1111 ; set-window-option -t codeman-aaaa1111 window-size latest'
);
// The else branch: a client that fails the guard (a browser tab's Esc
// then a digit) gets the key in its own pane instead of losing it.
expect(bind?.at(-1)).toBe('send-keys M-2');
expect(bind).toContain('if-shell');
expect(bind).toContain('-F');
expect(bind?.join(' ')).toContain('#{client_tty},/dev/pts/4');
expect(bind?.join(' ')).toContain('#{@codeman-tui-pid}');
expect(await client.bindSwitchKey('M-2', 'codeman-aaaa1111', '/tmp/not-a-tty')).toBe(false);
expect(await client.bindSwitchKey('M-2', 'other-session', '/dev/pts/4')).toBe(false);
// The key is interpolated into the else command, so only M-1..M-9 bind.
expect(await client.bindSwitchKey('M-2 ; kill-server', 'codeman-aaaa1111', '/dev/pts/4')).toBe(false);
expect(await client.bindSwitchKey('M-0', 'codeman-aaaa1111', '/dev/pts/4')).toBe(false);
expect(calls.filter((args) => args.includes('bind-key'))).toHaveLength(1);
});
});
});
describe('TUI switch-key guard', () => {
const guarded =
'if-shell -F "#{&&:#{==:#{client_tty},/dev/pts/4},#{==:#{client_pid},#{@codeman-tui-pid}}}" "switch-client -t codeman-aaaa1111 ; set-window-option -t codeman-aaaa1111 window-size latest"';
'if-shell -F "#{&&:#{==:#{client_tty},/dev/pts/4},#{==:#{client_pid},#{@codeman-tui-pid}}}" "switch-client -t codeman-aaaa1111 ; set-window-option -t codeman-aaaa1111 window-size latest" "send-keys M-2"';
it('recognises our binding and the legacy unscoped one, and leaves a user binding alone', () => {
expect(isTuiSwitchBinding(guarded)).toBe(true);
+12 -3
View File
@@ -142,8 +142,15 @@ describe.skipIf(!hasTmux)('TUI M-1..M-9 switch only the attaching client', () =>
if (socketPath.endsWith(`/${socket}`)) rmSync(socketPath, { force: true });
});
/** How many Esc-then-2 presses alpha's pane has echoed (the tty's ECHOCTL shows ESC as `^[`). */
function escTwosInAlpha(): number {
return run(['capture-pane', '-p', '-t', alpha]).split('^[2').length - 1;
}
it('switches the armed client only, including after that client has died', async () => {
run(['new-session', '-d', '-s', alpha, '-x', '80', '-y', '24', 'sleep 300']);
// alpha echoes what reaches its pane, so a key another client presses can
// be seen arriving there instead of being eaten by the root binding.
run(['new-session', '-d', '-s', alpha, '-x', '80', '-y', '24', 'stty -ixon; cat -v']);
run(['new-session', '-d', '-s', beta, '-x', '80', '-y', '24', 'sleep 300']);
helper = spawn('python3', ['-c', PTY_HELPER, socket, alpha], { stdio: ['pipe', 'pipe', 'inherit'] });
@@ -207,8 +214,10 @@ describe.skipIf(!hasTmux)('TUI M-1..M-9 switch only the attaching client', () =>
expect(otherTty).toMatch(CLIENT_TTY);
expect(otherTty).not.toBe(tty);
expect(escTwosInAlpha()).toBe(0);
other.write('\x1b2');
await new Promise((resolve) => setTimeout(resolve, 400));
// Not moved, and not swallowed: the else branch hands the key to its pane.
expect(await waitFor(() => escTwosInAlpha() === 1)).toBe(true);
expect(sessions().get(otherTty)?.session).toBe(alpha);
expect(sessions().get(tty)?.session).toBe(beta);
@@ -220,7 +229,7 @@ describe.skipIf(!hasTmux)('TUI M-1..M-9 switch only the attaching client', () =>
expect(run(['show-options', '-gv', '@codeman-tui-pid']).trim()).toBe(pid);
other.write('\x1b2');
await new Promise((resolve) => setTimeout(resolve, 400));
expect(await waitFor(() => escTwosInAlpha() === 2)).toBe(true);
expect(sessions().get(otherTty)?.session).toBe(alpha);
// restore() drops the key because it still names this tty, and the dead pid.