Skip to content

fix(cron): distinguish fire-claim ownership loss from shutdown in ledger - #108912

Open
duanyangformxa wants to merge 3 commits into
NousResearch:mainfrom
duanyangformxa:pr/fire-claim-ledger
Open

duanyangformxa wants to merge 3 commits into
NousResearch:mainfrom
duanyangformxa:pr/fire-claim-ledger

Conversation

@duanyangformxa

Copy link
Copy Markdown

What does this PR do?

Cron runs that lose their fire-claim ownership (per-job fire-fence lock timeout under long-running delivery) were recorded in executions.db as failed with the error string "Interrupted by shutdown before terminal completion." — indistinguishable from real shutdown kills, even when the delivery had already succeeded.

A 20-case investigation across 4 profiles (2026-09-12, executions.db × agent.log cross-verification) showed:

  • 12/20 "interruptions" were delivered-then-misrecorded (strongest case: fence timeout 21:32:49 → delivery succeeded 21:39:22, ledger said failed/shutdown);
  • 7/20 were fence-timeout misrecords with no shutdown at all;
  • only 1/20 aligned with a real gateway restart.

Root cause: _under_fire_fence lock timeout (30s, fail-closed) → heartbeat heartbeat_fire_claim=False → 180s grace exhausted → lost_ownership → _record_fire_ownership_lost unconditionally writes the shutdown string. This PR makes the ledger reflect what actually happened; it complements #108359 (which prevents the false ownership loss at the heartbeat layer) — if the heartbeat no longer times out, this ledger layer still correctly classifies genuine losses (real cancellation, dashboard drain, actual shutdown).

Related Issue

Addresses #108341 (ledger-accuracy layer; #108359 covers the heartbeat-prevention layer).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • cron/scheduler.py:
    • Delivered-then-lost → mark_job_run(success, "Fire claim ownership lost after successful delivery; recorded as completed (delivery confirmed).") — never overwrite a healthy delivered run with the shutdown string;
    • Pre-delivery loss → distinct reason "Fire claim ownership lost before delivery; run result discarded (not a shutdown)." (owner-fenced write kept; discard fallback on fence rejection) — a pre-delivery loss must never read as a shutdown;
    • Heartbeat interval configurable via cron.heartbeat_seconds through _cron_heartbeat_seconds() — default 60s unchanged, fail-closed on invalid config.

How to Test

python -m pytest tests/cron/test_fire_claim_ownership_lost_ledger_20260912.py tests/cron/test_script_claim_heartbeat.py -q
# 18 passed, 1 skipped

python -m pytest tests/cron/test_scheduler.py tests/cron/test_dead_owner_claim_reclaim.py tests/cron/test_run_one_job.py -q
# 122 passed

New regression file tests/cron/test_fire_claim_ownership_lost_ledger_20260912.py covers 5 cases: delivered-then-lost → completed-with-warning; pre-delivery loss → distinct reason; genuine heartbeat loss (non-fence) → shutdown-string semantics preserved; delivery-error bookkeeping; cron.heartbeat_seconds fail-closed default.

Compatibility

The only known executions.db consumer parsing these rows aggregates by status='completed' and never matches error text — new strings are zero-blocking; post-fix, delivered-run counting becomes more accurate.

Evidence appendix (abridged)

case profile/job fence-timeout later delivery old ledger new ledger
09-11 21:30 qc-bot e0beb63d 21:32:49 21:39:22 ✓ failed "shutdown" completed+warning
09-12 07:35 default D-lane 07:36:54 07:37:42 ✓ failed "shutdown" completed+warning
… 10 more delivered-then-lost cases, 7 pre-delivery fence-timeout cases in the investigation report

…ger (K3 case-1)

Delivered-then-lost runs now record completed-with-warning; pre-delivery loss gets
a distinct reason string; heartbeat interval configurable (fail-closed default).
…antics

5 cases: delivered-then-lost / pre-delivery distinct reason / shutdown-string
semantics preserved / delivery-error bookkeeping / heartbeat config fail-closed.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Sep 12, 2026
@kshitijk4poor

Copy link
Copy Markdown

Heads-up: the source of the false Interrupted by shutdown before terminal completion. rows landed on main via #109310 (merge 9a60a7f) — the heartbeat no longer times out on the fire fence its own run holds, so a long delivery is no longer recorded as an ownership loss. The 12/20 misattributed rows in your investigation should stop occurring; the ledger-message distinction here is still a reasonable UX improvement for genuine losses, but please re-measure against current main and trim the PR to what remains.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants