mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
0e1191b774ad7f1ac32696ee0a0d9de1d3496904
22
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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
|
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.
|