diff --git a/.changeset/tmux-prefix-table.md b/.changeset/tmux-prefix-table.md new file mode 100644 index 00000000..d711b9c2 --- /dev/null +++ b/.changeset/tmux-prefix-table.md @@ -0,0 +1,5 @@ +--- +"aicodeman": patch +--- + +Browser terminals can no longer be moved onto another session by tmux's prefix keys: typing Ctrl+B then `)`, `(`, `s` or `w` in a tab switched its tmux client to a different session while the tab kept the original's name. Codeman's tmux socket now keeps only Ctrl+B Ctrl+B (send-prefix) in its prefix table, so `Ctrl+B d` no longer detaches a terminal attached there: `codeman tui` and `scripts/tmux-manager.sh` detach with F1. diff --git a/CLAUDE.md b/CLAUDE.md index bb78756b..ebadaabe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -102,6 +102,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph - **`xterm-zerolag-input` is single-source** — BOTH echo addons live ONLY in `packages/xterm-zerolag-input/src/`, bundled into TWO **gitignored** vendor files: `vendor/xterm-zerolag-input.js` (buffer overlay, entry `zerolag-input-addon.ts`) and `vendor/xterm-predictive-echo.js` (codex write-through, entry `predictive-echo-addon.ts`) — dev by `scripts/postinstall.js`, prod by `scripts/build.mjs`. `app.js`/terminal-ui.js only **consume** them via `new LocalEchoOverlay(terminal)` / `new PredictiveEchoOverlay(terminal)`; there is no inline copy. So: change the package source, then rerun the bundle step (`npm install` for dev, `npm run build` for prod). **Never hand-edit `app.js` for overlay behavior, and never commit the gitignored vendor bundles.** Always test on mobile after touching it. → [architecture-invariants#xterm-zerolag-input-is-single-source](docs/architecture-invariants.md#xterm-zerolag-input-is-single-source), `docs/local-echo-overlay-plan.md` - **Default bind is loopback-only; non-loopback without a password starts but warns** — the server defaults to `--host 127.0.0.1`. Binding non-loopback (`--host`/`-H`/`CODEMAN_HOST`) without `CODEMAN_PASSWORD` starts anyway but prints a loud warning; `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` acknowledges it. ⚠️ The production systemd unit passes no `--host`, so prod binds **localhost only**: reach it via `tailscale serve`/tunnel to `127.0.0.1`. A loopback bind is reachable through a same-host tunnel but NOT by a browser hitting the box's LAN IP. `install.sh` is separate and prompts for the binding (defaulting to LAN + a password), and preserves the existing binding on re-runs. → [architecture-invariants#default-bind-and-the-non-loopback-warning-path](docs/architecture-invariants.md#default-bind-and-the-non-loopback-warning-path), `docs/security-architecture.md` - **Instance isolation / multi-instance attach danger** — the data dir (`~/.codeman`) and tmux socket (`tmux -L codeman`) are PROCESS-WIDE and shared by every Codeman on the machine, derived from `CODEMAN_INSTANCE` via `src/config/instance.ts`. ⚠️ A 2nd instance on the SAME socket **discovers and attaches PTYs to the first instance's live sessions**, resizing and mutating them. `$HOME` isolation is NOT enough because tmux is system-global. To run two instances, give each a distinct `CODEMAN_INSTANCE` (scopes dir + socket together), or set `CODEMAN_TMUX_SOCKET` + `CODEMAN_DATA_DIR` individually; `scripts/run-beta.sh` does this for a beta alongside prod. **Any new `~/.codeman/...` path MUST go through `dataPath()`**, never `join(homedir(), '.codeman', …)`, and **any new `tmux -L` caller through `resolveTmuxSocketName()`** (both in `config/instance.ts`): the TUI shells out to tmux from a second process, and a hardcoded `codeman` there would point a beta instance at prod's panes. → [architecture-invariants#instance-isolation-and-the-multi-instance-attach-danger](docs/architecture-invariants.md#instance-isolation-and-the-multi-instance-attach-danger) +- **The prefix table on Codeman's socket holds only ` send-prefix`** — browser tabs are tmux attach clients, and tmux's stock bindings let Ctrl+B `)`/`s` move a tab onto another session. `createSession` resets the table, and `reconcileSessions()` at boot for sessions that survived an upgrade (`src/tmux-prefix-table.ts`); `root` and `C- detach-client` (the TUI attach's way out) are left alone, so while a `codeman tui` attach is open its `root` keys reach browser tabs too, Alt+1..9 `switch-client` included (open follow-up). ⚠️ So `Ctrl+B d` does not exist there: anything attaching a user's terminal claims its own detach key (F1 in `root`, as `codeman tui` and `scripts/tmux-manager.sh` do). → [architecture-invariants#the-prefix-table-on-codemans-socket-holds-only-send-prefix](docs/architecture-invariants.md#the-prefix-table-on-codemans-socket-holds-only-send-prefix) - **node-pty's macOS `spawn-helper` ships without `+x`** (issues #6, #204): `node-pty@1.1.0` publishes `prebuilds/darwin-/spawn-helper` as mode 0644, and macOS launches every PTY through it, so a stock macOS install fails every session start with `Error: posix_spawnp failed.` **Linux can never reproduce it**: `spawn-helper` is an `OS=="mac"` gyp target and node-pty ships no Linux prebuild, so node-gyp always emits an executable helper there. ⚠️ The flip side of that: since Linux has no prebuild, `npm install` **needs a C/C++ toolchain there** (`make`, `g++`, `python3`), so `install.sh` checks for and installs one alongside Node/tmux/git — a stock Ubuntu 24 server has none and died inside node-gyp with `not found: make`. Do not drop that step. ⚠️ Look in **`prebuilds/-/`**, not just `build/Release/`, which does not exist on macOS. Repair is a chmod, never a mandatory rebuild (that would require Xcode CLI tools and deletes `prebuilds/` before compiling): `npm run fix:node-pty` chmods every helper then proves it by really opening a PTY. `spawnPtyWithHelperRepair()` (`utils/node-pty-repair.ts`) wraps every `pty.spawn()` in `session.ts` and self-heals a broken install on the first failure. → [architecture-invariants#node-ptys-macos-spawn-helper-must-be-executable](docs/architecture-invariants.md#node-ptys-macos-spawn-helper-must-be-executable) - **User-chosen paths are probed BOUNDED, never with `existsSync`/`statSync`** (#516): a linked case or a `workingDir` can sit on a network mount that stopped answering. A synchronous check there freezes the whole server, and an unbounded async `stat`/`lstat`/`readFile` holds one of libuv's threadpool workers (4 by default, shared with every `fs`, `dns.lookup` and `crypto` call) until the mount returns. On a request or spawn path use `probePath()`/`probePathKind()` (`utils/bounded-path-probe.ts`, read its `@fileoverview`). ⚠️ `unknown` is NEVER `absent`: nothing is created, scaffolded or 404'd on it. Bulk scans keep the stall cap; a request for ONE path the user named may pass `{ pastCap: true }`, which still stops at the ceiling that keeps one worker free; and a helper that would otherwise touch the path skips whatever is still `unknown`. User-facing errors for it go through `describeUnknownPath()`, so a refused probe is not reported as a broken folder. - **Headless screenshots: `deviceScaleFactor` MUST be 1, and write unique filenames** — under DSF=2 xterm's WebGL renderer draws glyphs at ~2× nominal size while still *reporting* nominal cell dims, so only the pixels reveal it and only the terminal font looks wrong. And overwriting a fixed output path leaves OS image viewers showing the old render, which reads as "the fix didn't work"; `scripts/capture-real-overview.mjs` mints a timestamped filename per run. Seed the per-device `localStorage` keys (`codeman:skin`, `codeman-font-size`, `codeman-app-settings`) so the capture matches a real device. → [architecture-invariants#headless-screenshot-capture](docs/architecture-invariants.md#headless-screenshot-capture) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 45d02ca5..473a52b8 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -16,6 +16,10 @@ Implementation detail extracted from `CLAUDE.md` so that file stays small enough **Instance isolation / multi-instance attach danger** — data dir (`~/.codeman`) and tmux socket (`tmux -L codeman`) are PROCESS-WIDE and shared by every Codeman on the machine, derived from `CODEMAN_INSTANCE` via `src/config/instance.ts` (`getDataDir()`/`dataPath()`/`DEFAULT_TMUX_SOCKET`). ⚠️ A 2nd instance on the SAME socket **discovers and attaches PTYs to the first instance's live sessions** (`tmux -L codeman attach-session …`), resizing/mutating them — `$HOME` isolation is NOT enough (tmux is system-global). To run two instances, give each a distinct `CODEMAN_INSTANCE` (scopes BOTH dir+socket: `~/.codeman-` + `-L codeman-`), or set `CODEMAN_TMUX_SOCKET` + `CODEMAN_DATA_DIR` individually. **`CODEMAN_INSTANCE` defaults to empty = the production layout (`~/.codeman`, `-L codeman`, port 3000)**, so this branch is safe to ship to master without disturbing existing installs. To run THIS beta alongside prod, launch with `scripts/run-beta.sh` (`CODEMAN_INSTANCE=beta` + `CODEMAN_PORT=5000`) — it never collides with prod's data dir/socket/port. Any new `~/.codeman/...` path MUST go through `dataPath()`, never `join(homedir(), '.codeman', …)`, and any new `tmux -L` caller through `resolveTmuxSocketName()` (same module): it applies the `CODEMAN_TMUX_SOCKET` override only when the name is safe and falls back to `DEFAULT_TMUX_SOCKET` otherwise. `TmuxManager` was the only such caller until `codeman tui` shelled out to tmux from a SECOND process for its degraded-mode listing and its attach handoff; a hardcoded `codeman` there would have pointed a beta instance straight at prod's panes. +### The prefix table on Codeman's socket holds only send-prefix + +**Browser tabs are tmux clients, so tmux's prefix table must not reach `switch-client`** (`src/tmux-prefix-table.ts`, called from `createSession` before `respawn-pane`, and from `reconcileSessions()` at boot whenever the socket lists a pane, so sessions that survived an upgrade are covered). Every keystroke typed into a browser terminal reaches the `tmux attach-session` client behind it, and app.js deliberately lets Ctrl+B through to the terminal (readline, Claude Code's "ctrl+b ctrl+b"). With tmux's stock prefix table, Ctrl+B then `(` `)` `L` `s` `w` moved that client onto ANOTHER session, which the tab then showed and typed into under the original's label; `d` detached it, `$` renamed the session, `:` opened tmux's prompt. So the table is reduced to ` send-prefix` (the socket's configured prefix, read with `show-options -gv prefix`), once per tmux server in practice: the reset only runs while `list-keys -T prefix` shows anything else. ⚠️ Keep `send-prefix`: Claude Code inside tmux backgrounds a command with "ctrl+b ctrl+b". ⚠️ Never touch the `root` table, and tolerate (and re-bind after a reset) `C- detach-client`: `codeman tui`'s attach claims F1 and Alt+1..9 in `root` and the held-Ctrl alias `C-d` in `prefix` (`beginAttachHandoff`), and a session created mid-attach must not take that way out away. ⚠️ Key tables are server-wide, so what the TUI binds for its attach applies to browser tabs as well: F1 and `C-d` detach a tab's client, and **`M-1`..`M-9` (`switch-client -t `) move it onto another session**, the very thing this reset exists to stop. The browser keeps a typed Alt+digit out of the terminal (`terminal-ui.js` swallows it for its own tab switching), but Esc followed by a digit within tmux's `escape-time` (500 ms on tmux 3.4) arrives as the same `M-`. This lasts for the length of the attach, and indefinitely after a TUI that is killed before `restore()` runs. Not fixed here; scoping those bindings to the TUI's own client is the open follow-up. ⚠️ The consequence for every terminal attach: `Ctrl+B d` does not exist on this socket. Anything that attaches a user's terminal claims its own way out for the attach, as `beginAttachHandoff` and `scripts/tmux-manager.sh` (F1 in `root`) do, and never advertises tmux's stock chord. Tests: `test/tmux-prefix-table.test.ts` (real tmux on a private socket, driven through a pty attach client), the wiring in `test/tmux-manager.test.ts`, the script in `test/tmux-manager-script-attach.test.ts`. + ### Reverse-proxy base path **Reverse-proxy base path** (`--base-url` / `CODEMAN_BASE_URL`, default `/`; `src/config/base-path.ts` is the pure single-source, normalized to `''` for root or `/foo`): lets Codeman be mounted under a sub-path behind a proxy that **forwards the prefix unchanged** (does NOT strip it). Deliberately few choke points, mirrored ingress/egress: diff --git a/docs/tui.md b/docs/tui.md index 9907d5f1..55d981e6 100644 --- a/docs/tui.md +++ b/docs/tui.md @@ -178,8 +178,11 @@ bar first named the wrong letter (tmux binds lowercase `d` to `detach-client` an capital `D` to `choose-client`), and once corrected it still failed for anyone who kept Ctrl held, because that sends `Ctrl+D`, which tmux leaves unbound. So the TUI claims `F1` in tmux's prefix-less key table for the length of the attach and gives -it back afterwards. The chord still works; it is simply not what you are told to -press. +it back afterwards. The held-Ctrl chord (`Ctrl+B Ctrl+D`) works too, as the TUI +claims it for the attach as well. Plain `Ctrl+B d` does not: on Codeman's socket +the prefix table holds nothing but `send-prefix`, because the browser's terminals +are tmux clients on the same server and must not be able to `switch-client` or +`detach-client` themselves (`src/tmux-prefix-table.ts`). You do not have to remember any of it. For as long as the attach lasts the pane wears a bar across the top: diff --git a/docs/wiki/Core-Concepts.md b/docs/wiki/Core-Concepts.md index 60da25e6..3aab2bae 100644 --- a/docs/wiki/Core-Concepts.md +++ b/docs/wiki/Core-Concepts.md @@ -97,7 +97,10 @@ would. That indirection buys: - **Real scrollback.** History is held by tmux, so reconnecting replays what happened while you were gone instead of starting from blank. - **Attach from anywhere else.** The same session is reachable from a terminal over SSH - with `codeman tui`, or plain `tmux -L codeman attach`. + with `codeman tui` (F1 detaches), `./scripts/tmux-manager.sh` (likewise F1), or plain + `tmux -L codeman attach`. A plain attach has no detach key: Codeman keeps tmux's prefix + table on its socket down to `send-prefix`, so `Ctrl+B d` does nothing there; leave it + with `tmux -L codeman detach-client` from another shell, or bind a key yourself. - **Secrets off the command line.** Environment overrides are injected with socket-scoped `tmux setenv` rather than being visible in the spawn command. diff --git a/scripts/tmux-manager.sh b/scripts/tmux-manager.sh index 169535ef..a3777ac9 100755 --- a/scripts/tmux-manager.sh +++ b/scripts/tmux-manager.sh @@ -27,6 +27,13 @@ SESSIONS_FILE="${HOME}/.codeman/mux-sessions.json" CODEMAN_TMUX_SOCKET="${CODEMAN_TMUX_SOCKET:-codeman}" TMUX_CMD=(tmux -L "$CODEMAN_TMUX_SOCKET") +# Codeman keeps the prefix table on its socket down to send-prefix (browser tabs +# are attach clients on the same server, src/tmux-prefix-table.ts), so Ctrl+B d +# does not detach here. An attach claims F1 in the root table instead, like +# `codeman tui` does, and only when nothing else holds it. +DETACH_KEY="F1" +CLAIMED_DETACH_KEY=0 + # Cached data CACHED_JSON="" @@ -36,12 +43,46 @@ REFRESH_INTERVAL=5 # Refresh data every 5 seconds # Cleanup on exit cleanup() { + release_detach_key tput cnorm 2>/dev/null stty echo 2>/dev/null echo "" } trap cleanup EXIT +# Command bound to $DETACH_KEY in the root table; empty when the key is free +detach_key_binding() { + "${TMUX_CMD[@]}" list-keys -T root 2>/dev/null \ + | sed -nE "s/^bind-key +(-[^T ][^ ]* +)*-T root ${DETACH_KEY} +//p" | sed -E 's/ +$//' +} + +release_detach_key() { + if [[ $CLAIMED_DETACH_KEY -eq 1 ]]; then + [[ "$(detach_key_binding)" == "detach-client" ]] && "${TMUX_CMD[@]}" unbind-key -T root "$DETACH_KEY" 2>/dev/null + CLAIMED_DETACH_KEY=0 + fi +} + +# Attach to a session with a key that gets you back out +attach_session() { + local mux_name=$1 bound status + bound=$(detach_key_binding) + if [[ -z "$bound" ]] && "${TMUX_CMD[@]}" bind-key -T root "$DETACH_KEY" detach-client 2>/dev/null; then + CLAIMED_DETACH_KEY=1 + bound="detach-client" + fi + if [[ "$bound" == "detach-client" ]]; then + echo -e "${CYAN}Attaching... (${DETACH_KEY} to detach)${NC}" + else + echo -e "${CYAN}Attaching... (${DETACH_KEY} is taken; detach from another shell: tmux -L $CODEMAN_TMUX_SOCKET detach-client -s $mux_name)${NC}" + fi + sleep 0.3 + "${TMUX_CMD[@]}" attach-session -t "$mux_name" + status=$? + release_detach_key + return $status +} + # Check dependencies check_dependencies() { if ! command -v jq &> /dev/null; then @@ -352,9 +393,7 @@ interactive_menu() { if check_alive "$mux_name"; then tput cnorm clear - echo -e "${CYAN}Attaching... (Ctrl+B D to detach)${NC}" - sleep 0.3 - "${TMUX_CMD[@]}" attach-session -t "$mux_name" + attach_session "$mux_name" tput civis need_full_redraw=1 force_refresh @@ -483,7 +522,11 @@ main() { [[ -z "${2:-}" ]] && { echo "Usage: $0 attach "; exit 1; } force_refresh local mux_name=$(get_session_field $(($2-1)) "muxName") - check_alive "$mux_name" && "${TMUX_CMD[@]}" attach-session -t "$mux_name" || echo "Session dead or not found" + if check_alive "$mux_name"; then + attach_session "$mux_name" + else + echo "Session dead or not found" + fi ;; kill) [[ -z "${2:-}" ]] && { echo "Usage: $0 kill "; exit 1; } diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 92af82b8..fbcd2733 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -125,6 +125,7 @@ import { import { EXEC_TIMEOUT_MS } from './config/exec-timeout.js'; import { ensureDeepSeekStatusShim } from './deepseek-status-shim.js'; +import { restrictTmuxPrefixTable } from './tmux-prefix-table.js'; /** How long a cached process snapshot stays usable. */ const PROC_SNAPSHOT_TTL_MS = 2000; @@ -2491,6 +2492,10 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { /* Non-critical */ } + // Browser keystrokes reach the attach client, so tmux's stock prefix table would + // let Ctrl+B ( or s move a tab onto another session. Before the agent starts. + restrictTmuxPrefixTable(this.tmux()); + // Per-CLI env: API keys, config blobs, config-sourced vars, status bridges. All of // it is registry data, so this is one unconditional call rather than a per-mode ladder. this._configureCliEnv( @@ -3176,6 +3181,11 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { active = new Map(); } + // Sessions that survived an upgrade may sit on a server whose prefix table was never + // reduced (createSession does it for new ones), so cover them at boot too. Only with + // panes listed: on an empty socket `list-keys` spins up a throwaway server instead. + if (active.size > 0) restrictTmuxPrefixTable(this.tmux()); + // Check tracked sessions against the live pane list. for (const [sessionId, session] of this.sessions) { const pid = active.get(session.muxName); diff --git a/src/tmux-prefix-table.ts b/src/tmux-prefix-table.ts new file mode 100644 index 00000000..4e37716c --- /dev/null +++ b/src/tmux-prefix-table.ts @@ -0,0 +1,138 @@ +/** + * @fileoverview Keep tmux's prefix table on Codeman's socket down to `send-prefix`. + * + * Codeman drives tmux through commands, never through key bindings, but every + * keystroke typed into a browser terminal reaches the `tmux attach-session` + * client behind it. With tmux's stock prefix table that client obeys the whole + * menu: Ctrl+B then `(`, `)`, `L`, `s` or `w` runs `switch-client` / + * `choose-tree` and re-attaches the tab's client to a DIFFERENT session, so the + * tab shows, and types into, another session's agent while Codeman still labels + * it as the original. Ctrl+B `d` detaches the client, `$` renames the session + * under Codeman's feet, `:` opens tmux's command prompt. Ctrl+B is not a rare + * key in a terminal (readline backward-char, Claude Code's background chord), + * and whatever is typed next lands in that menu. + * + * The prefix itself stays: Claude Code running inside tmux asks for + * "ctrl+b ctrl+b" to background a command, which only works through + * `send-prefix`. So the table keeps exactly that one binding, on whatever key + * the socket's prefix is (the socket reads the user's `~/.tmux.conf`). + * + * `codeman tui` shares the socket (src/tui/tui-app.ts, beginAttachHandoff). For + * the length of an attach it claims a held-Ctrl detach alias (`C-d`) in the + * prefix table, and its one-key way out (F1) and Alt+1..9 in the `root` table. + * `root` is never touched here, and a `C- detach-client` binding is left + * alone (and put back when a reset clears the rest of the table around it), so + * creating a session while a TUI attach is open does not take the attach's way + * out away. The reset only runs while the table holds anything else, i.e. once + * per tmux server in practice. + * + * That tolerance is server-wide like every key table: while such an alias is + * bound, a browser tab's client can detach itself with it too. The same goes + * for the `root` keys an attach claims, including Alt+1..9, which run + * `switch-client`: a tab that receives Esc then a digit within tmux's + * `escape-time` is moved onto another session for as long as those bindings + * exist. Leaving `root` alone keeps the attach working; it does not close that. + * + * @module tmux-prefix-table + */ + +import { execSync } from 'node:child_process'; +import { EXEC_TIMEOUT_MS } from './config/exec-timeout.js'; +import { shellescape } from './remote-hosts.js'; + +/** tmux's stock prefix, used when `show-options` cannot be read. */ +const DEFAULT_PREFIX = 'C-b'; + +/** One `list-keys -T prefix` line: key and command. */ +const PREFIX_LINE = /^bind-key\s+(?:-\S+\s+)*?-T\s+prefix\s+(\S+)\s+(.+)$/; + +/** The held-Ctrl detach alias `codeman tui` claims during an attach (heldCtrlAlias). */ +const TUI_DETACH_ALIAS = /^C-[a-z]$/; + +/** + * A key as `list-keys` prints it, without tmux's backslash escaping: `show-options` + * prints the prefix `C-\` as is, while the listing shows `C-\\` (and `\#`, `\$`, `\"`). + */ +function unescapeListedKey(key: string): string { + return key.replace(/\\(.)/g, '$1'); +} + +/** + * Whether the prefix table (as printed by `tmux list-keys -T prefix`) still holds + * a binding other than ` send-prefix` or a TUI detach alias, or lacks + * ` send-prefix`. Unparseable output counts as "needs a reset": + * resetting is idempotent, skipping one leaves `switch-client` reachable from + * the browser. A prefix of `None` expects no `send-prefix` binding at all. + */ +export function prefixTableNeedsReset(listKeysOutput: string, prefix: string = DEFAULT_PREFIX): boolean { + let hasSendPrefix = false; + for (const raw of listKeysOutput.split('\n')) { + const line = raw.trim(); + if (!line) continue; + const match = PREFIX_LINE.exec(line); + if (!match) return true; + const key = unescapeListedKey(match[1]); + const command = match[2].trim(); + if (command === 'send-prefix' && key === prefix) { + hasSendPrefix = true; + continue; + } + if (command === 'detach-client' && TUI_DETACH_ALIAS.test(key) && key !== prefix) continue; + return true; + } + return prefix === 'None' ? false : !hasSendPrefix; +} + +/** The `C- detach-client` bindings in a `list-keys -T prefix` listing (see prefixTableNeedsReset). */ +function tuiDetachAliases(listKeysOutput: string, prefix: string): string[] { + const keys: string[] = []; + for (const raw of listKeysOutput.split('\n')) { + const match = PREFIX_LINE.exec(raw.trim()); + if (!match) continue; + const key = unescapeListedKey(match[1]); + if (match[2].trim() === 'detach-client' && TUI_DETACH_ALIAS.test(key) && key !== prefix) keys.push(key); + } + return keys; +} + +/** Runs one tmux command line and returns its stdout. */ +export type TmuxRunner = (command: string) => string; + +const defaultRunner: TmuxRunner = (command) => + execSync(command, { encoding: 'utf8', timeout: EXEC_TIMEOUT_MS, stdio: ['ignore', 'pipe', 'ignore'] }); + +/** + * Reduce the prefix table of the tmux server behind `tmux` (e.g. `tmux -L 'codeman'`) + * to ` send-prefix`, when it holds anything more. Never throws: a failure + * only leaves tmux's stock bindings in place. Returns whether a reset ran. + */ +export function restrictTmuxPrefixTable(tmux: string, run: TmuxRunner = defaultRunner): boolean { + let prefix = DEFAULT_PREFIX; + try { + prefix = run(`${tmux} show-options -gv prefix`).trim() || DEFAULT_PREFIX; + } catch { + /* keep tmux's stock prefix */ + } + let listing: string | null; + try { + listing = run(`${tmux} list-keys -T prefix`); + } catch { + listing = null; // an empty table makes list-keys fail on some tmux builds + } + if (listing !== null && !prefixTableNeedsReset(listing, prefix)) return false; + try { + run(`${tmux} unbind-key -a -T prefix`); + } catch { + /* an empty table makes unbind-key -a fail too ("table prefix doesn't exist"); nothing to clear */ + } + try { + if (prefix !== 'None') run(`${tmux} bind-key -T prefix ${shellescape(prefix)} send-prefix`); + // `unbind-key -a` also took a running TUI attach's held-Ctrl way out; give it back. + for (const key of tuiDetachAliases(listing ?? '', prefix)) { + run(`${tmux} bind-key -T prefix ${shellescape(key)} detach-client`); + } + return true; + } catch { + return false; + } +} diff --git a/src/tui/tui-app.ts b/src/tui/tui-app.ts index b482ad15..cc837aee 100644 --- a/src/tui/tui-app.ts +++ b/src/tui/tui-app.ts @@ -317,6 +317,18 @@ export function detachChord(prefix?: string, key?: string): string { return `${formatPrefixKey(prefix)} then ${key || DEFAULT_DETACH_KEY}`; } +/** + * The chord an attach advertises when it could not claim the one-key way out. + * + * With no key bound to `detach-client` in the prefix table there is no stock + * `d` to name: Codeman keeps only `send-prefix` there (src/tmux-prefix-table.ts), + * so the held-Ctrl alias the attach claimed is the chord that works. + */ +export function fallbackDetachChord(prefix?: string, detachKey?: string, heldAlias?: string): string { + if (!detachKey && heldAlias) return detachChord(prefix, formatPrefixKey(heldAlias)); + return detachChord(prefix, detachKey); +} + /** `#` opens `#[…]`/`#{…}` in a tmux format, so a name carrying one must double it. */ function escapeTmuxFormat(value: string): string { return value.replace(/#/g, '##'); @@ -355,10 +367,10 @@ export function buildAttachBanner(options: { /** Alt+1..9 really switch sessions, so the bar may say so. */ switchKeys?: boolean; }): Record { - const chord = escapeTmuxFormat(detachChord(options.prefix, options.detachKey)); + const chord = escapeTmuxFormat(fallbackDetachChord(options.prefix, options.detachKey, options.heldAlias)); // Named on the bar because it is what people actually type: keeping Ctrl held // is the common way to press this, and the bar has to say that it works. - const held = options.heldAlias ? escapeTmuxFormat(formatPrefixKey(options.heldAlias)) : ''; + const held = options.heldAlias && options.detachKey ? escapeTmuxFormat(formatPrefixKey(options.heldAlias)) : ''; const alias = held ? ` (or ${held})` : ''; const label = escapeTmuxFormat(truncateLabel((options.label ?? '').trim(), ATTACH_BANNER_LABEL_MAX)); // ONE option, not `status-left`/`status-right`/`status-style`: `status-format[0]` @@ -624,7 +636,7 @@ export async function beginAttachHandoff( if (target === muxName) await client.followAttachingClient(target); } return { - chord: oneKey ? ONE_KEY_DETACH : detachChord(prefix, detachKey), + chord: oneKey ? ONE_KEY_DETACH : fallbackDetachChord(prefix, detachKey, claimed && alias ? alias : undefined), async restore(): Promise { for (const key of switchKeys) await client.unbindSwitchKey(key); if (oneKey) await client.unbindDetachKey(ONE_KEY_DETACH, 'root'); @@ -702,7 +714,7 @@ export interface TuiKeymapContext { /** False in degraded mode, where the only verb that works is attach. */ server: boolean; approval?: TuiApprovalKeys; - /** tmux's detach chord as this socket reports it. Defaults to the stock `Ctrl+B D`. */ + /** How the last attach said to leave it. Defaults to F1, the key an attach claims. */ detach?: string; /** A dead-row card is offering `r` to resume, so the footer has to say so. */ resumeOffer?: boolean; @@ -761,7 +773,7 @@ export function helpKeysFor(glyphs: TuiGlyphSet, context: TuiKeymapContext): Arr ['tab', 'next session'], // The one key that is not the TUI's: an attach hands the terminal to tmux, // and leaving it is the question every first attach asks. - [context.detach ?? detachChord(), 'detach from an attached session, back to here'], + [context.detach ?? ONE_KEY_DETACH, 'detach from an attached session, back to here'], ]; if (context.server) { keys.push( @@ -1109,11 +1121,12 @@ class TuiApp { private readonly glyphs: TuiGlyphSet; private readonly socket = resolveTmuxSocketName(); /** - * How to leave an attach, in the local tmux's own prefix. Read per attach - * (a session can override the prefix) and remembered so the help overlay - * names the real chord even before the first attach. + * How to leave an attach. Read per attach (F1 may be the user's, a session + * can override the prefix) and remembered for the help overlay; until the + * first attach it names F1, what an attach claims unless the user's config + * already holds it. */ - private detachChordLabel = detachChord(); + private detachChordLabel: string = ONE_KEY_DETACH; /** True for the length of one resume. The only thing standing between a resume and a loop. */ private resuming = false; diff --git a/test/tmux-manager-script-attach.test.ts b/test/tmux-manager-script-attach.test.ts new file mode 100644 index 00000000..6998fc06 --- /dev/null +++ b/test/tmux-manager-script-attach.test.ts @@ -0,0 +1,123 @@ +/** + * @fileoverview scripts/tmux-manager.sh attaches with a way back out. + * + * Codeman keeps the prefix table on its socket down to `send-prefix` + * (src/tmux-prefix-table.ts), so tmux's `Ctrl+B d` no longer detaches there. The + * script's attach claims F1 in the root table for the length of the attach instead, + * and leaves a binding the user owns alone. + * + * Runs the real script against a REAL tmux server on a private throwaway socket + * (never Codeman's own) with a throwaway HOME, through a pty like a terminal would. + * + * Port: N/A (no server) + */ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { execSync } from 'node:child_process'; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import * as pty from 'node-pty'; + +const hasTools = (() => { + try { + execSync('tmux -V && jq --version', { stdio: 'ignore' }); + return true; + } catch { + return false; + } +})(); + +describe.skipIf(!hasTools)('scripts/tmux-manager.sh attach (real tmux, private socket)', () => { + const socket = `codeman-script-test-${process.pid}`; + const tmux = `tmux -L ${socket} -f /dev/null`; + const run = (cmd: string): string => execSync(cmd, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }); + const rootKeys = () => run(`${tmux} list-keys -T root`); + const clients = () => run(`${tmux} list-clients -F '#{client_session}'`).trim(); + let home = ''; + + async function waitFor(check: () => boolean, ms = 4000): Promise { + const deadline = Date.now() + ms; + while (Date.now() < deadline) { + if (check()) return true; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + return check(); + } + + /** `tmux-manager.sh attach 1` in a pty; resolves its output once it exits. */ + function attach(): { script: pty.IPty; exited: Promise } { + const env = { ...process.env, HOME: home, CODEMAN_TMUX_SOCKET: socket } as Record; + delete env.TMUX; // the suite may itself run inside tmux + const script = pty.spawn('bash', ['scripts/tmux-manager.sh', 'attach', '1'], { + name: 'xterm-256color', + cols: 100, + rows: 24, + cwd: process.cwd(), + env, + }); + let output = ''; + script.onData((data) => (output += data)); + const exited = new Promise((resolve) => script.onExit(() => resolve(output))); + return { script, exited }; + } + + beforeAll(() => { + home = mkdtempSync(join(tmpdir(), 'codeman-script-home-')); + mkdirSync(join(home, '.codeman')); + writeFileSync(join(home, '.codeman', 'mux-sessions.json'), JSON.stringify([{ name: 'alpha', muxName: 'alpha' }])); + run(`${tmux} new-session -d -s alpha -x 80 -y 24 'cat'`); + // What Codeman leaves on its socket: nothing but send-prefix in the prefix table. + run(`${tmux} unbind-key -a -T prefix`); + run(`${tmux} bind-key -T prefix C-b send-prefix`); + }); + + afterAll(() => { + let socketPath = ''; + try { + socketPath = run(`${tmux} display-message -p '#{socket_path}'`).trim(); + run(`${tmux} kill-server`); // our own throwaway socket only + } catch { + /* already gone */ + } + if (socketPath.endsWith(`/${socket}`)) rmSync(socketPath, { force: true }); + if (home) rmSync(home, { recursive: true, force: true }); + }); + + it('F1 detaches back out of the attach, and the claim is given back afterwards', async () => { + expect(rootKeys()).not.toMatch(/-T root F1\s/); + const { script, exited } = attach(); + try { + expect(await waitFor(() => clients() === 'alpha')).toBe(true); + expect(rootKeys()).toMatch(/-T root F1\s+detach-client/); + + script.write('\x1bOP'); // F1 + const output = await exited; + + expect(output).toContain('F1 to detach'); + expect(clients()).toBe(''); + expect(rootKeys()).not.toMatch(/-T root F1\s/); + expect(run(`${tmux} has-session -t alpha && echo alive`).trim()).toBe('alive'); + } finally { + script.kill(); + } + }); + + it("leaves an F1 binding of the user's own alone and says how to detach instead", async () => { + run(`${tmux} bind-key -T root F1 display-message mine`); + const { script, exited } = attach(); + try { + expect(await waitFor(() => clients() === 'alpha')).toBe(true); + expect(rootKeys()).toMatch(/-T root F1\s+display-message mine/); + + run(`${tmux} detach-client -s alpha`); + const output = await exited; + + expect(output).toContain('detach-client -s alpha'); + expect(output).not.toContain('F1 to detach'); + expect(rootKeys()).toMatch(/-T root F1\s+display-message mine/); + } finally { + script.kill(); + run(`${tmux} unbind-key -T root F1`); + } + }); +}); diff --git a/test/tmux-manager.test.ts b/test/tmux-manager.test.ts index cf0889e1..ebd5f927 100644 --- a/test/tmux-manager.test.ts +++ b/test/tmux-manager.test.ts @@ -727,20 +727,99 @@ describe('TmuxManager (unit)', () => { }); }); - describe('tmux launch cwd hardening', () => { - async function importWithTmuxCommandsEnabled(): Promise { - const originalVitest = process.env.VITEST; - vi.resetModules(); + async function importWithTmuxCommandsEnabled(): Promise { + const originalVitest = process.env.VITEST; + vi.resetModules(); + delete process.env.VITEST; + const module = await import('../src/tmux-manager.js'); + if (originalVitest === undefined) { delete process.env.VITEST; - const module = await import('../src/tmux-manager.js'); - if (originalVitest === undefined) { - delete process.env.VITEST; - } else { - process.env.VITEST = originalVitest; + } else { + process.env.VITEST = originalVitest; + } + return module.TmuxManager; + } + + describe('prefix table on the codeman socket', () => { + it("reduces the socket's prefix table to send-prefix before the agent starts", async () => { + // Browser keystrokes reach the attach client; with the stock table Ctrl+B ) or s + // would move a tab onto another session (the behavior itself: tmux-prefix-table.test.ts). + mockedExecSync.mockImplementation((cmd: string) => { + if (typeof cmd === 'string' && cmd.includes('which tmux')) return '/usr/bin/tmux\n'; + if (typeof cmd === 'string' && cmd.includes('display-message') && cmd.includes('#{pane_pid}')) return '4242\n'; + // Not tmux's stock C-b, so a hardcoded prefix cannot pass for the one read from tmux. + if (typeof cmd === 'string' && cmd.includes(' show-options -gv prefix')) return 'C-a\n'; + if (typeof cmd === 'string' && cmd.includes(' list-keys -T prefix')) { + return 'bind-key -T prefix C-a send-prefix\nbind-key -T prefix ) switch-client -n\n'; + } + return ''; + }); + const NonTestTmuxManager = await importWithTmuxCommandsEnabled(); + const nonTestManager = new NonTestTmuxManager(); + + try { + await nonTestManager.createSession({ + sessionId: 'abc12345-1234-5678-90ab-cdef12345678', + workingDir: '/project', + mode: 'shell', + }); + const commands = mockedExecSync.mock.calls.map(([cmd]) => String(cmd)); + expect(commands).toContain(`tmux -L 'codeman' unbind-key -a -T prefix`); + const bind = commands.indexOf(`tmux -L 'codeman' bind-key -T prefix 'C-a' send-prefix`); + expect(bind).toBeGreaterThan(commands.indexOf(`tmux -L 'codeman' unbind-key -a -T prefix`)); + expect(bind).toBeLessThan(commands.findIndex((c) => c.includes(' respawn-pane '))); + } finally { + nonTestManager.destroy(); } - return module.TmuxManager; + }); + + /** execSync for a boot-time reconcile: `panes` is what `list-panes -a` prints. */ + function mockReconcileTmux(panes: string): void { + mockedExecSync.mockImplementation((cmd: string) => { + if (typeof cmd === 'string' && cmd.includes('which tmux')) return '/usr/bin/tmux\n'; + if (typeof cmd === 'string' && cmd.includes(' list-panes -a ')) return panes; + if (typeof cmd === 'string' && cmd.includes(' show-options -gv prefix')) return 'C-a\n'; + if (typeof cmd === 'string' && cmd.includes(' list-keys -T prefix')) { + return 'bind-key -T prefix C-a send-prefix\nbind-key -T prefix ) switch-client -n\n'; + } + return ''; + }); } + it('reduces it at boot too, for sessions that survived an upgrade', async () => { + mockReconcileTmux('codeman-abc12345|4242|0||\n'); + const NonTestTmuxManager = await importWithTmuxCommandsEnabled(); + const nonTestManager = new NonTestTmuxManager(); + + try { + await nonTestManager.reconcileSessions(); + const commands = mockedExecSync.mock.calls.map(([cmd]) => String(cmd)); + const unbind = commands.indexOf(`tmux -L 'codeman' unbind-key -a -T prefix`); + expect(unbind).toBeGreaterThan(-1); + expect(commands.indexOf(`tmux -L 'codeman' bind-key -T prefix 'C-a' send-prefix`)).toBeGreaterThan(unbind); + } finally { + nonTestManager.destroy(); + } + }); + + it('leaves the key tables alone at boot when the socket lists no pane', async () => { + // On an empty socket `list-keys` would start a throwaway tmux server. + mockReconcileTmux(''); + const NonTestTmuxManager = await importWithTmuxCommandsEnabled(); + const nonTestManager = new NonTestTmuxManager(); + + try { + await nonTestManager.reconcileSessions(); + const commands = mockedExecSync.mock.calls.map(([cmd]) => String(cmd)); + expect(commands.some((c) => c.includes(' list-panes -a '))).toBe(true); + expect(commands.filter((c) => /show-options -gv prefix|list-keys|unbind-key|bind-key/.test(c))).toEqual([]); + } finally { + nonTestManager.destroy(); + } + }); + }); + + describe('tmux launch cwd hardening', () => { beforeEach(() => { mockedExecSync.mockImplementation((cmd: string) => { if (typeof cmd === 'string' && cmd.includes('which tmux')) { diff --git a/test/tmux-prefix-table.test.ts b/test/tmux-prefix-table.test.ts new file mode 100644 index 00000000..0219c60f --- /dev/null +++ b/test/tmux-prefix-table.test.ts @@ -0,0 +1,280 @@ +/** + * @fileoverview The tmux prefix table on Codeman's socket holds only + * ` send-prefix`, so keys typed into a browser terminal cannot move its + * attach client onto another session, without taking `codeman tui`'s way out away. + * + * Two layers here (the createSession wiring is in tmux-manager.test.ts): + * - the decision (`prefixTableNeedsReset`) on real `list-keys` shapes, + * - the effect against a REAL tmux server on a private throwaway socket (never + * Codeman's own), driven through a real attach client in a pty, the way a + * browser tab drives one. + * + * Port: N/A (no server) + */ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { execSync } from 'node:child_process'; +import { rmSync } from 'node:fs'; +import * as pty from 'node-pty'; +import { prefixTableNeedsReset, restrictTmuxPrefixTable } from '../src/tmux-prefix-table.js'; + +/** A trimmed copy of tmux 3.4's stock prefix table, as `list-keys -T prefix` prints it. */ +const STOCK = [ + 'bind-key -T prefix C-b send-prefix', + 'bind-key -T prefix C-o rotate-window', + 'bind-key -T prefix C-z suspend-client', + 'bind-key -T prefix ( switch-client -p', + 'bind-key -T prefix ) switch-client -n', + 'bind-key -T prefix L switch-client -l', + 'bind-key -T prefix d detach-client', + 'bind-key -T prefix s choose-tree -Zs', + 'bind-key -T prefix w choose-tree -Zw', + 'bind-key -r -T prefix Up select-pane -U', +].join('\n'); + +const RESTRICTED = 'bind-key -T prefix C-b send-prefix\n'; + +describe('prefixTableNeedsReset', () => { + it("resets tmux's stock table (switch-client, choose-tree, detach-client on d)", () => { + expect(prefixTableNeedsReset(STOCK)).toBe(true); + }); + + it('leaves a table that holds only C-b send-prefix alone', () => { + expect(prefixTableNeedsReset(RESTRICTED)).toBe(false); + }); + + it("leaves a codeman tui attach's held-Ctrl detach alias alone", () => { + expect(prefixTableNeedsReset(`${RESTRICTED}bind-key -T prefix C-d detach-client\n`)).toBe( + false + ); + }); + + it('resets when a single foreign binding is back, or the stock detach key d', () => { + expect(prefixTableNeedsReset(`${RESTRICTED}bind-key -T prefix s choose-tree -Zs\n`)).toBe( + true + ); + expect(prefixTableNeedsReset(`${RESTRICTED}bind-key -T prefix d detach-client\n`)).toBe( + true + ); + }); + + it('resets an empty table or one without send-prefix, and output it cannot read', () => { + expect(prefixTableNeedsReset('')).toBe(true); + expect(prefixTableNeedsReset('bind-key -T prefix C-d detach-client\n')).toBe(true); + expect(prefixTableNeedsReset('something unexpected\n')).toBe(true); + }); + + it("follows the socket's configured prefix instead of assuming C-b", () => { + const ctrlA = 'bind-key -T prefix C-a send-prefix\n'; + expect(prefixTableNeedsReset(ctrlA, 'C-a')).toBe(false); + // C-b send-prefix under a C-a prefix is a leftover, not the binding we want. + expect(prefixTableNeedsReset(RESTRICTED, 'C-a')).toBe(true); + expect(prefixTableNeedsReset('', 'None')).toBe(false); + }); + + it('reads a key tmux lists backslash-escaped as the key itself', () => { + // `prefix C-\` is printed as is by show-options, but listed as `C-\\`. + expect(prefixTableNeedsReset('bind-key -T prefix C-\\\\ send-prefix\n', 'C-\\')).toBe(false); + }); +}); + +describe('restrictTmuxPrefixTable (recorded commands)', () => { + /** A runner that answers from `replies` (a thrown Error stands for a failing command) and records every call. */ + function fakeTmux(replies: Record): { run: (cmd: string) => string; calls: string[] } { + const calls: string[] = []; + const run = (cmd: string): string => { + calls.push(cmd); + const sub = cmd.replace(/^tmux -L 'codeman' /, ''); + const reply = replies[sub] ?? ''; + if (reply instanceof Error) throw reply; + return reply; + }; + return { run, calls }; + } + + it('still binds send-prefix when the table is empty and unbind-key -a fails on it', () => { + // tmux 3.4 on an empty prefix table: both commands exit 1 with "table prefix doesn't exist". + const missing = new Error("table prefix doesn't exist"); + const tmux = fakeTmux({ + 'show-options -gv prefix': 'C-b\n', + 'list-keys -T prefix': missing, + 'unbind-key -a -T prefix': missing, + }); + expect(restrictTmuxPrefixTable(`tmux -L 'codeman'`, tmux.run)).toBe(true); + expect(tmux.calls).toContain(`tmux -L 'codeman' bind-key -T prefix 'C-b' send-prefix`); + }); + + it('does not reset again when the listed prefix is only escaped differently', () => { + const tmux = fakeTmux({ + 'show-options -gv prefix': 'C-\\\n', + 'list-keys -T prefix': 'bind-key -T prefix C-\\\\ send-prefix\n', + }); + expect(restrictTmuxPrefixTable(`tmux -L 'codeman'`, tmux.run)).toBe(false); + expect(tmux.calls.filter((c) => c.includes('bind-key'))).toEqual([]); + }); +}); + +const hasTmux = (() => { + try { + execSync('tmux -V', { stdio: 'ignore' }); + return true; + } catch { + return false; + } +})(); + +describe.skipIf(!hasTmux)('restrictTmuxPrefixTable against a real tmux (private socket)', () => { + // Throwaway server, never Codeman's socket; -f /dev/null keeps the user's config out. + const socket = `codeman-prefix-test-${process.pid}`; + const tmux = `tmux -L ${socket} -f /dev/null`; + const run = (cmd: string): string => execSync(cmd, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }); + const keys = (table: string) => run(`${tmux} list-keys -T ${table}`); + let client: pty.IPty | null = null; + + /** The session the attach client is looking at right now. */ + const clientSession = () => run(`${tmux} list-clients -F '#{client_session}'`).trim(); + + async function waitFor(check: () => boolean, ms = 3000): Promise { + const deadline = Date.now() + ms; + while (Date.now() < deadline) { + if (check()) return true; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + return check(); + } + + /** Types keys into the attach client, then gives tmux time to act on them. */ + async function type(keysToSend: string): Promise { + for (const ch of keysToSend) { + client!.write(ch); + await new Promise((resolve) => setTimeout(resolve, 100)); + } + await new Promise((resolve) => setTimeout(resolve, 300)); + } + + beforeAll(async () => { + // `cat -v` prints a received Ctrl+B as ^B, which is how send-prefix shows up. + run(`${tmux} new-session -d -s alpha -x 80 -y 24 'cat -v'`); + run(`${tmux} new-session -d -s beta -x 80 -y 24 'cat -v'`); + const env = { ...process.env } as Record; + delete env.TMUX; // the suite may itself run inside tmux + // What a browser tab is: a pty running `tmux attach-session` (tmux-manager getAttachArgs). + client = pty.spawn('tmux', ['-L', socket, 'attach-session', '-t', 'alpha'], { + name: 'xterm-256color', + cols: 80, + rows: 24, + env, + }); + expect(await waitFor(() => clientSession() === 'alpha')).toBe(true); + }); + + afterAll(() => { + client?.kill(); + let socketPath = ''; + try { + socketPath = run(`${tmux} display-message -p '#{socket_path}'`).trim(); + run(`${tmux} kill-server`); // our own throwaway socket only + } catch { + /* already gone */ + } + // tmux leaves the socket file behind; remove ours (and only ours). + if (socketPath.endsWith(`/${socket}`)) rmSync(socketPath, { force: true }); + }); + + it("with tmux's stock table, Ctrl+B ) moves the attach client onto another session", async () => { + // The bug, reproduced so the next test means something. + await type('\x02)'); + expect(await waitFor(() => clientSession() === 'beta')).toBe(true); + run(`${tmux} switch-client -c $(${tmux} list-clients -F '#{client_name}') -t alpha`); + expect(await waitFor(() => clientSession() === 'alpha')).toBe(true); + }); + + it('after the reset, Ctrl+B ) and Ctrl+B s leave the client where it is, Ctrl+B Ctrl+B reaches the pane', async () => { + const rootBefore = keys('root'); + + expect(restrictTmuxPrefixTable(tmux, run)).toBe(true); + + const prefix = keys('prefix') + .split('\n') + .filter((l) => l.trim()); + expect(prefix).toHaveLength(1); + expect(prefix[0]).toMatch(/-T prefix C-b\s+send-prefix$/); + expect(keys('root')).toBe(rootBefore); + expect(run(`${tmux} show-options -gv prefix`).trim()).toBe('C-b'); + + await type('\x02)'); + await type('\x02s'); + expect(clientSession()).toBe('alpha'); + expect(run(`${tmux} display-message -p -t alpha '#{pane_in_mode}'`).trim()).toBe('0'); + + await type('\x02\x02'); + expect(await waitFor(() => run(`${tmux} capture-pane -p -t alpha`).includes('^B'))).toBe(true); + }); + + it("keeps a codeman tui attach's way out when another session is created mid-attach", () => { + // What beginAttachHandoff claims: C-d in the prefix table, F1 and M-1 in root. + run(`${tmux} bind-key -T prefix C-d detach-client`); + run(`${tmux} bind-key -T root F1 detach-client`); + run(`${tmux} bind-key -T root M-1 switch-client -t alpha`); + + expect(restrictTmuxPrefixTable(tmux, run)).toBe(false); + + expect(keys('prefix')).toMatch(/-T prefix C-d\s+detach-client/); + expect(keys('root')).toMatch(/-T root F1\s+detach-client/); + expect(keys('root')).toMatch(/-T root M-1\s+switch-client -t alpha/); + }); + + it("gives a running attach's held-Ctrl way out back when the reset clears a stock table", () => { + // A tmux server whose table is still stock while a codeman tui attach holds C-d: + // `unbind-key -a` takes C-d along with the rest. + try { + run(`${tmux} bind-key -T prefix d detach-client`); + run(`${tmux} bind-key -T prefix ')' switch-client -n`); + run(`${tmux} bind-key -T prefix C-d detach-client`); + expect(restrictTmuxPrefixTable(tmux, run)).toBe(true); + const prefix = keys('prefix') + .split('\n') + .filter((l) => l.trim()); + expect(prefix).toHaveLength(2); + expect(keys('prefix')).toMatch(/-T prefix C-b\s+send-prefix/); + expect(keys('prefix')).toMatch(/-T prefix C-d\s+detach-client/); + } finally { + run(`${tmux} unbind-key -T prefix C-d`); + } + }); + + it('binds send-prefix to the configured prefix when the socket uses another one', () => { + run(`${tmux} set-option -g prefix C-a`); + try { + expect(restrictTmuxPrefixTable(tmux, run)).toBe(true); + const prefix = keys('prefix') + .split('\n') + .filter((l) => l.trim()); + expect(prefix).toEqual([expect.stringMatching(/-T prefix C-a\s+send-prefix$/)]); + } finally { + run(`${tmux} set-option -g prefix C-b`); + } + }); + + it('binds send-prefix on a server whose prefix table is empty', () => { + // On an empty table tmux fails both `list-keys -T prefix` and `unbind-key -a -T prefix`. + run(`${tmux} unbind-key -a -T prefix`); + expect(() => run(`${tmux} unbind-key -a -T prefix`)).toThrow(); + + expect(restrictTmuxPrefixTable(tmux, run)).toBe(true); + const prefix = keys('prefix') + .split('\n') + .filter((l) => l.trim()); + expect(prefix).toEqual([expect.stringMatching(/-T prefix C-b\s+send-prefix$/)]); + }); + + it('resets once, not on every call, for a prefix tmux lists escaped (C-\\)', () => { + run(`${tmux} set-option -g prefix 'C-\\'`); + try { + expect(restrictTmuxPrefixTable(tmux, run)).toBe(true); + expect(keys('prefix')).toMatch(/-T prefix C-\\\\\s+send-prefix/); + expect(restrictTmuxPrefixTable(tmux, run)).toBe(false); + } finally { + run(`${tmux} set-option -g prefix C-b`); + } + }); +}); diff --git a/test/tui/tui-app.test.ts b/test/tui/tui-app.test.ts index bdc34735..309be727 100644 --- a/test/tui/tui-app.test.ts +++ b/test/tui/tui-app.test.ts @@ -13,6 +13,7 @@ import { describe, it, expect } from 'vitest'; import { applyLiveMetrics, applyMuxNames, + beginAttachHandoff, buildAttachBanner, buildListLines, confirmKillStep, @@ -38,6 +39,7 @@ import { tmuxSocketFromEnv, } from '../../src/tui/tui-app.js'; import { createTuiModel, stateSince } from '../../src/tui/tui-model.js'; +import type { TuiClient } from '../../src/tui/tui-client.js'; import { glyphsFor } from '../../src/tui/tui-render.js'; import type { TuiLiveSessionMetrics, TuiTmuxSession } from '../../src/tui/tui-client.js'; import type { TuiConfirmState, TuiRow, TuiSessionRow } from '../../src/tui/tui-types.js'; @@ -596,6 +598,40 @@ describe('the held-Ctrl detach alias', () => { }); }); +describe('the way out when F1 is taken, on a socket with no detach key', () => { + // Codeman keeps only ` send-prefix` in its socket's prefix table + // (src/tmux-prefix-table.ts), so tmux's stock `Ctrl+B then d` does nothing there. + it('names the held-Ctrl chord on the bar instead of the stock d', () => { + const bar = buildAttachBanner({ prefix: 'C-b', heldAlias: 'C-d' })['status-format[0]']; + expect(bar).toContain('#[bold]Ctrl+B then Ctrl+D#[nobold]'); + expect(bar).not.toContain('then d'); + }); + + it('hands the same chord back from the attach handoff, for the help overlay and the hint line', async () => { + const free = new Set(['C-d', 'M-1']); + const client = { + readPrefixKey: async () => 'C-b', + readDetachKey: async () => null, + readPrefixBinding: async (key: string) => (free.has(key) ? null : 'display-message mine'), + bindDetachKey: async () => true, + unbindDetachKey: async () => true, + bindSwitchKey: async () => true, + unbindSwitchKey: async () => true, + 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 handoff = await beginAttachHandoff(client, 'codeman-abc12345', 'w1-case'); + + expect(handoff.chord).toBe('Ctrl+B then Ctrl+D'); + }); +}); + describe('naming a session the TUI starts', () => { it("follows the web UI's w- convention", () => { expect(nextSessionName('mirofish', [])).toBe('w1-mirofish'); @@ -737,8 +773,11 @@ describe('the way out of an attach', () => { const keys = helpKeysFor(GLYPHS, { server: true, detach: 'Ctrl+A then d' }); const detach = keys.find(([key]) => key === 'Ctrl+A then d'); expect(detach?.[1]).toContain('detach'); - // Degraded mode still attaches, so it still needs the way out. - expect(helpKeysFor(GLYPHS, { server: false }).map(([key]) => key)).toContain('Ctrl+B then d'); + // Degraded mode still attaches, so it still needs the way out. Before the first + // attach that is F1: Codeman's socket has no `d` in its prefix table to name. + const before = helpKeysFor(GLYPHS, { server: false }).map(([key]) => key); + expect(before).toContain(ONE_KEY_DETACH); + expect(before).not.toContain('Ctrl+B then d'); }); }); diff --git a/test/tui/tui-e2e.test.ts b/test/tui/tui-e2e.test.ts index 6e93d011..15c001a5 100644 --- a/test/tui/tui-e2e.test.ts +++ b/test/tui/tui-e2e.test.ts @@ -558,6 +558,12 @@ describe('codeman tui (under a pty)', () => { term.write('?'); await waitFor(() => frameLines(output).some((line) => line.includes('Keys')), 'the help overlay'); expect(frameLines(output).join('\n')).toContain('kill (y to confirm)'); + // Nothing has been attached yet, so this is the dashboard's starting label: + // F1, what an attach claims, not tmux's stock chord, which Codeman's socket + // does not bind (src/tmux-prefix-table.ts). + const detachLine = frameLines(output).find((line) => line.includes('detach from an attached session')); + expect(detachLine).toContain('F1'); + expect(detachLine).not.toContain('Ctrl+B'); term.write('\u001b'); await waitFor(() => !frameLines(output).some((line) => line.includes('Keys')), 'escape to close the overlay');