Skip to content

Commit 39b68dd

Browse files
fix(stage-ui): robust spark:command result + TTS session isolation (#1949)
## Summary Two small, generic robustness fixes, split out of the Minecraft desktop integration (#1916) so they can land independently of that rework — as suggested in @shinohara-rin's review: > The generic fixes in this PR, like the TTS session cleanup and `spark:command` result handling, seem separable and worth keeping in a smaller PR. Both are in shared `stage-ui` runtime code and contain **no Minecraft-specific logic**. ## What's included - **`spark:command` result handling** (`tools/character/orchestrator/spark-command.ts`): `command.destinations` may be `undefined` — the channel sender (`stores/llm.ts` `sendSparkCommand`) deletes it to broadcast to all authenticated peers. The success message's `.join()` therefore surfaced `Cannot read properties of undefined (reading 'join')` back to the LLM even though the send had already succeeded. Guard it and report a broadcast instead. - **TTS session isolation** (`components/scenes/Stage.vue` `openTtsSession`): a completing/erroring session now only clears the module-level `currentSession` if it **is** that session. The previous code cleared it whenever any `stream-` session completed, which becomes unsafe once sessions exist that are not assigned to `currentSession` (e.g. one-off read-aloud sessions) — one of those finishing would null a still-active chat session and drop the rest of the reply. ## How tested ```bash pnpm -F @proj-airi/stage-ui exec vitest run src/tools/character/orchestrator/spark-command.test.ts # 9 passed (+1 new) pnpm -F @proj-airi/stage-ui typecheck # 0 errors pnpm exec eslint <changed files> # 0 problems ``` - Regression test for the broadcast-destinations result message. ## Context This is the first, low-risk slice of reworking the Minecraft↔desktop integration around the neutral Context Flow architecture (per the #1916 review). The Minecraft relay/read-aloud behavior will be reintroduced through a Minecraft-owned adapter in a follow-up. ---------
1 parent 4ff491a commit 39b68dd

4 files changed

Lines changed: 61 additions & 12 deletions

File tree

packages/stage-ui/src/components/scenes/Stage.vue

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,17 @@ function resolveSpeechTransport(providerId: string | null | undefined): SpeechTr
688688
}
689689
690690
function openTtsSession(): StageTtsSession {
691-
return createStageTtsSession<AudioBuffer>({
691+
// A session must only clear the module-level `currentSession` if it IS that session. The previous
692+
// code cleared it whenever any `stream-` session completed, which is unsafe once sessions exist that
693+
// are not assigned to `currentSession` (e.g. one-off read-aloud sessions): one of those finishing
694+
// would null a still-active chat session and drop the rest of the reply. Capture the session and
695+
// compare identity; the `stream-` guard is preserved so segmenter sessions still don't self-clear.
696+
let session: StageTtsSession | null = null
697+
const clearIfActive = () => {
698+
if (session && currentSession === session && session.intentId.startsWith('stream-'))
699+
currentSession = null
700+
}
701+
session = createStageTtsSession<AudioBuffer>({
692702
transport: resolveSpeechTransport(activeSpeechProvider.value),
693703
streaming: buildStreamingSnapshot,
694704
audioContext,
@@ -706,15 +716,14 @@ function openTtsSession(): StageTtsSession {
706716
model: activeSpeechModel.value,
707717
error: err,
708718
})
709-
if (currentSession?.intentId.startsWith('stream-'))
710-
currentSession = null
719+
clearIfActive()
711720
},
712721
onDone: () => {
713-
if (currentSession?.intentId.startsWith('stream-'))
714-
currentSession = null
722+
clearIfActive()
715723
},
716724
},
717725
})
726+
return session
718727
}
719728
720729
watch(latestStopRequest, (request) => {

packages/stage-ui/src/libs/speech/streaming-pipeline.test.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -287,11 +287,21 @@ describe('createStreamingTtsPipeline', () => {
287287
await server.startObserved
288288
handle.cancel()
289289

290-
await new Promise<void>((resolve) => {
291-
onDone.mockImplementation(() => resolve())
292-
setTimeout(resolve, 500)
293-
})
294-
295-
expect(cancelObserved).toBe(true)
290+
// ROOT CAUSE:
291+
//
292+
// This test was flaky on CI: asserting `cancelObserved` right after `onDone`
293+
// resolved raced the mock server's `message` event.
294+
// `cancel()` queues the cancel frame in the ws write buffer, then `terminate()`
295+
// defers `ws.close()` + `onDone()` by one macrotask (streaming-pipeline.ts) —
296+
// but the frame still has to cross a real loopback socket and be dispatched to
297+
// the server's `message` listener, which on a loaded runner can happen AFTER
298+
// the client-side `onDone` fired.
299+
//
300+
// We fixed this by polling for both observations instead of asserting
301+
// immediately after `onDone`.
302+
await vi.waitFor(() => {
303+
expect(cancelObserved).toBe(true)
304+
expect(onDone).toHaveBeenCalledTimes(1)
305+
}, { interval: 10, timeout: 1500 })
296306
})
297307
})

packages/stage-ui/src/tools/character/orchestrator/spark-command.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,4 +300,28 @@ describe('tools/character/orchestrator/spark-command', () => {
300300
expect(result).toContain('spark:command sent')
301301
expect(result).toContain(command.commandId)
302302
})
303+
304+
it('reports a broadcast without crashing when the channel sender clears destinations', async () => {
305+
// The real sendSparkCommand (stores/llm.ts) deletes command.destinations to broadcast to every
306+
// authenticated peer; the success message must not then call .join on undefined.
307+
const sendSparkCommand = vi.fn((command: { destinations?: unknown }) => {
308+
delete command.destinations
309+
})
310+
const tools = await createSparkCommandTool({ sendSparkCommand })
311+
312+
const result = await tools[0].execute({
313+
destinations: [],
314+
interrupt: 'soft',
315+
priority: 'normal',
316+
intent: 'action',
317+
ack: null,
318+
parentEventId: null,
319+
guidance: null,
320+
contexts: null,
321+
}, { messages: [], toolCallId: 'tool-call-id' })
322+
323+
expect(sendSparkCommand).toHaveBeenCalledOnce()
324+
expect(result).toContain('spark:command sent')
325+
expect(result).toContain('broadcast')
326+
})
303327
})

packages/stage-ui/src/tools/character/orchestrator/spark-command.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,13 @@ export async function createSparkCommandTool(options: CreateSparkCommandToolOpti
6565

6666
options.sendSparkCommand(command)
6767

68-
return `spark:command sent (${command.commandId}) to ${command.destinations.join(', ')}`
68+
// `destinations` may be undefined: the channel sender (stores/llm.ts sendSparkCommand) deletes
69+
// it to trigger broadcast-to-all-authenticated-peers. Guard the .join so we don't surface
70+
// "Cannot read properties of undefined (reading 'join')" back to the LLM after a successful send.
71+
const dests = Array.isArray(command.destinations) && command.destinations.length > 0
72+
? command.destinations.join(', ')
73+
: 'all authenticated peers (broadcast)'
74+
return `spark:command sent (${command.commandId}) to ${dests}`
6975
},
7076
}),
7177
]

0 commit comments

Comments
 (0)