Skip to content

feat(members): steer-only composer for Crew Members DM threads - #8852

Open
CrysisDeu wants to merge 1 commit into
mainfrom
feat/members-chat-steer-only
Open

feat(members): steer-only composer for Crew Members DM threads#8852
CrysisDeu wants to merge 1 commit into
mainfrom
feat/members-chat-steer-only

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

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 ErrorNotice banner, 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): ChatPane previously passed no steer path to ChatInput, so a busy split-view pane showed the warn-coloured queue-only button. Wiring the steer path is a ChatPane change, and split view is ChatPane'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):

crew member page 应该是 steer by default,和一个人聊天是没有 queue 这个概念的,发出去一并处理。

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; ChatPane mounted ChatInput with only onSend/onStop and no canSteer/onSteer, so ChatInput fell through to the queue-only button. chatSlice already reconciles a slot-scoped optimistic steer bubble by sendId, so no backend or store change is needed.

What changed

  • ChatInput — new busyMode?: 'split' | 'steer-only' (default 'split'). steer-only while 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's steerMutation (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 via prepareSendPayload (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. Passes canSteer/onSteer/busyMode to ChatInput; busyMode is a pass-through prop, default 'split', decided by the host.
  • MembersPage — mounts ChatPane with busyMode="steer-only".
  • Steer badgetranscriptRenderers gains hideSteerBadge (a same-id user entry over the SDK row, drawn by the same ChatPageMessageContent.renderUserContent — paste chips, inline images, file cards — one prop different); ChatPane sets it from busyMode === 'steer-only'; UserMessage.hideSteerBadge skips 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.
  • Refused send copy — the server's reason is framed instead of shown bare: "Couldn't send this message: {{error}}. Your text is back in the composer." New key pages.chatPage.send_failed_with_error_restored in en.manual.json + 11 locales (line-anchored, each locale's punctuation) + en-XA regenerated. The shared delivery_unconfirmed notice now says "check the conversation above" (English only; the other locales already said "conversation").
  • Dead keypages.chatPage.dismiss_upload_error lost its last reference when main moved the pane's upload banner to ErrorNotice; removed from all locales + en-XA so the dead-key ratchet stays at baseline.
  • QueueStack in steer-only: rendered on server state only — a steer-only host never asks for a queue, but a message the server did park stays visible and cancellable.
  • No ChatPage.tsx changes.

Draft / recovery hardening (from #8942)

ChatPane's composer is pane-local state, while its host can rebind the same instance to another slot (Members switches slotKey on member click, no key) 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 (createSlotDraftStore on pane-owned sessionStorage keys mc-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) and subscribePaneDraft let the pane showing a slot own the one live copy and receive late arrivals.
  • Attachments on a steerdoSteer carries the ordered non-image file list on the optimistic bubble's and the POST's meta.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 in ChatPane.steerOnly.test.tsx.
  • Attachments on a steer-flagged sendmain now inlines every pane send's attachments via prepareSendPayload ([insider regression] Crew Members DM does not render attached images in the transcript #9433) and carries meta.files through 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 as doSteer and 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 own sendId counts as delivered); uploads carry their originating slot — paths stage into that slot's live or parked composer, a failure shows as an ErrorNotice banner 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 its queue_id in queuedSendStash so cancelling the card restores text and re-stages files.

Tests (run in CI)

  • src/test/ChatInput.test.tsxbusyMode="steer-only" block: plain send button, no split/caret/Queue/Send-options; click and Enter call onSteer not onSend; persisted Queue preference ignored; soft_pending still falls back to onSend; 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-send present, no split/queue, Enter calls sendChat(…, 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 echoes busyMode; the thread test asserts steer-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] path into 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 through ErrorNotice and 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, jscpd clean.

Screenshots

Capture harness: website/scripts/capture-members-steer-only.mjs (asserts each frame's state before writing; --expect=before photographs the base commit). Entries capture/members-page.tsx (?busy=1, runtime frame hooks) and capture/chatpage-scroll-shell.tsx (?scene=busy) gained the busy scenes.

Members DM, radar working — before (queue-only button) after (plain Send, steers)
before after
Split view (⌘D) pane — NEW: Steer/Queue split button (was queue-only) ChatPage — unchanged split button
split view ChatPage

After pressing Send while the member is working:

Optimistic bubble, composer cleared Server steer echo confirmed — plain message, no badge
post-send confirmed

Steer failure states:

Refused steer (409) — framed error row, draft restored, bubble withdrawn Receipt never arrived — delivery-unconfirmed notice, draft restored
refused unconfirmed

Server-parked message on the steer-only surface — queue card still shown and cancellable:

queue card

Draft / recovery frames (from #8942):

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
banner transcript row

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).

Recordings of the composer's idle → busy transition (webm; the capture container has no full ffmpeg, so no GIF — the script prints the ffmpeg command, per the repo's other recording scripts):

Out of scope / 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).
  • Server refusal reasons ("slot agent mismatch") are backend-owned strings; making them plain language is a backend copy task.
  • ChatPage's two refusal sites keep send_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.
  • doSteer and ChatPage.steerMutation are 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.

@CrysisDeu
CrysisDeu requested a review from a team September 6, 2026 00:43
@CrysisDeu
CrysisDeu requested a review from a team as a code owner September 6, 2026 00:43
@CrysisDeu
CrysisDeu requested a review from patrigao September 6, 2026 00:43
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

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

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

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

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

  • Item 4 — symptom-level: the harm named ("a bare reason reads as the agent erroring… says nothing about the restore") holds equally at ChatPage.tsx:996 (steer refusal + handBack()) and ChatPage.tsx:3181 (refusal + restoreComposerAfterFailedSend()), both left on bare send_failed_with_error.

What this change ships

Intent: 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:

  1. Members DM busy composer: plain Send steers; no Steer/Queue split, no queue option — justified
  2. Confirmed steers in a DM draw as ordinary messages (no badge/pulse/requeued note) — justified
  3. Split-view busy panes gain the Steer/Queue split button (was queue-only) — justified
  4. Pane refusal copy reframed: "Couldn't send this message… back in the composer" — symptom-level, 2 restoring ChatPage siblings unfixed
  5. delivery_unconfirmed now says "conversation above" (English) — justified
  6. Dead key dismiss_upload_error removed — justified
  7. Drafts park per member; switching members no longer carries text over — justified
  8. Late send/upload results route to the member they belong to (composer, parked draft, or transcript) — justified
  9. Cancelling a demoted-steer queue card restores typed text and files — justified
  10. Verified capture script, shared roster fixture, committed frames — justified

Watch

  • Point patch on copy: send_failed_with_error_restored has 1 consumer (ChatPane.tsx:585); grep of send_failed_with_error leaves 7 non-test sites, of which the 2 ChatPage ones verifiably also restore the draft. "No ChatPage.tsx changes" pins the scope but never says this divergence is left behind.
    Clears when: ChatPage:996 and :3181 adopt the restored frame, or the split is declared deliberate.
  • One slot's draft now has two homes that never merge: a split-view pane parks into mc-pane-drafts while the main chat reads its own chatDrafts — a draft typed in a split pane will not appear in the main composer for the same slot. The separate key has a stated mechanism cause (ChatPage's wholesale persist would clobber a co-writer), so this is a seam to confirm, not surface to delete.
    Clears when: the author confirms the pane/main-chat draft split is intended for shared (non-member) slots.

[FIRST-PRINCIPLES-REVIEWED] b38cf75

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ human override accepted

Human judgment by @CrysisDeu overrides the GPT 5.6 finding for b38cf7544f81511d642379f62c0de50229074998; the recorded reason is authoritative for this commit.

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:
/ai-review override gpt b38cf7544f81511d642379f62c0de50229074998: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] b38cf75

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

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

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

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

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

  • Split-view busy composer (02-splitview-busy-composer.png) now shows BusySendButton, whose glyph the blind reader could not decode: "I honestly can't tell what the spiral means… I don't know what a spiral-send does versus a normal send" — and would not dare click. Discovery exists only on hover/caret. Moderate frequency (every busy split pane) × hesitation-not-failure × until-learned. The glyph is the main chat's pre-existing control (shot 03-chatpage shows it unchanged) and the before-state queue-only button confused the reader just as much, so the smallest fix — a visible "Steer" text label on the pill — belongs to the shared BusySendButton, likely as a follow-up; a human should decide whether it rides here.
  • Shot-07's refusal row and shot-08's unconfirmed notice both end in "your text is back in the composer," and the reader could not tell "same failure shown two ways, or two different failures." They never co-occur live and each was individually understood, so low severity; if touched, differentiate the openers, not the restore sentence.

[UX-REVIEWED] b38cf75

@CrysisDeu
CrysisDeu force-pushed the feat/members-chat-steer-only branch from e7bb0cc to 5755ddb Compare September 6, 2026 01:41
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 2 — head 5755ddb2712a8ea08dfa8849734fae358ce11d8c. Dispositions per lane:

GPT 5.6 (BLOCKING, ChatPane.tsx:604 — sub-agents-only steer + response-late loses the draft) — fixed. doSend now tracks whether an optimistic bubble was minted; on response-late with no bubble and no echo yet (user row by sendId, queue card by wire text) it restores the text + files and appends the delivery-unconfirmed notice. Applies to the steer-flag path that was flagged and to the plain queued path, which had the same gap. Pinned by the new "sub-agents-only busy … response-late" test.

Frontend Lint (jscpd) — fixed. The roster fixture the two Members capture scripts shared is now one module, scripts/lib/members-fixtures.mjs.

UX Review (CONCERNS)

  • "Steered into the running turn" badge on every busy DM send — implemented in this PR: transcriptRenderers.hideSteerBadge (same-id user entry, same content renderer) wired from busyMode === 'steer-only'; UserMessage.hideSteerBadge skips badge, tint and entrance ring. Default mode keeps the badge (pinned both ways). Frame 04b shows the confirmed steer as a plain message.
  • Evidence gaps — added: post-send optimistic bubble (04), confirmed echo (04b), refused steer with restored draft (05), delivery-unconfirmed notice (06), and two recordings of the composer's idle→busy transition (07 Members: same Send element throughout; 08 split view: Send → Steer/Queue). The recordings are webm: the capture environment has no full ffmpeg (Playwright's bundled build has no GIF encoder), so the script follows the repo's other recording scripts and prints the exact ffmpeg command for a GIF.
  • Target glyph icon-only on the split button — no change. BusySendButton's fire half already carries title="Steer — act on this as soon as possible (Enter)" and aria-label="Steer"; the blind reader saw a static frame, where a tooltip cannot show. The split button is pre-existing ChatPage design now reaching a second host; changing its glyph or adding a text label is a BusySendButton decision for both hosts, out of this PR's scope.

First Principles (CONCERNS)

  • Split view silently gaining the split button — accepted and now declared in the PR body as its own item with its own justification (split-view panes are the main chat's sessions side by side; the queue-only button was the same missing-props defect the DM had). Frame 02's caption now reads "NEW", and the split-view test pins the new state explicitly rather than calling it unchanged.
  • "Drop the ComposerBusyMode enum, take steerOnly?: boolean" — kept as busyMode: 'split' | 'steer-only'. The prop names what the busy send does, and the two values are two nameable behaviours rather than a flag on one of them; a third mode (e.g. a queue-only host) would be a value, not a second boolean. This is also the shape the product owner asked for. One consumer today, but ChatPane reads it in two places (composer + transcript badge), so the name carries meaning beyond the single call site.

Design Review (PASS, suggestion) — hoisting doSteer's receipt ruling to share with ChatPage.steerMutation: agreed in principle, deferred. ChatPage.tsx is under active slimming (P5) and this PR was asked to leave it alone; a one-consumer helper would not remove the duplication. Filed in the PR body's follow-ups so the P5 pass picks it up with the ChatPage side of the edit.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/members-chat-steer-only branch from 5755ddb to f03a44c Compare September 6, 2026 02:13
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 3 — head f03a44c9543b87579cd336dd8c816054cfca7f9f.

GPT 5.6 (BLOCKING, ChatPane.tsx:553 — an older identical queue entry suppresses recovery) — fixed as suggested: doSend snapshots the slot's queue entries (by queueId, falling back to ts|content) before the POST, and on response-late a matching queue card counts as this send's echo only if it is new since the send. Pinned by the new test "an OLDER identical queue entry does not stand in for the unconfirmed send".

Frontend Lint (i18n strict, ChatInput.tsx 4 → 5) — fixed. The steer-only button's offline aria-label passed a literal 'Send' to offlineProps; it now passes i18nT('components.chatInput.send'). ChatInput's strict count is back to the base value.

UX Review (CONCERNS)

  • Raw server reason on a refused send — implemented: reportSendFailure frames it as "Couldn't send this message: {{error}}. Your text is back in the composer." (new key send_failed_with_error_restored, 13 locales + en-XA; the option-chip variant that does not restore keeps send_failed_with_error). Frame 05 recaptured.
  • Mid-transition red arrow in frame 05 — recaptured with a 400 ms settle after the error row lands; the control now reads as the purple Send it is.
  • Evidence gap, QueueStack on the steer-only surface — added frame 09: a server-parked message renders its queue card (cancellable) in the Members DM while the composer stays the plain Send.

First Principles — unchanged from the round-2 disposition: split-view change is declared as its own item; busyMode stays a named two-value prop (read in two places in ChatPane: composer + transcript badge) and is the shape the product owner asked for.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

CI did not schedule for head f03a44c (only the readiness/fork guards ran); closing and reopening to re-trigger the pull_request workflows.

@CrysisDeu CrysisDeu closed this Sep 6, 2026
@CrysisDeu CrysisDeu reopened this Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/members-chat-steer-only branch from f03a44c to 2905478 Compare September 6, 2026 02:41
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: checking Automated validation is still running labels Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/members-chat-steer-only branch from 2905478 to d1f5f9d Compare September 6, 2026 03:13
@github-actions github-actions Bot added the merge conflict Branch has merge conflicts with its base — author must resolve before merge label Sep 6, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/members-chat-steer-only branch from 86d0fc2 to ae56361 Compare September 6, 2026 08:03
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Scope change — head ae56361c5a0774b4a1a571baf256fe9c7cb2baa1.

Per product decision this PR is converged back to the minimal steer-only change: busyMode on ChatInput/ChatPane, the chat-core steer path with canSteer/onSteer, MembersPage steer-only (split view keeps the split button), the hidden steer badge on steer-only, the framed refusal copy, and the tests and frames for exactly those.

The draft/recovery hardening that review grew here over rounds 1–10 — per-slot draft parking on member switch/unmount (chatPaneDrafts), routing late send/upload results to the sending slot, upload-failure attribution + ErrorNotice banner, demoted-steer queuedSendStash binding, unconfirmed-receipt restore for busy sends — is moved wholesale, with its 11 tests and evidence frames 10–12, to #8942 (stacked on this branch). Findings in that area on this PR will be answered with a pointer to #8942 rather than fixed here.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Head f91cf59a186a72af59d7be52ac18b8b3101dbc72 — dispositions.

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)test_snapshot.py:2383/2483 snapshot-copy concurrency assertions; known flaky on main (red at 235d36a62, green on neighbours), no backend files in this diff. Re-run.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/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.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for f91cf59a186a72af59d7be52ac18b8b3101dbc72.

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.

This decision applies only to this commit. A new push requires a new judgment.

bolichen97
bolichen97 previously approved these changes Sep 6, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@bolichen97

Copy link
Copy Markdown
Collaborator

@CrysisDeu This PR and #8947 are both yours and they collide, so the landing order matters. The audit read this branch at 0afdd12; the head has since moved to cfb189b, but the file list still contains the same three shared files, so the overlap below still holds.

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 ChatPane.tsx rewrite, then rebase #8947 on top of it and add its prop to the already-updated mount line. Please note that #8613 is also yours and is a third claimant on that same MembersPage mount line and the same ChatPane props block, so it is worth choosing one order for all three rather than reconciling them pairwise. This PR needs a rebase onto main either way.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Design Review "Watch" (steer receipt policy in three hand-synced copies) — tracked as #9457, which names all three sites (ChatPage.steerMutation, ChatPane.doSteer, ChatPane.doSend's steer-flagged branch), the rulings that must stay identical, and the proposed chat-core helper shape (adopt in ChatPane first, ChatPage under the P5 pass). Kept out of this PR so the product change stays reviewable on its own.

@bolichen97

Copy link
Copy Markdown
Collaborator

Rebased onto main 1192049a by a maintainer as part of the 2026-09-08 open-PR audit. New head b4a9c294.

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: tsc --noEmit clean. vitest on the six test files this PR touches could not run in the audit sandbox because its shared node_modules is missing @excalidraw/excalidraw, which SketchDialog.tsx (main's file, untouched here) imports; that is an environment gap, not a finding. CI runs them.

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.
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

/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.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Human judgment recorded

@CrysisDeu marked the gpt AI finding as false positive, not applicable, or explicitly accepted for b38cf7544f81511d642379f62c0de50229074998.

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.

This decision applies only to this commit. A new push requires a new judgment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

readiness: passed Eligible automated validation passed for the current revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants