fix: apply the review items for #688 at merge

disarmTuiSwitchGuard matches the closed `#{client_tty},<tty>}` form, so a TUI on /dev/pts/4 no longer removes the hook armed for a second TUI on /dev/pts/41 (unit test plus a real-tmux case)
beginAttachHandoff arms the one-shot attach hook as its last tmux call before spawnSync, after the switch keys and the per-session dressing loop, with an order assertion in tui-app.test.ts and a sentence in the invariants paragraph
docs/remote-sessions.md and the buildRemoteLaunchCommand JSDoc show `remain-on-exit failed` at the end of the remote create chain

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

No files matched your search

+16 -2
View File
@@ -638,6 +638,8 @@ describe('attach switch keys stay on the TUI client', () => {
const unbinds: Array<{ key: string; owner: string | undefined }> = [];
let armed: string | null = null;
let disarmed: string | null = null;
// tmux calls in order, to pin where the attach hook is armed.
const order: string[] = [];
const client = {
readPrefixKey: async () => 'C-b',
readDetachKey: async () => 'd',
@@ -645,6 +647,7 @@ describe('attach switch keys stay on the TUI client', () => {
bindDetachKey: async () => true,
unbindDetachKey: async () => true,
armTuiSwitchGuard: async (tty: string) => {
order.push('arm');
armed = tty;
return true;
},
@@ -652,6 +655,7 @@ describe('attach switch keys stay on the TUI client', () => {
disarmed = tty;
},
bindSwitchKey: async (key: string, target: string, tty: string) => {
order.push('bind');
binds.push({ key, target, tty });
return true;
},
@@ -659,11 +663,17 @@ describe('attach switch keys stay on the TUI client', () => {
unbinds.push({ key, owner });
},
readSessionOptions: async () => null,
applySessionOptions: async () => true,
applySessionOptions: async () => {
order.push('dress');
return true;
},
restoreSessionOptions: async () => true,
readWindowSizing: async () => null,
presizeWindow: async () => true,
followAttachingClient: async () => true,
followAttachingClient: async () => {
order.push('follow');
return true;
},
restoreWindowSizing: async () => true,
} as unknown as TuiClient;
@@ -681,6 +691,10 @@ describe('attach switch keys stay on the TUI client', () => {
'/dev/pts/4'
);
expect(armed).toBe('/dev/pts/4');
// Armed once, as the LAST tmux call of the handoff: a TUI killed while the
// bars were being dressed must not leave a one-shot hook for its tty that a
// browser tab's attach on a reused pty would fire.
expect(order).toEqual(['bind', 'bind', 'dress', 'follow', 'dress', 'arm']);
expect(binds).toEqual([
{ key: 'M-1', target: 'codeman-aaaa1111', tty: '/dev/pts/4' },
{ key: 'M-2', target: 'codeman-bbbb2222', tty: '/dev/pts/4' },
+21
View File
@@ -690,6 +690,27 @@ describe('TUI switch-key guard', () => {
expect(hook).toContain('\\#{client_pid}');
expect(hook).toContain('set-hook -gu client-attached[90]');
});
it('disarms only the hook armed for its own tty, not one for a tty it is a prefix of', async () => {
// `/dev/pts/4` is a substring of `/dev/pts/41`: a bare includes() removed a
// second TUI's hook and left its Alt keys dead for that attach.
const disarm = async (armedFor: string, ownTty: string): Promise<string[][]> => {
const calls: string[][] = [];
const exec: TuiExecFile = async (_file, args) => {
calls.push([...args]);
if (args.includes('show-hooks')) {
return { stdout: `client-attached[90] ${tuiSwitchAttachHook(armedFor)}\n`, stderr: '' };
}
return { stdout: '', stderr: '' };
};
await new TuiClient({ baseUrl, socket: 'codeman-beta', exec }).disarmTuiSwitchGuard(ownTty);
return calls.filter((args) => args.includes('set-hook'));
};
expect(await disarm('/dev/pts/41', '/dev/pts/4')).toEqual([]);
expect(await disarm('/dev/pts/4', '/dev/pts/4')).toEqual([
['-L', 'codeman-beta', 'set-hook', '-gu', 'client-attached[90]'],
]);
});
});
describe('parsePrefixBinding', () => {
+12 -1
View File
@@ -154,7 +154,9 @@ describe.skipIf(!hasTmux)('TUI M-1..M-9 switch only the attaching client', () =>
expect(await client.armTuiSwitchGuard(tty)).toBe(true);
const hooks = run(['show-hooks', '-g']);
expect(hooks).toContain('client-attached[90]');
expect(hooks).toContain(tty);
// The closed form, as stored: disarmTuiSwitchGuard matches on it, so a
// hook for /dev/pts/41 is not taken for /dev/pts/4's.
expect(hooks).toContain(`#{client_tty},${tty}}`);
// The stored hook still names the format. Expanding it while the hook is
// stored would save an empty pid (no client yet) and the guard would never match.
expect(hooks).toContain('#{client_pid}');
@@ -226,5 +228,14 @@ describe.skipIf(!hasTmux)('TUI M-1..M-9 switch only the attaching client', () =>
await client.disarmTuiSwitchGuard(tty);
expect(run(['list-keys', '-T', 'root'])).not.toContain(`switch-client -t ${beta}`);
expect(() => run(['show-options', '-gv', '@codeman-tui-pid'])).toThrow();
// A hook armed for a tty this one is a prefix of (pts/4 vs pts/41) is
// another TUI's, and this tty's disarm must leave it.
const longer = `${tty}1`;
expect(await client.armTuiSwitchGuard(longer)).toBe(true);
await client.disarmTuiSwitchGuard(tty);
expect(run(['show-hooks', '-g'])).toContain('client-attached[90]');
await client.disarmTuiSwitchGuard(longer);
expect(run(['show-hooks', '-g'])).not.toContain('client-attached[90]');
});
});