fix(chat): dispatch orchestrator plan follow-ups from grid panes

[dwu96]

Problem / Motivation

deriveFollowUpOptions returns { followUpOptions, followUpIsPlan } on every branch. ChatPage reads the flag and dispatches plan follow-ups (Go / Go All / Cancel) to POST /api/chat/slots/{slot}/plan-action. ChatPane dropped the flag — it destructured { followUpOptions } only, with zero planAction references — so a plan chip in a grid pane fell through to the generic composer-append path.

Corrections to the issue as filed, plus one to the initial triage of this PR itself:

1. "Cannot dispatch" understates the symptom. The chip was not inert: the click typed the literal label into the composer — one Enter away from sending it to the agent as an ordinary chat message. A control that silently does the wrong thing, not a dead button.
2. The prior needs-investigation parking reason is obsolete. It was parked because the chips were not on main; fix(chat): pass derived follow-up options into ChatPane's composer (#5870) #5895 merged 2026-08-26T04:33Z and put them there (its own body names this exact delta as "Tracked as follow-up ChatPane renders plan-approval follow-up chips but cannot dispatch plan actions #5893"). The defect is live.
3. SideChat is deliberately NOT patched, and the first revision of this PR (which patched it) was wrong to. SideChat's chips derive from the side transcript — a separate agent session whose prompt forbids continuing the main thread's in-flight work — so a plan can never legitimately originate there. Because the side prompt embeds the parent transcript verbatim, a side answer quoting the plan is a reachable false positive: a plan branch in SideChat would dispatch side-agent output against the parent session's plan (clicking a quoted Cancel would stop the parent's orchestrator tracker and cancel its running subagents from a panel documented as read-only background). Both adversarial review lanes flagged this independently; the branch was removed. Net scope: ChatPane + the shared hook + ChatPage consuming it.

Scope decision: parity, not suppression

The issue offered two designs: port plan-dispatch into the pane, or suppress plan chips in panes with a pointer to main chat. This PR takes the port, deliberately: the same control must not mean two different things depending on which surface renders it; suppression trades a misrouting bug for a discoverability one; and plan-action is already slot-scoped — which every pane has.

What changed

• website/src/hooks/usePlanActionMutation.ts (new): the plan-action mutation, lifted from ChatPage's inline copy so both hosts share one convention. Review-hardened beyond the lift:
  • isPlanAction allowlist — the endpoint accepts only go / go all / cancel (case-normalized server-side), and the plan pipeline normalizes every real plan footer to [OPTION: Go | Go All | Cancel]. Hosts gate on it so a plan-shaped message with non-protocol labels (an agent quoting a plan while offering its own choices) keeps the composer path instead of firing a dispatch the server would 400 — which would also skip the append, leaving a dead chip.
  • Per-slot single-flight per action class, anchored to the ROW it acted on and held until transcript acknowledgement — mutation.isPending is a render snapshot and one session can occupy two grid panes, so the latches are module-level and synchronous. Lifecycle (converged over two adversarial-review rounds): a FAILED dispatch releases its class immediately for retry; a SUCCESSFUL one stores the identity of the options-bearing transcript row (followUpSourceKey, new field on deriveFollowUpOptions: the row's mid/ts) and stays held until the hook observes a different non-null row — never a host remount, a slot re-entry, or option-LABEL equality. That closes every stale-chip path: an HTTP 200 with the WS down leaves the chips stale and the latch held (a re-click would queue_append an unintended extra Go); a pane remount on a warm staleTime: Infinity cache re-derives the SAME row and releases nothing; and a single-write reconnect hydration whose stage-2 footer has byte-identical labels still releases, because the new row's identity differs. The acknowledgement lives INSIDE the hook — hosts pass (slot, followUpSourceKey) as arguments, so wiring it cannot be forgotten (a missing argument is a type error). A duplicate Go/Go All is dropped over the whole window; a re-Cancel likewise; Cancel is never blocked by a pending Go. The hosts' own render-scoped isPending pre-checks are removed for the same reason.
• ChatPane.tsx: destructures followUpIsPlan from the existing memo; plan guard prepended ahead of quick-send/toggle with ChatPage's ordering and isPending check; dispatches against the pane's own slotKey, gated on the slot record's mode (the same source ChatPage's effectiveMode resolves from). Additionally no-ops a plan click while the slot record is unresolved: on a reload with a restored grid the pane hydrates its transcript before the first WS slots snapshot (the store deliberately refuses pre-slotsLoaded empty frames), so the mode is unknown in that window — dispatching is unsafe and appending re-creates the reported bug. Deletes the stale "panes have no orchestrator plan mutation" comment.
• ChatPage.tsx: consumes the shared hook and the same isPlanAction gate (non-protocol labels now append instead of silently 400ing — previously unreachable on the main slot thanks to the footer normalization, but now uniformly guarded).

Deliberate non-change, named so it reads as a decision: the chips' double-click-to-send path (onFollowUpSend) sends the raw label as a chat message on all hosts including ChatPage — pre-existing on the reference surface, untouched here.

Tests

ChatPane.followUpOptions.test.tsx (+10), ChatPage.followUpToggle.test.tsx (+2 — the dispatch and the allowlist pinned on the MAIN surface too), and usePlanActionMutation.test.ts (isPlanAction unit, mirroring the server's .strip().lower()), all against the real protocol labels:

• a plan chip dispatches (api.planAction called with the pane's own slot and Go) and does NOT append to the composer;
• a non-plan chip still toggles/appends exactly as before;
• a plan-shaped chip outside orchestrator mode falls through to the composer;
• a plan-shaped message with non-protocol labels keeps the composer path (allowlist);
• a plan chip is a no-op while the slot record is unresolved (never appends an approval label);
• re-entrancy across renders: a second click while pending does not fire twice;
• same-tick latch: Go + Go All landing in one tick (both debounce timers advanced inside one act, no render in between) dispatch once — only the synchronous latch can stop the second;
• Cancel is never swallowed: Cancel goes through while a Go is still in flight; a double-Cancel dedupes; a failed dispatch releases only its own class (retry works, the pending Go stays latched); a Go superseded by a Cancel still runs its error release (pinned rather than assumed from query-core internals); a successful dispatch stays latched over the stale-chip window and frees on transcript acknowledgement; an identical next-stage footer in a single hydration write still releases (row identity, not labels); a pane remount on a latched slot with a warm cache does NOT release (same stale row);
• latch release: after a dispatch settles, a later stage can be approved again — the half of single-flight whose regression is a chip dead for the process lifetime;
• slot isolation: two live panes, the dispatch from pane B carries pane B's slot.

Mutation-checked (25/25 killed across six rounds; one documented survivor — the null-never-releases guard is defense-in-depth whose bypass requires a live stream, which contradicts the stale-chip premise): guard disabled / always-on / wrong slot / allowlist dropped (each host) / unresolved-slot fall-through restored / latch removed / latch-release (onSettled) dropped / cancel-bypass removed / isPlanAction normalization dropped — each confirmed red, then restored.

Verification

• Full website suite: 1553 files, 24,332 passed, 0 failed; electron suite 1332 passed, 0 failed; eslint zero new findings; tsc -b clean.
• Backend untouched; the change-scoped local gate's 366 cross-surface backend guard files run on this branch vs an origin/main worktree produced identical failure-id sets (one delta reproduced on main with the same node_modules present — host Node version, environmental).
• black gate green (black==26.3.1, 0 python files in scope).
• Two adversarial review lanes (gpt-5.6-sol, claude-opus-5) ran pre-push; every finding was verified against source and addressed (SideChat removal, allowlist, synchronous latch, unresolved-slot no-op, protocol-label fixtures) — disposition details in the PR comment below.

Screenshots / video

Evidence harness website/scripts/capture-chatpane-plan-dispatch.mjs runs the real built SPA with stubbed /api, seeds a split view whose first pane is an orchestrator session mid-plan (footer exactly [OPTION: Go | Go All | Cancel]), clicks Go, and asserts the wire behaviour before shooting — pre-fix build (true origin/main dist): zero plan POSTs and the composer contains Go; fixed build: exactly one POST /api/chat/slots/chat-orchestrator/plan-action with {"action":"Go"} and an empty composer.

Before (origin/main build): the click types the label into the pane's composer:

After (this PR): the click dispatches the plan action; composer stays empty:

Plan chips rendering in the pane pre-click

Related Issues

Fixes #5893

The pre-existing server-side cancel-before-tracker race surfaced during review is tracked in #6046 (not introduced here; the client behaviour is correct given the server contract).

Checklist

• At most two commits, Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — N/A, behaviour mirrors the documented ChatPage surface
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of bac4a603a9ec4d633de7ebb72e4549b9cabf6a9d — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The repo has an established temp-screenshots/ convention, so no hygiene finding there. I've reviewed the diff against the description; the fidelity is high and the design decisions are explicitly argued. One design trade-off deserves human eyes.

Design-Verdict: CONCERNS

Sound parity port at the right layer; but the fail-closed latch turns any ambiguous failure into a silently dead chip — including Cancel, the stop control.

Watch

• The latch's "held until a DIFFERENT row appears" release has no path out when the dispatch itself failed: a 5xx that did not commit means the plan never advances, so no new row ever arrives and the chip is wedged until reload — with zero affordance (deferred to Plan chips give no pending/failed affordance on dispatch (all chip surfaces) #6056). For Go that is the safe direction; for Cancel it inverts fail-safe: the user's abort silently no-ops to prevent a cosmetic duplicate "Plan cancelled" row. Consider releasing the Cancel latch on any settle (duplicate row is strictly cheaper than a swallowed stop), or land Plan chips give no pending/failed affordance on dispatch (all chip surfaces) #6056 before this ships to real users.
• Two module-level Maps are now load-bearing, invisible singleton state for every chip surface; anyone adding a third host or a new action class must know the latch taxonomy lives in usePlanActionMutation.ts, not in the hosts. Fine today, but it is the piece that will be misused first.

[DESIGN-REVIEWED] bac4a60

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of bac4a603a9ec4d633de7ebb72e4549b9cabf6a9d and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] bac4a60

False positive or not applicable? A repository writer can comment:
/ai-review override gpt bac4a603a9ec4d633de7ebb72e4549b9cabf6a9d: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of bac4a603a9ec4d633de7ebb72e4549b9cabf6a9d — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

The fix and its hardening are derived and declared, but the root cause has one uncounted sibling: ChatEmbed drops followUpIsPlan exactly the way ChatPane did.

What this change ships

Intent: FIX — a plan approval chip clicked in a grid pane must dispatch the plan action instead of typing the label into the composer.

1. Plan chips in grid panes dispatch Go/Go All/Cancel to the pane's own slot — the fix; justified.
2. ChatPage's inline plan mutation replaced by shared usePlanActionMutation — justified (removes the divergence that caused ChatPane renders plan-approval follow-up chips but cannot dispatch plan actions #5893).
3. Non-protocol labels on plan-shaped messages now append on BOTH surfaces — rides along; derived (server 400s anything but go/go all/cancel, chat_orchestrator.py:894-896).
4. Main chat: Cancel now goes through while a Go is in flight (isPending pre-check removed) — rides along, declared, derived.
5. Duplicate Go/Cancel clicks dropped until a NEW options row appears (module-level latches) — rides along, declared; derived (queue_append, one session in two panes).
6. A 5xx/transport failure now silently wedges the chip until the next plan row or reload — declared trade-off, deferred to Plan chips give no pending/failed affordance on dispatch (all chip surfaces) #6056.
7. New followUpSourceKey field + rowIdentity — 3 real consumers (both hosts + hook); justified as the latch's ack key.
8. sourceKey click-time snapshot threaded through FollowUpBar/Chip/ChatInput — rides along; guards a 220ms race.
9. Plan click is a silent no-op while the slot record is unresolved — derived (unknown mode; appending re-creates the bug).
10. New capture harness + extracted split-pane-fixture.mjs — derived (jscpd pretest fails duplicated scripts); screenshots follow repo convention (2,510 files under temp-screenshots/).

Watch

• Unfixed sibling. Grepped deriveFollowUpOptions: 4 hosts. ChatPage and ChatPane fixed; SideChat excluded with a derived rationale; app-sdk/ChatEmbed.tsx:95 destructures { followUpOptions } only — the exact pre-fix ChatPane shape, so a plan footer in an embedded orchestrator slot still types "Go" into the draft. The description's "Net scope: ChatPane + the shared hook + ChatPage" never dispositions it.
• The fix for a chip that did the wrong thing ships a new way for a chip to do nothing: an ambiguous failure wedges Cancel — the stop control — until reload, with no affordance (deferred to Plan chips give no pending/failed affordance on dispatch (all chip surfaces) #6056). Declared, but a human should weigh a wedged abort against the duplicate-row it prevents.
• Item 8's forever-cost (a third argument on a shared onSelect signature across four components) buys a window bounded by 220ms and a byte-identical replacement footer; it is derived and tested, but it is the thinnest harm in the set.

[FIRST-PRINCIPLES-REVIEWED] bac4a60

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed bac4a603a9ec4d633de7ebb72e4549b9cabf6a9d — this comment is updated in place on each push.

Review details

The single candidate hinges on a plan/options row falling through rowIdentity to the idx:${i} fallback so a history prepend re-keys it and frees the latch. Verifying (a): the latch is only ever set for an orchestrator plan row (followUpIsPlan && isPlanAction), i.e. an assistant turn. Every such row either arrives from the server with a real ts (m.ts), or, if ts-less, is stamped meta.clientTs by ensureMsgId/mintMsgId on every ingestion and hydration path (chatSlice.ts:47-51, 939, 1023, 1069, 2814, 2886, 4004, 4117, 4171) and carried across reloads by mergePreservedClientTs. So rowIdentity returns clientTs or ts — both prepend-stable — never idx:. The idx: fallback is reachable only for fixture-grade rows and, at most, note rows, which set followUpIsPlan: false and therefore never arm the latch, so freeing it is a no-op. Input (a) does not occur in practice; the candidate concedes as much. Dropped.

No self-originated finding survives the same bar.

No findings.

[OPUS-REVIEWED] bac4a60

Verdict parsed from the review's SHA-scoped output markers for commit bac4a603a9ec4d633de7ebb72e4549b9cabf6a9d.

False positive or not applicable? A repository writer can comment:
/ai-review override fable bac4a603a9ec4d633de7ebb72e4549b9cabf6a9d: <one-sentence reason>

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of bac4a603a9ec4d633de7ebb72e4549b9cabf6a9d — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: CONCERNS

The fix makes the chip keep its promise, but every new dispatch path — success, refusal, and failure — is completely silent, including a wedged Cancel.

Watch

• Plan dispatch renders no state, so all its outcomes look identical to doing nothing. after-2-after-click.png is pixel-identical to the pre-click shot: no pressed/pending mark on the clicked chip, and the hook explicitly "renders no pending or error state" (its own comment). The sharpest instance: an ambiguous failure (5xx/lost response) holds the cancel latch, so every later Cancel click on that slot silently no-ops (console.error only) until a new plan row or a reload — the stop control for an autonomous plan goes dead with no signal to the user watching it run. Frequency low (needs a server fault) × impact high (can't stop, can't tell why) × persists until reload. Smallest fix inside this PR's surface: mark the clicked chip pending/disabled while its latch is held and surface a definitive-rejection error inline, rather than deferring the whole affordance to Plan chips give no pending/failed affordance on dispatch (all chip surfaces) #6056. The same silence covers the unresolved-slot and stale-row refusals — one root cause, one fix point.

[UX-REVIEWED] bac4a60

[dwu96]

🤖 Local adversarial review round 1 (pre-server-lanes) — two pinned lanes, both BLOCK-MERGE; every finding verified against source and dispositioned in head 2caa07ce5:

GPT lane (gpt-5.6-sol):

1. Fixtures/evidence asserted a payload the endpoint rejects (Stage-1-APPROVE vs the go/go all/cancel contract at the plan-action handler) — verified true, fixed: all tests and the evidence harness now use the real protocol labels, and the new isPlanAction allowlist makes a non-protocol dispatch structurally impossible.
2. planActionMutationRef.current.isPending is a render snapshot, not a synchronous latch — verified true, fixed: the shared hook now single-flights per slot with a module-level set (set before mutate, cleared onSettled), covering both the same-tick race and the same-slot-in-two-panes case; pinned by a same-tick Go+Cancel test that advances both debounce timers inside one act.

Opus lane (claude-opus-5):

1. BLOCK/High — SideChat dispatched a side conversation's chips at the parent session's plan (side transcript flag + parent slot mode + parent-scoped endpoint; the side prompt embeds the parent transcript verbatim, so a quoted plan is a reachable false positive; a quoted Cancel would stop the parent's orchestrator tracker) — verified true; the SideChat branch and its test file were removed entirely. Rationale recorded in the PR body (correction 3).
2. No allowlist on the raw label — fixed as above (isPlanAction).
3. Test+evidence chain asserted a non-existent protocol — fixed as above.
4. ChatPane's mode gate had an unresolved-slot window (pane hydrates its transcript before the first WS slots snapshot; a click in that window appended the label — the reported bug, transiently) — verified against dashboardSlice's pre-slotsLoaded frame refusal, fixed: plan clicks are a no-op while paneSlot is undefined, pinned by a test.
5. Per-instance re-entrancy guard doesn't cover one slot in two panes — fixed by the per-slot latch above.
6. Hook docstring claimed an error path no caller implements — fixed: onError logging added, docs rewritten to match behaviour.

Re-verified after the round: typecheck + eslint clean, 1552 test files / 24,305 passed / 0 failed, electron 1332/0, 11/11 mutations killed, evidence regenerated from a true origin/main build vs this branch. Both local lanes re-dispatched on the new head.

[dwu96]

🤖 Local adversarial review round 2 on head 2caa07ce5 — GPT lane PASS (no findings; confirmed React Query invokes hook-level onSettled on success, error, and unmount-mid-flight, so the latch cannot leak). Opus lane PASS with 7 advisories, each dispositioned; adopted items shipped in head 9b4ae5821:

Adopted:

1. (Medium) Nothing pinned that the latch RELEASES — a dropped onSettled would dead-chip every later stage approval for the process lifetime with all tests green. → Added a latch-release test (resolving mock, dispatch, settle, dispatch again → 2 calls). Mutation-verified: deleting the onSettled cleanup now fails it.
2. (Medium) ChatPage's narrowed guard had zero behavioural coverage on the main surface, and isPlanAction had no unit test. → Added 2 ChatPage tests (plan chip dispatches with the active slot; non-protocol label keeps the composer path) and a usePlanActionMutation.test.ts unit suite mirroring the server's .strip().lower() ('GO', ' go all ', 'Go All', 'Approve', …). A future server-side action added without updating the allowlist now fails a main-surface test.
3. (Medium) A Cancel arriving while a Go was in flight was silently swallowed — and round 2 had tested that in. → Cancel now bypasses the latch in both directions (never latched on the way in, never releases another action's latch on the way out); the server's cancel path is re-entrant (if tracker and not tracker.stopped). The hosts' render-snapshot isPending pre-checks were removed — single-flight now lives entirely in the hook's synchronous per-slot latch, which is strictly stronger. Pinned by a Cancel-during-pending test; the same-tick latch test now uses Go+Go All (both stage-advancing).
4. (Low-Medium) mutateAsync was spread through unlatched. → No longer exposed; the docblock names why.
5. (Low/nit) The useMemo never memoized (fresh object per render). → Dropped.

Accepted, not changed (with rationale):
6. (Low) Chips on a pane whose slot record is permanently gone (deleted session, stale grid leaf) are silent no-ops with no aria feedback. Accepted: not a regression (pre-PR the chip typed into a composer that could not send either), and the right fix is disabled-chip affordance across all follow-up surfaces — out of scope for a dispatch-parity PR.
7. (Low) Failed dispatches are console-only; neither host renders mutation errors. Accepted: matches ChatPage's pre-existing convention (steerMutation logs the same way); a toast belongs in a deliberate error-surfacing pass, not riding along here.

Re-verified on 9b4ae5821: typecheck + eslint clean, 1553 files / 24,329 passed / 0 failed, 16/16 mutations killed across three rounds, after-evidence re-captured from the final build (one POST …/plan-action {"action":"Go"}, composer empty). Both local lanes re-dispatched on this head.

[dwu96]

🤖 Local adversarial review round 3 on head 9b4ae5821 — both lanes PASS, five Low advisories between them, all dispositioned in head 5881a6fc8:

Adopted:

1. Double-Cancel became unbounded (the server guards only tracker.stop(); the "🛑 Plan cancelled." append + broadcasts run on every POST, so removing the render-scoped isPending check traded swallowed-Cancel for doubled-Cancel). → Split the latch into two per-slot sets: Go/Go All and Cancel each dedupe against themselves; Cancel is still never blocked by a pending Go, and a settling Cancel cannot release a Go's latch. Docstring no longer overstates server re-entrancy. Pinned by a double-Cancel test, mutation-verified.
2. The onSettled cancel-guard's own direction was untested (deleting it left every test green while a resolved Cancel would free a still-in-flight Go's slot). → Test added: never-resolving Go + resolving Cancel + second Go → exactly 2 dispatches. Mutation-verified (onSettled deleting from both sets fails it).
3. The superseded-Go-still-releases property rested on unpinned query-core internals (an observer moving to a new Mutation must not swallow the old one's onSettled). → Test added: deferred Go, superseding Cancel, resolve the Go, then Go All must dispatch. Mutation-verified (dropping onSettled fails it).
4. Unique-slot-key isolation in the test file was load-bearing and undocumented. → Comment added at the describe head.

Filed, not changed here:
5. Server-side cancel-before-tracker race (a Cancel processed before the first Go's stage loop creates the tracker no-ops on it: transcript says cancelled, plan advances; pre-existing, only newly UI-reachable). → Tracked as #6046 with the suggested slot._plan_cancelled flag fix — a server change that must not ride along on a frontend parity PR.

Accepted: the latch-release flush style (single microtask await under fake timers — fails loud, not false-green) and the no-client-timeout trade-off are now named in the docstring.

Re-verified on 5881a6fc8: typecheck + eslint clean, 1553 files / 24,332 passed / 0 failed, 19/19 mutations killed across four rounds. History squashed to one commit. Both pinned local lanes re-dispatched on this head.

[dwu96]

Round 5 disposition — head dc9e44eeb

1. GPT 5.6 BLOCKING: "Transport failures reopen committed plan actions" — ACCEPTED and applied

The finding is correct and I have no rebuttal. onError released the latch on any rejection, and a rejection is not evidence the server did nothing: the plan-action handler can raise after queue_append has already landed, and a fetch that rejects outright cannot distinguish "never arrived" from "committed, response lost". Either way the retry queued a second Go or wrote a second 🛑 Plan cancelled. row — the exact duplicate-dispatch this hook exists to prevent, re-opened by its own error path.

Classification rule (isDefinitiveRejection, website/src/hooks/usePlanActionMutation.ts):

Failure	Verdict	Why
ApiError 4xx excluding 408/429 (400, 403, 404, 409, …)	RELEASE	A response the server produced declining to act. Nothing was mutated, so retry is safe — and not releasing would leave a dead chip on a recoverable error.
ApiError 5xx	RETAIN	Can be raised after queue_append committed. Not proof of non-mutation.
Non-ApiError (bare TypeError from fetch)	RETAIN	Never reached j(), so there is no status at all. The request may have been fully served with only the response lost.
ApiError 408 / 429	RETAIN	4xx status, ambiguous meaning: 408 is an edge giving up on a request it may already have forwarded, and 429 is the tunnel throttle that api/queryClient.ts itself treats as retryable. A retryable rejection is by definition not proof of non-mutation, so the release cannot key on the 4xx range alone.

No new plumbing: api.planAction already funnels through j(), which throws the exported ApiError carrying .status.

The source-key guard is unchanged and still applies on top. The two conditions are independent and both necessary — the classification narrows which failures qualify, the guard narrows whose latch is freed. The latch structure and the settled success/ack lifecycle are untouched.

2. Trade-off, stated in code

A genuinely lost dispatch now keeps that action class latched until a different plan row arrives (or a reload). On the Go map that is the safe direction; on the Cancel map it wedges the stop control for that slot. The user-visible cost is that the wedged retry has no affordance — the chip silently does nothing, because this hook renders no pending or error state. Recorded in the hook's latch docstring and in the onError comment, both citing #6056, which is the right home for the fix (the shared FollowUpBar, so every chip surface gains it at once). Deliberately not fixed here.

3. Tests — split, red-before, mutants

The old single case a failed dispatch releases its own class for retry — and only its own became four, all in website/src/test/ChatPane.followUpOptions.test.tsx:

Test	Rationale
a DEFINITIVE 4xx rejection releases its own class for retry — and only its own	the original per-class assertions, now rejecting with ApiError(400) — the only release-eligible shape
a 5xx failure KEEPS the latch — the server may have committed and lost the response	ApiError(500); the retry Cancel must be dropped
a TRANSPORT rejection with no response KEEPS the latch	TypeError('Failed to fetch'), the no-status path
a retryable %i KEEPS the latch even though it is a 4xx (it.each([408, 429]), one slot key each)	pins that the release excludes the ambiguous 4xx, so a future refactor to a plain range check fails here

Red-before, proven by stashing only the hook change and re-running: the 4 retain cases fail, AssertionError: expected "vi.fn()" to be called 1 times, but got 2 times; the other 23 pass. With the hook change: 27/27.

Mutants — 3 dispatched, 3 killed, one test each:

Mutant	Killed by
release on 5xx too (drop the < 500 bound)	a 5xx failure KEEPS the latch (1 failed / 26 passed)
release on non-ApiError too	a TRANSPORT rejection with no response KEEPS the latch (1 failed / 26 passed)
release on 429 (drop it from AMBIGUOUS_4XX)	a retryable 429 KEEPS the latch — 408 still passed (1 failed / 26 passed)

Two pre-existing tests were re-typed, not weakened — both rejected with a plain Error, which the new rule correctly retains, so they had to name a release-eligible error to keep testing what they were written to test:

• a Go superseded by a Cancel … still runs its error release → ApiError(409). Verified necessary: left as new Error('502') it fails against the fixed hook (expected 3 times, got 2), because the retain is then doing the work and the test can no longer see whether onError ran at all.
• a LATE failure from an old dispatch does not free a newer dispatch's latch → ApiError(409), i.e. an error that is release-eligible. Otherwise the classifier would hold the latch on its own and the test would pass green without ever exercising the source-key guard it exists to pin.

4. First Principles subtraction: delete releasePlanLatches — ACCEPTED, deleted

Verified honestly, repo-wide (grep -rn releasePlanLatches over .ts/.tsx/.js/.mjs/.md, excluding node_modules): three hits total — the definition, and one import + one beforeEach call in ChatPage.followUpToggle.test.tsx. Zero production consumers. No rebuttal available, so the export is gone.

The escape hatch existed because both plan tests in that file shared the fixed slot key chat-1, so it took the lane's suggested remedy: makeStore / renderPage now take a slot key and the two plan tests dispatch on chat-plan-dispatch and chat-plan-allowlist. Unique keys make the module-latch collision structurally impossible rather than reset per test, which is the stronger version of the same guarantee — and it removes an exported production-facing function whose only purpose was test hygiene.

5. Design lane: file a tracking issue for the onFollowUpSend double-click path — filed as #6240

#6240. FollowUpBar's handleDoubleClick and the visible Send now split-button segment both call onSend?.(…), skipping the onSelect branch where this PR's plan gate lives — so on a plan footer they send the literal label as ordinary chat. Cancel is the sharp edge (not special-cased server-side, so the plan is never stopped); Go/Go All degrade less visibly via chat_handlers.py's typed-approval path, losing Go All's auto-run escalation. Both gestures also bypass the single-flight latches entirely.

Separable because routing onSend through the plan gate is a shape change in the shared chip component affecting every follow-up surface (ChatPage, ChatPane, SideChat, ChatEmbed) — a different decision than this parity fix makes, and not one to ride along on it.

Verification run (docker node:24-bookworm; the host is node 16)

• tsc -b — clean
• eslint on the three touched files — clean (1 pre-existing no-console warning on the unchanged console.error('plan action failed', e))
• vitest run on usePlanActionMutation.test.ts, ChatPane.followUpOptions.test.tsx, ChatPage.followUpToggle.test.tsx — 57/57 passed (ChatPane 23 → 27)
• vitest run src/test/ChatPa (wider blast radius for the shared hook) — 58 files, 565/565 passed
• jscpd . — 0 clones

Not rebased: main is ahead but has zero overlap with this PR's files, behind_by 0.

_{🤖 AI-assisted review response. Amended onto the single commit (PR Hygiene 3-commit cap); force-pushed with a SHA-pinned lease from a1a2ddf1c → dc9e44eeb.}

[dwu96]

Rebase + semantic-merge fix — head aa4c1a73f (was dc9e44eeb), rebased onto current main.

The previous head failed Frontend Lint & Type Check, Build Wheel, both Build Desktop lanes and E2E — none of them caused by this PR's own diff. Root cause was a semantic merge conflict with main: #5737 (737dc0fa5, "let a note carry [OPTIONS:] buttons") added a new early return in deriveFollowUpOptions for the note-carried-options path, while this PR made followUpSourceKey a required field on FollowUpDerivation. The two changes touch different lines, so git merged them silently, but the merge does not typecheck:

src/app-sdk/protocol/options.ts(105,9): error TS2741: Property 'followUpSourceKey' is missing in
type '{ followUpOptions: string[]; followUpIsPlan: false; }' but required in type 'FollowUpDerivation'.

The wheel/desktop/E2E failures were all downstream of that one frontend build.

Resolution. The note branch now returns a row identity like every other options-bearing branch. The identity expression was duplicated in the process, so it is extracted into one rowIdentity(m, i) helper that both the note and assistant branches call — the derivation itself is unchanged (meta.mid ?? ts ?? meta.clientTs ?? idx:i). The FollowUpDerivation doc comment said "the assistant row"; corrected, since a note row can now be the source.

This is not merely a compile fix: a null key on a note row would have read as "no options on offer" while chips were on screen, and two byte-identical cron notes would have been indistinguishable to the latch — the same failure mode the assistant-side key exists to prevent. Note rows only started reaching this derivation when #5737 landed, so nothing pinned their key.

Verification (docker node:24-bookworm; host node is 16):

• 2 new tests in deriveFollowUpOptions.test.ts — a note row gets a non-null identity, and two byte-identical notes key apart. Both red before the resolution (reverting only the note-branch return fails exactly those 2, other 30 pass), green after.
• tsc -b clean. eslint clean on all three touched files (one pre-existing no-console warning on an unchanged line).
• vitest 129/129 across deriveFollowUpOptions, usePlanActionMutation, ChatPane.followUpOptions, ChatPage.followUpToggle, AppSdkMessageRenderersCov80.

Still one commit; no behavioural change to the transport-classification fix dispositioned above.

[dwu96]

Round 6 disposition — debounce/stale-click finding ACCEPTED and applied

usePlanActionMutation.ts — "Debounced click can approve a newer plan stage". Verified against source, not rebutted. Head is now 917f05845.

The mechanism

Chip in FollowUpBar.tsx debounces a single click by FOLLOWUP_CHIP_DEBOUNCE_MS (220ms) — that timer is what lets a double-click cancel the pending select — and calls onSelect from inside the setTimeout. A byte-identical replacement footer (Go | Go All | Cancel again) keys the same chips (key={o}), so React reuses the element: no remount, no cleanup, the timer survives. mutate then read sourceKeyRef.current after the row advanced.

Neither existing guard covered it:

• the acknowledgement effect had already seen the new row and freed the latch for it, so the single-flight let the click through;
• a live options row was on screen, so the source === null refusal did not apply.

Net effect: one click on a footer the user was looking at approved the stage that had silently replaced it.

The design (as prescribed)

1. FollowUpBar gains an optional sourceKey?: string | null. Chip snapshots it at click time, next to the existing shiftKey/detail snapshot, and passes it to onSelect as an optional third argument — read through the render closure deliberately, because a ref would be re-read when the timer fires, which is the bug itself.
2. ChatInput forwards it (followUpSourceKey → sourceKey); both hosts pass their derived key: ChatPane.tsx and ChatPage.tsx.
3. mutate takes clickedSourceKey and refuses a supplied key that differs from the current row — before touching the latch, so a stale click cannot consume the new row's single-flight slot either.
4. The non-debounced instant-send path is unchanged: it calls onSelect synchronously from the click, so there is no window for the row to advance and nothing to compare.

Every existing guard is intact — the source === null refusal, the per-slot per-action-class single-flight, and the onError isDefinitiveRejection classification with its own source-key guard. The latch and acknowledgement/release lifecycle is not restructured; the five earlier rounds' shape stands.

Why no existing caller breaks

FollowUpBar is rendered in exactly one place (ChatInput.tsx); onSelect/onFollowUpSelect is supplied only by ChatPane and ChatPage (plus tests). The third parameter and the sourceKey prop are both optional, so every caller still typechecks (npx tsc -b clean), and a caller that supplies no key keeps its previous behaviour exactly: undefined is treated as not supplied, never as a mismatch. That asymmetry is deliberate — refusing an unsupplied key would silently disable plan dispatch for any chip surface not yet wired, which is a worse failure than the race being closed. null is a supplied key (chips derived from no row) and cannot match a live row.

Tests — 5 added, each red before the source change

Red-before proven by stashing only the five source files and re-running: 5 failures.

Test	Why
FollowUpBar — hands onSelect the sourceKey from CLICK time after the row advances mid-debounce	The load-bearing one. Click, change sourceKey without remounting (same options → same chip keys), let 220ms elapse, assert onSelect got the original key. Also asserts the button node is the same element after the rerender, so the no-remount premise the whole race rests on is pinned rather than assumed.
FollowUpBar — hands onSelect the current sourceKey when the row does not change	The unchanged-row control: the snapshot must not become stale on its own.
hook — refuses a click whose captured row has since been replaced, without consuming the latch	The rejection itself, plus that the refusal leaves the single-flight untouched (a following click from the current row still dispatches).
hook — dispatches a click whose captured row is still the current one	Positive control: matching key is not collateral damage.
hook — dispatches a click that supplies NO row key at all	Back-compat: the unwired path is not refused.
ChatPane — a click whose footer is REPLACED during the 220ms debounce never dispatches	End-to-end proof of the wiring, not just the hook: bar → ChatInput → pane → mutate. Any missing link and the pending timer dispatches against the new row. Also asserts the composer stays empty (a refused plan click must not fall through to the append path — that is #5893 itself) and that the new row is not wedged.

Two of these (the matching-key and no-key controls) assert unchanged behaviour and so are green before the change by construction; they earn their place as the detectors for mutant (c) below. The other four are red before, green after.

One existing assertion was updated: debounces single click 220ms before calling onSelect asserted the exact call shape ('Ship it', Any<Object>) and now reads ('Ship it', Any<Object>, undefined) — the third arg is undefined because that caller supplies no sourceKey. Behaviour is unchanged; the assertion is strict on argument count.

Mutants — 3 planted, 3 killed

Mutant	Killed by
(a) read the key at timer-fire instead of click time (via a ref)	FollowUpBar click-time test and the ChatPane end-to-end test
(b) drop the mismatch rejection in mutate	hook refuses-a-replaced-row test and the ChatPane end-to-end test
(c) also reject when no key is supplied (clickedSourceKey !== source)	hook no-row-key back-compat test

Gates (docker node:24-bookworm)

• npx tsc -b — clean
• npx eslint on all 8 touched files — 0 errors; 15 warnings, all pre-existing (no-console, react-hooks/exhaustive-deps), none on a touched line
• npx vitest run over the follow-up/plan surface plus a wider ChatPa* sweep (the bar is shared): 83 files, 1097 tests, all pass
• npx jscpd . — 0 clones

Note

This is the sixth validated finding on the plan-dispatch span, and the sixth accepted rather than rebutted: render-snapshot isPending → per-slot synchronous latch → two-class (Go vs Cancel) latch → held-until-transcript-acknowledgement → anchored to the acted-on row identity → and now the click-time capture of that identity, because the row can move between the click and the dispatch. Each round has been a real, verified edge of the same stateful machine, and each fix has been the reviewer's own named design. The residual affordance gap is unchanged and still tracked: a refused or wedged click renders nothing (#6056), and the double-click / Send-now segment still bypasses the plan gate entirely (#6240).

_{🤖 AI-assisted review disposition.}

[dwu96]

Round-7 disposition — ACCEPTED and applied. Head bac4a603a (was 917f05845).

The lane asked to prefer meta.clientTs before meta.mid in rowIdentity. Accepted, and the deciding evidence is that the codebase already had this convention and my helper contradicted it. From website/src/store/chatSlice.ts (~line 1892), which exists solely to carry clientTs onto a reloaded server copy:

the reloaded server copy has an authoritative ts but NO clientTs. The renderer keys virtual rows by clientTs ?? ts, so without this the row's key flips bornKey → serverTs

So the store's row key is clientTs ?? ts, and there is dedicated machinery keeping clientTs stable precisely so a row cannot re-key when it is enriched. rowIdentity checked mid first, inventing a second, conflicting identity — and a reconnect refresh preserves clientTs while adding mid, so the same row re-keyed mid-flight, the acknowledgement effect read it as a different row and freed the duplicate-action latch, and a stale second click could queue an unintended extra Go. New order: clientTs ?? mid ?? ts ?? idx:i.

Worth recording: this is the same defect the Opus lane flagged as F1 (Critical) several rounds ago — followUpSourceKey is not stable for one row across hydrations — which was noted and left undispositioned. Two independent lanes converged on it, and it is now closed rather than deferred.

Verification (docker node:24-bookworm):

• 3 new tests in deriveFollowUpOptions.test.ts: a reconnect refresh that adds mid + ts to a clientTs row keeps ONE identity (born-7); mid then ts still serve as fallbacks for a row that never carried a client stamp; two different client-stamped rows still key apart, so the stability fix did not become a collision.
• Red before, with only the ordering reverted to mid-first: exactly the hydration test fails (1 failed | 34 passed) — the reviewer's scenario reproduced, then closed.
• tsc -b clean, eslint clean on both touched files, wider sweep 1100 tests across 83 files all pass (ChatPa* / FollowUp* / ChatInput / deriveFollowUpOptions / usePlanActionMutation).

Still one commit. Process note: this is the seventh validated finding on the plan-dispatch span. Every one has been mechanically real and none has inverted an earlier demand, so I applied it rather than escalating — but the span has now absorbed seven rounds, and a maintainer may reasonably want to draw a line even on legitimate findings.

[dwu96]

Advisory disposition — all three CONCERNS lanes on bac4a603a. Both blocking lanes (GPT 5.6, Opus 4.8) are ✅ on this head; all 62 checks pass; PR Readiness success; zero unresolved threads.

The three advisory lanes converge on one substantive concern, so I'll take it first.

1. The wedged Cancel — raised independently by Design, First Principles and UX. NOT changed, and deliberately surfaced for a human.

All three make the same argument and it is correct on its own terms: fail-closed is the right direction for Go (a duplicate advances a stage) but it inverts fail-safe for Cancel, where the duplicate is a cosmetic second "Plan cancelled" row and the swallowed click is a dead stop control on a running plan. Design's proposed remedy — release the Cancel latch on any settle — is a small, well-scoped change.

I am not applying it, for a reason the lanes cannot see from their own vantage point: it directly reverses this PR's round-5 blocking demand from the GPT lane, which named this exact behaviour as the defect —

Server commits action → response is lost → fetch rejects and clears latch → retry queues an extra Go or duplicates cancellation.

So the two review families disagree on which harm dominates for Cancel: GPT treats duplicated cancellation as corruption; Design/FP/UX treat a swallowed abort as worse. Both positions are defensible and the choice is a product judgement about which failure a user would rather have, not something I can settle by reading code — and flipping it unilaterally would re-open the review cycle this PR has already spent seven rounds in. Recommendation: make Cancel release on any settle (I agree a duplicate cancel row is cheaper than a dead stop button), but it needs a maintainer to overrule the round-5 verdict rather than me.

UX's "smallest fix inside this PR's surface" — mark the clicked chip pending/disabled and surface definitive rejections inline — is the right fix and I agree it has one root cause and one fix point. It is also a new rendering surface across every chip host, i.e. a scope extension beyond this PR's parity fix, which is exactly why it is #6056. Landing #6056 before this reaches real users is a reasonable release gate and I'd support sequencing it that way.

2. First Principles — "unfixed sibling: ChatEmbed.tsx:95". Correct, and already tracked.

The lane is right that the PR description never dispositions it. It is #6057 (open), filed from an earlier round of this same PR: ChatEmbed destructures { followUpOptions } only — the exact pre-fix ChatPane shape — so a plan footer in an embedded orchestrator slot still types the label into the draft. It is out of scope here because ChatEmbed has no dashboard slot record and no mode source in scope, so the fix needs a mode source it does not currently have; that is a design step, not a port of this diff. The open triage question on #6057 is whether an embed slot can actually be an orchestrator session — if it cannot, it closes as unreachable.

3. Design — "two module-level Maps are invisible singleton state; the piece that will be misused first."

Agreed, no code change. The latch taxonomy is documented at length in usePlanActionMutation.ts and the export that let hosts reach past it (releasePlanLatches) was deleted earlier in this PR precisely so the module is the only owner. A fourth host adding a new action class must go through latchFor, which is where the taxonomy lives. If that proves too subtle in practice, the durable fix is the server-side idempotent (slot, source) dedupe discussed in round 5 — which would make the client latch best-effort and retire this whole class.

4. First Principles — "item 8's forever-cost buys a 220ms window."

Accepted as stated, and it is the thinnest harm in the set. Worth noting the window is not merely 220ms of wall clock: it requires a byte-identical replacement footer arriving inside it, which is precisely the orchestrator's stage-to-stage case ([OPTION: Go | Go All | Cancel] repeats verbatim), so the trigger is the common path rather than a rare one. The third argument is optional and every existing caller compiles unchanged.

---

State: review-ready on the automated gates. Not merging and not arming auto-merge — leaving it for a human maintainer, with the Cancel-latch question above as the one open decision.