fix(gateway): keep an ancestor gateway visible so the update it spawned can pause it

[jackulau]

Summary

hermes update --gateway is spawned by the gateway when a user issues /update from a messaging platform. That puts a real gateway run process in the updater's own ancestor chain, and _scan_gateway_pids was excluding every ancestor unconditionally, so the scan reported no gateway at all.

The consequences, in the order the reporter hits them:

1. find_gateway_pids(all_profiles=True) returns [].
2. The update pause path has nothing to pause, so the gateway keeps running.
3. The updater mutates the venv while the gateway still holds its .pyd files.
4. The venv-holder guard aborts with Other Hermes processes are running from this install's venv.

On Windows step 3 is the one that matters, because open .pyd handles cannot be replaced in place, which is exactly why the pause exists. The user is left having to kill the gateway by hand.

The blanket ancestor exclusion came from #13242, whose real case is narrower: keep the CLI that invoked the scan (hermes gateway status, hermes update) from being counted as a running gateway. Two facts make the narrower rule safe:

• looks_like_gateway_command_line requires the subcommand to be exactly run, and the runtime matcher accepts only {run, restart}. Neither gateway status nor update can match either one.
• The ancestor exclusion is applied after the strict matcher has already accepted a process. So by the time it fires, the only thing it can still remove is a genuine gateway runtime, which is precisely the bug.

So this gates the exclusion on the command line instead of removing it: an ancestor that looks like a gateway runtime stays visible, everything else is still suppressed. #13242's behaviour is preserved verbatim.

Changes

• hermes_cli/gateway.py
  • Hold the ancestor set separately from exclude_pids. exclude_pids belongs to the caller and stays unconditional; ancestry is a heuristic that the scan now resolves with the command line in hand.
  • Add _suppressed_as_ancestor(pid, command), true only when the pid is our ancestor and its command line does not look like a gateway runtime.
  • Wire it into all three scan arms so behaviour is identical everywhere: Windows wmic, POSIX /proc, and the ps fallback.
• tests/hermes_cli/test_gateway_scan_ancestor_gateway.py (new): 8 regression tests.

I deliberately did not take the larger option of dropping the ancestor exclusion entirely. It would also fix the bug, and the analysis above suggests it is probably safe, but it changes behaviour for every caller rather than only the broken one, and the #13242 report is old enough that I would rather keep its guarantee mechanical than argue it away.

ruff format wants to reformat pre-existing unrelated code in hermes_cli/gateway.py (the PATH concatenation near the top and several text=True, encoding=... blocks). That is already true on a clean upstream/main, so I left it alone rather than bury this diff in unrelated churn.

Testing

New file, 8 cases:

• TestAncestorGatewayStaysVisible (6 cases, Windows arm). Stubbed end to end so it runs on any host, since Windows is where the reported failure lives.
• TestAncestorGatewayViaProc (2 cases, @pytest.mark.linux_only). Same contract through the /proc arm that Linux hosts actually take.

Both the fix case and the #13242 guardrails are covered on purpose:

Test	Asserts
test_gateway_that_spawned_us_is_reported	the reported bug: a real gateway run ancestor is found
test_updater_ancestor_is_not_reported	the updater is in its own chain and is still not a gateway
test_non_gateway_ancestor_is_still_excluded	#13242 still holds
test_gateway_status_ancestor_is_still_excluded	#13242's original case, verbatim
test_caller_supplied_exclusions_stay_unconditional	exclude_pids still outranks the matcher
test_unrelated_gateway_is_unaffected	a non-ancestor gateway was never in question

Verified the tests actually catch the regression by stashing hermes_cli/gateway.py and re-running:

$ git stash push hermes_cli/gateway.py
$ pytest tests/hermes_cli/test_gateway_scan_ancestor_gateway.py -q
1 failed, 5 passed, 2 skipped

test_gateway_that_spawned_us_is_reported
  AssertionError: a real `gateway run` in our ancestor chain is the process the
  update pause path exists to find
  assert 14112 in []

That empty list is the reporter's list(find_gateway_pids(all_profiles=True)) == []. The two #13242 guardrail tests pass both before and after the fix, which is the evidence that the older contract is not loosened.

With the fix applied:

$ pytest tests/hermes_cli/test_gateway_scan_ancestor_gateway.py -q
6 passed, 2 skipped

$ pytest tests/hermes_cli/test_gateway.py tests/hermes_cli/test_gateway_proc_fallback.py \
         tests/hermes_cli/test_gateway_windows.py \
         tests/hermes_cli/test_gateway_scan_ancestor_gateway.py \
         tests/hermes_cli/test_subcommands_profile_gateway.py -q
39 passed, 9 skipped

$ ruff check hermes_cli/gateway.py tests/hermes_cli/test_gateway_scan_ancestor_gateway.py
All checks passed!

$ python scripts/check-windows-footguns.py --all
973 files checked, no issues

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactor / code quality
• Test coverage

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (no user-facing behaviour change; the fix restores the documented pause)
• My changes generate no new warnings
• I have added tests that prove my fix is effective
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

Cross-platform

• No hardcoded path separators; nothing new touches paths at all
• All three scan arms updated identically, so Windows, Linux /proc, and the ps fallback share one rule
• Windows behaviour is covered by tests that run on every host, not gated behind a marker
• scripts/check-windows-footguns.py --all clean

Fixes #87594

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

PR: fix(gateway): keep an ancestor gateway visible so the update it spawned can pause it

1. hermes_cli/gateway.py::_suppressed_as_ancestor gates the suppression on looks_like_gateway_runtime_command_line(command), while the include decision uses _matches_gateway_runtime(command) — which can be True via the strict matcher even when include_restart_managers is False and looks_like is False. For any command that matches the strict matcher but not the "looks-like" heuristic, a real gateway ancestor would still be suppressed, and the Windows: hermes update spawned from the gateway's own /update command can't pause the gateway — ancestor-exclusion in _scan_gateway_pids excludes the gateway itself #87594 regression returns for that shape. Consider gating the suppression on the SAME predicate used to include (_matches_gateway_runtime) so the two cannot diverge.

2. _scan_gateway_pids is shared by gateway status / stop / restart paths, not only the update-pause machinery. A status/stop command that happens to run with a gateway ancestor will now report that ancestor. For status that is arguably correct, but for destructive paths invoked inside a gateway-spawned context, confirm that is intended (the include_restart_managers flag is what distinguishes callers — worth a scan of each call site).

3. The new tests only cover the python -m hermes_cli.main gateway run form of the gateway command line. The strict matcher may also accept other entrypoint forms; adding a case where an ancestor matches strict-but-not-looks-like would pin down the divergence question in point 1.

[mck156]

Thanks for the fix! I applied your patch locally on my Windows 11 machine and confirmed the first part works: find_gateway_pids(all_profiles=True) now returns the uv-side worker (gateway run --replace) instead of [].

While testing though, I found a second spot the PR doesn't cover, and it can still abort the update in exactly the scenario you fixed:

_venv_launcher_ancestors() in hermes_cli/update_cmd.py has the same unconditional ancestor-skip. On Windows the gateway is a two-process chain: venv\Scripts\python.exe (the launcher, which holds the .pyd files) spawns the uv-managed interpreter (the worker, which writes the PID file). The pause path calls _venv_launcher_ancestors([worker_pid]) to find the launcher — but its skip set includes the whole ancestor chain, and when the update is spawned by the gateway itself, the launcher is in that chain. So:

before: _venv_launcher_ancestors([1580]) → []          # launcher hidden → venv stays locked
after:  _venv_launcher_ancestors([1580]) → [23668]      # my local patch → venv unlocked

So even with this PR merged, a /update from a messaging platform can still hit the venv-holder guard: the worker gets paused, but the venv-side launcher is left alive holding .pyd files.

My local patch uses the same gating idea as yours (looks_like_gateway_runtime_command_line — a gateway-looking ancestor stays visible, everything else is skipped, with a fallback to the old whole-chain skip on error). Happy to open a follow-up PR if you think it's worth it.

Two more things from the automated review that I think are worth your attention:

1. Predicate divergence: _suppressed_as_ancestor gates on looks_like_gateway_runtime_command_line, while the include decision uses _matches_gateway_runtime — which can be True via the strict matcher even when the looks-like heuristic is False. For a command that matches strict but not looks-like, a real gateway ancestor would still be suppressed and Windows: hermes update spawned from the gateway's own /update command can't pause the gateway — ancestor-exclusion in _scan_gateway_pids excludes the gateway itself #87594 would return in that shape. Worth gating the suppression on the same predicate used for inclusion.

2. Shared scan call sites: _scan_gateway_pids is also used by gateway status / stop / restart, not just the update-pause path. With this change, a status/stop command running with a gateway ancestor will now report that ancestor. For status that's arguably correct, but for destructive paths (stop/restart) invoked from inside a gateway-spawned context, worth confirming that's intended.

3. Test coverage: the new tests only cover the python -m hermes_cli.main gateway run command-line form. The strict matcher may accept other entrypoint forms — a case where an ancestor matches strict-but-not-looks-like would pin the regression properly.

[jackulau]

Thanks both. @mck156 the independent Windows repro is the most useful thing on this PR so far, and your second site is real. Taking the four points in turn.

1. Predicate divergence: the set is empty, and I have pinned it

Both of you flagged that _suppressed_as_ancestor gates on looks_like_gateway_runtime_command_line while the include decision uses _matches_gateway_runtime. I checked whether a command can satisfy the include predicate and fail the suppression one, and it cannot, because one matcher is a subset of the other (gateway/status.py:444 and :449):

def looks_like_gateway_command_line(command):
    return _gateway_command_subcommand(command) == "run"

def looks_like_gateway_runtime_command_line(command):
    return _gateway_command_subcommand(command) in {"run", "restart"}

So _matches_gateway_runtime implies the suppression predicate on both arms: arm one is run, which is in {run, restart}, and arm two is literally the suppression predicate. Nothing the include decision accepts can be suppressed.

I would rather not leave that as an argument in a thread, so 4f2ca0a adds test_strict_matcher_is_a_subset_of_the_runtime_matcher, which fails the moment the strict matcher accepts something the broad one does not. That is the divergence itself, caught at the source rather than through one scan shape.

I did consider switching to _matches_gateway_runtime as suggested. I kept the broad predicate because the two differ only in robustness direction: _matches_gateway_runtime is the narrower of the two (it additionally requires include_restart_managers on the restart arm), so if the matchers ever do drift, gating suppression on the broad one hides less, and hiding a real gateway is the failure this PR exists to remove. To be exact about what that is worth: it is not currently reachable, since with include_restart_managers False a restart process is not included at all and with it True both predicates accept. So this is a robustness preference, not a live difference, and I am happy to switch if maintainers prefer the symmetry.

2. Test coverage for a non-run form

Fair, and now covered. The same commit adds test_restart_hosted_runtime_ancestor_stays_visible for the gateway restart shape, which the original eight tests omitted entirely. That form matters on Windows specifically, because include_restart_managers is not supports_systemd_services(), and a no-supervisor gateway restart runs run_gateway() in its own process, holding the .pyd files while its argv still says restart.

The suggested "matches strict but not looks-like" case cannot be written, per point 1. The subset test is what stands in for it.

3. Shared scan call sites (status / stop / restart)

Correct that the change is not scoped to the update-pause path, and worth stating the consequence plainly. Before this PR, a gateway in the caller's ancestor chain was invisible to every consumer of _scan_gateway_pids. After it, such a gateway is reported to all of them.

For status that is a straightforward correction: the previous behaviour was that a gateway which spawned your shell did not appear in its own status output.

For stop I also believe it is right, and deliberately so. If a gateway spawns a process that runs hermes gateway stop, the gateway the operator means is the one that spawned it, and hiding it meant stop reported success while the gateway kept running. The narrower alternative, exposing ancestors only on the pause path, would put two different answers to "which gateways are live" into the same module, and matcher drift between two views of one process table is precisely what produced the launcher/worker dead-end that _venv_launcher_ancestors exists to patch over. I would rather have one answer.

The one behaviour I would call out for a maintainer: a /stop issued from a messaging platform now stops the gateway that is serving it, where previously it did not. That reads as correct to me, and it is a real change, so I do not want it to land unnoticed.

4. _venv_launcher_ancestors in update_cmd.py

Confirmed, and it is the same bug in a second function. hermes_cli/update_cmd.py:3411:

    skip: set[int] = {os.getpid()}
    try:
        for anc in psutil.Process().parents():
            skip.add(int(anc.pid))

The comment above it gives the same intent as #13242 ("a CLI hermes update runs from the venv python and would otherwise nominate itself") and the same over-broad implementation: the whole chain is skipped, so when the update is spawned by the gateway, the gateway's own venv-side launcher is in that chain and is never nominated. The worker gets paused and the launcher keeps the .pyd files mapped, which is the venv-holder abort you saw.

Please do open the follow-up. You have the two-process repro and a working patch, and it should be your commit rather than mine. Two things I would suggest from doing the equivalent here:

• Keep os.getpid() unconditional. Only the inherited parents() entries need the command-line gate; the updater itself is never the launcher we want, and leaving that one alone keeps the original intent exactly.
• _venv_launcher_ancestors promises "never raises", so if you gate on a predicate that reads the parent's command line, that read wants the same try/except treatment the parent.exe() call already gets. Your described error fallback to the old whole-chain skip sounds right.

Tag me on it and I will review. If you would rather not carry it, say so and I will fold it into this PR instead, but the credit is yours either way.