fix(cron): a completed run keeps its result when a fire-claim heartbeat sample misses (#113357, #105861, salvage #113364)

[teknium1]

A cron job that runs past one fire-claim heartbeat interval (>60 s) and finishes with a complete response is no longer recorded as last_status: error / Interrupted by shutdown before terminal completion. when a single heartbeat sample reads the claim as lost.

• cron/scheduler.py::_FireOwnership.lost — an explicit transport cancel stays terminal; a latched heartbeat miss is re-checked against the store instead of being trusted as the verdict. A claim that still validates keeps the run's real outcome (ok, or the real error); the owner-fenced mark_job_run / fire_claim_fence remain the authority, so a genuinely re-owned claim still yields (nothing delivered, no terminal write over the new owner, ledger row Fire claim ownership lost; stale result was discarded.). An unreachable store after a latched miss stays fail-closed.
• cron/scheduler.py::_run_with_fire_claim_heartbeat._heartbeat_loop — a miss is re-sampled once (_FIRE_CLAIM_MISS_CONFIRM_SECONDS, 1 s) before it latches. The latch cancels the live agent run and ends the lease refresh, so one sample must not do that; a genuinely re-owned claim misses twice and latches ~1 s later.
• tests/cron/test_fire_claim_lost_after_delivery.py — two invariant tests on the real store with the real heartbeat thread: one-sample miss never cancels the run and two-miss latch still records the completed run; a genuinely re-owned claim yields. Each seam proven to bite independently (revert A → latched case delivers nothing; revert B → one-sample case cancels the run; base fails both; the steal control passes everywhere).
• website/docs/developer-guide/cron-internals.md — fire-claim lease semantics during a run.

Root cause: the fire-claim heartbeat thread latched lost_ownership on a single sample that read claim["by"] != expected_owner, and _FireOwnership.lost() returned True on that latch without re-asking the store — so _record_fire_ownership_lost (which re-validates the owner and writes the "interrupted" message only when the owner still holds the claim) terminalized a completed run as an error and never saved its output.

On the issue's hypothesis (b) — worker process identity under systemd-run --scope: falsified by the code. The owner token is stored in fire_claim.by at claim time (cron/jobs.py::claim_job_for_fire) and every check compares against that stored value (cron/jobs.py::_refresh_claim); _machine_id() is never recomputed for the comparison, and the heartbeat thread runs inside contextvars.copy_context() so it reads the same profile store. The reporter's own error string is the proof the loss was transient: it is only written when heartbeat_fire_claim(...) returns True at record time.

Live repro

Real store under a temp HERMES_HOME (claim_job_for_fire → run_one_job → mark_job_run / finish_execution / fire_claim_fence all real); only run_job is stubbed with a complete 4-tuple and heartbeat_fire_claim returns False for exactly N samples on the heartbeat thread (/tmp/batch9/cron-scheduler/probe_claim_loss_complete.py). Heartbeat interval 0.2 s, run 1 s.

arm	origin/main @ c62bd9f	this PR
one heartbeat sample misses, run complete	last_status=error, last_error="Interrupted by shutdown before terminal completion.", ledger failed, no output file	ledger completed, output file saved, record identical to the no-miss control
two consecutive misses (latched), owner re-validates at completion	same false error	ledger completed, output saved
control: no miss	ledger completed, output saved	identical
control: claim genuinely re-owned mid-run (by rewritten)	no terminal write, ledger Fire claim ownership lost; stale result was discarded.	identical (still yields)

Tests: scripts/run_tests.sh over tests/cron/test_fire_claim_lost_after_delivery.py test_claim_job_for_fire.py test_cron_relay_run_forward.py test_parallel_pool.py test_script_claim_heartbeat.py test_shutdown_interrupt.py → 6 files, 99 passed, 0 failed.

Issues

Fixes #113357
Related to #105861 (thread not re-read in this lane; maintainer to confirm closure) — main already carried the post-delivery half (fall-through after a completed delivery); this closes the remaining atoms: the pre-delivery check right after run_job returns, delivery-less jobs (deliver: none / no target, where delivery_attempted is False), and the one-sample latch itself.
Salvages #113364 (@KoNit-K) — slimmer redo of the same direction (revalidate the owner before trusting the latch), credited as Co-authored-by.

Dropped hunks

• fix(cron): preserve completed jobs after claim loss #113364 _CombinedCancelEvent.fire_claim_is_set / other_cancel_is_set and the isinstance(self.claim_lost, _CombinedCancelEvent) branch in lost() — dead in production (claim_lost is always the raw threading.Event; only the PR's third test passed a combined event). The transport-vs-heartbeat split it aimed for already exists on main as _FireOwnership.transport_cancelled(), which this PR uses. Its three monkeypatched tests replaced by two real-store invariant tests (guarded cases re-probed: completed run kept; incomplete/stolen still fenced out; transport cancel still interrupts — covered by the existing test_transport_cancel_during_delivery_stays_fail_closed).

Infographic

(image_generate returned HTTP 429 from the subscription gateway twice; style picked: hermes-cyanotype / bento-grid)

[github-actions]

૮ >ﻌ< ა ci review

_{ran on 62d65f8 — fix(cron): a completed run keeps its result when a fire-clai}

all good!

[ehz0ah]

Request changes conclusion

GitHub does not permit this account to submit a formal change-request review, so this comment records the same blocking verdict.

The issue is valid, and the durable owner token remains the correct authority for delivery and terminal writes. The new two-sample rule reduces false latches, but a confirmed sample miss can still cancel work before the patch revalidates ownership.

I reproduced this on the exact head and after a clean merge into current main 01382698fc32ec7740b6a204d9b7a6abeac74d33. A real no_agent script was still owned in jobs.json. I forced only the first two heartbeat-thread samples to return false, then restored the real store check. The script was killed and the job ended with last_status=error and Script cancelled because cron fire ownership was lost.

The submitted focused suite passes 99 tests. Ruff and git diff --check pass. Hosted required checks are green. The inline finding is blocking because the production runner does not behave like the test stub after cancellation.

Verdict: REQUEST_CHANGES at 62d65f8342c248b93d3cd6d5bd881ad95286c589.

[ehz0ah]

After the second sampled False, this publishes lost_ownership to the real runner before durable ownership is revalidated. _run_job_script sees the combined cancel event, kills the subprocess, and returns Script cancelled because cron fire ownership was lost. _run_agent_with_watchdog likewise interrupts the agent. The later _FireOwnership.lost() recheck cannot restore work that was already stopped.

I reproduced this with a real no_agent script and an unchanged stored claim. I forced only the first two heartbeat-thread samples to return False, then let all later calls use the real heartbeat_fire_claim. The script was killed and the job ended last_status=error even though it still owned the claim. The same result survives a clean merge into current main. The new latched test hides this because its fake run_job observes the cancel event but returns success anyway. Please do not expose a cancellation signal to the runner until the durable ownership check fails, and cover the path with a real script or agent cancellation test.