[dskwe]

What does this PR do?

Fixes the PATH-race half of the Windows gateway ABI crash-loop: a gateway launcher generated from a shell whose PATH puts a newer standalone Python (e.g. 3.14) ahead of the checkout's venv baked that system interpreter into gateway.cmd/gateway.vbs, because get_python_path() fell back to sys.executable when no PM store Python was committed. The detached gateway then booted under 3.14 while every compiled wheel in the checkout lives in its 3.11 venv → ModuleNotFoundError: pydantic_core._pydantic_core on every boot, ~5 rapid deaths, then the start verifier gives up.

Two halves, one contract:

1. Generator side — get_python_path() (hermes_cli/gateway.py) now falls back to the checkout's own venv interpreter (project_venv_dir) instead of sys.executable. A committed PM store Python still wins when recorded, so PM installs are never hijacked back onto a legacy in-tree venv (the other half of this failure class, tracked separately in [Windows] Gateway on the managed store Python (3.14) overlays the in-tree 3.11 venv site-packages → hosted_room_worker dies on pydantic_core ABI mismatch #122736 / Windows gateway picks legacy 3.11 venv over PM-committed 3.14 venv, breaking all binary extensions #122325).
2. Runtime side — run_gateway() re-execs under the committed interpreter before from gateway.run import start_gateway binds any third-party code. This repairs launchers already on disk from before this fix without waiting for a regeneration. The re-exec plan replays the -m hermes_cli.main argv verbatim (flags/profile args pass through), leaves console-script trampolines (no -m) alone since those already embed the right interpreter, and sets a HERMES_GATEWAY_PYTHON_REEXEC marker so the child can never loop. Non-Windows boots and installs with no resolvable committed interpreter proceed exactly as before.

The guard runs at the top of run_gateway(), which is provably ahead of the crash: the reporter's traceback shows hermes_cli.gateway (and its pure-Python deps) import fine under the wrong interpreter — the failure is logged by the gateway.run logger, i.e. during gateway/run.py's heavy import graph, which only binds after this point.

Related Issue

Fixes #123185

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• hermes_cli/gateway.py: get_python_path() prefers the checkout's venv interpreter over sys.executable when no PM store Python is committed
• hermes_cli/gateway.py: new _committed_gateway_interpreter() / _gateway_reexec_plan() (pure, cross-host testable) + _reexec_gateway_under_committed_python() (Windows executor) called at the top of run_gateway()
• tests/hermes_cli/test_gateway_python_reexec.py: regression tests (red on base, green with fix)

How to Test

Windows host with a standalone Python newer than the checkout's pinned venv on PATH ahead of venv\Scripts:

1. On base: hermes gateway restart → "no stable gateway process detected", gateway.log shows repeated ModuleNotFoundError: pydantic_core._pydantic_core.
2. With this PR: same command → gateway boots clean under venv\Scripts\python.exe (re-exec path), and a regenerated gateway.cmd/.vbs embeds the venv interpreter directly (generator path).
3. Non-Windows: run_gateway() unchanged (guard returns immediately); ./venv/bin/python3 -m pytest tests/hermes_cli/test_gateway_python_reexec.py -q → 5 passed, 1 skipped (Windows-only executor test skips on POSIX hosts).

Checklist

• Tests added: interpreter-preference contract, re-exec plan (replay / already-on-target / trampoline no-op), and the Windows executor path
• Red verified on base (4 failures on the pristine tree), green with the fix
• Neighboring suites green: tests/hermes_cli/test_gateway.py, test_gateway_service.py, test_gateway_windows.py, test_old_updater_additional_shims.py (168 passed), test_update_launch_completion.py, test_source_launcher_publication.py, tests/test_managed_runtime_resolution.py (48 passed)

WIP status note (first version): logic is complete and unit/regression-tested cross-host; the Windows executor path itself could not be exercised live from this macOS workstation (no Windows host available in this environment) — the repo's wine2e live lane or a manual check on a Windows machine with a newer system Python on PATH would provide the live receipt. Everything else (plan logic, generator fallback, no-loop marker, non-Windows no-op) is covered by the added tests.

[kiramakes]

🤖 Hermes Agent automated review: PR reviewed. Diff analyzed (280 lines changed). Check CI status and manual review recommended.

[arkheioncorp]

Review: fix(gateway): boot Windows gateway under the checkout's committed interpreter

Verdict: APPROVE

Findings

This fixes #123185 — Windows gateway crash-looping when launched under a newer system Python that wins the PATH race, causing ABI-mismatched wheel imports (pydantic_core._pydantic_core).

What the fix does:

1. get_python_path() now prefers the committed store Python, then the checkout venv interpreter, only falling back to sys.executable as last resort
2. _reexec_gateway_under_committed_python() runs early in run_gateway() before heavy imports bind
3. On Windows, if the gateway boots under the wrong interpreter, it re-execs under the committed one via subprocess.call (Windows spawn-and-exit contract)
4. Guard env var HERMES_GATEWAY_PYTHON_REEXEC prevents re-execution loops

Strengths:

• The re-execution is Windows-only and early — no impact on other platforms
• sys.orig_argv replay preserves -m hermes_cli.main shape; console-script trampolines (no -m) are left alone
• Comprehensive test coverage in test_gateway_python_reexec.py covering: venv preference, store python precedence, reexec plan shapes, noop cases, and the full reexec path

No security or logic concerns. The fix is well-scoped and well-tested.

[ehz0ah]

[Bug] (blocking) This fallback also changes systemd and launchd, although the reported race and runtime repair are Windows-specific. With no committed PM store Python, project_venv_dir(PROJECT_ROOT) accepts any existing in-tree venv/ or .venv/ before considering the interpreter that owns the current Nix/developer runtime. That conflicts with _launchers.py and _prepare_service_launcher(), which preserve external ownership when resolve_store_python() is absent. For example, a developer can run service installation from a valid Nix Python while an old checkout venv/ directory remains; this branch silently rewrites the service to the stale venv and can make the next gateway boot fail. Please keep no-store POSIX launchers on their external interpreter and scope the legacy in-tree fallback to the Windows case it repairs.

[dskwe]

Scoped in b9c62e1 — the in-tree-venv fallback is now Windows-only in both get_python_path() and _committed_gateway_interpreter(). With no committed store Python, POSIX launcher generation keeps the running interpreter as the unit's owner (the _launchers.py / _prepare_service_launcher() ownership rule), so a developer installing from a Nix runtime can no longer have systemd/launchd units silently rewritten onto a stale checkout venv/. The PATH race this PR repairs is a Windows shape; POSIX keeps external ownership. Regression: test_get_python_path_posix_keeps_running_interpreter_over_stale_venv pins a Nix sys.executable winning over a present in-tree venv.

[ehz0ah]

[Bug] (blocking) Legacy Windows launchers can reach this repair under pythonw.exe, where sys.stdout and sys.stderr are None. These unconditional calls then raise AttributeError before subprocess.call() starts the committed interpreter, so the recovery path fails for a launcher shape the repository still explicitly handles in gateway_windows._resolve_detached_python(). A reviewer regression with both streams set to None failed here before the child was invoked.

[dskwe]

Fixed in b9c62e1 — flushing moved into _flush_reexec_streams(), which skips None streams (and survives a stream that raises; flushing is best-effort, the child matters). Legacy pythonw.exe launchers with sys.stdout/sys.stderr None no longer die with AttributeError before subprocess.call() starts the committed interpreter. Regression: test_reexec_flush_tolerates_console_less_streams exercises both-None and broken-stream shapes.

[ehz0ah]

[Bug] (blocking) hermes_cli.main arms the startup watchdog before it imports and dispatches this command, but this parent waits for the long-lived gateway child without disarming its own handle. Only the child reaches the normal gateway disarm point. A reviewer regression observed the parent watchdog still armed at this subprocess.call() seam. After the configured startup timeout, the parent can call os._exit(75) while the child is healthy, returning a failure to a foreground caller or supervisor and breaking ownership of that gateway process.

[dskwe]

Fixed in b9c62e1 — _reexec_gateway_under_committed_python() now calls disarm_startup_watchdog() (the stdlib-only module's process-wide disarm) before the subprocess.call() wait, since this parent never reaches the gateway child's own disarm point inside GatewayRunner. The child re-arms through main's normal path in the new interpreter and disarms at its event loop. Regression: test_reexec_disarms_the_parent_startup_watchdog arms a real handle (arm_startup_watchdog(timeout_s=3600)), runs the guard with a stubbed child spawn, and asserts handle.disarmed is True at the spawn seam.

[ehz0ah]

[Bug] (non-blocking) When the stale launcher is the legacy console-less pythonw.exe form, this plain subprocess.call() starts console python.exe without the hidden-console flags used by current Windows launchers. The repository's gateway_windows._resolve_detached_python() and _spawn_detached() contract pairs console Python with CREATE_NO_WINDOW specifically to prevent visible console windows. This repair path bypasses that contract and can flash or leave a console visible during recovery.

[dskwe]

Fixed in b9c62e1 — the recovery child is spawned through _reexec_spawn_kwargs(), which adds CREATE_NO_WINDOW on Windows (the single hidden-console flag _resolve_detached_python()'s contract pairs with console python; no detach/new-process-group bits because this synchronous call waits on the child and forwards its exit status). A detached pythonw.exe launcher's recovery no longer flashes or leaves a console visible. Regression: test_reexec_spawn_kwargs_hide_console_on_windows_only pins the flag present with is_windows=True and absent otherwise.

[Enough1122]

AI code review — automated follow-up for reference; not a maintainer.

Reviewed at head b9c62e1a20. The production behaviour looks right: I agree with the Windows-only scoping of the in-tree-venv fallback in both get_python_path() and _committed_gateway_interpreter(), with disarm_startup_watchdog() ahead of the subprocess.call() wait, and with _reexec_spawn_kwargs() pairing console python.exe with _CREATE_NO_WINDOW. The four earlier findings on this thread look addressed to me.

What I want to flag is narrower and is about the test, not the code. Two of the three test files' worth of machinery in tests/hermes_cli/test_gateway_python_reexec.py never runs, and the gap is exactly the part of the fix that is load-bearing.

1. test_run_gateway_reexecs_before_heavy_imports does not drive the call site it is named after, so the placement claim is untested.

   The docstring at tests/hermes_cli/test_gateway_python_reexec.py:116 asserts "the guard runs before gateway.run is imported", and the whole argument of the repair is positional: _reexec_gateway_under_committed_python() must sit at hermes_cli/gateway.py:4702, before the heavy from gateway.run import start_gateway at hermes_cli/gateway.py:4727, because the ABI-mismatched import it repairs is unreachable-recoverable anywhere earlier or later.

   But the test's only entry point into the module is the helper itself — gateway._reexec_gateway_under_committed_python() at line 151. It never invokes gateway.run_gateway(). The single other occurrence of the name in the file is the test function's own name at line 115.

   That matters because the helper is a pure function of its inputs, so the tests confirm the re-exec mechanics (argv rewriting, env marker, None-stream tolerance, hidden-console flag) and say nothing about where the call sits. I confirmed by AST over the head sources that _reexec_gateway_under_committed_python has exactly one production call site, hermes_cli/gateway.py:4702. Deleting that line, or sliding it below the gateway.run import at 4727, leaves this test fully green — the exact mutation that would silently disable the fix.

   Suggested fix: have the test call gateway.run_gateway() (with gateway.run.start_gateway stubbed) and assert the re-exec fires, rather than calling the helper directly. That also makes the next point unnecessary.

2. The four monkeypatches at lines 146-147 are dead setup, which is a good tell that this test used to go through run_gateway().

   The loop neutralises _guard_official_docker_root_gateway, _attach_to_host_gateway_or_guard, _guard_supervised_gateway_conflict, and _guard_existing_gateway_process_conflict, under the comment "neutralize the pre-guard lifecycle guards so we isolate the re-exec path". None of the four is read or called anywhere in the test body — they are set and never touched.

   They are also not free-standing: in the head source each of those four has its only production call site inside run_gateway(), at hermes_cli/gateway.py:4697, 4698, 4699 and 4700 respectively. They are isolated exclusively for the benefit of a run_gateway() call that this test does not make. So this is inert setup left behind by a test that was rewritten to call the helper directly and kept its old preamble.

   This is a Minor rather than a defect in itself — the guards are genuinely needed once point 1 is addressed, so the fix is to make the call real rather than to delete the loop. I would not merge a fix that just drops the loop, since that would leave the test unable to call run_gateway() at all.

3. Minor.

   • test_get_python_path_posix_keeps_running_interpreter_over_stale_venv writes its fake interpreter to venv/bin/python but the Windows fallback resolves through venv_python_path() -> pm.environments.venv_python(), which derives the bin dir from os.name == "nt" at call time. On a POSIX host that lands on bin/python, so the fixture is correct here — but the same shape in test_reexec_disarms_the_parent_startup_watchdog (which forces is_windows=True on a POSIX host via the keyword) depends on that host-dependent default rather than passing windows=True down explicitly. It works today; it just reads as if it were testing a Windows layout when it is testing a POSIX one. Not a bug.
   • The Windows-only CI lane selects with -m "platforms and not integration", so test_get_python_path_windows_falls_back_to_checkout_venv and test_run_gateway_reexecs_before_heavy_imports are the only two tests in this file that execute on the Windows runners. The unmarked tests run on the Linux lane. That is worth keeping in mind when judging how much of this file is actually exercised where the bug lives.

Unverified / please confirm:

• Read from source, not executed: every claim about call sites, line numbers and dead setup above. I parsed the head sources with ast (hermes_cli/gateway.py and tests/hermes_cli/test_gateway_python_reexec.py, both fetched at b9c62e1a20 with .size self-check: 272139/272139 and 9967/9967 bytes, so neither read was truncated) and listed every call site of the four guards and of _reexec_gateway_under_committed_python.
• Not run — no mutation test. I did not execute the suite or physically delete hermes_cli/gateway.py:4702 and re-run, so "deleting the call site leaves the test green" is inferred from the AST result (no run_gateway invocation) plus the fact that the guard symbol has zero hits on upstream/main, i.e. no pre-existing test references it. A maintainer with a working environment can confirm in one step by commenting out line 4702 and running this file.
• Not run — no CI check. I did not check whether the two platforms("windows") tests currently pass on the Windows lane; the runner list under .github/workflows/tests-os.yml is what I read, not the job results.
• Please confirm your intent on point 1: if the helper-level tests are meant to be the only coverage and the placement is considered safe to change, that is a defensible call — I would just suggest saying so in the module docstring, because the current docstring reads as though placement is covered. If it is meant to be covered, then the test needs to call run_gateway().

[dskwe]

Fixed in 239cca6 — point 1 addressed the way you suggested; point 2 resolved by exactly that fix.

Point 1 (placement untested): test_run_gateway_reexecs_before_heavy_imports now calls gateway.run_gateway(), not the helper. gateway.run is replaced in sys.modules by a types.ModuleType subclass whose PEP 562 __getattr__ records the access and raises — so the from gateway.run import start_gateway fetch itself is the tripwire: if the guard line is deleted, or slides below the import, the fetch happens first and the test fails with "gateway.run was imported before the re-exec guard". Your mutation is now executed, not inferred: with run_gateway's _reexec_gateway_under_committed_python() line removed, this test fails (ImportError from the stub at the heavy-import seam); with it restored, the file is green.

Point 2 (dead monkeypatches): the four guard neutralizations are live again — run_gateway() genuinely calls all four before the re-exec guard, so they are now load-bearing setup, not dead code.

Shape of the split: the placement test is host-agnostic (guard stubbed to fire, heavy module stubbed) so the ordering contract runs on every lane, not only Windows — worth having since the line numbers you cited sit in shared code. The Windows-marked test is renamed test_run_gateway_reexecs_end_to_end_under_committed_python and keeps the full real-guard shape (launcher-shaped orig_argv, wrong sys.executable, argv replay, env marker) but also goes through run_gateway() and asserts the stub was never touched. One note: pytest.raises(SystemExit) wraps run_gateway(), and the four neutralized guards are what let execution reach the re-exec seam — if a future guard fires first, the test fails loudly rather than silently passing.

Minor 3 (fixture bin-dir): fair reading; the host-agnostic placement test removes the POSIX/Windows layout ambiguity from the load-bearing path, and the Windows-layout resolution stays covered by the windows-marked tests on a real runner.

Local: tests/hermes_cli/test_gateway_python_reexec.py 9 passed + 2 windows-skips on macOS; neighbors test_gateway.py, test_ensure_gateway_service.py 46 passed / 11 skipped, no regressions.

[Enough1122]

Re-verified at 239cca6468. Both points are closed, and making the placement contract host-agnostic is the better half of the change — the line ordering it protects lives in shared code, so a test that only runs on the Windows runner is a test that reports green on every lane that cannot break it.

The sys.modules stub is the right tripwire, and PEP 562 is what makes it work as a tripwire rather than just a blocker. Replacing sys.modules["gateway.run"] with a types.ModuleType subclass whose __getattr__ records and raises turns from gateway.run import start_gateway itself into the assertion point — the test fails at the import seam rather than at a later behavioural check:

    def __getattr__(self, name):
        type(self).accessed = True
        raise ImportError(f"gateway.run.{name} bound before the re-exec guard fired")

That is a real improvement over the version I reviewed, which asserted on behaviour further downstream and so could pass with the guard line moved as long as nothing observable changed. Here, deleting the guard line or sliding it below the import makes the fetch happen first, accessed flips to True, and the test fails with a message that names the actual defect. I confirmed the ordering this depends on on this head:

  220049-220052: the four lifecycle guards
  220054: _reexec_gateway_under_committed_python()
  220078: from gateway.run import ...     <- first heavy import
  220106: from gateway.run import ...

24 lines between the guard and the first heavy import, with nothing that touches gateway.run in between. And your executed mutation is the part that makes this trustworthy — removing run_gateway's guard line and getting ImportError from the stub, then restoring it and getting green, is the check I would otherwise have had to ask for.

Point 2 is correct and the framing matters: those four neutralizations are load-bearing setup, not dead code. run_gateway() genuinely calls all four before the re-exec guard, so reaching the seam requires neutralizing them — which is exactly why they are needed, and why calling them "dead" was my error. It is a nice property of the new test that the same four lines are simultaneously what makes the path reachable and what documents that other guards exist in that window: a future guard firing first will now fail the test loudly, as you note, rather than the test silently exercising a path the real boot never takes.

Keeping the Windows-marked test for the real guard is the right division of labour. The host-agnostic test stubs the guard to fire and proves ordering; the @pytest.mark.platforms("windows") test keeps the real guard, the launcher-shaped orig_argv, the wrong sys.executable, argv replay and the env marker, and now also goes through run_gateway() while asserting the stub was never touched. So the real-guard mechanics and the placement contract are each pinned on the platform and in the shape where they can actually fail, rather than one test trying to cover both. Having the Windows test go through run_gateway() too closes the gap where the mechanics passed while the call site was wrong.

On minor 3, your reading is the fair one and the resolution is the right shape: the placement contract is host-agnostic and no longer depends on bin-dir layout, while the POSIX/Windows layout resolution stays covered by the windows-marked tests on a real runner. Moving that coverage to where the platform matters is better than trying to make one test cover both.

9 passed + 2 windows-skips locally with the neighbouring suites at 46 passed / 11 skipped and no regressions is the right verification for a change whose entire value is in the order of two statements.

Nothing outstanding from me.

I am not a maintainer and my review is not maintainer approval.

[kshitijk4poor]

Leaving this open for a maintainer. #123996 fixes #122183 by keeping the pre-PM in-tree venv off sys.path on PM installs, in both the gateway and cron scripts. It does not touch get_python_path or the gateway re-exec that this PR changes.

My read from the triage: on a PM install, get_python_path already returns the committed store Python. So the "system 3.14" in #123185 may be that store interpreter, and the crash may be the same legacy-venv overlay that #123996 removes. If so, #123996 alone fixes #123185. I haven't confirmed this on a Windows host, so I'm not closing this PR on that basis. Once #123996 merges, a re-test of #123185 would settle it.

[kshitijk4poor]

Merged: #123996 landed on main as 86f449b.

[dskwe]

@kshitijk4poor — thanks for the follow-up. I noted that #123996 landed as 86f449b1721f; it addresses the separate pre-PM in-tree venv overlay and does not change this PR's launcher fallback or Windows re-exec path. I’ll leave this PR focused on that remaining Windows-specific repair rather than duplicate the merged change.