Skip to content

fix(cron): reject invalid cron expressions on disabled jobs before persisting#13

Open
suboss87 wants to merge 51 commits into
mainfrom
fix/74459-cron-edit-invalid-cron-expr
Open

fix(cron): reject invalid cron expressions on disabled jobs before persisting#13
suboss87 wants to merge 51 commits into
mainfrom
fix/74459-cron-edit-invalid-cron-expr

Conversation

@suboss87
Copy link
Copy Markdown
Owner

@suboss87 suboss87 commented Apr 30, 2026

Closes openclaw#74459

Problem

openclaw cron edit <id> --cron "invalid expr" on a disabled job silently persisted the invalid expression. Two bugs:

  1. ops.update only calls computeJobNextRunAtMs (which validates via croner) for enabled jobs. Disabled jobs skip that path entirely, so invalid cron syntax passes through to persist().
  2. When the user later ran cron edit --enable, applyJobPatch set job.enabled = true before computeJobNextRunAtMs threw, leaving the in-memory state.store.jobs entry with enabled: true even though persist was never reached. This "live state" corruption persisted until the next forced reload from disk.

Root cause

src/cron/service/ops.ts:update:

applyJobPatch(job, patch, ...); // mutates job.enabled = true in-place
// ...
if (job.enabled) {
  job.state.nextRunAtMs = computeJobNextRunAtMs(job, now); // only here does croner throw
}
// For disabled jobs: skips computeJobNextRunAtMs entirely → persist runs with invalid expr

Fix

Added validateCronScheduleExpr(schedule) to src/cron/validate-timestamp.ts. It dry-runs computeNextRunAtMs in a try/catch to detect croner parse errors for any cron-kind schedule.

Three call sites:

  • gateway/server-methods/cron.tscron.add and cron.update handlers: validate before reaching context.cron.* (returns structured error to caller)
  • src/cron/service/ops.tsupdate: validate before applyJobPatch so the job object is never mutated when the expression is invalid (defense in depth)

Tests

New file src/cron/service.cron-invalid-expr.test.ts (3 tests):

  • Rejects invalid cron expression on a disabled job; original schedule unchanged
  • Rejects invalid expression when trying to enable with the bad schedule
  • Accepts a valid cron expression change on a disabled job

Generated by Claude Code


Open in Devin Review

claude added 30 commits April 1, 2026 15:34
claude and others added 21 commits April 18, 2026 03:46
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: cron edit persists invalid --cron expressions on disabled jobs and can leave them enabled after failed enable

2 participants