Skip to content

fix(qqbot): unlink temp wav on STT failure path - #23313

Closed
Zhekinmaksim wants to merge 1 commit into
NousResearch:mainfrom
Zhekinmaksim:fix/qqbot-stt-temp-leak
Closed

fix(qqbot): unlink temp wav on STT failure path#23313
Zhekinmaksim wants to merge 1 commit into
NousResearch:mainfrom
Zhekinmaksim:fix/qqbot-stt-temp-leak

Conversation

@Zhekinmaksim

Copy link
Copy Markdown
Contributor

_stt_voice_attachment writes the downloaded voice payload to a NamedTemporaryFile(delete=False) and then calls the configured self-hosted STT endpoint. Previously the temp WAV was unlinked only after _call_stt returned - if it raised httpx.HTTPStatusError, httpx.TransportError, or IOError, the outer except swallowed the error but left the file behind. In a long-running bot, transient STT outages turn into a steady leak of one WAV per failed transcription in $TMPDIR.

Wrap _call_stt in try/finally so wav_path is always unlinked, matching the silk_path cleanup pattern already used a few lines below in _convert_audio_to_wav_file.

Test plan

Added two regression tests in TestVoiceAttachmentTempCleanup:

  • test_temp_wav_cleaned_up_on_stt_failure - asserts the temp file no longer exists after _call_stt raises httpx.HTTPStatusError. Fails on current main, passes with this patch.
  • test_temp_wav_cleaned_up_on_stt_success - sanity check that the happy path still cleans up.

Full tests/gateway/test_qqbot.py: 144 passed, 2 unrelated failures (test_connect_uses_redirect_guard_hook, test_open_ws_honors_proxy_env) that fail on unmodified main as well due to local httpx mock setup.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery platform/qqbot QQ Bot adapter labels May 10, 2026
@Zhekinmaksim
Zhekinmaksim force-pushed the fix/qqbot-stt-temp-leak branch from 07c6a0c to 5c90361 Compare June 3, 2026 16:18
@Zhekinmaksim

Copy link
Copy Markdown
Contributor Author

Quick bump on this PR. The cleanup is still current from my side and should be ready for review if it is still relevant.

@Zhekinmaksim

Copy link
Copy Markdown
Contributor Author

Following up on this one as well. The PR is still technically clean from my side: all recorded CI checks on the branch are green, it is still mergeable, and I do not see any review comments or requested changes attached to it. If the fix is still relevant, this looks like it only needs maintainer review/decision rather than more code work from me.

@Zhekinmaksim

Copy link
Copy Markdown
Contributor Author

Hi, gentle follow-up on this PR. The cleanup is still current from my side, and I would appreciate a review when convenient if it is still relevant.

@teknium1

Copy link
Copy Markdown
Contributor

Merged into main via consolidated salvage PR #73515 (merge c911a5f10f). Your QQ temp-WAV cleanup fix (always unlink, not just on the happy path) was cherry-picked with your authorship.

Your contribution is credited to you in git history. Thank you! Closing this PR as merged-via-salvage.

@teknium1 teknium1 closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have platform/qqbot QQ Bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants