feat(members): steer-only composer for Crew Members DM threads - #8852
feat(members): steer-only composer for Crew Members DM threads#8852CrysisDeu wants to merge 1 commit into
Conversation
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Right layers throughout: host declares the mode, pane wires the existing transport, store untouched; every steer failure path is handled, and server-parked queue state stays visible. Suggestions
[DESIGN-REVIEWED] b38cf75 |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of All evidence is gathered and verified. Composing the review now. First-Principles-Verdict: CONCERNS The refused-send reframe ships to one restoring surface; ChatPage's two restoring refusal sites keep the bare copy this PR itself calls misleading. Not justified as shipped
What this change shipsIntent: make DMing a crew member behave like talking to a person — a send while they work just goes through. ADDITION (with FIX items riding in from #8942). More than 10 items; the 10 most user-visible:
Watch
[FIRST-PRINCIPLES-REVIEWED] b38cf75 |
GPT 5.6 Review — ✅ human override acceptedHuman judgment by @CrysisDeu overrides the GPT 5.6 finding for This comment is updated in place on each push. The model was not re-run because an authorized human decision supersedes it. False positive or not applicable? A repository writer can comment: |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of All evidence reconciled: the blind read, all 11 screenshots, the three recordings, and the diff. The primary control (the plain Send that steers in the Members DM) was read cold with full confidence; the one comprehension failure sits on the declared side change. Final review: UX-Verdict: CONCERNS The Members DM Send reads perfectly cold; the split-view side change spreads the spiral Steer pill the blind reader "would not press without knowing." Watch
[UX-REVIEWED] b38cf75 |
e7bb0cc to
5755ddb
Compare
|
Round 2 — head GPT 5.6 (BLOCKING, ChatPane.tsx:604 — sub-agents-only steer + response-late loses the draft) — fixed. Frontend Lint (jscpd) — fixed. The roster fixture the two Members capture scripts shared is now one module, UX Review (CONCERNS)
First Principles (CONCERNS)
Design Review (PASS, suggestion) — hoisting |
5755ddb to
f03a44c
Compare
|
Round 3 — head GPT 5.6 (BLOCKING, ChatPane.tsx:553 — an older identical queue entry suppresses recovery) — fixed as suggested: Frontend Lint (i18n strict, ChatInput.tsx 4 → 5) — fixed. The steer-only button's offline UX Review (CONCERNS)
First Principles — unchanged from the round-2 disposition: split-view change is declared as its own item; |
|
CI did not schedule for head f03a44c (only the readiness/fork guards ran); closing and reopening to re-trigger the pull_request workflows. |
f03a44c to
2905478
Compare
2905478 to
d1f5f9d
Compare
86d0fc2 to
ae56361
Compare
|
Scope change — head Per product decision this PR is converged back to the minimal steer-only change: The draft/recovery hardening that review grew here over rounds 1–10 — per-slot draft parking on member switch/unmount ( |
ae56361 to
f91cf59
Compare
|
Head GPT 5.6 (2 BLOCKING) — both are the draft/recovery hardening that this PR was deliberately converged away from (see the scope-change comment above); both are fixed in #8942, stacked on this branch, with tests:
Not fixed here by product decision: this PR is the one-sentence steer-only change; the robustness work converges in #8942. Backend Tests (3.12, 4) — |
|
/ai-review override gpt f91cf59: Both findings (subagent-only steer timeout restore; late recovery targeting the wrong member) are the draft/recovery hardening deliberately split out of this PR by product decision; they are fixed with tests in #8942, stacked on this branch. This PR is converged to the minimal steer-only change and does not take those fixes. |
Human judgment recorded@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
bolichen97
left a comment
There was a problem hiding this comment.
Tech Lead review — approved on the code, with one sequencing condition below.
Steer-only is not an authorization control, so the "client-side only" question does not bite. busyMode='steer-only' removes the Queue affordance and makes the busy send a steer; it grants no capability. sendTurn({ steer: true }) was already reachable for these same slots from ChatPage, and the server stays authoritative: steer_into_running_turn returns STEER_UNAVAILABLE for a slot with no live steer-capable client, for an identical in-flight steer, and demotes to the queue on teardown — the client cannot force an injection. A user calling the raw API past the UI gets the old queue behaviour, which this PR deliberately keeps rendering (QueueStack on server state, cancellable). Nothing moved a trust boundary into the browser.
Verified independently: the hideSteerBadge transcript entry passes exactly the app-sdk default user renderer's props (content, meta, timestamp, timestampTitle, renderContent) plus the one new flag, so no edit/pin affordance is lost on steer-only surfaces. doSteer's five-way receipt policy matches ChatPage.steerMutation ruling for ruling, restoreIntoComposer(raw, files) correctly hands back the pre-prepareSendPayload text, and the split-view behaviour change is declared and pinned both ways.
Condition — do not cut a release between this and #8942. The two GPT 5.6 BLOCKING findings on this head were cleared only by the author's own /ai-review override, and Design Review's Watch list names the same risk independently: merged alone, this makes steer the default gesture on the DM composer while the recovery hardening (per-slot draft parking, routing a late refusal to the sending slot, receipt-less sub-agent steer restore) lives only in stacked #8942. The override reasoning is legitimate — the fixes are real, tested, and #8942 is readiness: passed and mergeable — but the residual is a broadened draft-loss path on a new default gesture. Land #8942 in the same train.
Advisory-only: Design Review and First Principles CONCERNS (the doSteer/steerMutation twin, deferred to the P5 fold with the ChatPage side) and the PR Scope 999-line warning — 11 of the 41 files are evidence binaries and 13 are one-line locale edits, so the real source surface is ~236 lines. Not blockers.
|
@CrysisDeu This PR and #8947 are both yours and they collide, so the landing order matters. The audit read this branch at Where it overlaps #8947:
The goals are independent (a steer-only busy composer here, Quote / Ask on selected text there) and neither PR implements any part of the other, so both should land. Suggested order: take this one first, because it carries the bigger Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong. |
|
Design Review "Watch" (steer receipt policy in three hand-synced copies) — tracked as #9457, which names all three sites ( |
|
Rebased onto main Conflicts: none, clean rebase (the branch was only 19 commits behind after your last update). No resolution decisions were made, so the diff is unchanged in intent: 46 files, +1783/-63. Gates run locally: Please review that the rebase result still matches your intent. A maintainer push makes the maintainer the last pusher, so under the repo's last-push rule a second approver is needed. Reply here if anything looks wrong. |
A member DM is a conversation with one named member, not an operator
console. Talking to a person has no "queue this until they finish" step,
so while the member is working a send now goes straight into its running
turn: the composer keeps the plain Send button (no Steer/Queue split, no
queue stack) and a confirmed steer renders as an ordinary message (no
"Steered into the running turn" badge). The main chat keeps its split
button; split view (Cmd+D) gains it (it was queue-only, the same missing
props as the DM).
ChatInput gains `busyMode: 'split' | 'steer-only'` (default split).
steer-only renders the idle send button while busy and ignores the
persisted per-slot Queue preference, since there is no Queue to pick.
ChatPane wires the chat-core steer path (`sendTurn({ steer: true })`,
slot-scoped optimistic steer bubble, ChatPage's receipt policy) and
passes `canSteer`/`onSteer`/`busyMode` through; a sub-agents-only busy
pane sends with the steer flag like ChatPage does. transcriptRenderers
gains `hideSteerBadge`, wired by ChatPane from busyMode. MembersPage
mounts the pane with `busyMode="steer-only"`. A refused pane send
frames the server's reason (new key send_failed_with_error_restored,
13 locales + en-XA) instead of showing it bare.
Draft/recovery hardening surfaced by review (per-slot draft parking,
cross-member/unmount recovery routing, upload-failure attribution,
demoted-steer queue binding) landed via the stacked PR #8942 and rides
in this commit. In steer-only, `hideSteerBadge` also silences main's
"Steering..." pending pulse and requeued note -- same vocabulary.
The steer-only `user` row reuses ChatPageMessageContent.renderUserContent
(main's single owner of user-bubble content since #9433).
Tests (CI-run): ChatInput steer-only pins, ChatPane.steerOnly (busy DM
steers, idle sends plain, default pane keeps the split, refused steer
restores, badge hidden only in steer-only, framed refusal copy),
MembersPage asserts the host asks for steer-only. Capture harness,
frames and idle->busy recordings under temp-screenshots/members-steer-only.
|
/ai-review override gpt b38cf75: On main the pane composer is component-local state, so collapsing split view lost the text outright; this PR parks it (sessionStorage + in-tab mirror, chatPaneDrafts.ts) and restores it verbatim when the pane remounts (ChatPane.tsx:177-179). Collapse shows an empty single-chat composer but nothing is lost within the tab. Unifying the pane and ChatPage draft stores is a ChatPage-side refactor, outside this PR's declared scope. |
Human judgment recorded@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
Summary
The Crew Members page's DM composer has no queue concept anymore. While a member is working, a send goes straight into its running turn as a steer: the composer keeps the plain Send button — no Steer/Queue split, no Queue option, no queue stack — and a confirmed steer renders as an ordinary message (no "Steered into the running turn" badge). The main chat (ChatPage) is untouched and keeps its Steer/Queue split button.
Includes #8942 (merged into this branch): the draft/recovery hardening that review surfaced here (per-slot draft parking on member switch/unmount, routing late send/upload results to the slot they belong to, upload-failure attribution through the
ErrorNoticebanner, demoted-steer queue binding, unconfirmed-receipt restore for busy sends) was reviewed on its own stacked PR and now rides in this single commit — see Draft / recovery hardening below.Declared side change — split view (⌘D):
ChatPanepreviously passed no steer path toChatInput, so a busy split-view pane showed the warn-coloured queue-only button. Wiring the steer path is aChatPanechange, and split view isChatPane's other host, so it now shows the same Steer/Queue split button as the main chat (Enter = Steer by default, per-slot preference honoured). Deliberate: split-view panes are the main chat's own sessions side by side. Pinned by its own test and frames.Why
User's product call (session 576 diagnosis):
A member DM is a conversation with one named member — the chat projection of a crew member (#8614) — not an operator console. Talking to a person has no "queue this and wait until they finish, then they'll listen" step, so offering that choice inside a DM presents console mechanics in a chat. This PR is the "chat, not console" consistency close-out for Crew DM.
Diagnosis (confirmed in code): MembersPage mounts
ChatPane;ChatPanemountedChatInputwith onlyonSend/onStopand nocanSteer/onSteer, soChatInputfell through to the queue-only button.chatSlicealready reconciles a slot-scoped optimistic steer bubble bysendId, so no backend or store change is needed.What changed
ChatInput— newbusyMode?: 'split' | 'steer-only'(default'split').steer-onlywhile busy: the idle Send button stays in place (aria-label"Send", accent, ↑), click/Enter =onSteer, and the persisted per-slot Queue preference is not consulted (no Queue to have picked). Without a steer path it still degrades to the queue button, like split mode. Stop states keep their precedence.ChatPane— wires the steer path through the chat-core transport:sendTurn({ steer: true, meta: { sendId } })with the same optimistic-bubble + receipt policy as ChatPage'ssteerMutation(refused/transport-error → drop bubble, error row, restore; response-late → drop + restore + warn notice unless the echo already reconciled;steered→ echo owns the row; demoted →resolveOptimisticSteer). Attachments ride inlined viaprepareSendPayload(steer channel is text-only, as ChatPage). A pane busy only because sub-agents run sends with the steer flag (doSend(…, steerNow)), same rule as ChatPage. PassescanSteer/onSteer/busyModetoChatInput;busyModeis a pass-through prop, default'split', decided by the host.MembersPage— mountsChatPanewithbusyMode="steer-only".transcriptRenderersgainshideSteerBadge(a same-iduserentry over the SDK row, drawn by the sameChatPageMessageContent.renderUserContent— paste chips, inline images, file cards — one prop different);ChatPanesets it frombusyMode === 'steer-only';UserMessage.hideSteerBadgeskips the badge/tint/entrance ring and (after rebasing over fix(dashboard): give pending and requeued steers their own treatment #9037's pending/requeued treatments) the muted "Steering…" pulse and the "turn ended before this applied" note — all three are the steer/queue vocabulary this surface exists to hide. Default surfaces are unchanged. No other surface changes.pages.chatPage.send_failed_with_error_restoredinen.manual.json+ 11 locales (line-anchored, each locale's punctuation) +en-XAregenerated. The shareddelivery_unconfirmednotice now says "check the conversation above" (English only; the other locales already said "conversation").pages.chatPage.dismiss_upload_errorlost its last reference whenmainmoved the pane's upload banner toErrorNotice; removed from all locales + en-XA so the dead-key ratchet stays at baseline.ChatPage.tsxchanges.Draft / recovery hardening (from #8942)
ChatPane's composer is pane-local state, while its host can rebind the same instance to another slot (Members switchesslotKeyon member click, nokey) or unmount it with sends and uploads in flight. Anything that resolves late — a refused or unconfirmed send, a finished or failed upload, a steer the server demoted to the queue — used to land on whatever was on screen, or nowhere. Every late result is now routed to the slot it belongs to: its live composer, its parked draft, or its transcript.utils/chatPaneDrafts.ts(new) — the pane's per-slot draft store (createSlotDraftStoreon pane-owned sessionStorage keysmc-pane-drafts/mc-pane-file-drafts, 256 KiB cap, write-through in-memory mirror so a refused persist still hands the draft back for the tab's life).takePaneDraft(read + clear) andsubscribePaneDraftlet the pane showing a slot own the one live copy and receive late arrivals.doSteercarries the ordered non-image file list on the optimistic bubble's and the POST'smeta.files(the[attached_file N]index the transcript chip resolves through; the echo merges meta onto the bubble), so a filename containing spaces renders as a whole chip instead of the renderer's whitespace-bounded fallback. Pinned inChatPane.steerOnly.test.tsx.mainnow inlines every pane send's attachments viaprepareSendPayload([insider regression] Crew Members DM does not render attached images in the transcript #9433) and carriesmeta.filesthrough the busy-slot queue (fix(chat): carry attachment lists through the busy-slot queue #9486), so the sub-agents-only busy path (doSend(…, steerNow)) gets the same wire shape asdoSteerand ChatPage for free; this PR only pins it for the steer-flagged path (test below) and keeps the raw file list as the restore / cancel-stash payload.ChatPane— parks the outgoing slot's composer on rebind and unmount, takes the incoming slot's parked draft, subscribes to late arrivals;restoreIntoComposer(text, files, forSlot)merges a recovery for an off-screen slot into that slot's parked draft; a busy send with no optimistic bubble whose receipt never arrives restores the draft with the delivery-unconfirmed notice (only an echo carrying the send's ownsendIdcounts as delivered); uploads carry their originating slot — paths stage into that slot's live or parked composer, a failure shows as anErrorNoticebanner while that slot is on screen and otherwise lands in that slot's transcript as an error row; a demoted steer binds{ raw, files, sent }to itsqueue_idinqueuedSendStashso cancelling the card restores text and re-stages files.Tests (run in CI)
src/test/ChatInput.test.tsx—busyMode="steer-only"block: plain send button, no split/caret/Queue/Send-options; click and Enter callonSteernotonSend; persisted Queue preference ignored;soft_pendingstill falls back toonSend; idle sends normally; no steer path → queue button; default busyMode is the split button.src/test/ChatPane.steerOnly.test.tsx(new) — busy DM:steer-only-sendpresent, no split/queue, Enter callssendChat(…, steer=true)with{ sendId }, slot-scoped{ steer, optimistic }bubble, no queue card, composer cleared; idle DM sends with no steer flag; default pane (split view) keeps the Steer/Queue split and its Steer fires the same steer flag; refused steer drops the bubble, appends the framed error row, restores the text; a confirmed steer shows no badge in steer-only and keeps the badge in the default mode.src/pages/members/MembersPage.test.tsx— the ChatPane stub echoesbusyMode; the thread test assertssteer-only.src/test/ChatPane.dirSend.test.tsx— two assertions updated to the framed refusal copy.src/test/ChatPane.draftRecovery.test.tsx(new, 12 cases) +src/test/chatPaneDrafts.test.ts(new, 4 cases) — sub-agents-only busy + response-late restores and warns; sub-agents-only busy + attachment inlines[attached_file 1] pathinto the steer-flagged wire text and stashes the raw file for cancel-restore; 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; late upload success/failure after a switch lands on the member it was picked in; on-screen failure renders throughErrorNoticeand is dismissible; refusal after close + reopen reaches the new pane; upload failure after unmount lands in the transcript; park/take, sessionStorage not localStorage, mirror survives a refused write, subscriber scoping.Local gates:
tsc -b,eslint,i18n-check,jscpdclean.Screenshots
Capture harness:
website/scripts/capture-members-steer-only.mjs(asserts each frame's state before writing;--expect=beforephotographs the base commit). Entriescapture/members-page.tsx(?busy=1, runtime frame hooks) andcapture/chatpage-scroll-shell.tsx(?scene=busy) gained the busy scenes.After pressing Send while the member is working:
Steer failure states:
Server-parked message on the steer-only surface — queue card still shown and cancellable:
Draft / recovery frames (from #8942):
ErrorNoticebanner, draft intactPer-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).
Recordings of the composer's idle → busy transition (webm; the capture container has no full ffmpeg, so no GIF — the script prints the
ffmpegcommand, per the repo's other recording scripts):Out of scope / follow-ups
doSendand ChatPagesend):queuedSendStashis bound after the HTTP receipt, so aqueue_pushthat arrives first exposes a cancel that restores text without attachments. Closing it needsqueue_pushto carry the send'ssendId(backend).ChatPage's two refusal sites keepsend_failed_with_error: they restore into the slot's draft store, so the pane's "back in the composer" sentence would be false after a session switch; own wording under the P5 pass.doSteerandChatPage.steerMutationare two copies of one receipt policy; folding them into a chat-core helper is a P5-adjacent refactor with the ChatPage side of the edit.