fix(windows): close two console-flash sites #53829's sweep missed (gateway gh/git probes) - #53844
fix(windows): close two console-flash sites #53829's sweep missed (gateway gh/git probes)#53844lpaiu-cs wants to merge 3 commits into
Conversation
… missed through the chokepoint NousResearch#53791/NousResearch#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 NousResearch#53810 chokepoint), matching the convention NousResearch#53829 used for its other sites. See NousResearch#52310 / canonical NousResearch#42544. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The console-spawn guard (NousResearch#53791/NousResearch#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 <noreply@anthropic.com>
Same Windows console-flash family as merged #53829 / #53810 / #53791 and the open #53123 / #53397 / #53781 -- but these two spawn sites ( |
|
Thanks — agreed, related not duplicate. To confirm the split: #53791 / #53810 / #53829 covered the rest of cluster B, including #53123, which I've since closed in favor of this PR (3 of its 5 sites were swept by that chain). The remaining two are disjoint precisely because they're structurally invisible to that sweep: |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Windows console-flash closure (5 files). Well-structured fix:
- Routes
gh auth tokensubprocess through_subprocess_compatchokepoint - Adds
tui_gatewayto the footgun checker's scan roots - New test
TestGhCliTokenHidesConsoleverifies the routing - Proper comment explaining why the footgun checker can't catch this site itself (variable program name)
Clean fix for a known Windows console-flash issue.
…ch#53853 revert The first cut routed these probes through _subprocess_compat.run/popen (the NousResearch#53810 chokepoint), but NousResearch#53853 reverted NousResearch#53791/NousResearch#53810/NousResearch#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 NousResearch#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 <noreply@anthropic.com>
|
Heads-up @tonydwb — I've updated this since your review, so it's worth a re-look. The first version routed both probes through the The new commit switches both sites to |
|
#53844 is covered all issue that I said, so I will close this PR. LGTM. |
|
The issue is still there on the latest version. |
What does this PR do?
Hides the console window on two Windows console-flash sites the (now-reverted) sweep chain never reached: the
gh auth tokenprobe inhermes_cli/copilot_auth.py, and the Cmd-P file-finder'sgitprobes intui_gateway/server.py. Both flash a console from the windowless desktop gateway today.Why these two were missed
capture_outputis not a no-window boundary on a console-less parent (pythonw.exe) — a console-subsystem child still allocates (and flashes) a console. Even so, two sites wouldn't be caught by a call-site checker:copilot_auth.py— lint-invisible. The call issubprocess.run(cmd, …)wherecmd = [gh_path, "auth", "token"]is a variable, so a literal-argv rule can't tell it'sgh.tui_gateway/server.py— out of scan scope.tui_gateway/isn't in the footgun checker's package roots, so it was never linted.Fix
Pass
creationflags=windows_hide_flags()(CREATE_NO_WINDOW on win32,0/no-op on POSIX) to theghprobe and the twogitprobes. No new helper surface — uses what survived #53853.Scope note
This targets the idle / periodic probe flash (constant cadence, no live session) — the same mechanism captured on Windows 11 while debugging #52310 (gateway
gh/gitprobe → visible console window; the no-window flag removes it). It is not aimed at thehermes update→ gateway-restart flurry that #53853 reset to baseline (that lives in the re-exec / launcher legs, not these call sites). Sibling #53892 covers the periodicgit_probeleg + the re-exec phantom; this covers theghand Cmd-Pgitsites it doesn't.Verification — Windows 11
pytest tests/hermes_cli/test_copilot_auth.py→ 26 passed, includingTestGhCliTokenHidesConsole::test_try_gh_cli_token_passes_no_window_flag, which asserts theghlookup passes the no-window flag — the only regression guard for that lint-invisible site.Related
Type of Change