Skip to content

fix(update): normalize Windows gateway resume launchers - #53459

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

fix(update): normalize Windows gateway resume launchers#53459
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:fix/windows-resume-gateway-wrapper

Conversation

@helix4u

@helix4u helix4u commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the Windows post-update gateway resume path when the captured gateway command line is a Hermes launcher wrapper such as hermes.exe gateway run instead of a Python interpreter argv.

Current main already routes the live restart watcher through gateway_windows.windowless_gateway_restart_spec() before respawning the gateway. That helper rewrites console python.exe argv to windowless pythonw.exe, but it still returned non-Python launcher argv unchanged. So an unmapped gateway captured as hermes.exe gateway run could still be replayed as the wrapper after update, preserving the console-wrapper shape this path is supposed to avoid.

This change teaches the helper to conservatively recognize captured Hermes gateway launchers and normalize them to the same direct pythonw.exe -m hermes_cli.main ... gateway run argv used by a clean Windows gateway start. Explicit --profile / -p selectors and gateway run tail args such as --replace are preserved.

This is the narrow follow-up to the closed #53260: the older PR mixed the mapped pid-file drain wait and wrapper normalization. This PR only addresses the wrapper replay issue, and adds a regression test through _spawn_gateway_restart_watcher() so the normalization is proven to sit on the live respawn path.

Related Issue

Follow-up to #53260.

Related Windows console-wrapper/update-resume work: #52239.

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/gateway_windows.py
    • Add conservative detection for captured Hermes launcher wrappers (hermes, hermes.exe, hermes.cmd, hermes.bat) that contain a gateway run tail.
    • Normalize those wrapper argv to direct windowless gateway argv from _build_gateway_argv().
    • Preserve explicit profile selectors and the captured gateway run ... tail.
    • Leave unrelated non-Python launchers unchanged.
  • tests/tools/test_windows_native_support.py
    • Add helper coverage for non-Python Hermes launcher normalization.
    • Add a live watcher-path regression test proving _spawn_gateway_restart_watcher() passes the normalized argv into the watcher command instead of replaying hermes.exe.

How to Test

  1. Run syntax checks:
    python -m py_compile hermes_cli\gateway_windows.py tests\tools\test_windows_native_support.py
  2. Run the focused Windows restart-spec tests:
    python scripts\run_tests_parallel.py -j 4 tests\tools\test_windows_native_support.py -- -k "GatewayDetachedWatcherWindowsFlags or WindowlessGatewayRestartSpec"

Focused test result: 11 passed, 0 failed.

Note: the Windows local runner printed a post-summary cp1252 progress-callback traceback while rendering a checkmark, but the test command exited 0 and reported 100% pass.

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. Not run; used focused scripts/run_tests_parallel.py -j 4 coverage for the changed Windows restart path.
  • 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 checkout, focused mocked Windows regression tests

Documentation & Housekeeping

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

Screenshots / Logs

Focused checks passed locally:

  • python -m py_compile hermes_cli\gateway_windows.py tests\tools\test_windows_native_support.py
  • python scripts\run_tests_parallel.py -j 4 tests\tools\test_windows_native_support.py -- -k "GatewayDetachedWatcherWindowsFlags or WindowlessGatewayRestartSpec" — 11 passed, 0 failed
  • git diff --check -- hermes_cli/gateway_windows.py tests/tools/test_windows_native_support.py

@helix4u
helix4u marked this pull request as ready for review June 27, 2026 07:04
@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 27, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Normalizes captured Hermes launcher wrappers (hermes.exe, hermes.cmd, etc.) to direct windowless pythonw.exe argv for the Windows post-update gateway resume path. Prevents console wrapper from being replayed after update.

  • Well-scoped Windows-specific fix (79 additions, 9 deletions in source)
  • Thorough test coverage: launcher normalization, unrelated launcher passthrough, live watcher path regression
  • Conservative approach: only touches known Hermes launcher names
  • Preserves explicit --profile/-p selectors and gateway run tail args
  • Good separation of helper functions (_visible_profile_args, _gateway_run_tail, _canonical_gateway_restart_from_wrapper)

Reviewed by Hermes Agent

@helix4u

helix4u commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Users are still reporting gateway flashing windows. This PR is meant to prevent one specific way we keep reintroducing that problem after update.

The issue is that the post-update gateway resume watcher can capture the running gateway as a launcher-shaped command, like:

hermes.exe gateway run

Then after update, it replays that same launcher shape. On Windows, replaying the Hermes console wrapper can create or flash a terminal window again. So even if other gateway starts are cleaned up, update can put the user right back onto a visible-console gateway launch path.

This PR changes that resume path so captured Hermes gateway launchers are restarted through the direct windowless form instead:

pythonw.exe -m hermes_cli.main ... gateway run

It preserves profile args and gateway run tail args, and it leaves unrelated launchers alone. The point is not general argv cleanup. The point is to stop post-update resume from replaying the console wrapper and causing another gateway window flash.

This probably does not fix every flashing subprocess in the whole product, but it should reduce the gateway flashing reports by removing this update-resume wrapper replay path. If this shape is wrong, can someone please point at the preferred gateway/update owner or the preferred implementation shape so this does not just stay broken?

@helix4u

helix4u commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

close after reading:
this wasnt merged, so the updater restarted the gateway wrong, its subprocesses didnt ever see the changes we made to force no window, and it caused more flashing. this is now obsolete and we need to just need to make sure we're launching the same way everywhere to avoid further problems. the way you launch the gateway should be the same in all places we do it. gateway manual restart, regular start, updater restart, etc. all need to be aligned to start the gateway process the same way. otherwise, we'll be chasing ghosts at some point asking, "how did you restart your gateway last".
but yeah, this is prob the root cause of the other fixes not working. the updater was starting the gateway wrong for that fix route. other gateway start routes probably would not have had the flashing issue. since dev requires a manual restart instead of exercising the updater's gateway restart method, the fixes worked for dev. just not after running the updater gateway launcher.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the captured-launcher resume case. Current main still has the gap: windowless_gateway_restart_spec() derives a sibling from arbitrary argv[0] (hermes_cli/gateway_windows.py:846-860), while _derive_venv_pythonw() returns the original launcher when hermesw.exe is absent (hermes_cli/gateway_windows.py:703-716). The post-update unmapped-gateway path forwards captured argv into that helper through launch_detached_gateway_restart_by_cmdline() and _spawn_gateway_restart_watcher() (hermes_cli/main.py:9242-9252, hermes_cli/gateway.py:698-711, hermes_cli/gateway.py:759-767).

The proposed constrained Hermes-launcher recognition and direct _build_gateway_argv() normalization match the existing clean-start mechanism. The helper test covers preservation of profile and gateway-tail arguments, and the watcher test confirms the normalized result is passed into the live watcher path.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/install-update Installer, updater, packaging, wheels, doctor labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded: this patches the pythonw-era windowless_gateway_restart_spec() that PR #70205 (commit 0dbf639) rewrote — the helper now deliberately keeps the venv console python.exe (the pythonw rewrite produced the per-descendant conhost storm, #54220/#56747), so the diff no longer applies and the direction is reversed.

The residual idea here is still real though: captured hermes.exe [--profile X] gateway run wrapper argv bypass the spec's cwd/env-overlay normalization (main returns them unchanged). A rebased follow-up that normalizes wrapper argv to the console interpreter + env overlay would improve post-update resume robustness — happy to review that if you want to re-cut it against current main.

@teknium1 teknium1 closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

4 participants