Skip to content

fix(cron): skip startup catchup for missed jobs already completed with status=ok#102000

Closed
tzy-17 wants to merge 2 commits into
openclaw:mainfrom
tzy-17:fix/101988-cron-catchup-skip-ok
Closed

fix(cron): skip startup catchup for missed jobs already completed with status=ok#102000
tzy-17 wants to merge 2 commits into
openclaw:mainfrom
tzy-17:fix/101988-cron-catchup-skip-ok

Conversation

@tzy-17

@tzy-17 tzy-17 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #101988: When the Gateway restarts, overdue cron jobs with lastRunStatus=ok and lastRunAtMs >= nextRunAtMs were still considered runnable by isRunnableJob, causing duplicate notifications (e.g. a 09:10 daily reminder firing again after an 11:00 restart).

Why This Change Was Made

Added a check in isRunnableJob at the nowMs >= next branch: if the job already completed successfully (lastRunStatus === "ok") at or after its scheduled time (lastRunAtMs >= nextRunAtMs), skip catchup. This matches user intent — a job that already succeeded for its current schedule tick should not be replayed.

The fix is a single guard condition in src/cron/service/timer.ts:1642, plus a focused regression test.

Impact

  • No more duplicate cron notifications after Gateway restart
  • Cron jobs with lastRunStatus=ok within the current schedule window are correctly skipped
  • Error-retry behavior is unchanged (status != ok still triggers catchup)

Evidence

  • pnpm test src/cron/service.restart-catchup.test.ts — 16 tests pass (incl. 1 new)

🤖 Generated with Claude Code

@clawsweeper

clawsweeper Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close: this PR is superseded by the existing cron catch-up cluster and is not a safe landing candidate because its current guard can skip legitimate later missed cron slots before the existing previous-slot guard runs.

Root-cause cluster
Relationship: superseded
Canonical: #101988
Summary: This PR is one of several candidates for the same stale successful cron restart catch-up issue, but it is superseded because its implementation is weaker than the remaining canonical cluster.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Canonical path: Close this branch and consolidate the final fix on the canonical cron catch-up issue, using the proof-positive candidate plus any needed boundary coverage from the sibling boundary-preserving PR.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Close this branch and consolidate the final fix on the canonical cron catch-up issue, using the proof-positive candidate plus any needed boundary coverage from the sibling boundary-preserving PR.

Do we have a high-confidence way to reproduce the issue?

Yes, source-level: startup catch-up evaluates persisted due cron slots before maintenance recompute, and this PR's new branch can skip before the previous-slot guard runs. I did not run a live Gateway restart in this read-only review.

Is this the best way to solve the issue?

No. The branch changes the right decision point, but the early return false is too broad; the better solution is the canonical boundary-aware scheduler fix with real CronService proof.

Security review:

Security review cleared: The diff only changes cron scheduler selection logic and a focused test; I found no concrete security or supply-chain concern.

AGENTS.md: found and applied where relevant.

What I checked:

  • Current PR head over-skips: At the current PR head, the due-nextRunAtMs branch returns false as soon as lastRunStatus is ok and lastRunAtMs >= next, so it bypasses the later previous-slot comparison that protects legitimate missed cron slots. (src/cron/service/timer.ts:1651, d5be0aa1263b)
  • Main still has the original runnable gap: Current main returns true for any finite due nextRunAtMs before reaching the cron previous-slot versus lastRunAtMs guard, which is the linked bug this cluster is repairing. (src/cron/service/timer.ts:1642, 4ae8d735bf54)
  • Startup catch-up runs before maintenance recompute: Cron startup calls runMissedJobs before the later maintenance recompute, so stale persisted due slots can be evaluated by startup catch-up first. (src/cron/service/ops.ts:269, 4ae8d735bf54)
  • Existing invariant belongs in the scheduler path: Maintenance recompute already advances an expired slot only when lastRunAtMs covers nextRunAtMs, which supports a boundary-aware fix rather than this PR's early false return. (src/cron/service/jobs.ts:785, 4ae8d735bf54)
  • Proof-positive sibling candidate: The open sibling candidate keeps the due branch from returning true only for already-completed cron slots and falls through to the existing previous-slot guard; it also carries a proof: sufficient label and real CronService terminal proof in its body. (src/cron/service/timer.ts:1642, 5870f97ec24c)
  • Related open PR search: Live GitHub search found fix(cron): skip completed restart catchup slots #101998 as the open proof-positive candidate for the same linked cron catch-up issue.

Likely related people:

  • Takhoffman: Authored and merged the restart catch-up replay semantics PR that established the relevant missed-run guard behavior. (role: feature-history contributor; confidence: high; commits: 79d00ae39860, d659a8ebb028, 1df8e5b2cfd6; files: src/cron/service/timer.ts, src/cron/service/jobs.ts, src/cron/service.restart-catchup.test.ts)
  • vincentkoc: Current blame for the central runnable function points to a recent cron localization refactor, and recent adjacent cron commits touched execution and catch-up behavior. (role: recent area contributor; confidence: medium; commits: 3dbe98a1f737, 44ae2fd93601, c716e1b8acf4; files: src/cron/service/timer.ts, src/cron/service/jobs.ts, src/cron/service/ops.ts)
  • steipete: Recent cron event-trigger work changed the same scheduler files and startup catch-up outcome path. (role: recent cron scheduler contributor; confidence: medium; commits: a6768d9de567, 91edf1a83736, ce1219159858; files: src/cron/service/timer.ts, src/cron/service/jobs.ts, src/cron/service.restart-catchup.test.ts)
  • RichChen01: Authored recent startup catch-up deferral persistence work in the same scheduler state path. (role: adjacent startup catch-up contributor; confidence: medium; commits: 808e8f09bab3, 7be62779dd79, 16e7d24e3cd8; files: src/cron/service/timer.ts, src/cron/service/jobs.ts, src/cron/service/state.ts)

Codex review notes: model internal, reasoning high; reviewed against 4ae8d735bf54.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 8, 2026
@tzy-17
tzy-17 force-pushed the fix/101988-cron-catchup-skip-ok branch 2 times, most recently from fbc6ec9 to f11eb7f Compare July 8, 2026 04:34
@openclaw-barnacle openclaw-barnacle Bot added the extensions: lobster Extension: lobster label Jul 8, 2026
@tzy-17
tzy-17 force-pushed the fix/101988-cron-catchup-skip-ok branch from c8cff3b to f11eb7f Compare July 8, 2026 04:37
@openclaw-barnacle openclaw-barnacle Bot removed the extensions: lobster Extension: lobster label Jul 8, 2026
@tzy-17
tzy-17 force-pushed the fix/101988-cron-catchup-skip-ok branch 3 times, most recently from 0bf2c43 to e8c53db Compare July 8, 2026 05:41
@clawsweeper clawsweeper Bot removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 8, 2026
@tzy-17
tzy-17 force-pushed the fix/101988-cron-catchup-skip-ok branch from e8c53db to ae180f5 Compare July 8, 2026 05:54
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 8, 2026
@tzy-17
tzy-17 force-pushed the fix/101988-cron-catchup-skip-ok branch from ae180f5 to 2bfdf3a Compare July 8, 2026 06:08
tzy-17 and others added 2 commits July 8, 2026 16:16
…h status=ok

When the Gateway restarts, overdue cron jobs with lastRunStatus=ok
and lastRunAtMs >= nextRunAtMs were still considered runnable,
causing duplicate notifications to users.

Add a check in isRunnableJob: if the job already ran successfully
at or after its scheduled time, skip catchup.

Fixes openclaw#101988

Co-Authored-By: Claude <[email protected]>
@tzy-17
tzy-17 force-pushed the fix/101988-cron-catchup-skip-ok branch from 01b1cda to d5be0aa Compare July 8, 2026 08:16
@clawsweeper

clawsweeper Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

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

Labels

merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Skip catchup rerun for cron jobs with lastRunStatus=ok within current schedule window

1 participant