fix(gateway): resolve uv-managed pythonw for Windows scheduled-task wrapper - #30312
briandevans wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the Windows Scheduled Task .cmd wrapper generation to match the detached-spawn behavior for uv-managed virtual environments, ensuring the base pythonw.exe is used and required paths are injected for imports.
Changes:
- Update
_build_gateway_cmd_scriptto use_resolve_detached_python()and conditionally setPYTHONPATHfor uv venvs. - Add a unit test to verify the
.cmdwrapper uses basepythonw.exe(not the venv shim) and includes expected env vars/paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/hermes_cli/test_gateway_windows.py | Adds coverage for uv venv .cmd generation behavior (base pythonw.exe + PYTHONPATH). |
| hermes_cli/gateway_windows.py | Aligns Windows .cmd wrapper with uv venv detached interpreter resolution and import path injection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pythonw_path, venv_dir, extra_pythonpath = _resolve_detached_python(python_path) | ||
| lines.append(f'set "VIRTUAL_ENV={venv_dir.resolve()}"') | ||
| if extra_pythonpath: | ||
| pythonpath_value = os.pathsep.join([working_dir, *extra_pythonpath]) |
| line for line in content.splitlines() if line.startswith('set "PYTHONPATH=') | ||
| ) | ||
| pythonpath_value = pythonpath_line.split("=", 1)[1].rstrip('"') | ||
| pythonpath_entries = pythonpath_value.split(gateway_windows.os.pathsep) |
| pythonw_path, venv_dir, extra_pythonpath = _resolve_detached_python(python_path) | ||
| lines.append(f'set "VIRTUAL_ENV={venv_dir.resolve()}"') | ||
| if extra_pythonpath: | ||
| pythonpath_value = os.pathsep.join([working_dir, *extra_pythonpath]) | ||
| lines.append(f'set "PYTHONPATH={pythonpath_value}"') |
alt-glitch
added
type/bug
|
CI audit — the
Fix is in flight in #30334 (mine, opened ~1h after the regression landed) — adds an opt-in |
|
@copilot All three findings addressed in commit
|
8b9975c to
7fb962d
Compare
7fb962d to
617bd23
Compare
617bd23 to
b1ef4a4
Compare
b1ef4a4 to
42e0c12
Compare
|
@copilot Re-anchoring on the current head (42e0c129d) — all three findings are already addressed on this commit:
No code change required this cycle — the fixes were folded into the current head. |
…rapper `_build_gateway_cmd_script` derived the GUI interpreter via `_derive_venv_pythonw`, which only looks for a sibling `pythonw.exe` in `venv/Scripts/`. On uv-managed venvs that sibling is a ~44KB shim launcher that internally respawns the base `python.exe` (console subsystem), so the Scheduled Task surfaces a visible `cmd.exe` window even though we asked for `pythonw.exe`. Switch the .cmd generator to `_resolve_detached_python` — the same resolver `_build_gateway_argv` already uses for direct spawns — which inspects `pyvenv.cfg`, detects the `uv = ...` marker, and returns the base `pythonw.exe` plus the site-packages path that has to be injected on `PYTHONPATH` so imports still resolve without the venv launcher in the loop. Refs NousResearch#30308.
Address Copilot review on NousResearch#30312: - `_build_gateway_cmd_script` and its test both used `os.pathsep` to build/split the PYTHONPATH baked into the generated .cmd. Since that script is consumed by cmd.exe on Windows, the separator must be `;` literally — `os.pathsep` would yield `:` if the generator runs on Linux/macOS (cross-platform build tooling or unit tests on a non- Windows host). - Update the `_build_gateway_cmd_script` docstring to note PYTHONPATH is now conditionally exported when the resolver returns extra PYTHONPATH entries (uv venv case). No production behavior change on Windows; only the cross-platform robustness of the generator and its test.
42e0c12 to
6da8bd0
Compare
|
Closing to focus the queue on security/file-safety work where civilian merges are landing. Happy to reopen if maintainers want this picked up. |
What does this PR do?
hermes gateway starton Windows pops up a visiblecmd.execonsole window when Hermes is installed inside a uv-managed venv. Root cause is inhermes_cli/gateway_windows.py::_build_gateway_cmd_script, which derives the GUI interpreter via_derive_venv_pythonw. That helper only checks for a siblingpythonw.exeundervenv/Scripts/— on uv venvs that sibling is a ~44KB launcher shim that respawns the basepython.exe(console subsystem), so the Scheduled Task ends up running console Python despite asking forpythonw.exe.The direct-spawn path (
_build_gateway_argv) already handles this correctly via_resolve_detached_python, which inspectspyvenv.cfg, detects theuv = ...marker, and returns the basepythonw.exeplus the site-packages path that has to be injected onPYTHONPATH. This PR switches the Scheduled-Task.cmdgenerator to use the same resolver so both code paths mirror each other.Mirrors
_resolve_detached_pythonprecedence used by_build_gateway_argv:pyvenv.cfg.uv+home=→ basepythonw.exe+PYTHONPATHinjection; non-uv venvs fall through to the previous_derive_venv_pythonwbehavior.Related Issue
Fixes #30308
Type of Change
Changes Made
hermes_cli/gateway_windows.py—_build_gateway_cmd_scriptnow calls_resolve_detached_python(python_path)instead of_derive_venv_pythonw(python_path), and emits aset "PYTHONPATH=<working_dir>;<site-packages>"line when the resolver returns extraPYTHONPATHentries (uv-venv case).VIRTUAL_ENVcontinues to point at the venv dir.tests/hermes_cli/test_gateway_windows.py— newtest_build_gateway_cmd_script_uses_base_pythonw_for_uv_venv_launchermirrors the existing_build_gateway_argvtest: arranges a uv-shaped venv (pyvenv.cfgwithuv = 0.11.14+home = <base>), calls_build_gateway_cmd_scriptdirectly, and asserts the emitted script (a) references the basepythonw.exe, not the venv shim, (b) setsVIRTUAL_ENVto the resolved venv dir, and (c) setsPYTHONPATHto include both the project working dir and the venv site-packages.How to Test
hermes gateway installthen trigger the Scheduled Task. Before this PR a visiblecmd.exewindow appears; after this PR the task runs hidden.uv run --with pytest --with pytest-xdist --with pytest-asyncio --with pytest-timeout python3 -m pytest tests/hermes_cli/test_gateway_windows.py -v— all 22 tests pass, including the new regression test (reverting the production change alone makes only the new test fail).Checklist
Code
fix(scope):,feat(scope):, etc.)Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/Apyvenv.cfg.uvso non-uv venvs and non-Windows hosts are unaffected.Sibling code paths
Screenshots / Logs
The issue includes a screenshot of the rogue
cmd.exewindow on Windows; after this PR the Scheduled Task action completes without surfacing a console window because the basepythonw.exeis genuinely a GUI-subsystem binary.