Skip to content

fix(desktop): guard voice loop during playback - #55581

Closed
fabianxagent wants to merge 1 commit into
NousResearch:mainfrom
fabianxagent:fix/desktop-voice-playback-guard
Closed

fix(desktop): guard voice loop during playback#55581
fabianxagent wants to merge 1 commit into
NousResearch:mainfrom
fabianxagent:fix/desktop-voice-playback-guard

Conversation

@fabianxagent

@fabianxagent fabianxagent commented Jun 30, 2026

Copy link
Copy Markdown

Summary

  • prevent the ordinary desktop voice recorder from re-arming while shared TTS playback is preparing or speaking, without disabling the dedicated playback barge-in monitor
  • propagate rejected voice transcript submits back into both normal and captured-barge voice paths so they do not enter a stale thinking state
  • preserve the pending microphone restart until the session is idle and playback has ended
  • add regression coverage for playback-gated mic start, rejected normal submits, and rejected captured-barge submits

Root cause

The ordinary voice-conversation recorder only gated microphone re-arm on its local conversation status and session busy state. It did not observe the shared voice playback state, and it cleared pendingStartRef before all start guards had passed. A playback overlap could therefore either reopen the normal recorder too early or consume the pending restart without retrying it.

The composer submit contract returns false when a prompt is rejected, but submitVoiceTurn() discarded that result and the voice loop unconditionally entered thinking. Current main also has a separate captured-utterance path for streaming TTS barge-in; that sibling path had the same rejected-submit behavior.

This update keeps the new monitorSpeechDuringPlayback / streaming speech path intact. The playback guard applies only to the ordinary recorder restart, while the dedicated barge-in monitor continues to own microphone capture during playback.

Testing

  • npm --workspace apps/desktop run test:ui -- src/app/chat/composer/hooks/use-voice-conversation.test.tsx (3 passed)
  • npm --workspace apps/desktop run test:ui (243 files, 2052 passed, 1 skipped)
  • npm --workspace apps/desktop run typecheck
  • targeted ESLint and Prettier checks for the three changed files
  • npm --workspace apps/desktop run build

Notes

There is an adjacent open PR (#54067) that improves voice-conversation latency and TTS gaps. This PR remains narrower: it guards the ordinary mic re-arm and keeps rejected voice submits out of the response-waiting loop while preserving the newer streaming/barge-in implementation on main.

@fabianxagent
fabianxagent force-pushed the fix/desktop-voice-playback-guard branch 2 times, most recently from d181cc9 to 63a9e08 Compare June 30, 2026 10:42
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) tool/tts Text-to-speech and transcription labels Jun 30, 2026
@fabianxagent
fabianxagent marked this pull request as ready for review June 30, 2026 10:42

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Well-designed fix for the desktop voice loop restarting during active playback. The onSubmit return type change to boolean | void allows the caller to signal rejection, and the isVoicePlaybackActive() guard prevents the mic from restarting while audio is playing. Good test coverage.

Looks Good

  • The accepted === false check correctly resets voice state without entering thinking
  • Multiple guard points (isVoicePlaybackActive()) prevent mic restart during playback
  • The pendingStartRef reset moved after guards is correct ordering
  • Test cases cover both the playback-wait and submit-rejection paths

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused desktop voice-loop fix. Current main still has both reported failure paths: use-voice-conversation.ts:192-194 starts listening without checking shared playback, while :169-172 unconditionally enters thinking after submission. The submit pipeline can reject a normal prompt while busy at use-prompt-actions/submit.ts:114-115, so propagating false is necessary.

The shared state already models both preparing and speaking as active (lib/voice-playback.ts:153-154), and the proposed store subscription makes the loop retry when that state returns to idle. The added focused tests cover playback-gated microphone start and rejected submission handling. The target implementation files are unchanged from the composer-voice extraction through current HEAD, so this remains a narrowly scoped salvage.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@fabianxagent
fabianxagent force-pushed the fix/desktop-voice-playback-guard branch from 63a9e08 to b6fb923 Compare July 24, 2026 08:01
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by the merged voice playback/barge-in rework #69511 (93e9061f15, merged 2026-07-22) plus #69602: the desktop voice stack was rewritten around speech-stream sessions with barge-in capture and native resume-listening, and the code this PR patches no longer exists. Playback-time voice-loop guarding is native to the new stack — thanks for the guard work.

If anything in this area still misbehaves on current main, please open a fresh PR against the new stack.

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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages tool/tts Text-to-speech and transcription type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants