From de864e7d63e499d83dc924050109864bd3ae537f Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Mon, 14 Sep 2026 23:40:10 +0200 Subject: [PATCH 01/18] fix(terminal): restore the history anchor after xterm parses, not before flushPendingWrites() captured the viewport of a user who was reading scrollback, called terminal.write(), and restored the anchor on the next line. xterm parses on its own schedule, so at that point the buffer has not moved: the guard `viewportY !== preserveViewportY` was false, scrollToLine was never called at all, and the Codex redraw landed a tick later and took the viewport to the live bottom with nothing left to pull it back. Scrolling up during a stream still got dragged down, which is what #358 reports, and a refresh was the only way back to a coherent view. The restore moves inside xterm's write callback, the first moment the redraw's effect exists, and runs before _scheduleTerminalWriteFlush() so a deferred remainder re-captures the restored anchor rather than the bottom. Two things follow from it running later: - A live anchor now wins over the sticky scroll-to-bottom. The two are captured at different moments (_wasAtBottomBeforeWrite at the frame's first batchTerminalWrite, the anchor at flush time), so a scroll-up in between leaves both set, and running both would jump to the bottom and come back a frame later instead of staying put. - The anchor is dropped if the active session changed or a buffer load started while the write was in flight. It indexes the buffer it was captured from, and selectSession() resets the terminal and chunk-loads a different scrollback. The existing regression passed throughout, because its write mock moved the viewport synchronously, which real xterm never does. The harness now models an asynchronous parse (redraw lands, then the callback fires), and all five of the anchor tests fail against the old code. Fixes #358 Co-Authored-By: Claude Opus 5 (1M context) --- .../terminal-history-anchor-after-parse.md | 5 + src/web/public/terminal-ui.js | 50 ++++++- test/terminal-flush-budget.test.ts | 140 +++++++++++++++++- 3 files changed, 179 insertions(+), 16 deletions(-) create mode 100644 .changeset/terminal-history-anchor-after-parse.md diff --git a/.changeset/terminal-history-anchor-after-parse.md b/.changeset/terminal-history-anchor-after-parse.md new file mode 100644 index 00000000..418171c4 --- /dev/null +++ b/.changeset/terminal-history-anchor-after-parse.md @@ -0,0 +1,5 @@ +--- +"aicodeman": patch +--- + +Keep the terminal anchored where you are reading while an agent streams (#358). Scrolling up during a Codex response could still be dragged back to the live bottom by the next redraw: the flush captured the viewport before writing and restored it immediately after, but xterm parses asynchronously, so at that moment the buffer had not moved yet, the restore compared the anchor against itself and did nothing, and the redraw landed a tick later with nothing left to pull the view back. The restore now runs inside xterm's own write callback, which is the first point at which the redraw's effect exists, and it holds across consecutive and chunked redraws. It is dropped if you switch sessions or a history replay starts before the write parses, since the anchor indexes the buffer it was captured from. diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 7ecc994c..e88e1c07 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -3543,6 +3543,29 @@ Object.assign(CodemanApp.prototype, { this._sendInputAsync(this.activeSessionId, text); }, + /** + * Re-assert a history anchor captured before a terminal write (#358). + * + * Called from xterm's write callback, never synchronously after write(): + * xterm parses on its own schedule, so the buffer only carries the redraw's + * effect once that callback fires. A null anchor means the user was following + * live output and nothing needs restoring. + */ + _restoreTerminalViewport(preserveViewportY, sessionId) { + if (preserveViewportY === null || preserveViewportY === undefined) return; + // The anchor is a row index into the buffer it was captured from. Now that + // this runs a parse later instead of synchronously, a session switch can land + // in between: selectSession() resets the terminal and chunk-loads the new + // session's scrollback, and scrolling THAT buffer to a row that meant + // something in the previous one is not a restore, it is a jump to an + // arbitrary place. Both checks cover one half of that window. + if (sessionId !== undefined && sessionId !== this.activeSessionId) return; + if (this._isLoadingBuffer) return; + if (typeof this.terminal?.scrollToLine !== 'function') return; + if (this.terminal.buffer?.active?.viewportY === preserveViewportY) return; + this.terminal.scrollToLine(preserveViewportY); + }, + /** * Flush pending writes to terminal, processing DEC 2026 sync markers. * Strips markers and writes content atomically within a single frame. @@ -3578,6 +3601,8 @@ Object.assign(CodemanApp.prototype, { // scroll-to-bottom below, where it protects against a mid-flush race. const preserveViewportY = this.terminal.buffer?.active && !this.isTerminalAtBottom() ? this.terminal.buffer.active.viewportY : null; + // Which buffer the anchor belongs to, checked again when the write parses. + const flushSessionId = this.activeSessionId; const writeChunk = joined.slice(0, MAX_FRAME_BYTES); if (_joinedLen > MAX_FRAME_BYTES) { @@ -3592,6 +3617,16 @@ Object.assign(CodemanApp.prototype, { this.terminal.write(writeChunk, () => { this._terminalWriteInFlight = false; this._terminalWriteInFlightBytes = 0; + // Restore INSIDE the callback (#358). xterm parses asynchronously, so + // the moment write() returns the buffer has not moved yet: the old + // restore ran here, found viewportY still equal to the anchor, and did + // nothing at all — then the parse landed and a cursor-addressed Codex + // redraw dragged the viewport to the live bottom with nothing left to + // pull it back. The callback is xterm's own "this chunk is parsed" + // signal, which is the earliest point the anchor can actually be + // reasserted. (The synchronous version passed its regression test only + // because the test's write mock moved the viewport synchronously.) + this._restoreTerminalViewport(preserveViewportY, flushSessionId); this._scheduleTerminalWriteFlush(); }); } catch (err) { @@ -3599,13 +3634,6 @@ Object.assign(CodemanApp.prototype, { this._terminalWriteInFlightBytes = 0; throw err; } - if ( - preserveViewportY !== null && - this.terminal.buffer?.active?.viewportY !== preserveViewportY && - typeof this.terminal.scrollToLine === 'function' - ) { - this.terminal.scrollToLine(preserveViewportY); - } const bytesThisFrame = deferred ? MAX_FRAME_BYTES : _joinedLen; const _dt = performance.now() - _t0; if (_dt > 100 || deferred) @@ -3617,7 +3645,13 @@ Object.assign(CodemanApp.prototype, { // Give manual scroll-up gestures a short grace window so high-frequency // Codex status ticks do not snap the viewport back while the user is // trying to inspect earlier output. - if (this._wasAtBottomBeforeWrite && !this._hasRecentUserScrollUp()) { + // + // A live anchor wins outright. The two flags are captured at different + // moments (_wasAtBottomBeforeWrite at the frame's first batchTerminalWrite, + // the anchor at flush time), so a scroll-up in between leaves both set; now + // that the anchor is reasserted after the parse, running both would jump to + // the bottom and then back one frame later instead of simply staying put. + if (preserveViewportY === null && this._wasAtBottomBeforeWrite && !this._hasRecentUserScrollUp()) { this.terminal.scrollToBottom(); } diff --git a/test/terminal-flush-budget.test.ts b/test/terminal-flush-budget.test.ts index 514e93da..82e0575c 100644 --- a/test/terminal-flush-budget.test.ts +++ b/test/terminal-flush-budget.test.ts @@ -49,6 +49,39 @@ function loadTerminalUiHarness(mode: string) { return { app, writes }; } +/** + * Swap in a terminal whose write() parses ASYNCHRONOUSLY, the way xterm.js does. + * + * The real renderer queues the chunk and applies it later, firing the write + * callback once it has been parsed; a redraw that addresses a row past the + * viewport (Codex's status line) drags the viewport to the live bottom at that + * point, not when write() returns. `parse()` runs that pending work. + */ +function attachAsyncParsingTerminal(app: any, opts: { viewportY: number; baseY: number }) { + const buffer = { viewportY: opts.viewportY, baseY: opts.baseY }; + const pending: Array<() => void> = []; + app.terminal.buffer = { active: buffer }; + app.terminal.write = vi.fn((_data: string, callback?: () => void) => { + pending.push(() => { + buffer.viewportY = buffer.baseY; // the redraw lands + callback?.(); + }); + }); + app.terminal.scrollToLine = vi.fn((line: number) => { + buffer.viewportY = line; + }); + app.terminal.scrollToBottom = vi.fn(() => { + buffer.viewportY = buffer.baseY; + }); + return { + buffer, + parse: () => { + const queued = pending.splice(0, pending.length); + for (const run of queued) run(); + }, + }; +} + function loadAppHarness() { const dir = resolve(import.meta.dirname, '../src/web/public'); const fetchMock = vi.fn(); @@ -337,20 +370,111 @@ describe('terminal flush budget', () => { it('restores the user scroll position when Codex Working redraws move the viewport', () => { const { app } = loadTerminalUiHarness('codex'); - const buffer = { viewportY: 40, baseY: 100 }; - app.terminal.buffer = { active: buffer }; - app.terminal.write = vi.fn(() => { - buffer.viewportY = buffer.baseY; - }); - app.terminal.scrollToLine = vi.fn((line: number) => { - buffer.viewportY = line; - }); + const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 }); app._wasAtBottomBeforeWrite = true; app._lastUserScrollUpAt = 0; app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)'); app.flushPendingWrites(); + parse(); expect(buffer.viewportY).toBe(40); }); + + // Issue #358. xterm.js parses on its own schedule, so the buffer still holds + // the pre-write viewport the instant write() returns: restoring there compared + // the anchor against itself, did nothing, and left the redraw free to drag the + // viewport to the live bottom a tick later. The previous regression passed + // because its write mock moved the viewport synchronously, which real xterm + // never does. These drive the callback explicitly instead. + it('restores the history anchor only AFTER xterm has parsed the write (#358)', () => { + const { app } = loadTerminalUiHarness('codex'); + const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 }); + app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)'); + + app.flushPendingWrites(); + // Nothing has parsed yet, so nothing may have been restored yet either. + expect(app.terminal.scrollToLine).not.toHaveBeenCalled(); + expect(buffer.viewportY).toBe(40); + + parse(); + + expect(app.terminal.scrollToLine).toHaveBeenCalledWith(40); + expect(buffer.viewportY).toBe(40); + }); + + it('holds the anchor across consecutive Codex redraws', () => { + const { app } = loadTerminalUiHarness('codex'); + const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 }); + + for (const frame of ['\x1b[55;1H\x1b[2m• Working (6s)', '\x1b[55;1H\x1b[2m• Working (7s)']) { + app.pendingWrites.push(frame); + app.flushPendingWrites(); + parse(); + expect(buffer.viewportY).toBe(40); + } + }); + + it('holds the anchor across a chunked write whose remainder is deferred', () => { + const { app } = loadTerminalUiHarness('codex'); + const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 }); + // Over the 32KB codex frame budget, so the flush defers a remainder and the + // second chunk goes out from the write callback's reschedule. + app.pendingWrites.push('x'.repeat(40000)); + + app.flushPendingWrites(); + parse(); + expect(buffer.viewportY).toBe(40); + + app.flushPendingWrites(); + parse(); + expect(buffer.viewportY).toBe(40); + expect(app.pendingWrites).toHaveLength(0); + }); + + it('drops the anchor when the user switched sessions before the write parsed', () => { + // The anchor indexes the buffer it came from. selectSession() resets the + // terminal and chunk-loads a different scrollback, so replaying row 40 into + // that one is a jump to an arbitrary place, not a restore. Only reachable now + // that the restore runs a parse later than the write. + const { app } = loadTerminalUiHarness('codex'); + const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 }); + app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)'); + + app.flushPendingWrites(); + app.activeSessionId = 'session-2'; // the user clicked another tab + parse(); + + expect(app.terminal.scrollToLine).not.toHaveBeenCalled(); + expect(buffer.viewportY).toBe(buffer.baseY); + }); + + it('drops the anchor while a buffer load is replaying history', () => { + const { app } = loadTerminalUiHarness('codex'); + const { parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 }); + app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)'); + + app.flushPendingWrites(); + app._isLoadingBuffer = true; // chunkedTerminalWrite owns the viewport now + parse(); + + expect(app.terminal.scrollToLine).not.toHaveBeenCalled(); + }); + + it('does not bounce off the bottom when the sticky flag and an anchor disagree', () => { + // _wasAtBottomBeforeWrite is captured at the frame's first batchTerminalWrite + // and the anchor at flush time, so a scroll-up in between leaves both live. + // The anchor wins: scrolling to the bottom and back would be a visible jump. + const { app } = loadTerminalUiHarness('codex'); + const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 }); + app._wasAtBottomBeforeWrite = true; + app._lastUserScrollUpAt = 0; + app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)'); + + app.flushPendingWrites(); + parse(); + + expect(app.terminal.scrollToBottom).not.toHaveBeenCalled(); + expect(buffer.viewportY).toBe(40); + }); }); From 3f2928ae730a647e89cef7c8817706fcbd65f0d2 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Tue, 15 Sep 2026 09:10:37 +0800 Subject: [PATCH 02/18] chore(cli-registry): clean up dead code and stale claims left after #380 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the "left as they are"/"worth knowing" items Ark0N named when merging #380 (the CLI-catalogue-driven install.sh + Docker agent image PR), none of which were correctness-blocking but all of which were real: - Removed install.sh's dead _cli_index/check_cli/get_cli_path helpers: the catalogue-driven menu and hints stopped calling them and nothing else ever did. - The generator no longer emits CLI_KIND/CLI_NPM, two bash arrays install.sh never read (the .mjs/docker-hosts.ts producers already read the JSON catalogue's kind/npmPackage fields directly, so only the bash copies were dead). - detect_all_clis now skips a disabled entry's probe entirely instead of running it and filtering the result downstream. No stock entry ships disabled today, so this closes a latent inefficiency before it is a latent bug rather than fixing an observed one. - The install hint for a launcherProfile entry (DeepSeek today) now explains in one line why it's a docs link and not a command: its own docs page documents `npm install -g @deepseek-ai/dsh`, which installs the launcher only and can't drive a pane, the exact trap the menu already avoids by withholding the command. Driven by a new generated CLI_LAUNCHER_ONLY array (from discovery.launcherProfile), not an id check, so any future launcherProfile entry gets the same caveat free. - Corrected the non-interactive-default comment: on a wget-only host, Claude's curl one-liner is filtered out of the offered list first, so the default becomes whichever npm-based entry sorts earliest instead (Codex today), not always Claude. Behaviour is unchanged — it was already printed, never silent — only the comment overclaimed. Tests: extended test/install-sh-invariants.test.ts with a positive guard for the new array and the trimmed array list, a negative guard that CLI_KIND/CLI_NPM/the three dead helpers cannot come back, and two real-bash tests (driven the same way the existing skip-menu tests are) proving a disabled entry is genuinely never probed rather than merely filtered after the fact. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011WzDjJnbK7zug8iQWnCc9z --- .changeset/cli-catalog-followups.md | 24 +++++++++++ install.sh | 65 +++++++++++++---------------- scripts/generate-cli-catalog.mts | 13 +++--- test/install-sh-invariants.test.ts | 65 +++++++++++++++++++++++++++-- 4 files changed, 121 insertions(+), 46 deletions(-) create mode 100644 .changeset/cli-catalog-followups.md diff --git a/.changeset/cli-catalog-followups.md b/.changeset/cli-catalog-followups.md new file mode 100644 index 00000000..5970e119 --- /dev/null +++ b/.changeset/cli-catalog-followups.md @@ -0,0 +1,24 @@ +--- +"aicodeman": patch +--- + +Cleans up the loose ends the maintainer flagged as "worth knowing rather than fixing" when +merging the CLI-catalogue-driven `install.sh`/Docker-agent-image PR (#380): + +- `install.sh` no longer carries `_cli_index`/`check_cli`/`get_cli_path`, three generic + lookup helpers left behind once the catalogue-driven menu and hints stopped calling them. +- The generator no longer emits `CLI_KIND`/`CLI_NPM`, two bash arrays nothing in `install.sh` + read (the `.mjs`/`docker-hosts.ts` producers already read the JSON catalogue's `kind`/ + `npmPackage` fields directly). +- `detect_all_clis` now skips a disabled entry entirely rather than probing it and filtering + the result downstream — no stock entry ships disabled today, so this is a latent + inefficiency closed before it is a latent bug, not a behaviour change. +- The install hint for a `launcherProfile` entry (DeepSeek today) now explains, in one line, + why it is a docs link rather than a runnable command — its docs page documents + `npm install -g @deepseek-ai/dsh`, which installs the launcher only and cannot drive a pane + on its own, the exact trap the menu already avoids by withholding the command. Driven by a + new generated `CLI_LAUNCHER_ONLY` array (from `discovery.launcherProfile`), not an id check. +- The non-interactive default's comment no longer claims it is always Claude Code: on a + wget-only host, Claude's curl one-liner is filtered out of the offered list first, so the + default becomes whichever npm-based entry sorts earliest instead. Behaviour is unchanged + (and was already printed, so never silent) — only the comment was wrong. diff --git a/install.sh b/install.sh index 5be359c2..8b5672bc 100755 --- a/install.sh +++ b/install.sh @@ -93,8 +93,7 @@ export PUPPETEER_SKIP_DOWNLOAD="${PUPPETEER_SKIP_DOWNLOAD:-1}" CLI_IDS=('claude' 'shell' 'opencode' 'codex' 'gemini' 'antigravity' 'pi' 'grok' 'deepseek' 'omp') CLI_LABELS=('Claude' 'Shell' 'OpenCode' 'Codex' 'Gemini' 'Antigravity' 'Pi' 'Grok' 'DeepSeek' 'OMP') CLI_ENABLED=(1 1 1 1 1 1 1 1 1 1) -CLI_KIND=('agent' 'shell' 'agent' 'agent' 'agent' 'agent' 'agent' 'agent' 'agent' 'agent') -CLI_NPM=('@anthropic-ai/claude-code' '' 'opencode-ai' '@openai/codex' '@google/gemini-cli' '' '@earendil-works/pi-coding-agent' '' '@deepseek-ai/dsh' '') +CLI_LAUNCHER_ONLY=(0 0 0 0 0 0 0 0 1 0) CLI_DOCS=('https://docs.claude.com/claude-code' '' 'https://opencode.ai/docs' 'https://developers.openai.com/codex/cli' 'https://github.com/google-gemini/gemini-cli' 'https://antigravity.google/cli' 'https://pi.dev' 'https://github.com/xai-org/grok-build' 'https://github.com/deepseek-ai/deepseek-harness' 'https://omp.sh') CLI_CMD_LINUX=('curl -fsSL https://claude.ai/install.sh | bash' '' 'curl -fsSL https://opencode.ai/install | bash' 'npm install -g @openai/codex' 'npm install -g @google/gemini-cli' 'curl -fsSL https://antigravity.google/cli/install.sh | bash' 'npm install -g --ignore-scripts @earendil-works/pi-coding-agent' 'curl -fsSL https://x.ai/cli/install.sh | bash' '' 'curl -fsSL https://omp.sh/install | sh') CLI_CMD_DARWIN=('curl -fsSL https://claude.ai/install.sh | bash' '' 'curl -fsSL https://opencode.ai/install | bash' 'npm install -g @openai/codex' 'npm install -g @google/gemini-cli' 'curl -fsSL https://antigravity.google/cli/install.sh | bash' 'npm install -g --ignore-scripts @earendil-works/pi-coding-agent' 'curl -fsSL https://x.ai/cli/install.sh | bash' '' 'brew install can1357/tap/omp') @@ -410,22 +409,6 @@ check_build_tools() { # test/install-sh-detection-parity.test.ts: the process PATH first (each declared # binary name in turn), then each known install path, dir-major. -# Index of "$1" in CLI_IDS -> CLI_IDX, returning 1 with CLI_IDX=-1 when unknown. -# A global rather than an echo because this runs inside loops, and a subshell per -# lookup is a fork per CLI per call site. -CLI_IDX=-1 -_cli_index() { - local want="$1" i - CLI_IDX=-1 - for ((i = 0; i < ${#CLI_IDS[@]}; i++)); do - if [[ "${CLI_IDS[$i]}" == "$want" ]]; then - CLI_IDX=$i - return 0 - fi - done - return 1 -} - # `dsh` is the hardest name of the lot: Debian ships an unrelated `dsh` # (dancer's shell). The server-side resolver settles it by demanding the # harness's own help banner; detection here only feeds the "you have no AI CLI" @@ -465,7 +448,8 @@ _cli_candidate_ok() { # Resolve every CLI in ONE pass, memoized. # -# CLI_FOUND_PATH is parallel to CLI_IDS ('' when not found). CLI_FOUND_COUNT +# CLI_FOUND_PATH is parallel to CLI_IDS ('' when not found, and also '' for a +# DISABLED entry — it is never probed at all, see below). CLI_FOUND_COUNT # counts only ENABLED entries that have a binary to look for, which is what the # "no AI CLI found" gate asks about — `shell` has no binary and must never make # that gate think an agent is installed. @@ -485,6 +469,16 @@ detect_all_clis() { for ((i = 0; i < ${#CLI_IDS[@]}; i++)); do found="" + # A disabled entry is never even probed: every consumer already filters + # on CLI_ENABLED before showing anything, so the command-v/stat calls + # below would be pure waste — and, unlike filtering downstream, skipping + # the probe here is what makes CLI_ENABLED mean "look for it" rather + # than just "offer it once found". + if [[ "${CLI_ENABLED[$i]}" != "1" ]]; then + CLI_FOUND_PATH[$i]="" + continue + fi + # 1. The process PATH, each declared binary name in turn. bin_end=$((${CLI_BIN_OFF[$i]} + ${CLI_BIN_LEN[$i]})) for ((j = ${CLI_BIN_OFF[$i]}; j < bin_end; j++)); do @@ -520,20 +514,6 @@ detect_all_clis() { return 0 } -# Is this CLI installed? Unknown id is "no", never an error. -check_cli() { - detect_all_clis - _cli_index "$1" || return 1 - [[ -n "${CLI_FOUND_PATH[$CLI_IDX]}" ]] -} - -# Where it was found, or nothing. -get_cli_path() { - detect_all_clis - _cli_index "$1" || return 1 - printf '%s\n' "${CLI_FOUND_PATH[$CLI_IDX]}" -} - # ---------------------------------------------------------------------------- # Catalogue helpers # ---------------------------------------------------------------------------- @@ -589,7 +569,12 @@ cli_catalog_names() { # the registry but an empty one here: installing the launcher alone leaves # nothing that can drive a pane, so the generator withholds the command for # any launcherProfile entry (see installCommandFor in generate-cli-catalog.mts) -# and this hint falls through to the docs URL instead. +# and this hint falls through to the docs URL instead — CLI_LAUNCHER_ONLY adds +# one line explaining WHY it is a docs link and not a command, so a user who +# follows that link straight to `npm install -g @deepseek-ai/dsh` (which the +# docs page itself documents) does not land back in the same "installed but +# cannot drive a pane" trap the menu exists to avoid. Data-driven, not an id +# check: any future launcherProfile entry gets the same caveat for free. cli_catalog_print_install_hints() { detect_all_clis local i @@ -601,6 +586,9 @@ cli_catalog_print_install_hints() { echo -e " ${CYAN}${CLI_INSTALL_CMD_TRUSTED[$i]}${NC} # ${CLI_LABELS[$i]}" elif [[ -n "${CLI_DOCS[$i]}" ]]; then echo -e " ${CLI_LABELS[$i]}: see ${CYAN}${CLI_DOCS[$i]}${NC}" + if [[ "${CLI_LAUNCHER_ONLY[$i]}" == "1" ]]; then + echo -e " (its package installs a launcher only — it needs a profile that can drive a pane, see the docs above)" + fi fi done } @@ -679,9 +667,12 @@ offer_ai_cli_install() { local cli_choice="" if [[ "$NONINTERACTIVE" == "1" ]] || ! has_tty; then - # Explicit automation opt-in: default to the first offered entry, - # which is registry order, which is Claude Code (order 0) — the - # same default this prompt has always taken non-interactively. + # Explicit automation opt-in: default to the first OFFERED entry. + # That is registry order, which is Claude Code (order 0), UNLESS + # this is a wget-only host and Claude's curl one-liner was just + # filtered out of offer_idx above — there, the first survivor is + # whichever npm-based entry sorts earliest (Codex today), not + # Claude. Printed either way so the choice is never silent. cli_choice="1" info "CODEMAN_NONINTERACTIVE=1: defaulting to ${CLI_LABELS[${offer_idx[0]}]}" else diff --git a/scripts/generate-cli-catalog.mts b/scripts/generate-cli-catalog.mts index ce9cd7e4..e17df0ec 100644 --- a/scripts/generate-cli-catalog.mts +++ b/scripts/generate-cli-catalog.mts @@ -135,8 +135,7 @@ export function renderInstallShBlock(entries: CliEntry[] = STOCK_CLIS): string { const ids: string[] = []; const labels: string[] = []; const enabled: string[] = []; - const kinds: string[] = []; - const npm: string[] = []; + const launcherOnly: string[] = []; const docs: string[] = []; const cmdLinux: string[] = []; const cmdDarwin: string[] = []; @@ -151,8 +150,11 @@ export function renderInstallShBlock(entries: CliEntry[] = STOCK_CLIS): string { ids.push(shQuote(entry.id as string)); labels.push(shQuote(entry.label)); enabled.push(entry.enabled ? '1' : '0'); - kinds.push(shQuote(entry.kind)); - npm.push(shQuote(entry.discovery.install.npmPackage ?? '')); + // Parallel to CLI_IDS: 1 when this entry's install command installs a launcher rather + // than something that can drive a pane on its own (see installCommandFor below). Purely + // derived from discovery.launcherProfile — install.sh's hint printer reads this to add a + // caveat instead of hardcoding which id it means. + launcherOnly.push(entry.discovery.launcherProfile ? '1' : '0'); docs.push(shQuote(entry.discovery.install.docsUrl ?? '')); cmdLinux.push(shQuote(installCommandFor(entry, 'linux'))); cmdDarwin.push(shQuote(installCommandFor(entry, 'darwin'))); @@ -195,8 +197,7 @@ export function renderInstallShBlock(entries: CliEntry[] = STOCK_CLIS): string { arr('CLI_IDS', ids), arr('CLI_LABELS', labels), arr('CLI_ENABLED', enabled), - arr('CLI_KIND', kinds), - arr('CLI_NPM', npm), + arr('CLI_LAUNCHER_ONLY', launcherOnly), arr('CLI_DOCS', docs), arr('CLI_CMD_LINUX', cmdLinux), arr('CLI_CMD_DARWIN', cmdDarwin), diff --git a/test/install-sh-invariants.test.ts b/test/install-sh-invariants.test.ts index 126c406d..abab770f 100644 --- a/test/install-sh-invariants.test.ts +++ b/test/install-sh-invariants.test.ts @@ -48,13 +48,12 @@ describe('install.sh generated-catalogue block', () => { ); }); - it('declares every array the detection code indexes', () => { + it('declares every array install.sh actually reads', () => { for (const name of [ 'CLI_IDS', 'CLI_LABELS', 'CLI_ENABLED', - 'CLI_KIND', - 'CLI_NPM', + 'CLI_LAUNCHER_ONLY', 'CLI_DOCS', 'CLI_CMD_LINUX', 'CLI_CMD_DARWIN', @@ -69,6 +68,17 @@ describe('install.sh generated-catalogue block', () => { } }); + it('declares no array install.sh never reads', () => { + // CLI_KIND and CLI_NPM were generated and read by nothing (the .mjs/docker-hosts.ts + // producers read the JSON's `kind`/`npmPackage` fields directly; only these two bash + // arrays were dead). A generated-but-unread array is a maintenance trap the generator + // itself cannot warn about — it has no reader to check against — so this pins the + // opposite of the test above: naming what must NOT come back rather than what must. + for (const name of ['CLI_KIND', 'CLI_NPM']) { + expect(new RegExp(`^${name}=\\(`, 'm').test(SOURCE), `${name} is declared but nothing reads it`).toBe(false); + } + }); + it('keeps no hand-written per-CLI detection behind', () => { // The nine `*_SEARCH_PATHS` arrays and eighteen `check_`/`get__path` pairs are // what this change removes. One left behind would be a second source of truth that the @@ -93,6 +103,16 @@ describe('install.sh generated-catalogue block', () => { [] ); }); + + it('keeps no dead generic-lookup helpers behind', () => { + // _cli_index/check_cli/get_cli_path were the ungenericized precursor to the per-CLI + // helpers above: same shape, one level of indirection, called from nowhere once the + // catalogue-driven menu and hints stopped needing a lookup-by-id. Unlike the per-CLI + // pairs these are exact names, not derived from the catalogue. + for (const fn of ['_cli_index()', 'check_cli()', 'get_cli_path()']) { + expect(CODE.includes(fn), `${fn} should have been removed as dead code`).toBe(false); + } + }); }); describe('install.sh trust boundary', () => { @@ -245,3 +265,42 @@ describe('install.sh AI CLI install menu', () => { expect(run.status).toBe(1); }); }); + +describe('install.sh detect_all_clis and a disabled entry', () => { + // No stock entry ships disabled today, so this is characterization rather than a regression + // pin on real data: it drives the real function in a real bash with entry 0 fabricated + // disabled, and points its binary at `bash` — guaranteed resolvable via `command -v` — to + // prove the entry is genuinely never PROBED (CLI_FOUND_PATH stays empty) rather than merely + // filtered out downstream by every consumer's own `CLI_ENABLED` check. + function driveDetect(disableEntry0: boolean) { + const driver = ` + set -euo pipefail + export CODEMAN_INSTALL_SH_LIB=1 + . "$1" + k=0; while [[ $k -lt \${#CLI_ALL_BINS[@]} ]]; do CLI_ALL_BINS[$k]="codeman-test-no-such-bin-$k"; k=$((k + 1)); done + k=0; while [[ $k -lt \${#CLI_ALL_PATHS[@]} ]]; do CLI_ALL_PATHS[$k]="/nonexistent/codeman-test/$k"; k=$((k + 1)); done + # Point entry 0's first declared binary at something that WILL resolve, so a probe that + # runs at all finds it. + CLI_ALL_BINS[\${CLI_BIN_OFF[0]}]="bash" + ${disableEntry0 ? 'CLI_ENABLED[0]="0"' : ''} + CLI_DETECT_DONE="" + detect_all_clis + echo "path0=[\${CLI_FOUND_PATH[0]}]" + echo "found=$CLI_FOUND_COUNT" + `; + const result = spawnSync('bash', ['-c', driver, 'bash', INSTALL_SH], { encoding: 'utf-8', timeout: 30_000 }); + return { status: result.status, stdout: result.stdout ?? '', stderr: result.stderr ?? '' }; + } + + it('probes an enabled entry (control case)', () => { + const run = driveDetect(false); + expect(run.stdout, run.stderr).not.toContain('path0=[]'); + expect(run.stdout).toContain('found=1'); + }); + + it('never probes a disabled entry', () => { + const run = driveDetect(true); + expect(run.stdout, run.stderr).toContain('path0=[]'); + expect(run.stdout).toContain('found=0'); + }); +}); From a1c35da0d8edb8f948905bfb07a17f92326cc98b Mon Sep 17 00:00:00 2001 From: codeman-local Date: Wed, 16 Sep 2026 09:26:30 +0800 Subject: [PATCH 03/18] fix(input): deliver a recovered keystroke before the Enter that submits it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every message typed on an Android phone lost its last character. An Android soft keyboard commits the last typed character and sends the Enter key in ONE InputConnection transaction, so the committed-text `input` event and the Enter keydown are both processed before any zero-delay timer runs. The orphaned-input recovery from #388 resolved its candidate only on such a timer, and that lost the character twice over: * ORDER — xterm emits `\r` synchronously from the Enter keydown, and the local-echo composer submits `pendingText` right there. The recovered character arrived one macrotask too late to be part of the prompt. * LOSS — that same `\r` bumps the canonical counter, so by the time the candidate resolved, `canonicalCount > snapshot` read as "xterm spoke for this keystroke" and stood the recovery down. The character was not merely late, it was dropped. Drain pending candidates synchronously at the next keydown instead, from xterm's custom key handler, which runs before xterm processes that key. The counter then still holds the value it had while the candidate's own keystroke was current, so the stand-down decision is made against the right keystroke, and the recovered byte reaches the composer ahead of whatever the new key emits. The timer stays as the fallback for a keystroke with no key after it. Physical keyboards are unaffected: there the timer has already resolved the candidate long before the next key arrives. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/android-last-character-on-enter.md | 5 ++ .../public/terminal-keycode229-recovery.js | 37 +++++++++++++++ test/terminal-keycode229-recovery.test.ts | 46 +++++++++++++++++++ 3 files changed, 88 insertions(+) create mode 100644 .changeset/android-last-character-on-enter.md diff --git a/.changeset/android-last-character-on-enter.md b/.changeset/android-last-character-on-enter.md new file mode 100644 index 00000000..0af4c14a --- /dev/null +++ b/.changeset/android-last-character-on-enter.md @@ -0,0 +1,5 @@ +--- +"aicodeman": patch +--- + +Stop a phone keyboard losing the last character of every message it sends. Android soft keyboards commit the last typed character and send the Enter key in one InputConnection transaction, so the `input` event and the Enter keydown are both processed before any zero-delay timer runs. The orphaned-input recovery from #388 only resolved its candidate on such a timer, and lost it both ways: xterm emits `\r` synchronously from the Enter keydown, so the local-echo composer submitted the prompt before the recovered character existed, and that `\r` bumped the "did xterm speak for this keystroke" counter, so the candidate then stood itself down and dropped the character outright. Pending candidates are now drained synchronously at the next keydown, from xterm's custom key handler — before xterm processes that key — so the counter still holds the value it had while the candidate's own keystroke was current, and the recovered byte reaches the composer ahead of the Enter. Typing on a physical keyboard is unaffected: there, the timer has already resolved the candidate before the next key arrives. diff --git a/src/web/public/terminal-keycode229-recovery.js b/src/web/public/terminal-keycode229-recovery.js index 3e00ea65..d6a95f74 100644 --- a/src/web/public/terminal-keycode229-recovery.js +++ b/src/web/public/terminal-keycode229-recovery.js @@ -66,6 +66,38 @@ let composing = false; const pending = []; + /** + * Resolve every candidate still pending, right now, instead of waiting for + * its zero-delay timer. + * + * Android soft keyboards commit the last character and send the Enter key + * in ONE InputConnection transaction: the `input` event and the Enter + * keydown are both processed before any timer runs. Left on its timer the + * candidate lost BOTH ways — xterm emits '\r' synchronously from the Enter + * keydown (so the local-echo composer submitted the prompt without the + * character), and that '\r' bumps `canonicalCount`, so the candidate then + * read "xterm spoke for this keystroke" and stood down, dropping the + * character outright. That is the "every message loses its last character" + * report from phones. + * + * Draining at the next keydown is correct on both counts: the counter still + * holds the value it had while this candidate's keystroke was current, and + * the byte reaches the composer ahead of whatever the new key emits. + */ + function flushPending() { + for (const candidate of pending.splice(0)) { + if (candidate.timer !== null) { + try { + clearTimer(candidate.timer); + } catch { + // A broken timer host must not break input handling. + } + candidate.timer = null; + } + resolveCandidate(candidate); + } + } + function cancelPending() { for (const candidate of pending.splice(0)) { candidate.active = false; @@ -111,6 +143,11 @@ */ function handleKeyEvent(event) { if (destroyed || event?.type !== 'keydown') return; + // Settle the PREVIOUS keystroke before this one can move the counter or + // reach the PTY — see flushPending(). This runs from xterm's custom key + // handler, i.e. before xterm processes the key, so a recovered character + // is always ordered ahead of the bytes this keydown produces. + flushPending(); keydownSnapshot = canonicalCount; } diff --git a/test/terminal-keycode229-recovery.test.ts b/test/terminal-keycode229-recovery.test.ts index 6a8a38a6..ebddc629 100644 --- a/test/terminal-keycode229-recovery.test.ts +++ b/test/terminal-keycode229-recovery.test.ts @@ -218,6 +218,52 @@ describe('orphaned terminal input recovery', () => { expect(reads).toEqual([]); }); + it('delivers the last character BEFORE the Enter that submits it (defect 4)', () => { + // Android soft keyboards commit the last character and send the Enter key in + // ONE InputConnection transaction, so the `input` event and the Enter keydown + // are processed before any zero-delay timer runs. Two things then went wrong + // with a candidate that only resolved on its timer: + // + // 1. ORDER — xterm emits '\r' synchronously from the Enter keydown, and the + // local-echo composer submits `pendingText` right there. The recovered + // character arrived one macrotask too late to be part of the prompt. + // 2. LOSS — that '\r' bumps the canonical counter, so by the time the + // candidate resolved, `canonicalCount > snapshot` read as "xterm spoke + // for this keystroke" and stood the recovery down. The character was + // dropped outright: every message sent from the phone lost its last + // character. + // + // Resolving pending candidates synchronously at the NEXT keydown fixes both: + // the counter still holds the value it had when that candidate was created, + // and the byte reaches the composer ahead of the Enter. + const h = harness(); + h.keydown(); + h.input('o'); + expect(h.emitted).toEqual([]); + + h.keydown({ key: 'Enter' }); + expect(h.emitted).toEqual(['o']); + + // xterm now emits '\r' for the Enter. The already-resolved candidate must + // not fire a second time when its timer is flushed. + h.controller.notifyCanonicalData(); + h.flushTimers(); + expect(h.emitted).toEqual(['o']); + expect(h.pendingTimers()).toBe(0); + }); + + it('still stands down at the next keydown when xterm spoke for the candidate', () => { + // The synchronous resolve must not become a "forward everything" path: a + // keystroke xterm delivered itself is still a duplicate if recovered. + const h = harness(); + h.keydown(); + h.input('x'); + h.controller.notifyCanonicalData(); + h.keydown({ key: 'Enter' }); + h.flushTimers(); + expect(h.emitted).toEqual([]); + }); + it('ignores input events that are not committed text', () => { const h = harness(); for (const inputType of ['insertCompositionText', 'deleteContentBackward', 'insertLineBreak', 'insertFromPaste']) { From da933d70bedbf501bf689e2eeb57e88d8c324527 Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Wed, 16 Sep 2026 08:05:55 +0200 Subject: [PATCH 04/18] feat(sessions): offer to rebuild the sessions a host reboot destroyed A host reboot takes the tmux server down with it, so every pane dies, reconciliation finds nothing to attach to, and the board comes up empty. Picking yesterday's work back up meant finding each conversation in history and resuming it by hand, one at a time. The boot pass now works out what the reboot killed and leaves it on offer. It runs inside restoreMuxSessions(), in the window where reconciliation has reported the dead sessions and cleanupStaleSessions() has not pruned their records yet, which is the only place the records can still be read. The board shows a banner, and nothing is created until the user clicks it. A click rather than an automatic restore is what makes the reboot heuristic acceptable. The heuristic cannot tell a reboot from a crash that took tmux down inside the same window, so it decides whether to ASK, never whether to act: a wrong yes costs a line of text the user dismisses instead of N CLI processes nobody asked for. Four things are re-checked when the click arrives rather than trusted from boot, because hours can pass and the board moves on. The owner's privilege grant re-resolves through the env clamp. The workspace must still be on disk. A conversation the user already resumed by hand from the Resume list is skipped, since two panes running --resume on one conversation would fight over the same transcript. Entries leave the plan synchronously before the first await, and the route is single-flighted, so a double-click or two devices cannot both reach the same entry. A restored session comes back attached, idle and disarmed. Respawn controllers and Ralph loops are deliberately not re-armed: a machine that just came up is the worst moment to turn an autonomous run loose. Its workspace hooks are installed by the restore route itself, because the boot-time sweep sits behind a gate that is false after a reboot and has finished long before the click; without them a session goes silently blind, with no stop or idle events for respawn, no Approvals Inbox item and no red tab on a blocking dialog. Stats collection starts the same way. The pane is new, so the conversation continues and the terminal scrollback does not. The banner says so rather than letting an empty pane read as a broken restore. The plan lives in memory only. A server restart drops it, which costs the convenience this adds and never the conversation: the conversation is the transcript under ~/.claude/projects, which the Welcome screen's Resume list and the Session Manager already read, so a dropped plan returns the user to resuming by hand. clampEnvOverridesForOwner moves to src/session-env-clamp.ts, since the question it answers is about session privilege rather than about HTTP and it now has a caller outside the route layer. Its test hook stays re-exported from session-routes.ts. Claude sessions only for this pass. The other CLIs name their thread in their own config object, which this does not thread through yet. Remote and docker sessions are skipped on purpose, because both need another host or a container to be up and a freshly booted machine cannot promise either. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/reboot-restore-banner.md | 5 + src/reboot-restore.ts | 225 +++++++++++++ src/session-env-clamp.ts | 91 ++++++ src/web/public/app.js | 2 + src/web/public/index.html | 19 ++ src/web/public/reboot-restore-ui.js | 95 ++++++ src/web/public/styles.css | 75 +++++ src/web/reboot-restore-registry.ts | 142 +++++++++ src/web/routes/index.ts | 1 + src/web/routes/reboot-restore-routes.ts | 194 ++++++++++++ src/web/routes/session-routes.ts | 67 +--- src/web/schemas.ts | 14 + src/web/server.ts | 57 +++- test/reboot-restore.test.ts | 367 ++++++++++++++++++++++ test/routes/reboot-restore-routes.test.ts | 175 +++++++++++ 15 files changed, 1462 insertions(+), 67 deletions(-) create mode 100644 .changeset/reboot-restore-banner.md create mode 100644 src/reboot-restore.ts create mode 100644 src/session-env-clamp.ts create mode 100644 src/web/public/reboot-restore-ui.js create mode 100644 src/web/reboot-restore-registry.ts create mode 100644 src/web/routes/reboot-restore-routes.ts create mode 100644 test/reboot-restore.test.ts create mode 100644 test/routes/reboot-restore-routes.test.ts diff --git a/.changeset/reboot-restore-banner.md b/.changeset/reboot-restore-banner.md new file mode 100644 index 00000000..33942048 --- /dev/null +++ b/.changeset/reboot-restore-banner.md @@ -0,0 +1,5 @@ +--- +'aicodeman': minor +--- + +Offer to rebuild the sessions a host reboot destroyed. A reboot takes the tmux server down with it, so every pane dies and the board comes up empty. Codeman now works out what was running, and the board offers to restore it behind a click. The conversations come back; the terminal scrollback does not, and the banner says so. diff --git a/src/reboot-restore.ts b/src/reboot-restore.ts new file mode 100644 index 00000000..c8b6bd5e --- /dev/null +++ b/src/reboot-restore.ts @@ -0,0 +1,225 @@ +/** + * @fileoverview Decide which sessions a host reboot destroyed and may be rebuilt. + * + * A server restart and a host reboot both leave `reconcileSessions()` reporting + * dead sessions, and they need opposite handling. A server restart leaves the + * tmux panes running, so recovery ATTACHES to them. A host reboot takes the tmux + * server down with it, so there is nothing to attach to and the pane has to be + * created again. This module holds the decision half of that second case, kept + * free of tmux and disk access so it can be unit tested without either. Every + * observation it reads is gathered by the caller and passed in. + * + * "Eligible" here means a session the user did not end on purpose. The rule that + * an intentional kill or detach is never auto-revived is enforced at runtime by + * an in-memory guard in `TmuxManager`, and memory does not survive a reboot. The + * durable equivalent is the record `cleanupSession()` leaves behind. An unpinned + * kill deletes the record outright, so it is already absent here. A pinned kill + * goes through `demoteOrRemoveSession()` and lands as `status: 'stopped'`, which + * is the marker this module refuses. Pruning keeps a pinned record WITHOUT + * touching its status, so a pinned session a reboot killed still reads `idle` or + * `busy` and stays eligible. + * + * @dependencies types (SessionState), config/cli-registry + * @consumedby web/server (plan build at boot), web/routes/reboot-restore-routes + * + * @module reboot-restore + */ + +import type { SessionState } from './types.js'; +import { getCli } from './config/cli-registry/registry.js'; + +/** Session statuses a reboot restore may rebuild. `stopped` is the kill marker. */ +const RESTORABLE_STATUSES: ReadonlySet = new Set(['idle', 'busy', 'error']); + +/** Observations the reboot heuristic reads. Gathered by the caller, never here. */ +export interface RebootEvidence { + /** Sessions that still had a live pane during reconciliation. */ + livePaneCount: number; + /** Sessions reconciliation just marked dead. */ + deadSessionCount: number; + /** `os.uptime()`, in seconds. */ + uptimeSeconds: number; + /** Newest `lastActivityAt` across the persisted records, in ms since the epoch. */ + newestPersistedActivityAt: number; + /** `Date.now()` when the evidence was gathered, in ms. */ + now: number; +} + +/** + * Decide whether the machine plausibly rebooted rather than the server restarting. + * + * Two signals have to agree. The socket must hold no panes at all while state + * still lists sessions, which rules out an ordinary server restart. The host + * must also have booted after the newest persisted session activity, which is + * the corroboration `os.uptime()` provides cheaply. A wiped tmux socket on a + * long-uptime host fails the second test, so a user who killed the tmux server + * by hand does not get every session offered back to them. + * + * This heuristic decides whether to ASK, never whether to act. A wrong yes costs + * the user a banner they dismiss, because the restore itself waits for a click. + */ +export function looksLikeHostReboot(evidence: RebootEvidence): boolean { + if (evidence.deadSessionCount === 0) return false; + if (evidence.livePaneCount > 0) return false; + if (evidence.newestPersistedActivityAt <= 0) return false; + const bootedAt = evidence.now - evidence.uptimeSeconds * 1000; + return bootedAt > evidence.newestPersistedActivityAt; +} + +/** + * Pick the conversation the rebuilt pane should resume. + * + * The chain's tail is the newest conversation the session was holding, which is + * what a compact or a clear leaves behind; `resumeSessionId` covers a session + * that was itself started as a resume, and the session id is the original + * conversation for everything else. + */ +export function resolveResumeConversationId(state: SessionState): string { + const chain = state.claudeSessionChain; + const chainTail = Array.isArray(chain) && chain.length > 0 ? chain[chain.length - 1] : undefined; + return chainTail || state.resumeSessionId || state.id; +} + +/** Why one session was passed over. Reported for logging and assertions. */ +export interface RebootRestoreRejection { + sessionId: string; + reason: + | 'no-persisted-record' + | 'intentionally-ended' + | 'respawn-blocked' + | 'remote-or-docker' + | 'unsupported-mode' + | 'no-working-dir' + | 'workspace-missing' + | 'already-live'; +} + +/** One restorable session, as the banner shows it and the rebuild replays it. */ +export interface RebootRestoreEntry { + sessionId: string; + name?: string; + workingDir: string; + owner?: string; + mode: string; + /** The conversation the rebuilt pane resumes. */ + resumeConversationId: string; + /** + * The persisted record, kept whole so the rebuild can replay what it held. + * Read at boot, before pruning deletes it, and held in memory until the click. + */ + state: SessionState; +} + +export interface RebootRestorePlan { + restore: RebootRestoreEntry[]; + skipped: RebootRestoreRejection[]; +} + +/** + * Split the sessions reconciliation just killed into the ones a reboot restore + * may offer and the ones it must leave alone. + * + * @param deadSessionIds Session ids `reconcileSessions()` reported as dead. + * @param persisted The `state.json` session records, which `cleanupStaleSessions()` + * has not pruned yet at the point this runs. + * @param workspaceExists Whether a working directory is still on disk. A tmux + * session can outlive its deleted repo, and rebuilding one there would scaffold + * an empty tree. The caller owns the disk access; the click re-checks, because + * a repo can be deleted between the boot and the click. + */ +export function planRebootRestore( + deadSessionIds: readonly string[], + persisted: Readonly>, + workspaceExists: (workingDir: string) => boolean +): RebootRestorePlan { + const restore: RebootRestoreEntry[] = []; + const skipped: RebootRestoreRejection[] = []; + + for (const sessionId of deadSessionIds) { + const state = persisted[sessionId]; + if (!state) { + // An unpinned kill already deleted the record, so absence IS the guard. + skipped.push({ sessionId, reason: 'no-persisted-record' }); + continue; + } + if (!RESTORABLE_STATUSES.has(state.status)) { + // A pinned kill was demoted to `stopped`. Reviving it would undo the kill. + skipped.push({ sessionId, reason: 'intentionally-ended' }); + continue; + } + if (state.respawnBlocked === true) { + // The crash-loop breaker tripped on this pane. Re-creating it restarts the loop. + skipped.push({ sessionId, reason: 'respawn-blocked' }); + continue; + } + if (state.remote || state.docker) { + // Both need another host or a container to be up, which a just-booted machine + // cannot promise. The remote reconnect watcher owns the remote case already. + skipped.push({ sessionId, reason: 'remote-or-docker' }); + continue; + } + // Capability, not a CLI id: this pass resumes by handing the CLI a conversation + // id through the top-level `resumeSessionId`, which only a CLI whose history the + // claude-jsonl reader understands can consume that way. Others carry their thread + // id in their own `Config`, which this pass does not thread through. + if (getCli(state.mode ?? 'claude')?.capabilities.transcript !== 'claude-jsonl') { + skipped.push({ sessionId, reason: 'unsupported-mode' }); + continue; + } + if (!state.workingDir) { + skipped.push({ sessionId, reason: 'no-working-dir' }); + continue; + } + if (!workspaceExists(state.workingDir)) { + skipped.push({ sessionId, reason: 'workspace-missing' }); + continue; + } + restore.push({ + sessionId, + name: state.name, + workingDir: state.workingDir, + owner: state.owner, + mode: state.mode ?? 'claude', + resumeConversationId: resolveResumeConversationId(state), + state, + }); + } + + return { restore, skipped }; +} + +/** + * Drop the entries whose conversation is already on screen. + * + * Hours can pass between the boot that built the plan and the click that spends + * it, and the Resume list can reach the same conversation in the meantime. Two + * panes running `claude --resume` on one conversation is the failure this + * prevents, so a match on either the session id or the conversation id is enough + * to skip the entry. + */ +export function rejectAlreadyLive( + entries: readonly RebootRestoreEntry[], + liveSessionIds: ReadonlySet, + liveConversationIds: ReadonlySet +): RebootRestorePlan { + const restore: RebootRestoreEntry[] = []; + const skipped: RebootRestoreRejection[] = []; + for (const entry of entries) { + if (liveSessionIds.has(entry.sessionId) || liveConversationIds.has(entry.resumeConversationId)) { + skipped.push({ sessionId: entry.sessionId, reason: 'already-live' }); + continue; + } + restore.push(entry); + } + return { restore, skipped }; +} + +/** Newest `lastActivityAt` across persisted records, or 0 when there are none. */ +export function newestPersistedActivity(persisted: Readonly>): number { + let newest = 0; + for (const state of Object.values(persisted)) { + const stamp = state.lastActivityAt ?? state.createdAt ?? 0; + if (stamp > newest) newest = stamp; + } + return newest; +} diff --git a/src/session-env-clamp.ts b/src/session-env-clamp.ts new file mode 100644 index 00000000..edaecd5c --- /dev/null +++ b/src/session-env-clamp.ts @@ -0,0 +1,91 @@ +/** + * @fileoverview The env-var half of the multi-user privilege clamp. + * + * A session's `envOverrides` can hand back privilege that the per-CLI config + * clamp removed, so a non-granted owner's overrides get the privileged keys + * stripped before the session is built. Two callers need that today. The create + * and resume routes clamp what a request asked for, and the reboot-restore route + * clamps what a persisted record carried, because a record written while its + * owner held a grant must not replay that grant after the grant is gone. + * + * This lives outside `web/routes` on purpose. The question it answers is about + * session privilege rather than about HTTP, and `cron/cron-service.ts` sets the + * precedent by importing `canUsernameRunPrivilegedCommands` from `user-store.ts` + * directly and re-resolving the owner's grant when a job fires. Every caller here + * re-resolves the grant at the moment it builds a session, for the same reason. + * + * @dependencies user-store (canUsernameRunPrivilegedCommands), config/cli-registry + * @consumedby web/routes/session-routes, web/routes/reboot-restore-routes + * + * @module session-env-clamp + */ + +import { canUsernameRunPrivilegedCommands } from './user-store.js'; +import { enabledClis } from './config/cli-registry/registry.js'; + +/** + * Env-var keys a non-granted owner must not be able to set, because each one + * hands back privilege `clampExternalCliBypassForOwner()` just removed, or redirects a + * credential-resolution endpoint. + * + * The DeepSeek three are reachable because `DSH_*` and `DEEPSEEK_*` are + * allowlisted `envOverrides` prefixes (schemas.ts) — which they have to be, since + * that is also how a user configures the harness's non-privileged knobs. + * + * - `DSH_PERMISSION_MODE` IS the harness's permission switch. Every other CLI's + * bypass is a command-line FLAG, reachable only through the per-CLI config the + * clamp already owns; this one is an env var, so the config clamp alone is + * half a gate. + * - `DSH_HOME` points the launcher at a profile tree, and a profile's plugin code + * executes at BOOT, before any approval row can apply. A user who can write a + * workspace can put a profile in it, so this is the wider of the two. + * - `DEEPSEEK_BASE_URL` aims the provider endpoint, and `_configureCliEnv()` + * forwards the SERVER's own `DEEPSEEK_API_KEY` into every dsh pane before + * `applyEnvOverrides()` runs — so a non-granted owner who could set the base + * URL would have the operator's API key sent as a bearer credential to a host + * of their choosing. (`DEEPSEEK_API_KEY` itself stays overridable: supplying + * your OWN key removes privilege rather than granting it.) + * - `OMP_AUTH_BROKER_URL`/`OMP_AUTH_BROKER_TOKEN` are where omp resolves + * credentials from — the same shape as `DEEPSEEK_BASE_URL` above, reachable + * because `OMP_*` is an allowlisted prefix. Unlike DeepSeek, Codeman does not + * forward any operator-held key into an omp pane today (omp's provider + * credentials live in `~/.omp` config files, not env vars), so there is no + * known concrete exfiltration path yet — clamped defensively anyway, since a + * non-granted owner redirecting where a shared multi-tenant deployment + * resolves auth from is not something to allow silently (found in + * Ark0N/Codeman#353 review; omp's own knobs are otherwise mostly `PI_*`, + * already allowlisted for pi and not addressed here — see resolveOmpHome()). + */ +export function ownerClampedEnvKeys(): string[] { + return enabledClis().flatMap((entry) => entry.capabilities.privilegedEnvKeys); +} + +/** + * Env-var half of the multi-user bypass clamp. + * + * `clampExternalCliBypassForOwner()` in `web/routes/session-routes.ts` clamps the + * per-CLI CONFIG, and for every CLI + * but DeepSeek that is the whole story. Here it is not: `applyEnvOverrides()` runs + * AFTER `_configureCliEnv()` in tmux-manager, so an override sent on the SAME + * request lands last and wins, and a non-granted owner could restore + * `danger-full-access` on the very request the config clamp downgraded. + * + * Keys are DROPPED rather than rewritten: dropping falls through to what + * `_configureCliEnv()` exports, which is the clamped config and the server's own + * `DSH_HOME`, i.e. exactly the intended state. No-op in single-user mode and for a + * granted owner, like every other clamp here + * (`canUsernameRunPrivilegedCommands()` returns true when `!isMultiUserMode()`), + * and it returns the caller's own object untouched when there is nothing to strip. + */ +export async function clampEnvOverridesForOwner( + owner: string | undefined, + envOverrides: Record | undefined +): Promise | undefined> { + if (!envOverrides) return envOverrides; + const keys = ownerClampedEnvKeys(); + if (!keys.some((key) => key in envOverrides)) return envOverrides; + if (await canUsernameRunPrivilegedCommands(owner)) return envOverrides; + const clamped = { ...envOverrides }; + for (const key of keys) delete clamped[key]; + return clamped; +} diff --git a/src/web/public/app.js b/src/web/public/app.js index 4c688899..1f0ae409 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -957,6 +957,8 @@ class CodemanApp { this.registerServiceWorker(); // Fetch tunnel status for header indicator (desktop only) this.loadTunnelStatus(); + // Ask whether a host reboot left sessions worth rebuilding (banner, never automatic) + this.initRebootRestoreBanner?.(); // Share a single settings fetch between both consumers const settingsPromise = fetch('/api/settings').then(r => r.ok ? r.json() : null).then(env => env?.data ?? null).catch(() => null); this.loadQuickStartCases(null, settingsPromise); diff --git a/src/web/public/index.html b/src/web/public/index.html index 2ba80243..cbe9a480 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -213,6 +213,24 @@ + + +