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. ---------
This commit is contained in:
@@ -688,7 +688,17 @@ function resolveSpeechTransport(providerId: string | null | undefined): SpeechTr
|
||||
}
|
||||
|
||||
function openTtsSession(): StageTtsSession {
|
||||
return createStageTtsSession<AudioBuffer>({
|
||||
// A session must only clear the module-level `currentSession` if it IS that session. The previous
|
||||
// code cleared it whenever any `stream-` session completed, which is 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. Capture the session and
|
||||
// compare identity; the `stream-` guard is preserved so segmenter sessions still don't self-clear.
|
||||
let session: StageTtsSession | null = null
|
||||
const clearIfActive = () => {
|
||||
if (session && currentSession === session && session.intentId.startsWith('stream-'))
|
||||
currentSession = null
|
||||
}
|
||||
session = createStageTtsSession<AudioBuffer>({
|
||||
transport: resolveSpeechTransport(activeSpeechProvider.value),
|
||||
streaming: buildStreamingSnapshot,
|
||||
audioContext,
|
||||
@@ -706,15 +716,14 @@ function openTtsSession(): StageTtsSession {
|
||||
model: activeSpeechModel.value,
|
||||
error: err,
|
||||
})
|
||||
if (currentSession?.intentId.startsWith('stream-'))
|
||||
currentSession = null
|
||||
clearIfActive()
|
||||
},
|
||||
onDone: () => {
|
||||
if (currentSession?.intentId.startsWith('stream-'))
|
||||
currentSession = null
|
||||
clearIfActive()
|
||||
},
|
||||
},
|
||||
})
|
||||
return session
|
||||
}
|
||||
|
||||
watch(latestStopRequest, (request) => {
|
||||
|
||||
@@ -287,11 +287,21 @@ describe('createStreamingTtsPipeline', () => {
|
||||
await server.startObserved
|
||||
handle.cancel()
|
||||
|
||||
await new Promise<void>((resolve) => {
|
||||
onDone.mockImplementation(() => resolve())
|
||||
setTimeout(resolve, 500)
|
||||
})
|
||||
|
||||
expect(cancelObserved).toBe(true)
|
||||
// ROOT CAUSE:
|
||||
//
|
||||
// This test was flaky on CI: asserting `cancelObserved` right after `onDone`
|
||||
// resolved raced the mock server's `message` event.
|
||||
// `cancel()` queues the cancel frame in the ws write buffer, then `terminate()`
|
||||
// defers `ws.close()` + `onDone()` by one macrotask (streaming-pipeline.ts) —
|
||||
// but the frame still has to cross a real loopback socket and be dispatched to
|
||||
// the server's `message` listener, which on a loaded runner can happen AFTER
|
||||
// the client-side `onDone` fired.
|
||||
//
|
||||
// We fixed this by polling for both observations instead of asserting
|
||||
// immediately after `onDone`.
|
||||
await vi.waitFor(() => {
|
||||
expect(cancelObserved).toBe(true)
|
||||
expect(onDone).toHaveBeenCalledTimes(1)
|
||||
}, { interval: 10, timeout: 1500 })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -300,4 +300,28 @@ describe('tools/character/orchestrator/spark-command', () => {
|
||||
expect(result).toContain('spark:command sent')
|
||||
expect(result).toContain(command.commandId)
|
||||
})
|
||||
|
||||
it('reports a broadcast without crashing when the channel sender clears destinations', async () => {
|
||||
// The real sendSparkCommand (stores/llm.ts) deletes command.destinations to broadcast to every
|
||||
// authenticated peer; the success message must not then call .join on undefined.
|
||||
const sendSparkCommand = vi.fn((command: { destinations?: unknown }) => {
|
||||
delete command.destinations
|
||||
})
|
||||
const tools = await createSparkCommandTool({ sendSparkCommand })
|
||||
|
||||
const result = await tools[0].execute({
|
||||
destinations: [],
|
||||
interrupt: 'soft',
|
||||
priority: 'normal',
|
||||
intent: 'action',
|
||||
ack: null,
|
||||
parentEventId: null,
|
||||
guidance: null,
|
||||
contexts: null,
|
||||
}, { messages: [], toolCallId: 'tool-call-id' })
|
||||
|
||||
expect(sendSparkCommand).toHaveBeenCalledOnce()
|
||||
expect(result).toContain('spark:command sent')
|
||||
expect(result).toContain('broadcast')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -65,7 +65,13 @@ export async function createSparkCommandTool(options: CreateSparkCommandToolOpti
|
||||
|
||||
options.sendSparkCommand(command)
|
||||
|
||||
return `spark:command sent (${command.commandId}) to ${command.destinations.join(', ')}`
|
||||
// `destinations` may be undefined: the channel sender (stores/llm.ts sendSparkCommand) deletes
|
||||
// it to trigger broadcast-to-all-authenticated-peers. Guard the .join so we don't surface
|
||||
// "Cannot read properties of undefined (reading 'join')" back to the LLM after a successful send.
|
||||
const dests = Array.isArray(command.destinations) && command.destinations.length > 0
|
||||
? command.destinations.join(', ')
|
||||
: 'all authenticated peers (broadcast)'
|
||||
return `spark:command sent (${command.commandId}) to ${dests}`
|
||||
},
|
||||
}),
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user