Skip to content

fix(update): resume Windows gateways without console wrappers - #53260

Closed
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:codex/windows-update-gateway-resume
Closed

fix(update): resume Windows gateways without console wrappers#53260
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:codex/windows-update-gateway-resume

Conversation

@helix4u

@helix4u helix4u commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the Windows hermes update gateway resume path so a cleanly-stopped gateway does not make update wait for the full drain timeout, and so captured gateway launcher commands are not replayed as terminal/console wrappers.

Root cause found in live testing on Windows: the update pause helper wrote the planned-stop marker and the gateway exited cleanly, clearing gateway.pid and gateway.lock, but the old pythonw.exe process object could linger. The update helper waited only on raw PID existence, so with agent.restart_drain_timeout: 180 it looked hung even though the gateway had already stopped. Separately, unmapped Windows gateway resume replayed captured argv; if that argv was a launcher such as hermes.exe gateway run, the watcher could preserve a wrapper-shaped launch instead of normalizing to direct windowless Python.

This PR makes mapped profile waits treat a cleared/replaced profile pid file as stopped, and normalizes captured non-Python gateway launcher argv to the same direct pythonw.exe -m hermes_cli.main ... gateway run shape used by clean gateway starts.

Related Issue

N/A. Related prior PR discovered during duplicate search: #49025 covers an older pythonw-only approach in hermes_cli/gateway.py; this PR targets the current update pause/resume path on main and the mapped-profile wait hang reproduced locally.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/main.py: pass mapped profile pid-file paths into the Windows update gateway wait helper, and treat the old gateway as stopped once its profile pid file no longer resolves to the old PID.
  • hermes_cli/gateway_windows.py: normalize captured non-Python gateway launcher argv, such as hermes.exe --profile work gateway run, to direct windowless pythonw.exe -m hermes_cli.main --profile work gateway run before the restart watcher respawns it.
  • tests/hermes_cli/test_update_concurrent_quarantine.py: add coverage for the mapped Windows gateway case where the pid file is cleared while the old process still exists.
  • tests/tools/test_windows_native_support.py: add coverage that captured hermes.exe ... gateway run is not replayed and is not treated as a Python interpreter.

How to Test

  1. Run the focused update pause/resume tests:
    python scripts/run_tests_parallel.py -j 4 tests/hermes_cli/test_update_concurrent_quarantine.py -- -k "windows_update_gateway_exit or pause_windows_gateways_for_update_stops_profile"
  2. Run the focused Windows restart-spec tests:
    python scripts/run_tests_parallel.py -j 4 tests/tools/test_windows_native_support.py -- -k TestWindowlessGatewayRestartSpec
  3. On Windows, with a running gateway and agent.restart_drain_timeout: 180, run the update-style pause/resume helpers. Verified locally that the helper returned in 9.23s instead of timing out, restarted the gateway, and the final gateway process was pythonw.exe -m hermes_cli.main gateway run --replace with pid/lock/runtime status passing.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11 / AppData Hermes install

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Focused checks passed:

  • python -m py_compile hermes_cli\main.py hermes_cli\gateway_windows.py
  • python scripts\run_tests_parallel.py -j 4 tests/hermes_cli/test_update_concurrent_quarantine.py -- -k "windows_update_gateway_exit or pause_windows_gateways_for_update_stops_profile" — 2 passed
  • python scripts\run_tests_parallel.py -j 4 tests/tools/test_windows_native_support.py -- -k TestWindowlessGatewayRestartSpec — 4 passed
  • git diff --check -- hermes_cli/main.py hermes_cli/gateway_windows.py tests/hermes_cli/test_update_concurrent_quarantine.py tests/tools/test_windows_native_support.py

The local test runner printed a post-summary cp1252 progress callback traceback when rendering a checkmark, but both focused test invocations exited 0 and reported 100% pass.

@helix4u helix4u changed the title [codex] fix Windows gateway resume after update fix(update): resume Windows gateways without console wrappers Jun 26, 2026
@helix4u
helix4u marked this pull request as ready for review June 26, 2026 20:43
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows P2 Medium — degraded but workaround exists labels Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the detailed write-up and the live-testing work, @helix4u. We're going to pass on this one.

The drain-timeout half (treating a cleared/replaced profile pid file as stopped in _pause_windows_gateways_for_update) is sound and reachable. But the wrapper-normalization half is wired to dead code: the captured-argv normalization lives inside windowless_gateway_restart_spec, and nothing in the production resume path actually calls it. _resume_windows_gateways_after_update -> launch_detached_gateway_restart_by_cmdline -> _spawn_gateway_restart_watcher replays run_argv verbatim and never invokes the spec. There's already a standing-red test on main (test_watcher_rewrites_console_python_to_windowless) asserting gateway.py should call that helper — it doesn't. So the wrapper fix improves a helper that never runs, and the actual replay bug it's meant to address stays open.

Closing rather than merging a partial fix. If you want to take another pass, the real fix is wiring windowless_gateway_restart_spec into _spawn_gateway_restart_watcher so the normalization is on the live respawn path (which would also turn that standing-red test green). Appreciate the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants