Skip to content

fix(desktop): stop voice status leaking across session switch (#46194)#46279

Open
harjothkhara wants to merge 1 commit into
NousResearch:mainfrom
harjothkhara:fix/desktop-voice-leak-session-switch-46194
Open

fix(desktop): stop voice status leaking across session switch (#46194)#46279
harjothkhara wants to merge 1 commit into
NousResearch:mainfrom
harjothkhara:fix/desktop-voice-leak-session-switch-46194

Conversation

@harjothkhara

Copy link
Copy Markdown
Contributor

What does this PR do?

Stops the desktop composer's voice status from leaking across a session switch. After switching sessions, the prior session's in-flight voice conversation (listening) and the global read-aloud playback status ($voicePlaybackPreparing audio / Speaking) were not reset, so session B's status area showed session A's carried-over voice state.

Both switch paths are handled:

  • Warm switch (composer stays mounted): a colocated hook, useEndVoiceOnSessionSwitch, fires on a real session change and tears the voice state down — it disables the conversation (so the idle effect can't auto-restart listening under the new session) and calls conversation.end() (cancels recording, stops playback). The direct end() is load-bearing for the read-aloud-only case, where flipping voiceConversationActive is a no-op and won't trigger the conversation hook's own enabled→false teardown.
  • Cold switch (resuming a session with no warm runtime unmounts the composer): the component-local conversation state dies with the unmount, but the module-level read-aloud singleton survives, so useStopVoicePlaybackOnUnmount stops it on teardown.

The switch guard skips the initial mount and the null→id persist of the session already in view (mirrors the composer's existing placeholder-reset), treating undefined and null as the same "no session". Leaving a session (id→null) and moving between two real sessions both count as a switch.

Related Issue

#46194 (scoped — see below; the issue should stay open for the remaining async leak).

This fixes the displayed voice-status slice only. The other symptoms named in the issue are already handled or out of scope here:

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • New apps/desktop/src/app/chat/composer/hooks/use-voice-session-reset.tsuseEndVoiceOnSessionSwitch (warm) + useStopVoicePlaybackOnUnmount (cold).
  • Wired both into apps/desktop/src/app/chat/composer/index.tsx (the ChatBar composer).
  • use-voice-session-reset.test.ts — 9 cases.

How to Test

npm --workspace apps/desktop run test:ui -- src/app/chat/composer/hooks/use-voice-session-reset.test.ts

Manual: start a read-aloud (or voice conversation) in session A, switch to session B before it settles — B's composer no longer shows A's Preparing audio/Speaking/listening state. Repeat with a cold resume of B (no warm runtime) to exercise the unmount path.

Checklist

  • My code follows the project's style guidelines
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally (tsc + eslint clean; only the pre-existing trigger-popover.test.tsx @assistant-ui/tap packaging failure is unrelated)
  • Tested on platform: macOS

🤖 Generated with Claude Code

@harjothkhara harjothkhara marked this pull request as ready for review June 14, 2026 22:15
@liuhao1024

Copy link
Copy Markdown
Contributor

✅ Verified — Desktop voice status leaking across session switch

Reviewed the diff for correctness of the session-switch detection logic and teardown coverage.

  • Warm switch guard (useEndVoiceOnSessionSwitch): Correctly skips the initial mount (no previous session) and the null → id transition (session already in view). Treats undefined and null as the same "no session" state to avoid false-positive switches at startup. Fires once per real session-to-session transition — confirmed by test coverage for A→B, B→C, C→A, and id→null.
  • Cold switch guard (useStopVoicePlaybackOnUnmount): Cleanup function in useEffect([], []) stops module-level read-aloud playback on unmount. Correct pattern for singleton teardown.
  • Callback freshness: onSwitch is in the useEffect dependency array, so stale closures are avoided. Test confirms the latest callback is used.
  • Integration: Hooks wired into ChatBar component alongside the existing voice conversation teardown — the comment explains the load-bearing conversation.end() call for the read-aloud-only case.

Clean fix with thorough test coverage. No issues found.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 14, 2026
@harjothkhara

Copy link
Copy Markdown
Contributor Author

@teknium1 — green/mergeable fix for #46194: the desktop composer's voice status (in-flight listening + the global read-aloud $voicePlayback) was leaking across a session switch, so session B showed session A's carried-over voice state. Covers both paths — warm switch (colocated useEndVoiceOnSessionSwitch teardown) and cold switch (unmount stop for the module-level read-aloud singleton) — scoped to the desktop composer. Verified clean by @liuhao1024. A review when convenient would be appreciated.

Copy link
Copy Markdown
Contributor Author

@teknium1 @OutThisLife gentle review ping here. This was opened on June 14, so it is about 6 days old now.

Importance: this is a focused desktop session-isolation bug fix. It stops voice status from leaking across a session switch, with dedicated hook coverage, so the UI does not show stale recording/voice state for the wrong conversation.

@harjothkhara harjothkhara force-pushed the fix/desktop-voice-leak-session-switch-46194 branch from 2e8105c to 0794709 Compare June 26, 2026 04:43
@alt-glitch alt-glitch added comp/desktop Electron desktop app (apps/desktop/*) and removed comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 26, 2026
@konsisumer

Copy link
Copy Markdown
Contributor

I opened #61448 to consolidate this with related work across several related issues — it aims to fold in your change alongside the others so the maintainer can land one clean PR. Flagging for visibility; no action needed from you, and your PR stays open.

…search#46194)

The composer's voice state surfaced under the wrong session after a
switch: its in-flight voice conversation (listening) and the global
read-aloud playback status (`$voicePlayback` → `Preparing audio` /
`Speaking`) were not reset, so session B's status area showed session
A's carried-over voice state.

Handle both switch paths:

- Warm switch (composer stays mounted): a colocated hook,
  useEndVoiceOnSessionSwitch, fires on a real session change and tears
  the voice state down — it disables the conversation (so the idle
  effect can't auto-restart listening under the new session) and calls
  conversation.end() (cancels recording, stops playback). The direct
  end() is load-bearing for the read-aloud-only case, where flipping
  voiceConversationActive is a no-op and won't trigger the conversation
  hook's own enabled→false teardown.
- Cold switch (resuming a session with no warm runtime unmounts the
  composer): the component-local conversation state dies with the
  unmount, but the module-level read-aloud singleton survives, so
  useStopVoicePlaybackOnUnmount stops it on teardown.

The guard skips the initial mount and the null→id persist of the
session already in view (mirrors the composer's placeholder-reset),
treating undefined and null as the same "no session"; leaving a session
(id→null) and moving between two real sessions both count as a switch.

Scope: this fixes the *displayed* voice-status leak only. The queued
follow-up state (NousResearch#45414/NousResearch#43959) and transcript path are already keyed
per session on main, and the second-hand "wrong chat contents" report
is not reproducible there. A separate async leak remains and is left
for follow-up (NousResearch#46194 stays open): a dictation or in-flight voice turn
whose transcribe/submit resolves after the switch can still land in the
newly active session (the stale-submit cross-binding via
ensureSessionState → rewriteOptimistic).

Tests: use-voice-session-reset.test.ts covers mount / unchanged /
switch / null→id / id→null / undefined↔null / latest-callback and the
cold-path unmount teardown (renderHook).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@harjothkhara harjothkhara force-pushed the fix/desktop-voice-leak-session-switch-46194 branch from 0794709 to c4157b4 Compare July 10, 2026 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants