Skip to content

feat: add recurring card scheduler#39

Open
Jhacarreiro wants to merge 1 commit into
openclaw:mainfrom
Jhacarreiro:feat/recurring-cards-scheduler
Open

feat: add recurring card scheduler#39
Jhacarreiro wants to merge 1 commit into
openclaw:mainfrom
Jhacarreiro:feat/recurring-cards-scheduler

Conversation

@Jhacarreiro

Copy link
Copy Markdown

Summary

Adds first-class recurring cards so operational work can run on a cadence through the existing card/run scheduler instead of through long-running loops or ad-hoc cron jobs.

What changed

  • Adds schedule_json, next_run_at, and last_scheduled_run_at card columns.
  • Adds an interval schedule helper with validation and next-run computation.
  • Allows POST /api/cards to accept a schedule, for example { "kind": "interval", "everyMs": 86400000 }.
  • Wires recurring-card ticks into the existing Worker scheduled() handler.
  • Adds an owner-only /api/admin/scheduler/tick endpoint for manual scheduler ticks.
  • Documents recurring cards in the project docs/spec.

Validation

  • pnpm run build
  • pnpm test
  • pnpm run lint
  • pnpm run format

@clawsweeper

clawsweeper Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 15, 2026, 5:19 AM ET / 09:19 UTC.

Summary
The PR adds recurring card schedules through new cards-table columns, schedule parsing helpers, cron/admin tick wiring, tests, and docs.

Reproducibility: yes. from source inspection: POST /api/cards with a schedule such as {"kind":"interval","everyMs":60000,"startAt":0} reaches nextRecurringRunAt(schedule, now, null) and loops forward interval by interval. I did not execute it to keep the checkout read-only.

Review metrics: 2 noteworthy metrics.

  • Persistent schema: 1 D1 migration added. The PR changes stored card rows, so deployment and upgrade behavior matters before merge.
  • Scheduler entry points: 1 cron path changed, 1 owner API endpoint added. The new behavior runs from the per-minute Worker scheduled handler and a manual admin tick route.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • [P1] Fix next-run catch-up to O(1) and add regression coverage for historic startAt values.
  • Post redacted real Worker/D1 proof showing card creation, scheduler tick, run attempt creation, and nextRunAt advancement.
  • Show fresh-install and existing-database migration output for the new cards columns.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists validation commands but no after-fix real API/cron output; the contributor should post redacted terminal logs or live output and update the PR body to trigger a fresh review, or ask a maintainer for @clawsweeper re-review.

Risk before merge

  • [P1] A schedule with a historic startAt can make card creation or a cron tick spend many iterations catching up, which can stall Worker execution after merge.
  • [P1] The PR changes the persistent cards schema and then selects the new columns on normal card reads, so maintainers need fresh-install and existing-database upgrade proof.
  • [P1] The PR lacks after-fix real behavior proof; the listed validation commands do not show a live card creation, scheduler tick, run attempt, or nextRunAt advance.

Maintainer options:

  1. Fix bounded next-run math (recommended)
    Compute missed intervals in O(1), add regression coverage for historic startAt and stale lastScheduledRunAt, then retest create and tick paths.
  2. Prove D1 upgrade behavior
    Before merge, show fresh database and existing database migration output plus redacted real Worker/D1 scheduler proof.
  3. Pause core recurrence
    If maintainers do not want persistent recurrence in Crabfleet core yet, defer this PR to a narrower scheduler design discussion.

Next step before merge

  • [P1] Contributor action is needed to fix the boundedness bug and add real behavior plus migration proof; automation should not repair while the external proof gate is still missing.

Security
Cleared: The diff adds an owner-only scheduler tick route and no new dependencies, secret handling, action refs, or package-resolution changes; the concrete blocker is availability, not security.

Review findings

  • [P1] Avoid looping through every missed interval — src/recurring-cards.ts:45-46
Review details

Best possible solution:

Keep the card/run-level recurrence model only after next-run computation is bounded, the D1 upgrade path is proven, and create/tick behavior is demonstrated in a real Worker/D1 setup.

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

Yes, from source inspection: POST /api/cards with a schedule such as {"kind":"interval","everyMs":60000,"startAt":0} reaches nextRecurringRunAt(schedule, now, null) and loops forward interval by interval. I did not execute it to keep the checkout read-only.

Is this the best way to solve the issue?

No: modeling recurrence at the card/run layer is plausible, but the proposed next-run computation must be bounded and the schema/runtime path needs real upgrade proof before this is the best maintainable solution.

Full review comments:

  • [P1] Avoid looping through every missed interval — src/recurring-cards.ts:45-46
    normalizeCardSchedule allows startAt all the way down to 0, and createCard calls this helper with lastScheduledRunAt null. For { kind: "interval", everyMs: 60000, startAt: 0 }, this loop has to add one minute at a time from 1970 to now before returning, which can stall card creation or cron ticks. Compute the catch-up arithmetically or clamp old anchors instead of iterating each missed interval.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.89

AGENTS.md: not found in the target repository.

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

Label changes

Label changes:

  • add P2: This is a normal-priority feature PR with a blocking implementation issue but no merged user-facing regression yet.
  • add merge-risk: 🚨 compatibility: The PR adds persistent cards columns and normal read paths depend on them, so existing D1 databases need proven migration safety.
  • add merge-risk: 🚨 availability: The accepted schedule shape can trigger an unbounded catch-up loop that may stall card creation or cron ticks after merge.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists validation commands but no after-fix real API/cron output; the contributor should post redacted terminal logs or live output and update the PR body to trigger a fresh review, or ask a maintainer for @clawsweeper re-review.

Label justifications:

  • P2: This is a normal-priority feature PR with a blocking implementation issue but no merged user-facing regression yet.
  • merge-risk: 🚨 availability: The accepted schedule shape can trigger an unbounded catch-up loop that may stall card creation or cron ticks after merge.
  • merge-risk: 🚨 compatibility: The PR adds persistent cards columns and normal read paths depend on them, so existing D1 databases need proven migration safety.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists validation commands but no after-fix real API/cron output; the contributor should post redacted terminal logs or live output and update the PR body to trigger a fresh review, or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Target AGENTS.md check: No AGENTS.md exists inside the crabfleet git root; the only AGENTS.md found was in the parent ClawSweeper checkout and was not applied as target policy.
  • Current main lacks recurring card implementation: Current main has no schedule_json, next_run_at, last_scheduled_run_at, CardSchedule, or nextRecurringRunAt symbols, so the requested feature is not already implemented. (3849a6234c41)
  • Blocking catch-up loop: The PR accepts startAt values as low as 0 and then advances next by repeatedly adding everyMs until it passes now, which can mean tens of millions of loop iterations for an old startAt with the minimum interval. (src/recurring-cards.ts:45, a132c4f8ac7b)
  • Create path reaches the loop: createCard stores schedule_json and computes next_run_at by calling nextRecurringRunAt(schedule, now, null), so a historic startAt can stall card creation before the scheduler even runs. (src/index.ts:13658, a132c4f8ac7b)
  • Persistent schema change: The PR adds three persistent cards columns and an index, so merge safety depends on D1 fresh-install and upgrade migration proof. (migrations/0020_recurring_cards.sql:1, a132c4f8ac7b)
  • Proof check: The PR body lists build, test, lint, and format commands, but includes no terminal output, logs, API output, screenshot, recording, or linked artifact showing the new schedule behavior running in a real setup. (a132c4f8ac7b)

Likely related people:

  • Peter Steinberger: git blame points the current createCard, claimRunning, scheduled handler, and base card/run migrations to the initial crabfleet import. (role: introduced current card/run scheduler surface; confidence: high; commits: c18ec62b96e9; files: src/index.ts, migrations/0001_initial.sql, migrations/0007_run_attempts.sql)
  • Vincent Koc: Recent src/index.ts and lifecycle/migration work shows substantial adjacent maintenance on the Worker and persistence paths this PR extends. (role: recent area contributor; confidence: medium; commits: 8f28f362dbeb, e058374195c7, c82cc041af6e; files: src/index.ts, migrations/0026_openclaw_lifecycle_guarantees.sql, tests/openclaw-service.test.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: 🧂 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. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant