fix(chat): exempt a note breadcrumb from the turn-start voice and status effects

[rnoack1]

Problem / Motivation

A note breadcrumb arrives over the websocket as a chat_message with role: 'inject' and a cls carrying reconcile-note. It records a visible row on the transcript, but unlike a queued prompt it does not start an agent turn — so no chat_done frame ever follows it.

Two effects in the chat_message handler in website/src/hooks/useWebSocket.ts fire for every inbound prompt role (user, inject, subagent) on the assumption that a turn is beginning, and both are meant to be undone by turn completion:

• Speech is stopped. stopVoice() runs, the voice progress ref is cleared and the synthesis chain is reset. This is right for a real prompt — you interrupted the agent, so stop reading its previous reply aloud.
• A "Thinking…" status is set on the slot via setSlotStatusDetail. Only turn completion clears it.

Applied to a note, both are wrong, and nothing arrives to repair either one.

Why it matters

This becomes user-visible the moment something starts emitting those rows. #3248 (POST /api/chat/slots/{slot}/note) is the change that does exactly that: its visible half appends role="inject" with the reconcile-note class. Once it merges, every note posted while the agent is being read aloud would cut text-to-speech off mid-sentence, and would leave a "Thinking…" status pinned to the slot indefinitely — a spinner for a turn that was never running. The guard needs to be in place before or alongside #3248 rather than after it, so there is no window where notes strand the UI.

What changed

Three files.

website/src/lib/noteContract.ts (new) owns the wire contract: RECONCILE_NOTE_CLS holds the class name, and isReconcileNote(cls) answers whether a frame carries it. The token lives in exactly one place in this tree, so there is one spelling to change rather than several to keep in sync.

website/src/hooks/useWebSocket.ts gates the two turn-scoped effects on that predicate:

const isPassiveNote = data.role === 'inject' && isReconcileNote(data.cls)

Why membership and not string equality. cls on a chat message is a space-separated class list — msg msg-a and msg msg-a crew-reply are ordinary values, and other consumers in the tree already match a single class with a whitespace-bounded test rather than comparing the whole string. To be clear about the strength of this: #3248's producer currently emits the token on its own, so exact equality would match today too. Membership is defensive rather than load-bearing — it means the guard keeps working if the token is later emitted alongside other classes, which would otherwise silently disable it while every test here still passed. isReconcileNote splits on whitespace, so a longer class name that merely contains the token (reconcile-note-draft) is correctly treated as a different class.

Deliberately not changed: the slot-recency bump a few lines above already includes inject and is left exactly as it is. A note is activity on the slot and should move it up the ordering, and it stays a visible transcript row. The exemption is scoped strictly to the two effects that depend on a turn completing.

Tests

website/src/test/useWebSocket.passiveNote.test.ts — 6 tests, using the existing renderHook + mock-websocket + real-store harness already used by the other useWebSocket.* tests. Every frame is built from RECONCILE_NOTE_CLS rather than a retyped string, so a frame can never drift from the value the guard accepts.

stopVoice() is a hook-local callback with nothing to spy on, so it is observed through the one store write it makes: setVoicePlaying(false). Inside the chat_message case that dispatch has no other caller, and the sseChatMessage reducer never writes voicePlaying — so a true -> false flip is attributable to stopVoice() having run. The status effect is read directly off slotStatusDetail[slot].kind.

What the tests pin:

• a note leaves speech running and sets no status;
• a plain role: 'inject' with no class still stops speech and still sets thinking — the behaviour that was already correct and must not regress;
• a note still lands on the transcript;
• the token arriving alongside other classes (msg reconcile-note) is still treated as a note;
• a different class that merely contains the token (msg reconcile-note-draft) is not treated as a note, so the widening is membership and not substring matching;
• one test asserts the constant's literal value. Because the frames are built from the constant, they would follow a rename without complaint; this assertion is what makes a change to the wire value fail loudly instead.

Since the frames build from the constant, the membership rule was verified by perturbing the guard directly: with the constant in place but the comparison left as exact equality, the wrapped-class test fails with expected false to be true (speech was cut), and it passes once the predicate tests membership. The substring case stays failing-as-expected in both, which is what rules out an over-broad includes.

Suite on this branch: 20 test files, 207 tests, 0 failures across the touched file and every useWebSocket.* suite (the hook is shared, so that scope includes the thinking-status and auto-speak suites). tsc --noEmit clean. ESLint on the touched files reports 4 warnings, all pre-existing — the identical set appears on the main baseline (no-console x3 and one react-hooks/exhaustive-deps).

Note on an earlier revision of this description

An earlier version of this description argued for keeping the class name written inline at the call site and repeated in the test frames, on the grounds that repeating it acted as a tripwire. That reasoning has been superseded and the paragraph making it is gone: the change now centralises the token in noteContract.ts and the tests import it, with an explicit assertion on the literal value serving the tripwire role instead. That earlier text also described a single-file change with three tests, which no longer matches what ships. Anything still reading as an argument against the shared constant is stale wording, not the current design.

[iamwhatever]

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

[iamwhatever]

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

[bolichen97]

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed b8822058922e93abe1159c6c146b96e213c77936 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] b882205

[github-actions]

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of b8822058922e93abe1159c6c146b96e213c77936 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff is a narrow behavioral fix: a passive reconcile-note inject no longer cuts text-to-speech mid-sentence or strands a permanent "Thinking…" status, while plain injects keep both turn-start effects. No new strings, surfaces, or visual components — and the base code confirms setSlotStatusDetail "thinking" is only cleared by turn completion, so exempting no-turn notes is the correct experience. Tests pin both directions.

UX-Verdict: PASS

Notes no longer hijack speech or pin a phantom "Thinking…" on the slot; the fix is invisible-when-right, with both directions pinned by tests.

[UX-REVIEWED] b882205

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of b8822058922e93abe1159c6c146b96e213c77936 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound, scoped guard — but its wire contract lives in an unmerged PR and rides a field this codebase has already found unreliable.

Watch

• The token is pinned frontend-only (RECONCILE_NOTE_CLS = 'reconcile-note' in noteContract.ts), while the producer is feat(chat): POST /api/chat/slots/{slot}/note -- visible line plus silent next-turn context #3248's Python endpoint — still open, with no shared constant or cross-side test tying the two spellings together. If feat(chat): POST /api/chat/slots/{slot}/note -- visible line plus silent next-turn context #3248's emitted class drifts before merge, isReconcileNote silently stops matching and the exact bug this PR prevents (cut speech, pinned "Thinking…") ships anyway with every test here green. Add a producer-side pin in feat(chat): POST /api/chat/slots/{slot}/note -- visible line plus silent next-turn context #3248 that mirrors this constant's literal test.
• Carrying a behavioral semantic in cls repeats a pattern the tree already retreated from: TurnBlock.tsx:103-106 documents that the persistence flush drops cls for every role except system, which is why crew-reply moved to meta.crew_reply. The guard here is live-frame-only so it works today, but since feat(chat): POST /api/chat/slots/{slot}/note -- visible line plus silent next-turn context #3248 is unmerged the contract is still cheap to change — a meta flag (e.g. meta.note = true) would survive persistence and match the codebase's established durable-marker convention.

[DESIGN-REVIEWED] b882205

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed b8822058922e93abe1159c6c146b96e213c77936 via the fork AI-review pipeline; updated in place on each push.

Review details

The diff is small and self-contained. The recording (sseChatMessage, line 937) and recency (bufferSlotActivity, lines 945–949) both fire before the new guard, so the note is still transcribed and re-ranked. Only the two turn-scoped effects (stopVoice and the Thinking… status, lines 956–958) are gated behind !isPassiveNote. The isReconcileNote helper safely handles undefined/null/empty and uses whitespace-bounded membership rather than substring matching. No user-facing strings, dates, numbers, or sort orders are introduced. Nothing is groundable to the 80+ bar for a new finding.

No findings.

[OPUS-REVIEWED] b882205

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of b8822058922e93abe1159c6c146b96e213c77936 via the fork AI-review pipeline — 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 gathered. Key facts: reconcile-note has zero occurrences in the base tree (the producer is unmerged PR #3248), cls is genuinely a space-separated class list in this tree (msg msg-u), and the tree already marks message semantics with a meta flag (meta: { steer: true }, useWebSocket.ts:991). Composing the review now.

First-Principles-Verdict: CONCERNS

The guard defends against a frame nothing in this tree emits — grep reconcile-note at base: 0 files — so every shipped behavior is dormant until #3248 merges.

What this change ships

Intent: keep a posted note from cutting text-to-speech and stranding a "Thinking…" spinner. Framed as a FIX, but at base no producer of these frames exists, so it is an ADDITION staged ahead of one.

1. A note row no longer stops speech mid-sentence — justified, but unreachable until feat(chat): POST /api/chat/slots/{slot}/note -- visible line plus silent next-turn context #3248 lands
2. A note row no longer pins "Thinking…" forever — same
3. New wire-contract module noteContract.ts (RECONCILE_NOTE_CLS, isReconcileNote) — declared; zero producers in tree
4. Guard matches the token as a class-list member, not by equality — justified: cls is demonstrably a class list (msg msg-u)
5. Plain inject behavior explicitly pinned unchanged — justified regression guard

Watch

• Zero producers, counted. Grepped reconcile-note across the repo: 0 matches at base. The named harm is real but exists only in unmerged feat(chat): POST /api/chat/slots/{slot}/note -- visible line plus silent next-turn context #3248; if that PR's wire changes or it dies, noteContract.ts is a stranded contract on main with one consumer guarding an impossible frame. The description concedes it: "This becomes user-visible the moment something starts emitting those rows."
• An existing mechanism marks frame semantics. The steer path already flags meaning in-band with meta: { steer: true } (useWebSocket.ts:991); discriminating behavior on a CSS class token is a second spelling of "mark this frame's kind," coupled to presentation. Count: 1 existing meta-flag instance, 0 existing cls-token discriminators.

Subtractions

• Defer: fold these three files into feat(chat): POST /api/chat/slots/{slot}/note -- visible line plus silent next-turn context #3248 itself, so guard and producer land atomically and main never carries a contract for a frame it cannot receive.
• Replace noteContract.ts with the existing meta-flag mechanism (meta.note, mirroring meta.steer at useWebSocket.ts:991), coordinated in feat(chat): POST /api/chat/slots/{slot}/note -- visible line plus silent next-turn context #3248 — deletes the new module and the membership predicate outright.

[FIRST-PRINCIPLES-REVIEWED] b882205