fix(windows): suppress console-window flashes from gateway subprocesses + cache nearest_root - #47971
Conversation
…es + cache nearest_root On Windows, the headless gateway (pythonw.exe) shells out to several console programs without CREATE_NO_WINDOW, so a cmd.exe window flashes on screen. Worst offenders fire on every agent file edit/turn: - agent/lsp/client.py: .cmd-wrapped language servers (pyright-langserver.CMD) launched via cmd.exe /c - agent/lsp/install.py: npm/pip/go LSP auto-install - tools/checkpoint_manager.py: shadow-git snapshot on every edit - agent/coding_context.py: per-turn git status/branch probe - agent/context_references.py: git/rg for @-file references All pass CREATE_NO_WINDOW (0x08000000) via creationflags, gated on sys.platform == 'win32' so it is an inert 0 on POSIX (still a valid Popen kwarg). capture_output is preserved (hide, not detach). Also memoize agent/lsp/workspace.py::nearest_root, which did an uncached upward filesystem walk (markers x parents stat calls) ~5x per file write. Adds _root_cache mirroring the existing _workspace_cache, cleared in clear_cache(). Same staleness profile as the git-worktree cache.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the LSP-specific Windows subprocess gap. The core issue remains on current main, but this branch needs a focused salvage rather than a clean cherry-pick.
Problems
agent/lsp/client.py:273-282now containsstart_new_session=True, added by18a9467fcfor a TUI process-group safety fix. Preserve it when adding the no-window flag.agent/lsp/install.py:351-356now delegates pip installation tohermes_cli/tools_config.py::_pip_install; its subprocess calls athermes_cli/tools_config.py:640-677do not yet pass no-window flags. The original direct pip hunk no longer reaches the runtime path.- No regression tests cover the LSP client/install flags or
nearest_rootcache invalidation. Existing Windows checks cover the already-landed coding-context, context-reference, and checkpoint paths (tests/test_windows_subprocess_no_window_flags.py:97-153,:341-356).
Suggested changes
- Re-scope to the remaining LSP client/install paths and retain
start_new_session=True. - Add focused Windows-flag and
nearest_rootcache/reset tests. - Omit the three paths already covered by
cb1bb1a48andcb982ad997.
Automated hermes-sweeper review.
| @@ -140,6 +149,28 @@ def nearest_root( | |||
| markers_list = list(markers) | |||
There was a problem hiding this comment.
Please add a regression test that proves a repeated identical lookup avoids a second filesystem walk and that clear_cache() restores lookup behavior. Current tests/agent/lsp/test_workspace.py covers root resolution but not this new cache contract.
…r subprocesses Salvaged from PR #47971 (LSP subset). On Windows, .cmd-wrapped language servers (e.g. pyright-langserver.CMD launched via cmd.exe /c) and the npm/go/pip LSP auto-installers spawn without CREATE_NO_WINDOW, so a console window flashes whenever the spawn happens under a console-less parent — e.g. a VS Code/Zed extension host running the ACP adapter. - agent/lsp/client.py::_spawn: pass creationflags=windows_hide_flags() to the language-server asyncio subprocess (inert 0 on POSIX; start_new_session is kept — it is POSIX-only and ignored on Windows). - agent/lsp/install.py: same flags on the npm and go installer subprocess.run calls. The pip path goes through hermes_cli.tools_config._pip_install, which already hides its windows. Adapted from the PR's hand-rolled _NO_WINDOW constant to the repo's hermes_cli._subprocess_compat.windows_hide_flags() convention.
…o installers Regression tests for the #47971 salvage: the LSP language-server spawn must pass windows_hide_flags() creationflags while keeping PIPE stdio and start_new_session, and the npm/go LSP auto-installer subprocess.run calls must carry the same hide flags with DEVNULL stdin and capture_output intact.
…r subprocesses Salvaged from PR #47971 (LSP subset). On Windows, .cmd-wrapped language servers (e.g. pyright-langserver.CMD launched via cmd.exe /c) and the npm/go/pip LSP auto-installers spawn without CREATE_NO_WINDOW, so a console window flashes whenever the spawn happens under a console-less parent — e.g. a VS Code/Zed extension host running the ACP adapter. - agent/lsp/client.py::_spawn: pass creationflags=windows_hide_flags() to the language-server asyncio subprocess (inert 0 on POSIX; start_new_session is kept — it is POSIX-only and ignored on Windows). - agent/lsp/install.py: same flags on the npm and go installer subprocess.run calls. The pip path goes through hermes_cli.tools_config._pip_install, which already hides its windows. Adapted from the PR's hand-rolled _NO_WINDOW constant to the repo's hermes_cli._subprocess_compat.windows_hide_flags() convention.
…o installers Regression tests for the #47971 salvage: the LSP language-server spawn must pass windows_hide_flags() creationflags while keeping PIPE stdio and start_new_session, and the npm/go LSP auto-installer subprocess.run calls must carry the same hide flags with DEVNULL stdin and capture_output intact.
|
Merged via PR #70263 — your commit was cherry-picked onto current main with authorship preserved (rebase-merge). The LSP client + npm/go installer hunks landed; we converted the hand-rolled constant to the shared |
…r subprocesses Salvaged from PR NousResearch#47971 (LSP subset). On Windows, .cmd-wrapped language servers (e.g. pyright-langserver.CMD launched via cmd.exe /c) and the npm/go/pip LSP auto-installers spawn without CREATE_NO_WINDOW, so a console window flashes whenever the spawn happens under a console-less parent — e.g. a VS Code/Zed extension host running the ACP adapter. - agent/lsp/client.py::_spawn: pass creationflags=windows_hide_flags() to the language-server asyncio subprocess (inert 0 on POSIX; start_new_session is kept — it is POSIX-only and ignored on Windows). - agent/lsp/install.py: same flags on the npm and go installer subprocess.run calls. The pip path goes through hermes_cli.tools_config._pip_install, which already hides its windows. Adapted from the PR's hand-rolled _NO_WINDOW constant to the repo's hermes_cli._subprocess_compat.windows_hide_flags() convention.
…o installers Regression tests for the NousResearch#47971 salvage: the LSP language-server spawn must pass windows_hide_flags() creationflags while keeping PIPE stdio and start_new_session, and the npm/go LSP auto-installer subprocess.run calls must carry the same hide flags with DEVNULL stdin and capture_output intact.
What & why
On Windows, the headless gateway runs as
pythonw.exe(no console). When it shells out to console programs without the Win32CREATE_NO_WINDOWflag, acmd.exewindow flashes on screen. The worst offenders fire on every agent file edit / turn, so the user sees windows popping up repeatedly during normal operation.This was originally tracked down from
pyright-langserver.CMDbeing launched viacmd.exe /cby the LSP client (npm shims are.cmdfiles), but the same gap exists in several other gateway-reachable spawn sites.Changes
Console-flash suppression (
CREATE_NO_WINDOW):agent/lsp/client.py.cmd-wrapped language servers (pyright etc.)agent/lsp/install.pytools/checkpoint_manager.pyagent/coding_context.pygitstatus/branchagent/context_references.pygit/rgfor@-file refs@referencesAll pass
creationflagsgated onsys.platform == 'win32'so the value is an inert0on POSIX (still a validPopenkwarg — no behavior change off Windows).capture_outputis preserved (we hide the window, not detach the process), reusing the existingwindows_hide_flags()helper where imported.Performance:
agent/lsp/workspace.py: memoizenearest_root. It previously did an uncached upward filesystem walk (markers × parent dirs of.exists()stat calls) and is invoked ~5× per file write (viaenabled_for,_get_or_spawn,_mark_broken_for_file, and the typescript double-resolve). Adds_root_cachemirroring the existing_workspace_cache, cleared inclear_cache()— same staleness profile as the git-worktree cache (reset on shutdown /hermes lsp restart).Testing
tests/agent/lsp/suites pass (workspace, service, install, broken-set, backend-gate): 42 passed. The one pre-existing failuretest_normalize_path_expands_tildeis an unrelated WindowsHOME/USERPROFILEquirk present onmainand untouched here.nearest_rootcache hit/miss consistency, population, andclear_cache()reset.Notes
0).(server_id, root)and reuses them — there is no per-write process churn; the window-flash was purely a spawn-flag omission.