Skip to content

fix(terminal): hide console windows on Windows backend probes (#62734) - #66605

Closed
TheSmokeDev wants to merge 1 commit into
NousResearch:mainfrom
TheSmokeDev:fix/terminal-probe-no-window
Closed

fix(terminal): hide console windows on Windows backend probes (#62734)#66605
TheSmokeDev wants to merge 1 commit into
NousResearch:mainfrom
TheSmokeDev:fix/terminal-probe-no-window

Conversation

@TheSmokeDev

Copy link
Copy Markdown
Contributor

Closes #62734 (claimed there earlier today).

Problem

Three subprocess.run probe sites in tools/terminal_tool.py launch without CREATE_NO_WINDOW, so each flashes a visible console window on native Windows:

  • check_terminal_requirements() — the docker version probe and the apptainer/singularity --version probe (the two named in the issue)
  • _sudo_nopasswd_works() — the sudo -n true probe: same class, same file, and Windows 11 ships sudo.exe now. Included; say the word and I'll split it out.

Fix

The house pattern, verbatim: hermes_cli._subprocess_compat.windows_hide_flags() — already used at 8 call sites in agent/, cron/, gateway/, and whose docstring names version probes as the intended use. creationflags passed only when IS_WINDOWS; POSIX behavior byte-identical.

Verification (native Windows 11, CPython 3.13)

Four regression tests appended to tests/tools/test_terminal_requirements.py (existing conventions: module handle + monkeypatch, recorder stand-in for subprocess.run):

  • docker probe asserts creationflags on Windows
  • singularity probe asserts creationflags on Windows
  • POSIX asserts no creationflags (the compat module's no-damage guarantee)
  • sudo probe asserts creationflags on Windows

A/B: all four fail without the fix (stash check), full file 13 passed with it. ruff clean.

Note: running this file through scripts/run_tests.sh on native Windows requires the hermetic-env fix in #66496 (USERPROFILE passthrough — the file is uncollectable under the wrapper without it; receipt over there). Direct pytest + the wrapper-with-#66496 both green.

…search#62734)

The docker-version and apptainer/singularity-version probes in
check_terminal_requirements(), plus the sudo -n probe in
_sudo_nopasswd_works() (same class, same file — Windows 11 ships
sudo.exe), called subprocess.run without CREATE_NO_WINDOW, so each
probe flashed a visible console window on native Windows.

Apply the existing house pattern from hermes_cli._subprocess_compat
(windows_hide_flags(), already used at 8 call sites in agent/, cron/,
gateway/ — its docstring names version probes as the intended use):
creationflags passed only when IS_WINDOWS, byte-identical POSIX
behavior.

Four regression tests: each probe asserts creationflags on Windows,
plus a POSIX no-creationflags guarantee. All four fail without the
fix; full file 13 passed.
@alt-glitch alt-glitch added type/bug Something isn't working tool/terminal Terminal execution and process management platform/windows Native Windows-specific behavior or breakage P2 Medium — degraded but workaround exists needs-decision Awaiting maintainer decision before any implementation sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #62761 and #62734: #62761 already repairs the docker and singularity probes. This PR adds the separately useful Windows sudo probe, so it is a competing superset rather than a duplicate.

@TheSmokeDev

Copy link
Copy Markdown
Contributor Author

Good triage link. Credit where due: #62761 (@liuhao1024) was first to the docker/singularity probe sites, and since windows_hide_flags() returns 0 on non-Windows, both styles are POSIX-safe.

What this PR carries beyond #62761:

  1. The third probe site_sudo_nopasswd_works() runs sudo -n true in the same file, same class of flash, and Windows 11 ships sudo.exe now.
  2. Regression tests — four tests in tests/tools/test_terminal_requirements.py locking all three sites plus the POSIX no-creationflags guarantee (A/B verified: all four fail without the fix, 13/13 with it).
  3. Conditional-kwargs parity**({"creationflags": windows_hide_flags()} if IS_WINDOWS else {}), matching the 8 existing call sites in agent/, cron/, gateway/.

Either resolution works for me: take this as the superset, or if #62761 lands first I'll rebase this down to the sudo site + the test file on top of it. Maintainers' call.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for covering all three direct probe spawns. Current main still has unflagged subprocess.run calls for sudo, Docker, and Singularity at tools/terminal_tool.py:693, :2926, and :2932; windows_hide_flags() is the intended synchronous-version-probe helper (hermes_cli/_subprocess_compat.py:186-201).

Problems

  • The new Windows tests assert only that creationflags exists. Because they patch terminal_tool_module.IS_WINDOWS but not windows_hide_flags(), a non-Windows run can pass with the helper's real value of 0; an incorrect flag value would also pass. The existing house pattern asserts the exact 0x08000000 value in tests/test_windows_subprocess_no_window_flags.py:47-68.

Suggested changes

  • Stub terminal_tool_module.windows_hide_flags to 0x08000000 in each Windows-path test and assert that exact value for Docker, Singularity, and sudo. Keep the POSIX no-keyword assertion.

Automated hermes-sweeper review.

@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

needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: two terminal_tool.py backend probes spawn visible console windows (missing windows_hide_flags)

3 participants