From 21a6c98451afc86e15a0200796136da5c6229a56 Mon Sep 17 00:00:00 2001 From: lpaiu-cs Date: Sun, 28 Jun 2026 07:06:27 +0900 Subject: [PATCH 1/3] fix(windows): route the two gateway probes #53829's sweep missed through the chokepoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #53791/#53829's console-spawn rule exempts output-captured calls — right in general, but the desktop gateway runs under console-less pythonw.exe, where a captured console child still allocates (and flashes) a new console. Two sites slipped the sweep: - hermes_cli/copilot_auth.py (gh auth token): the argv is a variable, so the checker's literal-argv rule can't see it's gh. Lint-invisible -> added a regression test as its only guard. - tui_gateway/server.py (Cmd-P finder git rev-parse / ls-files): tui_gateway isn't in the checker's --all roots, so these were never scanned. Both now go through _subprocess_compat.run (the #53810 chokepoint), matching the convention #53829 used for its other sites. See #52310 / canonical #42544. Co-Authored-By: Claude Opus 4.8 --- hermes_cli/copilot_auth.py | 9 +++++++- tests/hermes_cli/test_copilot_auth.py | 30 ++++++++++++++++++++++++++- tui_gateway/server.py | 8 +++++-- 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/hermes_cli/copilot_auth.py b/hermes_cli/copilot_auth.py index e6f63a1557c9..9f651a0523d3 100644 --- a/hermes_cli/copilot_auth.py +++ b/hermes_cli/copilot_auth.py @@ -27,6 +27,8 @@ from pathlib import Path from typing import Optional +from hermes_cli import _subprocess_compat + logger = logging.getLogger(__name__) # OAuth device code flow constants (same client ID as opencode/Copilot CLI) @@ -135,7 +137,12 @@ def _try_gh_cli_token() -> Optional[str]: if hostname: cmd += ["--hostname", hostname] try: - result = subprocess.run( + # Route through the _subprocess_compat chokepoint so the gh probe + # doesn't flash a console window when spawned from the windowless + # desktop gateway (pythonw.exe). The footgun checker can't catch + # this site itself — the program (cmd) is a variable, not a literal + # argv, so its console-spawn rule can't see it's `gh`. See #52310. + result = _subprocess_compat.run( cmd, capture_output=True, text=True, diff --git a/tests/hermes_cli/test_copilot_auth.py b/tests/hermes_cli/test_copilot_auth.py index 3d0b0bdeb722..52a48f7762c5 100644 --- a/tests/hermes_cli/test_copilot_auth.py +++ b/tests/hermes_cli/test_copilot_auth.py @@ -1,7 +1,7 @@ """Tests for hermes_cli.copilot_auth — Copilot token validation and resolution.""" import pytest -from unittest.mock import patch +from unittest.mock import patch, MagicMock class TestTokenValidation: @@ -199,3 +199,31 @@ def test_copilot_env_vars_order_matches_docs(self): assert copilot.api_key_env_vars == ( "COPILOT_GITHUB_TOKEN", "GH_TOKEN", "GITHUB_TOKEN" ) + + +class TestGhCliTokenHidesConsole: + """The `gh auth token` fallback must route through the _subprocess_compat + chokepoint so it doesn't flash a console window when spawned from the + windowless desktop gateway (pythonw.exe). The footgun linter can't guard + this site — the argv (`cmd`) is a variable, not a literal, so its + console-spawn rule can't see it's `gh` — so this test is the only + regression guard. See #52310. + """ + + def test_try_gh_cli_token_routes_through_chokepoint(self, monkeypatch): + from hermes_cli import copilot_auth + + called = {} + + def fake_run(cmd, **kwargs): + called["cmd"] = cmd + return MagicMock(returncode=0, stdout="gho_token_from_gh\n") + + # If the code regressed to a raw subprocess.run, `called` stays empty. + monkeypatch.setattr(copilot_auth._subprocess_compat, "run", fake_run) + monkeypatch.setattr(copilot_auth, "_gh_cli_candidates", lambda: ["gh"]) + + token = copilot_auth._try_gh_cli_token() + + assert token == "gho_token_from_gh" + assert called.get("cmd", [])[:3] == ["gh", "auth", "token"] diff --git a/tui_gateway/server.py b/tui_gateway/server.py index ec61aed6d57e..7e7e2e12a16a 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -24,6 +24,7 @@ set_hermes_home_override, ) from hermes_cli.env_loader import load_hermes_dotenv +from hermes_cli import _subprocess_compat from utils import is_truthy_value from tui_gateway import git_probe from tui_gateway.transport import ( @@ -11619,7 +11620,10 @@ def _list_repo_files(root: str) -> list[str]: files: list[str] = [] try: - top_result = subprocess.run( + # _subprocess_compat.run hides the console window: this git probe runs + # from the windowless desktop gateway (pythonw.exe), where capturing + # output does NOT prevent a new console from being allocated. See #52310. + top_result = _subprocess_compat.run( ["git", "-C", root, "rev-parse", "--show-toplevel"], capture_output=True, timeout=2.0, @@ -11628,7 +11632,7 @@ def _list_repo_files(root: str) -> list[str]: ) if top_result.returncode == 0: top = top_result.stdout.decode("utf-8", "replace").strip() - list_result = subprocess.run( + list_result = _subprocess_compat.run( [ "git", "-C", From 0cd2a9661291363c2c9a9e2ec69934b9e69dbd4a Mon Sep 17 00:00:00 2001 From: lpaiu-cs Date: Sun, 28 Jun 2026 07:06:27 +0900 Subject: [PATCH 2/3] fix(windows): add tui_gateway to the footgun checker's --all roots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The console-spawn guard (#53791/#53829) scans a fixed package list, and tui_gateway/ was absent — so the gateway's own git probes were never linted (this is why server.py slipped). Adding it makes the guard cover the gateway package. Surfaces two pre-existing hasattr-guarded signal handlers in entry.py; marked '# windows-footgun: ok' (false positives — guard is on the line above, which the line-based scanner can't see). Co-Authored-By: Claude Opus 4.8 --- scripts/check-windows-footguns.py | 1 + tui_gateway/entry.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/check-windows-footguns.py b/scripts/check-windows-footguns.py index 35a9b4103d55..d8f98c40d1f7 100644 --- a/scripts/check-windows-footguns.py +++ b/scripts/check-windows-footguns.py @@ -836,6 +836,7 @@ def main(argv: list[str]) -> int: roots = [ REPO_ROOT / "hermes_cli", REPO_ROOT / "gateway", + REPO_ROOT / "tui_gateway", REPO_ROOT / "tools", REPO_ROOT / "cron", REPO_ROOT / "agent", diff --git a/tui_gateway/entry.py b/tui_gateway/entry.py index 8b6b7539f462..bd275bd70768 100644 --- a/tui_gateway/entry.py +++ b/tui_gateway/entry.py @@ -169,11 +169,11 @@ def _hard_exit() -> None: # ``hermes --tui``) imports cleanly there. SIGBREAK (Windows' Ctrl+Break) # is installed when available as a weaker equivalent of SIGHUP. if hasattr(signal, "SIGPIPE"): - signal.signal(signal.SIGPIPE, signal.SIG_IGN) + signal.signal(signal.SIGPIPE, signal.SIG_IGN) # windows-footgun: ok (hasattr-guarded above) if hasattr(signal, "SIGTERM"): signal.signal(signal.SIGTERM, _log_signal) if hasattr(signal, "SIGHUP"): - signal.signal(signal.SIGHUP, _log_signal) + signal.signal(signal.SIGHUP, _log_signal) # windows-footgun: ok (hasattr-guarded above) elif hasattr(signal, "SIGBREAK"): # Windows-only: Ctrl+Break in a console window delivers SIGBREAK. # Route it through the same handler so kills are diagnosable. From 1955d02425c9e320a11a96c092c0d7c8ceeec2c7 Mon Sep 17 00:00:00 2001 From: lpaiu-cs Date: Sun, 28 Jun 2026 13:45:35 +0900 Subject: [PATCH 3/3] fix(windows): use surviving windows_hide_flags() after the #53853 revert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first cut routed these probes through _subprocess_compat.run/popen (the #53810 chokepoint), but #53853 reverted #53791/#53810/#53829 — so those calls would now AttributeError on main (the wrappers are gone; windows_hide_flags() survived). Switch the two sites to pass creationflags=windows_hide_flags() directly, the same surviving-helper approach #53892 took post-revert. Also drop the now-moot footgun changes: the console-spawn rule was reverted, so adding tui_gateway to its --all roots no longer buys anything (and would only surface unrelated, hasattr-guarded signal handlers in entry.py). Net change is now just the two gateway probe sites + the test, which asserts the no-window flag is passed. Co-Authored-By: Claude Opus 4.8 --- hermes_cli/copilot_auth.py | 14 ++++++++------ scripts/check-windows-footguns.py | 1 - tests/hermes_cli/test_copilot_auth.py | 24 ++++++++++++++---------- tui_gateway/entry.py | 4 ++-- tui_gateway/server.py | 14 +++++++++----- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/hermes_cli/copilot_auth.py b/hermes_cli/copilot_auth.py index 9f651a0523d3..195c12f6e52e 100644 --- a/hermes_cli/copilot_auth.py +++ b/hermes_cli/copilot_auth.py @@ -137,17 +137,19 @@ def _try_gh_cli_token() -> Optional[str]: if hostname: cmd += ["--hostname", hostname] try: - # Route through the _subprocess_compat chokepoint so the gh probe - # doesn't flash a console window when spawned from the windowless - # desktop gateway (pythonw.exe). The footgun checker can't catch - # this site itself — the program (cmd) is a variable, not a literal - # argv, so its console-spawn rule can't see it's `gh`. See #52310. - result = _subprocess_compat.run( + # gh runs from the windowless desktop gateway (pythonw.exe), where a + # captured console child still allocates — and flashes — a console + # window. windows_hide_flags() is CREATE_NO_WINDOW on win32, 0 on + # POSIX. (The #53810 `_subprocess_compat.run` chokepoint was rolled + # back in the #53853 revert, so this uses the surviving helper + # directly.) See #52310. + result = subprocess.run( cmd, capture_output=True, text=True, timeout=5, env=clean_env, + creationflags=_subprocess_compat.windows_hide_flags(), ) except (FileNotFoundError, subprocess.TimeoutExpired) as exc: logger.debug("gh CLI token lookup failed (%s): %s", gh_path, exc) diff --git a/scripts/check-windows-footguns.py b/scripts/check-windows-footguns.py index d8f98c40d1f7..35a9b4103d55 100644 --- a/scripts/check-windows-footguns.py +++ b/scripts/check-windows-footguns.py @@ -836,7 +836,6 @@ def main(argv: list[str]) -> int: roots = [ REPO_ROOT / "hermes_cli", REPO_ROOT / "gateway", - REPO_ROOT / "tui_gateway", REPO_ROOT / "tools", REPO_ROOT / "cron", REPO_ROOT / "agent", diff --git a/tests/hermes_cli/test_copilot_auth.py b/tests/hermes_cli/test_copilot_auth.py index 52a48f7762c5..bcdc1c2662da 100644 --- a/tests/hermes_cli/test_copilot_auth.py +++ b/tests/hermes_cli/test_copilot_auth.py @@ -202,28 +202,32 @@ def test_copilot_env_vars_order_matches_docs(self): class TestGhCliTokenHidesConsole: - """The `gh auth token` fallback must route through the _subprocess_compat - chokepoint so it doesn't flash a console window when spawned from the - windowless desktop gateway (pythonw.exe). The footgun linter can't guard - this site — the argv (`cmd`) is a variable, not a literal, so its - console-spawn rule can't see it's `gh` — so this test is the only - regression guard. See #52310. + """The `gh auth token` fallback must pass CREATE_NO_WINDOW so it doesn't + flash a console window when spawned from the windowless desktop gateway + (pythonw.exe). The footgun checker can't guard this site — the argv (`cmd`) + is a variable, not a literal, so its console-spawn rule can't see it's `gh` + — so this test is the only regression guard. See #52310. """ - def test_try_gh_cli_token_routes_through_chokepoint(self, monkeypatch): + def test_try_gh_cli_token_passes_no_window_flag(self, monkeypatch): + import subprocess from hermes_cli import copilot_auth called = {} def fake_run(cmd, **kwargs): called["cmd"] = cmd + called["kwargs"] = kwargs return MagicMock(returncode=0, stdout="gho_token_from_gh\n") - # If the code regressed to a raw subprocess.run, `called` stays empty. - monkeypatch.setattr(copilot_auth._subprocess_compat, "run", fake_run) + monkeypatch.setattr(subprocess, "run", fake_run) monkeypatch.setattr(copilot_auth, "_gh_cli_candidates", lambda: ["gh"]) token = copilot_auth._try_gh_cli_token() assert token == "gho_token_from_gh" - assert called.get("cmd", [])[:3] == ["gh", "auth", "token"] + assert called["cmd"][:3] == ["gh", "auth", "token"] + assert ( + called["kwargs"]["creationflags"] + == copilot_auth._subprocess_compat.windows_hide_flags() + ) diff --git a/tui_gateway/entry.py b/tui_gateway/entry.py index bd275bd70768..8b6b7539f462 100644 --- a/tui_gateway/entry.py +++ b/tui_gateway/entry.py @@ -169,11 +169,11 @@ def _hard_exit() -> None: # ``hermes --tui``) imports cleanly there. SIGBREAK (Windows' Ctrl+Break) # is installed when available as a weaker equivalent of SIGHUP. if hasattr(signal, "SIGPIPE"): - signal.signal(signal.SIGPIPE, signal.SIG_IGN) # windows-footgun: ok (hasattr-guarded above) + signal.signal(signal.SIGPIPE, signal.SIG_IGN) if hasattr(signal, "SIGTERM"): signal.signal(signal.SIGTERM, _log_signal) if hasattr(signal, "SIGHUP"): - signal.signal(signal.SIGHUP, _log_signal) # windows-footgun: ok (hasattr-guarded above) + signal.signal(signal.SIGHUP, _log_signal) elif hasattr(signal, "SIGBREAK"): # Windows-only: Ctrl+Break in a console window delivers SIGBREAK. # Route it through the same handler so kills are diagnosable. diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 7e7e2e12a16a..6fea17a0a77f 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -11620,19 +11620,22 @@ def _list_repo_files(root: str) -> list[str]: files: list[str] = [] try: - # _subprocess_compat.run hides the console window: this git probe runs - # from the windowless desktop gateway (pythonw.exe), where capturing - # output does NOT prevent a new console from being allocated. See #52310. - top_result = _subprocess_compat.run( + # This git probe runs from the windowless desktop gateway (pythonw.exe), + # where capturing output does NOT stop a new console from being + # allocated (and flashing). windows_hide_flags() = CREATE_NO_WINDOW on + # win32, 0 on POSIX. (#53810's chokepoint was reverted in #53853, so we + # use the surviving helper directly.) See #52310. + top_result = subprocess.run( ["git", "-C", root, "rev-parse", "--show-toplevel"], capture_output=True, timeout=2.0, check=False, stdin=subprocess.DEVNULL, + creationflags=_subprocess_compat.windows_hide_flags(), ) if top_result.returncode == 0: top = top_result.stdout.decode("utf-8", "replace").strip() - list_result = _subprocess_compat.run( + list_result = subprocess.run( [ "git", "-C", @@ -11647,6 +11650,7 @@ def _list_repo_files(root: str) -> list[str]: timeout=2.0, check=False, stdin=subprocess.DEVNULL, + creationflags=_subprocess_compat.windows_hide_flags(), ) if list_result.returncode == 0: for p in list_result.stdout.decode("utf-8", "replace").split("\0"):