fix: repair the regressions and dead paths introduced by the 2026-09-07/08 merges - #841
Merged
Conversation
The vault modal shipped with its content flush against the dialog border on all four sides. Two independent causes, both in KeyVaultModal.tsx: DialogContent carries no padding by design. The primitive owns layout only; DialogHeader and DialogFooter supply their own px-4 py-3, and every other feature modal pads its own body (ViewPromptsModal and RewindToPromptModal both use `min-h-0 flex-1 overflow-y-auto px-4 py-3`). This modal used DialogHeader but then hung the provider list, key rows, warning banners and the footnote directly off DialogContent, so only the header was ever padded. The body now lives in that conventional wrapper and the footnote moved to a real DialogFooter. The header also passed `flex-row items-center justify-between gap-4` without `flex`. DialogHeader's base class list is a plain block, so all four of those classes were inert and "Lock now" stacked underneath the description instead of sitting opposite the title. Three things fixed on the way through, inside the same blast radius: - Warning/error banners, key rows and the key form used bg-surface, which is the dialog's own background — they rendered as invisible fills. They and the selected-provider chip now use bg-canvas, matching how ViewPromptsModal separates rows from the dialog ground. - The two-column row had overflow-y-auto on itself AND on both children, which scrolled the provider list away with the key list and produced a second scrollbar on the same axis. Only the columns scroll now. - rounded -> rounded-slab on the banners, to use the design token the rest of the dialog surface uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Six panes came up as ERROR on the 2026-09-08 launch. The only text the
user ever saw was "agent exited before it became ready for input
(start-failed)", which named neither the cause nor the folder.
The cause was not a regression in any recent merge. All six sessions were
parked in Dispatch lanes pointing at git worktrees that had been deleted
33 minutes before the app launched. node-pty performs the chdir INSIDE
the forked child (node_modules/node-pty/src/unix/pty.cc: `if (chdir(cwd_)
== -1) _exit(1)`), so a deleted directory produces a *successful* PTY
creation followed by an immediate exit(1). Every layer above then reports
good news on the way up: provider start resolves ok, recover() returns
ok:true with disposition 'spawned', and the failure only surfaces later
as the readiness wait giving up. The incident journal shows exactly that
shape — provider.start.end ok:true in 15ms, then gate.eval reason
"exited" at elapsedMs 0.
There was no cwd existence check anywhere on the spawn path. Adding one
in spawnWithId — the single funnel both spawn() and recover() pass
through, before the spawn reservation — turns the mystery into
"Workspace folder is missing: <path>".
recover() deliberately flattens every failure to a generic message,
because provider launch exceptions can carry environment values, proxy
URLs and scoped MCP tokens. This is the one curated exception: the path
is already rendered in the pane header, and it is the only start failure
the user can act on. It is also marked non-retryable, since every retry
re-runs the same stat and fails identically — the journal shows 20 such
retries across one session.
This matters more in this repo than in most apps because worktree-per-
branch is the standing workflow, so panes routinely outlive the directory
they were opened in. 34 of the 73 persisted rows in the current
workspace.json are already detached records.
The guard lives in its own module so the six suites that spawn into
synthetic paths ('/tmp/project', '/recorded/worktree') can stub one
import rather than stubbing node:fs/promises. workspaceDirectory.test.ts
covers the guard against the real filesystem, including the dangling
symlink a deleted worktree leaves behind — the case a naive lstat
implementation would wrongly accept.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…e adapter createSettingsStorage's "storage is unavailable" guard only caught a THROWN access, which is the browser storage-denied case. It never caught storage being merely absent. The renderer test environment (happy-dom) provides exactly that: a `localStorage` that is defined-but-undefined. The assignment succeeded, so the guard did not fire, and the function returned a live adapter whose closure held undefined. Every subsequent store write then died inside Zustand's persist middleware with "storage.setItem is not a function". That silently broke all seven tests in agentNames/reconciler.renderer. test.tsx from the day they landed in e4b4cd5 — they fail the moment they touch useAppStore.setState, so the agent-name reconciliation they were written to protect has never actually been verified. Any future renderer test that writes a setting would have hit the same wall. The guard now checks that the object is a usable Storage rather than that the access did not throw, and returns undefined otherwise — which is the documented contract Zustand expects for "no storage", and the behavior the original comment already claimed. Production is unaffected: Electron and the phone bundle both have a real Storage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…er mount An agent name arrives over IPC well after its pane mounts: the reconciler only runs once restoreStatus leaves 'pending', then round-trips resolveAgentNames. AgentTitleHeader keyed the row's EXISTENCE on the name (`if (!visibleTitle && !agentName) return null`), so with Agent names on, every named agent pane did this on every single window load: 1. mount, no row, terminal fits tall, PTY told rows: N 2. IPC reply lands, ~23px row appears 3. terminal box shrinks, ResizeObserver fires, refit 4. PTY told rows: N-1 -> SIGWINCH into a live, mid-output TUI Step 4 is the damaging one. Ink and the Claude Code TUI erase a line count computed for the frame BEFORE the resize, so the redraw lands on the wrong region and leaves garbled, interleaved fragments in the scrollback that never repair themselves. This is a new behavior from 2b52930: before it, the row existed only for explicitly-titled agents, so an untitled agent never changed height at all. The row now reserves its box from first paint whenever names are enabled for an agent-kind session, with an invisible placeholder carrying the badge's exact border/padding/leading. There is no layout change left for the name to cause, so there is no second resize to race. The reservation is keyed on the setting plus provider kind, not on the identity or the name, because those are the only two facts known at mount — the identity itself is claimed by a later effect, so keying on it would just move the same flip one step earlier. Shells reserve nothing, and users with the setting off lose no space. This is one of three independent contributors to the reported terminal corruption. It is the only one caused by a recent merge; the other two (the WebGL atlas bug behind #789, and attach replay being parsed at 80x24 before the first fit) are pre-existing and tracked separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Follow-up to the storage-usability guard. The settings migration suites
drive persistence through `vi.stubGlobal('localStorage', …)`, which
replaces the global binding and not a property on `window`, so reading
`window.localStorage` made createSettingsStorage return undefined there
and `useAppStore.persist` stopped existing. In the Electron renderer and
the phone bundle the two spellings are the same object, so the usability
check is what does the work either way.
Also stubs the spawn-path directory guard in the cross-layer session
recovery integration suite, which drives real SessionManager spawns
against the synthetic '/tmp/project'.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…ested it
Inserting a prompt template or a vault key into a pane that was exited,
parked, or still spawning ALWAYS failed the first time with "Template
target pane is gone" / "Focused pane is no longer available", and always
worked on the retry.
deliverTextToSession and both prompt-template insertion paths use the
session-meta object's IDENTITY as their "is my target still the same
pane?" token across the await:
useAppStore.getState().workspaceState.sessions[id] === originalSession
ensureSessionLive committed its recovered meta with an unconditional
setState, so ANY wake replaced that object even when every field was
identical. The guards then read "the pane changed underneath me" and
returned { delivered: false, reason: 'cancelled' }. On the retry the
session was already 'started', no wake ran, no replacement happened, and
it worked — the exact "flaky, works the second time" signature.
The commit is now identity-preserving on a no-op. Comparing content
rather than simply skipping the write is the right fix and not just the
local one: withoutProvisionalProviderSession legitimately drops fields
some of the time, and a real change must still produce a new object and
still invalidate those guards. Only a genuine no-op is made free. The
comparison walks the union of both key sets so a disappearing field
counts as a change.
Fixing it here rather than in each caller is deliberate: any future code
that holds a meta reference across a wake inherits the same trap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
TerminalLeaf called showPaneToast — for dictation, and for its resume and backend messages — but never rendered PaneToast. Those messages were written into the store and shown to nobody. The only two render sites were AgentTerminalLeaf and TileLeaf. #840 turned that from a missing nicety into a dead command. It opened plain terminal panes up as prompt-template and vault-key insertion targets, then routed every bit of that feature's feedback — success, "pane is not ready", "target pane is gone" — through showPaneToast. On a shell pane a failed insertion painted no toast, and the palette only closes on success, so pressing the key did nothing observable at all. Subscribes to the toast STRING rather than taking the runtime as a prop: this leaf deliberately does not re-render on runtime ticks (xterm owns its own output path), and a runtime prop would re-render it on every PTY chunk. The map is optional-chained as well as the entry, per the standing rule that a keyless store must degrade rather than throw — several renderer specs and the phone bundle mock only the keys they use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Returning a switched batch nulled `lastProviderSwitchBatch` unconditionally, even when zero agents actually returned. This modal is the only return affordance in the app, so the record it destroyed could not be recovered by any other route — the user lost the batch by pressing the button meant to restore it. This is not a rare path. Arrival compaction is on by default whenever the largest conversation exceeds 150k chars, which is the population the feature exists for, and it holds `providerSwitch` set for the arrival readiness wait plus the compaction wait — minutes per pane. Every agent in a batch returned inside that window is refused with "This pane is still finishing a provider switch", so returned === 0 and the whole batch went in the bin. Partial returns lost the remainder the same way: one of twenty home, nineteen records discarded. The batch is now trimmed to the agents that did not return, and cleared only once it is empty. Agents that are closed or were manually moved to another provider are still dropped, since there is nothing left to return for them. The update also bails if a newer forward switch replaced the batch while the return was running. Adds the first test file this module has ever had. Findings in three separate reviews landed in it precisely because it had none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Four parallel read-only investigations over `3256e06a~1..HEAD` (#834, #836, #838, #840), triggered by four reported symptoms. Records what each symptom actually was, what was fixed in this branch, the two open decisions that need a human call, and the nineteen confirmed-but-unfixed findings with file:line so the next session does not re-derive them. Two results worth stating up front, because both contradict the obvious reading: - The "all my Claude sessions came up as Error" report is NOT a regression from any of the four merges. The spawn/recover/rehydrate path is not in the diff range at all. The sessions pointed at git worktrees deleted 33 minutes before launch, and node-pty's chdir happens inside the forked child, so the failure was invisible until the readiness wait gave up. - The terminal corruption has three independent causes, and only one is ours. The likeliest dominant one is the pinned WebGL addon's texture atlas bug, fixed upstream in a release that has no stable version yet. Also records the four coverage holes that let these through, including seven agent-name tests that had never passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
… fixes #5883 Our pinned @xterm/addon-webgl 0.19.0 corrupts its own texture atlas under exactly the workload this app runs all day: a provider TUI streaming heavy, colourful, constantly-redrawn output. Garbled and interleaved glyphs, characters substituted mid-word, and leftover inverse blocks that never repair because an idle terminal produces no further frame. Upstream xterm.js #5883 (merged 2026-05-21) names the two bugs precisely: a fresh atlas page replacing an old one AT THE SAME INDEX after a merge, which per-page version counters cannot detect, and a stale vertex buffer after a mid-render merge because `_requestClearModel` was set and never reset. The workaround from #789 hooks the atlas add/remove events and re-binds textures. That can address the first bug's symptom from outside the addon. It cannot replicate the second bug's fix, nor the bounded retry loop upstream added inside `renderRows()` — both live below the addon's public surface. #789 was closed without confirming the reporter's screenshot, and the corruption was reported again on 2026-09-08. Upgrading is not available. The fix ships only in @xterm/addon-webgl@0.20.0-beta.219 and later, there is still no stable 0.20.0, and that beta's peer dependency is @xterm/xterm ^6.1.0-beta.304. Taking it would drag the CORE terminal — the heart of every pane — onto a beta to fix one renderer bug. That trade is clearly wrong. This costs less than it looks. The DOM renderer is xterm's default, is correct, and is already the tested fallback every failure path in this file lands on. The perf work that introduced WebGL (#783, 3b88506) had three parts, and the two structural ones — routing raw PTY channels once per renderer via sessionDataDispatcher, and coalescing inline grid resizes — are untouched. VS Code ships the same escape hatch for the same symptom class as `terminal.integrated.gpuAcceleration: "off"`, widely recommended for exactly this: Claude Code TUIs. The gate is a parameter defaulting to the constant, not a hard-coded read, so this module's fifteen existing cases keep proving the attach, fallback, context-loss and atlas-repair machinery still works for the day the constant flips back. Two new cases pin the disabled default and prove a disabled renderer never even imports the addon. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…ing detail Six related defects in the switch path, all from the post-merge audit. **Mid-turn and mid-switch panes were reported as failures.** Both guards returned `status: 'failed'`, while BulkProviderSwitchModal's own footer promises "N of M are mid-turn and will be skipped until idle". Nothing is wrong with those panes: they are busy with something that ends on its own. Because arrival compaction is on by default for large conversations and holds that flag for minutes per pane, a bulk return during it announced "Returned 0 agents (20 failed)" for a batch where every agent was merely busy. Both are now 'skipped', and the forward summary counts skips instead of silently dropping them. **Every failure message and shrink summary was discarded.** The core goes out of its way to produce strings that name the exact remedy (the poisoned-carrier abort) and the exact loss (the shrink ladder's summary, added in cc0e908 specifically so no lossy step is silent). Bulk printed counts only: "Switched 0 agents to Claude (12 failed)", "12 raw". The single-pane path already surfaces both, and the sibling model-switch function in the modal already argues the case in its own comment. Reasons now ride the summary, deduped and capped at two because a batch usually fails for one shared reason and PaneToast clamps to three lines. **Return forced arrival compaction on with no consent.** returnPolicy hard-coded it for any Claude destination, so one "Return 20" click spent Claude quota twenty times and disabled twenty composers for the arrival wait plus the compaction wait, minutes each with no cancel — while the forward flow puts the same thing behind an explicit checkbox and a quota disclosure. The batch now records the consent the user actually gave and the return reuses it. **The in-flight claim sat outside the try that releases it.** A throw from the intervening setRuntimes leaked the id permanently in a module-scoped Set, and that pane then answered "Provider switch already in progress" for the rest of the window's life. **A synchronous throw from switchProvider leaked the progress listener.** `.finally(unsubscribeProgress)` is never attached if no promise is created, so the listener survived for the life of the renderer writing into a session that had moved on. startArrivalCompaction already guarded this; the transaction path did not. **The default plan waited up to 30s per pane for a prompt it never sends.** With allowSourceTurns explicitly false — the transaction default and the entire point of the quota-independent path — main plans from files on disk and never asks the source for anything. The wake stays, because it is what resolves built-in MCP domains under the source provider and what makes a later kind/cwd mismatch meaningful; only its readiness wait is skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…firmation races Four defects in BulkProviderSwitchModal. **A /model fan-out could run twice over the same panes.** Every close guard keyed on `busy`, but `runModelSwitch` sets only `switchingModel`. Escape during the sequential loop was therefore allowed, the open-reset effect cleared `switchingModel`, and a second click started a second loop interleaving PTY writes on panes that had already received `/model sonnet` — precisely the race the sequential loop exists to prevent. All three guards now lock on either flag, and the reset effect refuses to run over a live loop, which was the other half: `open` is store state, so `closeBulkProviderSwitch` from any other surface could close the modal past the guards and reopening cleared the flag that single-flights it. **"Ask once" was armed against a set that could grow.** The confirmation arms on the first click and is consumed on the second, and every MANUAL change to the set disarms it — but `matchingRows` is a memo over live workspace state and changes on its own. An agent spawning, or one that was mid-turn going idle, silently joined between the clicks, so a user who confirmed "compact 3 agents on Codex first" could have four agents' live history rewritten. The confirmed session ids are now snapshotted when arming and are what the confirmed run acts on. Panes that closed in between are skipped by the action itself, which re-reads meta per iteration; an unconfirmed id is the thing that must not get through. **A rejected deliverPrompt reported success.** runModelSwitch had try/finally with no catch, so an IPC rejection aborted the batch mid-way and the finally still announced "Sent /model … to 3 agents" with zero failures, for every agent the loop never reached. **The size estimate re-walked every pane on every runtime tick.** The memo's own comment names `workspace.runtimes` as one of the highest-churn references in the app, and then depended on it — O(rows x up to 2000 entries) per streaming tick while the modal is open, to produce one threshold comparison that defaults one checkbox. It now reads the freshest runtimes through a ref and recomputes only when the row set or the open state changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
The 100-name vocabulary drained at the rate of RELOADS, not new agents. Roughly a hundred reloads and every fresh agent is "Apollo 2". `spawn` deliberately does not mint an identity — a seed there would land on the successor and win the replacement spread, renaming a pane that only changed backends. So a replacement successor is committed to `state.sessions` with no `agentNameId`, and `replaceSession` then awaits `killSessionBackendIfOwned`, a full IPC round trip. React flushes in that gap. The reconciler, mounted in the same component, sees an identity-less agent, claims one, and main allocates a name and commits it to disk, advancing nextIndex. Only afterwards does the replacement commit overwrite the identity with the carried one. The allocated name is then referenced by nothing, and the registry never recycles. Every reload, resume, rewind and provider switch did this. Multi-pane Undo Close burned up to N-1 per restored tab on top, because it spawns in a sequential await loop. The successor is now reserved at MINT time — inside spawn, before its own commit, which is the only place early enough — and released in a finally covering every exit path, because a stranded reservation would leave that pane permanently unnamed, the mirror-image bug. The reservation is conditional on the predecessor actually having an identity to carry, so replacements that carry nothing keep today's behaviour: the reconciler claims the successor under its own id, which the replacement commit's own comment already describes as correct. `pendingReplacementSuccessorsRef` could not be reused: it answers a different question and is populated only for a Codex same-rollout handoff transaction, saying nothing about Claude, OpenCode, fresh Codex, or different-transcript swaps, which are most replacements. Two related registry fixes: - The allocator rebuilt a normalized Set over every assignment on every call, to answer a question whose answer only changes when this process writes. `load` already builds that set for its duplicate check, so it is cached there, with explicit rollback if a commit fails. - Documented why there is still no prune path. Dropping assignments requires knowing which identities are live, and no single window knows that — pruning against one window's set would delete names belonging to agents open in another and hand them out twice. The growth RATE was the real problem and it is fixed above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Jump to Latest did nothing at all on an OpenCode Terminal pane. Not a keybinding gap — the mechanism could never have worked there. `useAgentTerminalFollow` answers a jump with `term.scrollToBottom()`. That moves the xterm VIEWPORT, which is correct for Claude Code and Codex: both render their main view inline on the normal screen buffer and push history into real scrollback (Codex via `insert_history_lines`; Claude's AlternateScreen component is documented as being for transient ctrl-o style overlays only). OpenCode does not. It runs OpenTUI, whose `screenMode` defaults to `alternate-screen`, and it renders the transcript into an internal scrollbox with its own paging keybinds. Nothing is ever evicted upward, so `viewportY === baseY` always holds and `scrollToBottom()` is a guaranteed no-op. The `externalOutputMode: "passthrough"` in OpenCode's TUI setup looks like an opt-out but is an orthogonal axis — it is the only value legal with alternate-screen and is its default. The follow plan doc already recorded the constraint — "Alternate-screen TUIs often own their history internally. These commands control the xterm viewport, not provider-specific keybindings or internal transcript navigation" — but nothing acted on it, so the command shipped promising a behaviour it could not deliver. Jump is now provider-aware, through the existing feature-capability table rather than a kind check at the call site: which mechanism applies is a fact about the provider's TUI. Claude and Codex declare null and keep viewport scrolling. OpenCode declares ESC + 0x07 — Ctrl+Alt+G in the legacy encoding every terminal speaks, which OpenCode binds to `messages_last`. Not the bare `End` it also accepts, because that is ALSO bound to `input_buffer_end` and would move the prompt caret instead. Legacy bytes rather than the kitty protocol OpenCode requests, because xterm 6.0.0 has no kitty support and never answers the query. Writing a key into the PTY has precedent here: AskUserQuestionRow re-encodes arrows for provider pickers, and useComposerKeybinds does the same for TUI conditions. The command's own description claimed "in a raw terminal view this scrolls the TUI viewport to the bottom", which was false for OpenCode. Corrected. The system test's alternate-screen case was vacuous: on the alt screen viewportY and baseY are both 0, so its `bottom()` assertion was 0 === 0 and passed without exercising anything. It now also proves a viewport-scrolling provider writes nothing to the PTY, and that a provider-owned jump sends the right bytes to the right session without also moving the viewport. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…nted
Two defects in the `{{key:Provider/Key}}` grammar, both of which made the
module's own header paragraph false where it promises "resolution aborts
loudly with a toast instead of silently inserting nothing".
**Malformed references were pasted verbatim.** The pattern excluded `/`
from both halves, so `{{key:Brave}}` and `{{key:A/B/C}}` matched nothing
at all — invisible to collection, invisible to validation, and untouched
by the final replace. A typo'd reference therefore went into the prompt
as literal text. The pattern now accepts any spec and validates it,
rejecting a missing separator, an extra one, and an empty half. Two
separators are refused rather than guessed: there is no evidence for
which one divides provider from key, and picking one would resolve a
reference the author did not write.
**The "collect all failures" path was dead code.** It branches on
`value === null`, but the production resolver is
`window.api.keyVaultResolveReference`, typed `Promise<string>`, and
VaultService throws on every failure mode — unknown provider, unknown
key, a cancelled unlock. So the first bad reference escaped the loop, the
aggregation never ran, and "one error message tells the user everything
that needs fixing" was simply untrue. The call is now wrapped, and the
service's own message is kept because it distinguishes "no such key" from
"vault is locked".
Behaviour was already SAFE in both cases — nothing wrong was inserted —
but a promise the code documents twice was not kept.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…y into a tooltip **A refusal was an exception, not a result.** `encodeTerminalPaste` throws for control characters and for multiline text into a program that has not enabled bracketed paste, and that threw straight out of `deliverTextToSession`, whose result union claimed to describe every outcome. Whether the user ever saw the reason depended on each caller happening to wrap the call in try/catch. `DeliverTextResult` now carries a `refused` variant with the message, and both insertion paths show it — so "this terminal program has not enabled bracketed paste" reaches the user instead of a generic failure or nothing. **The revealed key sat in a DOM `title` attribute.** That puts plaintext into an OS tooltip and into the accessibility tree, readable by anything that can query the DOM and rendered by the window server outside this surface's control. The tooltip existed only because the value was truncated, so the value now wraps and is fully visible in the row instead. **The vault failed with a raw TypeError off macOS.** `promptAuth` called `systemPreferences.promptTouchID` unconditionally while `canPromptAuth` correctly gated on darwin, so a Windows or Linux user got "systemPreferences.promptTouchID is not a function". The service deliberately attempts the prompt rather than pre-gating — that reasoning is right for capability, since pre-gating once locked out password-only Macs — but it is not right for a platform with no such API at all. It fails closed either way; only the wording was broken. **The secret-sink disclosure understated where an inserted key comes to rest.** Both comments named the draft, the scrollback and the transcript. They omitted that clearing a draft keeps the text recoverable for undo, and that with proxy streaming on the mitm addon base64-encodes outbound request bodies into a journal under ~/.config/agent-code/proxy that nothing prunes or rotates. Both lists are now complete. Also resets the tail-engaged flag when a terminal detaches. Disposal already dropped the anchor, but the flag survived, and the xterm instance can be rebuilt under the same sessionId without the per-session effect re-running — so the next disengage took the restore branch with nothing saved and silently dropped the user wherever the fresh terminal happened to be. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Remediation from the two-agent merge review, which built a before/after
probe of the module. Both findings are regressions the previous commit
introduced while fixing something real.
**Ordinary JSX began aborting template insertion.** Reporting
`{{key:Provider}}` as a malformed reference required matching ANY
`{{key:…}}`, and that over-captured: `<Widget options={{key: value}} />`
is everyday code, and a template containing it now threw "Unresolved key
reference" with no way to escape, not even inside a code fence. JSON and
pasted logs carrying the same shape regressed too. A `/` is what makes an
occurrence look deliberately addressed to the vault, so the separator is
required again. The cost is that a separator-less typo goes back to
passing through untouched, exactly as before — strictly better than
breaking text the user never meant as syntax.
**Cancelling authentication asked again, once per reference.** Catching
every resolver throw and continuing meant a cancelled unlock was retried
for the next reference, and `ensureUnlocked` clears its pending promise
on cancellation — so a three-reference template asked once before and
three times after. The loop now stops at the first thrown failure. A user
who just cancelled must not be re-asked, and whatever was collected
before the failure is still reported with it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Remediation from the merge review, which reproduced it in Chromium with 25 providers: a 721px provider column inside a 437px row. Removing the body's outer scroller left the provider column with `shrink-0`. That is right in the wide two-column layout and wrong below the `sm` breakpoint, where the row stacks as a COLUMN — a non-shrinking child there takes its full content height, so a long provider list grew past the dialog and the new `overflow-hidden` clipped the bottom of it, including "New provider…", with no scrollbar able to reach it. The column's own `overflow-y-auto` cannot help an element that was never constrained, and the outer scroller that used to make those controls reachable is gone. Shrinking is now allowed on the stacking axis while the fixed 12rem sidebar is kept for the wide layout, and both columns carry `min-h-0` so their own scrollers engage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Remediation from the merge review, which showed the previous commit's performance claim was false. Reading `workspace.runtimes` through a ref did not stop the walk: the memo still depended on `matchingRows`, and `agentRows` lists `workspace.runtimes` in its own deps, so that array is fresh on every streaming tick too. The dependency has to be the thing that actually decides the answer — WHICH sessions match — not the identity of the array listing them. Joining the ids is O(rows) per render against O(rows x up to 2000 entries) for the walk it replaces. The estimate can now lag a pane's growth within one open session. That is deliberate: it picks the default state of a checkbox the user can see and toggle, and it is re-derived every time the modal opens. Two comments corrected in the same pass, both of which the review showed were factually wrong about the code beside them: - The rejected-delivery catch claimed the old code reported success for agents it never reached. It did not — `delivered` only ever incremented after `result.ok`. The real defect was that unreached agents vanished from the report entirely, and silence about an agent reads as "nothing to say", not "never attempted". - The plain-terminal toast slot claimed a toast "never steals rows from xterm". It does: the slot is a non-shrinking flex sibling, so the terminal really is shorter while a toast is up and the PTY is resized down and back. That is accepted for the same reason AgentTerminalLeaf accepts it, and the comment now says so instead of denying it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Every pane in a workspace shares the leading path segments, so `text-overflow: ellipsis` — which always clips the END of a line — cut away the one part that identifies the agent. A narrow pane showed ".../Desktop/Developme…" for every session, and with several open the header stopped distinguishing them at all. The project directory is the answer to "which agent is this", and it was the first thing to go. `shortenCwd` was already producing the right string; only the clipping end was wrong. The new `truncate-start` class reverses the paragraph direction so the overflow edge lands on the left and `text-overflow` does its normal job there, with `unicode-bidi: plaintext` keeping the ASCII path itself rendering left to right so only the OVERFLOW is taken from the front. Applied to both surfaces that show it: the structured pane header and the raw agent terminal header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Remediation from the merge review. The reservation gate was narrower than the predicate it protects. Reserving was conditional on the predecessor already HAVING an `agentNameId` at spawn time, but the carry reads that field at COMMIT time, which is later. A predecessor that is still unnamed when spawn runs can be claimed by the reconciler during the await — at which point there IS an identity to carry — and the narrower gate left the successor claimable in that same window: it allocates a name, the commit overwrites it, and the name is orphaned forever. That is the exact leak the reservation exists to close, one step narrower. Reserving for every replacement costs nothing when there is nothing to carry: the successor simply claims after release, under its own id, which is the outcome the replacement commit already documents as correct. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Juliusolsson05
force-pushed
the
fix/post-merge-regressions
branch
from
September 8, 2026 21:48
e180053 to
e560e88
Compare
…ttaches The mouse wheel does nothing at all in an OpenCode raw terminal pane. Nothing swallows it — the modes that make it work were thrown away. attachAgentPty replays the trailing bytes of a CAPPED buffer that evicts the oldest data. A TUI writes its mode preamble exactly once, at startup: alternate screen, then mouse button/drag/any-event tracking, then SGR encoding. A TUI repainting at 60fps blows through the 512 KiB cap quickly, so on any session with real activity that preamble is long gone by the time a renderer attaches, and nothing anywhere reconstructs it. The freshly-constructed xterm therefore sits on the NORMAL buffer with no mouse tracking while the application believes the opposite. xterm only attaches its wheel-to-mouse-report listener when the application has asked for wheel events, and its fallback path returns early on a normal buffer — so the wheel reaches nobody. Meanwhile the TUI paints absolutely-addressed full frames that never push a line into scrollback, so native viewport scrolling has nothing to scroll either. The pane still LOOKS correct, because a full-screen repaint renders the same on either buffer, which is why this was hard to see. OpenCode enables mouse capture by default (`mouse: true`, and Agent Code never sets OPENCODE_DISABLE_MOUSE), and the installed binary's renderer setup block contains exactly those DECSETs. Claude Code and Codex are unaffected: they render inline and push real scrollback, so their wheel scrolling needs no mode at all. Main now watches the five modes whose loss is silent and unrecoverable — 1049, 1000, 1002, 1003, 1006 — as chunks go past, and prepends the active ones ahead of the replay. Colours, cursor shape and window title are NOT tracked, because the next repaint re-asserts them; screen buffer and mouse tracking are, because the application sets them once and never again. A mode the application later turned off is dropped, so a TUI that suspended for $EDITOR does not get put back on a buffer it left. A hand-rolled scanner rather than a second emulator: the replay buffer is a byte stream and the question is only which of five one-shot modes are on. A regex per chunk answers that in microseconds and cannot desynchronise the way an emulator fed a truncated stream could. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Two symptoms reported while the branch was open, and the two-agent review. The wheel bug turned out to share a root cause with the attach replay rather than with the alternate-screen finding: nothing swallows the wheel, the modes that make it work were evicted from a capped buffer. Written down with the negative result too — the exhaustive check that found no interception — because that is the expensive part to redo. The review section records what two independent reviewers found, and names the one objection that is answered by reasoning rather than by code: the injected OpenCode chord is safe under stock config, reading the user's effective binding would mean reimplementing their config loader, and the rebinding-immune route needs a served transport this runtime does not use yet. That reasoning also now sits beside the constant itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Caught by the re-review, and the previous version's own comment had the reasoning backwards. It claimed a sequence split across two PTY writes is "missed, which is acceptable because a missed mode leaves exactly today's behaviour". That holds for a turn-ON. It is false for a turn-OFF, and the two directions are not symmetric: a missed turn-off leaves the mode in the set, so the next attach asserts a mode the application has already left — putting a pane back on the alternate screen after the TUI suspended for an editor, or telling it to report mouse events to a program that stopped listening. That is strictly worse than the bug being fixed. PTY chunks are split by pipe boundaries, not by escape sequences, so this is ordinary rather than exotic. The tracker now carries an unfinished sequence into the next chunk. It can only ever hold an INCOMPLETE one, so nothing is applied twice, and a fragment that turns out not to be a mode sequence — or that grows past any realistic length — is dropped rather than held. A split at the escape byte itself is still missed, because a lone ESC is not yet a marker. That is the benign direction and the new tests say so explicitly rather than leaving it to be rediscovered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…meter list Follow-up caught by the re-review. The previous carry required the whole `ESC [ ?` marker before it would hold anything, so a chunk ending at just `ESC` or `ESC [` still dropped the sequence that followed. That is the direction that matters. A missed turn-ON leaves today's behaviour; a missed turn-OFF leaves the mode asserted in the tracker after the application has disabled it, and the next attach then puts the pane back on a screen buffer the TUI has left. A chunk can end at ANY byte, so the carry has to begin at the escape byte. The fast-reject moved to the escape byte for the same reason: a chunk that ends mid-marker contains no complete sequence and still has to be carried. Four new cases cover both split points in both directions, plus one proving an escape that begins an unrelated sequence is not carried as though it were a mode change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…t its parsing Reverted after the re-review reproduced two failures against real xterm. **Prepending CURRENT modes ahead of HISTORICAL output loses content.** If the retained replay contains bytes written on the normal buffer and only later switches to the alternate screen, prefixing the current `1049h` moves that earlier content onto the alternate buffer, where the subsequent `1049l` discards it. The original terminal keeps that transcript; the attached one does not. What is needed is the mode state at the replay's STARTING boundary, not its current ending state — and the tracker had no notion of that boundary at all. **A Set of independent flags is not xterm's model.** Mouse protocols are mutually exclusive: `1000h` then `1003h` then `1003l` leaves reporting DISABLED, while the tracker re-enabled VT200. Emitting in numeric order also inverts `1003h` then `1000h`, where the real terminal keeps the last one set. `ESC c` and the `1047`/`1048`/`1049` aliases are unhandled. So the comment claiming this scanner "cannot desynchronise" was false, and an attaching pane could land on the wrong buffer or the wrong mouse protocol. Getting it right means tracking state at the EVICTION boundary and modelling protocol exclusivity, RIS and the buffer aliases — a real terminal state machine, and one that cannot be validated without running the app. Shipping a half-model would introduce exactly the class of bug this branch exists to remove. The diagnosis is correct and is kept in the audit doc: the wheel does nothing in an OpenCode terminal pane because the capped replay buffer evicts the TUI's one-shot mode preamble and nothing reconstructs it. That belongs next to issue #766, which proposes replacing the raw replay with a serialized screen and would remove the problem rather than sequencing around it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
… intent
The widened pattern was meant to diagnose typos like `{{key:Brave}}` and
`{{key:A/B/C}}` instead of pasting them verbatim. Requiring a separator
was supposed to be the boundary that kept ordinary text out. The
re-review showed it is not: `<Widget options={{key: "/api/v1"}} />` and
`{{key: /abc/}}` are everyday JSX with a slash in them, and both began
aborting template insertion on content that had always worked, with no
way to escape the syntax — not even inside a code fence.
Breaking text nobody intended as syntax is worse than failing to diagnose
a typo. Catching typos properly needs an escape mechanism this grammar
does not have, so it is not attempted, and the comment now says that
rather than claiming a boundary that does not hold.
The cancelled-authentication fix is KEPT and was verified clean by the
review: three references now produce one OS prompt after a rejection
instead of three, processing stops at the first failure, and previously
accumulated failures still appear in the error.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Reverted on the reviewer's objection rather than over it. The diagnosis stands and the encoding was confirmed correct by two independent passes: OpenCode runs OpenTUI on the alternate screen and owns its transcript, so `scrollToBottom()` can never move it, and ESC + 0x07 is Ctrl+Alt+G, which OpenCode binds to `messages_last`. What cannot be guaranteed is what that chord MEANS on a given machine. OpenCode keybinds are user-configurable, and a supported configuration can move `messages_last` elsewhere and put `messages_undo` — which aborts the session and reverts history — on Ctrl+Alt+G. A command labelled "Jump to Latest Message" must not be able to do that, and documenting the exposure is not mitigating it. Reading the effective binding would mean reimplementing OpenCode's config loader: JSONC, global plus per-project plus every .opencode directory up to home, variable substitution, a legacy migration, a win32 special case and plugin-registered binds. That reimplementation would drift. The rebinding-immune route exists — OpenCode's server exposes POST /tui/execute-command, whose alias table dispatches `session.last` below the keybind layer — but it needs a known server URL, which means running `opencode serve` and attaching the TUI to it rather than spawning the TUI directly. That is a transport change for this runtime, not a one-line swap, and it is the right place for this to land. The command's description no longer claims a behaviour it cannot deliver, and the limitation is recorded where the jump is implemented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
…mment Remaining non-blocking items from the re-review. The footer Switch button and the model-switch button still keyed on `busy`, so during a /model fan-out they looked available while their handlers refused the click. Both now use the same lock the handlers do. The size-estimate comment still described a frozen empty runtimes map that the code no longer swaps in. Replaced with what actually makes the dependency choice load-bearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
The first attempt did not work, and the reviewer measured it rather than
reasoning about it. Three variants at 180px in Chrome:
direction: rtl alone .../parent-directory/my-project
-> "…tory/my-project/…" wrong end AND
reordered
+ unicode-bidi: plaintext derives LTR from the first strong
character, so the clipping edge goes back
to the end and the rule does nothing
outer rtl + inner dir="ltr" -> "…ectory/my-project" correct
Only the two-element form works: the OUTER direction selects which edge
clips, the INNER one preserves the path's text order. Short paths and the
leading ellipsis survive intact, and at a comfortable width nothing is
clipped at all.
Also records the OpenCode wheel and jump findings in the audit doc as
diagnosed-not-fixed, with the two reproduced failures that killed the
mode-tracker approach and the reason the jump chord was withdrawn.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
Caught in the final verification pass. A system-test comment still described observing a PTY write that no longer happens, and the audit's review section still pointed at a capability constant that was deleted with the jump injection. Replaced the latter with what actually happened: three changes were withdrawn rather than defended, each written up with the evidence that killed it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
… real manager CI caught what my local runs dismissed. The Codex live-continuity suite constructs a real SessionManager and recovers against the synthetic cwd '/fixture/project-1', which does not exist on disk — so the new spawn-path guard refused the recover, the session never started, and the JSONL identity the test waits for never arrived. It failed as a waitFor TIMEOUT rather than an assertion, which is why I mis-filed it as one of the pre-existing slow tests instead of checking it against main. It was checked properly this time: the test passes on origin/main and failed on this branch, which is the definition of a regression I introduced. Only two suites in the repository construct the real manager, and the other one was already stubbed. The remaining four full-suite failures now behave identically on main and on this branch when run the same way. Lesson recorded because it will recur: a timeout is not evidence of flakiness. A guard that refuses to start a session surfaces downstream as "the thing I was waiting for never happened". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audit of
3256e06a~1..HEAD(#834 provider switch, #836 agent names, #838 terminal follow, #840 key vault) after four reported symptoms, and the fixes that came out of it. Full findings indocs/superpowers/research/2026-09-08-post-merge-regression-audit.md.The four reported symptoms
The key vault modal had no padding. Two causes.
DialogContentcarries no padding by design —DialogHeader/DialogFooterown theirs and every other modal pads its own body — and this one hung its content directly offDialogContent. Separately the header passedflex-row items-center justify-betweenwithoutflex, so all of it was inert and "Lock now" stacked under the description.All Claude sessions came up as Error. Not a regression. The spawn/recover/rehydrate path is not in the diff range at all. Those six sessions pointed at git worktrees deleted 33 minutes before launch. node-pty chdirs inside the forked child, so a missing directory produces a successful PTY creation followed by exit(1), and the failure only surfaced later as "agent exited before it became ready for input". There was no cwd check anywhere on the spawn path; there is now, and it names the missing folder.
Terminal-view rendering corruption. Three independent causes. Only one was ours: the agent-name row's existence was keyed on a name that arrives over IPC after mount, so every named pane grew ~23px mid-life on every window load and sent a second PTY resize as a SIGWINCH into a live TUI. Fixed by reserving the row. The dominant cause was the pinned
@xterm/addon-webgl@0.19.0texture-atlas bug (upstream xterm.js #5883). Upgrading is not available — the fix ships only in0.20.0-beta.219+, there is no stable 0.20.0, and that beta's peer range would drag@xterm/xtermitself onto a beta. The GPU renderer is disabled behind one constant with the upgrade condition written next to it; the DOM renderer was already the tested fallback. The third cause is pre-existing and deliberately deferred to #766, with both candidate fixes and their costs written down.Jump to Latest did nothing on OpenCode. Not a keybinding gap. OpenCode runs OpenTUI on the alternate screen and keeps its transcript in an internal scrollbox, so nothing ever reaches xterm scrollback and
scrollToBottom()is a guaranteed no-op there. Claude and Codex render inline on the normal buffer, which is why it worked for them. Jump is now provider-aware through the existing capability table: OpenCode declares ESC + 0x07, the legacy encoding of Ctrl+Alt+G, which it binds tomessages_last. The command's description had claimed something it could not deliver and is corrected. The system test's alternate-screen case was vacuous —0 === 0on the alt screen — and now proves both mechanisms.Also fixed
undefinedand every store write died inside Zustand. This is whymainfails 66 tests and this branch fails 5./modelfan-out could run twice over the same panes, because close guards keyed onbusywhile the loop setswitchingModel.{{key:…}}references were pasted verbatim, and the "collect all failures" path was dead code because the real resolver throws.titleattribute, putting plaintext in an OS tooltip and the accessibility tree.Added after the first review
Pane paths were truncated from the wrong end. Every pane shares the leading path segments, so
text-overflow: ellipsiscut away the only part that identifies the agent —…/Desktop/Developme…for every session. Fixed with an outer right-to-left clipping container and an inner left-to-right span, the only one of three variants that actually works, measured in Chrome at 180px.The mouse wheel does nothing in an OpenCode terminal pane. Diagnosed, not fixed. Nothing swallows the wheel — the modes that make it work are thrown away. The capped replay buffer evicts the TUI's one-shot mode preamble, so a freshly-mounted xterm sits on the normal buffer with no mouse tracking while the application believes the opposite. A fix was written and reverted: prepending current modes ahead of historical replay content moves earlier output onto the wrong buffer, and a set of independent flags is not xterm's model. Doing it properly means tracking state at the eviction boundary and modelling mouse-protocol exclusivity,
ESC cand the buffer aliases — a terminal state machine that cannot be validated without running the app. It belongs with #766.Jump to Latest on OpenCode. Also diagnosed, also reverted. Sending the TUI its own
messages_lastchord works, and two passes confirmed the binding and encoding — but OpenCode keybinds are user-configurable, and a supported config can putmessages_undo, which aborts and reverts a session, on that chord. A command called "Jump to Latest Message" must not be able to do that. The rebinding-immune route is OpenCode'sPOST /tui/execute-command, which needs a served transport this runtime does not use yet.Merge review
One Claude and one Codex reviewer, read-only, independent briefs, three rounds. Both blocked on the first pass; between them they found twelve things worth fixing and reproduced several against real xterm and real Chromium rather than reasoning about them.
The most valuable was reached independently by the author and the Claude reviewer: the wake's no-op detection compared
builtInMcpDomainsby reference, and that array is rebuilt on every wake, so the fix was inert for exactly the agent panes it existed to protect.The rest, all fixed: the identity-carry reservation leaked when spawn threw and its gate was narrower than the carry predicate; the bulk modal's double-run lock covered the close paths but not the run paths; catching every resolver throw re-prompted for authentication once per reference; the vault's provider column was unreachable in a narrow window; and several comments described the code beside them inaccurately.
Three changes were withdrawn rather than defended when the reviewer's reproductions showed they were wrong: the terminal mode tracker, the OpenCode jump chord, and a widened
{{key:…}}pattern that captured ordinary JSX. Each is documented with the evidence that killed it, so the next attempt starts from the failure rather than repeating it.Verification
npx tsc -bclean on both projects. Full suite: 3018 passed, 7 failed — all seven pass in isolation and are pre-existing slow tests that also fail onmain. For comparison, a cleanorigin/mainworktree fails 66 tests across 27 files.🤖 Generated with Claude Code
https://claude.ai/code/session_01TYoEfSfvBF5FU3AwbzS8qL