fix(cron): tri-state fire-claim heartbeat — lock timeout is 'unverifiable', not ownership loss

[sycamoregroupltd]

Problem

heartbeat_fire_claim() in cron/jobs.py:3376 returns False in two distinct situations:

1. ownership is positively observed to have changed (claim absent / owned by another), and
2. the per-job fire fence could not be acquired within _JOBS_LOCK_TIMEOUT_SECONDS (30s) — a fail-closed "could not verify".

cron/scheduler.py:6985 collapses both into fire claim ownership lost; interrupting stale run and kills a healthy run, which is then recorded as a cron FAILURE.

Under cgroup memory/CPU starvation (measured: PSI full avg10 26–37%, memory.current == memory.high, swap exhausted), a 30s lock wait is routine — so a single heartbeat tick can false-positive "ownership lost" and kill a run that was never stolen.

Fix

Change the contract to tri-state Optional[bool]:

• True — claim still owned by expected_owner and refreshed
• False — ownership positively observed changed (claim absent/other owner)
• None — could not verify (fence lock unavailable) — NOT evidence of loss

Callers updated:

• fire-claim heartbeat loop (scheduler): False interrupts immediately; None feeds the existing _FIRE_CLAIM_HEARTBEAT_GRACE_SECONDS grace path (same as exceptions), so a run is interrupted only when ownership is positively observed changed or verification has failed continuously past the grace window.
• initial validation: None fails closed as Fire claim ownership could not be validated... (not "lost").
• shutdown probes (_fire_claim_ownership_lost, shutdown mark_job_run paths): only True counts as owned; None is not treated as loss.

Verification

• 3 new regression tests in tests/cron/test_script_claim_heartbeat.py:
  • test_unverifiable_fire_claim_not_killed_immediately — the exact incident: None heartbeats must NOT interrupt a live run.
  • test_unverifiable_fire_claim_cancels_after_bounded_grace — sustained None past grace still cancels.
  • test_initially_unverifiable_fire_claim_finishes_without_running — initial None fails closed without claiming loss.
• Full tests/cron/ suite: 1035 passed, 1 skipped. The single failure (test_run_job_cron_execute_code_deny_does_not_pollute_later_gateway_execute_code) is pre-existing on main (verified by stash) and unrelated (execute-code approval session isolation).

Related

• Fleet incident: cron 'fire claim ownership lost' is a fail-closed lock timeout, not claim theft (2026-08-29) — heartbeat_fire_claim() returns False on lock timeout and the scheduler kills a healthy run.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[alt-glitch]

This was generated by AI during triage.

Related to #95432: both repair false fire-claim ownership loss under fence contention for #95307. This version routes an unverifiable heartbeat through the bounded grace path; #95432 skips that beat immediately.

[andrexibiza]

Reviewed exact head 474eb4ea1653e1defda9490d834b8b429ad5b23e against live main@ac6c8028e00d01ee2f299ba7fd03329c7f10382d; this is currently a clean 1-commit/3-file branch directly on main. I read the actual patch plus the surrounding fire-fence, cancellation, delivery/finalization paths; the exact-head workflows; open #95307; direct predecessor #95432; and the complementary generation-fencing work tracked by #88688.

There is good work here. Splitting heartbeat_fire_claim() into owned / positively-lost / unverifiable is the right semantic direction. The initial None path now refuses to start without falsely claiming takeover, the shutdown probes stop using truthiness as ownership proof, and the hosted Python/e2e/macOS/Windows/lint lanes are green. The remaining blockers are at the exact boundary this PR is trying to repair.

P1 — sustained None is still converted into the exact positive ownership-loss signal.

In cron/scheduler.py::_heartbeat_loop (the hunk beginning around @@ -6978), None correctly enters the grace path, but after _FIRE_CLAIM_HEARTBEAT_GRACE_SECONDS that path calls lost_ownership.set(). The same event is then passed into run_job() as the fire-claim cancellation event. run_job::_abort_if_fire_claim_lost() consumes any set event by calling agent.interrupt("Cron fire claim ownership was lost") and raising RuntimeError("... lost its durable fire claim ownership").

So the new state machine is only tri-state at the probe. At the execution sink it collapses back to two states:

None for long enough -> lost_ownership event -> "ownership was lost".

That is not just wording. A healthy execution whose fence remains unavailable/contented through the grace window is still terminated through the same authority-loss channel as an observed successor owner. The incident class is therefore delayed rather than closed for long-running contention. The new test_unverifiable_fire_claim_cancels_after_bounded_grace currently codifies that collapse by asserting only that the loss event becomes set; it never asserts the exact terminal reason or exercises the real fire fence.

There is an important other side here before simply choosing “skip None forever.” _fire_job_lock() deliberately fails closed when the cross-process fire-fence backend is unavailable, and the same acquired=False surface also represents lock acquisition failure/contention. Those are not equivalent facts either. A busy fence held by the legitimate owner can itself be evidence that a replacement owner is excluded; a missing/broken cross-process locking backend is genuinely unverifiable. Optional[bool] still cannot carry that distinction.

Required fix: keep uncertainty distinct from observed loss end-to-end. If policy requires aborting after bounded unverifiability, use a separate typed cancellation/settlement reason (unverifiable / indeterminate, not lost) so no downstream path can claim takeover without a False observation. Better still, make the probe result explicit enough to distinguish at least OWNED, LOST, FENCE_BUSY, and UNAVAILABLE/UNKNOWN, because those states have different safe continuations. Add deterministic witnesses using the real fire-fence path: (1) legitimate fence contention that lasts across multiple heartbeat beats and past the configured grace, (2) unavailable locking backend, and (3) an actual owner replacement. Assert both effect suppression/continuation and the exact terminal reason. A real takeover must remain the only path that says ownership was lost.

Merge/interlock — this is a second implementation of the same open defect class already carried by #95432.

#95432 (BrunoBza) predates this PR, explicitly fixes reporter alex-frolov's #95307, changes the same cron/jobs.py + cron/scheduler.py surfaces, and introduces the same tri-state contract. Its semantic choice differs: a busy-fence None does not consume the exception grace budget, and its dedicated real-store regression suite exercises a long fenced delivery, post-delivery bookkeeping, startup contention, and a genuine takeover control. This PR is much fresher and is cleanly based on current main, but it should not silently become a parallel owner of the same repair.

Please converge these into one canonical landing object: preserve BrunoBza's implementation/test credit and alex-frolov's reporter/root-cause lineage, absorb the strongest real-fence witnesses, and make the contention-vs-backend-unavailable distinction explicit rather than choosing between two incomplete None policies. #88688 is complementary long-term work: exact ownership generation/CAS is the broader architecture that should eventually remove these ambiguous probes; it is not a duplicate of this incident repair.

Acceptance blocker — exact head is not green.

For this exact SHA:

• CI 33226106068: failure — https://github.com/NousResearch/hermes-agent/actions/runs/33226106068
• Check contributors / check-attribution job 99030104601: failure at “Check for unmapped contributor emails” — https://github.com/NousResearch/hermes-agent/actions/runs/33226106068/job/99030104601
• required-check aggregate 99030582376: failure as a consequence — https://github.com/NousResearch/hermes-agent/actions/runs/33226106068/job/99030582376
• Docker 33226105599: success — https://github.com/NousResearch/hermes-agent/actions/runs/33226105599
• Nix 33226105712: success — https://github.com/NousResearch/hermes-agent/actions/runs/33226105712

The sole commit is authored as Frank Spencer <frank@frankspencer.dev>; the current tree has no mapping for that email, matching the attribution failure. Preserve Frank's authorship and add the repository contributor mapping rather than rewriting history, then require the corrected exact head to go fully green.

The core insight here is strong: “could not verify” is not “lost.” Carry that distinction all the way through cancellation and settlement, reconcile the earlier carrier instead of competing with it, and get the exact commit green. That closes the class instead of moving the false-positive boundary three minutes downstream. 🚀

[jrodriguezruibal]

Confirming this diagnosis from a production gateway, and adding the piece I think is missing: the fail-closed "could not verify" branch is triggered routinely, not only under a same-job race.

Environment: single hermes-gateway.service (systemd user unit), Linux, ~14 cron jobs, checkout at a0749d583a, managed topology (agent jobs handed to cron.scheduler --external-worker-file transient scopes). Two independent events on the same agent-mode job (1dd048646ab6, every 30m, run time 3-11 min):

# event 1
13:17:15  Cron job '60db2e8eec47' handed to restart-safe worker pid=116668
13:17:44  ERROR cron.jobs: Timed out waiting for local fire fence <HERMES_HOME>/cron::1dd048646ab6; failing closed
13:17:44  WARNING __main__: Job '1dd048646ab6': fire claim ownership lost; interrupting stale run
13:18:46  Job '1dd048646ab6': delivered to Bot Chat of profile 'inbox-watch'
13:18:46  status=error, "Interrupted by shutdown before terminal completion."

# event 2
13:37:15  Cron job '945e877ca286' handed to restart-safe worker pid=122177
13:37:15  Cron job '1dd048646ab6' handed to restart-safe worker pid=122178
13:39:27  [cron_1dd048646ab6_20260910_133757] Turn ended: reason=text_response api_calls=13/90
13:40:45  ERROR cron.jobs: Timed out waiting for local fire fence <HERMES_HOME>/cron::1dd048646ab6; failing closed
13:40:45  WARNING __main__: Job '1dd048646ab6': fire claim ownership lost; interrupting stale run
13:41:34  Job '1dd048646ab6': delivered to Bot Chat of profile 'inbox-watch'

Both times the fence timeout and the ownership-lost line land in the same second, and the timeout is exactly _JOBS_LOCK_TIMEOUT_SECONDS (30s) after a different job's worker was spawned. The victim job was healthy and mid-run (heartbeating every 60s: 13:39:45 ok, 13:40:45 lost), it finished its turn, it delivered — and it was still recorded as failed.

Triggers observed: (a) two jobs firing in the same tick, each in its own worker (13:37:15 above); (b) a worker spawning for another job while this one is mid-run (13:17:15). In every case the victim was a job whose next_run_at still pointed at the fire already in flight.

Mechanism as I read it: a worker process spawned for job A runs the tick for all jobs. Its in-flight guard (_running_job_ids + _running_since) is per-process, so a job in flight in another process still looks due — next_run_at is not advanced at dispatch — and it gets re-fired while inside its catch-up window. That re-fire blocks on the per-job fire fence for 30s, fails closed, and the collapsed fail-closed path makes the other process's healthy run kill itself. So this is reachable on any gateway where jobs share a tick or where a short-period watchdog worker starts while a longer job is running — not a rare race.

I can't tell from the logs whether the claim record is actually overwritten by the failed acquire or whether the running worker's sampled flag flips on its own re-read of the fence; if it's the latter, the failed acquire is destructive on its own and worth asserting in a test.

Side note for other reporters: the work is not lost — the digest was delivered both times; only the status/dedup bookkeeping is wrong (delivered + failed, and the recovery text tells the operator the result was discarded when it wasn't). Same family as #105861 / #105882.

Mitigation on our side (no code change): moving data collection into a pre-run script cut the job from ~11 min to ~2m53s, which shrinks the window in which a re-fire can land mid-run. That's damage control, not a fix.

[kshitijk4poor]

The fix for this bug landed on main via #109310 (merge 9a60a7f): heartbeat_fire_claim now refreshes under _jobs_lock only and no longer waits on the per-thread fire fence its own run holds, so a delivery/agent turn longer than the 30s fence timeout is no longer misread as ownership loss. _refresh_claim still CASes on claim["by"], so a real takeover keeps returning False (pinned by test).

Thanks — the tri-state/unverifiable classification was a correct reading of the failure. The landed fix removes the fence wait from the heartbeat entirely, so False now means only "another owner holds the claim"; the grace-path routing this PR adds becomes unnecessary. Re @jrodriguezruibal's production confirmation: that routine "could not verify" path is exactly what is gone. Closing with credit.