Repository navigation
fix(gateway): per-session quiescence + task-liveness reaper (busy-gateway deferred restart never fires) - #137
Conversation
🔎 Lint report:
|
| Rule | Count |
|---|---|
unsupported-operator |
1 |
unresolved-attribute |
1 |
not-subscriptable |
1 |
invalid-assignment |
1 |
First entries
tests/gateway/test_quiescence_status.py:49: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["active_agent_keys"]` and `dict[str, Any] | None`
tests/gateway/test_quiescence_status.py:39: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `None` in union `dict[str, Any] | None`
tests/gateway/test_quiescence_status.py:81: [not-subscriptable] not-subscriptable: Cannot subscript object of type `None` with no `__getitem__` method
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`
✅ Fixed issues (2):
| Rule | Count |
|---|---|
unresolved-attribute |
2 |
First entries
run_agent.py:3129: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`
Unchanged: 6426 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
|
| Filename | Overview |
|---|---|
| gateway/run.py | Core runtime changes: adds _running_agent_tasks dict, task-liveness reaper loop (_reap_dead_running_agents + _reap_dead_running_agents_loop), task capture in _handle_message, and task-handle cleanup in _release_running_agent_state. Grace-period logic and per-entry crash isolation are sound. |
| gateway/status.py | Adds active_agent_keys parameter and schema_version=2 to write_runtime_status; switches _write_json_file to mode=0o600 (TOCTOU-safe atomic write). Read-modify-write semantics mean an omitted active_agent_keys retains the previous value — intentional and handled by _persist_active_agents always passing the field. |
| tests/gateway/test_quiescence_reaper.py | New test file covering all key reaper invariants: dead-task reaped (AC-4), live-long-running task survives (AC-5), registration-window grace (AC-5b), crash isolation (AC-6), idempotent double-release (AC-10), and the async loop's continued execution after a sweep exception. |
| tests/gateway/test_quiescence_status.py | New test file covering active_agent_keys persistence, schema_version=2, back-compat when the field is omitted, explicit empty-list semantics, and the 0o600 file-mode requirement. |
| tests/gateway/test_quiescence_task_tracking.py | New test file verifying task-handle release on slot clear, idempotent double-release, _persist_active_agents writing keys, and sentinel exclusion from the snapshot. |
| tests/gateway/test_session_race_guard.py | Existing race-guard test updated to include _running_agent_tasks = {} in the test runner fixture, keeping it in sync with the new dict added to __init__. |
| tests/gateway/test_status.py | Existing status test updated to expect mode=0o600 in the atomic_json_write call signature, correctly reflecting the new 0600 file permission. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant H as _handle_message
participant RA as _running_agents
participant RAT as _running_agent_tasks
participant PAA as _persist_active_agents
participant WRS as write_runtime_status
participant Reaper as _reap_dead_running_agents_loop
Note over H: Turn starts (NO await before task capture)
H->>RA: "[key] = real_agent"
H->>RAT: "[key] = asyncio.current_task()"
H->>PAA: _persist_active_agents()
PAA->>WRS: "active_agents=N, active_agent_keys=[key,...]"
Note over Reaper: Periodic sweep (~60s)
loop each _running_agents entry
alt sentinel
Reaper-->>Reaper: skip (never reap)
else "task.done() == False"
Reaper-->>Reaper: skip (live turn, any age)
else "task.done() == True"
Reaper->>RA: _release_running_agent_state(key)
RA->>RAT: pop(key)
RA->>PAA: _persist_active_agents()
PAA->>WRS: "active_agent_keys=[...without key]"
else "no task + age <= 30s"
Reaper-->>Reaper: skip (registration window)
else "no task + age > 30s"
Reaper->>RA: _release_running_agent_state(key)
end
end
Note over H: Turn ends (normal unwind)
H->>RA: _release_running_agent_state(key)
RA->>RAT: pop(key)
RA->>PAA: _persist_active_agents()
PAA->>WRS: "active_agents=N-1, active_agent_keys=[...without key]"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant H as _handle_message
participant RA as _running_agents
participant RAT as _running_agent_tasks
participant PAA as _persist_active_agents
participant WRS as write_runtime_status
participant Reaper as _reap_dead_running_agents_loop
Note over H: Turn starts (NO await before task capture)
H->>RA: "[key] = real_agent"
H->>RAT: "[key] = asyncio.current_task()"
H->>PAA: _persist_active_agents()
PAA->>WRS: "active_agents=N, active_agent_keys=[key,...]"
Note over Reaper: Periodic sweep (~60s)
loop each _running_agents entry
alt sentinel
Reaper-->>Reaper: skip (never reap)
else "task.done() == False"
Reaper-->>Reaper: skip (live turn, any age)
else "task.done() == True"
Reaper->>RA: _release_running_agent_state(key)
RA->>RAT: pop(key)
RA->>PAA: _persist_active_agents()
PAA->>WRS: "active_agent_keys=[...without key]"
else "no task + age <= 30s"
Reaper-->>Reaper: skip (registration window)
else "no task + age > 30s"
Reaper->>RA: _release_running_agent_state(key)
end
end
Note over H: Turn ends (normal unwind)
H->>RA: _release_running_agent_state(key)
RA->>RAT: pop(key)
RA->>PAA: _persist_active_agents()
PAA->>WRS: "active_agents=N-1, active_agent_keys=[...without key]"
Reviews (2): Last reviewed commit: "feat(gateway): task-liveness reaper for ..." | Re-trigger Greptile
Phase 1 of busy-gateway-quiescence. write_runtime_status gains an additive active_agent_keys param (the live running-session keys, not just the count) so the safe-restart watcher can do per-session quiescence. Adds schema_version:2. The file now writes 0600 (TOCTOU-safe via atomic_json_write mode=) since active_agent_keys carry chat-id+user-id multi-user metadata (D-1/RC-5). Back-compat: absent arg → field omitted (INV-6). 6 new tests; updated the atomic_json_write change-detector for mode=. Spec v0.4; pinned fork HEAD 0e21a7b.
…e 1 done) Tasks 1.2-1.4 of busy-gateway-quiescence: - _running_agent_tasks dict; capture asyncio.current_task() synchronously at the slot-set (RC-1 no-await invariant, guarded by a load-bearing comment — turn runs inline so current_task() IS the turn task, no registration-ordering window). - Clear the handle in _release_running_agent_state (single chokepoint, idempotent → AC-10). - _persist_active_agents writes active_agent_keys (live non-sentinel session keys). - Self-heal (getattr/hasattr) for object.__new__ test partials; fixed the test_session_race_guard helper to seed the new attr. 7 new tests; 161 release/persist-caller regression tests green. Spec v0.4.
…hase 2) Periodic background sweep that reclaims _running_agents entries whose turn TASK is genuinely done()/cancelled (a slot leaked when a turn coroutine was killed without unwinding — hard kickstart SIGKILL, handler crash). A leaked entry inflates active_agents forever, making the safe-restart/safe-reboot quiescence wait (active_agents==0) unreachable. Amputation-safe by construction (spec D-3, the Opus-review B1 killer): - evicts ONLY on task.done(); a LIVE task is NEVER reaped, any age (AC-5) — does NOT reuse the :8861 idle/wall-age predicate, which is unsafe proactively because _touch_activity is dead code (the idle signal is fake). - a non-sentinel entry with no task yet recorded survives a 30s grace window (AC-5b, the B1-NEW registration-window killer). - pending sentinels never reaped. Crash-safe: per-entry try/except + outer loop wrap (a dead reaper would silently regress to the bug it fixes — AC-6/INV-5); REAPER_EVICTED structured log + heartbeat stamp. Idempotent _release → reaper/finally double-release safe (AC-10). Interval config HERMES_RUNNING_AGENT_REAP_INTERVAL (60s default). Launched in _background_tasks at startup. 12 tests incl. live-survives, registration-window, concurrency, crash-safety. Benefits BOTH safe-restart and safe-reboot (shared gateway). Spec v0.4.
1c6a8e6 to
75fefdf
Compare
…bservability + reboot-parity proof (#142) * fix(gateway): auto-surface a self-initiated restart that was drain-interrupted (restart_consumed_interrupted) A turn whose last act fires safe-restart.py sets _session_initiated_restart, so on shutdown the F1/F2 cascade-breaker stamps restart_consumed — deliberately EXCLUDED from _AUTO_RESUME_REASONS to break the restart->resume->restart cascade. But if that same turn was ALSO still running when the 180s drain timed out (genuinely interrupted in-flight work), it fell in a seam: the cascade guard suppressed the auto-wake AND #136 preserve-and-prompt never got to surface it, so the session stayed silent until the next inbound message (observed live 2026-06-30 20:30 — Apollo's Discord turn preserved but not surfaced). Fix (restart-only; reboot is unaffected — proven): a new resume_reason restart_consumed_interrupted, stamped when a self-initiated restart is marked at the POST-DRAIN-TIMEOUT site (interrupted=True) rather than the pre-drain speculative site. It IS in _AUTO_RESUME_REASONS (so startup auto-resume proactively surfaces the interrupted work, surface-and-wait, no tool replay), but still records the F2 replay-mark, so a genuine restart->resume->restart loop is still bounded and suspended at threshold within the 300s window. - _resume_reason_for_shutdown_mark gains interrupted kwarg; _self_initiated helper also treats an entry already carrying restart_consumed[_interrupted] as self-initiated. - Only the post-timeout mark site passes interrupted=True (the pre-drain speculative mark, which is cleared on graceful finish, stays interrupted=False). - _resume_reason_phrase maps the new reason to 'a gateway restart'. Reboot parity (Task C, proof-not-code): a self-initiated REBOOT never sets _session_initiated_restart (the detector matches literal safe-restart.py, not safe-reboot.py; reboot-watcher writes no breadcrumb), so it marks shutdown_timeout (already auto-resumes). Reboot inherits #136/#137 preservation via the shared SIGTERM spine unchanged. Tests: 7 cascade unit tests + 1 integration call-site gate (real stop() through the drain-timeout branch) + 2 reboot-parity tests. The integration test is MUTATION-PROVEN: flipping the post-timeout site to interrupted=False turns it RED. Change-detector allow-list test converted to an additive-invariant assertion. Spec: plans/2026-07-01_restart-reboot-continuity-observability-SPEC.md (pass-3 APPROVE, all RCs folded). * feat(gateway): PHASE= observability for the shutdown mark + boot-resume decisions Task B (spec 2026-07-01 restart-reboot-continuity). The gateway shutdown spine already logs phase TIMINGS ('Shutdown phase: drain done at +Xs', #140's 'Executor drain:', 'Auto-resume skipped') — that existing 'Auto-resume skipped' line is what let us diagnose the 20:30 seam. This adds the two per-session DECISION points that were invisible: - PHASE=shutdown_mark key=... reason=... interrupted=... replay_marked=... marked=... at _mark_resume_pending_for_shutdown — makes the restart_consumed vs restart_consumed_interrupted vs shutdown_timeout choice observable per session. - PHASE=boot_resume_scheduled key=... reason=... platform=... when a session is proactively resumed at boot (surface-and-wait). Both use the module logger at INFO (INV-3/D-8: no fsync, no rollover in the hot path — logging must never eat the 180s drain budget) and carry session key + flags ONLY, never transcript content (INV-6). Tests: PHASE-line-emitted, no-content-leak (INV-6), and a slow/blocking-handler latency gate (D-8 — proves the mark path doesn't fan out or block). Watcher-side PHASE= markers (safe-restart watcher.py, safe-reboot reboot-watcher.py) ship in the skills tree. * fix(gateway): address Opus code-review — real INV-6/INV-3 gates, no overclaim Opus code-review BLOCKed the first cut: two invariant tests passed vacuously. Fixed both to be genuine gates, plus the required-changes: - INV-6 (no transcript content): the test was a TAUTOLOGY — it declared a SECRET marker but never seeded it where the PHASE logger could reach it, so the assertion was true by construction. Now seeds the sentinel into the transcript (append_to_transcript), _pending_messages, and the running-agent object, then asserts it never appears in any PHASE= line. MUTATION-PROVEN: adding msg=%s to the log line turns it RED. - INV-3 (fail-open / non-blocking): the old test over-claimed 'non-blocking' with a bound its own docstring admitted didn't prove it. Replaced with an honest single-emit + FAIL-OPEN gate, and made the CODE genuinely fail-open (both PHASE logs wrapped in try/except so a raising/broken sink can't abort the resume-mark — losing the mark = losing the interrupted work). Dropped the overclaiming code comment (handler blocking is a gateway-logging-config concern, not this site's). - RC#3: added a direct reason-UPGRADE test (mark clean restart_consumed → mark interrupted → upgraded to restart_consumed_interrupted) so the overwrite can't silently regress if mark_resume_pending ever gains idempotency. - Residual #31 (reason drift): added a test proving a self-restart reason does NOT leak into a later unrelated shutdown (clear_resume_pending nulls resume_reason + the post-turn gate pops _session_initiated_restart → back to shutdown_timeout). - Test hygiene: the logger-level test now restores the prior level in finally. - Maintainability: promoted the 4-site magic literal to a named module constant _REASON_RESTART_CONSUMED_INTERRUPTED. 199 passed / 1 skipped. * fix(gateway): code-review-2 follow-ups — WARNING-level audit breadcrumb + restart_consumed constant Code-review pass-2 = APPROVE WITH CHANGES (0 blockers; both prior fake-gate BLOCKERs certified genuinely resolved). Two non-blocking RCs landed: - (SRE) PHASE breadcrumbs were logger.info → lost when a deployment runs gateway.run at WARNING+, exactly while diagnosing a bad restart. Made the mark log LEVEL-AWARE: an INTERRUPTED or replay-marked mark (the diagnostic case) and the boot_resume_scheduled breadcrumb now log at WARNING (survive a raised threshold, matching the sibling drain-timeout warning); a routine clean mark stays INFO so normal shutdowns don't spam WARNING. - (Maintainability) promoted the clean-sibling literal 'restart_consumed' to a named constant _REASON_RESTART_CONSUMED alongside its twin _REASON_RESTART_CONSUMED_INTERRUPTED, so the cascade-guard pair moves together. Residual #31 (reason drift) verified against the PRODUCTION path: _apply_post_turn_resume_gate (called post-turn at the run loop) pops _session_initiated_restart AND calls clear_resume_pending (nulls resume_reason) — so the drift the test proves absent cannot ship. Tests: WARNING-level-survives + clean-stays-INFO + caplog-propagation-pin. 209 passed / 1 skipped. --------- Co-authored-by: Apollo <apollo@ang.ventures>
…ead code) + hoist compaction_stats imports The #137 reaper comment justified ignoring the idle heuristic by claiming _touch_activity is dead code with a fake monotonically-growing idle signal. Verified against live code: _touch_activity (run_agent.py) IS wired at ~20 call sites (every API call, tool call, stream delta) and two watchdogs depend on it (cron inactivity kill, gateway poll-loop eviction). The reaper's task-liveness design is still correct — idleness is the wrong question for leaked slots (agent object may be GC'd; a quiet live turn can look idle) — but the stated rationale would mislead the next person touching restart lifecycle. Comment-only change in gateway/run.py. Also hoist function-local 'import hashlib' / 'from collections import Counter' (x4) in agent/compaction_stats.py to module level — _row_signature runs per-row in the A-floor fallback path. Tests: 108 compaction-stats tests green under worktree PYTHONPATH.
…ead code) + hoist compaction_stats imports (#163) The #137 reaper comment justified ignoring the idle heuristic by claiming _touch_activity is dead code with a fake monotonically-growing idle signal. Verified against live code: _touch_activity (run_agent.py) IS wired at ~20 call sites (every API call, tool call, stream delta) and two watchdogs depend on it (cron inactivity kill, gateway poll-loop eviction). The reaper's task-liveness design is still correct — idleness is the wrong question for leaked slots (agent object may be GC'd; a quiet live turn can look idle) — but the stated rationale would mislead the next person touching restart lifecycle. Comment-only change in gateway/run.py. Also hoist function-local 'import hashlib' / 'from collections import Counter' (x4) in agent/compaction_stats.py to module level — _row_signature runs per-row in the A-floor fallback path. Tests: 108 compaction-stats tests green under worktree PYTHONPATH. Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
What & why
A deferred safe-restart on a busy multi-session gateway never fires. The watcher's⚠️ , and gives up without restarting — the gateway keeps running stale code/config.
wait_for_quiescencewaits for the gateway to go globally idle (active_agents == 0across every session). A gateway serving many channels + sub-agent fan-outs + cron turns rarely/never hits global-zero, so the watcher waits the fullQUIESCE_TIMEOUT, logsQUIESCE_TIMEOUT, posts aProven live 2026-06-30: a config change deferred-restarted the gateway; it waited >15 min and never fired while a second conversation + a burst of sub-agent sessions held
active_agentsat 2–3. (The first theory "the user kept re-activating it" was falsified — the gate is global; silence wouldn't have helped.)Secondary defect: leaked
_running_agentsentries have no background reaper — a turn coroutine killed without unwinding (hard SIGKILL mid-turn, handler crash) leaves a phantom entry makingactive_agents == 0permanently unreachable.Fix
gateway/status.pywrite_runtime_statusgainsactive_agent_keys(+schema_version,0600), so a watcher can ask "is my initiating session idle?" instead of "is the whole gateway idle?"._running_agentsslot iff its owning turn task isdone()/cancelled — a live long turn (a 30-min fan-out, a long fetch) is never reaped, any age. Amputation-safe by construction (capturescurrent_task()synchronously at the slot-set, no registration window; crash-safe per-iteration; idempotent release). This is the delicate part and is heavily unit-tested.Scope decision (Option A)
The spec originally proposed a gateway-side recheck at drain ("never interrupt the initiating turn"), but that rested on a false premise — the F2 restart-initiator breadcrumb is consumed at turn-end, before the post-quiescence kickstart ever drains, so a gateway-side recheck has nothing to read (logically vacuous). Per owner decision, Option A: accept that the shipped watcher per-session gate + the existing 180s graceful-drain + auto-resume is the complete fix. The bounded residual — a fresh >180s initiating turn landing in the poll→kickstart window — gets interrupt+auto-resume, identical to every turn on every restart today. No new code in the gateway shutdown/drain path (the highest-blast-radius zone).
Tests (22 focused, all green post-rebase)
tests/gateway/test_quiescence_status.py—active_agent_keyspublished,schema_version,0600, back-compat (missing field).tests/gateway/test_quiescence_task_tracking.py— the turn task is captured as the exactcurrent_task()with no await window.tests/gateway/test_quiescence_reaper.py— dead-task reaped; live-long-task survives; pending-sentinel survives; registration-window survives; concurrent register/release no-KeyError; crash-safe.Watcher-side Phase 3pt1 (both
safe-gateway-restartandsafe-rebootwatchers) ships via the skills/home repo (already committed there) — noted so the reviewer knows this PR is the hermes-agent half.