Skip to content

fix(cron): honor configured retry.backoffMs for recurring error backoff floor#93051

Merged
steipete merged 3 commits into
openclaw:mainfrom
Alix-007:fix/cron-backoff-config
Jun 19, 2026
Merged

fix(cron): honor configured retry.backoffMs for recurring error backoff floor#93051
steipete merged 3 commits into
openclaw:mainfrom
Alix-007:fix/cron-backoff-config

Conversation

@Alix-007

@Alix-007 Alix-007 commented Jun 14, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

Summary

For recurring (every/cron) cron jobs that error, the safety-net backoff floor that sets nextRunAtMs was computed with the hardcoded DEFAULT_ERROR_BACKOFF_SCHEDULE_MS, silently ignoring the operator-configured cronConfig.retry.backoffMs. Every sibling path handling the same recurring-error backoff window passes the configured schedule (errorBackoffMs(..., retryConfig.backoffMs) on the retry-window branch; cronConfig?.retry?.backoffMs ?? DEFAULT... at the isErrorBackoffPending / deferPendingBackoffMissedCronSlots scheduler gates). Only the recurring safety-net floor in applyJobResult diverged. Result: the persisted nextRunAtMs is computed on the default schedule while the gate that decides runnability uses the configured one, so they disagree (early wasted wake-ups when config > default; over-long delays when config < default).

What changed

  • applyJobResult now passes the configured schedule to the recurring-error backoff floor: errorBackoffMs(job.state.consecutiveErrors ?? 1, state.deps.cronConfig?.retry?.backoffMs ?? DEFAULT_ERROR_BACKOFF_SCHEDULE_MS) — matching the sibling gate paths and the documented cron.retry.backoffMs behavior for recurring jobs.

Real behavior proof

  • Behavior or issue addressed: a recurring cron job that errors used the hardcoded default backoff floor for its persisted nextRunAtMs, ignoring cronConfig.retry.backoffMs; the scheduler gate enforced the configured window, so persisted next-run and gate disagreed.
  • Real environment tested: exact prepared PR head on AWS Crabbox cbx_c1082c8bfe60 (run run_54ed64e26dcd). The proof drives the real cron service execution path end-to-end — run(state, jobId, "due") → executeJobCoreWithTimeout (real runIsolatedAgentJob throwing a permanent error) → applyJobResult → persist into the SQLite-backed store — then reads nextRunAtMs back from SQLite via loadCronStore. Nothing on the backoff/persistence path is mocked; only the agent runner throws, to produce the error result.
  • Exact steps or command run after this patch: node scripts/run-vitest.mjs src/cron/service/timer.backoff-config-readback.test.ts
  • Evidence after fix (terminal capture): 
    $ node scripts/run-vitest.mjs src/cron/service/timer.backoff-config-readback.test.ts
 ✓ persists the configured retry.backoffMs floor across a real run and SQLite readback
 Test Files  1 passed (1)
      Tests  1 passed (1)

AWS Crabbox run_54ed64e26dcd
Test Files  2 passed (2)
     Tests  65 passed (65)
cron-backoff-sqlite-e2e=ok configured_ms=300000
  • Observed result after fix: with cronConfig.retry.backoffMs=[300000] and an every-1s job hit by a non-retryable permanent error, the value read back from the SQLite store is nextRunAtMs === endedAt + 300000 (configured floor), with lastStatus === "error" and consecutiveErrors === 1, instead of endedAt + 30000 (hardcoded default).
  • What was not tested: a long-lived multi-tick scheduler daemon across process restarts; the proof drives a single real service run and verifies the durable SQLite state rather than an in-memory value.
  • Negative control: reverting timer.ts to the default-only floor (git checkout eefeab5766^ -- src/cron/service/timer.ts) and re-running the test fails with AssertionError: expected 1772452830000 to be 1772453100000 — i.e. SQLite persisted endedAt+30000 instead of endedAt+300000 — confirming the test is not a tautology and the fix is required. Restoring the fix turns it green.

Tests and validation

  • node scripts/run-vitest.mjs src/cron/service/timer.backoff-config-readback.test.ts src/cron/service/timer.regression.test.ts → 65 passed locally and on AWS Crabbox (real run() execution + SQLite readback plus regression suite).
  • Redundant direct-unit coverage was removed during maintainer prep; the durable service-path test proves the same branch through persistence.
  • Branch autoreview and the final local duplicate-test deletion review are clean.
  • tsgo:core:test clean on changed files; oxfmt --write clean.

Risk checklist

  • User-visible behavior change? Yes (a fix) — recurring cron error backoff now honors cron.retry.backoffMs.
  • Config/env/migration change? No.
  • Security/auth/secrets/network/tool-execution change? No.
  • Highest-risk area: changing the persisted next-run for errored recurring jobs; covered by the new SQLite-readback test, the unit floor test, and the retained 63 regression tests.

@openclaw-barnacle openclaw-barnacle Bot added size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 19, 2026, 3:26 PM ET / 19:26 UTC.

Summary
The branch updates applyJobResult to pass configured cron.retry.backoffMs into the recurring error backoff floor and adds a service-level SQLite readback regression test.

PR surface: Source +3, Tests +80. Total +83 across 2 files.

Reproducibility: yes. Current main is source-reproducible: the recurring apply-time floor omits configured cron.retry.backoffMs, while sibling gates and maintenance recompute use the configured schedule; the PR also supplies terminal proof with SQLite readback and a negative control.

Review metrics: none identified.

Root-cause cluster
Relationship: canonical
Canonical: #93051
Summary: This PR is the live focused fix for the configured recurring error backoff floor; related cron backoff work is adjacent and does not supersede it.

Members:

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

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] Exact-head CI/security checks were still queued in live GitHub status at review time, so maintainers should wait for the normal required checks before landing.

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused call-site fix after exact-head checks pass, preserving the existing documented cron.retry contract for recurring jobs.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No automated repair is needed; maintainers should review or land the focused PR after exact-head checks pass.

Security
Cleared: The diff only changes cron scheduling logic and a Vitest regression test; it does not touch dependencies, workflows, package metadata, secrets, auth, network, install, or release surfaces.

Review details

Best possible solution:

Land the focused call-site fix after exact-head checks pass, preserving the existing documented cron.retry contract for recurring jobs.

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

Yes. Current main is source-reproducible: the recurring apply-time floor omits configured cron.retry.backoffMs, while sibling gates and maintenance recompute use the configured schedule; the PR also supplies terminal proof with SQLite readback and a negative control.

Is this the best way to solve the issue?

Yes. Passing the existing configured schedule into the one divergent errorBackoffMs call is the narrowest maintainable fix; the adjacent draft #74068 overlaps reload/recompute behavior but is not a viable replacement for this exact mismatch.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority cron scheduling correctness bug with limited blast radius to errored recurring jobs using custom retry backoff.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from an AWS Crabbox cron service run with SQLite readback plus a negative control showing the test fails without the runtime change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from an AWS Crabbox cron service run with SQLite readback plus a negative control showing the test fails without the runtime change.
Evidence reviewed

PR surface:

Source +3, Tests +80. Total +83 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 4 1 +3
Tests 1 80 0 +80
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 84 1 +83

What I checked:

  • Repository policy read: Root AGENTS.md was read fully and applied; no scoped AGENTS.md exists under src/cron, and the only maintainer note present is Telegram-specific and not relevant to cron. (AGENTS.md:1, efc36d71bdad)
  • Current-main bug source: On current main, the recurring error safety-net floor calls errorBackoffMs(job.state.consecutiveErrors ?? 1) without the configured retry schedule. (src/cron/service/timer.ts:896, efc36d71bdad)
  • Sibling gate uses configured backoff: The runnability gate already computes pending recurring error backoff with state.deps.cronConfig?.retry?.backoffMs ?? DEFAULT_ERROR_BACKOFF_SCHEDULE_MS, showing the apply-time floor is the divergent path. (src/cron/service/timer.ts:1516, efc36d71bdad)
  • Sibling recompute uses configured backoff: Maintenance/reload recompute already preserves recurring error backoff with the configured retry schedule, matching the PR's chosen call-site fix. (src/cron/service/jobs.ts:620, efc36d71bdad)
  • Existing helper supports the fix: errorBackoffMs already accepts an optional schedule array, so the PR reuses the existing scheduling helper instead of adding a parallel policy path. (src/cron/service/jobs.ts:67, efc36d71bdad)
  • User-facing config contract: The configuration reference describes cron.retry.backoffMs as the backoff delays for cron retries and says recurring jobs use the same transient retry policy plus error backoff. Public docs: docs/gateway/configuration-reference.md. (docs/gateway/configuration-reference.md:1317, efc36d71bdad)

Likely related people:

  • hugenshen: Merged retry-policy work introduced cron.retry, including backoffMs, and touched the timer/config surfaces this PR now aligns for recurring jobs. (role: introduced retry config surface; confidence: high; commits: ea3955cd78a9; files: src/cron/service/timer.ts, src/config/types.cron.ts)
  • mbelinky: Merged unresolved next-run backoff preservation touched timer.ts and jobs.ts and updated the recurring backoff/recompute boundary adjacent to this fix. (role: adjacent scheduler backoff contributor; confidence: high; commits: 190a4b48697b; files: src/cron/service/jobs.ts, src/cron/service/timer.ts)
  • steipete: The live PR is assigned to steipete, and the current head includes his cleanup commit keeping only the durable SQLite readback regression proof. (role: PR cleanup contributor and assigned reviewer; confidence: medium; commits: c8026d0aef2e; files: src/cron/service/timer.backoff-config-readback.test.ts)
  • Vincent Koc: Current checkout blame for the inspected recurring error block points to a recent carry-forward commit in the shallow checkout, useful as routing context but not original authorship. (role: recent area contributor; confidence: low; commits: a876f8d073cf; files: src/cron/service/timer.ts, src/cron/service/jobs.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 14, 2026
@Alix-007

Alix-007 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Upgraded the proof from unit-level to a service-level run with SQLite readback: a recurring job with cronConfig.retry.backoffMs=[300000], hit by a permanent error through the real run(state, id, "due") path → applyJobResult → persist, has nextRunAtMs = endedAt + 300000 read back from the SQLite-backed store via loadCronStore (not just the in-memory value). Negative control: reverting the timer.ts fix makes the SQLite readback show endedAt + 30000 and the test fails. tsgo core/test clean, oxfmt clean.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 15, 2026
@Alix-007

Alix-007 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@vincentkoc thanks for landing #93333! 🦞 When you're next sweeping the ready queue, #93051 is another ready diamond fix: it honors the configured retry.backoffMs floor for recurring cron error backoff (currently the configured floor is ignored). proof: sufficient + MERGEABLE, single-file and focused. No rush — appreciate a look whenever convenient.

@Alix-007
Alix-007 force-pushed the fix/cron-backoff-config branch from c6d4b97 to 8517995 Compare June 17, 2026 02:31
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 17, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 17, 2026
@Alix-007
Alix-007 force-pushed the fix/cron-backoff-config branch from 8517995 to 66f7fd2 Compare June 17, 2026 07:08
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 17, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 17, 2026
@Alix-007
Alix-007 force-pushed the fix/cron-backoff-config branch from 66f7fd2 to e7dc1e6 Compare June 18, 2026 02:15
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 18, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 18, 2026
@Alix-007
Alix-007 force-pushed the fix/cron-backoff-config branch from e7dc1e6 to b590d34 Compare June 19, 2026 13:55
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 19, 2026
@Alix-007
Alix-007 force-pushed the fix/cron-backoff-config branch 2 times, most recently from feb6ecf to da4eeef Compare June 19, 2026 14:02
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 19, 2026
@steipete steipete self-assigned this Jun 19, 2026
@steipete
steipete force-pushed the fix/cron-backoff-config branch from da4eeef to 15155ca Compare June 19, 2026 18:16
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 19, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 19, 2026
Alix-007 and others added 3 commits June 19, 2026 15:11
@Alix-007 @steipete
fix(cron): honor configured retry.backoffMs for recurring error backo…
d508337 
…ff floor
@Alix-007 @steipete
test(cron): prove configured backoffMs floor persists via real run + …
27ab3e3 
…SQLite readback

Complement the unit-level applyJobResult test with a service-level proof: a recurring job configured with cronConfig.retry.backoffMs, hit by a permanent error through the real run() execution path, persists nextRunAtMs = endedAt + configured backoffMs into the SQLite-backed store (read back via loadCronStore), not the hardcoded default.
@steipete
test(cron): keep durable backoff regression proof
c8026d0
@steipete
steipete force-pushed the fix/cron-backoff-config branch from 15155ca to c8026d0 Compare June 19, 2026 19:11
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 19, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 19, 2026
@steipete

steipete commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Maintainer verification complete.

  • Traced recurring error scheduling through run, applyJobResult, errorBackoffMs, persisted SQLite state, and sibling retry/backoff gates.
  • Kept the correct one-line owner-boundary fix and removed redundant direct-unit coverage; durable run/readback regression remains.
  • node scripts/run-vitest.mjs src/cron/service/timer.backoff-config-readback.test.ts src/cron/service/timer.regression.test.ts: 65 passed.
  • AWS Crabbox cbx_c1082c8bfe60, run run_54ed64e26dcd: 65 tests plus real cron run and SQLite readback passed.
  • Branch and final local autoreviews: clean.
  • Exact-head fallback CI: https://github.com/openclaw/openclaw/actions/runs/27844260733
  • Known gaps: none for the changed scheduler branch.

Land-ready at c8026d0aef2e6acfac1a752fecab0a302fb14e34.

@steipete
steipete merged commit 16fba65 into openclaw:main Jun 19, 2026
305 checks passed
@steipete

steipete commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

Thanks @Alix-007!

@github-actions github-actions Bot mentioned this pull request Jun 19, 2026
Open
This was referenced Jun 20, 2026
Merged
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 20, 2026
@Alix-007 @steipete
fix(cron): honor configured retry.backoffMs for recurring error backo…
ddb5627 
…ff floor (openclaw#93051)

Merged via squash.

Prepared head SHA: c8026d0
Co-authored-by: Alix-007 <[email protected]>
Co-authored-by: steipete <[email protected]>
Reviewed-by: @steipete
@clawsweeper clawsweeper Bot mentioned this pull request Jun 20, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Alix-007 @steipete