Repository navigation
feat(estop): keep an allowlisted identity working through a pause, and bound the pause itself - #121054
feat(estop): keep an allowlisted identity working through a pause, and bound the pause itself#121054yoyodine-industries wants to merge 81 commits into
Conversation
… pause itself `hermes pause` (ESTOP) today holds everything, operator included, and has no self-release: a window job that dies between arm and release parks the fleet until a human notices. Two additions, both operator-facing. One sentinel, one predicate; no second mechanism. - `allow.user_ids` / `allow.profiles` exempts an authenticated identity from the pause - the operator's user id first, the serving profile as the secondary key for a maintenance lane. The gateway turn gate consults it through `agent.estop.is_allowed`, so an exempt turn is SERVED while everyone else gets the pause notice. No allowlist => nobody is exempt (fail closed); an unreadable sentinel => nobody is exempt. - `expires_at` is a deadman. Past that instant the sentinel is NOT engaged, the lift is logged once, loudly, and the dead file is retired - guarded by a re-check of the engagement key, so a re-arm landing between the read and the unlink is never eaten. An empty (touched) file, a corrupt body or an unparsable stamp all stay ENGAGED: fail safe in both directions. `hermes pause --allow-user <id> [--allow-profile <p>] [--ttl 45m]` arms it (a --ttl that will not parse is refused before the sentinel is written); `hermes status` renders the reason, the deadman deadline and the allowlist. Tests: tests/cron/test_estop.py (deadman, allowlist, CLI and status contracts, plus the cron consumer lifting on expiry), tests/gateway/test_estop_allowlist.py (the gateway turn gate: exempt identity served, peer refused, missing allowlist admits nobody), tests/gateway/test_control_socket_pause.py (control traffic is never paused, so a paused fleet is still reachable). Agent: platform-coder
The pause-allowlist tests described the exempt and refused cases with this fleet's own bot profile names - 11 string literals across tests/cron/test_estop.py and tests/gateway/test_estop_allowlist.py. A lane name is internal identification, not a fixture detail, and it does not belong in world-readable history: the ALLOWLISTED example profile is now the neutral `primary-lane`, and both BLOCKED cases are `other-lane`. Rename only. No production code, and every assertion keeps its polarity and shape: the allowlisted profile is still admitted (`is None`), an unlisted profile is still refused (`is not None`). The suite count is unchanged. Agent: platform-coder
alt-glitch
added
type/feature
andrexibiza
left a comment
andrexibiza
left a comment
There was a problem hiding this comment.
Security review — exact head 1bfac93481a19c7d33e4687f75186e4a7a0bfead.
I selected this as the highest-leverage unreviewed security/safety boundary in the current candidate set because ESTOP is a shared admission control across gateway turns, cron, and Kanban. A mistake here changes who may act while the fleet is intentionally held, and whether a fresh hold survives concurrent recovery.
I found three P1 authority/lifecycle blockers, all in the new ESTOP primitive rather than pre-existing behavior:
- Expired-sentinel retirement is not generation-safe. The final key comparison and
unlink()are separate filesystem operations. A concurrent re-arm can land after the equality check and before the unlink, so the expiry path can delete the successor engagement it was supposed to preserve. - Profile and fleet-root holds do not compose.
is_engaged()correctly treats any active candidate sentinel as a hold, butget_state()returns the first parsable one andis_allowed()authorizes against only that state. A permissive profile sentinel can therefore bypass a simultaneous stricter fleet-root ESTOP. Multiple restraints need deny-wins/intersection semantics, not path-order precedence. - A profile name is promoted from routing coordinate to bypass authority.
is_allowed()ORsprofiledirectly with authenticateduser_id, so any otherwise-authorized caller routed through an allowlisted profile is exempt regardless of principal identity. Under #93943/#95028, coordinates select; they do not grant authority. A maintenance profile can narrow an explicit principal grant, but should not independently mint one.
These are the same defect family from three sides: stale/path-local coordinates are being trusted where the mutation/admission edge needs explicit current authority, exact scope, and generation identity. The relevant interlocks are #121053, #93943 (Nothing Ambient Survives), #90144 (proof scope = mutation scope), #90145 (generation-fenced recovery/teardown), #92618 (security assurance), and #95028 (Authority Execution Layer).
Required closure shape: make retirement consume an exact engagement generation atomically enough that a successor cannot be deleted; aggregate all active ESTOP authorities so every active hold must authorize an exemption; and bind any maintenance-lane scope to an authenticated principal instead of treating profile as authority by itself. Add deterministic regressions for the re-arm race, simultaneous profile+root sentinels in both strict/permissive orders, unreadable/corrupt overlap, expired+fresh overlap, and wrong-user/right-profile admission.
Exact-object evidence: PR base 7135182569f7b3773e517717798bca03d5d91ebf; current main at review 58c896ea4ebaff5425a068f461b6c6fedadb8b40; merge base 068db016fbfb3e9b44169de154781908f91bceae. The head is 82 commits behind current main, but main drift itself is not a finding and I am not asking for a rebase on that basis. Exact-head hosted CI, Docker, and Nix runs are all action_required, so there is no hosted green receipt on this object. The PR reports 51 targeted tests passing locally; I did not independently execute the branch test suite in this review environment. No independent 5×2×3 acceptance is claimed here.
The deadman/allowlist direction is coherent and the corrupt/unreadable-body fail-safe is the right instinct; the three blockers are specifically where the new exemptions and retirement cross authority/lifecycle boundaries. This head is not clear for merge until those classes are closed and fresh exact-head evidence exists.
| return | ||
| removed = False | ||
| try: | ||
| if _engagement_key(path) == key: |
There was a problem hiding this comment.
P1 — this is still a check-then-unlink race, not a generation fence. _engagement_key(path) == key and path.unlink() are separate filesystem operations. An expired engagement can pass this equality check, then another process can re-arm the same sentinel path, and this call will unlink the new engagement. That directly contradicts the stated guarantee that a re-arm between read and unlink is never eaten. Please make retirement operate on an exact engagement generation with a race-safe protocol (lock/CAS/rename/inode-generation ownership as appropriate), and add a deterministic regression that interleaves re-arm after the final validation but before retirement.
| "expires_at": payload.get("expires_at") or None, | ||
| "allow": _normalize_allow(payload.get("allow")), | ||
| } | ||
| break |
There was a problem hiding this comment.
P1 — active ESTOP authorities are composed by path order instead of deny-wins. is_engaged() treats any active candidate sentinel as a hold, but get_state() breaks on the first readable payload ([profile, fleet-root]) and is_allowed() then authorizes against only that payload. A permissive profile ESTOP can therefore exempt a turn even while a simultaneous fleet-root ESTOP is active with no exemption (or a stricter one). Multiple active restraints need to compose by intersection / “every active hold authorizes this exemption”; an unreadable active hold must deny. Please add profile+root overlap regressions for strict/permissive order, corrupt/unreadable, and expired+fresh combinations.
| allow = state.get("allow") or {} | ||
| if user_id is not None and str(user_id) in (allow.get("user_ids") or []): | ||
| return True | ||
| return bool(profile) and str(profile) in (allow.get("profiles") or []) |
There was a problem hiding this comment.
P1 — profile is a routing coordinate here, not an authenticated principal. This OR means any otherwise-authorized caller whose event is routed through an allowlisted profile bypasses ESTOP regardless of user_id; the new test explicitly blesses that behavior. Under the repository’s #93943/#95028 authority contract, a profile name may select/narrow an operation but cannot itself grant authority. Keep maintenance-profile scope bound to an explicit authenticated principal/current authority object (for example principal + exact profile/connection scope), and add a wrong-user/right-profile regression that remains paused.
Absorb upstream 28e6496 (base was e8acdc5) so the PR is mergeable again. One conflict, agent/estop.py. Upstream 6e294f5, the fleet-wide "read source files with encoding=utf-8-sig" sweep, switched that file's one sentinel read to utf-8-sig; this branch had refactored the same read into _read_payload(). Resolved as a union: the helper stays, and it adopts the BOM-tolerant decode. The union is behavioural, not textual — plain utf-8 leaves the BOM on the string, so a BOM'd body (a Windows editor writes one) parses as junk and is dropped, which fails safe but silently: the reason disappears and a past expires_at reads as "no expiry", so a deadman hold would never lift. test_bom_encoded_sentinel_reads_like_a_plain_one pins both halves; it fails on the pre-merge tip (1 failed, 38 deselected) and passes here. Conflicts in the PR's other files were auto-resolved without overlap (upstream's Termux-status, sessions-file encoding, steer-fold and platforms-marker changes). Agent: platform-stl
Two blockers on
|
There was a problem hiding this comment.
Security review — exact head 0bf513902a55c307c0ba08c60c63fd419f91275c
Re-review at a moved head. My earlier review object (5300434480) and its three inline findings were pinned to 1bfac93481a19c7d33e4687f75186e4a7a0bfead; this head adds upstream main through merge commits cb327c7 and 0bf5139 (merge base e9c60209a786891bb56dfbd7a5151d6cdda7ac0e). @Enough1122 also posted two blockers in the timeline at fa09d438. Every item below was re-checked at this head, and each was reproduced locally.
Assessment
The deadman and allowlist direction is coherent, and treating a corrupt or unreadable body as still-engaged is the right fail-safe; the BOM-tolerant read added by the merge is a real improvement. All four blockers are in the newly added surface: three are prior findings this head did not address, and the fourth (re-engage stripping the exemption and the deadline) was reported by @Enough1122 and is reproduced here together with a sibling path they did not cover.
The shared shape is that the two new fields are treated as payload contents rather than as a stateful engagement with an identity, a scope, and a lifecycle — so a stale decision can delete a fresh hold, a sibling file can outvote a stricter hold, a route can mint authority, and a re-arm can erase the guarantee. The repairs compose into one coherent model (engagement identity, deny-wins composition, principal-bound scope, non-destructive re-arm) without adding a second mechanism.
Blockers
F-001 — expired-sentinel retirement is not generation-safe
P1. Retirement re-derives the engagement identity from the file at call time instead of consuming the engagement the expiry decision examined.
A re-arm landing between the expiry decision and retirement is deleted, cancelling a hold an operator just armed.
Trigger. A re-arm landing between another process's expiry decision and its retirement — exactly the hermes pause --ttl <dur> re-arm the module's own WARNING line instructs operators to run. The deadman firing is precisely when an operator re-arms.
Evidence. repro/F-001.py injects the re-arm after the expiry decision and before retirement through the real is_engaged() path; the fresh hold is deleted (fresh_hold_survived=false, sentinel_exists_after=false, exit 1). Only the interleaving point is instrumented; the code under test is unmodified.
Required repair. Fence retirement to the examined engagement's identity and remove only that generation, atomically.
Regression contract. Deterministic interleavings: re-arm after the expiry decision and before retirement, and re-arm inside the unlink interval; the successor must survive and is_engaged() be True.
F-002 — active holds are composed by path order, not deny-wins
P1. Multiple active holds are composed by path order because get_state() stops at the first parsable payload and is_allowed() consults only that one.
A permissive profile hold exempts a turn while a strict fleet-root hold is active; a corrupt active hold admits through a permissive sibling.
Trigger. Two active holds on disk — the documented profile-gateway case (HERMES_HOME=/profiles/ plus the operator's /ESTOP) — where only one carries allow.
Evidence. repro/F-002.py writes both sentinels and reads the real is_engaged()/is_allowed(): strict root + permissive profile -> is_allowed(op-7)=True (expected False); permissive root + strict profile -> False; corrupt profile + permissive root -> True (expected False); exit 1.
Required repair. Aggregate all active holds with deny-wins semantics; an unreadable hold denies.
Regression contract. Profile+root overlap in both orders, corrupt/unreadable overlap, expired+fresh overlap; each asserts the conjunction — served only when every active hold admits.
F-003 — a profile name is a routing coordinate, not an authenticated principal
P1. The profile key is ORed with the authenticated identity, so the serving route grants the exemption by itself.
Any principal routed through an allowlisted profile bypasses the pause without ever being granted the exemption.
Trigger. hermes pause --allow-profile <name> on a gateway that multiplexes several principals onto one serving profile. The new test test_profile_key_is_the_secondary_fallback_for_a_lane pins that behaviour.
Evidence. repro/F-003.py arms allow={'profiles': ['lane-a']} and runs the real GatewayRunner._hm_estop_gate: a non-allowlisted user_id routed via lane-a is served (refused=false) while the same user on lane-b is refused; is_allowed('peer-uid','lane-a') is True; exit 1.
Required repair. Bind any maintenance-lane scope to an authenticated principal; a profile may narrow, not grant.
Regression contract. Wrong-user/right-profile remains paused; right-user/any-profile served only where identity alone admits; a multiplexed-routing case pinning that no principal is exempt by route alone.
F-004 — a re-arm silently strips the exemption and the deadman
P1. Every re-engage path rebuilds the sentinel payload from scratch, so the new allow and expires_at fields are dropped.
An admitted peer or a plain CLI re-arm removes the operator's exemption and the deadline, locking the operator out and making the pause unbounded.
Trigger. Any authorized sender whose /pause <reason> the gate admits (recognized slash commands pass the gate by design), or any plain hermes pause --reason ... on an already-armed pause.
Evidence. repro/F-004.py: (1) arms allow={'user_ids': ['operator-uid-7']} with a 45m deadman, runs the real gate and then the real _handle_pause_command for a non-allowlisted sender — the command is admitted and the payload ends with allow={} and expires_at=None, is_allowed(operator) False; (2) the real cmd_pause re-arm with no flags strips both fields. First reported by @Enough1122 for the in-band path; reproduced here with the CLI sibling.
Required repair. Preserve existing allow/expires_at across re-engagement unless explicitly replaced, or refuse the overwrite without an explicit reset.
Regression contract. After arming with allow+ttl, each of a direct engage(reason=...), the in-band /pause <reason> path, and the no-flag CLI re-arm leaves allow and expires_at intact (or is refused), with is_allowed(operator) still True.
Prior items at this head
| Item | Where | Status |
|---|---|---|
| P1 — retirement check-then-unlink race | inline 4090446400 at 1bfac93 |
Still present; re-anchored as F-001 and strengthened: retirement recomputes its own key, so it can delete a successor engagement outright rather than only racing the unlink. |
| P1 — authorities composed by path order | inline 4090446405 at 1bfac93 |
Still present as F-002; independently corroborated by @Enough1122 (their blocker 2). |
| P1 — profile promoted to bypass authority | inline 4090446407 at 1bfac93 |
Still present as F-003. |
P1 — in-band /pause strips allowlist + deadman |
@Enough1122, timeline at fa09d438 |
Still present as F-004; reproduced here with the CLI sibling path and the operator-lockout consequence. |
Evidence and limits
Runtime: Python 3.11.11 on Windows 11, one isolated HERMES_HOME per probe, run against a worktree pinned at this exact head with the code under test unmodified (only F-001's interleaving point is instrumented at the module seam).
Suite receipt at this head: tests/gateway/test_estop_allowlist.py + tests/gateway/test_control_socket_pause.py + tests/cron/test_estop.py -> 50 passed, 2 skipped (posix-only socket transport), 0 failed. That is a suite-execution receipt, not independent behavioural evidence: none of the four interleavings, overlaps, or re-arms below is covered by those tests, which is why they are green while these defects are live.
Hosted CI has no green receipt on this object: check-runs at the exact head returns total_count=0, and the CI, Docker, and Nix workflow runs are all action_required (fork approval). Main tip at review time is 2f14d5e6e4e99eb6e663008513e6be04d83c4f22; the head is six commits behind. Drift is not a finding and I am not asking for a rebase on that basis.
Not exercised: a live multiplexed gateway with two simultaneous holds (F-002 and F-003 are reproduced against the predicate and the real gate function, not through a running adapter), and the full control-socket path for F-004.
Merge posture
This head is not clear for merge until the four authority/lifecycle classes are closed and fresh exact-head evidence exists. Each blocker is individually actionable from its inline comment, and the required regressions are listed per finding.
| return | ||
| removed = False | ||
| try: | ||
| if _engagement_key(path) == key: |
There was a problem hiding this comment.
P1 — expired-sentinel retirement is not generation-safe
Expired-sentinel retirement is not bound to the engagement the expiry decision examined: is_engaged() decides a sentinel is expired, then _retire_expired() re-derives the engagement key from the file at call time and unlinks when that self-derived key equals itself, so a successor engagement written in between is read as current and deleted.
Trigger. A re-arm landing between another process's expiry decision and its retirement — exactly the hermes pause --ttl <dur> re-arm the module's own WARNING line instructs operators to run. The deadman firing is precisely when an operator re-arms.
Impact. A hold an operator just armed is destroyed and the fleet resumes unpaused, so authority over who may execute is settled by a stale decision plus a self-referential guard. The PR's stated guarantee ('a re-arm landing between the read and the unlink is never eaten') does not hold.
Evidence. repro/F-001.py injects the re-arm after the expiry decision and before retirement through the real is_engaged() path; the fresh hold is deleted (fresh_hold_survived=false, sentinel_exists_after=false, exit 1). Only the interleaving point is instrumented; the code under test is unmodified.
Required repair. Consume the examined engagement's identity in retirement: capture it when the expiry decision is made, re-verify the on-disk engagement still matches immediately before removal, and remove only that generation through an atomic operation (rename or handle-bound unlink). A successor engagement must never be deletable by its predecessor's retirement.
Regression contract. Deterministic interleavings: re-arm after the expiry decision and before retirement, and re-arm inside the unlink interval; the successor must survive and is_engaged() be True.
| "expires_at": payload.get("expires_at") or None, | ||
| "allow": _normalize_allow(payload.get("allow")), | ||
| } | ||
| break |
There was a problem hiding this comment.
P1 — active holds are composed by path order, not deny-wins
Multiple active holds are composed by path order: is_engaged() treats any active candidate sentinel as a hold, but get_state() breaks on the first parsable payload (profile home, then fleet root) and is_allowed() authorizes against only that state.
Trigger. Two active holds on disk — the documented profile-gateway case (HERMES_HOME=/profiles/ plus the operator's /ESTOP) — where only one carries allow.
Impact. A permissive hold's allowlist authorizes a turn while a stricter hold is active, so the strict restraint is bypassed and the verdict is decided by read order. A corrupt active hold admits a caller through a permissive sibling instead of denying, contradicting the declared 'no allowlist — or an unreadable sentinel — admits nobody' contract.
Evidence. repro/F-002.py writes both sentinels and reads the real is_engaged()/is_allowed(): strict root + permissive profile -> is_allowed(op-7)=True (expected False); permissive root + strict profile -> False; corrupt profile + permissive root -> True (expected False); exit 1.
Required repair. Compose every active hold: an exemption requires all active holds to authorize it (deny-wins intersection), and an unreadable active hold must deny. is_allowed() must aggregate over _candidate_sentinel_paths() rather than one get_state() payload.
Regression contract. Profile+root overlap in both orders, corrupt/unreadable overlap, expired+fresh overlap; each asserts the conjunction — served only when every active hold admits.
| allow = state.get("allow") or {} | ||
| if user_id is not None and str(user_id) in (allow.get("user_ids") or []): | ||
| return True | ||
| return bool(profile) and str(profile) in (allow.get("profiles") or []) |
There was a problem hiding this comment.
P1 — a profile name is a routing coordinate, not an authenticated principal
A profile name is treated as bypass authority: is_allowed() ORs the serving profile with the authenticated user_id, so an allowlisted profiles entry admits any otherwise-authorized caller whose turn is routed through that profile, regardless of the principal.
Trigger. hermes pause --allow-profile <name> on a gateway that multiplexes several principals onto one serving profile. The new test test_profile_key_is_the_secondary_fallback_for_a_lane pins that behaviour.
Impact. A routing coordinate grants an admission exemption the authenticated principal never received. Under the repository's #93943/#95028 authority contract, a profile name may select or narrow an operation; it cannot mint authority.
Evidence. repro/F-003.py arms allow={'profiles': ['lane-a']} and runs the real GatewayRunner._hm_estop_gate: a non-allowlisted user_id routed via lane-a is served (refused=false) while the same user on lane-b is refused; is_allowed('peer-uid','lane-a') is True; exit 1.
Required repair. Bind the maintenance-lane exemption to an authenticated principal — e.g. require an accompanying user_ids grant or carry principal+profile scope on a current authority object — so a profile narrows a grant instead of minting one. If profile-as-authority is intended, it contradicts the landed authority architecture and needs an explicit maintainer decision rather than a silent OR.
Regression contract. Wrong-user/right-profile remains paused; right-user/any-profile served only where identity alone admits; a multiplexed-routing case pinning that no principal is exempt by route alone.
| seconds = parse_duration(ttl) | ||
| if seconds: | ||
| expiry = now + timedelta(seconds=seconds) | ||
| payload: dict = {"engaged_at": now.isoformat(), "reason": reason or None} |
There was a problem hiding this comment.
P1 — a re-arm silently strips the exemption and the deadman
Re-engaging builds a fresh payload and drops the two new fields: gateway/run_busy.py::_handle_pause_command calls estop.engage(reason=args or None), and hermes_cli/subcommands/pause.py::cmd_pause passes empty/None allow+ttl when no flags are given, so an admitted /pause <reason> or a plain hermes pause --reason ... erases the operator's exemption and the deadman.
Trigger. Any authorized sender whose /pause <reason> the gate admits (recognized slash commands pass the gate by design), or any plain hermes pause --reason ... on an already-armed pause.
Impact. A peer can strip the operator's exemption — locking the operator out of their own pause — and remove the deadline, converting a bounded maintenance window into exactly the unbounded, human-required-to-lift pause this PR exists to eliminate.
Evidence. repro/F-004.py: (1) arms allow={'user_ids': ['operator-uid-7']} with a 45m deadman, runs the real gate and then the real _handle_pause_command for a non-allowlisted sender — the command is admitted and the payload ends with allow={} and expires_at=None, is_allowed(operator) False; (2) the real cmd_pause re-arm with no flags strips both fields. First reported by @Enough1122 for the in-band path; reproduced here with the CLI sibling.
Required repair. Make re-engagement non-destructive at the shared engage() boundary: preserve an existing allow/expires_at when the caller does not explicitly supply replacements (distinguishing omitted from explicitly-empty), or refuse to overwrite an armed pause without an explicit reset (--force / /pause force).
Regression contract. After arming with allow+ttl, each of a direct engage(reason=...), the in-band /pause <reason> path, and the no-flag CLI re-arm leaves allow and expires_at intact (or is refused), with is_allowed(operator) still True.
P2 — a
|
--ttl |
result | sentinel | is_engaged() |
|---|---|---|---|
45m (control) |
rc=0, armed |
yes | True |
banana (control) |
rc=2, "NOT pausing." |
no | False |
2900000d (largest that works) |
rc=0, armed |
yes | True |
2915000d, 3000000d, 999999999d |
OverflowError | no | False |
The operator gets a traceback instead of the ⛔ Invalid --ttl refusal, and nothing is armed — cron, kanban and the gateway turn gate keep running as if the pause had succeeded. That inverts the guarantee written 40 lines above it at agent/estop.py:217-219: "An unusable ttl still engages — it never half-arms a pause — and is reported by the CLI." Base has no --ttl and no parse_duration at all, so this path is new here. It does not corrupt an armed pause: a failed re-arm leaves the old sentinel intact.
Suggested repair: bound the resolved expiry in the guard that already refuses banana, or wrap estop.py:224-226 so out-of-range seconds falls back to expiry = None, which already engages without a deadman.
Unverified / please confirm: I did not run the real GatewayRunner._hm_estop_gate — the failure is entirely inside engage() + cmd_pause, both executed for real. Please confirm which bound you prefer (reject at the CLI vs. clamp to no-deadman), and whether any caller outside cmd_pause passes a user-controlled ttl to engage().
This branch has not been deployed
4 participants
What does this PR do?
hermes pauseis all or nothing, and it never lifts on its own.Two consequences. The operator cannot work through their own pause:
agent/estop.py::is_engaged()is the single predicate behind the gateway turn gate, the cron tick and the kanban dispatcher, so
an armed pause blocks the operator's new turns exactly like everyone else's, and the in-band
/pause offneeds a turn the pause is blocking. And a pause outlives its reason when the job thatarmed it dies: the sentinel records
engaged_atandreasonbut no deadline, so a crashedmaintenance window parks every consumer until a human removes the file by hand.
This adds two fields to that one sentinel and one branch to the gate that reads it. Same sentinel,
same predicate, no second mechanism.
allow.user_ids/allow.profilesexempt an authenticated identity.agent.estop.is_allowed()is consulted at the top of
gateway/run_inbound.py::_hm_estop_turn_allowed, ahead of the commandcheck, so an allowlisted turn is served while everyone else still gets the pause notice.
user_idis the primary key because it survives a platform or profile change;
profileis the secondary keyfor naming a maintenance lane directly. Fail-closed: no allowlist, or a body that cannot be read,
admits nobody.
expires_atis a deadman. Past that instantis_engaged()reports not engaged, the lift is loggedonce at WARNING, and the dead sentinel is retired. The unlink re-checks the engagement's identity
first, so a re-arm landing between the read and the unlink is never eaten. An empty, corrupt or
unparsable body stays engaged.
hermes pause --allow-user <id> [--allow-profile <name>] [--ttl 45m]arms it, and an unusable--ttlis refused before the sentinel is written.hermes statusrenders the reason, the deadlineand the allowlist.
One disclosure up front: this is two behaviors on one sentinel, not one. They answer the same gap
(a pause that cannot be survived and cannot end), they touch the same two functions, and splitting
them means two PRs against the same predicate. Say the word and I will split them.
Related Issue
Fixes #121053
Type of Change
Changes Made
agent/estop.py: the sentinel gainsallowandexpires_at;is_engaged()is expiry-aware andretires a dead sentinel (
_parse_stamp,_is_expired,_retire_expired,_engagement_key);new
is_allowed()and_normalize_allow();engage()takesallow/ttl/expires_at;get_state()reports the two new fields;paused_reply()andcheck_paused()name theauto-resume when there is one. New local
parse_duration().gateway/run_inbound.py: the allowlist branch at the top of_hm_estop_turn_allowed(line 264),plus its docstring.
hermes_cli/subcommands/pause.py:--allow-user,--allow-profile,--ttl.hermes_cli/status.py: the pause line carries reason, deadline and allowlist.tests/cron/test_estop.py,tests/gateway/test_control_socket_pause.py, and newtests/gateway/test_estop_allowlist.py.How to Test
hermes pause --reason window --allow-user <your-id> --ttl 45m, thenhermes statusshows thereason,
auto-resumes <stamp> (deadman TTL)andallowed through: <your-id>.pause notice still comes back.
hermes resumelifts it. Then re-arm with--ttl, and it lifts itself, with one WARNING linenaming the sentinel.
hermes pause --ttl bananarefuses withNOT pausingand leaves the sentinel untouched.scripts/run_tests.sh tests/cron/test_estop.py tests/gateway/test_estop_allowlist.py tests/gateway/test_control_socket_pause.pygives 51 passed, 0 failed.For the red side of that last one: revert the four source files to the parent commit and keep the
tests, and the same run gives 18 failed, 26 passed with
tests/gateway/test_estop_allowlist.pyfailing to collect (it imports
is_allowed). Revert onlygateway/run_inbound.pyand 4 of the 7tests in that file fail, one of them on the operator turn that should have been served.
Checklist
Code
feat(estop):; theestopscope is already in this repo's history, e.g.fix(estop): honor canonical ~/.hermes/ESTOP from profile gateways)scripts/run_tests.sh tests/cron/test_estop.py tests/gateway/test_estop_allowlist.py tests/gateway/test_control_socket_pause.py→ 3 files, 51 tests passed, 0 failed (log below). The fullpytest tests/ -qwas not run: fork PRs sit ataction_requiredhere until a maintainer approves the workflow run, so it has not had a chance to speak.Documentation & Housekeeping
docs/, docstrings) — module and function docstrings only; no README ordocs/page describes the pausecli-config.yaml.exampleif I added/changed config keys — N/A, no config keysCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/Apathlib,datetimeat explicit UTC, no signals, no/proc, no hardcoded temp paths; the sentinel is a plain fileFor New Skills
Not applicable, no skill added.
Screenshots / Logs
Green, at this commit, with the repo's own runner:
Red, the four source files reverted to the parent commit with the tests kept:
Red, only
gateway/run_inbound.pyreverted:The other two consumers were exercised directly, against a snapshot of a live board and a scratch
home, rather than through their dispatch wrappers. A due one-shot job reports
executed=0with 0executionsrows while the pause holds, thenexecuted=1with 1 row after the deadman expires,leaving
next_run_atunchanged across both ticks. The dispatcher writesspawned=0across twoticks with 68 ready cards while the gate is held, then 16 spawned on the control tick after the
expiry. Both hold, and both lift.
Finally, one thing for you to weigh rather than something I am hiding:
parse_durationlands inagent/estop.pyas the third copy in this tree (cron/jobs.pyreturns minutes and raises;hermes_cli/session_filters.pyis CLI-facing). Importingcronfromagentwould invert thelayering, so this one is local and returns seconds or None. Consolidating all three into one shared
helper is a clean follow-up if you would rather have it in this PR.
Authored with AI assistance (Hermes Agent); I reviewed the diff.