mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
55790964b7099c892374d61ff35079f4fa4effbf
49
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michael GrundbergandClaude Opus 5
|
e558264977 |
chore: leave the changeset to the maintainer
CONTRIBUTING says releases are handled by the maintainer via changesets after merge, and every `.changeset/*.md` on master was written by him or by the release bot — including the ones covering other people's pull requests. The summary this file carried moves to the pull-request description, where it is the maintainer's to reuse or rewrite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
9a48c43aa1 |
docs(watching): a restart is not a gap, and here is the measurement
Claimed after a manual test that a session comes back from a server restart without its badge until it next produces output. Measured instead of assumed, and it is wrong: a codex session with a background terminal still running had its label back within about 20 seconds of the restart, with no input from anyone. Reconciliation re-attaches the pane, the attach repaint carries the composer glyph, the idle confirmation arms on it, and the probe re-reads the label — the ordinary path, doing the ordinary thing. What produced the false claim was a session whose monitor had simply expired while it sat there. Its footer carries no chip, so `watching: null` was the right answer and there was nothing missing to restore. Recorded at the field and in the invariants, because the shape of this invites exactly one wrong fix: a polling timer to keep a value fresh that the pane already refreshes by itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
ac6236b268 |
fix(terminal): clean a copy once, and reach every pane that copies
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> |
||
|
Michael GrundbergandClaude Opus 5
|
b13672596f |
fix(watching): tell the page when the badge goes away
Reported from a manual test: a Codex session went on showing the watching badge after its background terminal had finished. The server was right and the page was stale — `Session.watching` changes while the session's status does not, and nothing broadcast it. The label is usually SET on the idle transition, which broadcasts anyway, so the badge always appeared correctly. It CLEARS when the work ends, and a CLI can end background work without taking a turn: codex repaints its background-terminal row away and stays idle, so `_confirmIdle()` concludes without emitting `idle` (that emit is guarded by `wasWorking || isInitialReady`) and no other event fires. Every open page kept drawing a badge the server had already dropped. `_readWatching()` now emits `watchingChanged` when, and only when, the label really changes, and the wiring pushes the session state on it. No new SSE event: the badge reads off the session payload every surface already has. A/B measured on an isolated beta with the page loaded and then left untouched. Without this commit the server dropped the label at t+50s and the page still showed the badge at t+100s; with it, page and server cleared in the same ten-second window. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
8d45b92eba |
docs(codex): record that a sub-agent leaves no row to read
Measured on the same beta, codex-cli 0.154.0: a sub-agent started without waiting outlives the turn exactly as a background terminal does — the sandboxed process was still running — and codex shows nothing for it. The last rows of the pane are the composer and the status line, and `Sub-agents running` belongs to the on-demand `/subagents` panel rather than to the row above the composer. So there is no second codex label to add. A codex session waiting on a sub-agent reads as plainly idle, which misfiles nothing (codex raises no idle prompts) and simply leaves that one kind of quiet unexplained until codex pins a row for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
05c788ce9d |
fix(watching): close the review findings on the label and its window
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> |
||
|
Michael GrundbergandClaude Opus 5
|
9c286eeddf |
fix(session): persist an exit retraction, and let tests reach the watcher
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>
|
||
|
Michael GrundbergandClaude Opus 5
|
ce80b7a212 |
feat(terminal): take the transcript gutter off a copy, at the width the CLI declares
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> |
||
|
Michael GrundbergandClaude Opus 5
|
90fd0a5a15 |
fix(tmux): gate the pane-exit read, and mute the dot on the rich rail too
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> |
||
|
Michael GrundbergandClaude Opus 5
|
64c288a683 |
feat(codex): read Codex's own background-terminal row
Codex states background work too, and it says so in a different place. Claude writes `· 1 monitor ·` on the last row of the screen; Codex pins `1 background terminal running · /ps to view · /stop to close` ABOVE its composer, which puts that row third from the bottom once the status line and the composer are counted. So how far up the screen to look is now per-CLI data as well: `capabilities.workDetect.watchingLines`, bounded to 1..8 by the schema, and defaulting to Claude's two. That bound is the point. The window is half the injection guard, since every row it adds is another row the agent itself may be able to write, and the label is what silences an idle alert. The other half is the anchor, and Codex's is ` · /ps to view`: chrome naming a slash command only the CLI can offer, so a session that writes "I left 1 background terminal running for you" into its own output matches nothing. Measured against a live codex-cli 0.154.0 pane rather than read out of a binary. The row appears when the terminal starts, follows the composer down as the conversation grows, and is gone after `/stop`. Verified end to end on an isolated beta: the session payload carried `watching: "1 background terminal"` and the badge rendered with it, and both cleared when the terminal stopped. The fixtures in the tests are that capture verbatim. Codex has no hook signals, so no idle prompt and no false NEEDS YOU row: for a Codex session this is the badge alone, which is the case the maintainer said a registry field could cover and a hook never could. Cross-CLI tests pin that neither pattern fires on the other's screen, and that a CLI declaring nothing still reports nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
74884a20eb |
feat(approvals): let a watching session keep quiet, and fix the tui gate
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> |
||
|
Michael GrundbergandClaude Opus 5
|
1cb0441bd8 |
fix(session): degrade the resume pin to the session id, not to nothing
A single pin that failed its transcript gate returned the options untouched, so `resumeSessionId` fell back to `_resumeSessionId` — undefined for an ordinary session — and the renderer emitted the bare `claude --dangerously-skip-permissions --session-id "<this.id>"`. Every session prompted before its first `/clear` owns a transcript under that id, so the dropped pin handed back exactly the refusal this branch removes, with no `||` branch to catch it. It was also a regression against master on the `restartCli()` path, which pinned `_claudeSessionId ?? this.id` and, since the constructor seeds that field, could never land unpinned. The pin now walks three candidates in priority order — the conversation chain's tail, the launch seed, then the session's own id — and takes the first one a transcript backs. A candidate that misses is passed over rather than ending the walk. Falling off the end pins nothing, which also settles the second half of the problem: the old code skipped the transcript check whenever the pin was the session's own id, so a genuinely new pane rendered the two-branch form after all. That costs a brand-new session claude's "No conversation found" line in its scrollback, and `wrapWithNice()` prefixes only the first branch of the rendered `a || b`, so the branch that actually runs loses its priority for the life of the session. With no transcript anywhere the bare `--session-id` is the correct command, so the comment claiming an unchanged shape is now true. The transcript lookup reads the server process's own `CLAUDE_CONFIG_DIR` when a session declares none. A pane inherits the server environment through tmux, so on an install that exports it the CLI writes its transcripts there and every lookup under `~/.claude` was a false negative — which under the old code meant the colliding command. `claudeCredentialsPath()` and `realClaudeConfigDir()` resolve the same directory the same way. The header sentence calling a skipped resume "the safe direction" described the opposite of what happens at this call site, and says so now. The create-path fallback writes `_resumeSessionId` alongside the create options. That branch leaves `isRestored` false, so `_claudeSessionId` is recomputed from the launch fields and settled on `this.id` while the CLI resumed the chain tail; the response viewer, Read My Mind and the unified-list alias map read that field until the next first-hand hook. Four new tests: a chain tail with no transcript while the session id has one, no transcript anywhere, the create path's alias, and the process-env lookup. All four fail against the previous commit. Two existing tests move with the gate — the guess-refusal test now backs the session's own id, and the custom-model restart test gives its working pane the transcript that makes `--session-id` collide in the first place, alongside a new one pinning the no-transcript case. CLAUDE.md described the pin as a `restartCli()`-only thing sourced from the live conversation id. All three halves of that moved here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
3f2cde2db7 |
feat(session): say when a session is watching its own background work
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> |
||
|
Michael GrundbergandClaude Opus 5
|
47e7935274 |
fix(session): resume the conversation when respawning a dead pane
A CLI that launches with `--session-id <id>` refuses an id that is already in use (claude: `Error: Session ID ... is already in use.`), and every session whose agent has been prompted owns a transcript under that id. The dead-pane respawn in `_setupOrAttachMuxSession()` passed the bare launch line, so recovering such a session relaunched a CLI that died on startup, the pane went dead again at once, and the conversation was stranded behind a tab that looked merely idle. `restartCli()` has pinned a resume id against this since the custom-model work, and its comment states the assumption that made the other path look safe: "Unlike the dead-pane respawn, this one kills a WORKING pane whose conversation already has a transcript". A pane whose agent exited has a transcript too. Both relaunch paths now build options through `_buildRespawnPaneOptionsWithResumePin()`, and so does the create-path fallback after a failed respawn, which otherwise met the same refusal that made it the fallback. Four gates guard the pin, each standing for a way of resuming the WRONG conversation or of making a working relaunch fail. A remote or docker session is never pinned. Unlike `restartCli()`, whose route refuses both, the dead-pane respawn is reached by every session shape. Their pane commands already render a self-healing `--session-id || --resume`, and both flip to resume-first once the resume id differs; the conversation lives on the far side, so a local id resolves to nothing there and the `--session-id` fallback then collides with the transcript the far side does hold. The id comes from the conversation CHAIN rather than `_claudeSessionId`, which also holds history-correlated guesses keyed on the working directory. `_recordClaudeSessionInChain()` refuses those so they cannot "write a foreign conversation into this pane's permanent record", and launching from one is worse than the display bug that rule prevents. The chain tail also outranks the launch seed, which is written once at construction and never moves off a `/clear`. A pin no transcript backs is dropped, because the fallback branch keeps `--session-id <this.id>` and would collide. A synthetic `restored-<fragment>` id from socket discovery is dropped too, and logged: it fails claude's `uuid` token pattern, so the renderer would emit the unpinned command while the caller believed otherwise. Tests cover each gate and the rendered command. Four of them fail against the unfixed source; the remote and docker ones were separately checked against a build with only that guard removed, since they pass on master for the wrong reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
c67c130caa |
feat(web): mark a session tab whose agent has exited
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>
|
||
|
Michael GrundbergandClaude Opus 5
|
90a95f562b |
feat(session): publish and persist a local pane's agent exit
The mux layer now knows a pane's agent has exited. This puts it on the session
record, where the board and, later, the reboot restore can see it.
`SessionState.paneExit` carries `{ status?, signal?, at }` and rides the
existing `session:updated` broadcast through `toState()`. No new SSE event. The
server pulls each answer from `mux.getPaneExit()` rather than off a broadcast
payload, so the raw reading never reaches a browser: for a remote or docker
session that reading is the death of an ssh client or a `docker exec`, not of
the agent.
The field is tri-state, and the third state is its absence: `undefined` means
Codeman does not know, and it never reads as alive. `Session.setPaneExit()`
forces that unknown for every shape a dead local pane does not describe. A
direct-PTY session owns no pane. A remote SSH session's local pane holds the
ssh client, whose death means a transport drop OR an exit, which is the
ambiguity PR #355 settled by not guessing. A docker case's local pane holds a
`docker exec` into the container's own tmux. And a session rebuilt from the
socket has no provenance at all: `reconcileSessions()` gives it a synthetic
`restored-<fragment>` id that matches no `state.json` entry, so a remote
session rediscovered after `mux-sessions.json` was lost arrives with no
`remote` field and looks local — `MuxSession.discovered` marks it, and absent
metadata there counts as unproven rather than as proof. The scoping lives on
`Session` rather than in `TmuxManager` so there is one copy of the rule.
`status` and `pid` are untouched. `status: 'error'` belongs to the PTY-exit
circuit breaker and makes the browser offer a restart, and a null `pid` is what
makes the browser re-attach and launch a fresh CLI. A reading that repeats the
previous answer writes nothing and broadcasts nothing.
An unknown answer never reads as alive, but a stale KNOWN one would keep
reading as exited, so `clearPaneExitForNewPane()` retracts it on every path
that puts a new command in the pane: the start/attach path, the `restartCli()`
relaunch behind a custom-model switch, and the remote reattach. Without the
second of those, switching an endpoint on an exited session launched a new
command and then persisted and broadcast the old exit straight back onto it.
`toState()` is also what `state.json` persists, so the record survives a
reboot, which is the only thing that does: a reboot takes the tmux server, and
with it every live signal and every `mux-sessions.json` entry. Nothing reads it
there yet — making the restore refuse such a session is a behavior change that
belongs with the part that closes them. Recovery threads the saved value back
through the constructor so the first persist after boot cannot blank it.
Refs Ark0N/Codeman#446.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Michael GrundbergandClaude Opus 5
|
02dc46dcd7 |
feat(tmux): report a dead pane's exit from the batched pane list
Codeman creates every tmux pane with `remain-on-exit on`. When the agent exits,
tmux keeps the pane, the tmux session, and the `tmux attach-session` process
Codeman records as the session's pid, so no PTY exit handler fires and nothing
writes the exit down. tmux itself knows: it marks the pane dead and reports the
exit status. This reads that.
`PANE_LIST_FORMAT` gains `#{pane_dead}`, `#{pane_dead_status}` and
`#{pane_dead_signal}`, and `startPaneExitWatcher()` refreshes a
muxName-to-observation map from ONE batched `tmux list-panes -a` per tick. Boot
reconciliation already ran that same call, so it now fills the map too and
recovery starts with a reading.
The watcher owns its own interval rather than riding `startStatsCollection()`,
which the issue suggested. That collector is armed when a browser opens the
Monitor panel and DISARMED when it closes it, and boot skips it entirely unless
recovery found a live session, so a session created on a freshly booted server
would publish nothing and one browser could turn detection off for every other.
Measured on an isolated instance: a dead pane with status 0 reported nothing
until `POST /api/mux-sessions/stats/start` was called by hand. It is still one
batched read per tick; only the timer changed.
Three rules keep a positive answer trustworthy. A session answers only when
tmux listed exactly one pane for it, because Codeman never splits a pane and a
session the user split by hand has none that speaks for the agent. A pane
answers only when `#{pane_dead}` said 1 or 0, because an empty field is a tmux
that did not answer. An absent status stays absent rather than becoming 0:
measured on tmux 3.2a, a SIGKILLed pane reports neither a status nor a signal,
and calling that a clean exit would be wrong in the direction that matters.
Two guards stop a slow read undoing a fast one. `EXEC_TIMEOUT_MS` is 5000 ms
against a 2000 ms interval, so a read can outlive two ticks: one already in
flight suppresses the next, and a generation counter that every
`clearPaneExit()` bumps discards a read that started before a respawn or a
kill. An observation also carries its pane pid, so a second command in the same
pane that exits the same way starts a new timestamp rather than inheriting the
first death's.
A non-empty read of `list-panes -a` is authoritative for the whole socket, so
sessions missing from it are pruned, which also bounds the map as tmux sessions
come and go outside `killSession()`. A failed or empty read retracts nothing.
The manager reports the raw pane reading and applies no session-shape scoping,
because the remote-reconnect watcher beside it needs exactly that raw reading.
`parsePaneList` becomes `parsePaneRows`, returning one row per pane instead of
a name-to-pid map; reconciliation builds its map from the rows. The parser's
existing cases carry over unchanged, including the launchd/systemd literal-tab
regression from PR #71.
Refs Ark0N/Codeman#446.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Michael GrundbergandClaude Opus 5
|
95dc6fe944 |
fix(terminal): remember a geometry replay that did not converge
`resizeRetry` caps the recursion inside one select and says nothing about the next one, so a pane this browser cannot size reported the same mismatch on every select and bought the same failed repair each time: two fetches per tab switch for the life of the page, measured as a running count of 2, 4, 6 across three selects. That is the case this branch describes as happening every time rather than occasionally, a phone whose resize `Session.resize` declines while a desktop claim is live, and it is not the only one — any pane Codeman cannot size lands there, including one a second tmux client is also holding. Each wasted pass costs another `capture-pane`, which is `execSync` and blocks the server's event loop, plus a reset and chunked rewrite, a discarded snapshot and cache entry, and a dropped and reopened WebSocket. `_geometryRetryUseless` mirrors the existing `_fullHistoryRepullUseless`: a retry pass whose frame still does not fit adds the session, geometry that fits removes it, and the replay gate consults it. The proof has to come from a retry pass rather than a first one, because the retry ran at the size that stuck and the pane ignored it. Clearing on a fitting frame is what stops a pane that becomes sizeable again, once the desktop tab closes or its claim goes idle, from staying permanently unrepaired. The race case never reaches the latch, since it converges on its first attempt. The new browser case walks all of that: three selects reading 2, 3, 4 instead of 2, 4, 6, then a fitting frame, then a mismatch diagnosed afresh. Without the gate it fails on the second switch with `expected 4 to be 3`. Rebased onto master, which has moved to 1.30.0 and taken #436. The one conflict was `config/test-suites.ts`, where both branches appended a glob to `BROWSER_TEST_GLOBS`; both are kept. Everything else merged clean, #436's own changes to the same buffer-load path included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
383f834704 |
fix(terminal): flush unsent local echo before the geometry replay
On a touch device the characters the user has typed live only in the local-echo overlay until Enter; they have never reached the PTY. The replay re-enters `selectSession` with `forceReload` on the session that is still active, and that branch nulled `activeSessionId` before `_cleanupPreviousSession` ran. The flush there is guarded on a session it can still see, so it was skipped, and the unconditional `_localEchoOverlay.clear()` that follows took the characters with it. Measured in chromium against the previous head: typing into the overlay and then making the call the replay makes left `pendingText` empty with nothing crossing into the delivery layer on either transport. The flush moves into `_flushLocalEchoTo(sessionId)`, called from both `_cleanupPreviousSession` and the `forceReload` branch before it nulls the id. The session is a parameter because the two callers mean different ones: cleanup flushes to the tab being left, the branch to the tab being reloaded. This was reachable before this branch, through the one gesture that already takes the `forceReload` path on an active session. What is new is that nothing the user does triggers it. The replay fires on its own the moment a tab switch finishes, which is exactly when someone typing into a still-loading terminal has text in the overlay, and on a phone beside an active desktop tab that is every tab switch. A seventh browser case pins it: it forces the overlay on, since headless chromium reports no touch support and the case would otherwise pass vacuously, asserts the typed characters really are sitting unsent, then triggers the replay and asserts they reached the session. Without the fix it fails with nothing delivered at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
e0d4477edc |
fix(terminal): keep the geometry replay to the pass that can converge
Three follow-ups to the source gate, each one measured rather than reasoned. A pane already drawing at the size the client just requested is left alone. The replay runs at `dimsAfterLoad`, so it can only change what is on screen if the pane was drawing at some other size; when the reported geometry already IS that size, the second pass captures the identical frame and pays a full reload to do it, including a visible re-flash, a dropped and reopened WebSocket and a deleted xterm snapshot. That equality is the signature of a clamp rather than a race: `getTerminalDimensions()` floors at 40x10 while `fitAddon.fit()` does not, so a terminal narrower than 40 columns or shorter than 10 rows reports a pane permanently bigger than itself and replayed on every tab switch without ever converging. A race never produces the equality, since its premise is that the pane was still at the size it was asked to leave. The declined-resize case does not produce it either, so that one still costs the single capped attempt and needs the pane-ownership question this does not touch. The full-history re-arm is unreachable and now says so. A pass that consumed the flag sent `full=1`, and the route answers `full=1` with `mux-full-history` or `history`, never `mux-visible`, so the source gate already rules out every such pass. The line stays for the invariant, but its comment no longer reads as if a page load retries, and the suite pins that it does not. The response no longer reports geometry for a body that carries no capture. The full-history path writes `capturedGeometry` from the cursor query and then returns '' for a pane holding nothing visible, which drops the source to `history` with the geometry already recorded: a `full=1` request whose capture reported 100x50 and returned nothing answered `source: "history"` with both fields set. Nothing acted on it, because the client ignores geometry on any other source, but the field said a frame had been drawn at a size when none had. The browser stub now derives `source` from the request the way the route does, rather than answering `full=1` with `mux-visible`, which the route cannot produce. Each case reaches a visible-frame response the way production does, by not being the first select of the page. Three cases pin the new behaviour and each fails without its guard: the clamp case sees two fetches instead of one, the scope case and the full-history case both see a replay the gate forbids, and the width case sees one fetch instead of two. The changeset now describes the change from 1.29.x rather than the difference between the two commits on this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
5cfb98fb8b |
fix(terminal): compare capture geometry only on a visible-frame response
Only a visible-frame capture positions its rows absolutely, so only that frame can be damaged by a terminal of the wrong size. A `full=1` body is linear scrollback closed by a relative cursor move, which is relative precisely so the browser's row count need not match the pane's, and a `history` body is the byte stream, which carries no row alignment to protect. The geometry comparison ran on all three, so it fired most often on the one response it cannot help: `_fullHistoryLoaded` is empty on the first select of every non-shell session per page, and a session whose pane a desktop tab holds too tall to ever fit then paid a second whole-scrollback capture, reset and replay on every page load and every first tab switch. `framePositionsRowsAbsolutely` gates both the captured-geometry comparison and `sizeMovedUnderLoad`. A size that moved under a byte-stream or scrollback replay is healed by xterm's own reflow plus the SIGWINCH the trailing `sendResize` already sends. A pane WIDER than the terminal damages the same frame a second way, so `captureCols` is now compared rather than only logged. `formatPaneSnapshot` paints each row out to the pane's own width, so a narrower browser wraps every painted row, and the wrap on the last one scrolls the whole frame up by a row. The terminal response no longer falls back to `session.ptyCols`/`ptyRows` when the capture reported no geometry. The cursor query is what produces the absolute addressing in the first place, so a capture that lost it returned a raw frame that was never positioned, and a byte-history response was never positioned either. Naming the session's own PTY size there described a frame that does not exist and invited a repair for damage that is not present. `_ptyCols` is also written only by `resize()` while the PTY is spawned at the size queried from tmux, so it can be wrong on its own terms. Both fields are now absent instead, and the `Session` getters added for that fallback go with it. Two browser cases cover the new behaviour and each fails without its fix: a `mux-full-history` response with both dimensions mismatched asserts one fetch (two without the gate), and a `mux-visible` response wider than the terminal but short enough to fit asserts two (one without the width comparison). Corrects a claim in the comment above `capturedGeometry` in tmux-manager.ts. Both replay paths do not address rows absolutely; the full-history one ends in a relative move, which is the whole reason the gate is right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
3edf9aae2f |
fix(terminal): replay a pane capture at the geometry it was taken at
A visible-frame capture repaints each row at an absolute position, counting up to the pane's height. A terminal shorter than that clamps every address past its own height onto its last line. The overflow rows then overwrite one another, and the rows underneath are lost. Replaying a real 50-row capture into a 30-row terminal rendered 28 lines of a 45-line command and drew the frame twice. Nothing in the response said what height the frame was built for, so the client could not detect this. A capture now reports the geometry it was really taken at through `capturedGeometry` on `PaneCaptureOptions`, and the terminal response carries it as `captureCols` and `captureRows`. When the captured pane is taller than the terminal, or the size that produced the capture did not survive the load, `selectSession` replays once at the size that stuck. `resizeRetry` caps that at one attempt, so two competing fits cannot trade replays forever. The retry re-arms the full-history flag only when the pass that ran had consumed it. A tab switch takes the bounded tail, so its retry takes the tail too: clearing the flag unconditionally would upgrade that switch into a fresh scrollback capture the user never asked for, which the route's own comments put at tens of megabytes. What this repairs is a capture that won a race against the resize meant to precede it. It does not repair a capture whose pane was too tall because `Session.resize` declined the resize outright, which it does for a small viewport while a desktop viewport's size claim is live. The retry re-sends the same declined resize and captures the same pane, and `resizeRetry` then stops it. Repairing that means changing who owns the pane size, which is a policy question this does not touch. The reported geometry still helps there, because the client can see the mismatch at all rather than being blind to it. Follows #395, #396 and #397, which fixed the other ways the replayed frame and the terminal could disagree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
f9edb33d15 |
fix(terminal): trim the padding and shared indent out of a copied selection
xterm hands back whole screen rows and trims only the cells that were never written to, so the real spaces a full-screen TUI paints across the unused part of a row count as content and reach the clipboard. Measured against Claude Code in a 282-column pane, single lines arrived carrying 138 trailing spaces, and every line carried the two-space transcript indent as well. Windows Terminal, iTerm2 and GNOME Terminal all trim that for you, decideAutoCopy already calls a wall of spaces "never what the gesture meant", and _selectTouchSelectionLine already treats those cells as padding — the mouse and keyboard paths never had the same rule. CodemanCopySelection.clean lives in constants.js beside decideAutoCopy, its pure sibling. It drops the trailing run from each line, and removes the leading run only where every selected row shares one. A selection of a single row keeps its run, because one row shares nothing with anything and stripping it would silently reindent one line of `git log` body text or one line out of `less`. A drag that began inside a row keeps its partial first line untouched and out of the measurement, which otherwise pins the shared run to zero and leaves every following row indented. Every pass over a line is a scan rather than a regex. `/[ \t]+(\r?)$/` is quadratic on a line whose spaces are followed by a non-space character, which is what right-aligned or centred TUI content looks like: measured over 50 000 rows with a 280-column run it took 2.9s, against 1.3ms for the scan, and a 2 000-column run took 16s. The scan is also the faster of the two on an ordinary padded row. cleanedTerminalSelection in terminal-ui.js is the half that needs the live terminal. It returns a COLUMN selection untouched: Alt+drag makes one, and a rectangle's rows lining up is the point of the gesture, so both halves of the clean would destroy it. xterm exposes the mode nowhere public, so the check reads terminal._core._selectionService, the way this file already reads terminal._core for cell dimensions, and cleans normally if a future xterm renames the field. A test pins that assumption against the library rather than against a stub repeating the literal. The Ctrl+C chord decides on the cleaned selection, not the raw one. A drag across the blank part of a row selects real padding spaces, so the raw text is truthy, and testing it would spend that press on a copy of nothing and make the user press again to interrupt. A padding-only selection is now dropped and the press falls through to the PTY, while Ctrl+Shift+C still never falls through. copyTerminalSelection gates on trim() for the same reason, since a multi-row drag across padding cleans to line breaks alone and a bare newline pasted into a chat composer submits it. All four of the main terminal's copy paths go through it: the Ctrl+C chord, right-click, the phone selection button and Auto Copy. The browser's own Edit menu copy, a disabled copy shortcut and the subagent windows still copy raw rows, as they did before, and the invariants doc now says so rather than claiming every copy is cleaned. Auto Copy resolves its own toggle before it reads the selection, since it is off by default and a selection can run to the 50 000-row scrollback ceiling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
3730bc7df5 |
docs(terminal): correct what selectSession does with the viewport
The JSDoc on `_syncStickyScrollBaseline` said `selectSession` deliberately ends
at the bottom, so the baseline the replay samples is already true there. It
does not. `selectSession` calls `scrollToBottom()` after the write and then
ends at `scrollToLastNonEmptyLine()` (app.js:6512), which targets
`lastNonEmptyLine - rows + 2` and therefore parks ABOVE `baseY` whenever the
replayed frame keeps trailing blank rows — which a full capture does on
purpose, since no transform that can delete a line may run over one.
Its baseline really is a stale true. What covers it is the sticky snap itself:
since
|
||
|
Michael GrundbergandClaude Opus 5
|
cfd771d1d8 |
test(terminal): pin all four buffer-load paths to the shared flush helper
The first version of this fix decided the flush policy in `selectSession`
alone, and a later pass found it still covering one path of four. Nothing in
the CI gate stops a fifth path, or an inlined `{ flushQueued: true }`, from
splitting that policy up again — the browser suite that would notice is
excluded from `npm test`.
A static scan over `selectSession`, `_onSessionNeedsRefresh`,
`_onSessionClearTerminal` and `_maybeRefetchFullHistory` asserts each one asks
`_bufferLoadFinishOpts`, reusing the `methodBody` slice the sticky-scroll guard
already needed. Verified by inlining the policy back into
`_onSessionClearTerminal`, which fails it by name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Michael GrundbergandClaude Opus 5
|
75a028e825 |
fix(terminal): re-take the sticky-scroll baseline after a replay
A capture load now replays its queued tail, and that replay runs through `batchTerminalWrite`, which samples `_wasAtBottomBeforeWrite` before it queues. It runs inside `chunkedTerminalWrite`, before that promise resolves, with the terminal freshly reset and rewritten — so the sample is always true. The caller then restored the reader's position and the next `flushPendingWrites` scrolled straight back to the bottom off the latched flag, undoing it. The only thing in the way was `_hasRecentUserScrollUp()`, a 1500ms window a server-triggered refresh is usually past. `_syncStickyScrollBaseline()` re-takes the flag from wherever the viewport now sits, and the two paths that restore a position call it right after doing so: `_onSessionNeedsRefresh` and `_maybeRefetchFullHistory`. Those are the paths #259 and #205 exist for, and they are also where a non-empty queue is most likely, since a needsRefresh fires when output is flooding. Re-taking rather than suppressing the sampling: suppressing leaves whatever stale value the flag held from before the load, which on the full-history re-pull has no reason to be false. `selectSession` and `_onSessionClearTerminal` deliberately end at the bottom, so the sampled true is already the truth there and they do not call it. `_bufferLoadFinishOpts` gains the coverage the CI gate can see: both mux sources flush, `history` does not, and a payload naming no source does not. Its only coverage was the browser suite, which CI does not run. The JSDoc and the changeset now record the one duplicate window this cutoff cannot close. The server appends output to the byte buffer in the same tick it emits, but broadcasts on a batch timer — 8ms over WebSocket, 16 to 50ms over SSE — so a batch pending when `capture-pane` ran leaves the server after the reply and is replayed although the capture holds it. It is one batch interval wide against a recovery window spanning the whole chunked write, and closing it means flushing that batch server side before the capture. The second browser test asserts its session was created, so a failed create fails it instead of passing with zero hits. docs/architecture-invariants.md no longer claims the replay leaves the queued-event discard window alone. That clause now describes what decides how a load ends, the baseline rule, the batch window, and the three covering tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
62ceb4e87b |
fix(sessions): correct what the missing-pid rule actually recognises
A second real reboot disproved the mechanism the previous commit was built on. Typing `/exit` does not persist `pid: null`, and the session was restored anyway. The pid a session record carries is its `tmux attach-session` process, not the agent. `/exit` ends the CLI inside the pane, `remain-on-exit` keeps the pane, and the attach process stays alive throughout — so Codeman's PTY never exits, no exit handler runs, and the record keeps both its pid and `status: 'idle'`. The lifecycle log for the session that came back shows created, started, stale_cleaned and recovered, with no exit event at all, which is the proof: Codeman never learned the agent was gone. So nothing durable distinguishes an exited agent from a session that was idle when the power went, and this pass restores both. Ark0N/Codeman#446 is about making Codeman notice the dead pane; contrary to what the previous commit's message claimed, this genuinely does wait on that. Until a record can say the agent is gone, the user dismisses or closes those sessions. The rule itself is kept, because a record with no attach process does describe a session that never started or whose pane died outright, and refusing it is right. Only its documentation was wrong. The module header, the branch comment and the test names now say what it recognises instead of claiming the case it cannot see. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
5108a24bf0 |
fix(sessions): never restore a session whose agent was already exited
Found by a real reboot, which is the first thing to catch it. Typing `/exit` ends the CLI process and leaves the session record behind, and the process-exit handler persists `pid: null` with `status: 'idle'` before anything else runs. By status alone that is indistinguishable from a session sitting idle when the power went, so the boot pass offered those sessions back and a click spawned the agents the user had deliberately closed — the exact case the eligibility rule exists to exclude. The absent pid is what tells the two apart, and the plan step now refuses a record without one, under its own `not-running` reason so the boot log says why. On a healthy board every running session carries a pid; a record with none describes an agent that is already gone. Deliberately the conservative direction. A session that somehow persisted no pid while genuinely running is not offered, and its conversation stays reachable from the Resume list, which is where every session would be without this feature. The opposite error spawns processes nobody asked for. Ark0N/Codeman#446 covers the dead panes those exits leave behind, but this does not wait on it: the rule belongs here whether or not the record's shape changes later. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
5f55f9cb65 |
fix(sessions): never let the reboot-restore plan fail recovery
The plan build runs inside the try that decides whether restoreMuxSessions() succeeded, so a throw would be caught there, report restoration as failed, and block the stale cleanup and layout reconciliation that follow. An optional convenience would then break the recovery it exists to help. It is guarded on its own now: the correct way for this to fail is an offer nobody gets. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
18ab2ab595 |
docs(sessions): correct what a failed rebuild is actually likely to be
Ran the feature against a real server for the first time, on an isolated instance, and two claims in the code turned out to be wrong. A rebuild that fails after the session is registered was documented as commonly caused by a CLI binary missing from a freshly booted machine's PATH. It is not: the resolver finds its binary by absolute path, so PATH never enters into it, and a server started without claude on PATH restored every session normally. Nor does an un-enterable workspace fail — tmux falls back to another directory and the pane comes up there. Neither obvious cause throws, so the discard path is defended rather than expected, and the comments now say that instead of naming a cause that cannot happen. The four review rounds that shaped this path all reasoned about a trigger none of them could test. The path itself is still worth having, since a mux failure would reach it, but its comments should not claim a likelihood the machine disagrees with. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
39976041e0 |
fix(sessions): let a dismiss reach the entries a restore is holding
Fourth review of the reboot-restore branch, and the third to find a defect in the previous round's fix. This one is the same shape as its predecessor: a counter keyed on one thing, compared against a set keyed on another. The generation counter was indexed by the entry's owner, while the in-flight set holds the caller doing the restoring. Those are the same person exactly when a user restores their own sessions, which is every case the tests covered. The route deliberately supports the other case: an admin may spend another user's entries. So when an admin restored Bob's sessions and Bob dismissed the banner, nothing matched, the entries came back, and a plan Bob had explicitly dismissed was re-armed for another twenty-four hours. Rather than reconcile the two key spaces, the counter is gone. `take()` now parks the entries it hands out, remembering which caller is spending them, and they stay parked until that restore ends. A dismiss filters the parked entries by `canAccess(entry.owner)` — the same predicate it already applies to the plan — so it reaches them wherever they are. `releaseFlight()` puts back only what is still parked. Expiry and a fresh boot plan unpark everything, for the same reason. There is one key space now, the entry's owner, and the spender is only ever used to tell two concurrent flights apart. That removes `generations`, `snapshotGenerations()`, `bump()`, `bumpAll()` and the argument threaded through the route. The discard grew the teardown it still lacked. A rebuild can fail after startInteractive() resolved, and a restored workspace still carries Codeman's hooks, so the CLI can post a hook event within milliseconds; the transcript watcher that starts from it, the attachment registry, the wait registry and the approvals inbox all outlive the listeners and would meet the retry, which reuses the session id by design. Its steps also run in reverse order now, so no live listener can reach a tracker that has already stopped, and the mux kill has its own guard, because stop() kills the pane in its last block after destroying four trackers. Tests. The run-summary test named an interval and asserted a map entry, so dropping stop() left it green; it now spies on stop(). Nothing pinned that before-spawn must precede setupSessionListeners, which reads the flag that phase restores, so swapping the two lines was silent; the ordering test now includes the listener setup. The retry assertion was a tautology and now asserts a different refs object. Both strengthened tests were verified by reverting their fix. Two new tests cover the admin-restores-another-owner cases this round was about. The server in the discard test is built once and stopped, since its constructor registers handlers on module-level watchers, and the workspace is removed through safeRmHomeTree. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
71ed7b127c |
fix(sessions): make the discard a real inverse of the construction
Third review of the reboot-restore branch. The narrow discard the previous commit introduced avoided everything cleanupSession() did wrongly, and in dropping so much of it also dropped four things it had to keep. The worst broke the retry the whole design rests on. setupSessionListeners() returns early while sessionListenerRefs still holds the session id, and the discard never cleared that entry. So the advertised flow — a rebuild fails because the agent binary is missing, the user fixes their PATH and clicks again — reused the same id, wired no listeners at all, and produced a tab that never showed output, never updated its status and never persisted. That is worse than the leak the discard was added to prevent. Three more registrations leaked with it: a RunSummaryTracker and its interval, an image watcher on the workspace, and the Ralph fix-plan watcher. The discard now undoes each registration setupSessionListeners() makes, in its order, and the per-session custom-model config directory, which holds the endpoint's API key literally and which nothing else would ever remove. The image-watcher flag was restored after the code that reads it, so a session came back reporting the feature as on with nothing watching. It moves to the before-spawn phase, and that phase now runs before the listeners rather than after them. The generation counter that lets a mid-restore dismiss win was global while clear() is ownership-scoped, so one user's dismiss discarded another user's unspent entries, permanently, because nothing rebuilds an in-memory plan. It is now per owner. Bumping only the owners of entries the dismiss removed was not enough either: take() has already emptied the plan by then, so a dismiss landing mid-restore saw nothing of that owner's to remove and invalidated nothing. The owners that matter are those with a restore in flight, filtered by what the dismissing user may access, and that is what clear() now bumps. Plan expiry bumps too, so a restore straddling the 24-hour boundary cannot hand entries back and give an expired plan another full day. Tests. discardPartiallyBuiltSession had no test at all: the only implementation any test ran was the mock's one-line stub, which is why every defect above was invisible. test/discard-partially-built-session.ts drives the real WebServer, and the retry assertion fails if the listener refs are left behind — verified by reverting the fix. The dismiss-race test drove the registry by hand, so deleting the route's generation argument left it green; it now goes through the route, and two further tests cover the multi-user cases. The mock context has now gone stale twice, because route tests pass it as `ctx as never` and tsconfig.json includes only src, so nothing ever compares it to the ports. A type-level guard is therefore inert — I wrote one and confirmed it never fires. test/mocks/mock-route-context-completeness.ts compares the mock's keys against WebServer.createRouteContext() at runtime instead, and names what is missing. Also: the API reference now says workspace-forbidden is judged against the owner's grant, the banner's module header no longer claims Restore always dismisses it, and the detail span gets the same min-width: 0 the phone rule already needed. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
fa52753e8b |
fix(sessions): undo a failed rebuild without deleting the user's data
A second review of the previous commit found that its own repair for the session leak introduced three defects, all from reaching for cleanupSession() to undo a half-built session. That function is the user-initiated delete, not an undo. It banked the session's historical token and cost totals into the lifetime figures, and a reboot never runs cleanup, so those totals had never been counted before; every failed rebuild added them again. It saw the pin that had just been restored and demoted the record to `stopped`, which this pass reads as the durable marker of a deliberate kill, so a pinned session whose rebuild failed became permanently unrestorable. And it recursively removed `.claude-images` from the working directory, which belongs to the workspace rather than to the session, so a failed rebuild destroyed the pasted images of any other live session in that repo. discardPartiallyBuiltSession() now undoes only what the construction did: the map entry, the tab-layout slot, the listeners and any pane the launch created before throwing. The persisted record, the lifetime totals, the Ralph state and the workspace's files are left alone. Re-applying the persisted state also splits in two, which removes the first two defects at the root rather than only at the call site. The half that shapes the pane, the custom-model environment and the nice priority, still runs before the spawn. The half that is the session's own history now runs after it, so a session whose pane never started carries no totals and no pin for anything downstream to misread. The rest of that review. The multi-user workspace confinement re-check read the requesting user's grant, and returns true for an admin, so the case its own comment described was the one it missed; it now resolves the entry owner's grant through isWorkingDirAllowedForUsername, the way cron does. A forbidden workspace goes back on offer, matching both the registry's stated contract and the API reference. The client re-reads the plan after a restore instead of blanking the banner, so entries the server put back stay reachable, and a 409 now says a restore is already running rather than reporting a failure. A dismiss arriving mid-restore wins, through a generation counter the route carries across its take. The re-application also restores the tab colour, the image-watcher flag and the original pinnedAt, via a new Session.restorePin that does not re-stamp the pin time. The phone breakpoint gains min-width: 0, without which a nowrap flex item never shrinks and the buttons still overflow, and it folds into the existing phone block. Ralph's loop configuration still does not survive a restore, because toState() reads it off a live tracker and there is no way to keep it without arming the loop. The method now says so rather than leaving it implied. Tests. The capacity test could not fail on the property it existed for: it filled the board past the cap before the loop, so a single pre-loop check would have passed it. It now leaves one seat, so only a per-iteration check restores exactly one entry. New tests cover the ordering around the spawn, a throw before the loop returning the whole plan and releasing the flight, the dismiss-during-restore race, and that the failure path calls the narrow discard rather than the delete. The shared mock context gains the port method it was missing, which is what made the first run of these tests fail for the wrong reason. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
fbede5cd2a |
fix(sessions): act on the dual review of the reboot-restore route
Fifteen findings from two independent reviews of #442, three of them blocking. Every one is addressed here. The three blockers all sat in the restore route. A rebuild that threw after addSession left a registered session with no pane behind it, visible on the board, holding a layout slot and written to state.json, with its plan entry already spent; the catch now cleans the session up and puts the entry back. The loop checked neither the global nor the per-user session cap, so one click could take a board past a documented limit; capacity is now re-checked per iteration, because the loop is itself creating the sessions it counts. Worst of the three, a rebuilt session carried none of the state its constructor has no parameter for and then persisted itself over the record that held it, zeroing token and cost totals and dropping the pin. The pin matters most: pruning keeps a record only while it is pinned, so discarding it handed the record to the next stale sweep. A new reapplyPersistedSessionState() on the session port restores the pin, the token totals, auto-compact, auto-clear, auto-resume, nice priority, the flicker filter and the custom-model selection, and it runs before both startInteractive and the first persist. The rest, in the order they bite a user. Every rebuild failure was reported as workspace-missing, so the banner told users their repo was gone when the agent had simply failed to start; there are now distinct reasons, and the toast names each one. The client read restored and skipped off the outer response object rather than through the uniform envelope, so every count came back zero and neither toast ever fired. A board left open across the reboot never learned an offer existed, because the banner was seeded only on the page-load path; it now re-reads on every SSE init. The workspace check was existence-only, skipping the multi-user confinement that the create route applies, so a withdrawn grant would not be noticed. The banner had no phone breakpoint while its text was nowrap and its buttons could not shrink. Smaller: a missing workspace is now re-offered rather than dropped, while an already-open conversation is dropped rather than re-offered forever; a throw anywhere in the route returns the unspent entries instead of discarding the plan; the single flight is keyed by owner, since take() already stops two callers receiving one entry; the env clamp's header no longer claims a protection it cannot provide on this path today, and names the check that does bite; the three endpoints are documented in docs/api-reference.md; and the module header now says that os.uptime() reads the host's clock, so the feature is effectively off inside a container. The review also explained why the tests missed all of this: they proved the construction claim through their own copy of the construction rather than through the route, and the route tests used workspaces that did not exist, so no Session was ever built. test/routes/reboot-restore-rebuild-failure.ts mocks the Session module to drive the route's real path, and covers the cleanup, the reason reported, the re-application ordering, the broadcast and the caps. The mock route context gains the port method and the mux call the route needs. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
da933d70be |
feat(sessions): offer to rebuild the sessions a host reboot destroyed
A host reboot takes the tmux server down with it, so every pane dies, reconciliation finds nothing to attach to, and the board comes up empty. Picking yesterday's work back up meant finding each conversation in history and resuming it by hand, one at a time. The boot pass now works out what the reboot killed and leaves it on offer. It runs inside restoreMuxSessions(), in the window where reconciliation has reported the dead sessions and cleanupStaleSessions() has not pruned their records yet, which is the only place the records can still be read. The board shows a banner, and nothing is created until the user clicks it. A click rather than an automatic restore is what makes the reboot heuristic acceptable. The heuristic cannot tell a reboot from a crash that took tmux down inside the same window, so it decides whether to ASK, never whether to act: a wrong yes costs a line of text the user dismisses instead of N CLI processes nobody asked for. Four things are re-checked when the click arrives rather than trusted from boot, because hours can pass and the board moves on. The owner's privilege grant re-resolves through the env clamp. The workspace must still be on disk. A conversation the user already resumed by hand from the Resume list is skipped, since two panes running --resume on one conversation would fight over the same transcript. Entries leave the plan synchronously before the first await, and the route is single-flighted, so a double-click or two devices cannot both reach the same entry. A restored session comes back attached, idle and disarmed. Respawn controllers and Ralph loops are deliberately not re-armed: a machine that just came up is the worst moment to turn an autonomous run loose. Its workspace hooks are installed by the restore route itself, because the boot-time sweep sits behind a gate that is false after a reboot and has finished long before the click; without them a session goes silently blind, with no stop or idle events for respawn, no Approvals Inbox item and no red tab on a blocking dialog. Stats collection starts the same way. The pane is new, so the conversation continues and the terminal scrollback does not. The banner says so rather than letting an empty pane read as a broken restore. The plan lives in memory only. A server restart drops it, which costs the convenience this adds and never the conversation: the conversation is the transcript under ~/.claude/projects, which the Welcome screen's Resume list and the Session Manager already read, so a dropped plan returns the user to resuming by hand. clampEnvOverridesForOwner moves to src/session-env-clamp.ts, since the question it answers is about session privilege rather than about HTTP and it now has a caller outside the route layer. Its test hook stays re-exported from session-routes.ts. Claude sessions only for this pass. The other CLIs name their thread in their own config object, which this does not thread through yet. Remote and docker sessions are skipped on purpose, because both need another host or a container to be up and a freshly booted machine cannot promise either. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
c9515b1d4c |
fix(terminal): keep the output a pane capture could not contain
Live terminal events are queued while a buffer load runs, and the load discards that queue when it ends. That is right when the loaded buffer is the server's accumulated byte history. The route appends to that history right up to the moment it serializes the response, so a queued event already appears in it and replaying it would duplicate output, most visibly Ink's cursor-up redraws. A tmux pane capture is a photograph, current only as of the instant `capture-pane` ran. Output printed afterwards was queued and then dropped, and nothing scheduled a re-fetch to recover it: `_onSessionNeedsRefresh` is wired only to the 128KB overflow path. The CLI's next partial redraw then landed on a frame the terminal never received. How much went missing depended on which capture the route served. A `?full=1` load returns the capture alone, with no history in front of it, so it lost everything from the capture to the end of the chunked write. A `?tail=` load returns history, a clear, and then the capture, and the route reads that history after the capture, so it lost everything from the response to the end of that write. The chunked write dominates either way. An agent CLI hides the loss on its next full redraw; a shell session does not, because its output is linear and nothing repaints it. Queue entries now carry their arrival time, and `_finishBufferLoad` takes a `since` cutoff, so a capture load replays exactly the tail that arrived after the response headers. The earlier events stay dropped, because a payload that carries history does hold those. All four paths that fetch a terminal buffer and write it now decide this the same way, through one `_bufferLoadFinishOpts` helper, so they cannot drift apart: `selectSession`, `_onSessionNeedsRefresh`, `_onSessionClearTerminal` and `_maybeRefetchFullHistory`. The second of those is the one that stings. It exists to restore output the client already dropped once under backpressure, and it was dropping more output while performing that recovery. The cache-hit write inside `selectSession` stays on discard deliberately: it runs before the fetch, so its queue holds only events the capture that follows already contains. Two further things had to change for that tail to still exist when the load ends, and a browser test is what found both. `chunkedTerminalWrite` is what ends the load for every non-empty buffer, so the flush policy travels to its own finish calls; the call in `selectSession` runs only when the write was skipped. `_beginBufferLoad` no longer empties the queue when one load re-enters it, which it does on every write, because that reset discarded the whole fetch window before anything could replay it. The response already distinguishes the sources. `source` reads `mux-visible` or `mux-full-history` for a capture and `history` for the byte stream. Follows #395, #396 and #397, which fixed the ways the replayed frame itself could disagree with the terminal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael GrundbergandClaude Opus 5
|
dae2ac580f |
fix(terminal): read the colour env from the registry on every local spawn path
buildClaudeEnv(), the direct-PTY fallback taken when mux creation fails, now
reads getCli('claude').env and applies its unset and exports lists. It used to
delete COLORTERM and CLAUDECODE from a hand-maintained list of its own, which
left it contradicting the registry entry that the tmux pane and the attach
client both read. An engine value needing a mux name has nothing to resolve
against on this path, so it is skipped rather than guessed.
Claude no longer unsets NO_COLOR. The invisible-background bug does not need
it, and unsetting it overrides a preference the user set deliberately, so a
user who exports NO_COLOR globally keeps monochrome panes. The other seven
truecolor CLIs still unset it; that inconsistency is intentional and the
comment on the entry says so.
The invariants doc gains a Terminal colour env paragraph under Session launch
modes, where a reader looking up Claude will find it — the previous sentence
sat under a heading that lists only the non-Claude CLIs. It now says the lists
are the stock catalog and a clis.json override replaces them wholesale, and
that the declarations reach the tmux pane, its attach client and the direct
PTY but not a remote pane, whose command carries no env exports at all. Docker
hands COLORTERM=truecolor to every mode, including the two the registry says
must unset it.
The changeset named six peer CLIs and there are seven: deepseek also exports
truecolor. A test beside the existing OpenCode assertion pins the new
behaviour, so a future registry edit cannot make the backgrounds vanish again
in silence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Michael GrundbergandClaude Opus 5
|
7767b16d4f |
fix(terminal): let Claude use truecolor so its themed backgrounds render
Claude draws the user's own messages as a block of background color, and inside a Codeman pane that block was invisible. tmux hands each pane TERM=screen, which supports-color reads as 16 colors, and Claude's registry entry deleted COLORTERM on top of that. Claude therefore quantized every RGB color its theme asked for down to the basic palette, where rgb(55, 55, 55) and every other dark background becomes ESC[40m, the terminal's own black. Changing the color in a custom Claude theme moved nothing on screen. Claude now exports COLORTERM=truecolor and unsets NO_COLOR, matching codex, gemini, antigravity, pi, grok and omp. CLAUDECODE stays unset, because Claude reads it as a signal that it is running nested inside itself. Both the tmux session and the attach client read this one registry entry, so they cannot disagree. PR #3 introduced the unset in February, citing xterm.js#484 for the claim that xterm.js mishandles truecolor. xterm.js closed that issue in April 2019, Codeman now depends on @xterm/xterm 6, and TmuxManager already sets terminal-overrides ",*:Tc" on its own tmux server, so 24-bit color reaches the browser today for every CLI that asks for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Michael Grundberg
|
77fcd65b4a |
fix(terminal): force the re-measure, bound the wait, and test both
Review of the previous commit found that waiting for the font does not, on its
own, do anything.
`FitAddon.proposeDimensions()` measures nothing — it divides the container by a
CACHED cell size, and xterm refreshes that cache only from `open()`, from a
resize that actually changed the grid, and on a device-pixel-ratio change.
Nothing in it listens for font loading. So a fit that runs after the font
arrives can still divide by the fallback cell, propose the grid it already has,
and short-circuit before anything re-measures. The wait now ends by calling
`_charSizeService.measure()` itself, which is the step that makes the following
fit see the real font. Private API, as FitAddon's own dependency on `_core` is,
and guarded because a terminal can be disposed mid-wait.
The wait was also unbounded, and it sat behind the buffer-load gate. Neither
`FontFaceSet.load()` nor `FontFaceSet.ready` has a deadline, so a font request
that never settled left the tab spinning with live output queued behind it —
permanently, and on every session, since they share one promise. The comment
claimed the opposite ("a font that never loads must not block the terminal, so
this always resolves"), which was true of the per-face loads and false of
`ready`. It is now raced against TERMINAL_FONT_WAIT_MS, and the await moved
ahead of `_beginBufferLoad` so a slow font cannot hold output back at all —
which also removes the stale-select interaction with `_restoringFlushedState`,
since that flag is not yet set when the wait runs.
The awaited set no longer includes faces that cannot move the measured cell.
The bundled symbols font is ~1.2MB of private-use-area glyphs and xterm
measures `W`, so awaiting it put a megabyte in front of the first frame for
nothing; the generic families match no FontFace at all.
A runtime font change had the same race the boot-time one did:
applyTerminalFontFamily wrote the new family and fit on the next line, against
a family the browser might not have loaded. It now re-arms the wait and fits
again when it settles.
The claim that this could not be tested was wrong: the repo's vm harness
reaches both halves. The new suite pins the family filter, the forced
re-measure, the deadline, a rejecting load, a browser with no font API, and a
terminal disposed mid-wait — plus the four ordering properties in
selectSession, including that iOS Safari's synchronous focus still precedes the
first await. Each assertion was checked by reverting its fix.
Also corrects the docstring's reason for calling `document.fonts.load` (the
stylesheet is render-blocking and long parsed by then; the real reason is that
the WebGL renderer rasterises through a canvas atlas, and canvas text never
triggers a CSS font fetch), restores the JSDoc block the previous commit
displaced from getTerminalDimensions, and fixes a comment that described the
first fit as already having run when the mobile-Safari branch defers it.
|
||
|
Michael Grundberg
|
2b57c595df |
fix(terminal): gate the row-preserving skips on a capture, not the query flag
Review of the previous commit found the guard inverted: the three skips keyed on `?full=1`, which is only what the client asked for. When the capture comes back null — ENOBUFS, a timeout, a vanished pane, or a session with no mux at all — the reply falls back to the byte history, which IS a stream of successive frames and still needs stripping. Gating on the request returned it whole: measured at 82KB against 4KB for the same buffer without `full=1`. A direct-PTY session takes that path on every first selection, not only during an outage. The skips now key on `isFullCapture`, meaning a capture arrived. Three further defects the same review surfaced, all on this path: Keeping the trailing rows is only sound when a cursor move follows to count back up from them. On the two branches where the cursor query fails there is no move, so the caret was left at the bottom of the pane — worse than before. The cursor is now read first and settles both decisions together. The move is relative rather than absolute. `CUP` numbers rows from the top of the browser's screen, so it is only right while the browser's row count equals the pane's, and `resizeWindow` does not wait for tmux, so a capture can be taken before a requested resize applies. Measured against real tmux with a browser four rows shorter than the pane: the absolute move lands on a blank row, the relative one lands on the caret's row. An all-blank pane no longer reads as content. Retaining trailing rows and appending a move made it non-empty, and the caller treats non-empty as "replay this", so a blank screen would have replaced real history — the downgrade `_replayWouldShrinkBuffer` refuses, arriving from the server side where that guard cannot see it. The documentation claimed one line per screen row. `-J` joins a hard-wrapped row into its logical line, so that is false whenever any row wrapped: measured at 10 lines for a 12-row pane. Both entries now say what actually holds, and the stale "NOT repositioned" contract in the mux interface is updated too. Tests: the byte-history fallback is stripped, an empty capture leaves history intact, and the extracted helpers are unit-tested directly rather than through source-text matching. The slice window in the capture test is bounded at the next method, having overrun into its neighbours. |
||
|
Michael Grundberg
|
070e8da81b |
fix(terminal): yield only the resize send, and take sizing back on redock
Review of the previous commit found four defects in it. The guard sat above the local fit, so it suppressed a reflow as well as the server write. tab-rail-resize performs its single settle-time refit through sendResize and has no fallback for a truthy activeSessionId, so dragging the rail stopped reflowing a detached session's terminal in the dashboard. The mobile-keyboard guard fourteen lines below already draws the line correctly — withhold the send, never the reflow — and the guard now sits after the fit. _lastResizeDims is one value for the whole window, and both guards skip updating it, so while a popup owns a session that value no longer describes the PTY. _redock repaired it only for the active session. Pop out A, switch to B, close the popup: selecting A later found unchanged dimensions, returned "unchanged", and selectSession skipped its 400ms redraw wait — while the server, comparing against the real pane, did resize and did raise SIGWINCH, so the fetch painted the pre-redraw frame. _redock now clears the record on every path, active or not. _redock could also fire a resize for a session already gone: _onSessionDeleted redocks before cleanup, so the id can be dead and the request is a guaranteed 404. It now checks the session still exists. restoreTerminalSize — the header's redraw button and Ctrl+Shift+R — silently did nothing for a detached session while still reporting success with dimensions nothing was set to. It now says the session is sized by its own window, where the same button works. The `force` comment claimed a client-side dedupe that does not exist; the deduplication is server-side against the real pane. Corrected to say what the flag actually buys. The _redock doc comment now records that the function writes to the server and is not idempotent. Tests: _redock was the untested half and is the half three of these defects sit in. It now has coverage for clearing the stale record on both the active and inactive paths, re-asserting only for the session being shown, and staying silent for a deleted session. The existing sendResize test now asserts the local fit still runs. |
||
|
Michael Grundberg
|
0e82443222 |
fix(terminal): fit the terminal only once the terminal font is measurable
Opening a session could render its frame with characters spliced into each other, as though two frames were overlaid — a status-line fragment landing in the middle of a file path, for instance. Resizing the browser window cleared it. The first fit runs while the browser is still painting with a fallback font. A cell measured against that font has a different width and height from one measured against the terminal font, so the fit produces the wrong column and row count. Codeman sizes the pane to it and replays the capture. When the font finishes loading the measurement changes, the pane is resized a second time, and the CLI repaints for a shape that does not match the frame already on screen. Its later partial updates then land on the wrong rows. selectSession now waits for the font before it measures, so the pane is sized once, at the size that sticks, and the capture is taken at that size. The wait always resolves, so a font that never loads cannot block a terminal, and it resolves immediately once the font is in, so a tab switch pays nothing after the first load. document.fonts.ready alone is not enough: it can resolve before the stylesheet declaring @font-face has been parsed. document.fonts.load for each family in the stack is what actually requests the faces. Measured on a session opening at 2328px wide: the cell went from 8.43x16.00 to 8.00x21.00 roughly 900ms in, moving the grid from 112x36 to 118x28 after the replay had already been painted. |
||
|
Michael Grundberg
|
323730a29d |
fix(terminal): keep row alignment in the full-history pane replay
Switching to a session left the caret one row below the composer's input line, on the box border, and every cursor-relative update the CLI sent afterwards was measured from the wrong row. Any fresh output repaired it, because the CLI then repainted the whole frame. Two things were wrong with the full-history replay, and they compound. The capture never restored the cursor. The visible-frame path ends with an absolute cursor move back to the pane's position; the linear path returned its text and left the caret wherever the last character landed, which for an agent CLI is the bottom-most row carrying text — the status line. The rows it addressed did not line up with the pane's rows either. Four transforms ran over the capture and each can delete a line: the trailing blank rows were stripped, redraw-bloat stripping ran, the trim that cuts everything above the Claude banner ran, and leading whitespace was removed. All four are right for a byte stream of successive frames. A capture is the rendered pane, one line per screen row, so each deletion shifted the frame out from under the restored cursor. The full-history path now appends the pane's own cursor position and keeps every row, so row N of the reply is row N of the pane. The visible-frame and tail paths are untouched. Restoring the cursor is what makes row alignment load-bearing here, and neither CLAUDE.md nor the architecture invariants said so — which is how four line-deleting transforms accumulated on the path. Both now record it. Verified against a live 315x59 pane: the reply carries 59 rows, its row 55 is the composer's input line matching tmux, and it ends with the cursor move that lands there. |
||
|
Michael Grundberg
|
5ac516dd3b |
fix(terminal): let a detached session's own window own its pane size
Popping a session out left both windows sizing the same pane. The dashboard keeps the session active and keeps measuring it, and its terminal is narrower than the popup because the session rail takes width the popup does not have. One PTY cannot hold two sizes, so the CLI drew frames that fit neither window and the popup showed a garbled frame. sendResize and the debounced window-resize handler now stand aside for a session this window has marked detached. A solo window is exempt, since it is the owner. _maybeRefetchFullHistory already stood aside on exactly this condition, so the rule is not a new one. Sizing has to come back when the popup closes: while it owned the session the dashboard sent no resizes, so the PTY still holds the popup's geometry. _redock now re-asserts, with force, because the dimensions the dashboard last sent are the ones it is about to send again. Reproduced with a dashboard and a popup on one session: before, the pane sat at 315 columns while the popup rendered 289. After, both report the same size and the popup's frame matches the pane exactly. |
||
|
Michael GrundbergandClaude Opus 5
|
b87bc6871b |
fix(paste): handle only the first paste event the Ctrl+V trap receives
Ctrl+V in the terminal inserted the clipboard text twice. Right-click →
Paste inserted it once.
`_handleImagePaste()` appends a hidden contenteditable div, focuses it, and
reads the clipboard out of the paste event that lands there. Two separate
routes deliver that event for a single keypress. The function issues
`document.execCommand('paste')` itself, which in Firefox dispatches a
trusted paste event and then returns false, because the trap cancels the
event and the command never completes; Chromium and WebKit refuse that
command and dispatch nothing. The keydown's own default action delivers the
other, because xterm calls the custom key handler before its own `cancel()`,
so returning false never calls preventDefault. Firefox therefore ran the
trap's listener twice and both runs reached `terminal.paste()`. The
context-menu paste involves no keydown at all, which is why that path stayed
correct.
The trap now accepts the first paste event and cancels every later one, so
how many paste events a browser delivers no longer changes what the PTY
sees. Measured on a live install, one Ctrl+V each: Firefox two events and
two writes before this change, Chromium and WebKit one and one, and every
engine one write after it.
The `execCommand('paste')` call stays. Stripping it out also ends the
doubling, and all three engines still deliver one event without it, since
`trap.focus()` has already run when the key's default action resolves. It is
kept because the trap technique arrived in #84 for plain HTTP and for
mobile, and a desktop measurement says nothing about real iOS Safari or
Android Chrome: where a browser aims the default action at the element
focused when the keydown began, the command is the only route into the trap,
and the trap is the only place clipboard image blobs are read.
test/image-paste-trap.test.ts loads image-input.js into a `node:vm` context
with a fake document and fires two paste events at the trap. It covers text
and images, and fails on the old code with the text pasted twice and the
image uploaded twice.
Docs: the invariant goes into docs/architecture-invariants.md as a Terminal
paste section and into CLAUDE.md as a Frontend entry, both recording the
measured event counts and why the redundant call is still there. README.md
and the Keyboard Shortcuts and Input and Voice wiki pages gain a Ctrl+V row,
which all three tables were missing while listing every other clipboard
binding.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Michael GrundbergandClaude Opus 5
|
2f9663e389 |
Merge branch 'master' into feat/codex-resume
master and this branch both rewrote the two `_claudeSessionId` resets inside
`start()`, so `src/session.ts` conflicted at both of them.
master's commit
|
||
|
Michael Grundberg
|
327e440607 |
fix(codex): fold a codex session into its own rollout row
Review fixes for #386. Duplicate rows. A codex conversation showed twice, once live and once as a past rollout row, because nothing aliased a codex session to its thread id. That is worse than cosmetic: the stale row still resumes, so clicking it starts a second `codex resume` on a thread already open in another pane. - A RESUMED session knows its thread id up front, so it folds from its own side: add `codexConfig.resumeSessionId` to the `claudeSessionId` chain. Not only in the constructor — `start()` recomputes that id at two further points (the mux branch, and the unconditional "third reset point" whose own comment already warned that omitting omp's fallback there stomps the mux branch's resolved alias). Both listed Claude's and omp's ids only, so for codex every mux reattach and boot recovery reset the alias back to the Codeman id and the duplicate returned. - A FRESH session has no thread id until codex writes the rollout, so it is folded from the other side. The scanner now reports `session_meta.originator`, which is `codeman_<sessionId>` for every pane Codeman spawns, and `gatherUnifiedInputs()` stamps the matching live and persisted rows, newest rollout winning (`/new` inside the TUI leaves several rollouts sharing one originator). - Persisted rows read `codexConfig.resumeSessionId` too. A resumed session demoted to a persisted-only record would otherwise lose its alias, and the originator fallback cannot rescue that one: a resumed rollout keeps its ORIGINAL session_meta, so it still names the pane that created the thread rather than the pane that resumed it. Identity cache. It was written as soon as the thread id was known, but codex writes the first user message only when the user submits, so any scan in that window pinned `firstPrompt: undefined` for the life of the process — and the home screen, the command palette and the search-index refresh all scan. `shouldCacheIdentity()` now keeps an identity only once the prompt is known or the head read filled its whole window. Also from review: both caps count emitted rows rather than file index, so a store of sub-agent threads no longer spends the `lastPrompt` budget before the first row that needed it; the cache is an `LRUMap` sized like the one beside it; the unreachable filename fallback is gone; a rollout recording no cwd is dropped rather than emitted with `workingDir: ''`; and the unified-session module header names all three transcript stores. Tests. The resume wiring now has cases for a row with a thread id, a row without one, and a `resumeId` on a non-codex row; the "no continuation is wired" case narrows to gemini/antigravity, which is no longer true of codex. `codex-resume-alias-survives-start.test.ts` drives a real Session through `start()` rather than asserting on pre-stamped inputs — that gap is why the reset points went unnoticed. Plus the maintainer's own cache repro, the tail-budget case, a no-cwd case, and merge cases for both folds. |
||
|
Michael Grundberg
|
8285fff91c |
feat(codex): list codex conversations and resume them
Codex conversations never appeared in the session list, and the resume path skipped codex, so picking one back up meant finding its thread id by hand and POSTing codexConfig.resumeSessionId to /api/sessions. Two gaps caused it: - The unified list is built from ~/.claude/projects plus omp's own store. Codex writes to neither: its rollouts live in ~/.codex/sessions/<y>/<m>/<d>. - terminal-ui.js sends a continuation only for the CLIs with a "continue most recent" flag. Codex has no such flag — it names a thread by an exact id — and nothing supplied one. Add codex-transcript.ts, the codex analog of omp-transcript.ts, and wire it into gatherUnifiedInputs() beside the omp scan. A rollout row carries `resumeId`, the thread id `codex resume` takes, and the resume path sends it as codexConfig.resumeSessionId. `resumeId` is what keeps the two kinds of row apart: only a transcript scanner sets it, so a LIVE codex row — whose sessionId is Codeman's own uuid — can never ask codex for a thread that does not exist. Three things measured against a real store of 519 rollouts rather than assumed: - Rollouts are far too large to read whole (median 407 KiB, p90 1.3 MiB, max 25 MiB, 381 MiB total), so this reads a 128 KiB head for the identity and the opening prompt and a bounded tail for the most recent one. session_meta is written once and never rewritten, so per-path identity is cached; a warm rescan of that store costs ~75ms against ~470ms cold. - codex 0.152.1 emits no event_msg/user_message rows at all. It writes event_msg/item_completed carrying an item.type of UserMessage. Both shapes are read, plus response_item as a last resort. - That last resort sees injected context, and the first such row is the repo's AGENTS.md every time, so injections are dropped rather than used as titles. Sub-agent threads (thread_source: 'subagent') are left out; codex spawns them for itself and on a real store they outnumber the resumable threads. |
||
|
Michael Grundberg
|
51957e2ed4 |
fix(session): let each CLI declare how its own pane shows work
A Codex session reported `isWorking: false` for its entire life, including
mid-turn. Codeman has four paths that mark a session working, and all four were
inert for Codex:
- The spinner fast path tests eight braille frames, and Codex animates none.
- The activity-streak fallback was wrapped in `!isExternalCliMode(mode)`.
- The pane probe inside `_confirmIdle` would have matched, since Codex prints
`esc to interrupt`, but arming it required the literal glyph `❯` and Codex
draws `›` on its composer row.
- The text detector sat inside `_processExpensiveParsers`, whose first statement
returns early for an external CLI.
Add an optional `workDetect: { promptGlyph, workingLine }` to CliCapabilities,
so the two strings that differ per CLI are registry data rather than constants
in the detector. Claude declares its existing pair and behaves as before. Codex
declares `›` and `esc to interrupt`. The text detector moves above the
external-CLI early return, guarded on the descriptor so a CLI without one still
skips the ANSI strip that the early return used to save it.
A CLI that declares no descriptor falls back to Claude's pair, and the
activity-streak gate now reads "has a descriptor, or is not external", so the
plain shell mode keeps the behaviour it had.
Rewrite the test that asserted the old premise in its own comment, so it makes
the same guarantee for a genuinely uncharacterised CLI, and add Codex coverage
built from verbatim pane captures on Codex CLI 0.152.1.
|