fix(cron): mark interrupted restart runs failed instead of lost#96174
fix(cron): mark interrupted restart runs failed instead of lost#96174markoub wants to merge 7 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 12, 2026, 2:17 AM ET / 06:17 UTC. Summary PR surface: Source +527, Tests +1003, Docs 0, Other 0. Total +1530 across 20 files. Reproducibility: yes. at source level. Current main still exposes the precise reservation/start timestamp divergence, while the PR's production-module proof demonstrates that aligning the durable marker changes the interrupted task outcome from lost to failed. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Port the atomic durable start claim and stale-reservation fencing onto current main’s canonical cron architecture, preserve concurrent delivery-target updates, keep the documented SDK helper as the approved narrow cross-package seam, and rerun restart plus concurrent-writeback proof on the resolved exact head. Do we have a high-confidence way to reproduce the issue? Yes at source level. Current main still exposes the precise reservation/start timestamp divergence, while the PR's production-module proof demonstrates that aligning the durable marker changes the interrupted task outcome from lost to failed. Is this the best way to solve the issue? Yes. Persisting the claimed actual start before task-row creation fixes the recovery key at the cron owner boundary; the remaining work is conflict-safe integration and exact-head validation, not a different recovery heuristic. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2a02ab6bbe6a. Label changesLabel justifications:
Evidence reviewedPR surface: Source +527, Tests +1003, Docs 0, Other 0. Total +1530 across 20 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
proof: sufficient
a334c79 to
27b9adc
Compare
openclaw-barnacle
Bot
added
docs
clawsweeper
Bot
added
the
merge-risk: 🚨 compatibility
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
2 participants
Closes #96161
What Problem This Solves
Fixes an issue where users or operators inspecting cron task runs after a gateway restart could see an interrupted cron task marked as
lostinstead offailedwhen the scheduler reservation timestamp differed from the actual task run start timestamp.Why This Change Was Made
Cron due-job and startup catch-up execution now persist the claimed run start timestamp before creating the task-ledger row, so restart task recovery uses the same timestamp as the cron task run id. If stop, restart recovery, or a newer cron generation wins during that short persistence window, the scheduler releases the claimed marker and does not emit a start event, create a task row, or start the job.
User Impact
Interrupted cron task rows recover to the terminal cron result instead of being mislabeled
lostafter restart. Operators also avoid new cron runs being admitted while restart recovery is taking over.Evidence
AI assistance disclosure: This PR was prepared with Codex assistance; I reviewed the changes and validation.
Real behavior proof: In a local OpenClaw checkout, a runtime-level Node/TS proof command used a temporary redacted
OPENCLAW_STATE_DIR, created a real SQLite-backed cron store, let the production cron timer path persist the due-job start marker and create the cron task row, simulated a gateway restart by clearing process-local cron/task mirrors while keeping the durable state, then ran production startup recovery and task-registry maintenance. This uses production cron/task registry modules; the isolated runner is held open only to simulate the interrupted run.Redacted terminal output from the proof command:
{ "proof": "OpenClaw #96174 runtime restart recovery", "tempState": "<redacted temp OPENCLAW_STATE_DIR>", "beforeRestart": { "cronRunningAtMs": 1782269100137, "taskRunId": "cron:proof-cron-restart:1782269100137", "taskStatus": "running", "taskStartedAt": 1782269100137, "markerMatchesTaskStartedAt": true, "runIdMatchesMarker": true }, "afterStartupRecovery": { "cronRunningAtMs": null, "cronLastRunAtMs": 1782269100137, "cronLastRunStatus": "error", "cronLastError": "cron: job interrupted by gateway restart", "taskStatusBeforeMaintenance": "running" }, "afterTaskMaintenance": { "summary": { "reconciled": 0, "recovered": 1, "cleanupStamped": 0, "pruned": 0 }, "taskStatus": "failed", "taskError": "cron: job interrupted by gateway restart", "taskEndedAt": 1782269700137, "failedNotLost": true } }Focused validation commands:
node scripts/run-vitest.mjs src/cron/service/timer.regression.test.ts(73 tests passed)node scripts/run-vitest.mjs src/tasks/task-registry.maintenance.issue-60299.test.ts(19 tests passed)pnpm exec oxfmt --check src/cron/service/timer.ts src/cron/service/timer.regression.test.tsnode scripts/run-oxlint.mjs src/cron/service/timer.ts src/cron/service/timer.regression.test.tsgit diff --check origin/main...HEADpnpm tsgo:core:testcodex review --base origin/main(passed after addressing review findings)