fix(cron): an unacquirable fire fence is not lost fire-claim ownership

[awatney]

What does this PR do?

Closes the remaining places where a fire fence that was never acquired is still recorded as a
verdict about the fire claim. A cron run could be reported failed after both its work and its
delivery succeeded — the execution row carrying an ownership-lost / interrupted error while the
deliveries row says delivered and the run's output markdown exists.

The heartbeat half of this class is already fixed on main (heartbeat_fire_claim deliberately
does not take the fence, pinned by
tests/cron/test_claim_job_for_fire.py::test_heartbeat_does_not_wait_on_the_fence_its_own_run_holds;
see closed #108341 / #108862 / #100401 / #106737). What is left is the fence boundary itself:

_fire_job_lock fails closed after _JOBS_LOCK_TIMEOUT_SECONDS = 30.0, and the fence is held
across network side effects by design — the run's own delivery. That call blocks ~50–57 s
client-side on the Matrix lane while the homeserver answers in 2–6 ms; 49 of 71 recent delivery
rows across the profiles measured here exceed the 30 s the fence allows. Two refusal sites then
turned "the fence did not get acquired" into an ownership verdict, although a lock timeout samples
nothing from the store:

1. fire_claim_fence (save output / deliver) → _FireClaimLostDuringSideEffect →
   _record_fire_ownership_lost, which re-confirms the claim is still ours and then writes
   _OWNERSHIP_LOST_INTERRUPTED (Interrupted by shutdown before terminal completion.) to the job
   record and the execution row.
2. _finish_completed_run → mark_job_run returning False → Fire claim ownership lost before terminal completion., after the delivery had already left the process.

False from the fence is ambiguous (genuine revocation or a contended/unavailable lock) and only
the first is evidence about the claim; #116164 / #105861 covered the delivered-run overwrite, this
covers why the refusal happened and what gets written for a refusal that carries no evidence.

Related Issue

Fixes no open issue: the tracking issues for this class are closed (#108341, #108862, #100401,
#106737, #105861, #116164) and the heartbeat-side fix they landed is in main. This is the
residual half, reproduced on the current head.

Related open PRs, complementary rather than duplicated: #110152, #116301, #108912 and #116463
change what the ledger/text says after a refused or presumed-lost claim. This PR makes the
refusal reason explicit at the fence boundary (a fence that was never acquired is never treated as
ownership evidence at all, at both the side-effect and terminal writes) and adds the same rule to
the heartbeat's store sample. Happy to fold any of it into those if maintainers prefer one PR.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• cron/jobs.py — _under_fire_fence, fire_claim_fence and mark_job_run accept a
  fence_busy out-list that is appended to when the FENCE (not the store) refused, so callers can
  tell a contended/unavailable fence from a stale owner; new jobs_lock_degraded() reports whether
  the calling thread's last _jobs_lock section ran without the cross-process flock (timeout or no
  backend).
• cron/scheduler.py — _FireFenceBusy (new) separates "the fence was not acquired" from
  _FireClaimLostDuringSideEffect; _run_one_job_body / _finish_completed_run record
  _FIRE_FENCE_BUSY for that case instead of an ownership-lost string, still failing closed (no
  delivery, no terminal status write without the fence, nothing recorded as a success, and the
  job record is deliberately left untouched — a refusal must not be laundered into last_status);
  the fire-claim heartbeat re-samples a miss whose compare-and-refresh ran without the
  cross-process jobs lock and keeps renewing until _FIRE_CLAIM_HEARTBEAT_GRACE_SECONDS instead
  of latching a loss that cancels a live, still-owned run.
• cron/AGENTS.md — the hardening-invariant entry for the above.
• tests/cron/test_fire_fence_busy_is_not_ownership_loss.py (new) and
  tests/cron/test_run_one_job.py (exact-kwargs assertion picks up the new argument).

How to Test

1. Hold a job's fire fence from a sibling owner (its own long side effect in flight) while the
   run's claim is unchanged, with _JOBS_LOCK_TIMEOUT_SECONDS lowered for speed. Before:
   failed | Interrupted by shutdown before terminal completion. with the claim still ours; after:
   failed | Fire fence busy before terminal completion; the run's result was not recorded.
2. Same, with the fence taken at the terminal write after a completed delivery. Before:
   failed | Fire claim ownership lost before terminal completion.; after: the fence-busy cause,
   never an ownership verdict.
3. pytest tests/cron/test_fire_fence_busy_is_not_ownership_loss.py tests/cron/test_run_one_job.py tests/cron/test_claim_job_for_fire.py tests/cron/test_fire_claim_lost_after_delivery.py -q

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate (see Related Issue — the open PRs cover the ledger text, this covers the fence refusal itself)
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/cron -q: 1396 passed, 3 skipped, 13 failed — the same 13 failures occur on the unpatched tree on this host (environment/interpreter-guard fixtures: test_estop ×4, test_mirror_origin_fallback ×5, test_cron_non_push_origin ×2, test_cron_script[pythonw], test_atomic_paused_creation); the patch introduces none
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Ubuntu 24.04 (Linux 6.8), Python 3.11

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — cron/AGENTS.md invariant + docstrings
• I've updated cli-config.yaml.example if I added/changed config keys — N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — cron/AGENTS.md
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — no new platform-specific code; jobs_lock_degraded() reads the existing lock state and reports degraded locking on any platform without a flock backend
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Reproduction on the previous head (bc7f58f3b0), fence held by a sibling owner, claim unchanged:

--- A: side-effect fence busy while the claim is unchanged
   delivered content     : []
   job last_status       : None
   execution status/error: failed | Interrupted by shutdown before terminal completion.
   claim still ours      : True
--- B: terminal-write fence busy after a completed delivery
   delivered content     : ['the report']
   job last_status       : None
   execution status/error: failed | Fire claim ownership lost before terminal completion.
   claim still ours      : True

With the patch, both rows read
failed | Fire fence busy before terminal completion; the run's result was not recorded. and the
job record is left untouched.

Production log pairing this recording (profile bob, the run whose ledger row is the false
failure):

ERROR cron.jobs: Timed out waiting for local fire fence /home/adam/.hermes/profiles/bob/cron::7f8da987592f; failing closed
WARNING __main__: Job '7f8da987592f': fire claim ownership lost; interrupting stale run

[Enough1122]

AI code review — automated review for reference; please use your judgment.

Reviewed at bd0fa18b. The fence-busy bookkeeping is right — the flag is threaded through all three sites, it fails closed, nothing is written without the fence. One finding: the second repair in this PR is a dead branch.

jobs_lock_degraded() is always False at its only call site, so the re-sample never happens.

It is thread-local state that _jobs_lock sets on entry and clears on exit:

• cron/jobs.py:315/:323 set degraded = True inside the section (flock timeout / no backend)
• cron/jobs.py:332 resets it in the outer finally — the moment the section closes
• the only reader is cron/scheduler.py:2676, in _heartbeat_loop, on its own thread (:2708)

The sample that reader is meant to qualify came from heartbeat_fire_claim (jobs.py:2832) → _with_job (with _jobs_lock(): — that section closes as it returns). By :2676 the flag is already reset.

AST-extracting the real _jobs_lock / jobs_lock_degraded / _acquire_flock from the head blob (no import), with _acquire_flock forced to return False — the actual flock-timeout branch:

#	scenario	degraded()	:2675
1	positive control — read inside the degraded section	True	yes
2	heartbeat miss, read after its section closed (real :2676 ordering)	False	no
3	real consumer — its own cron-fire-claim-heartbeat thread	False	no

So a fire-claim miss sampled while a sibling holds the lock past the timeout still latches lost_ownership and interrupts a live, still-owned run — exactly what the invariant added at cron/AGENTS.md:32-37 says this PR prevents. The docstring "the CALLING thread's last _jobs_lock section" describes a lifetime the flag doesn't have.

The new tests can't catch it: monkeypatch.setattr(sched, "jobs_lock_degraded", lambda: degraded) replaces the seam, so they prove the grace arithmetic while the real flag is never evaluated.

To fix: latch the flag for the caller instead of clearing it on exit, or put the sampling and the degraded test inside one _jobs_lock scope. If it is only meant to suppress the misleading logger.warning, say so — the AGENTS.md entry reads as a real guarantee.

Not reporting: leaving last_status/last_run_at untouched for FIRE_CLAIM_TTL_SECONDS (300s) to expire before re-firing — unchanged from base, pinned by your own assert.

[awatney]

Closing this. The automated review is right on the substantive point: the second repair is a dead branch. jobs_lock_degraded() reads thread-local state that _jobs_lock sets on entry and clears in its outer finally, while its only reader (cron/scheduler.py, in _heartbeat_loop) runs on a different thread — so by the time it is sampled the flag is already reset, the degraded case never latches, and a fire-claim miss under a sibling's lock still latches lost-ownership and interrupts a live run — the outcome the PR set out to prevent. The fence-busy bookkeeping in the first repair is sound, but the stated guarantee does not hold, and the tests monkeypatch the seam so they cannot catch it.

Rather than pursue the correction upstream now, I'm closing it and keeping the work local — the defect is not present in my runtime tree (cron/jobs.py there has no jobs_lock_degraded). Thanks for the review; flagging the dead branch rather than the shape of the diff was the useful call.