Review fixes for #469.
The Ctrl+C branch cleaned the selection to decide whether to copy and then
passed that cleaned string to copyTerminalSelection(), which cleans again. The
trailing trim is a fixed point, so that was safe until this PR; the margin
strip is not, because it takes the lesser of the declared width and the run
every line shares, so a second pass takes up to `margin` columns more. The
branch now gates on the cleaned string and hands the raw one on. Verified in
chromium with a real drag, a real Ctrl+C and a real clipboard read on a live
claude pane: an on-screen ` fix(terminal): trim it` reaches the clipboard
as ` fix(terminal): trim it`, and reverting the branch reproduces the
reported ` fix(terminal): trim it`.
Pane B of a split resolves its own width. `_cliGutterColumns()` and
`_normalisedSelectionRange()` take the session and the terminal to read,
defaulting to the primary pane's, so Pane B looks its own run mode up instead
of keeping a margin Pane A drops on the same keystroke. Verified live with two
claude panes open side by side.
A detached session window (`/session/:id`) receives the gutter map. The
injection sat inside the block that skips the run menu's payloads for a solo
window, so the toggle worked in the main window and did nothing in the popup on
the same device. It needs no availability probe, so it moved below that block
and the solo window still carries none of the payloads it skipped before.
The settings description said the width is measured and named Codex as exempt.
Nothing is measured, and Codex is one of the two panes that are stripped.
docs/wiki/Settings-Reference.md gains the row every Terminal and Input toggle
carries. CLAUDE.md no longer says the clean touches trailing runs "and nothing
else" one sentence before the leading-margin rule, and both it and
docs/architecture-invariants.md record that the strip is not idempotent.
Two round-trip tests run on a mode that declares a gutter, which the existing
copyTerminalSelection cases could not, since they all use the harness default
mode that declares none. The Ctrl+C branch itself is pinned at the source,
because it lives inside initTerminal's attachCustomKeyEventHandler closure over
a real xterm the vm harness cannot build. Both pins fail on the reintroduced
bug. Gate: 7865 passed, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_onSessionTerminal` drops an incoming frame once the app-owned render queues
already hold 128KB. That is the right call — the alternative is an unbounded
backlog — but a hole in a TUI byte stream is a desynced cursor, and a desynced
cursor is muffled text (#464). The drop was only half of it.
The recovery was a fire-and-forget timer: it nulled its own handle and then
called `_onSessionNeedsRefresh()`, which opens with four early returns. Two of
them — a buffer load already in flight, a refresh already owning this session —
are MOST likely to be true during exactly the output burst that caused the
drop. So the recovery was skipped precisely when it was needed, with nothing
left to retry it, and the dropped bytes were never replayed.
`_onSessionNeedsRefresh` reports whether it actually repainted now, and
`_scheduleDroppedOutputRecovery` re-arms while it has not. Bounded by
`DROP_RECOVERY_MAX_ATTEMPTS`, because every reason the refresh can be skipped is
transient contention that clears in seconds and a permanently failing refresh
must not become a loop against the API; giving up at the cap leaves exactly what
the old code left, so the floor is no worse than before. The same 2s debounce
still collapses a burst of drops into one attempt.
This is the principle Ark0N established reviewing #431 for the WebSocket
output-gap marker — only a repaint that actually happened settles the recovery —
applied to the one recovery path that still trusted a timer having fired.
The retry decision is a pure function in constants.js so the gate can reach it,
and the scheduler itself is driven from app.js under a fake clock. The retry
case and the no-retry case only pin the fix AS A PAIR: either alone passes
against something wrong, one against the old fire-and-forget timer and the other
against retrying forever. Checked by reverting app.js to the old shape, where
three of the twelve fail.
Two harness details that would otherwise have made the tests lie. The vm context
baked in the real `setTimeout`, so `vi.useFakeTimers()` could not reach the
scheduler and every case reported zero calls; it delegates lazily now. And
app.js reached `CodemanDroppedOutput` as a bare global, which resolves in a
browser but not in the vm — worth fixing beyond the test, because that call sits
inside a timer where a ReferenceError is swallowed and would take the recovery
with it. It reads through `window.` like terminal-ui.js does with its own
constants.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A dual review (Codex CLI and Claude's code-reviewer, same diff, same brief)
found the trust boundary weaker than the comments around it claimed. Eleven
findings, all applied.
The two blockers were both about who can write the row the label is read from.
Claude's window covered two rows, and the second one is the status line, whose
command a session running with permissions bypassed can write into its own
`.claude/settings.json` — so an agent could print `· 1 monitor ·` onto a row of
its own and silence its own idle alert. The default window is one row now, which
is the footer and nothing else, and the constant says why. Separately, the label
reached `data-tab-meta-sig` unescaped while the row is installed with innerHTML,
which is an injection sink for any config-supplied pattern whose capture group is
permissive; it goes through escapeHtml() like every other untrusted string in
that file.
The Codex entry could not be fixed the same way, and now says so. Its row is
third from the bottom only while a terminal runs; with none running that slot
holds the last row of the transcript, so matching the complete row (with the
`/stop to close` tail, window narrowed to three) raises the bar without closing
it. What contains it is `hooks: 'none'`: no hook event from a codex session
reaches notePrompt(), so a forged label costs a wrong badge and cannot quiet an
alert. The registry comment, `docs/cli-registry.md` and the test all state that
rather than claiming a guarantee the code does not have.
Also from the review: the TUI header badge no longer counts an acknowledged
item, which was the same gate the classifier fix already went through and was
wrong for human acknowledgement too; the TUI approval card reads the quiet
reason and drops to a new `info` tone instead of asking for a reply; the badge
carries an aria-label, because the phone it was built for has no hover target;
the schema refuses `watchingLines` without a `watchingLine`; and the pattern and
its window are resolved together rather than one memoized and one not.
Documentation moved with it. The mechanism now lives in
`docs/architecture-invariants.md` with CLAUDE.md keeping the rule and a pointer,
`docs/wiki/Notifications-And-Approvals.md` tells users why a session stopped
buzzing, and both that page and the changeset name the limitation neither did
before: a question asked in plain prose is not a dialog, so it is silenced along
with the false alarms while background work runs.
Verified live again after the narrowing, on an isolated beta: a Claude session
reported `1 monitor` and took its idle prompt acknowledged, and a Codex session
reported `1 background terminal` against the full-row anchor.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ten findings from a two-model review of this branch. Both reviewers cleared the
detection logic itself; everything here is a gap around it.
A route that starts a command in a pane now PERSISTS as well as broadcasts.
`/interactive` and `/shell` did neither before, and the pane-exit watcher cannot
cover for them: its next tick finds `paneExit` already cleared in memory,
reports no change and writes nothing, so `state.json` kept saying the agent had
exited for as long as the session stayed quiet. Nothing reads that record for a
decision yet, which is exactly why it had to be fixed now — part 2 is designed
to read it. The `clearPaneExitForNewPane()` docstring claimed its callers
already persisted; that claim was false for these two, and now says what the
caller owes instead.
The watcher's four guards were unreachable by any test. `refreshPaneExits()`
opened with `if (IS_TEST_MODE) return;`, so the read gate, the in-flight
suppression, the generation counter and the empty-read rule could each be
deleted with the whole suite green. The tmux call moves into `readPaneRows()`,
which a test subclass overrides — the shape `runRemoteReconnectTick` already
uses in this file for the same reason — and the test-mode gate moves with it, so
what a test cannot do is spawn a process rather than exercise the bookkeeping.
Each of the four guards now has a test that fails when it is deleted.
The muted status dot turned out to be a specificity fight on three surfaces, not
two. `.tab-status.error` was not excluded, so a session whose agent exited and
whose PTY-exit breaker then tripped lost its red dot to the mute — the state the
browser answers with a "restart it?" confirm, and a needs-you colour by the same
argument that protects the two alert classes. And mobile.css gives a `busy` dot
a 9px size and a green glow with `!important`, while `status` stays `busy` for a
pane whose agent died mid-turn, so a phone rendered a grey dot still wearing the
green halo beside a badge reading "exited". Both measured against the real
stylesheets, both now excluded, and the CSS test reads mobile.css too instead of
being structurally blind to half the problem.
Six comments said things that were not true. Two named the stats collector as
what replaces a restored reading, which is the opposite of the design. The
interval constant argued that 2000 ms keeps a read inside a tick, when the
5000 ms exec timeout means it cannot — which is why the in-flight guard exists.
`MuxSession.discovered` did not say the flag is permanent, though `saveSessions()`
serializes it. The empty-read docstring claimed a distinction that `|| true`
makes impossible. The invariants doc promised more than its drift test delivers.
And CLAUDE.md had no pointer at all, leaving its two hardest prohibitions
("never set `status: 'error'`", "never null the pid") only in the file it is
meant to route people to.
Refs Ark0N/Codeman#446.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five items, two of which he could only see by running it, plus six smaller
ones. Taking the two blockers first, because both were wrong in ways the
existing tests could not catch.
**Adopting the PTY's rows put the CLI's input line off-screen.** A phone that
took a desktop's 43 rows into a viewport with room for 18 painted an
`.xterm-screen` far taller than its container; xterm's own viewport then had
nothing to scroll, so the bottom of the frame sat below the container with no
gesture able to reach it. Output visible, typing invisible, for as long as the
desktop kept the claim hot. `reconcilePtyGeometry` adopts COLUMNS ONLY now:
width is the axis Ink's wrap and `eraseLines` arithmetic depend on, and keeping
the local row count keeps the composer at the bottom of a viewport that
scrolls. Measured at his geometry — a 360x300 container against a 198x43 pane
now keeps 13 rows, takes 198 columns, paints 202px into a 210px container, and
the input line is inside the box.
**`capture-geometry-retry.browser.test.ts` failed, and CI could not see it**
because the file is in `BROWSER_TEST_GLOBS`. Its premise WAS the clamp —
`getTerminalDimensions()` floored while `fitAddon.fit()` did not — which this
work removes at the source, so it can never hold again at any viewport. The
case survives on its own terms: a pane already drawing at the requested size
must not be replayed. Its premise is now the #464 invariant itself, that the
floored report and the terminal agree, which is a stronger guard because the
clamp coming back fails it here rather than silently restoring the replay loop.
The helper docblock that repeated the old premise is corrected too.
**A session with no pane reported 120x40 and the client adopted it.**
`resize()` writes `_ptyCols`/`_ptyRows` only when `ptyProcess` is set and
nothing seeds them from the spawn geometry, so a dead-pane session still held
the constructor defaults — clicking that tab resized the browser terminal to
120x40 and, on anything narrower, claimed another device owned the pane when
none existed. `Session.ptyGeometry` returns null without a pane, the HTTP route
answers `{}` and the socket sends no frame at all. The raw `ptyCols`/`ptyRows`
getters are deleted rather than left available to be misused again.
**The 40-column floor clipped the pane with nothing able to reach it.** The
affordance keyed on a PTY mismatch, and the floor produces no mismatch — xterm
and the PTY agree throughout, the terminal is simply wider than the box. It
keys on what does not FIT now, MEASURED (`.xterm-screen` against the container,
on the next frame, because the screen takes its width with the render) rather
than derived from cell arithmetic. Measured at 360px: font 24 applies 40
columns and paints 560px, and all 200px of the overhang is reachable.
`.pty-oversized` is renamed `.term-overflows-x`, because after this the old
name describes only one of the two causes.
**"Scroll sideways" did not work on touch for the sessions it targets.**
`touch-action: pan-x` is cancelled before it starts by the `preventDefault()`
`touchstart` calls on every 'content' tap. The terminal's own touchmove handler
pans the container now, with the axis locked once per gesture so a diagonal
cannot pan and scroll at once, and the CSS grants no `touch-action` at all —
handing the browser a pan AS WELL would move the pane twice for one finger on
the taps where that preventDefault does not run. Measured under real touch
dispatch: a 140px swipe reaches `scrollLeft` 140 where it reached 0 before, the
buffer does not move with it, and a vertical swipe still scrolls the scrollback.
Three defects in the above, found while checking it rather than by being told:
- `canPanHorizontally` first tested `scrollWidth > clientWidth` alone, which is
true of a container that is not a scroller — a sideways swipe would have
locked the axis, done nothing, AND suppressed the vertical scroll it should
have been. Gated on the class as well.
- The notice advised scrolling sideways whenever the PTY was wider, including
when it still fitted and nothing scrolled. It is gated on measured overflow,
and on a comparison against the width this container WOULD request rather
than the one it currently holds — once adopted those are equal, so the second
question answers itself false while the condition is still true.
- `_syncTerminalOverflowAffordance` could throw out of `document.getElementById`
before reaching its try block. It runs off every geometry change, so a
cosmetic affordance could have taken the resize down with it.
The smaller items:
- `docs/architecture-invariants.md` no longer explains the equality guard as a
clamp signature; it records what the clamp used to do and why it cannot any
more. Edited by hand — that file is outside the Prettier glob, and letting
Prettier near it rewrote eleven unrelated emphasis markers.
- `throttledResize`'s HTTP fallback reads the reply. It is the path where a
declined resize is least likely to be noticed, because no socket means no
`{"t":"zc"}` frame either.
- The changeset covers the whole release: the geometry work, the queued replay
clear, the renderer watchdog, the body-covering fetch deadline, the WebSocket
output-gap reconcile, the build-generated service-worker precache and
per-build cache key, and the crash-trail hygiene.
- `@xterm/headless` is declared in the root devDependencies instead of being
reached through workspace hoisting.
- The output-gap marker is cleared after any response arrives, not only when
the capture was non-empty: a server that answers with an empty capture HAS
reconciled us, and leaving the marker set refetched on every reconnect.
- `e587d845`'s message claimed a test asserted the failed-load copy against the
built asset. It did not — that assertion lived in a probe deleted with the
other scratch scripts, so the claim was false when it was written. There is a
real test now, and it reads the source rather than `dist/`, because `dist/` is
not committed and a test that skips when it is absent would pass for the wrong
reason in CI.
`Session.ptyGeometry` gets behavioural coverage against the real class in
`session-resize-arbitration.test.ts` rather than a source guard, including the
contrast — a pane that does exist still reports, and still follows a resize —
so "always null" would fail it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copying a paragraph out of a Claude Code or Codex pane puts that pane's own
two-column transcript gutter on the clipboard, so every pasted line arrives
indented. #451 shipped the trailing half of the copy clean and left the leading
half out, because deriving the width from the selection fires on 73% of ordinary
indented text and cannot tell a margin from content.
The width is DECLARED rather than derived. `capabilities.transcriptGutter` on
the CLI registry is a bounded integer; claude and codex each declare 2, measured
on live panes, and no other stock entry declares any, so a CLI whose transcript
layout nobody has measured is never touched. The server publishes the map as
`window.__codemanTranscriptGutter`, built by filtering `enabledClis()` on the
capability rather than by listing ids, and `_activeCliGutterColumns()` looks the
active session's mode up in it. The copy path reads no terminal buffer at all.
The declared width is a CEILING, not the answer: `clean()` strips the lesser of
it and the run every selected line shares. A block can therefore only shift as a
unit, the structure inside a selection survives by construction, and a selection
reaching column 0 loses nothing. That is what keeps a `git log` body at its own
four-space indent inside an agent's two-column gutter.
Codex was measured separately, because it renders nothing like Claude: it draws
boxes narrower than the pane and pushes its transcript into ordinary scrollback.
On a live 0.154.0 answer its `•`/`›`/`⚠` markers sit in the gutter, prose
continuations sit at 2, and a nested YAML block the model wrote rendered at
2/4/6/8 for its own 0/2/4/6. Replayed at 100, 120, 160, 198, 235 and 282 columns
its indents were 0, 2, 4, 6 and 8 at every one, never 1. Copying that YAML out
of a live Codex pane now yields 0/2/4/6: gutter gone, nesting intact.
Two derived versions were built and measured first, and both are recorded in the
code because both looked correct:
- Painted trailing padding — a full-screen TUI writes real spaces across the
unused part of a row, a shell leaves them never-written for xterm to trim —
has no false positives and never over-stripped. It is also a function of pane
WIDTH: the padding exists only while a rendered line stops short of the CLI's
own layout width, and Claude's prose wraps to fill it. Dragging the same two
prose rows of one live transcript at five window sizes, the share of padded
rows ran 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 columns, so the
strip silently did nothing at every ordinary size while a corpus captured
entirely at 282 columns said it worked.
- Taking the narrowest indent on the rows around the selection fires at every
width and over-strips about 1% of selections, because a file listing inside
the transcript can be the narrowest thing on screen.
Measured over 1,392,281 selections — every 1, 2, 3, 5, 10 and 20-row window of
real Claude screens replayed from live PTY streams at 100, 120, 160, 198, 235
and 282 columns — the declared width over-strips none, breaks no relative indent
and alters no text, and serves 100% of the selections whose own indent covers
the gutter. Verified end to end in a browser with a real mouse drag and a real
Ctrl+C: Claude and Codex panes paste flush at 123, 198 and 298 columns, a shell
pane is untouched at every one.
The strip sits behind `copyStripMargin` (App Settings, Selection & clipboard),
per-device and default ON: a display key, absent from the .strict()
SettingsUpdateSchema, read as `!== false` because the desktop branch of
getDefaultSettings() returns {}. The toggle is checked before the map.
Two review findings from #451, handled:
- The mid-row flag governs ONE line now. `range.start.x > 0` excludes only the
first selected line, the one whose margin the mousedown genuinely cut off, so
the same three rows no longer produce three different clipboard results.
- The reversed-drag finding does not reproduce on the pinned xterm.
`getSelectionPosition()` reads `_selectionService.selectionStart`, whose
getter returns `SelectionModel.finalSelectionStart`, and that swaps the pair
when `areSelectionValuesReversed()` says so. A real upward mouse drag through
chromium against xterm 6.0 reports the same range as the downward drag.
`_normalisedSelectionRange()` keeps the ordering as a guard, because the model
one layer down exposes the unnormalised fields under the same two names.
Tests: test/terminal-copy-clean.test.ts (64, up from 31), plus the injected
script stripped in test/server-index-title.test.ts. Every guard is pinned:
removing any one of seven reds at least one test, including declaring the wrong
gutter width. Full suite green, 7,861 passed, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A third pass in a real browser, at the widths this app actually renders at.
The notice a failed history load writes into the blanked pane was one
70-character sentence. At 430px that exactly filled the line; at 320px it
wrapped and left a lone '.' on a line of its own. The floor this app will
render at is 40 columns — reachable today by raising the font on a phone — so
the notice is three lines now, none over 25 columns, one fact each: what
failed, that the session is still alive, and what to do.
It says RELOAD rather than "reopen the tab" because `selectSession`
early-returns when the session is already active, so clicking the tab you are
already on retries nothing. The earlier wording named no next step at all,
which left a mostly-empty terminal and no way out of it.
CLAUDE.md no longer cites "758px reachable to the right" as evidence: that
figure is a property of the test content, not of the fix, and the file's value
is that a reader can trust a claim without re-deriving it. What is pinned
instead is the invariant that survives any content — the full pane width is
reachable, and removing the class returns scrollLeft to 0, so a resolved
mismatch cannot leave the pane parked off-screen.
Verified at 430, 360 and 320px against the shipped bundle, with the test
asserting the built asset carries the copy so an edit that never reached the
build fails rather than passing on the source's wording.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Issue #464, "text gets muffled sometimes, in both TUI default and fullscreen".
The screenshot is not a dropped frame or a frozen renderer — it is arithmetic.
Claude Code's TUI wraps its frame at the width the PTY reported and erases the
previous frame by walking the cursor up the rows it believes that frame took. A
browser terminal of a different width makes each logical line occupy more
physical rows than Ink counted, so `eraseLines(n)` clears too few and the new
frame paints over rows nothing erased: doubled lines, and short tool summaries
sitting inside longer prose rows with the prose's tail still visible.
Reproduced against this repo's own xterm before changing anything — a 120-column
PTY against a 62-column terminal renders every wrapped line twice. `test/
terminal-pty-geometry.test.ts` pins that, and pins the clean render at matching
widths beside it, so the assertion cannot be satisfied by code that fixes
nothing.
Four ways the two drifted apart, none of them observable from either end:
1. `fitAddon.fit()` resizes xterm to `proposeDimensions()` RAW while every
server-facing path reported those floored at 40x10. Measured in Chrome at
430px: font size 44 proposed 13 columns, the server was told 40, and xterm
stayed at 13. Three call sites each did their own fit-then-floor, and two
re-read the proposal after the fit — `_shrinkPaddingToFit()` runs exactly
there, so the container had moved.
2. `throttledResize` (keyboard up) and `sendResize` (session detached into its
own window) reflowed locally and withheld only the SIGWINCH. That is the one
combination that cannot be right: a reflow nothing is rendering for buys
nothing and costs correctness. Both now withhold everything, and the
keyboard's settle timer still sends the one resize that stops the PTY going
stale.
3. `setFontSize`/`setFontFamily`/`setFontWeight` move the cell size — a geometry
change — and told the server nothing at all, so raising the font on a phone
left the CLI wrapping at the old column count.
4. `Session.resize` DECLINES a small-viewport request while a desktop connection
holds an active sizing claim, and said nothing, because resize was write-only.
`syncTerminalGeometry()` is now the one function that may change the terminal's
size: it fits, floors and applies as a single step, so the numbers xterm holds
are the numbers the server is told. A test sweeps every module for a bare
`fit()` on the main terminal, and finds exactly one — the owner's own.
For (4) the client cannot win, so it is told the truth instead: both transports
answer a resize with `session.ptyCols`/`ptyRows` (`{"t":"zc"}` on the socket,
the body of the resize POST) and `_onPtyGeometryReport` adopts them. A terminal
that keeps a shape the PTY refused does not render "too narrow", it renders
garbled. Adopting can leave the pane wider than the screen and the container is
`overflow: hidden`, so `.pty-oversized` grants horizontal reach for exactly as
long as the mismatch lasts: correct-and-reachable beats correct-and-clipped
beats garbled. That rule sets both overflow axes and its own `touch-action`
because mobile.css loads later and sets `.terminal-container { overflow:
visible; touch-action: none }` — a bare `overflow-x` would leave overflow-y
computing to `auto` and hand the browser a vertical scroll container the
terminal's touch handler knows nothing about.
Verified in Chrome at 430px against a live server, with a desktop client holding
the claim: the phone adopts 198x43, gets `overflow-x: auto` / `overflow-y:
hidden` / `touch-action: pan-x`, 758px of reach to the right, and keeps its own
vertical scrolling. The pre-fix build was measured in the same harness for the
control.
Two things this deliberately does not do. It does not change who owns the pane
size — the desktop still wins, and `_startMobileResizeRetry` still takes it back
once that goes idle. And `throttledResize` still holds the PTY's shape for the
whole keyboard animation rather than sending a SIGWINCH per step; that decision
predates this and was not re-tested here.
Also in this commit, Ark0N's third-pass review items on #431:
- The response viewer's byte-buffer fallback and `_onSessionClearTerminal` both
used the no-param `/terminal` form, capped only by `terminalBufferMaxBytes`
(32MB) — the largest body the frontend asks for anywhere. One carried no
deadline at all and the other got the 15s tail budget. Both now take the
full-history budget.
- A `?full=1` capture that outruns its deadline falls back to the bounded tail.
The pane is blanked before that fetch, so an abort used to leave a black
rectangle, discard the queued live output and never reach `_connectWs`. A
failed load now still opens the socket, says one dim line where the content
would have been, and clears the tab's spinner — which nothing did, so a failed
select left `aria-busy="true"` set forever.
- `_wsOutputGapSession` is cleared at the repaint that settles it, not in a
`finally` that also ran on the catch. A reconcile that threw, or hit the new
deadline — the flaky link the marker exists for — dropped the gap with nothing
to retry it. `ws.onopen` no longer clears it up front either.
- The replay-clear invariant is pinned in the gate, which is the drift this PR
exists to fix: `_resetTerminalForReplay` must be a queued write and nothing
else, and no module may blank the terminal with a `clear()+reset()` pair.
- `DIAG_ENTRY_MAX_CHARS` replaces the hardcoded 300, bound through a local
first: `CodemanDiag?.x` still throws a ReferenceError when the identifier was
never declared, and that is the one function in the app that must not throw.
- panels-ui's two kill-all clears route through the same helper, and the
xterm-version guard's comment says "resolved lockfile version" rather than
"dependency RANGE", which is what it has pinned since the last round.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four changes the maintainer asked for on Ark0N/Codeman#446 before merging.
The pane-exit watcher stays always-on, but a tick now costs nothing when there
is nothing to observe. `hasObservablePaneSession()` skips the tmux exec while
every session on the manager is one of the shapes `Session.paneExitApplies`
already forces to UNKNOWN: a remote SSH session (its local pane holds the ssh
client), a docker case (a `docker exec` into the container's own tmux), and a
record rebuilt from the socket (no provenance at all). The timer is untouched.
Skipping retracts nothing, for the same reason a failed read does not: the map
still holds the last real reading, and every path that puts a new command in a
pane calls `clearPaneExit()` itself. The two copies of that rule are pinned
against each other in `test/session-pane-exit.test.ts`, because drift between
them is silent in both directions.
`DEFAULT_PANE_EXIT_INTERVAL_MS` was already a constant beside the stats and
remote-reconnect intervals; its comment now says why the watcher owns its own
cadence and why the number is what it is.
The never-default-an-absent-status rule is written where `PaneExit` is declared.
It names `status ?? 0` as the thing never to write, and says that an agent the
OOM killer took would otherwise read as a user typing `/exit` — which is what
absent-stays-absent keeps a later clean-exit sweep away from. Nothing fails when
somebody adds that `??`, which is why the sentence is there rather than a test.
Checking the dot's specificity found a second fight, and it was losing. On the
tab strip the alert rules win as intended: a session that exits with a
permission dialog pending still renders red, and yellow for an idle alert. On
the rich vertical tab rail they did not — that rail's own `tab-state-*` dot
rules are (0,9,1) against the strip's mute at (0,5,0), so an exited session
there kept a full green dot AND the working halo beside a badge reading
"exited". The rail twin matches that specificity exactly and therefore must stay
below those rules in source order; it clears the halo as well, which the strip's
rule never had to think about.
`test/session-pane-exit-ui.test.ts` now resolves the real stylesheet in jsdom
rather than matching selector text: postcss collects every rule that paints
`.tab-status`, a real engine decides, and the tests read back the answer. Two
mutations were run against it to prove it has teeth — dropping the hand-written
alert exclusions fails three cases, and moving the rail twin above the state
rules fails one.
Refs Ark0N/Codeman#446.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review fixes. Two of these are defects in the previous commit.
1. The fetch deadline only covered time-to-headers. `await fetch()` settles on
response headers, so clearing the abort timer in a finally around it left the
body — the multi-megabyte `?full=1` capture the deadline exists for —
completely unbounded; it only ever bounded a server that accepts a connection
and never replies. Measured against a server that sends headers immediately
and stalls the body 4s under a 1s deadline: fetch resolved at 30ms, timer
cleared there, body completed at 4026ms unaborted. Now the body is read
inside `_fetchTerminalCapture`, which returns {json, headers, headersAt} —
headers because two callers read server-timing, headersAt because those same
callers measure header-vs-body time and can no longer observe that moment.
`_terminalCaptureInflight` is scoped the same way, so a body still streaming
counts toward a capture starting beside it. Same test now aborts at 1005ms.
2. The precache could never be hit, and the previous commit made that expensive
rather than free. `renderIndexHtml` runs `cacheBustAssets`, which appends
`?v=<mtime>` to every same-origin .js/.css reference INCLUDING content-hashed
names — confirmed against a running instance:
`vendor/xterm-zerolag-input.6fee72f2.js?v=1789402869101`. `caches.match` is
query-sensitive, so entries keyed on the bare hashed path were unreachable;
deriving the list from the manifest turned cheap 404s into ~1.3MB downloaded
at every install that nothing could read back, once per deploy now that
CACHE_NAME rotates. The fallback match takes `{ ignoreSearch: true }`, which
also lets runtime-cached entries survive an mtime change.
3. `_wsOutputGapSession` was only cleared in ws.onopen, so paths that already
repaint the buffer left it set and the socket replayed everything a second
time. `selectSession` loads the buffer and only THEN calls `_connectWs`, so
neither the _isLoadingBuffer nor the _terminalRefreshOwner guard applied.
`_markTerminalBufferReconciled()` is now called from _onSessionNeedsRefresh's
finally, from selectSession after its load, and from _cleanupSessionData.
The scope claim was also wrong and is corrected in the comment: when the
network drops, SSE drops with it and handleInit's keepTerminal branch already
reconciles. The genuinely uncovered case is the WS dying while SSE stays up,
where _onSSETerminal discards SSE terminal frames until _wsReady flips in
onclose — up to the ping+pong window of output nothing writes.
4. CLAUDE.md said "all of them measured rather than reasoned", which the PR's
own "not verified" section contradicted. Split explicitly: the replay race is
measured, the watchdog mechanism is verified against xterm 6.0.0 under jsdom
(field path resolves, a forced stale handle makes refreshRows a no-op, the
kick schedules a fresh frame), and the iOS rAF-discard premise is reasoned
and still wants a device. Adds the two missing entries — the WebSocket
reconcile and the sw.js/build.mjs "keep these in sync or the build throws"
contract.
Also: test/xterm-private-api.test.ts pins the RESOLVED lockfile version instead
of the declared `^6.0.0` range, which was the wrong assertion in both directions
— a real upgrade to 6.4.0 can rename a private field while resolving inside the
range, and an innocuous range edit failed while changing nothing installed. And
test/sw-precache-manifest.test.ts now parses HASHABLE out of scripts/build.mjs
rather than hand-copying it, which was the same drift this PR exists to fix; the
parse is guarded against silently matching nothing.
The deadline fix has a behavioural test against a real socket plus a source
guard asserting `await res.json()` precedes the finally — verified to fail when
the helper is reverted to the old shape, so it is not vacuous.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four ways the terminal can silently stop being correct — in each case the
buffer keeps updating, nothing throws, and the only recourse is a reload.
1. Renderer freeze after backgrounding. iOS DISCARDS scheduled rAF callbacks
when a PWA backgrounds, and xterm's RenderDebouncer only clears its
`_animationFrame` handle from inside that callback — so one drop leaves it
permanently set and every later refresh() early-returns. Parsing is
decoupled from rendering, so bytes keep filling the buffer correctly while
nothing paints. Codeman has exactly ONE xterm for the whole page load, so a
single backgrounding wedges it until a reload. Adds a 2s liveness poll and
`_kickRenderer()`, which does what the dropped `_innerRefresh` would have.
2. Replay clears raced live output. xterm's write() is async-queued while
reset() is synchronous and, per upstream, "does not clear input buffers and
does not reset the parser" — so bytes queued before a reset are parsed after
it and fuse into the snapshot. Verified against the real xterm 6 here:
write('p8'); reset(); write('rmissions') renders "p8rmissions". The main
path was already safe via a queued erase; the needsRefresh and clearTerminal
paths were not. All three now share one queued `\x1bc` (RIS), which unlike
3J/H/2J also resets modes, charsets, scroll regions and SGR state.
3. Output lost on WebSocket reconnect. Input frames carry seq+cid and are
delivered exactly once; output frames carry nothing. ws.onopen re-sends dims
and flushes queued input, and needsRefresh only fires on external-CLI
startup and SSE backpressure drain — never on reconnect. Output produced
while offline was simply absent afterwards. Interim fix: reaching onclose
means the drop was unintentional, so the session is marked and the next open
reconciles from the server buffer. Sequencing output is the follow-up.
4. Terminal captures had no deadline. No AbortController anywhere in the
frontend, including `?full=1`, which the code itself calls "unbounded-ish
work: at the default history limit it can be megabytes". Adds a budget that
scales with full-vs-tail and with captures in flight, degrading to a plain
fetch where AbortController is missing.
Also: the service-worker precache was dead — the build content-hashes assets
but sw.js listed pre-hash names, so 15 of 23 entries 404'd (verified against a
running instance) and cache.add().catch() hid it. Offline still worked via
runtime caching, but CACHE_NAME was a constant so activate's cleanup never
deleted anything and every past release's assets accumulated. Both are now
derived from the build manifest. Crash-trail entries are flattened and capped,
since they are joined with \n into one value and one call site interpolates a
server-controlled WS close reason.
The watchdog reads xterm privates — there is no public API. Every access is
optional-chained so a shape change degrades to a no-op. `_renderService` only
exists after open(), which needs a real DOM, so the gate cannot assert the
field path; test/xterm-private-api.test.ts pins the dependency range instead.
Tests: 23 new (terminal-resilience, sw-precache-manifest, xterm-private-api),
all pure/static so they run in the gate, which excludes the mobile suite. One
static source guard in history-truncation-notice updated for the renamed call;
the behaviour it pins is unchanged.
Not verified: no browser available, so no runtime reproduction of the freeze
and no real-device test of the reconnect path. Both warrant a device pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The badge alone left the row in NEEDS YOU, which is the thing the issue was
about. The fix is the alert that does not fire.
An idle prompt from a session that is watching its own background work now
opens ALREADY acknowledged. `hook-event-routes` passes `Session.watching` to
`notePrompt()`, which sets `acknowledgedAt` and records why in a new
`acknowledgedReason`. Nothing new suppresses anything: `acknowledge()` has
always meant "the alert this prompt armed is spent", and the prompt itself
stays pending, answerable and available as Read My Mind context. A wrong label
therefore costs a card that does not blink, never an alert that was never
created.
Every surface follows from that. The broadcast carries the reason, so a live
page declines to arm the tab alert and raises no desktop notification. The push
is skipped, since a false alarm is hardest to ignore on a phone. A reloading
page reads `acknowledgedAt` in `seedApprovals()`, which it already did. And
`classifySession()` now reads it too, which is a pre-existing bug fixed here:
acknowledging on one device cleared the alert everywhere except `codeman tui`.
It re-arms for free, because the next idle prompt supersedes the item and is
built fresh. Only `idle` is eligible, so a dialog that blocks the agent still
goes red whatever else it started.
The label is pane-derived and therefore prompt-injectable, so it is now read
from the last two rows of the screen only, with Claude's pattern anchored on
the `·` its footer joins items with, ANSI-stripped and length-capped at the
source. An agent that prints `· 1 monitor ·` into its own output finds no
match.
Verified on an isolated beta: a session that armed a monitor took its idle
prompt acknowledged with no alert on any surface, wore the badge, and showed
"quiet, watching 1 monitor" on its still-answerable card; the same session with
the monitor killed alerted normally on the next prompt. `test/watching-no-alert.test.ts`
pins both directions across all four surfaces.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An agent that arms a monitor, backgrounds a shell or hands a task to a
cloud session is told to end its turn. The pane then falls quiet, Claude
Code's idle_prompt notification arrives a minute later, and every surface
files the session under NEEDS YOU with nothing for a human to answer.
Claude states what it is still running on the last row of its screen
(`⏵⏵ bypass permissions on · 1 monitor · ← for agents`). That row is now
`capabilities.workDetect.watchingLine` in the CLI registry, guarded by
compileVersionRegex() like every other config regex, and the idle probe
reads it off the capture it already takes: `watchingLabel()` in
session-activity.ts searches the last five lines only, so a session that
PRINTS "1 monitor" is not mistaken for one running it.
The label lands on Session.watching and rides toLightDetailedState() out
to every surface. The phone overview, the desktop home rail and the rich
sidebar rows wear it as a `watching` badge in the accent colour, beside
the state pill and never in place of it: an agent can arm a monitor and
ask a question in the same breath, and only the pill says which.
Verified end to end against a throwaway session on an isolated beta
instance: the payload carried `watching: "1 monitor"` once the turn
ended, the badge rendered next to a yellow `waiting` pill, and both
cleared when the monitor died.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tab now reads "exited (137)" beside the session name, drawn from the
`paneExit` field the server publishes. `applyPaneExitBadge()` owns the DOM
work, called from the incremental render path — the only path a live session
ever takes, since going from live to exited adds and removes no tab and so
never reaches the full rebuild.
An unknown answer draws nothing. A death tmux could not explain reads "exited"
with no number rather than "exited (0)", so an unexplained death and a clean
exit do not look alike. A signal death reads "exited (signal 9)".
The badge carries `data-i18n-skip`, like the status pills: it is generated
text, `i18n.js` walks inserted content, and a dictionary entry added later
would fight the renderer, whose in-place comparison is against English.
The tab also carries a `tab-agent-exited` class that mutes the status dot. That
dot is drawn from `status`, which stays `idle` or `busy` for an exited pane as
the issue requires, so without this a green or pulsing dot sits beside a badge
saying the agent is gone — the first thing a tester asked about. `status`
itself is untouched, so this is a rendering rule only. The CSS excludes the two
alert classes by hand, following the convention the rich-rail dot rules
document: a dot turning red or yellow because a session is blocked on a human
outranks "the agent exited".
The tab keeps its click behavior. X still closes it, and nothing here closes,
sweeps or restarts anything.
`docs/architecture-invariants.md` gains the mechanism under "Session data and
lifecycle", where every comparable one already lives: what the tri-state means,
the four shapes it is absent for, why the watcher cannot ride the stats
collector, why an absent `#{pane_dead_status}` is not 0, and the three things
that must never happen to an exited pane.
Refs Ark0N/Codeman#446.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two related fixes, found while re-measuring stock.ts's `accent` field
against the actual rendered UI (docs/cli-registry.md flags this field as
"transcribed, not authoritative — re-measure before wiring one up"):
1. A real, user-visible bug: `.btn-toolbar.btn-run.mode-gemini`,
`.mode-antigravity` and `.mode-omp` had no override rule inside the
`html:not([data-skin="og"])` block, unlike codex/pi/grok/deepseek, which
do. The generic `.btn-toolbar.btn-run` rule in that block resolves at
higher specificity than the base sheet's per-mode pair, so all three
rendered as plain claude-blue on every skin except `og` — including
`daylight-blue`, which is the actual DEFAULT skin for a fresh install
(index.html's pre-paint script), not an edge case. Added the three
missing rules, sourced from each CLI's own already-designed og-skin
colours (no new colours invented), mirroring the exact pattern
pi/grok/deepseek already use. Also corrected the stale comment on the
pi rule, which claimed this was still broken for gemini/antigravity.
2. `stock.ts`'s `accent` field was simply wrong for most CLIs — e.g. claude
was registered as Anthropic's brand orange (#d97757) while its button
renders blue, antigravity was registered purple while it renders cyan,
pi was registered green while it renders pink. Measured each CLI's real
`border-color` from its own `.mode-<id>` rule on the og skin (the
cleanest single representative hex each entry's gradient resolves
around) and corrected all 9 non-shell entries to match. `accent` has no
reader yet (confirmed via the DECLARED_FOR_LATER guard test), so this
changes no rendered output — it's a data-accuracy fix, matching the
registry's own "transcribed, not authoritative" warning taken literally.
Also fixed a false claim in types.ts's doc comment for the field
("CSS derives every per-CLI gradient from it via --cli-accent") — no
such CSS variable exists anywhere in the codebase.
Full gate: 406 files / 7721 tests / 0 failures, typecheck/lint/format:check/
check:public-assets/check:frontend-syntax all clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n
The maintainer's promised merge-time fixes from the final review of #453:
1. closeSplitPane() tears down a divider drag still in progress, so a split
that collapses mid-drag no longer leaves body.split-pane-resizing (the
page-wide col-resize cursor and user-select lock) set until a reload.
2. openSplitPane() re-applies the picker's own exclusions (detached session,
pid === null, no session record) for a row that went stale while the
menu sat open, refusing silently like its neighbouring gates.
3. architecture-invariants: the hard-hide of .btn-split is the
@media (max-width: 1179px) rule in styles.css, not mobile.css.
4. SplitTerminalPane.destroy() nulls onclose (and onerror) beside onopen
and onmessage.
5. Picker rows drop the data-session-id attribute nothing read.
6. The Pane-A-ends branch collapses with skipPrimaryResize, so the closing
resize is no longer aimed at the session the server just removed.
7. The {t:'r'} refresh path is single-flight across the fetch and the
chunked write, coalescing a mid-replay refresh into one trailing re-run.
Tests: split-pane-auto-collapse-unit gains the drag-teardown, exclusion and
skip-resize cases; the new split-pane-terminal-unit covers destroy() and the
refresh single-flight. All were run against the pre-fix module to confirm
they fail there.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit dbd39aed015ae5ae5870aba398bf4b4ab5118e47)
- styles.css: restate the composer overlay's own bottom gutter after the fold rules
(the generic .paste-overlay longhand erased it: 0px flat, hinge strip replacing it
folded) and subtract the fold strip from the dialog's max-height
- test/foldable-layout.test.ts: simulate the cascade for
.paste-overlay.prompt-composer-overlay (fails without the CSS fix); pin the palette
anchor by name instead of ELEMENTS.at(-1)
- keyboard-accessory.js: guard the app global in refreshForActiveSession() like the
rest of the file
- keyboard-accessory.js: a whitespace-only draft is empty (Send no longer submits
blank lines); the text still goes out untrimmed
- keyboard-accessory.js: derive _composerMaxLength and the frame refusal from one
64 KiB frame limit minus both bracketed-paste markers so they cannot drift
- keyboard-accessory.js: translate the textarea placeholder and label at build time,
since the DOM translator skips <textarea> subtrees
- i18n.js: zh-CN entries for the composer dialog copy
- docs/wiki/Mobile-Guide.md: describe the Compose key instead of a clipboard key
- CLAUDE.md: a "Mobile prompt composer" paragraph after the accessory bar one
- test/mobile-prompt-composer.test.ts: pin the whitespace rule and the derived budget
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit f6725ba52da17b0bdbee8be3b5011e7cae514f69)
- test/opencode-resize.test.ts: retarget the launcher guard at the real code (this.selectSession(firstSessionId), any this.activeSessionId assignment) with an anti-vacuity check; the old strings existed nowhere, so it could never fail
- session-ui.js: restore as comments the two invariants the merged bodies lost (deepseek leaves statusReporting unset, i.e. ON; no effort field for external CLIs, it is Claude-specific)
- docs/cli-registry.md: move the frontend-guard paragraph below the two backend-guard paragraphs so they keep their antecedent, and note the widened comparison shape
- test/frontend-cli-no-id-branching.test.ts: the comparison shape accepts any left-hand identifier (const m = this._runMode; m === 'codex' was invisible), normalized to `mode`; the two `m !== 'shell'` display filters are allowlisted and the remaining blind spots documented
- test/run-mode-dispatch.test.ts: table-driven pin of run() dispatch (claude to runClaude, each RUN_MODE_LAUNCH id to _runCliMode(id), shell to runShell, unknown to runClaude, lock held and released)
- CLAUDE.md: name the second CI-gated guard next to the backend one
- server.ts: every </head> injection passes a replacer function; a clis.json label containing $' re-injected the rest of the document past escapeScriptJson (two render tests pin it, proven failing on the string form)
- _isAltCliMode(): no reference anywhere in the tree, nothing to fix
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 1ea363ff808a62861559bc141e724b163cc1c56e)
The maintainer's promised follow-ups to opticon454's picker promotion,
applied on the landing branch after the merge (ecb95b5d):
- session-ui.js: the promotion tag ("Currently loaded" / "Last used") and
the "Default" pill are two separate spans, so a promoted row that is
also the endpoint's defaultModelId shows both instead of silently
losing its Default marking; two tests pin it (both fail on the old
exclusive-slot rendering).
- styles.css: a dedicated #customModelPickModal .set-scope rule, since
the pill was only styled inside the three settings modals and rendered
as plain body text here; same skin tokens, modal layout untouched.
- docs/wiki/Custom-Model-Endpoints.md: describe the promotion (currently
loaded, else last used per device), the separate Default pill, and
that nothing is ever auto-chosen.
- CLAUDE.md + docs/custom-model-endpoints.md: credit the real "Last used"
writers (_runCustomModelEntryViaRestart and
_quickStartWithCustomModelConfirm; runCustomModelEntry only dispatches
since 88e5b7b2) and drop the now-wrong "both defer to Default" sentence.
- Not done: moving the one-shot "last used" write into
_runCustomModelEntryOneShot, because the existing one-shot tests assert
that _quickStartWithCustomModelConfirm writes the key itself.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 0cb0f911adc14a852ba5c2951768a4aa87c25657)
The smart-copy gate only entered its selection-check block behind
hasSelection(), so a selection-less Ctrl+Shift+C skipped straight to
`return true` and ceded the keystroke to the browser's own handling
(e.g. Chrome's Inspect-Element binding) instead of matching Pane A's
"never falls through" contract for that chord.
Verified live in a real browser that this is a UX-parity fix, not an
interrupt-safety one: xterm's evaluateKeyboardEvent never emits PTY
data for a shifted ctrl-letter regardless of any gate (only "_" and
"@" get special-cased), so no accidental 0x03 was ever at risk. The
regression test added here asserts on the dispatched event's
defaultPrevented rather than the absence of a WS frame, since the
frame-count check passes vacuously for this exact key combo whether
or not the gate fires.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- buildSplitPickerSessions() now excludes any session with pid === null
(exited CLI, tripped PTY-exit breaker, a restore that never re-attached).
Pane B has no equivalent of selectSession()'s auto re-attach POST, so a
split opened onto one had nothing reading its tmux pane: no terminal
events ever arrived and Session.write() silently dropped every keystroke
with no ack either way, while the socket itself reported healthy.
- Fixed the hollow chord regression test: the synthetic keydowns carried no
keyCode, which is what xterm's evaluateKeyboardEvent switches on to
produce a data frame at all, so the assertion held regardless of whether
the gate fired. Adding real keyCodes surfaced a second, real bug in the
Alt+B case: the event bubbles to app.js's own document-level shortcut
dispatcher, which really toggles the sidebar and resets the layout
attribute the gate reads before Pane B's own (later, non-capture) handler
ever sees it — fixed by driving the app's real settings cache instead of
only the DOM attribute.
- Ported the two remaining primary-pane gates with real consequences:
Ctrl+Z (SIGTSTP) is swallowed for every non-shell session, matching
terminal-ui.js's reasoning (an Ink/TUI agent loop stops dead with no
visible output otherwise), and Shift/Ctrl+Enter now POSTs to
/api/sessions/:id/send-key for THIS pane's own session instead of
letting xterm send a bare \r, which used to submit an incomplete prompt
instead of inserting a newline. Smart-copy Ctrl+C is re-implemented
against Pane B's own terminal (copying app.copyTerminalSelection() would
have copied Pane A's selection instead).
- Updated docs/architecture-invariants.md and docs/split-pane-sessions-plan.md
to match, and added CLAUDE.md's missing .split-picker-menu z-index entry.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pane B had no attachCustomKeyEventHandler of its own, so the document
capture-phase shortcut handler's preventDefault() (which does not stop
xterm) left Ctrl+K/Alt+1/Alt+B ALSO writing their raw byte/escape
sequence into Pane B's live PTY on top of whatever the app action did
to Pane A. Pane B now installs the same registry-aware gates the
primary pane's own attachCustomKeyEventHandler uses. Ctrl+V is left on
xterm's default paste — no image-paste trap to route it to.
Plus the rest of the review's smaller items:
- Narrowing the window past the desktop gate now closes an open split
instead of leaving it stranded on screen.
- Split is refused while a web tab is active (activeWebviewId), which
used to open Pane B's socket behind a hidden container.
- Pane B now handles the server's `{t:'r'}` refresh frame via a shared
_loadBuffer() helper (also used by connect()), instead of ignoring it.
- The divider drag now uses pointer events + setPointerCapture (mirrors
tab-rail-resize.js), a button!==0 guard, preventDefault, and a
body.split-pane-resizing cursor/selection lock — a plain mousedown
drag selected the text under the cursor as it crossed both terminals.
- Pane B's close control and the picker rows are real <button>s now
(keyboard-reachable), with matching CSS chrome resets.
- Dropped the redundant CodemanBase.base prefix on the buffer fetch
(the global fetch wrapper already applies it).
- data-preview-order for the Split settings chip moved from a collision
with Ultracode Agents (both 15/12) to 11.5, matching its real
position between Multi-monitor and Ultracode Agents in the header;
widened test/app-settings-structure.test.ts's regex to allow the
decimal (Number() already parses it fine for the preview sort).
- Added zh-CN i18n entries for the Split button and empty-picker text.
- Dropped the stray unused `vi` import Ark0N flagged as unrelated to
this feature (vitest's `globals: true` makes it ambient anyway).
- Documented the fix and the deliberate no-cid/seq choice in the
split-pane-sessions architecture-invariants entry.
Added a real-Chromium regression test asserting Ctrl+K/Alt+1/Alt+B
dispatched at Pane B's own textarea send no `{t:'i'}` frame over its
WebSocket. Full CI gate green (409 files, 7736 tests) plus all 8
split-pane browser tests.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two majors:
- The divider drag was unthrottled: every mousemove did a full xterm
reflow on BOTH panes and sent Pane B a {t:'z'} resize frame with no
unchanged-dimensions skip, fanning out into a `tmux resize-window`
child plus a SIGWINCH per event — ~50 of each dragging across half a
wide viewport. SplitTerminalPane.fit() is now split into localFit()
(reflow only) and fit() (reflow + send); the drag coalesces moves
into one localFit() per animation frame via requestAnimationFrame,
and sends the real resize for both panes exactly once, at drag end,
matching the primary pane's own throttledResize convention.
- Pane B pulled the FULL scrollback unchunked for every session mode,
writing it in one terminal.write() call. Mirrors the primary pane's
own mode check (app.js's selectSession): shell sessions get a
bounded 1MiB ?tail= fetch instead of ?full=1, and the fetched buffer
is written through a minimal chunked writer (32KB slices, yielding a
frame between each) instead of one primary-pane chunkedTerminalWrite
this simpler, independently created/destroyed pane has no equivalent
of (no session-switch generation counters or live-output gate).
Smaller items from the same review:
- Pane B now follows live appearance changes (applyTerminalSkin,
applyTerminalFontFamily, applyTerminalFontWeights, setFontSize all
propagate to it, matching the teammateTerminals pattern) and reads
the real codeman-font-size/terminalFontFamily/weights/DEFAULT_SCROLLBACK
settings at construction instead of hardcoding fontSize 14 / scrollback 5000.
- The Pane-B-promotion path now skips selectSession() when
_closingSessions already owns this delete (the user closing Pane A's
own tab), matching _onSessionDeleted's own active-session-handoff guard.
- Detaching a session AFTER a split is already open now yields the PTY
size in _sendResize() too (not just at picker-open time), mirroring
sendResize's own detachedElsewhere guard.
- .btn-split joins the body.solo-mode hide list, next to .btn-multimonitor.
- The split row was 6px wider than its container (two flex-shrink:0
50% panes plus a 6px divider): both panes are now flex-shrink 1.
- Pane B's header and the split-picker rows are marked so i18n.js's
exact-string lookup skips them, matching .session-name elsewhere —
a session literally named e.g. "Sessions" was translatable on zh-CN.
- The Split button now reflects open/closed state via a `.split-open`
accent style, aria-pressed, and a title/aria-label that says which
behaviour the next click gets.
- _splitPane.connect() is no longer an unawaited call with no .catch().
- terminal-split.js's fileoverview pointed at a doc path that was
renamed away in the previous push; @dependency now credits
constants.js for CodemanTerminalFont, not terminal-ui.js.
- index.html's Split settings chip no longer reuses data-preview-order
"12" (already the Ultracode Agents chip's slot in the same "header"
preview group).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Blocker from Ark0N's second PR #453 pass: moving showSplitButton into
settings-ui.js's per-device displayKeys set was only half of making it
per-device. saveAppSettings() still put it in the object PUT to
/api/settings, SettingsUpdateSchema (.strict()) does not declare it,
the server answered 400 INVALID_INPUT, and because the call site never
checked res.ok the UI still reported "Settings saved" while NOTHING
persisted — workspaceHooksEnabled, agentSkillEnabled, tunnelEnabled,
claudeModel, every toggle, on every save, on every device. Strip it
out via the same destructure every other per-device key goes through
(`showSplitButton: _ssp,`), drop the stray mention from a schemas.ts
comment (a mention there reads as "this is a real field" to the next
grep), and add a static guard test mirroring
test/terminal-auto-copy.test.ts's three-way rule.
Also finishes the desktop gate the first pass only did in CSS at
599px: SPLIT_PANE_MIN_WIDTH (1180, matching HOME_SESSIONS_MIN_WIDTH)
now backs an actual JS width check in _applySplitButtonVisibility,
with a matchMedia listener so a live window resize hides/shows the
button without a reload — the CSS backstop in styles.css is the
reverse-direction guarantee for when JS hasn't run.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ark0N's PR #453 review: nothing gated this feature to desktop even
though the design called for it (two 240px min-width panes plus the
divider need ~486px, and the divider has no touch handlers), and
showSplitButton was a SYNCED setting, so turning it on at a desk also
put the button in the phone header.
- Hard-hide .btn-split on phones in mobile.css regardless of the
setting, matching the other desktop-oriented header buttons in the
same @media (max-width: 599px) block.
- Move showSplitButton into settings-ui.js's per-device displayKeys
set and drop it from SettingsUpdateSchema entirely, matching the
showFileViewerButton/skin precedent (CLAUDE.md's "per-device keys
... must NOT be added to SettingsUpdateSchema" rule) — a desktop
opt-in must never sync onto a phone that never asked for it. Removes
the now-invalid server-round-trip test for the setting.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Exclude popped-out (detached) sessions from the split picker:
SplitTerminalPane._sendResize() has no yield-to-detached-window check
the way the primary pane's sendResize() does, so splitting against a
detached session put its own window and Pane B in a fight over the
same PTY's dimensions. Simplest fix per the review: keep them out of
buildSplitPickerSessions() entirely.
- Show a visible dead state when Pane B's WebSocket drops. onData
already silently discards keystrokes while the socket isn't OPEN
(there is no reconnect for v1), so a dropped socket left the pane
looking normal while it quietly ate everything typed into it.
- openSplitPane() returns early with no active session, so a split
triggered from the home screen no longer creates and connects Pane B
behind the opaque welcome overlay with nothing to show for it.
- onMove() during a divider drag now bails when the split has
auto-collapsed mid-drag (the other pane's session ending) instead of
throwing on `divider.parentElement` being null.
- Promote Pane B via `selectSession(id, { auto: true })` when Pane A's
session ends — this is an app-driven selection, not the user clicking
a tab, so it must not spend the promoted session's idle alert.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ark0N's PR #453 review: fit() was only ever called from the divider
drag, and the trailing-edge ResizeObserver callback in terminal-ui.js
(throttledResize) only ever measured Pane A's own container. Split at
a wide viewport, shrink the window (or toggle the Alt+B sidebar, or
drag the tab rail), and Pane A's cols changed while Pane B silently
kept its stale PTY size in both xterm and the real pane.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pane B's SplitTerminalPane.connect() only opened a WebSocket and waited for
live output — ws-routes.ts's terminal socket sends nothing on connect, only
future 'terminal' events — so it stayed blank until the target session
happened to produce new output. It looked intermittent rather than
always-broken because a resize sent by _sendResize() often nudges the
session's real tmux window to a new size, and tmux repaints its current
screen on resize; that incidental repaint was what usually populated the
pane. When Pane B's computed dimensions already matched the session's
last-known size, Session.resize() skipped the resize as a no-op and the
pane stayed empty. Fetch the existing scrollback (?full=1) before opening
the socket, same as the primary pane does.
Pane A never told its own session's PTY/tmux about a size change at all,
relying purely on the passive 300ms-debounced ResizeObserver in
terminal-ui.js. openSplitPane() now force-resizes Pane A immediately on
entering split (mirroring closeSplitPane()'s existing symmetric call), and
the divider-drag handler force-resizes it once at drag end (matching the
codebase's established trailing-edge debounce convention rather than
flooding a resize per mousemove).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Nothing stopped a stale picker click (opened before switching tabs) or
clicking Pane B's own session tab while split from landing on
openSplitPane(sessionId) with sessionId === activeSessionId, or from
selectSession() rebinding the primary pane onto the session Pane B was
already showing — either way, two live WebSockets to one session, each
independently claiming PTY dimensions via its own {t:'z',...} resize frame.
openSplitPane() now refuses early when the target is already the active
session, and a new selectSession() prototype patch (same top-level pattern
as the existing _onSessionDeleted patch) closes an active split BEFORE the
primary pane rebinds to the session Pane B holds.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
.main.webview-active hid .terminal-wrap when a web tab became active, but
.terminal-wrap is reparented INSIDE .terminal-split-container while a split
is open, so Pane B and the divider stayed stranded on screen over the
dashboard iframe. Hide the whole split container as one unit, mirroring the
existing .terminal-wrap rule; no state is destroyed, so returning to the
session tab shows the split intact.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
.split-picker-menu/-item/-empty (created in openSplitPicker()) had zero CSS
and could not be dismissed except by picking an item — a default-path defect
since the Split button ships enabled to anyone who flips showSplitButton on.
Add CSS matching the sibling .run-mode-menu popover's look (floating-bg
backdrop blur, border, shadow, z-index 1000 above the header's 100), and
dismiss on outside click or Escape via the same one-shot listener pattern
session-ui.js already uses for its other transient popovers
(toggleCaseSettings(), toggleRunModeMenu()). Picking an item now routes
through the same _dismissSplitPicker() method as the outside-click/Escape
handlers, so the listeners never outlive the menu.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_sendResize() clamped Pane B's proposed cols/rows to a 40/10 floor before
sending the {t:'z',...} resize frame, so the PTY was misinformed of Pane B's
real width at the divider's own reachable 20% position, causing real
output-wrapping bugs. The primary pane (terminal-ui.js's
getTerminalDimensions()) sends fitAddon.proposeDimensions() unclamped and
lets the server enforce its own valid range ([1,500]/[1,200] in
ws-routes.ts); Pane B now matches that convention.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
.btn-split--hidden had no matching CSS rule anywhere, so the opt-in Split
header button shipped visible to every user on every viewport regardless of
the setting. Add the `display: none !important` rule alongside its sibling
marker classes (.btn-multimonitor--hidden etc.), plus a static regression
guard (test/split-pane-hidden-button-css.test.ts) that fails if any future
"*--hidden" marker class in index.html is missing a matching CSS rule.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codeman maintainer