fix: skip real AudioContext in Voice Mode onboarding preview under tests (build fix for vscode-engineering#3742) - #333858
Open
VS Code PR Bot (vscodebot-pr) wants to merge 3 commits into
Conversation
The onboarding voice-sample preview always built a real AudioContext and connected the analyser to context.destination, opening the host audio output device even when a test-supplied audio element was injected. In headless Electron test runs this device kept spinning with no producer, emitting endless `SyncReader::Read timed out` audio-glitch warnings and hanging the unit test task until timeout. Skip the best-effort analyser graph when an audioFactory override is provided; the sample still plays. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
VS Code PR Bot (vscodebot-pr)
September 1, 2026 20:15
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The functional change is narrowly test-scoped; only a minor comment-style issue remains.
Pull request overview
Prevents Voice Mode onboarding tests from opening a real audio output device.
Changes:
- Skips
AudioContextcreation when tests inject an audio element. - Preserves production waveform analysis.
File summaries
| File | Description |
|---|---|
voiceModeOnboarding.ts |
Bypasses the analyser graph for injected audio elements. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Megan Rogge (meganrogge)
marked this pull request as ready for review
September 1, 2026 20:31
Megan Rogge (meganrogge)
approved these changes
Sep 1, 2026
Megan Rogge (meganrogge)
enabled auto-merge (squash)
September 1, 2026 20:31
Dmitriy Vasyura (dmitrivMS)
approved these changes
Sep 1, 2026
roblourens
approved these changes
Sep 1, 2026
TylerLeonhardt
approved these changes
Sep 1, 2026
Sandeep Somavarapu (sandy081)
approved these changes
Sep 1, 2026
roblourens
approved these changes
Sep 2, 2026
Bhavya U (bhavyaus)
approved these changes
Sep 2, 2026
Anthony Kim (anthonykim1)
approved these changes
Sep 2, 2026
Martin Aeschlimann (aeschli)
approved these changes
Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Build failure
The
Run unit tests (Electron)task on the macOS insider stage hung and hit the task timeout right after theVoice Mode onboardingsuite. The runner emitted a burst of[WARNING:services/audio/sync_reader.cc] SyncReader::Read timed out, audio glitch count=...messages, then produced no further output for ~14 minutes until it was killed, so.build/logswas never written and the downstream publish/verify steps cascaded.Root cause
VoiceSamplePlayer.ensureAudio()insrc/vs/workbench/contrib/agentsVoice/browser/voiceModeOnboarding.tsalways constructs a realAudioContextand connects the analyser tocontext.destination:The onboarding tests inject a deterministic audio element via the
audioFactoryoption, but nothing suppresses the realAudioContext. Connecting the analyser tocontext.destinationopens the host's real audio output device. In headless Electron that device keeps polling for audio with no live producer, which is exactly what the repeatedSyncReader::Read timed outglitch warnings report. Combined with the sample-playback tests (clicking the playing voice...,previews the native voice...,swaps the chips...) that each trigger a preview, the open output stream stalls the runner and the whole test task times out.The
AudioContextgraph is best-effort waveform decoration only; the sample plays regardless. It was safe in interactive use but unsafe in headless CI, and there was no way for a test to opt out.How the fix works
When a caller supplies its own
audioFactory(only tests do),ensureAudio()now returns the injected element immediately and skips building theAudioContext/analyser graph. No real audio output device is opened, so theSyncReaderpolling loop never starts and the runner no longer stalls. Production code paths, which never passaudioFactory, are completely unchanged and still get the waveform analyser. The guard reuses the existingaudioFactoryfield, adds no new types or imports, and preserves all existing logging.Rollback evaluation
AudioContext/destinationgraph was introduced earlier (in the initial Voice Mode onboarding change) and is the mechanism of the hang; the change that immediately preceded the failing build reworked the voice test setup and surfaced the stall, but source inspection does not establish it as the code-level cause.@meganroggeValidation
Static source inspection only. The repository dependencies were not installed in this environment (
node_modulesabsent), sonpm run typecheck-clientand the Electron unit test task could not be run here. The change is a single early-return guard using an existing field with no new imports or types. Reviewers should run theagentsVoicebrowser tests (voiceModeOnboarding.test.ts) in Electron to confirm the suite no longer hangs and emits noSyncReader::Read timed outwarnings.Risk
Low. The behavioral change is limited to callers that pass an
audioFactoryoverride, which is only the tests. Interactive Voice Mode onboarding still constructs the analyser graph and renders the live waveform exactly as before.Recommended reviewer
Recommended owner:
@meganroggeFixes microsoft/vscode-engineering#3742