fix(windows): hide backend console-flash legs main missed (Electron base-pythonw, ffmpeg, netstat) - #53892
Conversation
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-attribute |
2 |
First entries
run_agent.py:3040: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`
✅ Fixed issues (1):
| Rule | Count |
|---|---|
invalid-assignment |
1 |
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`
Unchanged: 6123 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
Related: focused retry of the Windows console-flash family after the merged revert #53853 (which rolled back #53791/#53810/#53829). This PR targets only the two concrete spawn legs the revert called out — the periodic Projects-tree git probe (#53178) and the uv |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Clean Windows-specific fixes for orphan console windows and git probe flashing.
Two well-separated concerns:
detach_orphan_console(): Frees a console auto-allocated by uv's pythonw->python re-exec. UsesGetConsoleProcessList()to distinguish phantom consoles (single process) from user-interactive consoles (multiple processes). Called from gateway, web_server, and cron scheduler — never from the interactive CLI.- Git probe hidden:
run_gitroutes throughwindows_hide_flags()to prevent console flashing on the periodic Projects-tree probe.
Excellent test coverage:
- POSIX no-op, solo-owner console freed, shared console preserved, no-console no-op
- Import-time guard:
FreeConsoleonly insidedetach_orphan_console(), not in bootstrap run_gitroutes throughwindows_hide_flags()
The design is careful: the function is intentionally NOT wired into import-time bootstrap because the discriminator can't tell phantom consoles apart from interactive ones. Only known-background mains call it.
No concerns. Ready to merge.
Reviewed by Hermes Agent
…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>
main (cb982ad) wired windows_hide_flags() into the auxiliary git/gh/wmic/ bash/powershell/taskkill legs but left two it didn't reach, plus the Electron backend-launch leg it explicitly deferred. Cover them the same way: - apps/desktop/electron/main.cjs: getNoConsoleVenvPython resolves the BASE pythonw.exe instead of the venv Scripts\pythonw.exe shim, which re-execs a console python.exe and flashes a conhost the desktop backend can't suppress. Both backend creators put the venv site-packages on PYTHONPATH so imports still resolve under the base interpreter. (main's commit said this Electron leg "needs a Windows-tested change of its own".) - tools/tts_tool.py, tools/transcription_tools.py, plugins/platforms/discord: ffmpeg conversions (voice notes / TTS / STT) via windows_hide_flags(). - plugins/platforms/whatsapp: netstat + taskkill bridge-port cleanup via windows_hide_flags(). All no-ops on POSIX. Tests assert the base-pythonw preference and the ffmpeg legs pass CREATE_NO_WINDOW.
393b040 to
eeca59f
Compare
…pup-spawn-legs
…pup-spawn-legs
…pup-spawn-legs
…pup-spawn-legs
…pup-spawn-legs
Summary
Follow-up to the Windows console-flash fix that landed on
main(cb982ad99). That commit wiredwindows_hide_flags()into the auxiliarygit/gh/wmic/bash/powershell/taskkilllegs (notably the dashboard Projects-tree git probe — the ~118-spawns/60s flurry, #53178). This PR covers the legs it didn't reach, using the same pattern.Confirmed still-broken on
main(native Windows)A user on
main(0c2e6c00) reports terminals still flashing every ~20s, only while the Desktop GUI is running (closing the app but leaving the gateway headless → no flashes). Theirdesktop.logshows the cause directly:The desktop launches/relaunches its Python backend through the venv
Scripts\pythonw.exe, a uv launcher shim that re-execs the base consolepython.exeand allocates a conhostCREATE_NO_WINDOWcan't suppress.main's fix is Python-only and explicitly deferred this Electron leg ("needs a Windows-tested change of its own"). It is GUI-only because only the desktop spawns the backend this way; the standalone gateway already runs windowless.(Why it relaunches on a ~20s cadence is a separate matter — likely a slow local Ollama model, ~20s/call, tripping a desktop health-check restart — but each relaunch flashes purely because of the shim. This PR makes the relaunch windowless regardless.)
What main left open (this PR)
apps/desktop/electron/main.cjs) —getNoConsoleVenvPythonnow resolves the basepythonw.exeinstead of the venvScripts\pythonw.exere-exec shim; both backend creators put the venvsite-packagesonPYTHONPATHso imports still resolve under the base interpreter. This is the exact line in the log above.ffmpegin TTS / STT / Discord voice (tools/tts_tool.py,tools/transcription_tools.py,plugins/platforms/discord/adapter.py) — voice-note/speech conversions ran bare; nowcreationflags=windows_hide_flags().netstat+taskkillin the WhatsApp bridge-port cleanup (plugins/platforms/whatsapp/adapter.py).All
windows_hide_flags()returns0on POSIX, so every change is a no-op off Windows. This intentionally does not re-introduce a competing chokepoint primitive or aFreeConsolebootstrap — it matches the per-call-sitewindows_hide_flags()convention already onmain.Tests
apps/desktop/electron/windows-child-process.test.cjs— assertsgetNoConsoleVenvPythonprefers basepythonwbefore the venv re-exec shim.tests/test_windows_subprocess_no_window_flags.py— TTS opus conversion and local-STT audio prep passCREATE_NO_WINDOW.Test plan