7 commits

Author	SHA1	Message	Date
Danny Avila	c1c3d67837	test(e2e): use accessible message action locators	2026-08-02 19:03:13 +02:00
Danny Avila	e7f1838515	⚡ feat: Reliable Interrupt & Steer Escalation and Recovery (#14558) ... * feat: surface interrupt-steer escalation on waiting messages The interrupt & steer feature shipped reachable only through the composer chord, the send-button hovercard, and the composer button; a message already waiting (queued for after the run, or steered and parked at the next tool boundary) had no path to it. Both waiting surfaces now carry one: - Queued rows get an icon-only ZapOff escalation button beside the existing Steer primary. It routes through sendQueuedNow, which now takes a preempt option on its live-run path. The tooltip teaches the composer chord, derived through resolveComposerKeyDown so a rebound or yielded chord is never advertised. - In-flight steer bubbles get an "Interrupt now" overflow entry with the same race rules as Edit: reclaim first, and only a `reclaimed` outcome resubmits (via retrySteer with preempt, swapping the chip for an interrupting one). `applied` and run-ended-mid-reclaim outcomes stop at the existing informational toasts, so the words can never land twice. Not offered on a steer already preempting. - Every during-run overflow menu gains an "Always interrupt instead" toggle for steerInterruptsByDefault, next to the existing steer/queue default toggle. MenuEntry supports disabled for the new entries. Only one interrupt can be unresolved at a time: while one preempt is pending (or the run is paused on approval, where the server 409s), every escalation control disables instead of racing the same seal. Ten new tests across both surfaces; 381 green in the affected suites. * fix: lock escalation across its reclaim window, keep the paused control visible, label as steer Codex round 1, all three findings. P2, escalation race. The single-interrupt invariant had a window between clicking "Interrupt now" and the reclaim resolving, where no preempt chip existed for the chip-derived gate to see: two bubbles escalated back-to-back could both resubmit. A shared escalating flag (Jotai, per-conversation) now covers the window and disables every escalation control on both surfaces, and a fresh recheck before resubmitting catches an interrupt armed elsewhere meanwhile (composer chord, queued row); those words re-home to the queue with an informational toast instead of breaking the invariant. P2, unreachable paused state. canSteer is defined as hasRealConvoId && !pausedOnApproval, so gating the button on canSteer removed it exactly when it was meant to render disabled; the test only passed on an impossible stub combination. The render gate is now duringRunActive && (canSteer || pausedOnApproval), and the test uses the real invariant. P2, label semantics. "Interrupt & send now" borrowed the name of the hard-abort action; this one preserves the partial answer and steers. Renamed to "Interrupt & steer now" (com_ui_interrupt_steer_now). Both behavior fixes counterfactually verified; 384 tests green across the affected suites. * fix: disable bubble escalation while the run cannot accept a steer Codex round 2, one P2. Answer mode (ask_user_question) sets duringRunActive false while pausedOnApproval stays false, since that flag only detects approval-bearing tool calls. The bubble's escalation entry stayed enabled there, so clicking it cancelled a healthy waiting steer and the preempt resubmission bounced off RUN_PAUSED, degrading the words to the queue. The entry now also disables on !duringRunActive, matching the queued-row control's gate. Counterfactually verified: reverting the gate fails the new answer-mode test. * fix: recheck live run state after the reclaim, not just at the click Codex round 3, one P2, and it is the round-1 recheck principle applied one level deeper: the entry-time disable cannot see a run that pauses (tool approval, answer mode) while the reclaim round-trip is in flight, and the .then closure held the render's stale steering controls, so the resubmit would fire into a RUN_PAUSED rejection after the reclaim had already surrendered the steer's boundary slot. The escalation continuation now reads the LIVE controls through a latest-ref: if the run can no longer accept a steer, the words re-home to the queue with an informational toast instead of resubmitting, and the resubmit itself also goes through the live controls. Counterfactually verified: reading the stale closure instead of the ref fails the new mid-reclaim pause test. * refactor: make escalation one atomic server-side arm, in place Codex round 4: four P2s, every one an interleaving of the same window — escalation as reclaim-then-repost is a compound, non-atomic operation whose continuation must revalidate the world (FIFO position lost, ref assigned too late, no run fence, competing bubble actions). Rounds 1-3 patched that window with a lock and rechecks; round 4 shows the window itself is the defect, so this removes it instead of guarding it again. Escalation is now POST /chat/steer/arm: the server flips preempt on the EXISTING queued item in one atomic store op (new IJobStore.armSteer; a decode-patch-encode LSET Lua on Redis, an in-place mutation in memory), fenced to the validated generation and refused once the queue closes. The handler mirrors the steer POST's preempt contract exactly: durable flag gated on the owner's recorded capability, volatile requestPreempt fire-and-forget because the durable flag is the truth resume/handover re-arm from. By construction this resolves all four findings: FIFO survives (the item never moves; the whole queue still drains in instruction order at the seal), there is no continuation to hold stale controls, the store op is fenced to the original run, and a competing Edit/Queue/Cancel either beats the arm (armed:false, chip untouched) or operates on the armed item, whose cancel already disarms. The client escalation entry becomes one mutation: armed:true relabels the chip in place (same steerId, same position), PREEMPT_UNSUPPORTED and lost races toast honestly, and the round 1-3 machinery — the escalating lock atom, the latest-ref, the post-reclaim rechecks and their two toast strings — is deleted rather than extended. Verified: 7 new handler tests on the real in-memory manager (including FIFO preservation and the stale-generation fence), 2 Redis integration tests against real Redis (in-place arm keeps order and every field; missing/stale/closed all refuse), client suites 396 green. * fix: decide capability inside the atomic arm, neutralize the lost-race toast Codex round 5, both findings, both edges of the new arm design rather than its mechanism. P2, capability TOCTOU. A HITL resume on a rolling deploy rewrites preemptCapable for the SAME generation, so the handler's read could go stale between validation and the flag flip, arming a steer the live owner cannot seal. armSteer now returns armed | missing | incapable, with the owner's live capability part of the same atomic predicate as the generation fence (HGET preemptCapable inside the Lua; the flat job field, not a metadata blob — the in-memory store reads the same field). The handler's pre-check is deleted rather than kept alongside; the store predicate is the single source. New handler test rewrites the capability after queueing and expects PREEMPT_UNSUPPORTED with the item left unflagged; the Redis guards test now asserts the incapable refusal against real Redis. P2, ambiguous toast. armed:false covers injected, cancelled, re-homed, and run-over alike, so telling the user the message "already reached the agent" claimed one specific outcome. The lost-race branch now uses a neutral message (com_ui_steer_arm_lost_race) and defers to the events for what actually happened. * fix: flip the escalation lock synchronously before the arm request Codex round 6, one P2. Round 4 deleted the escalating flag along with the reclaim continuation it guarded, but that left the one-interrupt gate blind during the arm request's own round trip: the chip-derived check cannot see an arm until its response relabels the chip, so on a slow connection two bubbles could both arm before either response landed. Double-arm is harmless server-side now (the run seals once and drains the whole queue in order), but every escalation control advertises "one interrupt at a time" by disabling, and the controls must tell the truth. The per-conversation escalating flag returns as a pure UX gate: set synchronously at click, before the mutation, cleared on settlement, and folded into interruptPending on both surfaces. Unlike its round 1-3 ancestor there is no continuation behind it to guard and no recheck to pair with it. Counterfactually verified: without the synchronous set, the two-bubble race test arms twice. 207 tests green across the Chat Input suites. * test(e2e): cover escalation of waiting messages through the real seal Three mock-harness tests on E2E_SLOW_REPLY, a 160-chunk stream with no tool boundary, so an in-thread steer part can ONLY come from a genuine mid-stream seal — which makes each test a behavioral proof rather than a UI check: - Queued row escalation: the ZapOff button turns a waiting queued message into a preempt-armed steer (202 echoes preempt: true) that seals and injects, where the sibling steering.spec test proves the unescalated path waits for run end instead. - Bubble in-place arm: an ordinary steer (202 with no preempt echo) waits as a bubble, POST /chat/steer/arm answers armed: true, the bubble relabels in place (same single bubble, same text, escalation no longer offered on reopen), and the armed steer seals mid-stream. - Always-interrupt toggle: flipped from a waiting row's overflow menu, plain Enter now produces a preempt: true steer that seals in the SAME run, and the menu offers the way back. An afterEach clears the localStorage preference so a mid-test failure cannot leak preempt-by-default into the rest of the serial suite. All three verified locally through the full harness (real backend, mock LLM, seeded DB): 3 passed in 27s. * feat: dedicated escalation arrow + shortcut, menu split into actions and preferences The escalation was still half-hidden: the bubble only offered it inside the overflow menu, and the tooltip taught the composer chord, which does a different thing (interrupts with typed text, not this chip). Three changes make it a first-class command: - A shared EscalateNowButton (circular arrow, ghost-bordered like the composer's interrupt control) is always visible on BOTH surfaces: beside each queued row's Steer primary and on every waiting steer bubble next to its menu. It disappears once a steer is interrupting. - A dedicated registry shortcut, escalateSteer (Cmd/Ctrl+Shift+.), editing-allowed and rebindable like every other action. Deliberately NOT an Enter chord: the composer owns every Enter chord, and the yield design rests on no default binding using Enter besides submit. Its handler clicks the newest enabled arrow control (bubbles beat queued rows), so the shortcut can never diverge from the button, and the arrow's tooltip teaches THIS command via the registry display. - The overflow menus separate one-off actions from sticky behavior changes: Edit, Cancel, Queue, then a smaller "Preferences" section holding the queueing and always-interrupt toggles, each with the standard InfoHoverCard reusing the Settings panel's descriptions. "Interrupt & steer now" leaves the menu entirely. 386 client tests green, including a menu-structure test locking the order and the absence of the escalation entry; bubble escalation tests drive the visible arrow. The e2e spec's bubble test now clicks the arrow, and a fourth test drives the dedicated shortcut end to end through a real mid-stream seal. * style: bind the escalation arrow to its message (variant A anatomy) Two same-weight circles in a row read as one control group, leaving the arrow's ownership ambiguous, and a floating arrow stops meaning anything once several messages stack. The shared control now carries variant A's anatomy: a thin divider binds a small SOLID arrow (filled, inverted) to the message region on its left, and the menu ellipsis stays a bare glyph, so the two affordances can no longer blur together — and the divider+arrow pairing repeats cleanly per chip at N messages. * chore: drop the unused within import CI lint caught * fix: advertise the escalation shortcut only while the control is live Codex on the e2e head, one P2: the tooltip appended the chord hint even while the button was disabled, advertising a shortcut that does nothing during an approval pause. The flagged control (InterruptNowButton) was since replaced by the shared EscalateNowButton, which inherited the pattern; the successor now omits the chord whenever the control is disabled, matching the rule the during-run hovercard already follows. * fix: harden steer escalation lifecycle and recovery * test(e2e): disambiguate accessible steer preferences * test: align abort persistence coverage with prerequisites * chore(i18n): remove obsolete steer race message * chore: normalize imports across steering changes * test: exercise stream integration on Redis Cluster * test: scope HITL checkpoints to generation * test: fix cluster cleanup and locale policy * fix: keep escalation visible during ask pauses * fix: fence recovery downgrade and stale predecessors * fix: require generation owner abort acknowledgement * fix: validate delayed preempt arms * test: align final escalation fixtures * fix: preserve in-memory predecessor abort handoff * fix: restore controls for recovered queued messages * test: cover recovered queue controls * fix: close final steering review gaps	2026-07-31 20:07:56 -04:00
Danny Avila	9618be6eb3	🌿 fix: Preserve Viewed Branch on Sibling-Tree Churn (#13732) ... * 🌿 fix: Preserve Viewed Branch on Sibling-Tree Churn Regenerating a message could snap the view to an unrelated newest branch. MultiMessage reset siblingIdx to 0 (newest) on any messagesTree.length change, but getRegenerateSubmissionMessages slices the flat message array during a regenerate — the streaming handlers render a tree missing unrelated sibling branches, then finalHandler restores the full set. That 2→1→2 child-count swing snapped unrelated forks to their newest sibling, so regenerating the latest response on an older branch jumped to a previously regenerated branch. Replace the indiscriminate reset with per-fork branch memory: a 'seen' set distinguishes a genuinely new sibling (submission/regeneration/edit here — focus it) from one transiently dropped and restored (preserve the user's branch). Decision extracted as the pure, unit-tested resolveSiblingSelection. - client/src/utils/messages.ts: resolveSiblingSelection + tests - MultiMessage: seen/selectedId refs, structural id-signature effect - e2e: regenerate-latest-on-older-branch keeps the viewed branch (fails on the old reset, passes now) * 🧪 test: Long-Thread Branch Preservation E2E Add the user-reported scenario: in a multi-turn thread, regenerate an earlier response (forking a root branch), switch back to the original, then regenerate a later response on it — the original branch must stay intact. Uses labeled prompts so each turn's unique reply is a reliable settle signal. Verified it fails on the original MultiMessage and passes with the fix. * 🎨 style: Fix import order in MultiMessage (react before recoil) * 🌿 fix: Keep Unrelated Branches in Regenerate Optimistic Render Regenerating a message used a flat `messages.slice(0, targetIndex)` for the optimistic render, which also drops unrelated sibling branches that merely sit later in the flat array. Mid-regenerate the thread briefly collapsed to a short branch (visible flash) and the scroll jumped to the shrunken content and didn't recover — the same flat-array root cause as the branch-reset bug. Remove only the regenerated response and its descendants, keeping unrelated branches. The thread (and scroll) stay put through the regenerate. This array is render-only — the server regenerates from parentMessageId and createPayload doesn't include it — so summing by subtree never affects the request. Verified via a small-viewport scroll trace: old collapses 903->295px / 8->2 renders mid-stream; fixed stays 903px / 8 renders, scroll held at bottom. Unit test covers the keep-unrelated-branches behavior (fails on the old slice). * 🌿 fix: Let an Explicit Branch Selection Survive Streaming ID Churn resolveSiblingSelection focused any unseen sibling id before checking the committed selection. When an in-flight response's id is replaced mid-stream (placeholder → server/run id, e.g. useStepHandler re-keys to runId) after the user switched to a different sibling, that swap looked like a brand-new sibling and stole focus back to the streaming branch. Reorder: the committed selection wins while still present; only focus a fresh sibling when the selection is gone (regenerated away, or its own placeholder id was just replaced — that's how a regen/edit still takes focus, since the slice removes the old response). Added unit tests for both churn directions. * 🌿 fix: Only Focus a New Sibling When the Fork Actually Grew The previous churn fix (selection-wins-first) was too aggressive: a genuinely new sibling ADDED while the prior selection is still present — e.g. a follow-up re-parented as a sibling after a generation-start failure — was no longer focused, so its reply never rendered (broke message-tree generation-start recovery e2e). Gate new-sibling focus on actual growth: resolveSiblingSelection now takes prevCount and only focuses a never-seen id when ids.length > prevCount. A same-count placeholder→server id swap (churn) or a restored already-seen sibling is not growth, so the committed selection still wins there. Covers follow-up/new-branch focus, churn steal-prevention, and self-churn follow. message-tree + chat e2e: 17 passed (incl. the recovered generation-start test). * 🌿 refactor: Drop MultiMessage Branch-Memory in Favor of the Slice Fix The regenerate-slice fix (keep unrelated branches in the optimistic render) is the true root cause: with no spurious tree collapse, the original setSiblingIdx(0)-on-length-change never misfires, so the branch-reset is fixed without per-fork memory. The earlier MultiMessage rewrite (seen/selectedId/ prevCount + resolveSiblingSelection) was a symptom patch added before the root cause was found, and its per-instance memory generated two edge-case findings (placeholder→server id churn; divergence from external siblingIdx writes like resume restore). Revert MultiMessage to the simple upstream version and remove resolveSiblingSelection (+ its tests). The slice fix + the existing branch e2e (chat.spec: switch-back, regenerate-latest, long-thread) cover the behavior; all 17 chat + message-tree branch specs pass with this version. * 🌿 fix: Focus the Regenerated Response When Its Fork Count Is Unchanged When a parent already has multiple sibling responses and the user switches to a non-latest one and regenerates it, the optimistic slice drops the target but keeps the other siblings, so the child count is unchanged. MultiMessage only resets the (reversed) sibling index on a length change, so the stale index kept pointing at the kept sibling and the regenerating response stayed hidden until the server restored the dropped sibling at finalize (count bump → reset). Explicitly focus the newest sibling (reversed index 0 = the appended response) of the regenerated fork in createdHandler. Position-based, fires only on the regenerate action, so it doesn't reintroduce the placeholder→server id churn or external-write fragility that a per-render selection memory had. E2E: new during-stream test (slow+counted reply marker) asserting the regenerating response is visible before finalize; negatively verified (fails without the focus call, passes with it). * 🌿 fix: Eliminate Pre-Created Flash by Focusing at the Optimistic Render The createdHandler focus removed the until-finalize bug, but a brief flash remained between clicking regenerate and the `created` event: useChatFunctions renders the optimistic placeholder first, and that render has the same unchanged-count problem, so the kept sibling showed until createdHandler fired. Extract the focus into a shared useFocusRegeneratedResponse hook and apply it at the optimistic render too (useChatFunctions) and on `created` (useEventHandlers). The placeholder is now focused from the first frame. E2E: gated pre-created test — holds the SSE stream GET (the chat POST returns a stream id; the stream is a separate GET) so `created` cannot arrive, leaving only the optimistic render, then asserts the kept sibling is already gone. This isolates the optimistic focus (createdHandler cannot mask it); negatively verified (fails without the optimistic focus call). * 🧪 test: Extend Store Mock for the Regenerate Focus Hook useChatFunctions.regenerate.spec.tsx mocks ~/store and recoil partially; the new useFocusRegeneratedResponse calls store.messagesSiblingIdxFamily via a recoil `set`, neither of which the mock provided (TypeError on regenerate). Add messagesSiblingIdxFamily to the store mock and `set` to the useRecoilCallback mock. Test-only; production code unchanged.	2026-06-14 09:38:06 -04:00
Danny Avila	fd4728232c	🧵 fix: Reject Preliminary Parent Follow-Ups (#13619) ... * fix: Reject preliminary parent follow-ups * chore: Sort frontend imports * fix: Narrow preliminary parent detection * fix: Preserve refused submit state * fix: Propagate refused submit result	2026-06-09 12:06:51 -04:00
Danny Avila	3f4001fca1	🌿 fix: Anchor Post-Auth MCP Stream to Submission Message Tree (#13582) ... * fix: Preserve MCP OAuth post-auth message tree * fix: Hydrate MCP OAuth replay after created event * fix: Satisfy OAuth replay typecheck	2026-06-07 22:30:35 -04:00
Danny Avila	cb1d536874	📻 fix: Replay MCP OAuth Prompts for Coalesced Connections (#13565) ... * fix: Replay MCP OAuth URL for Joined Connections * chore: Sort MCP OAuth Imports * test: Restore MCP OAuth Registry Spies * fix: Replay pending MCP OAuth prompts * fix: Replay MCP OAuth on Stream Resume * fix: Preserve MCP OAuth Replay Context * chore: Format MCP OAuth Replay Context * test: Expect MCP OAuth Replay Expiry * fix: Render pending MCP OAuth prompts * chore: Clean MCP OAuth Replay Type Narrowing * fix: Stabilize new MCP OAuth chats * fix: Re-emit cached MCP OAuth prompts * fix: Replay pending OAuth for selected MCP tools * fix: Avoid stalling pending MCP OAuth replay * test: Clean MCP OAuth review findings * test: Restore MCP OAuth registry spy * fix: Resolve OAuth Typecheck Regressions * fix: Harden MCP OAuth replay edge cases * test: Cover MCP OAuth joined prompt expiry * test: Mark joined OAuth replay fixture * test: Use OAuth fixture for joined replay expiry * fix: Anchor resumed MCP OAuth prompts * fix: Seed resumable turn metadata before MCP init * test: Format resume metadata regression * fix: Prioritize resumable stream routes * fix: Preserve MCP OAuth resume message tree * test: Fix MCP OAuth Resume Test Types * fix: Replay MCP OAuth Regenerate Prompts * fix: Skip OAuth-only Abort Persistence * fix: Stabilize OAuth Resume Replay * fix: Target Non-Tail Regenerate Responses * fix: Scope Regenerate Step Updates * fix: Clean Up OAuth Abort State * fix: Preserve Regenerate Branch Siblings * fix: Preserve OAuth Resume Branch State * fix: Preserve OAuth Branch Resume State * chore: Sort OAuth Resume Imports * fix: Address OAuth Resume Review Findings * test: Fix Abort Fixture Typing	2026-06-07 10:45:54 -04:00
Danny Avila	15108f0f2f	🌲 test: Add E2E Coverage for Message Tree Streaming (#13570) ... * add e2e message tree stream coverage * fix e2e message tree review findings * expand message tree e2e recovery coverage * fix stream-start failure recovery coverage	2026-06-07 09:21:27 -04:00