fix(gateway): mark direct systemd stops as planned

[sfire123]

What does this PR do?

Direct systemctl stop / systemctl restart sends the generated systemd gateway an unmarked SIGTERM. The gateway then classifies a normal service-manager stop as unexpected, which can produce a non-zero exit and an unwanted restart.

This PR adds a best-effort, non-recursive ExecStop= hook to both generated user and system units. The hook writes the existing PID-scoped planned-stop marker for $MAINPID before systemd delivers SIGTERM.

The helper is deliberately a top-level stdlib-only module: python -m gateway.systemd_planned_stop would first import gateway/__init__.py and its full application dependency graph, which is unsafe while a service is being torn down. The new helper writes the same marker payload atomically and with owner-only permissions.

The shutdown marker classification is centralized in _classify_shutdown_signal(), which is used by the real POSIX signal handler. The cross-platform watcher still cannot consume systemd's marker before the implicit SIGTERM arrives. Existing CLI/Windows marker paths retain their watcher behavior by default, and raw/unmarked SIGTERM keeps its unexpected-shutdown behavior.

Scope boundary: systemd does not run ExecStop= when a Type=notify service is stopped before reaching READY=1; that pre-start activation case retains the existing behavior.

This is a current-main salvage of #42555, whose branch is stale and merge-conflicted. The original authorship is preserved.

Related Issue

Fixes #42517.
Fixes #41631.

Follow-up to #42675 and #43236. Supersedes the stale implementation in #42555 while preserving its authorship.

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

• Add hermes_systemd_planned_stop.py, a minimal stdlib-only ExecStop= helper that validates $MAINPID and writes the existing planned-stop marker.
• Add the helper to generated user and system units before ExecStopPost= cleanup.
• Keep helper failure non-blocking for systemd stop jobs.
• Centralize planned takeover/planned stop classification for the real signal handler and its watcher path.
• Add regression coverage for standalone import, helper exit behavior, SIGTERM marker consumption, watcher exclusion, PID-scoped markers, and both generated unit variants.
• Preserve unexpected/unmarked SIGTERM behavior.

How to Test

uv run pytest -q tests/gateway/test_systemd_planned_stop.py tests/gateway/test_gateway_shutdown.py tests/gateway/test_status.py tests/hermes_cli/test_systemd_planned_stop_unit.py
98 passed, 2 skipped

Additional verification on Linux:

• targeted ruff check: passed;
• python3 -S -m hermes_systemd_planned_stop standalone smoke: passed, including marker payload and 0600 permissions;
• systemd-analyze verify on a generated unit: passed;
• git diff --check: passed.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs and documented the stale/conflicting PR being salvaged
• My PR contains only changes related to this fix
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes
• I've tested on Linux with systemd

Documentation & Housekeeping

• Internal service lifecycle contract is documented in module docstrings; no user-facing documentation change is required
• No config keys were added or changed
• Cross-platform impact considered: generated systemd units are Linux-only; other service managers are unchanged
• No tool schemas or descriptions changed

Screenshots / Logs

Not applicable. The standalone helper smoke and generated-unit verification passed, and the focused shutdown suite covers the real SIGTERM marker-classification path.

Refresh onto current upstream main (2026-09-09)

• Rebased cleanly from 25ffe1530b4755095aa934b008b0c71e491400a2 onto upstream main at 0e9fc2cc152b4a4d9fd736f107412ace2a0c2555.
• Published head: d2f81f4681b2b5efe222ec67e67783f0d310ab0a.
• Updated the generated-systemd-unit test fixture for current main's four-field service identity.
• Focused canonical suite: 98 passed, 0 failed, 2 skipped (the skips are Windows-only on this Linux host).
• Ruff, py_compile, Windows-footguns, standalone helper smoke, systemd-analyze verify, and git diff --check passed.

[alt-glitch]

This was generated by AI during triage.

Related: #42555 and #24351 are open ExecStop-marker implementations for the same systemd planned-stop behavior. This PR is a current-main salvage of #42555 using a dedicated helper module; maintainers should select one implementation. #43236 is the merged s6/container complement.

[monerostar]

Ubuntu 26.04, kernel 7.0.0-28-generic, linux-5800x. Head 199f93fdcf.

Live unit hermes-gateway-main.service still has only
ExecStopPost=... gateway.cgroup_cleanup. No planned-stop ExecStop=.
Did not stop the live gateway.

origin/main generator emits the same Post-only pair.
This PR inserts
ExecStop=-/venv/bin/python -m gateway.systemd_planned_stop $MAINPID
before ExecStopPost=.

Isolated helper: main([our pid]) rc=0, marker file written,
planned_stop_marker_targets_self() is False (watcher leaves it),
consume_planned_stop_marker_for_self() is True. Invalid PID rc=2.

Focused tests: 5 passed in 2.36s.

Looks good from Linux. Current-main salvage vs #42555 / #24351. I only ran this tip.

[sfire123]

Thanks @monerostar — I really appreciate you reproducing this on Linux/systemd and validating the exact PR tip and focused tests. The detailed verification is very helpful.

[Enough1122]

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

fix(gateway): mark direct systemd stops as planned

1. ExecStop helper robustness at shutdown time — ExecStop=-{python_path} -m gateway.systemd_planned_stop $MAINPID runs the full interpreter and imports gateway.status (plus whatever it pulls in). If the venv/Python is unavailable at stop time (e.g., during an upgrade) or an import fails, the marker is never written and the stop is classified unexpected → non-zero exit → service manager may restart it. A stdlib-only marker writer that writes the JSON file directly (no heavy imports) would be more reliable for ExecStop. (hermes_cli/gateway.py ~line 3026)
2. HERMES_HOME/profile propagation — the helper derives the marker path from get_hermes_home() in its own environment. Under systemd, unless the unit's environment includes the same HERMES_HOME (or profile selection) the gateway process has, the marker lands in a different directory and the SIGTERM handler never sees it — the stop is misclassified as unexpected. Worth verifying the generated unit propagates the runtime's HERMES_HOME/profile env, and adding a test/assert for it.
3. Signal-handler consumption is untested — the unit test asserts planned_stop_marker_targets_self() is False (watcher leaves it) and that consume_planned_stop_marker_for_self() works, but there is no test that the actual SIGTERM handler path consumes a trigger_watcher=False marker before classifying the stop. A small integration-style test around the handler would lock the intended contract.
4. TTL cleanup interplay — trigger_watcher=False markers are left for the signal handler, but if the handler never runs (SIGTERM → TimeoutStopSec → SIGKILL), the marker relies on the TTL cleanup in planned_stop_marker_targets_self to avoid wedging the next instance; the new early-return happens after that cleanup, so this looks correct — a comment confirming the ordering would help future readers.

[sfire123]

Hi @alt-glitch — following up on the triage note. I checked the automated review observations: both generated systemd units already propagate HERMES_HOME; the SIGTERM handler consumes consume_planned_stop_marker_for_self(); and TTL cleanup runs before trigger_watcher=False markers are reserved for the handler. The remaining optional hardening would be a direct handler-path test and/or making the ExecStop helper more standalone.

Since #42555 and #24351 are older overlapping implementations, could you advise which direction maintainers would like to carry forward? #85901 is currently mergeable, preserves the original authorship, and its exact head 199f93fdcf was independently validated on Linux with 5 focused tests. Its workflows remain action_required; if this is the preferred direction, could a maintainer approve CI or assign a reviewer?

[sfire123]

@alt-glitch @teknium1

Refreshed this PR onto the current upstream main and pushed head 1033097b5e.

Follow-up changes since the previous review:

• moved the systemd ExecStop helper to top-level hermes_systemd_planned_stop.py, which uses only the Python standard library and avoids importing gateway/__init__.py plus optional dependencies during service teardown;
• generated user/system units now invoke python -m hermes_systemd_planned_stop $MAINPID before ExecStopPost=;
• centralized planned takeover/planned-stop classification in the code used by the real signal handler;
• added regression coverage for the SIGTERM marker path and standalone helper import.

Verification on the rebased head:

• focused gateway/status/systemd suite: 89 passed, 2 skipped;
• targeted ruff check: passed;
• standalone python3 -S -m hermes_systemd_planned_stop smoke: passed;
• generated unit systemd-analyze verify: passed;
• git diff --check: passed.

Could you please re-review the refreshed PR and advise whether this implementation should be carried forward over the older overlapping #42555/#24351 implementations?

[sfire123]

Rebased this PR onto the current upstream main and force-pushed the final head.

• Final head: 25ffe1530b4755095aa934b008b0c71e491400a2
• Main advanced by one commit during publication, so a second rebase was performed; it applied cleanly.
• The first rebase conflict was in gateway/run_shutdown.py; it was resolved by retaining upstream exit-verdict handling together with this PR’s planned-SIGTERM marker classification.
• Verification on the exact final head: 98 passed, 2 skipped; ruff, py_compile, and git diff --check passed.
• GitHub Actions for this fork are currently action_required for CI, Nix, and Docker.

Could a maintainer approve the fork workflows and re-review this final head?

[sfire123]

@NousResearch/hermes-agent-core — could a maintainer please approve the fork workflows for this PR and re-review the final head 25ffe1530b4755095aa934b008b0c71e491400a2?

CI, Nix flake check, and Docker Build, Test, and Publish are currently action_required, so the required checks cannot start. The exact final head has been locally verified: 98 passed, 2 skipped; ruff, packaging, generated systemd units, and diff checks pass.

Please also advise whether this current-main salvage should supersede the overlapping #42555/#24351 implementations.

[sfire123]

Refreshed this PR onto current upstream main.

• Previous head: 25ffe1530b4755095aa934b008b0c71e491400a2
• Current upstream main: 0e9fc2cc152b4a4d9fd736f107412ace2a0c2555
• New head: d2f81f4681b2b5efe222ec67e67783f0d310ab0a
• Rebase: clean; the three PR commits were preserved.
• Compatibility fix: updated the generated-unit test fixture for main's four-field _system_service_identity() result.
• Focused canonical suite: 98 passed, 0 failed, 2 skipped (Windows-only skips on Linux).
• Additional checks: Ruff, py_compile, Windows-footguns, standalone helper smoke, systemd-analyze verify, and git diff --check all passed.

The fork branch was read back after the force-with-lease push and matches the new head.

[sfire123]

@NousResearch/hermes-agent-core — could a maintainer please approve/allow the fork workflows for the current PR head d2f81f4681b2b5efe222ec67e67783f0d310ab0a?

The previous approval request referenced the superseded head 25ffe1530b; this PR has since been rebased onto the current upstream main. For the current head, these required workflows are action_required and cannot start without maintainer approval:

• CI
• Nix flake check
• Docker Build, Test, and Publish

Please also re-review the current head after the workflows are approved. The exact head has been locally verified: 98 passed, 2 skipped; Ruff, py_compile, Windows-footguns, standalone helper smoke, generated systemd unit verification, and diff checks pass.

[sfire123]

@NousResearch/hermes-agent-core — gentle follow-up on the review request for PR #85901. The PR is mergeable and ready for maintainer review. Please let me know if any changes or a different direction are preferred. Thanks!

[sfire123]

Rebased this PR onto the current upstream main and force-pushed the refreshed head with an explicit --force-with-lease.

• Previous head: 54d0b4f224bfd6c390cb842374f125cb7af96f51
• Current upstream/base: afc3b7c6f397c6d21fd2c129ca17b18b544dd8dc
• New head: 55724456c3e711a8899b4310702bff60884f1a6b
• The fork ref was read immediately before the push and read back afterward; it matches the new head.
• The upstream service-unit extraction was preserved; the generated units now route ExecStop through the standalone hermes_systemd_planned_stop helper before ExecStopPost.

Verification on the exact final head:

• canonical focused gateway/systemd suite: 26 passed, 0 failed;
• targeted Ruff: passed;
• Python bytecode compilation: passed;
• generated systemd unit: systemd-analyze verify passed;
• git diff --check: passed.

The PR remains open and non-draft. GitHub currently reports mergeable=true, mergeable_state=blocked; no checks are published for this fork head (gh pr checks reports no checks), so this is not being represented as green CI. Please approve/allow the fork workflows if required and re-review the current head.