fix(chat-pane): route late sends and uploads to the slot they belong to

[CrysisDeu]

Summary

Draft / recovery hardening for ChatPane, split out of #8852 (stacked on feat/members-chat-steer-only) so the steer-only product change stays minimal and this robustness work can converge on its own.

ChatPane's composer is pane-local state, while its host can rebind the same instance to another slot (the Members page switches slotKey on member click, no key) or unmount it with sends and uploads still in flight. Anything that resolves late — a refused or unconfirmed send, a finished or failed upload, a steer the server demoted to the queue — then lands on whatever is on screen, or nowhere. This PR routes every late result to the slot it belongs to: its live composer, its parked draft, or its transcript.

What the review found (GPT 5.6 on #8852, quoted)

Round 1:

BLOCKING -- website/src/components/ChatPane.tsx:604 -- A delayed subagent-only send discards the draft
if (!running) { doSend(undefined, true); return }
Active subagents + POST timeout -> doSteer -> doSend clears input -> response-late leaves no row, notice, or restored draft.

Round 2:

BLOCKING -- website/src/components/ChatPane.tsx:553 -- An older identical queue entry suppresses recovery of the current send
(m.role === 'queued' && m.content === llm)
Existing identical queue entry -> new request stalls before enqueue -> timeout sees the old entry -> cleared draft is neither delivered nor restored.

Round 4:

BLOCKING -- website/src/components/ChatPane.tsx:669 -- Late steer recovery targets the newly selected member
restoreIntoComposer(raw, files)
Send to member A -> switch to an already-open member B -> A's draft and attachments appear in B's composer.

BLOCKING -- website/src/components/ChatPane.tsx:574 -- An unrelated identical queue card suppresses recovery
Hung send -> another sender queues identical text -> unrelated card suppresses restoration and the cleared draft is lost.

Round 5:

BLOCKING -- website/src/components/ChatPane.tsx:439 -- Completed uploads are parked under the wrong member
Upload in A -> switch to B before uploadFiles resolves -> callback appends paths to B -> A loses its attachment and B can send it.

Round 7:

BLOCKING -- website/src/components/ChatPane.tsx:425 -- Upload failures are discarded across member switches
Upload in A -> switch or unmount before rejection -> callback drops the failure -> returning to A shows no error; active failures also bypass ErrorNotice.

Round 8:

BLOCKING -- website/src/components/ChatPane.tsx:190 -- Off-screen upload failures disappear on page exit
Upload in A -> switch to B -> failure enters component-only state -> leave page -> failure is never rendered.

Round 9:

BLOCKING -- website/src/components/ChatPane.tsx:513 -- Late recovery is lost after the pane remounts
Pending send → unmount and reopen member → old pane stores recovery → new pane later overwrites it with stale live state, losing text/files.

Round 10:

BLOCKING -- website/src/components/ChatPane.tsx:789 -- Queued fallback steers lose staged attachments when cancelled
Spaced-path attachment -> unavailable steer queues -> cancel fallback restores marker text without the file chip.

First Principles (round 6) additionally noted that an in-memory per-slot Map loses the parked drafts on unmount; the store below is the answer.

What changed

• utils/chatPaneDrafts.ts (new) — the pane's per-slot draft store, an instance of the repo's createSlotDraftStore on pane-owned keys (mc-pane-drafts / mc-pane-file-drafts, both sessionStorage with a 256 KiB cap — parking is a within-session hand-off, and staying out of localStorage keeps it clear of the quota ChatPage's two 2 MiB stores share). Every parked draft is also mirrored in memory (write-through), so a persist the browser refuses still hands the draft back for the life of the tab. Separate keys, not chatDrafts/chatFileDrafts: ChatPage holds those maps in memory and persists them wholesale on a debounce, so a second writer on the same key would be overwritten and never read until a reload. The pane does read-modify-write on every access, so split-view panes can share the keys. takePaneDraft (read + clear) and subscribePaneDraft let a pane that shows a slot own the one live copy and receive late arrivals for it.
• ChatPane —
  • parks the outgoing slot's composer on rebind and on unmount; takes the incoming slot's parked draft; subscribes to late arrivals for the slot on screen;
  • restoreIntoComposer(text, files, forSlot): a recovery for a slot no longer on screen (or a pane no longer mounted) merges into that slot's parked draft; every recovery site passes the sending slot;
  • a busy send that minted no optimistic bubble and whose receipt never arrives (response-late) hands the draft back with the delivery-unconfirmed notice; only an echo carrying the send's own sendId counts as delivered (queue cards carry none — text-matching cannot tell this send's card from anyone's identical "ok");
  • uploads carry their originating slot: paths stage into that slot's live or parked composer; a failure shows as a banner while that slot is on screen and otherwise goes into that slot's transcript as an error row (store-backed, survives rebinds and page exits);
  • the upload banner (already ErrorNotice on main) is fed from the per-slot failure map, so a failure never shows over another member's thread;
  • a steer the server demoted to the queue binds { raw, files, sent } to its queue_id in queuedSendStash, so cancelling the card restores typed text and re-stages files, exactly as doSend does.

Tests (run in CI) — src/test/ChatPane.draftRecovery.test.tsx (11 cases) + src/test/chatPaneDrafts.test.ts (4 cases: park/take, sessionStorage not localStorage, mirror survives a refused write, subscriber scoping)

Sub-agents-only busy + response-late restores the draft and warns; an identical queue card does not stand in for the unconfirmed send; a demoted steer binds pre-send state to its queue card; rebinding parks the draft; a refusal after a member switch restores into the SENDING member; a late upload after a switch stages for the member it was picked in; a late upload FAILURE after a switch lands in that member's transcript; the on-screen failure renders through ErrorNotice and is dismissible; a refusal after close + REOPEN reaches the new pane and survives its next park; an upload failure after unmount lands in the transcript; a draft survives unmount plus a refusal that lands while the pane is gone.

Local gates: tsc -b, eslint, i18n-check, jscpd clean.

Screenshots

Refused upload while the member is on screen — ErrorNotice banner, draft intact	Same refusal landing after switching to another member — no banner over fixer; error row in radar's transcript on return

Per-member draft parking (webm): 12-member-switch-draft-parking.webm — type for radar, switch to fixer (empty composer), type for fixer, back to radar (radar's text is back).

Pattern harvest

Rule candidate: review lens — an async callback that writes component state must carry the identity it was started for. Every finding above has the same shape: a promise started for slot A (sendTurn, uploadFiles) resolves after the pane was rebound to slot B or unmounted, and its .then writes setInput / setPendingFiles / setUploadError against whatever is current. The general fix is what this PR does — capture the target (slotKey) at call time, route the result through a slot-keyed store or the store-backed transcript when the target is no longer on screen, and make the live composer the single copy (take + subscribe). Candidate mechanical check: flag useMutation/.then callbacks in a component whose props include a slot/session id but whose closure does not reference it.

Follow-ups

• Pre-existing (main, doSend and ChatPage send): queuedSendStash is bound after the HTTP receipt, so a queue_push that arrives first exposes a cancel that restores text without attachments. Closing it needs queue_push to carry the send's sendId (backend).

• doSteer / ChatPage.steerMutation receipt policy and the two draft stores are candidates for one chat-core consolidation with the ChatPage side (P5).

• Server refusal reasons are backend-owned strings; plain-language variants are a backend copy task.

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Every late async result now carries the slot it was started for — a real loss class closed at the right layer, with the store/live-composer split soundly reasoned.

[DESIGN-REVIEWED] 7c31c07

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 7c31c07a46f9101a11157dec0877a6c755e65a32 — 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.

All the load-bearing claims verify: MembersPage.tsx:615 rebinds one ChatPane with no key, SessionGridView.tsx:131 also passes slotKey bare so the component-level fix covers both mounts; ChatPage does hold its draft maps in refs and persists wholesale on a debounce, so sharing its keys really would be last-write-wins clobbered; temp-screenshots/ is an established convention (1064 committed files); createSlotDraftStore is a factory built to host thin instances, and this is one. The one depth gap: ChatPage's own upload-failure path still writes a global banner with no slot routing, while its success path routes per slot — the exact root cause this PR's description generalizes.

First-Principles-Verdict: CONCERNS

Every item traces to a quoted blocking defect, but the rule the PR itself generalizes has one counted, unfixed sibling in ChatPage's upload-failure path.

What this change ships

Intent: make every late-resolving send or upload land on the member it was started for, not whoever is on screen — a FIX (nine quoted blocking findings from review of #8852).

1. Switching members parks each composer's text and files per member — justified (round 4, webm)
2. A late refusal restores into the SENDING member's composer or parked draft — justified (rounds 1, 4)
3. An unconfirmed busy send hands the draft back with a warning notice — justified (round 1)
4. An identical queue card no longer counts as delivery; only the send's own id does — justified (rounds 2, 4)
5. A late-finishing upload stages files for the member it was picked in — justified (round 5)
6. An off-screen upload failure becomes an error row in that member's transcript; on-screen stays a dismissible banner — justified (rounds 6–8)
7. Cancelling a demoted-steer queue card restores typed text and re-stages file chips — justified (round 10)
8. Parked drafts survive close-and-reopen via two new sessionStorage keys — justified (round 9; FP round 6 named the in-memory-only gap)
9. Three new capture scenarios plus committed screenshots/webm — justified (repo convention, 1064 files under temp-screenshots/)

Watch

• The description's own rule — "an async callback that writes component state must carry the identity it was started for" — has 1 unfixed sibling: ChatPage's upload FAILURE writes the global banner with no slot check (ChatPage.tsx:3798 and :3812, grepped uploadError in ChatPage.tsx), while its SUCCESS path already routes per slot (fileLandingSlot, :3800). Same defect as round 7, one component over; accept-and-defer explicitly rather than leave it unnamed.

Subtractions

• Drop the forSlot: string = slotKeyRef.current default on restoreIntoComposer (ChatPane.tsx:531) — all 4 call sites pass the slot explicitly (grepped restoreIntoComposer(), and the default silently re-creates the routed-to-whatever-is-on-screen bug for any future caller that omits it.
• Unexport PANE_FILE_DRAFTS_KEY, PANE_DRAFTS_MAX_BYTES, and readPaneDraft in chatPaneDrafts.ts — 0 non-test consumers each (grepped each symbol across website/src; only the defining module and its test file).

[FIRST-PRINCIPLES-REVIEWED] 7c31c07

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

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

The evidence is complete. Both screenshots reconcile with the diff, the recording covers draft parking, and both new user-facing strings pre-exist in the catalog (delivery_unconfirmed asserts state and offers an action; the code genuinely lacks delivery confirmation at that render site, so it is not hedging). The one comprehension signal in the blind read: the reader correctly identified the upload-failure banner and its dismiss X, but on the transcript-row variant said "I can't be sure it's meant to be the same thing since it moved and lost its close button."

UX-Verdict: CONCERNS

Same upload failure wears two faces — dismissible banner vs bare transcript row — and the blind reader couldn't tell they were the same thing.

Watch

• One failure, two presentations: on-screen refusal renders as ErrorNotice (icon + dismiss X, shot-01); off-screen it becomes a bare red transcript row (shot-02, no icon, no dismiss). Blind reader: "I can't be sure it's meant to be the same thing since it moved and lost its close button." Low frequency (upload failure × member switch) but recurring confusion each time; smallest fix: give the role: 'error' transcript row the same alert icon/shape as the banner so both read as one mechanism.

Evidence gaps

• The response-late state this PR adds — draft refilled + "Delivery not confirmed" notice (possibly beside a duplicate queue card) — appears in no screenshot; one shot of the notice above the refilled composer would close it.
• Cancelling a demoted-steer queue card now re-stages file chips (queuedSendStash.set(...)); no screenshot or recording shows the restored chip.

[UX-REVIEWED] 7c31c07

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 7c31c07

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

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

I've now examined both candidates against the actual code.

Candidate 1 (response-late restore fires for busy queued sends in split view): The new branch is reached whenever !bubbleMinted (i.e. busy) and no user-echo carries the send's sendId. In split view a busy queued send whose POST exceeds SEND_ABORT_MS (10s) does hit response-late, and queue cards carry no sendId, so echoed is false → the draft is refilled and a "delivery unconfirmed" notice appended. But at the moment response-late fires, delivery is genuinely indeterminate — the server may have dropped the message (silent loss) or queued it (later card). The code trades a recoverable, visible false-warning for the silent-loss case, and the notice explicitly tells the user to check before re-sending. It does not auto-send, so there is no automatic duplicate and no data loss. This is a deliberate, documented, uniform design trade-off ("the same ruling the steer path takes"), not a concrete defect. (c) is a debatable UX preference, not an observable wrong outcome. Below the bar.

Candidate 2 (fileStore lacks entry/byte caps): Reproduction requires >DRAFT_MAX_ENTRIES parked slots holding attachments (or a >256 KiB text blob) and a tab reload to clear the in-memory mirror, after which textStore evicts a slot's text while fileStore keeps its files. The result is attachment chips with no accompanying text — a valid, sendable state (attach-without-typing is legitimate), and the candidate itself classifies it as cosmetic, not data loss. Extreme edge case, non-defect outcome. Below the bar.

Neither candidate re-derives a concrete input → call path → observable wrong outcome at 80+. No new grounded finding surfaced during falsification.

No findings.

[OPUS-REVIEWED] 7c31c07

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

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

[CrysisDeu]

Head 7c31c07a46f9101a11157dec0877a6c755e65a32.

GPT 5.6 — chatPaneDrafts.ts:36 "Pane drafts exceed the aggregate storage budget" — fixed. The pane's text store no longer sits in localStorage beside ChatPage's two 2 MiB stores: both pane stores are now sessionStorage (parking is a within-session hand-off; it does not need to outlive the tab) with a 256 KiB byte cap, and every parked draft is also mirrored in memory (write-through; reads prefer the mirror, then storage for a reloaded tab). A persist the browser refuses — quota, disabled storage — therefore still hands the draft back for as long as the tab lives. New unit test chatPaneDrafts.test.ts: "hands the draft back from the mirror when storage refuses the write" (setItem mocked to throw QuotaExceededError), plus session-not-local, take-clears, and subscriber-scoping cases.

GPT 5.6 — ChatPane.tsx:713 "Queue cancellation can beat attachment stashing" (origin: validation) — pre-existing, not introduced or touched by this PR: doSend has bound queuedSendStash after the HTTP receipt since #560/#5891 (ChatPage's send does the same), and the line is only in this diff as context. The window is real (a queue_push can precede the receipt), but closing it needs the queue card to be correlatable before the receipt — i.e. the backend echoing the send's sendId on queue_push, which queue rows do not carry today. Filed as a follow-up in the body; out of scope for a client-only hardening PR.

[CrysisDeu]

Head 7c31c07a46f9101a11157dec0877a6c755e65a32 — all checks green; dispositions for the advisory lanes.

UX Review (CONCERNS)

• Banner vs transcript row for the same upload failure — the off-screen variant deliberately uses the transcript's shared error row (ErrorCard), the one place every "this did not go out" failure already lands (refused sends included), because a banner belongs to the composer on screen and the failure's owner is not. Giving ErrorCard the banner's icon/shape is a transcript-wide change to a shared component, out of this PR's scope; noted for the error-surface consolidation.
• Response-late evidence — that state is photographed on the base PR: feat(members): steer-only composer for Crew Members DM threads #8852 frame 06 (06-members-unconfirmed-steer.png: refilled composer + "Delivery not confirmed" notice); this PR reuses the same notice for the busy-send path.
• Demoted-steer cancel re-staging the chip — the restore path is useQueuedMessageActions's existing cancel (unchanged here); this PR only feeds it the stash. Covered by the unit test rather than a frame.

First Principles (CONCERNS)

• ChatPage's upload-failure banner has the same slot-agnostic write — accepted and deferred explicitly: ChatPage is under the P5 slimming pass and its own fileLandingSlot routing is the natural place to fold it in.
• restoreIntoComposer's forSlot default and the three unused exports — agreed; small hygiene, left for the next push on this branch rather than spending a full CI + review round on them alone.