Think scheduled tasks: "every 1 minutes" throws at reconcile, after app data is stored, and one invalid task stops all tasks reconciling · Issue #2638 · cloudflare/agents · GitHub
Skip to content

Think scheduled tasks: "every 1 minutes" throws at reconcile, after app data is stored, and one invalid task stops all tasks reconciling #2638

Open

Description

@TheDominator54

Versions: @cloudflare/think 0.20.2. Still reproduces on 0.21.0, and the inspected main at fc7f59f has the same code.

What happens

getScheduledTasks() may read app-owned data. The docs say to call internal_reconcileScheduledTasks() after changing it. Three problems follow:

  1. Strict plurals. The interval grammar accepts minute|minutes|hour|hours, then throws Use singular unit for "every 1 minutes" or Use plural unit for "every 2 minute". The meaning is unambiguous either way. A model or a user writing a schedule gets this wrong often, and the ThinkIntervalSchedule template type allows both forms.
  2. Checked only at reconcile. Nothing public validates a schedule before the app stores it. _declaredScheduleValidationError exists but is private. The error surfaces after the row is written.
  3. All or nothing. _normalizeDeclaredScheduledTasks() throws on the first invalid task, so no task is armed or updated, including valid ones. On startup the same throw is caught by _runBestEffortOnStartStep, which logs "Declared scheduled tasks were not reconciled on this wake; the next successful wake will reconcile them". But no wake will succeed until the bad row is gone.

Reproduction

export class ScheduleAgent extends Think<Env> {
  getDefaultTimezone() { return "UTC"; }
  getScheduledTasks() {
    const tasks: ThinkScheduledTasks = {};
    for (const j of this.sql`SELECT id, schedule FROM jobs`) tasks[j.id] = { schedule: j.schedule, handler: async () => {} };
    return tasks;
  }
  async set(schedules: string[]) {
    /* replace the jobs rows */
    await this.internal_reconcileScheduledTasks();   // throws for the cases below
  }
}
schedules reconcile tasks armed
["every 2 minutes"] ok 1
["every 1 minutes"] Use singular unit for "every 1 minutes" 0
["every 2 minute"] Use plural unit for "every 2 minute" 0
["every 2 minutes", "every 1 minutes"] Use singular unit … 0

Expected: accept either plural form and normalise it. Skip an invalid task with a warning, or report it per task, and arm the rest. Offer a public validator so apps can reject bad wording before storing it.

Proposed fix

  • In parseDeclaredTaskScheduleUnchecked, drop the two plural checks and normalise to every N minute(s).
  • In _normalizeDeclaredScheduledTasks, use tryParseDeclaredTaskSchedule, then console.warn and continue on error.
  • Export validateScheduledTaskSchedule(schedule, { timezone?, defaultTimezone? }): string | null, the public form of _declaredScheduleValidationError.

With the first two applied to the 0.21.0 build, all four cases above arm every valid task. ["every 2 minutes", "every 1 minutes", "every day at 9:00"] arms 2 tasks and logs Scheduled task "job2" skipped: Unsupported schedule DSL "every day at 9:00".

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions