mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 02:29:33 +01:00
fix(tmux): stop nested prefix keys and TUI Alt+digits from moving other clients
Docker and owned-remote tmux servers kept the stock prefix table, so Ctrl+Q then )/(/s/w/L switch-client'd the inner client onto a sibling. Those create chains now leave only C-q send-prefix. codeman tui bound M-1..M-9 in the server-wide root table. The keys stay, but switch-client runs only for the attaching client (tty plus a one-shot pid). A browser tab's Esc-then-digit no longer jumps sessions, and a killed TUI leaves a binding that matches no live client. Co-authored-by: DDT <1786035110@qq.com>
This commit is contained in:
14 files changed
+728
-62
No files matched your search
@@ -626,12 +626,90 @@ describe('the way out when F1 is taken, on a socket with no detach key', () => {
|
||||
restoreWindowSizing: async () => true,
|
||||
} as unknown as TuiClient;
|
||||
|
||||
const handoff = await beginAttachHandoff(client, 'codeman-abc12345', 'w1-case');
|
||||
const handoff = await beginAttachHandoff(client, 'codeman-abc12345', 'w1-case', [], undefined, undefined, null);
|
||||
|
||||
expect(handoff.chord).toBe('Ctrl+B then Ctrl+D');
|
||||
});
|
||||
});
|
||||
|
||||
describe('attach switch keys stay on the TUI client', () => {
|
||||
it('binds M-1 through the guard for the tty it was given, and does not bind without one', async () => {
|
||||
const binds: Array<{ key: string; target: string; tty: string }> = [];
|
||||
const unbinds: Array<{ key: string; owner: string | undefined }> = [];
|
||||
let armed: string | null = null;
|
||||
let disarmed: string | null = null;
|
||||
const client = {
|
||||
readPrefixKey: async () => 'C-b',
|
||||
readDetachKey: async () => 'd',
|
||||
readPrefixBinding: async () => null,
|
||||
bindDetachKey: async () => true,
|
||||
unbindDetachKey: async () => true,
|
||||
armTuiSwitchGuard: async (tty: string) => {
|
||||
armed = tty;
|
||||
return true;
|
||||
},
|
||||
disarmTuiSwitchGuard: async (tty: string) => {
|
||||
disarmed = tty;
|
||||
},
|
||||
bindSwitchKey: async (key: string, target: string, tty: string) => {
|
||||
binds.push({ key, target, tty });
|
||||
return true;
|
||||
},
|
||||
unbindSwitchKey: async (key: string, owner?: string) => {
|
||||
unbinds.push({ key, owner });
|
||||
},
|
||||
readSessionOptions: async () => null,
|
||||
applySessionOptions: async () => true,
|
||||
restoreSessionOptions: async () => true,
|
||||
readWindowSizing: async () => null,
|
||||
presizeWindow: async () => true,
|
||||
followAttachingClient: async () => true,
|
||||
restoreWindowSizing: async () => true,
|
||||
} as unknown as TuiClient;
|
||||
|
||||
const tabs = [
|
||||
{ index: 1, label: 'w1', active: true, muxName: 'codeman-aaaa1111' },
|
||||
{ index: 2, label: 'w2', active: false, muxName: 'codeman-bbbb2222' },
|
||||
];
|
||||
const handoff = await beginAttachHandoff(
|
||||
client,
|
||||
'codeman-aaaa1111',
|
||||
'w1',
|
||||
tabs,
|
||||
undefined,
|
||||
undefined,
|
||||
'/dev/pts/4'
|
||||
);
|
||||
expect(armed).toBe('/dev/pts/4');
|
||||
expect(binds).toEqual([
|
||||
{ key: 'M-1', target: 'codeman-aaaa1111', tty: '/dev/pts/4' },
|
||||
{ key: 'M-2', target: 'codeman-bbbb2222', tty: '/dev/pts/4' },
|
||||
]);
|
||||
// Takeover clears M-1..M-9 before claiming, so a killed TUI's leftovers go.
|
||||
expect(unbinds.filter((entry) => entry.owner === undefined).map((entry) => entry.key)).toEqual([
|
||||
'M-1',
|
||||
'M-2',
|
||||
'M-3',
|
||||
'M-4',
|
||||
'M-5',
|
||||
'M-6',
|
||||
'M-7',
|
||||
'M-8',
|
||||
'M-9',
|
||||
]);
|
||||
|
||||
await handoff.restore();
|
||||
expect(disarmed).toBe('/dev/pts/4');
|
||||
expect(unbinds.filter((entry) => entry.owner === '/dev/pts/4').map((entry) => entry.key)).toEqual(['M-1', 'M-2']);
|
||||
|
||||
binds.length = 0;
|
||||
armed = null;
|
||||
await beginAttachHandoff(client, 'codeman-aaaa1111', 'w1', tabs, undefined, undefined, null);
|
||||
expect(armed).toBeNull();
|
||||
expect(binds).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('naming a session the TUI starts', () => {
|
||||
it("follows the web UI's w<n>-<case> convention", () => {
|
||||
expect(nextSessionName('mirofish', [])).toBe('w1-mirofish');
|
||||
|
||||
+58
-11
@@ -25,6 +25,11 @@ import {
|
||||
parseSessionOptions,
|
||||
parseDetachKey,
|
||||
parsePrefixBinding,
|
||||
isTuiSwitchBinding,
|
||||
tuiSwitchBindingTty,
|
||||
tuiSwitchAttachHook,
|
||||
tuiSwitchGuard,
|
||||
isTuiClientTty,
|
||||
ATTACH_BANNER_MARKER,
|
||||
parseTmuxSessionList,
|
||||
parseWindowSizing,
|
||||
@@ -619,31 +624,73 @@ describe('attach window sizing', () => {
|
||||
});
|
||||
|
||||
describe('TuiClient.bindSwitchKey', () => {
|
||||
it('escapes the command separator, or tmux runs the second command instead of binding it', () => {
|
||||
it('guards switch-client to one client tty, and keeps both commands inside the if-shell string', () => {
|
||||
// ⚠️ A bare `;` argument is a command separator to tmux's OWN parser: it
|
||||
// ends the bind-key and executes what follows immediately. That bound only
|
||||
// `switch-client` and ran `window-size latest` against every session at
|
||||
// attach time, which is why sessions were left on `latest` after a detach —
|
||||
// the sizing snapshot was taken from already-corrupted state. Verified
|
||||
// against real tmux both ways before this test was written.
|
||||
// ends the bind-key and executes what follows immediately. The two commands
|
||||
// therefore live in the single if-shell argument, where `;` separates them
|
||||
// when the key fires rather than when the key is bound. An unscoped
|
||||
// `switch-client` is refused: key tables are server-wide, and that binding
|
||||
// would move a browser tab on Esc then a digit.
|
||||
const calls: string[][] = [];
|
||||
const exec: TuiExecFile = async (_file, args) => {
|
||||
calls.push([...args]);
|
||||
return { stdout: '', stderr: '' };
|
||||
};
|
||||
const client = new TuiClient({ baseUrl, socket: 'codeman-beta', exec });
|
||||
return client.bindSwitchKey('M-2', 'codeman-aaaa1111').then(() => {
|
||||
return client.bindSwitchKey('M-2', 'codeman-aaaa1111', '/dev/pts/4').then(async () => {
|
||||
const bind = calls.find((args) => args.includes('bind-key'));
|
||||
expect(bind).toBeDefined();
|
||||
expect(bind).toContain('\\;');
|
||||
expect(bind).not.toContain(';');
|
||||
// Both commands have to be in the ONE binding.
|
||||
expect(bind?.join(' ')).toContain('switch-client -t codeman-aaaa1111');
|
||||
expect(bind?.join(' ')).toContain('window-size latest');
|
||||
expect(bind).not.toContain('\\;');
|
||||
const command = bind?.at(-1) ?? '';
|
||||
expect(command).toContain(
|
||||
'switch-client -t codeman-aaaa1111 ; set-window-option -t codeman-aaaa1111 window-size latest'
|
||||
);
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
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"';
|
||||
|
||||
it('recognises our binding and the legacy unscoped one, and leaves a user binding alone', () => {
|
||||
expect(isTuiSwitchBinding(guarded)).toBe(true);
|
||||
expect(tuiSwitchBindingTty(guarded)).toBe('/dev/pts/4');
|
||||
expect(
|
||||
isTuiSwitchBinding(
|
||||
'switch-client -t codeman-aaaa1111 \\; set-window-option -t codeman-aaaa1111 window-size latest'
|
||||
)
|
||||
).toBe(true);
|
||||
expect(isTuiSwitchBinding('switch-client -t codeman-aaaa1111')).toBe(true);
|
||||
expect(tuiSwitchBindingTty('switch-client -t codeman-aaaa1111')).toBeNull();
|
||||
expect(isTuiSwitchBinding('switch-client -n')).toBe(false);
|
||||
expect(isTuiSwitchBinding('switch-client -t my-session')).toBe(false);
|
||||
expect(isTuiSwitchBinding('display-message mine')).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses a tty that is not safe to interpolate, and defers client_pid in the hook', () => {
|
||||
expect(isTuiClientTty('/dev/pts/4')).toBe(true);
|
||||
expect(isTuiClientTty('/dev/ttys003')).toBe(true);
|
||||
expect(isTuiClientTty('/tmp/pty')).toBe(false);
|
||||
expect(isTuiClientTty('/dev/pts/4; rm')).toBe(false);
|
||||
expect(tuiSwitchGuard('/dev/pts/4')).toContain('#{client_tty},/dev/pts/4');
|
||||
expect(tuiSwitchGuard('/dev/pts/4')).toContain('#{@codeman-tui-pid}');
|
||||
// A bare #{client_pid} is expanded when the hook is stored, with no client
|
||||
// attached, and the pid is then empty forever. The backslash defers it.
|
||||
const hook = tuiSwitchAttachHook('/dev/pts/4');
|
||||
expect(hook).toContain('/dev/pts/4');
|
||||
expect(hook).toContain('\\#{client_pid}');
|
||||
expect(hook).toContain('set-hook -gu client-attached[90]');
|
||||
});
|
||||
});
|
||||
|
||||
describe('parsePrefixBinding', () => {
|
||||
const REAL = [
|
||||
'bind-key -T prefix d detach-client',
|
||||
|
||||
@@ -0,0 +1,214 @@
|
||||
/**
|
||||
* @fileoverview Alt+1..9 on a `codeman tui` attach must not move any other tmux
|
||||
* client. Key tables are server-wide, so the binding exists for every client;
|
||||
* the command runs only for the TUI's own tty and pid.
|
||||
*
|
||||
* Against a real tmux on a private socket (never Codeman's). One client is
|
||||
* attached on a pty whose tty is known BEFORE `tmux attach` runs, which is how
|
||||
* the TUI arms the hook. A second client is what a browser tab is.
|
||||
*
|
||||
* Port: N/A (no server)
|
||||
*/
|
||||
import { afterAll, describe, expect, it } from 'vitest';
|
||||
import { execFileSync, execSync, spawn, type ChildProcess } from 'node:child_process';
|
||||
import { readlinkSync, rmSync } from 'node:fs';
|
||||
import { createInterface } from 'node:readline';
|
||||
import * as pty from 'node-pty';
|
||||
import { TuiClient } from '../../src/tui/tui-client.js';
|
||||
|
||||
const hasTmux = (() => {
|
||||
try {
|
||||
execSync('tmux -V', { stdio: 'ignore' });
|
||||
execSync('python3 -c "import pty"', { stdio: 'ignore' });
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
})();
|
||||
|
||||
/**
|
||||
* Holds a pty open, prints its tty, and execs `tmux attach` on that slave only
|
||||
* after stdin says GO. IXON is off so a byte is never swallowed as flow control.
|
||||
*/
|
||||
const PTY_HELPER = `
|
||||
import os, pty, sys, termios, signal
|
||||
socket, session = sys.argv[1], sys.argv[2]
|
||||
master, slave = pty.openpty()
|
||||
attr = termios.tcgetattr(slave)
|
||||
attr[0] &= ~(termios.IXON | termios.IXOFF)
|
||||
termios.tcsetattr(slave, termios.TCSANOW, attr)
|
||||
sys.stdout.write(os.ttyname(slave) + "\\n")
|
||||
sys.stdout.flush()
|
||||
if not sys.stdin.readline().startswith("GO"):
|
||||
sys.exit(1)
|
||||
env = os.environ.copy()
|
||||
env["TERM"] = "xterm-256color"
|
||||
env.pop("TMUX", None)
|
||||
pid = os.fork()
|
||||
if pid == 0:
|
||||
os.setsid()
|
||||
os.dup2(slave, 0)
|
||||
os.dup2(slave, 1)
|
||||
os.dup2(slave, 2)
|
||||
os.close(master)
|
||||
os.close(slave)
|
||||
os.execvpe("tmux", ["tmux", "-f", "/dev/null", "-L", socket, "attach-session", "-t", session], env)
|
||||
os.close(slave)
|
||||
sys.stdout.write("ATTACHED\\n")
|
||||
sys.stdout.flush()
|
||||
while True:
|
||||
line = sys.stdin.readline()
|
||||
if not line or line.startswith("EXIT"):
|
||||
break
|
||||
if line.startswith("KEY "):
|
||||
os.write(master, bytes.fromhex(line[4:].strip()))
|
||||
os.kill(pid, signal.SIGTERM)
|
||||
try:
|
||||
os.waitpid(pid, 0)
|
||||
except ChildProcessError:
|
||||
pass
|
||||
os.close(master)
|
||||
`;
|
||||
|
||||
describe.skipIf(!hasTmux)('TUI M-1..M-9 switch only the attaching client', () => {
|
||||
const socket = `codeman-tui-switch-${process.pid}`;
|
||||
const alpha = 'codeman-aaaa1111';
|
||||
const beta = 'codeman-bbbb2222';
|
||||
const run = (args: readonly string[]): string =>
|
||||
execFileSync('tmux', ['-L', socket, '-f', '/dev/null', ...args], {
|
||||
encoding: 'utf8',
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
});
|
||||
const client = new TuiClient({ socket });
|
||||
let helper: ChildProcess | null = null;
|
||||
let other: pty.IPty | null = null;
|
||||
const lines: string[] = [];
|
||||
let waiters: Array<(line: string) => void> = [];
|
||||
|
||||
function pushLine(line: string): void {
|
||||
const waiter = waiters.shift();
|
||||
if (waiter) waiter(line);
|
||||
else lines.push(line);
|
||||
}
|
||||
|
||||
function nextLine(): Promise<string> {
|
||||
const queued = lines.shift();
|
||||
if (queued !== undefined) return Promise.resolve(queued);
|
||||
return new Promise((resolve) => waiters.push(resolve));
|
||||
}
|
||||
|
||||
function sessions(): Map<string, { session: string; pid: string }> {
|
||||
let out = '';
|
||||
try {
|
||||
out = run(['list-clients', '-F', '#{client_tty}\t#{client_session}\t#{client_pid}']);
|
||||
} catch {
|
||||
return new Map();
|
||||
}
|
||||
const map = new Map<string, { session: string; pid: string }>();
|
||||
for (const line of out.split('\n')) {
|
||||
const [tty, session, pid] = line.trim().split('\t');
|
||||
if (tty && session) map.set(tty, { session, pid: pid ?? '' });
|
||||
}
|
||||
return map;
|
||||
}
|
||||
|
||||
async function waitFor(check: () => boolean, ms = 4000): Promise<boolean> {
|
||||
const deadline = Date.now() + ms;
|
||||
while (Date.now() < deadline) {
|
||||
if (check()) return true;
|
||||
await new Promise((resolve) => setTimeout(resolve, 40));
|
||||
}
|
||||
return check();
|
||||
}
|
||||
|
||||
afterAll(() => {
|
||||
try {
|
||||
helper?.stdin?.write('EXIT\n');
|
||||
} catch {
|
||||
/* helper already gone */
|
||||
}
|
||||
helper?.kill();
|
||||
other?.kill();
|
||||
let socketPath = '';
|
||||
try {
|
||||
socketPath = run(['display-message', '-p', '#{socket_path}']).trim();
|
||||
run(['kill-server']);
|
||||
} catch {
|
||||
/* already gone */
|
||||
}
|
||||
if (socketPath.endsWith(`/${socket}`)) rmSync(socketPath, { force: true });
|
||||
});
|
||||
|
||||
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']);
|
||||
run(['new-session', '-d', '-s', beta, '-x', '80', '-y', '24', 'sleep 300']);
|
||||
|
||||
helper = spawn('python3', ['-c', PTY_HELPER, socket, alpha], { stdio: ['pipe', 'pipe', 'inherit'] });
|
||||
createInterface({ input: helper.stdout! }).on('line', pushLine);
|
||||
const tty = (await nextLine()).trim();
|
||||
expect(tty).toMatch(/^\/dev\/pts\/[0-9]+$/);
|
||||
|
||||
expect(await client.armTuiSwitchGuard(tty)).toBe(true);
|
||||
const hooks = run(['show-hooks', '-g']);
|
||||
expect(hooks).toContain('client-attached[90]');
|
||||
expect(hooks).toContain(tty);
|
||||
// Expanded at set-hook time this is empty and the guard never matches.
|
||||
expect(hooks).toContain('#{client_pid}');
|
||||
|
||||
helper.stdin!.write('GO\n');
|
||||
expect(await nextLine()).toBe('ATTACHED');
|
||||
expect(await waitFor(() => sessions().get(tty)?.session === alpha)).toBe(true);
|
||||
|
||||
const pid = sessions().get(tty)?.pid ?? '';
|
||||
expect(pid).toMatch(/^[0-9]+$/);
|
||||
expect(await waitFor(() => run(['show-options', '-gv', '@codeman-tui-pid']).trim() === pid)).toBe(true);
|
||||
// One-shot: a later client on a reused pty is not recorded.
|
||||
expect(run(['show-hooks', '-g'])).not.toContain('client-attached[90]');
|
||||
|
||||
expect(await client.bindSwitchKey('M-2', 'not-a-codeman-session', tty)).toBe(false);
|
||||
expect(await client.bindSwitchKey('M-2', beta, '/tmp/not-a-tty')).toBe(false);
|
||||
expect(await client.bindSwitchKey('M-2', beta, tty)).toBe(true);
|
||||
const listed = run(['list-keys', '-T', 'root']);
|
||||
expect(listed).toContain('if-shell -F');
|
||||
expect(listed).toContain(tty);
|
||||
expect(listed).toContain(`switch-client -t ${beta}`);
|
||||
|
||||
// Esc then 2, one write, inside escape-time: tmux reads it as M-2.
|
||||
helper.stdin!.write('KEY 1b32\n');
|
||||
expect(await waitFor(() => sessions().get(tty)?.session === beta)).toBe(true);
|
||||
|
||||
const env = { ...process.env } as Record<string, string>;
|
||||
delete env.TMUX;
|
||||
other = pty.spawn('tmux', ['-L', socket, '-f', '/dev/null', 'attach-session', '-t', alpha], {
|
||||
name: 'xterm-256color',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
env,
|
||||
});
|
||||
const otherTty = readlinkSync(`/proc/${other.pid}/fd/0`);
|
||||
expect(otherTty).not.toBe(tty);
|
||||
expect(await waitFor(() => sessions().get(otherTty)?.session === alpha)).toBe(true);
|
||||
|
||||
other.write('\x1b2');
|
||||
await new Promise((resolve) => setTimeout(resolve, 400));
|
||||
expect(sessions().get(otherTty)?.session).toBe(alpha);
|
||||
expect(sessions().get(tty)?.session).toBe(beta);
|
||||
|
||||
// Killed before restore(): the binding remains, the pid is the dead client.
|
||||
helper.stdin!.write('EXIT\n');
|
||||
helper = null;
|
||||
expect(await waitFor(() => !sessions().has(tty))).toBe(true);
|
||||
expect(run(['list-keys', '-T', 'root'])).toContain(`#{client_tty},${tty}`);
|
||||
expect(run(['show-options', '-gv', '@codeman-tui-pid']).trim()).toBe(pid);
|
||||
|
||||
other.write('\x1b2');
|
||||
await new Promise((resolve) => setTimeout(resolve, 400));
|
||||
expect(sessions().get(otherTty)?.session).toBe(alpha);
|
||||
|
||||
// restore() drops the key because it still names this tty, and the dead pid.
|
||||
await client.unbindSwitchKey('M-2', tty);
|
||||
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();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user