From b8822058922e93abe1159c6c146b96e213c77936 Mon Sep 17 00:00:00 2001 From: Robert Noack Date: Tue, 18 Aug 2026 15:54:32 +0000 Subject: [PATCH] fix(chat): exempt a note breadcrumb from the turn-start voice and status effects A note (role=inject, cls=reconcile-note) starts no turn, so nothing follows to undo the stopped speech or the "Thinking..." status; the recency bump is left in place. --- website/src/hooks/useWebSocket.ts | 8 +- website/src/lib/noteContract.ts | 24 ++ .../src/test/useWebSocket.passiveNote.test.ts | 216 ++++++++++++++++++ 3 files changed, 246 insertions(+), 2 deletions(-) create mode 100644 website/src/lib/noteContract.ts create mode 100644 website/src/test/useWebSocket.passiveNote.test.ts diff --git a/website/src/hooks/useWebSocket.ts b/website/src/hooks/useWebSocket.ts index fc786f1e9d2..6d091444413 100644 --- a/website/src/hooks/useWebSocket.ts +++ b/website/src/hooks/useWebSocket.ts @@ -1,6 +1,7 @@ import { useEffect, useRef, useCallback } from 'react' import { useQueryClient } from '@tanstack/react-query' import { isArtifactEditing } from '../utils/artifactEditGuard' +import { isReconcileNote } from '../lib/noteContract' import { useAppDispatch } from '../store' import { store } from '../store' import { sseStatus, sseConnected, sseDisconnected, sseSlots, sseTodoUpdate, setChannelTrusted, sseSlotTitle, triggerRefresh, fetchSlots, markSlotUnread, setUpdateProgress, sseSubagentStatus, sseSubagentText, touchSlotActivity, patchSlotSourceLinks, type SubagentDetail } from '../store/dashboardSlice' @@ -953,8 +954,11 @@ export function useWebSocket() { // trigger (no-op unless an L2 theme with that manifest sound is // active + unmuted). User/tool messages don't chime. if (data.role === 'assistant') emitThemeSound('message-received') - if (data.role === 'user' || data.role === 'inject' || data.role === 'subagent') { stopVoice(); voiceProgressRef.current = null; synthChainRef.current = Promise.resolve() } - if (data.slot && (data.role === 'user' || data.role === 'inject' || data.role === 'subagent')) { + // A note breadcrumb starts no turn, so no chat_done arrives to undo either + // effect: cutting speech would strand it and a thinking status would never clear. + const isPassiveNote = data.role === 'inject' && isReconcileNote(data.cls) + if (!isPassiveNote && (data.role === 'user' || data.role === 'inject' || data.role === 'subagent')) { stopVoice(); voiceProgressRef.current = null; synthChainRef.current = Promise.resolve() } + if (!isPassiveNote && data.slot && (data.role === 'user' || data.role === 'inject' || data.role === 'subagent')) { dispatch(setSlotStatusDetail({ slot: data.slot, kind: 'thinking', text: 'Thinking…', ts: Date.now() })) } break diff --git a/website/src/lib/noteContract.ts b/website/src/lib/noteContract.ts new file mode 100644 index 00000000000..22a241cea24 --- /dev/null +++ b/website/src/lib/noteContract.ts @@ -0,0 +1,24 @@ +/** + * Wire contract for the `cls` token that marks a note breadcrumb. + * + * `cls` is a space-separated class list — `msg msg-a`, `msg msg-a crew-reply` — + * and other consumers already match a single class with a whitespace-bounded + * test. Today's producer emits this token on its own, so equality would match + * too; membership is defensive against the token later arriving alongside + * others, which would silently kill the guard while tests stayed green. + * + * The value lives here rather than inline at the call site so the tree has + * exactly one spelling of it. + */ +export const RECONCILE_NOTE_CLS = 'reconcile-note' + +/** + * True when `cls` carries the note class as a whole class. + * + * Splitting is what makes this a class test rather than a substring test: + * `reconcile-note-draft` contains the token but is a different class. + */ +export function isReconcileNote(cls: string | undefined | null): boolean { + if (typeof cls !== 'string' || cls.length === 0) return false + return cls.trim().split(/\s+/).includes(RECONCILE_NOTE_CLS) +} diff --git a/website/src/test/useWebSocket.passiveNote.test.ts b/website/src/test/useWebSocket.passiveNote.test.ts new file mode 100644 index 00000000000..6b609d42f36 --- /dev/null +++ b/website/src/test/useWebSocket.passiveNote.test.ts @@ -0,0 +1,216 @@ +/** + * A note breadcrumb arrives as `role: 'inject'` with `cls: 'reconcile-note'`. It + * records something on the transcript but starts no agent turn, so no + * `chat_done` ever follows it. + * + * Two effects on the inbound-prompt path assume a turn is beginning: they stop + * text-to-speech, and they set a "Thinking…" status that only turn completion + * clears. Applied to a note, the first cuts speech off mid-sentence and the + * second leaves a status stuck on the slot forever. Both directions are pinned + * here — a note is exempt, a plain inject still triggers both. + * + * `stopVoice()` is a hook-local callback with nothing to spy on, so it is + * observed through its one store write, `setVoicePlaying(false)`: inside the + * `chat_message` case that dispatch has no other caller. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { renderHook, act } from '@testing-library/react' +import { createElement } from 'react' +import { Provider } from 'react-redux' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { useWebSocket } from '../hooks/useWebSocket' +import { store } from '../store' +import { setActiveSlot, clearMessages, clearSlotState, setVoicePlaying } from '../store/chatSlice' +import { RECONCILE_NOTE_CLS, isReconcileNote } from '../lib/noteContract' + +vi.mock('../api/client', () => ({ + api: { + chatSlots: vi.fn().mockResolvedValue([]), + voiceConfig: vi.fn().mockResolvedValue({ autoSpeak: true }), + voiceSynthesize: vi.fn().mockResolvedValue({}), + approvals: vi.fn().mockResolvedValue([]), + notifications: vi.fn().mockResolvedValue({ notifications: [], unread: 0 }), + chatSlotDetail: vi.fn().mockResolvedValue({ messages: [], running: false, has_more: false, total: 0, queue: [] }), + }, +})) + +const WS_INSTANCES: MockWebSocket[] = [] + +class MockWebSocket { + static OPEN = 1 + static CONNECTING = 0 + readyState = MockWebSocket.CONNECTING + onopen: ((ev: Event) => void) | null = null + onmessage: ((ev: MessageEvent) => void) | null = null + onclose: ((ev: CloseEvent) => void) | null = null + onerror: ((ev: Event) => void) | null = null + send = vi.fn() + close = vi.fn() + + constructor() { WS_INSTANCES.push(this) } + + simulateOpen() { + this.readyState = MockWebSocket.OPEN + this.onopen?.(new Event('open')) + } + + simulateMessage(data: object) { + this.onmessage?.(new MessageEvent('message', { data: JSON.stringify(data) })) + } +} + +// A true -> false flip of voicePlaying means stopVoice ran: inside the +// chat_message case that dispatch has no other caller. +const voicePlaying = () => store.getState().chat.voicePlaying +const statusKind = (slot: string) => store.getState().chat.slotStatusDetail[slot]?.kind + +describe('useWebSocket passive note (role=inject, cls=reconcile-note)', () => { + let queryClient: QueryClient + + beforeEach(() => { + vi.clearAllMocks() + WS_INSTANCES.length = 0 + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + vi.stubGlobal('WebSocket', MockWebSocket) + // clearMessages does NOT reset slotStatusDetail, so a "Thinking…" set by one + // test would otherwise leak forward and make these assertions order-dependent. + store.dispatch(clearSlotState()) + store.dispatch(setActiveSlot('slot-1')) + }) + + afterEach(() => { + vi.unstubAllGlobals() + store.dispatch(clearMessages()) + store.dispatch(clearSlotState()) + store.dispatch(setActiveSlot(null)) + store.dispatch(setVoicePlaying(false)) + }) + + async function mount() { + function wrapper({ children }: { children: React.ReactNode }) { + return createElement(Provider, { store }, + createElement(QueryClientProvider, { client: queryClient }, children)) + } + const hook = renderHook(() => useWebSocket(), { wrapper }) + const ws = WS_INSTANCES[0] + act(() => { ws.simulateOpen() }) + await act(async () => {}) + return { hook, ws } + } + + it('leaves speech running and sets no thinking status', async () => { + const { hook, ws } = await mount() + + // Speech is in progress when the note lands. + act(() => { store.dispatch(setVoicePlaying(true)) }) + expect(voicePlaying()).toBe(true) + + act(() => { + ws.simulateMessage({ + type: 'chat_message', + data: { slot: 'slot-1', role: 'inject', cls: RECONCILE_NOTE_CLS, content: 'a note', ts: '10.0' }, + }) + }) + await act(async () => {}) + + expect(voicePlaying()).toBe(true) + expect(statusKind('slot-1')).toBeUndefined() + + hook.unmount() + }) + + it('still stops speech and sets a thinking status for a plain inject', async () => { + const { hook, ws } = await mount() + + act(() => { store.dispatch(setVoicePlaying(true)) }) + expect(voicePlaying()).toBe(true) + + // A queued prompt: no cls, a real turn follows, so both effects are correct. + act(() => { + ws.simulateMessage({ + type: 'chat_message', + data: { slot: 'slot-1', role: 'inject', content: 'a queued prompt', ts: '11.0' }, + }) + }) + await act(async () => {}) + + expect(voicePlaying()).toBe(false) + expect(statusKind('slot-1')).toBe('thinking') + + hook.unmount() + }) + + it('records the note on the transcript and bumps slot recency', async () => { + const { hook, ws } = await mount() + + // The exemption is scoped to the two turn-scoped effects: a note is still a + // visible row, and still counts as activity on the slot. + act(() => { + ws.simulateMessage({ + type: 'chat_message', + data: { slot: 'slot-1', role: 'inject', cls: RECONCILE_NOTE_CLS, content: 'a visible note', ts: '12.0' }, + }) + }) + await act(async () => {}) + + const messages = store.getState().chat.messages + expect(messages.some(m => m.content === 'a visible note')).toBe(true) + + hook.unmount() + }) + + it('is exempt when the token arrives alongside other classes', async () => { + const { hook, ws } = await mount() + + act(() => { store.dispatch(setVoicePlaying(true)) }) + + // `cls` is a class LIST — most rows in the tree look like `msg msg-a`. A + // producer emitting the token as one class among several is the same note. + act(() => { + ws.simulateMessage({ + type: 'chat_message', + data: { slot: 'slot-1', role: 'inject', cls: `msg ${RECONCILE_NOTE_CLS}`, content: 'a wrapped note', ts: '13.0' }, + }) + }) + await act(async () => {}) + + expect(voicePlaying()).toBe(true) + expect(statusKind('slot-1')).toBeUndefined() + + hook.unmount() + }) + + it('is not exempt for a different class that merely contains the token', async () => { + const { hook, ws } = await mount() + + act(() => { store.dispatch(setVoicePlaying(true)) }) + + // Membership, not substring: a longer class name is a different class. + act(() => { + ws.simulateMessage({ + type: 'chat_message', + data: { slot: 'slot-1', role: 'inject', cls: `msg ${RECONCILE_NOTE_CLS}-draft`, content: 'not a note', ts: '14.0' }, + }) + }) + await act(async () => {}) + + expect(voicePlaying()).toBe(false) + expect(statusKind('slot-1')).toBe('thinking') + + hook.unmount() + }) + + it('pins the sentinel spelling and the membership rule', () => { + // The tests above build their frames from the constant, so they would follow + // a rename silently. This pins the wire value the producer must emit. + expect(RECONCILE_NOTE_CLS).toBe('reconcile-note') + + expect(isReconcileNote('reconcile-note')).toBe(true) + expect(isReconcileNote('msg reconcile-note')).toBe(true) + expect(isReconcileNote(' msg reconcile-note ')).toBe(true) + expect(isReconcileNote('msg reconcile-note-draft')).toBe(false) + expect(isReconcileNote('msg msg-u')).toBe(false) + expect(isReconcileNote(undefined)).toBe(false) + expect(isReconcileNote('')).toBe(false) + }) +})