fix(gateway): a clean systemd stop exited 1, so a stop looked like a crash (CLAWD-3786) - #64
Merged
Merged
Conversation
…crash (CLAWD-3786)
`systemctl --user stop ai.hermes.gateway-<profile>` left the unit in
ActiveState=failed / Result=exit-code / ExecMainStatus=1 although the gateway
had drained cleanly in ~5.5s (operator canary, hermes-technology, 2026-08-12).
An operator, a dashboard probe or an alert rule keys on exactly that signal to
tell "this gateway died" from "this gateway was stopped"; today they are
identical. Same fail-open class as CLAWD-3756.
TRACED, measured on a real gateway process (boot -> SIGTERM -> wait):
gateway/run.py:25572 planned_stop = consume_planned_stop_marker_for_self()
-> False: nothing had written a marker
gateway/run.py:25604 _signal_initiated_shutdown = True
gateway/run.py:25965 if _signal_initiated_shutdown and not runner._restart_requested:
gateway/run.py:25970 return False
hermes_cli/gateway.py:5066 _hard_exit_after_gateway_teardown(1) -> os._exit(1)
Every stop path Hermes owns writes a short-lived planned-stop marker naming the
target PID BEFORE signalling — `hermes gateway stop` on systemd (systemd_stop),
launchd (launchd_stop), s6 (S6ServiceManager.stop) and Windows — and the
shutdown handler consumes it and exits 0. systemd's own stop path had no way to
write one: it sends KillSignal=SIGTERM directly, so an operator stop was
classified as an unexpected external kill. gateway/run.py:12654 already
documented "systemd/launchd ExecStop ... writes a planned-stop marker BEFORE
signalling"; there was no ExecStop=.
Add it. ExecStop=-<python> -m gateway.planned_stop $MAINPID runs while the main
process is still alive, immediately before systemd's SIGTERM, and it is a
property of the unit's stop JOB — so it covers every client that asks systemd to
stop us (systemctl stop/restart, a D-Bus/dashboard stop, loginctl
terminate-user, host shutdown), not only the ones routed through the Hermes CLI.
Leading '-' so a failure to mark the stop can never fail the stop job itself.
The crash-vs-stop distinction is preserved, not flattened: an external kill
still writes no marker and still exits 1. That is also why this is not fixed
with SuccessExitStatus=1 — that would additionally mask a genuine exit-1 fault,
which is the same defect one layer out.
Second symptom, same root cause: an operator stop no longer persists
gateway_state=running (gateway/run.py:12660).
MEASURED (repo venv, throwaway HERMES_HOME, real SIGTERM, no systemd):
bare SIGTERM (= today's systemctl stop): exit 1, gateway_state=running
ExecStop hook + SIGTERM: exit 0, gateway_state=stopped
bare SIGTERM with this fix in tree: exit 1, gateway_state=running
The hook costs 0.09-0.17s on the stop path.
UNVERIFIED: no live systemd verification was performed. The fleet's units are
hand-provisioned as ai.hermes.gateway-<profile>.service, outside this
generator's naming, so refresh_systemd_unit_if_needed() will not reach them —
they need the ExecStop line added by the operator (a drop-in suffices).
…t eat the stop marker (CLAWD-3786) Review of the ExecStop fix found that it leaves a race, and the race reproduces the exact defect the card is about. The planned-stop WATCHER polls for the marker every 0.5s and, on a self-targeted match, calls the same shutdown handler with signal=None. That invocation's consume_planned_stop_marker_for_self() is DESTRUCTIVE. If its tick lands in the ~22ms between ExecStop= writing the marker and systemd delivering SIGTERM, the signal finds no marker, takes the unexpected branch, sets _signal_initiated_shutdown and exits 1. gateway/planned_stop.py writes the marker gateway/run.py:_run_planned_stop_watcher 0.5s poll, fires handler(None) gateway/run.py (handler) destructive consume, marker unlinked gateway/run.py:25962 -> :25967 -> hermes_cli/gateway.py:5066 -> os._exit(1) Measured window 17.7-23.3ms over 8 runs against a 500ms poll => >=4.5% of stops on an idle host, and that is a lower bound. An intermittent false `failed` reads to an alert rule as an intermittent crash, which is worse than a deterministic one. The race is INHERITED, not new: `hermes gateway stop` writes the marker and then pays a whole `systemctl stop` round trip before the signal — a far wider window through the same seam. FIX: latch the classification. gateway/shutdown_classification.py owns the ladder (takeover -> planned stop -> unexpected) and the verdict for the life; a later trigger returns ALREADY_CLASSIFIED, which is not is_unexpected, so the flag cannot be set behind the verdict. start_gateway's handler asks it instead of re-deriving. That fixes every writer of the marker (CLI stop, s6, launchd, ExecStop) and every trigger (watcher tick, signal) at one point. Rejected: a non-consuming watcher (leaves a live marker able to silence a genuinely unexpected signal for up to its 60s TTL — trades a false `failed` for a false `success`, the worse direction); skipping the watcher under systemd (leaves the wider CLI window open on every platform, and the watcher is also the Windows shutdown path). GatewayRunner.stop() was already idempotent (run.py:12157-12159), so the duplicate handler invocation never started a second teardown — only the classification was at risk. No join is added at exit; the CLAWD-1673 / CLAWD-3556 daemon-thread invariants are untouched. Deterministic reproduction (waits for the watcher's OBSERVABLE effect — the marker being unlinked — then signals, instead of racing the poll): before: MODE=race gateway_exit_status=1 after: MODE=race gateway_exit_status=0, gateway_state=stopped Negative control unchanged: an unmarked external kill still exits 1. Also from review: * planned_stop.py wrote NOTHING on any path, and ExecStop=- discards the exit code, so a failed marker write silently restored the pre-fix behaviour. It now says so on stderr (StandardError=journal). * Restored the `pid > 0` guard. It is load-bearing on the documented stdlib fallback: with psutil absent _pid_exists(-1) is True (os.kill(-1, 0) addresses every process the caller can signal) and main(["-1"]) writes target_pid: -1. Its test now asserts at the parse, where it is a property of this code rather than of psutil. * Dropped the assertion that ExecStop= appears above ExecStopPost= in the unit file, with its incorrect comment: systemd parses them into separate lists and always runs the stop phase first, so a transposed unit behaves identically.
…tes (CLAWD-3786) Independent re-review of 410be34 confirmed the watcher race is genuinely closed (measured on real gateway processes against an origin/main control) and returned CHANGES-REQUESTED on three findings, all in the guards and the prose. B1 — THE FIX IS THE LATCH'S PLACEMENT, AND NOTHING PINNED IT. Moving the ShutdownClassifier constructor from start_gateway's body into shutdown_signal_handler is one line, restores the pre-CLAWD-3786 race verbatim (once per INVOCATION, not once per LIFE), and the reviewer measured it surviving tests/gateway/ + tests/hermes_cli/ at 8638 passed with a failure set IDENTICAL to the control, plus 93/93 on this PR's own three files. The guard was `len(calls_to("ShutdownClassifier")) == 1` with the message "one classifier per life" — a source-level SITE COUNT cannot see placement, so the message claimed a property the assertion could not establish. The walk now carries the enclosing function scope and asserts the construction is in start_gateway's own body. Revert-validated with the reviewer's exact mutation: it now fails naming the scope it was found at. Also N1 from the same review: the `classify` walk was unscoped, so any future intent/router `.classify(...)` anywhere in run.py's ~26k lines would redden it with "one place decides". Scoped to start_gateway. B2 — AN UNDECLARED RESIDUAL, CONTRADICTED BY FOUR SITES IN-TREE. After a PLANNED_STOP verdict the latch keeps that classification for the rest of the gateway's life, so a genuinely unexpected SIGTERM mid-teardown now exits 0 where main exits 1. REPRODUCED: marker -> planned SIGTERM -> six unannounced SIGTERMs -> six "keeping that classification" lines, exit 0. The classifier's own docstring asserted the opposite invariant. Worse, the PR rejected the alternative design because it left a marker live "for up to its 60s TTL" and would silence exactly this signal — while the shipped design has the same false-success property over a window bounded by the 180s agent drain, i.e. potentially LONGER. The choice stays; the distinction it was made on does not hold, and now says so. Swept the three sites that documented the now-unreachable False->True transition as live (run.py's arm comment, hard_exit.py's LATE-BINDING TRAP section, and the watchdog test's docstring). The callable shape is KEPT and recorded as knowingly-redundant per §19.2 so nobody simplifies it back on a stale comment. B3 — A GUARD COMMENT NAMED A FAILURE MODE IT DEMONSTRABLY DOES NOT COVER, and it is the mode the fleet rollout uses. A HERMES_HOME mismatch between the unit's Environment= and the gateway's own does not make the marker write FAIL — it makes it SUCCEED into the wrong directory: rc 0, empty stdout AND stderr, marker in the unit's home, absent from the gateway's, gateway exits 1 and persists state=running. Silently the pre-CLAWD-3786 behaviour, and unreachable through the unit generator but exactly the shape of a hand-written ExecStop= drop-in, which is this rollout's part 1 across 11 hand-provisioned units. Prose corrected AND a real guard added rather than only the sentence: the gateway writes gateway.pid under its OWN home, so a PID file naming somebody else means we are in the wrong directory — refuse loudly instead of writing a marker nobody reads. An unreadable probe is could-not-measure, not a finding: it says UNVERIFIED and proceeds, because refusing every stop on a broken probe is worse than the defect being guarded. Revert-validated: guard removed -> 2 red; restored -> 13/0. tests/gateway/ + tests/hermes_cli/: 8641 passed, 2 failed — the same two pre-existing failures the reviewer measured on the control (test_doctor, test_gateway_runtime_health), +3 from the new cases. Claude-Session: https://claude.ai/code/session_01Ca8boCERd9hfnmZGNkNvpZ
…as an int
NO-MERGE from round 3, and the finding is mine: the guard I added last round to
close B3 was a total, fleet-wide regression.
gateway.pid holds json.dumps(_build_pid_record()) (gateway/status.py:963). My
guard compared the file's RAW TEXT to str(pid). A JSON blob never equals a pid,
so the predicate was DEGENERATE — it returned "refuse" for the right home and
the wrong home alike — and ExecStop refused EVERY stop on EVERY gateway, writing
no marker, reinstating the exact pre-CLAWD-3786 defect on the systemd-native
path this card exists to fix. The journal line it printed blamed the operator's
unit for a HERMES_HOME mismatch that did not exist, with the SAME path on both
sides of the accusation.
It was invisible to anyone testing via `hermes gateway stop`, which writes its
own marker first. Only the systemd path — the entire point of the PR — broke.
Both of my new tests were structurally incapable of catching it: both wrote
"999999\n", a bare int the product never produces. The fixture and the subject
disagreed about the file format and only the fixture was consulted. My
revert-validation ("guard removed -> 2 red") is honest and reproduces, and it
proves only that the tests are coupled to the guard's EXISTENCE, never to its
CORRECTNESS.
FIXED by using the module's own readers, twenty lines from where I wrote my own:
_read_pid_record / _pid_from_record handle the JSON record AND the legacy
bare-int form, and they already catch UnicodeDecodeError — which my OSError-only
except did not, so a corrupt or binary gateway.pid killed ExecStop with an
uncaught traceback (round 3 B-3, also reproduced).
THE TEST THAT WOULD HAVE CAUGHT IT, now added: drive the REAL producer
(gateway.status.write_pid_file()) and then this consumer, instead of hand-writing
a fixture. Plus its negative twin in the real format, plus the corrupt-file case.
Revert-validated against the shipped defect: 2 red (the round-trip and the
corrupt-file case), restored 16/0. This module's docstring already declared the
round trip as load-bearing FOR THE MARKER; the guard reads what the GATEWAY
writes, and that direction had no round-trip test at all.
DECLARED RESIDUAL (round 3 N-2): a MISSING or unparseable pid file is not treated
as a mismatch, so the most likely wrong-home shape — a directory no gateway has
run in — passes silently. Refusing on absence would refuse every first stop after
a pid file is cleaned up. Stated in the code rather than implied away.
N-1: the AST walk tagged FunctionDef only while its comment claimed "any other
nested closure". A lambda IS one, and `_make = lambda: ShutdownClassifier()`
called from the handler defeated the guard completely at 9/0. Closed and
revert-validated — it now names scope ('start_gateway', '<lambda>').
N-3: "default agent.restart_drain_timeout=180" is wrong; it is 0
(hermes_cli/config_defaults.py:52), so the window is max(60,30)=60 — EQUAL to
the 60s marker TTL, not "potentially LONGER". Third attempt at that sentence.
The paragraph's point survives; its number and comparative did not.
N-4: the sweep missed a fourth site because the sentence WRAPS ACROSS TWO COMMENT
LINES and a single-line grep cannot see it — a trap this workspace documents and
I walked into anyway.
tests/gateway/ + tests/hermes_cli/ re-run below; the 2 failures are the same
pre-existing pair (test_doctor, test_gateway_runtime_health), independent of
this delta.
Claude-Session: https://claude.ai/code/session_01Ca8boCERd9hfnmZGNkNvpZ
B1 — the round-3 fix used the two LOWEST-LEVEL readers and stopped one level short of the one that answers the question. `_pid_from_record(_read_pid_record())` gives the raw recorded pid, and comparing it to $MAINPID treats "the record names somebody else" as proof of a HERMES_HOME mismatch. It is equally the signature of a STALE gateway.pid in the CORRECT home — reachable on every restart after an unclean exit, because a SIGKILLed or OOM-killed gateway never runs its atexit remover and the file keeps naming the dead pid until get_running_pid() cleans it, which happens after import + config load (>=0.35s for the import alone). A systemd stop in that window refused a legitimate stop, wrote no marker, and the SIGTERM then classified UNEXPECTED -> exit 1 -> ActiveState=failed. The pre-CLAWD-3786 symptom, in the path this card exists to fix, and correlated with exactly the moment an operator restarts a flapping unit. The module's own docstring argues an INTERMITTENT false `failed` is worse for an alert rule than a deterministic one — which is what I was about to ship. Now uses get_running_pid(cleanup_stale=False), which validates liveness AND identity, and which hermes_cli/gateway.py:3341 — the sibling stop path — already uses. Measured, that reader requires an ACTIVE RUNTIME LOCK before it will name a pid at all, so a crashed gateway's stale file resolves to None and the stop proceeds, while a genuinely live foreign gateway still refuses. Strictly narrower and safer than what I described: it refuses only when a real foreign GATEWAY is live in that home, not merely when some process is. Revert-validated: the previous record-only guard now reds the stale-pid case by name; restored, 17/0. BOTH WRONG-HOME FIXTURES WERE WRONG and the fix exposed it. They used pid 999999 — a DEAD pid — which under liveness validation is stale, so they were asserting a refusal that is now the wrong behaviour to want. They present a LIVE FOREIGN GATEWAY instead: a live pid, its real start_time, a gateway-shaped command line, and the runtime lock. That last one is simulating the environmental condition a live gateway creates, not stubbing the decision under test. N1 — closing Lambda alone left the isomorphic hole open. A generator expression is equally scope-creating, and `(ShutdownClassifier() for _ in iter(int,1))` with `next()` in the handler is the same per-invocation construction — measured still GREEN at 9/0 with only Lambda tagged, while the adjacent comment claimed "any other nested closure". All four comprehension forms are tagged now; the evasion reds by name. Second round running for this same class (instrument scope-tagging incomplete), so it is the tuple rather than another single node type. N2 — `assert capsys.readouterr().err == "" or True` is a TAUTOLOGY I wrote. It read as coverage of the stderr behaviour and asserted nothing. Now asserts no traceback, which is the property that case is about. N3 — after the round-3 fix, `test_an_unreadable_pid_file_does_not_refuse_the_stop` no longer covers an unreadable pid file: _read_pid_record swallows (OSError, UnicodeDecodeError) internally, so mode-000, binary, and a directory all return None without reaching the except branch. Behaviourally right, name and docstring wrong. A real stale-pid case now covers the state that matters. NOT bundled, carded instead: hermes_cli/gateway.py:3794 and :4136 still assert the falsified "default restart_drain_timeout = 180" — almost certainly where the 180 came from, out of this delta's declared scope. tests/gateway/ + tests/hermes_cli/ below; the 2 failures are the same pre-existing pair, independently confirmed by the reviewer against a regenerated parent baseline with non-empty sets on both sides. Claude-Session: https://claude.ai/code/session_01Ca8boCERd9hfnmZGNkNvpZ
mbs-vhs
added a commit
that referenced
this pull request
Aug 14, 2026
…862) CLAWD-3769 enumerated this file and asked whether the reference is a path the script READS -- because this is the one executable in its list, where a rename would change runtime behaviour rather than documentation accuracy. The material answer is that this is a text edit with no behaviour change, and that holds. THE MECHANISM I FIRST GAVE FOR IT WAS FALSE, and an independent reviewer measured it: I wrote "no code path reads, tests or resolves it", and two of those three verbs are wrong. Line 31 sits inside the `-h|--help` header block, which is emitted by `sed -n '2,69p' "$0"` at :86 -- so it IS read from the script and printed to the operator, and it IS pinned by a test (tests/scripts/test_opt_provenance_report.py, which asserts the sed range ends at the header boundary). What is true is narrower: it is never RESOLVED as a path, and this edit is line-count-neutral, so the pinned range and both guard tests are unaffected. Verification: git grep -In 'NEMESIS' -- . -> rc 1, zero hits (was 1) bash -n scripts/deploy-to-runtime.sh -> OK wc -l, HEAD vs origin/main -> 234 vs 234 (neutral) `set -euo pipefail` still at NR=70 -> range 2,69p still correct --help | grep TERMINUS -> prints the retargeted line pytest tests/scripts/test_opt_provenance_report.py -k help -> 2 passed Rebased onto origin/main (ef452df) -- the branch was cut from a local `main` one commit stale, missing merged PR #64. The sweep result is unchanged by that (`git grep` on origin/main returns the same single hit) but the branch should not carry the staleness.
mbs-vhs
added a commit
that referenced
this pull request
Aug 14, 2026
…862) (#65) CLAWD-3769 enumerated this file and asked whether the reference is a path the script READS -- because this is the one executable in its list, where a rename would change runtime behaviour rather than documentation accuracy. The material answer is that this is a text edit with no behaviour change, and that holds. THE MECHANISM I FIRST GAVE FOR IT WAS FALSE, and an independent reviewer measured it: I wrote "no code path reads, tests or resolves it", and two of those three verbs are wrong. Line 31 sits inside the `-h|--help` header block, which is emitted by `sed -n '2,69p' "$0"` at :86 -- so it IS read from the script and printed to the operator, and it IS pinned by a test (tests/scripts/test_opt_provenance_report.py, which asserts the sed range ends at the header boundary). What is true is narrower: it is never RESOLVED as a path, and this edit is line-count-neutral, so the pinned range and both guard tests are unaffected. Verification: git grep -In 'NEMESIS' -- . -> rc 1, zero hits (was 1) bash -n scripts/deploy-to-runtime.sh -> OK wc -l, HEAD vs origin/main -> 234 vs 234 (neutral) `set -euo pipefail` still at NR=70 -> range 2,69p still correct --help | grep TERMINUS -> prints the retargeted line pytest tests/scripts/test_opt_provenance_report.py -k help -> 2 passed Rebased onto origin/main (ef452df) -- the branch was cut from a local `main` one commit stale, missing merged PR #64. The sweep result is unchanged by that (`git grep` on origin/main returns the same single hit) but the branch should not carry the staleness.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes CLAWD-3786.
The defect
Operator canary,
hermes-technology, fleet idle, 2026-08-12:The shutdown itself is fine; only the exit status is wrong.
Result=exit-code/ActiveState=failedis the signal an operator, a dashboard probe or an alertrule uses to tell "this gateway died" from "this gateway was stopped" — today
they are identical. Same fail-open class as CLAWD-3756.
Traced cause
Measured, not inferred: a real gateway booted from this checkout under a
throwaway
HERMES_HOME, then SIGTERMed with no marker (whatsystemctl stopsends), and its exit status and state files read back.
gateway/run.py(handler)consume_planned_stop_marker_for_self()→ False — nothing wrote a markergateway/run.py_signal_initiated_shutdown = Truegateway/run.py:25962if _signal_initiated_shutdown and not runner._restart_requested:gateway/run.py:25967return Falsehermes_cli/gateway.py:5066_hard_exit_after_gateway_teardown(1)→os._exit(1)Corroborating state from the same run, which rules out the other exit-1 paths
(
should_exit_with_failure, the CLAWD-1023 hard-exit watchdog):gateway_state.json→"gateway_state": "running"— onlygateway/run.py:12660writes that, and only when
_signal_initiated_shutdownis set.state/gateway.lifecycle.json→{"exit_code": 1, "exit_reason": "graceful_shutdown"}— teardown completed and went through
_exit_after_graceful_shutdown.Why the marker was missing. Every stop path Hermes owns writes a short-lived
planned-stop marker naming the target PID before signalling —
hermes gateway stopon systemd (systemd_stop), launchd (launchd_stop), s6(
S6ServiceManager.stop) and Windows — and the shutdown handler consumes it andexits 0. systemd's own stop path had no way to write one: it sends
KillSignal=SIGTERMdirectly, so an operator stop was indistinguishable from anunexpected external kill.
gateway/run.py:12654already documents that"systemd/launchd
ExecStop… writes a planned-stop marker BEFORE signalling" —there was no
ExecStop=in either generated unit.The fix, part 1 — give systemd's stop path the marker
ExecStop=-{python} -m gateway.planned_stop $MAINPIDin both generated units(user + system scope), plus the small
gateway/planned_stop.pyit invokes(mirrors the existing
ExecStopPost=… -m gateway.cgroup_cleanupidiom).ExecStop=runs while the main process is still alive, immediately beforesystemd's SIGTERM, and it is a property of the unit's stop job — so it covers
every client that asks systemd to stop us (
systemctl stop/restart, aD-Bus/dashboard stop,
loginctl terminate-user, host shutdown), not just theones routed through the Hermes CLI. Leading
-: a failure to mark the stop mustnever fail the stop job itself. That failure is not silent, though — see
gateway/planned_stop.py, which prints to stderr (StandardError=journal) whenthe write fails, because the code systemd would have carried is discarded by the
same
-.Why not
SuccessExitStatus=1. It would also mark a genuine exit-1 fault assuccess — the remedy inheriting the disease it is meant to cure, one layer out.
It also cannot fix the second symptom below, because it changes only systemd's
reading of the exit code, not the gateway's own classification of the signal.
Second symptom, same root cause: an operator stop no longer persists
gateway_state=running(gateway/run.py:12660), which is the run-intent recordcontainer_bootreads.The fix, part 2 — classify the shutdown once per life (review finding)
Independent review found that part 1 alone leaves a race, and it is real. The
planned-stop watcher polls for the marker every 0.5s and, on a self-targeted
match, calls the same handler with
signal=None. That invocation'sconsume_planned_stop_marker_for_self()is destructive. If its tick lands inthe ~22ms between
ExecStop=writing the marker and systemd delivering SIGTERM,the signal finds no marker, takes the unexpected branch, and exits 1 — the
original defect back as a race.
Measured window: 17.7–23.3ms over 8 runs (median 22.3ms), against a 500ms poll ⇒
≥4.5% of stops on an idle host, and that is a lower bound (it excludes
systemd's own reap→signal latency, and interpreter teardown widens under load).
An intermittent false
failedis worse for an alert rule than a deterministicone.
The race is inherited, not introduced:
hermes gateway stophas alwayswritten the marker and then paid a whole
systemctl stopround trip before thesignal — a far wider window through the same seam. This PR narrows it and now
closes it.
Fix chosen: (a) latch the classification.
gateway/shutdown_classification.pyholds the ladder (takeover → planned stop → unexpected) and the verdict for the
life; a later trigger returns
ALREADY_CLASSIFIED, which is notis_unexpected, so_signal_initiated_shutdowncannot be set behind theverdict.
start_gateway's handler now asks it instead of re-deriving.Rejected alternatives:
marker on disk for up to its 60s TTL, so a genuinely unexpected signal in that
window would be silenced. That trades an intermittent false failed for an
intermittent false success, which is the worse direction for the very signal
this card is about.
CLI-stop window open on every platform, and the watcher is also the Windows
shutdown path. It treats one trigger of a shared defect.
(a) fixes every writer of the marker (CLI stop, s6, launchd,
ExecStop=) andevery trigger (watcher tick, signal) at one point.
Not touched:
tools/daemon_pool.py,tools/delegate_tool.py, and every join /drain in the shutdown path.
GatewayRunner.stop()was already idempotent(
run.py:12157-12159), so the duplicate handler invocation never started asecond teardown — only the classification was at risk.
Measured behaviour
Real gateway process, real SIGTERM, no systemd involved. Exit status and
gateway_stateread back from each run:origin/mainrunningrunning— unchanged, and the point: an unannounced kill is still a faultstoppedstoppedstoppedstoppedBefore this PR a
systemctl stoptook row 1, because nothing wrote a marker.Part 1 moves it to row 2 — except when the watcher wins the race, which is row
3, and is what part 2 fixes.
The race row is deterministic, not timing luck: it waits for the observable
effect of the watcher tick (the marker file being unlinked) and only then sends
the signal, forcing the interleaving instead of hoping for it.
The
ExecStophook costs 0.09–0.17 s on the stop path (3 runs), against aTimeoutStopSecof ≥60 s.Tests + revert validation
23 tests (11 hook + 9 classification + 3 unit-template). Every one was reverted
against a mutation and observed red; 20 mutations, each applied alone and
restored.
test_a_signal_behind_the_watcher_tick_stays_a_planned_stop(the race)test_repeated_signals_do_not_reclassify_eithertest_a_marker_arriving_after_the_verdict_is_not_consumedtest_an_unannounced_signal_is_unexpectedtest_only_unexpected_reports_itself_as_unexpectedALREADY_CLASSIFIED.is_unexpected→ Truetest_sigint_is_a_planned_stop_without_a_markertest_a_takeover_marker_outranks_a_planned_stop_markertest_no_verdict_before_the_first_signaltest_the_handler_has_exactly_one_classification_pointrun.pytest_marker_written_for_live_pid_is_consumed_by_that_processtest_the_exact_command_line_in_the_unit_marks_the_stop__main__entry pointtest_without_the_hook_the_shutdown_is_classified_unexpectedtest_no_marker_when_mainpid_is_unexpandedValueErrorguardtest_no_marker_for_a_non_numeric_argumentValueErrorguardtest_no_marker_without_an_argumenttest_a_pid_that_names_nothing_is_rejected_before_the_liveness_checkpid > 0guardtest_no_marker_for_a_dead_pidtest_reports_failure_when_the_marker_cannot_be_writtentest_a_failed_write_says_so_on_stderrtest_a_successful_write_is_quiettest_user_unit_marks_the_stop_as_plannedExecStopfrom the user unit / drop its-test_system_unit_marks_the_stop_as_plannedExecStopfrom the system unittest_exec_stop_runs_the_units_own_interpreterExecStopfrom the user unit / drop its-One test of my own was caught by this validation, not by the gate: the
wiring test first asserted on the run.py source as a string, and
_run_planned_stop_watcher's docstring names the consume it delegates — so theassertion was red before its mutation ever ran, which makes the "RED" reading
worthless. It now parses the AST and asserts on real call sites (the technique
tests/gateway/test_10710_auto_reset_evicts_cached_agent.pyalready uses on thismodule), and both states are recorded: green on the clean tree, red only under
the mutation. Every row above is a green-clean/red-mutated pair, not a red
sighting.
Two corrections from review, both of which changed the code and not just the
prose:
pid > 0guard was restored. It is load-bearing on the documentedstdlib fallback: with psutil absent,
_pid_exists(-1)is True (os.kill(-1, 0)addresses every process the caller can signal) andmain(["-1"])writes amarker naming
target_pid: -1. The test now asserts the guard at the parse,where it is a property of this code, instead of end-to-end where it was really
asserting psutil's behaviour.
ExecStop=appears aboveExecStopPost=in the file wasdropped, with its incorrect comment. systemd parses the two into separate
lists and always runs the stop phase first, so file position is not the
property that makes this correct — a unit with them transposed behaves
identically, and the mutation that "killed" that assertion was killing a
non-defect.
Gate
scripts/run_tests.sh(full suite, venv synced to the CI extras viauv sync --locked --extra all --extra dev …):No PR-attributable failures, verified against a control regenerated in the
same venv: the same files re-run on a pristine
origin/main(detached HEAD, sameinterpreter) fail identically. The absolute number is venv-specific — an earlier
run of this branch in a
.[dev]-only venv saw 184, a reviewer's fuller venv saw53, all of them present on both sides — so the number is not the claim; the diff
between the two runs is, and it is empty.
tests/hermes_cli/test_update_eol_churn.pyis flagged flaky by the runner(failed attempt 1, passed on retry) and passes on the control; it is a git
autocrlftest with no relation to this change, and it is left alone rather thanquietly retried away — flagging it here is the honest handling.
UNVERIFIED / not fixed here
No live systemd verification was performed — this session may not start,
stop or restart a gateway. The end-to-end evidence above is a real gateway
process signalled directly, not a unit transitioning under systemd.
This does not reach the live fleet on its own. The fleet's units are
hand-provisioned as
ai.hermes.gateway-<profile>.service; the generator hereproduces
hermes-gateway-<profile>.service, sorefresh_systemd_unit_if_needed()(which auto-rewrites a stale unit at gateway start) will not find them. Once the
runtime carries this commit, the 11 fleet units need the line added — a drop-in
is enough, no unit rewrite:
Note the latch (part 2) is code, so it ships with the runtime advance and
needs no unit change; only part 1 needs the drop-in.
A HUNG shutdown after a planned stop reports success, and this PR does not
change that. With the stop classified planned,
gateway/hard_exit.py'sCLAWD-1023 watchdog resolves 0 and force-exits
os._exit(0)at its 20s grace,inside
TimeoutStopSec. So a teardown that wedged is reported as a clean stop.This predates the PR (it is what
hermes gateway stophas always done) and isnot covered by "the distinction is preserved" above, which claims only
that an unmarked external kill still exits non-zero.
It is left alone deliberately rather than overlooked: the obvious fix — return
non-zero from the hung path — would make
Restart=on-failurerevive a gatewaythe operator deliberately stopped. Reporting it honestly needs an exit code
that is non-zero and listed in
RestartPreventExitStatus(the unit alreadydoes this for
GATEWAY_FATAL_CONFIG_EXIT_CODE=78), i.e. a new code plus a unitchange plus a hung-teardown reproduction — its own card, not a rider on this
one.