Skip to content

fix(cron): project recovered worker failures - #120347

Closed
KoNit-K wants to merge 1 commit into
NousResearch:mainfrom
KoNit-K:fix/120328-cron-adopted-worker-recovery
Closed

KoNit-K wants to merge 1 commit into
NousResearch:mainfrom
KoNit-K:fix/120328-cron-adopted-worker-recovery

Conversation

@KoNit-K

@KoNit-K KoNit-K commented Sep 23, 2026

Copy link
Copy Markdown

What does this PR do?

When an adopted restart-safe cron worker exits before writing a terminal result, its ledger row is still retained as unknown because external side effects may have happened. This change additionally projects that recovered attempt once into the job record as a failed run, so last_run_at, last_status, last_error, claims, incidents, and failure delivery no longer remain stale. The projection is keyed by execution ID under the jobs-store lock, preventing overlapping recovery scans from advancing a schedule or re-alerting twice.

Related Issue

Fixes #120328

Type of Change

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

Changes Made

  • cron/executions.py — returns the exact compare-and-swap recovered rows to scheduler callers and records the one recovery delivery outcome without changing the immutable unknown state.
  • cron/jobs.py — projects a recovered execution into jobs.json once, updates failed-run fields, clears claims, and advances the next schedule.
  • cron/scheduler.py and cron/scheduler_tick.py — apply the existing incident and failure-delivery path both immediately after an adopted worker exit and during periodic dead-owner recovery.
  • tests/cron/test_dead_owner_claim_reclaim.py — covers adopted-worker recovery, jobs/incident/delivery projection, and overlapping projection idempotency.

How to Test

  • HERMES_PYTHON="$(command -v python3)" scripts/run_tests.sh tests/cron/test_dead_owner_claim_reclaim.py tests/cron/test_execution_ledger.py tests/cron/test_restart_safe_worker.py -q — 55 passed, 4 Linux-only skipped on macOS.
  • python3 -m ruff check cron/executions.py cron/jobs.py cron/scheduler.py cron/scheduler_tick.py tests/cron/test_dead_owner_claim_reclaim.py tests/cron/test_restart_safe_worker.py — passed.
  • CONTROL: completed terminal executions remain immutable and live owners remain unrecovered in the focused ledger tests.

Evidence

  • BEFORE RED: the new adopted-worker recovery probe fails on origin/main because last_status remains None after the execution becomes unknown.
  • AFTER GREEN: the same probe updates the job to last_status == "error", creates one incident, delivers one failure notice, and records delivery_outcome == "delivered".
  • CONTROL: concurrent projections of the same execution ID produce no additional delivery; normal terminal executions are not rewritten.

Review follow-up

Docker and Nix CI lanes are derived from this Python production change but were not run locally. Linux-only tests are left to CI; they were skipped on this macOS host.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run relevant tests locally (see How to Test)
  • I've added tests for my changes
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • Documentation update: N/A
  • cli-config.yaml.example: N/A
  • CONTRIBUTING.md or AGENTS.md: N/A
  • Cross-platform impact considered
  • Tool descriptions/schemas: N/A

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Sep 23, 2026

KoNit-K commented Oct 7, 2026

Copy link
Copy Markdown
Author

Closing this PR rather than continuing with a rewrite.

Current main has since changed the direct external-worker exit/waiter path, so the original patch overlaps that handling. The separate periodic dead-owner recovery path may still need job-level outcome projection and delivery, but it requires a focused design around execution identity, ownership fencing, and idempotent incident/notification handling. I am not taking on that follow-up in this branch.

This closure is a maintenance decision, not a claim that the remaining recovery gap is fully fixed upstream. Thanks to everyone who looked at the issue.

@KoNit-K KoNit-K closed this Oct 7, 2026
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 comp/gateway Gateway runner, session dispatch, delivery 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.

[Bug]: cron worker killed after adoption (e.g. OOM) leaves the job record untouched — no last_status, no incident, no failure delivery

2 participants