Skip to content

fix: add creationflags to remaining tts_tool subprocess calls - #65667

Closed
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/tts-tool-remaining-creationflags
Closed

fix: add creationflags to remaining tts_tool subprocess calls#65667
AlexFucuson9 wants to merge 1 commit into
NousResearch:mainfrom
AlexFucuson9:fix/tts-tool-remaining-creationflags

Conversation

@AlexFucuson9

Copy link
Copy Markdown
Contributor

Summary

Three subprocess calls in tools/tts_tool.py were missing CREATE_NO_WINDOW (0x08000000), causing console window flashes on Windows:

  1. Line 731taskkill process tree termination (_terminate_command_tts_process_tree)
  2. Line 2015 — NeuTTS synthesis call
  3. Line 2094 — Piper voice download call

All three have stdin=DEVNULL and capture_output=True, so they are non-interactive. windows_hide_flags() is already imported at line 55.

Context

PR #64081 previously covered tts_tool.py lines 2002 and 2087 (ffmpeg conversion calls), but missed these three calls. This PR closes the remaining gap.

Changes

  • tools/tts_tool.py: +3 lines, -1 line

Test Plan

  • Unit tests pass
  • Verified on Windows: no console flash on taskkill, NeuTTS synthesis, or Piper download

Three subprocess calls in tts_tool.py were missing CREATE_NO_WINDOW:
- Line 731: taskkill process tree termination
- Line 2015: NeuTTS synthesis call
- Line 2094: Piper voice download call

All three have stdin=DEVNULL and capture_output=True, so they are
non-interactive. windows_hide_flags() is already imported at line 55.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets tool/tts Text-to-speech and transcription platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 16, 2026

@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: Comment

Overview

  • Fix: add creationflags to remaining tts_tool subprocess calls
  • 3 additions, 1 deletion

Assessment

  • Windows console flash prevention fix
  • No security concerns

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the three remaining direct calls; all three are still missing windows_hide_flags() on current main at tools/tts_tool.py:731, :2015, and :2094.

Problems

  • The same file has one additional command-provider launch at tools/tts_tool.py:785: its Popen(..., shell=True) receives only CREATE_NEW_PROCESS_GROUP, while hermes_cli/_subprocess_compat.py:186-201 defines windows_hide_flags() for synchronous captured-output spawns. This remains a Windows console-flash path.
  • The patch adds no regression tests for its three changed calls. Existing coverage at tests/test_windows_subprocess_no_window_flags.py:304-320 covers only ffmpeg conversion.

Suggested changes

  • Preserve the command-provider process-group flag and combine it with windows_hide_flags() at tools/tts_tool.py:785.
  • Add mocked assertions for the taskkill, NeuTTS, Piper, and command-provider spawn kwargs.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 18, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing after the class-level fix in PR #70205 (commit 0dbf639): Windows console flashes were caused by Hermes daemons running console-less (pythonw / DETACHED_PROCESS parents), forcing every console-subsystem child to allocate its own visible window. Main now launches every daemon (gateway, Scheduled Task, UAC handoff, dashboard, desktop backend) with a hidden console that all descendants inherit — so the spawn sites this PR flags no longer have a reachable flash under any shipped launch path (interactive terminals never flashed; children inherit the visible console there).

The sites you patched were genuinely unflagged, and the diagnosis was sound against the old launch topology — the ground just moved under it. Rather than keep growing per-site flags across an unbounded set of leaf spawns, we're standardizing on the parent-console fix. If a flash reappears on current main under a shipped launch path, that's a new bug — please file it with the flashing process name.

Thanks for the contribution and the Windows attention; sorry this one got mooted at the root.

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

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows 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