From fcf6aea87c5983f281dc04f954374b6cc38c3fa8 Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 25 May 2026 22:19:59 -0700 Subject: [PATCH] feat(scheduler): simpler schedule rules picker Replaces raw cron exposure in the schedule editor with a structured "Every N [hours|days|weeks] starting on [date] at [time]" composer that supports multiple rules per task (plus-button to add). - DB: nullable scheduled_tasks.schedule_rules JSONB (idempotent ALTER). Legacy cron_expr/cron_expression columns stay populated from rule[0] for back-compat with the croner engine. - API: POST/PATCH /api/scheduled-tasks accepts EITHER cron_expr (legacy) OR schedule_rules. Server derives + stores both. next_3_runs preview merges fires across all rules. - Scheduler registry: arms one Cron registration per rule; all fires route to dispatcher.fire(taskId). 5s cross-rule dedupe guard prevents overlapping rules from double-firing. Per-rule start_at gate skips pre-start fires; weekly interval>1 enforced in-process. - Web: new ScheduleRulesBuilder + ScheduleRuleRow components, native date/time inputs, mobile-stacked layout, indigo accent. Editor hydrates from schedule_rules when present, else seeds a default rule. Tests: 19 new unit tests for schedule-rules.ts. Existing 265 hub tests remain green. Co-Authored-By: Claude Opus 4.7 (1M context) --- hub/src/api/scheduled-tasks.ts | 95 +++++++-- hub/src/db/scheduled-tasks-dal.ts | 21 +- hub/src/db/schema.sql | 8 + hub/src/scheduler/registry.ts | 109 ++++++++--- hub/src/scheduler/schedule-rules.ts | 141 ++++++++++++++ hub/test/schedule-rules.test.ts | 122 ++++++++++++ web/src/components/ScheduleEditor.tsx | 35 +++- web/src/components/ScheduleRulesBuilder.tsx | 203 ++++++++++++++++++++ web/src/hooks/useSchedules.ts | 9 +- web/src/lib/schedule-rules.ts | 86 +++++++++ 10 files changed, 777 insertions(+), 52 deletions(-) create mode 100644 hub/src/scheduler/schedule-rules.ts create mode 100644 hub/test/schedule-rules.test.ts create mode 100644 web/src/components/ScheduleRulesBuilder.tsx create mode 100644 web/src/lib/schedule-rules.ts diff --git a/hub/src/api/scheduled-tasks.ts b/hub/src/api/scheduled-tasks.ts index bf5eed4..a324267 100644 --- a/hub/src/api/scheduled-tasks.ts +++ b/hub/src/api/scheduled-tasks.ts @@ -23,6 +23,7 @@ import { deleteTask, } from '../db/scheduled-tasks-dal.ts' import { validate as validateCron, nextRuns, isValidTimezone } from '../scheduler/cron.ts' +import { validateRules, ruleToCron, type ScheduleRule } from '../scheduler/schedule-rules.ts' import * as registry from '../scheduler/registry.ts' import * as dispatcher from '../scheduler/dispatcher.ts' import { @@ -53,7 +54,13 @@ const CreateSchema = z.object({ target_kind: TargetKindEnum, target_id: z.string().min(1).nullable().optional(), payload: z.record(z.any()).optional(), - cron_expr: z.string().min(1).max(200), + // Either `cron_expr` (legacy) or `schedule_rules` (new). Validated below. + cron_expr: z.string().min(1).max(200).optional(), + schedule_rules: z.array(z.object({ + interval: z.number().int().min(1).max(999), + unit: z.enum(['hours', 'days', 'weeks']), + start_at: z.string().min(1), + })).min(1).max(20).optional(), timezone: z.string().min(1).max(100), catchup_policy: CatchupPolicyEnum.optional(), max_concurrent: z.number().int().min(1).max(10).optional(), @@ -67,8 +74,23 @@ const PatchSchema = CreateSchema.partial() function withNext3(task: T) { if (!task.enabled) return { ...task, next_3_runs: [] as string[] } - const expr = task.cron_expr || task.cron_expression const tz = task.timezone || 'UTC' + const rules = Array.isArray(task.schedule_rules) ? (task.schedule_rules as ScheduleRule[]) : [] + if (rules.length > 0) { + const merged: number[] = [] + for (const r of rules) { + try { + const expr = ruleToCron(r, tz) + const startMs = Date.parse(r.start_at) + const from = Number.isFinite(startMs) && startMs > Date.now() ? new Date(startMs) : new Date() + for (const d of nextRuns(expr, tz, 3, from)) merged.push(d.getTime()) + } catch {} + } + merged.sort((a, b) => a - b) + const top = Array.from(new Set(merged)).slice(0, 3).map(ms => new Date(ms).toISOString()) + return { ...task, next_3_runs: top } + } + const expr = task.cron_expr || task.cron_expression if (!expr) return { ...task, next_3_runs: [] as string[] } const runs = nextRuns(expr, tz, 3).map((d) => d.toISOString()) return { ...task, next_3_runs: runs } @@ -78,6 +100,25 @@ function targetIdRequired(kind: string): boolean { return kind === 'session' || kind === 'supervisor' } +/** + * Resolve the effective cron_expr to persist + register. Priority: + * - explicit `cron_expr` from the body (legacy clients) + * - rule[0] of `schedule_rules` converted to cron + * - existing row's cron_expr (PATCH path) + * + * Returns null if neither is available. + */ +function resolveCronExpr( + body: { cron_expr?: string; schedule_rules?: ScheduleRule[] }, + tz: string, +): string | null { + if (body.cron_expr) return body.cron_expr + if (body.schedule_rules && body.schedule_rules.length > 0) { + return ruleToCron(body.schedule_rules[0], tz) + } + return null +} + interface ValidateOptions { cron_expr?: string timezone?: string @@ -220,8 +261,25 @@ scheduledTasks.post('/', async (c) => { } const data = parsed.data + // Require either cron_expr or schedule_rules. Derive cron_expr from rule[0] + // when only schedule_rules was sent. The legacy column stays populated. + if (!data.cron_expr && (!data.schedule_rules || data.schedule_rules.length === 0)) { + return c.json({ error: 'schedule_required', detail: 'cron_expr or schedule_rules is required' }, 400) + } + if (data.schedule_rules) { + const rv = validateRules(data.schedule_rules) + if (!rv.ok) return c.json({ error: 'invalid_schedule_rules', detail: rv.error }, 400) + } + const effectiveCron = resolveCronExpr( + { cron_expr: data.cron_expr, schedule_rules: data.schedule_rules as ScheduleRule[] | undefined }, + data.timezone, + ) + if (!effectiveCron) { + return c.json({ error: 'schedule_required' }, 400) + } + const v = validateInputs({ - cron_expr: data.cron_expr, + cron_expr: effectiveCron, timezone: data.timezone, target_kind: data.target_kind, target_id: data.target_id, @@ -246,7 +304,7 @@ scheduledTasks.post('/', async (c) => { target_kind: data.target_kind, target_id: data.target_id ?? null, payload: data.payload ?? {}, - cron_expr: data.cron_expr, + cron_expr: effectiveCron, }, data.name_suffix, data.name) const task = await createTaskV2({ @@ -256,17 +314,18 @@ scheduledTasks.post('/', async (c) => { target_kind: data.target_kind, target_id: data.target_id ?? null, payload: data.payload ?? {}, - cron_expr: data.cron_expr, + cron_expr: effectiveCron, timezone: data.timezone, catchup_policy: data.catchup_policy ?? 'skip', max_concurrent: data.max_concurrent ?? 1, enabled: data.enabled ?? true, post_run_actions: v.actions, session_id: sessionId, - cron_expression: data.cron_expr, + cron_expression: effectiveCron, prompt: typeof data.payload?.prompt === 'string' ? data.payload.prompt : '', name_prefix: built.prefix || null, name_suffix: built.suffix || null, + schedule_rules: data.schedule_rules ?? null, }) registry.register(task) @@ -286,10 +345,20 @@ scheduledTasks.patch('/:id', async (c) => { } const data = parsed.data - // Merge effective values for validation. + if (data.schedule_rules) { + const rv = validateRules(data.schedule_rules) + if (!rv.ok) return c.json({ error: 'invalid_schedule_rules', detail: rv.error }, 400) + } + + // Merge effective values for validation. If client sent schedule_rules, + // derive a cron from rule[0]; otherwise fall back to explicit cron_expr. + const effectiveTimezone = data.timezone ?? existing.timezone + const derivedFromRules = data.schedule_rules + ? resolveCronExpr({ schedule_rules: data.schedule_rules as ScheduleRule[] }, effectiveTimezone) + : null const effective = { - cron_expr: data.cron_expr ?? existing.cron_expr ?? existing.cron_expression, - timezone: data.timezone ?? existing.timezone, + cron_expr: data.cron_expr ?? derivedFromRules ?? existing.cron_expr ?? existing.cron_expression, + timezone: effectiveTimezone, target_kind: data.target_kind ?? existing.target_kind, target_id: data.target_id !== undefined ? data.target_id : existing.target_id, @@ -297,9 +366,8 @@ scheduledTasks.patch('/:id', async (c) => { data.post_run_actions !== undefined ? data.post_run_actions : existing.post_run_actions, } const v = validateInputs({ - // Only re-validate fields the caller actually changed; on partial PATCH - // we still want to reject if e.g. only the cron was sent and it's bad. - cron_expr: data.cron_expr, + // Validate the (possibly derived) effective cron when rules changed too. + cron_expr: data.cron_expr ?? derivedFromRules ?? undefined, timezone: data.timezone, target_kind: data.target_kind, // target_id pairing depends on the effective kind, so always check it: @@ -356,11 +424,12 @@ scheduledTasks.patch('/:id', async (c) => { target_kind: data.target_kind, target_id: data.target_id !== undefined ? data.target_id ?? null : undefined, payload: data.payload, - cron_expr: data.cron_expr, + cron_expr: data.cron_expr ?? derivedFromRules ?? undefined, timezone: data.timezone, catchup_policy: data.catchup_policy, max_concurrent: data.max_concurrent, post_run_actions: data.post_run_actions !== undefined ? v.actions : undefined, + schedule_rules: data.schedule_rules !== undefined ? (data.schedule_rules as any[]) : undefined, }) if (!updated) return c.json({ error: 'not_found' }, 404) diff --git a/hub/src/db/scheduled-tasks-dal.ts b/hub/src/db/scheduled-tasks-dal.ts index cdb1d35..5fe92d5 100644 --- a/hub/src/db/scheduled-tasks-dal.ts +++ b/hub/src/db/scheduled-tasks-dal.ts @@ -56,6 +56,10 @@ export interface ScheduledTask { // authoritative for back-compat — DAL writes keep all three in sync. name_prefix: string | null name_suffix: string | null + // Simpler-cron picker: structured rules. NULL on legacy rows (UI falls + // back to deriving a single rule from `cron_expr`). Array of + // `{interval, unit, start_at}`. + schedule_rules: any[] | null // Derived from latest finalized run via LATERAL JOIN in listTasksForUser/getTask. last_run_cost_usd?: number | null last_run_duration_ms?: number | null @@ -152,6 +156,12 @@ function normalize(row: any): ScheduledTask { typeof row.post_run_actions === 'string' ? JSON.parse(row.post_run_actions) : (row.post_run_actions ?? []) + const schedule_rules = + row.schedule_rules == null + ? null + : typeof row.schedule_rules === 'string' + ? JSON.parse(row.schedule_rules) + : row.schedule_rules // node-postgres returns NUMERIC as string; coerce when present. EXTRACT(EPOCH ...) // comes back as a number (double precision) but normalize defensively. const last_run_cost_usd = @@ -167,6 +177,7 @@ function normalize(row: any): ScheduledTask { on_complete, payload, post_run_actions, + schedule_rules, last_run_cost_usd, last_run_duration_ms, } @@ -207,13 +218,14 @@ export async function createTaskV2(input: { prompt?: string name_prefix?: string | null name_suffix?: string | null + schedule_rules?: any[] | null }): Promise { const rows = await sql` INSERT INTO scheduled_tasks ( user_id, session_id, name, cron_expression, prompt, enabled, task_type, target_kind, target_id, payload, cron_expr, timezone, catchup_policy, max_concurrent, post_run_actions, - name_prefix, name_suffix + name_prefix, name_suffix, schedule_rules ) VALUES ( ${input.user_id}, ${input.session_id}, ${input.name}, ${input.cron_expression ?? input.cron_expr}, ${input.prompt ?? ''}, @@ -223,7 +235,8 @@ export async function createTaskV2(input: { ${input.timezone}, ${input.catchup_policy ?? 'skip'}, ${input.max_concurrent ?? 1}, ${sql.json((input.post_run_actions ?? []) as any)}, - ${input.name_prefix ?? null}, ${input.name_suffix ?? null} + ${input.name_prefix ?? null}, ${input.name_suffix ?? null}, + ${input.schedule_rules ? sql.json(input.schedule_rules as any) : null} ) RETURNING * ` @@ -247,6 +260,7 @@ export async function updateTaskV2( post_run_actions: PostRunAction[] name_prefix: string | null name_suffix: string | null + schedule_rules: any[] | null }>, ): Promise { const sets: any[] = [] @@ -268,6 +282,9 @@ export async function updateTaskV2( if (fields.post_run_actions !== undefined) { sets.push(sql`post_run_actions = ${sql.json(fields.post_run_actions as any)}`) } + if (fields.schedule_rules !== undefined) { + sets.push(sql`schedule_rules = ${fields.schedule_rules ? sql.json(fields.schedule_rules as any) : null}`) + } if (sets.length === 0) return getTask(id, userId) sets.push(sql`updated_at = now()`) diff --git a/hub/src/db/schema.sql b/hub/src/db/schema.sql index 9153808..14eb831 100644 --- a/hub/src/db/schema.sql +++ b/hub/src/db/schema.sql @@ -189,6 +189,14 @@ ALTER TABLE scheduled_tasks ADD COLUMN IF NOT EXISTS post_run_actions JSONB NOT ALTER TABLE scheduled_tasks ADD COLUMN IF NOT EXISTS name_prefix TEXT; ALTER TABLE scheduled_tasks ADD COLUMN IF NOT EXISTS name_suffix TEXT; +-- Simpler-cron picker (feat/simpler-cron-ui): structured rules array. Each +-- rule shape: { interval: int, unit: 'hours'|'days'|'weeks', start_at: ISO }. +-- Legacy `cron_expr`/`cron_expression` columns are still populated from +-- rule[0] on write for back-compat with the croner engine. Multiple rules +-- arm multiple cron registrations; fires from any rule route through the +-- same dispatcher.fire(task.id). +ALTER TABLE scheduled_tasks ADD COLUMN IF NOT EXISTS schedule_rules JSONB; + -- W2/T8: drop legacy NOT NULL on session_id so fan-out tasks -- (all_agents/all_supervisors) and supervisor-targeted tasks can omit it. -- Idempotent — Postgres no-ops if the column is already nullable. diff --git a/hub/src/scheduler/registry.ts b/hub/src/scheduler/registry.ts index fc85a54..270b458 100644 --- a/hub/src/scheduler/registry.ts +++ b/hub/src/scheduler/registry.ts @@ -1,6 +1,12 @@ /** * Scheduler registry (W2/T5). Owns the in-memory map of croner jobs keyed by * scheduled_tasks.id. Each job's callback delegates to the dispatcher. + * + * A task may have MULTIPLE rules (`schedule_rules` array on the task row). + * Each rule arms its own Cron registration; all rules route to the same + * `dispatcher.fire(taskId)`. A short cross-rule dedupe window prevents two + * rules whose cron expressions overlap from double-firing. + * * Distinct from the legacy v0 scheduler at `hub/src/scheduler/index.ts`. */ import { Cron } from 'croner' @@ -10,6 +16,7 @@ import { getTaskById, setTaskFireTimestamps, } from '../db/scheduled-tasks-dal.ts' +import { ruleToCron, shouldSkipFire, type ScheduleRule } from './schedule-rules.ts' type Dispatcher = typeof import('./dispatcher.ts') let _dispatcher: Dispatcher | null = null @@ -19,15 +26,24 @@ async function dispatcher(): Promise { return _dispatcher } -const jobs = new Map() +const jobs = new Map() +const lastFireAt = new Map() +const DEDUPE_WINDOW_MS = 5000 -export function get(taskId: string): Cron | undefined { return jobs.get(taskId) } +export function get(taskId: string): Cron | undefined { return jobs.get(taskId)?.[0] } export function size(): number { return jobs.size } export function nextRunFor(taskId: string): Date | null { - const j = jobs.get(taskId) - if (!j) return null - try { return j.nextRun() ?? null } catch { return null } + const list = jobs.get(taskId) + if (!list || list.length === 0) return null + let best: Date | null = null + for (const j of list) { + try { + const n = j.nextRun() + if (n && (!best || n.getTime() < best.getTime())) best = n + } catch {} + } + return best } export async function loadAll(): Promise { @@ -46,43 +62,78 @@ export async function replace(taskId: string): Promise { } export function unregister(taskId: string): void { - const existing = jobs.get(taskId) - if (!existing) return - try { existing.stop() } catch {} + const list = jobs.get(taskId) + if (!list) return + for (const j of list) { try { j.stop() } catch {} } jobs.delete(taskId) + lastFireAt.delete(taskId) } export function pauseAll(): void { - for (const [, j] of jobs) { try { j.pause() } catch {} } + for (const [, list] of jobs) for (const j of list) { try { j.pause() } catch {} } } export function resumeAll(): void { - for (const [, j] of jobs) { try { j.resume() } catch {} } + for (const [, list] of jobs) for (const j of list) { try { j.resume() } catch {} } } function registerInternal(task: ScheduledTask): void { const existing = jobs.get(task.id) - if (existing) { try { existing.stop() } catch {} ; jobs.delete(task.id) } + if (existing) { for (const j of existing) { try { j.stop() } catch {} } ; jobs.delete(task.id) } if (!task.enabled) return - const expr = task.cron_expr || task.cron_expression - if (!expr) { - console.error(`[scheduler.registry] task=${task.id} has no cron expression`) + const tz = task.timezone || 'UTC' + + // Resolve schedule sources, in priority: + // 1) task.schedule_rules (new shape) + // 2) task.cron_expr / cron_expression (legacy) + const rules: ScheduleRule[] = Array.isArray(task.schedule_rules) ? task.schedule_rules : [] + const legacyExpr = task.cron_expr || task.cron_expression + + const armed: Cron[] = [] + + const armCron = (expr: string, rule: ScheduleRule | null) => { + try { + const job = new Cron(expr, { timezone: tz, protect: true, paused: false }, async () => { + // Cross-rule dedupe — if any rule for this task fired within the + // dedupe window, drop this fire. + const now = Date.now() + const last = lastFireAt.get(task.id) ?? 0 + if (now - last < DEDUPE_WINDOW_MS) return + // Rule-level skip gate (start_at + weekly interval anchoring). + if (rule && shouldSkipFire(rule, new Date(now))) return + lastFireAt.set(task.id, now) + try { + const d = await dispatcher() + await d.fire(task.id) + } catch (err: any) { + console.error(`[scheduler.registry] fire failed task=${task.id}: ${err?.message ?? err}`) + } + }) + armed.push(job) + } catch (err: any) { + console.error(`[scheduler.registry] arm failed task=${task.id} expr=${expr}: ${err?.message ?? err}`) + } + } + + if (rules.length > 0) { + for (const r of rules) armCron(ruleToCron(r, tz), r) + } else if (legacyExpr) { + armCron(legacyExpr, null) + } else { + console.error(`[scheduler.registry] task=${task.id} has no schedule_rules or cron expression`) return } - const tz = task.timezone || 'UTC' - try { - const job = new Cron(expr, { timezone: tz, protect: true, paused: false }, async () => { - try { - const d = await dispatcher() - await d.fire(task.id) - } catch (err: any) { - console.error(`[scheduler.registry] fire failed task=${task.id}: ${err?.message ?? err}`) - } - }) - jobs.set(task.id, job) - const next = job.nextRun() ?? null - void setTaskFireTimestamps(task.id, task.last_fire_at ? new Date(task.last_fire_at) : null, next) - } catch (err: any) { - console.error(`[scheduler.registry] registerInternal failed task=${task.id}: ${err?.message ?? err}`) + + if (armed.length === 0) return + jobs.set(task.id, armed) + + // Record the soonest next fire across all rules. + let nextOverall: Date | null = null + for (const j of armed) { + try { + const n = j.nextRun() ?? null + if (n && (!nextOverall || n.getTime() < nextOverall.getTime())) nextOverall = n + } catch {} } + void setTaskFireTimestamps(task.id, task.last_fire_at ? new Date(task.last_fire_at) : null, nextOverall) } diff --git a/hub/src/scheduler/schedule-rules.ts b/hub/src/scheduler/schedule-rules.ts new file mode 100644 index 0000000..77e2670 --- /dev/null +++ b/hub/src/scheduler/schedule-rules.ts @@ -0,0 +1,141 @@ +/** + * ScheduleRule — structured replacement for raw cron strings. + * + * Shape: { interval: int>=1, unit: 'hours'|'days'|'weeks', start_at: ISO } + * + * Conversion to cron is anchored at minute/hour-of-day of `start_at` in the + * task's timezone (resolved client-side as the wall-clock components of + * start_at). Interval anchoring beyond hour-of-day (e.g. "every 3 days from + * Tuesday the 14th") is approximate — cron cannot express arbitrary day + * anchoring — but the registry gates each fire with `start_at` so runs + * before the start date are skipped. + * + * Web mirror lives at `web/src/lib/schedule-rules.ts`; both files are + * intentionally tiny and duplicated to avoid a cross-package import. + */ + +export type ScheduleUnit = 'hours' | 'days' | 'weeks' + +export interface ScheduleRule { + interval: number + unit: ScheduleUnit + start_at: string // ISO 8601 +} + +export type RuleValidation = { ok: true } | { ok: false; error: string } + +export function validateRule(rule: unknown): RuleValidation { + if (!rule || typeof rule !== 'object') return { ok: false, error: 'rule must be an object' } + const r = rule as Partial + if (typeof r.interval !== 'number' || !Number.isInteger(r.interval) || r.interval < 1 || r.interval > 999) { + return { ok: false, error: 'interval must be integer 1..999' } + } + if (r.unit !== 'hours' && r.unit !== 'days' && r.unit !== 'weeks') { + return { ok: false, error: 'unit must be hours, days, or weeks' } + } + if (typeof r.start_at !== 'string' || Number.isNaN(Date.parse(r.start_at))) { + return { ok: false, error: 'start_at must be a valid ISO date string' } + } + return { ok: true } +} + +export function validateRules(rules: unknown): { ok: true; rules: ScheduleRule[] } | { ok: false; error: string } { + if (!Array.isArray(rules)) return { ok: false, error: 'schedule_rules must be an array' } + if (rules.length === 0) return { ok: false, error: 'schedule_rules must have at least one rule' } + if (rules.length > 20) return { ok: false, error: 'schedule_rules max 20 rules' } + for (let i = 0; i < rules.length; i++) { + const v = validateRule(rules[i]) + if (!v.ok) return { ok: false, error: `rule[${i}]: ${v.error}` } + } + return { ok: true, rules: rules as ScheduleRule[] } +} + +/** + * Convert a ScheduleRule to a 5-field cron expression. + * + * - hours: anchored on minute-of-hour from start_at; fires every N hours + * "MM STAR_SLASH_N STAR STAR STAR". For interval=1 the cron uses STAR. + * - days: anchored on hh:mm from start_at; fires every N days + * "MM HH STAR_SLASH_N STAR STAR". For interval=1 the DOM field is STAR. + * - weeks: anchored on hh:mm + weekday from start_at. interval is enforced + * by the dispatcher (cron can't express "every N weeks"). For interval=1 + * the cron is "MM HH STAR STAR DOW"; for >1 we still emit weekly cron + * and the registry skips fires where (now - start_at) / 7d % interval != 0. + * + * `tz` is the IANA timezone used to extract the wall-clock components of + * start_at — this matches how croner interprets the resulting cron. + */ +export function ruleToCron(rule: ScheduleRule, tz: string = 'UTC'): string { + const d = new Date(rule.start_at) + const parts = extractWallClock(d, tz) + const { mm, hh, dow } = parts + + switch (rule.unit) { + case 'hours': { + if (rule.interval === 1) return `${mm} * * * *` + return `${mm} */${rule.interval} * * *` + } + case 'days': { + if (rule.interval === 1) return `${mm} ${hh} * * *` + return `${mm} ${hh} */${rule.interval} * *` + } + case 'weeks': { + return `${mm} ${hh} * * ${dow}` + } + } +} + +function extractWallClock(d: Date, tz: string): { mm: number; hh: number; dow: number } { + try { + const fmt = new Intl.DateTimeFormat('en-US', { + timeZone: tz, + hour: '2-digit', + minute: '2-digit', + weekday: 'short', + hour12: false, + }) + const parts = fmt.formatToParts(d) + const get = (t: string) => parts.find(p => p.type === t)?.value ?? '' + let hh = parseInt(get('hour'), 10) + if (hh === 24) hh = 0 // some locales emit "24" + const mm = parseInt(get('minute'), 10) + const wk = get('weekday') + const dow = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat'].indexOf(wk) + return { + mm: Number.isFinite(mm) ? mm : 0, + hh: Number.isFinite(hh) ? hh : 0, + dow: dow >= 0 ? dow : 0, + } + } catch { + return { mm: 0, hh: 0, dow: 0 } + } +} + +/** + * Should a fire at `now` be skipped because the rule's start_at is in the + * future, OR because the rule is `weeks` with interval > 1 and `now` doesn't + * line up with the start_at weekly cadence? + */ +export function shouldSkipFire(rule: ScheduleRule, now: Date = new Date()): boolean { + const start = new Date(rule.start_at) + if (now.getTime() < start.getTime()) return true + if (rule.unit === 'weeks' && rule.interval > 1) { + const weekMs = 7 * 24 * 60 * 60 * 1000 + const elapsed = now.getTime() - start.getTime() + const weeksSince = Math.floor(elapsed / weekMs) + if (weeksSince % rule.interval !== 0) return true + } + return false +} + +/** Pretty plain-English render. */ +export function humanizeRule(rule: ScheduleRule): string { + const unitWord = rule.interval === 1 + ? (rule.unit === 'hours' ? 'hour' : rule.unit === 'days' ? 'day' : 'week') + : rule.unit + const start = new Date(rule.start_at) + const dateStr = isNaN(start.getTime()) + ? '—' + : start.toLocaleString(undefined, { dateStyle: 'medium', timeStyle: 'short' }) + return `Every ${rule.interval} ${unitWord} starting ${dateStr}` +} diff --git a/hub/test/schedule-rules.test.ts b/hub/test/schedule-rules.test.ts new file mode 100644 index 0000000..fbef025 --- /dev/null +++ b/hub/test/schedule-rules.test.ts @@ -0,0 +1,122 @@ +/** + * Unit tests for the ScheduleRule → cron conversion + validation utility. + */ +import { describe, expect, test } from 'bun:test' +import { + validateRule, + validateRules, + ruleToCron, + shouldSkipFire, + humanizeRule, + type ScheduleRule, +} from '../src/scheduler/schedule-rules.ts' + +describe('validateRule', () => { + test('accepts a valid rule', () => { + const r: ScheduleRule = { interval: 2, unit: 'hours', start_at: '2030-01-01T09:30:00.000Z' } + expect(validateRule(r)).toEqual({ ok: true }) + }) + + test('rejects non-integer interval', () => { + expect(validateRule({ interval: 1.5, unit: 'hours', start_at: '2030-01-01T00:00:00Z' } as any).ok).toBe(false) + }) + + test('rejects interval out of range', () => { + expect(validateRule({ interval: 0, unit: 'hours', start_at: '2030-01-01T00:00:00Z' } as any).ok).toBe(false) + expect(validateRule({ interval: 1000, unit: 'hours', start_at: '2030-01-01T00:00:00Z' } as any).ok).toBe(false) + }) + + test('rejects bad unit', () => { + expect(validateRule({ interval: 1, unit: 'months', start_at: '2030-01-01T00:00:00Z' } as any).ok).toBe(false) + }) + + test('rejects bad start_at', () => { + expect(validateRule({ interval: 1, unit: 'hours', start_at: 'nope' } as any).ok).toBe(false) + }) +}) + +describe('validateRules', () => { + test('requires at least one rule', () => { + expect(validateRules([]).ok).toBe(false) + }) + + test('rejects > 20 rules', () => { + const many = Array.from({ length: 21 }, () => ({ + interval: 1, unit: 'days' as const, start_at: '2030-01-01T00:00:00Z', + })) + expect(validateRules(many).ok).toBe(false) + }) + + test('rejects non-array', () => { + expect(validateRules('nope' as any).ok).toBe(false) + }) + + test('reports the failing rule index', () => { + const r = validateRules([ + { interval: 1, unit: 'days', start_at: '2030-01-01T00:00:00Z' }, + { interval: -1, unit: 'days', start_at: '2030-01-01T00:00:00Z' } as any, + ]) + expect(r.ok).toBe(false) + if (!r.ok) expect(r.error).toContain('rule[1]') + }) +}) + +describe('ruleToCron', () => { + test('hourly interval=1 emits minute-of-hour cron', () => { + const r: ScheduleRule = { interval: 1, unit: 'hours', start_at: '2030-01-01T09:30:00.000Z' } + expect(ruleToCron(r, 'UTC')).toBe('30 * * * *') + }) + + test('hourly interval=N emits stepped cron', () => { + const r: ScheduleRule = { interval: 4, unit: 'hours', start_at: '2030-01-01T09:30:00.000Z' } + expect(ruleToCron(r, 'UTC')).toBe('30 */4 * * *') + }) + + test('daily interval=1 emits hh:mm cron', () => { + const r: ScheduleRule = { interval: 1, unit: 'days', start_at: '2030-01-01T14:05:00.000Z' } + expect(ruleToCron(r, 'UTC')).toBe('5 14 * * *') + }) + + test('daily interval=N emits stepped DOM', () => { + const r: ScheduleRule = { interval: 3, unit: 'days', start_at: '2030-01-01T14:05:00.000Z' } + expect(ruleToCron(r, 'UTC')).toBe('5 14 */3 * *') + }) + + test('weekly emits DOW cron', () => { + // 2030-01-01 is a Tuesday → DOW=2 + const r: ScheduleRule = { interval: 1, unit: 'weeks', start_at: '2030-01-01T14:05:00.000Z' } + expect(ruleToCron(r, 'UTC')).toBe('5 14 * * 2') + }) +}) + +describe('shouldSkipFire', () => { + test('skips when now < start_at', () => { + const r: ScheduleRule = { interval: 1, unit: 'hours', start_at: '2099-01-01T00:00:00Z' } + expect(shouldSkipFire(r, new Date('2030-01-01T00:00:00Z'))).toBe(true) + }) + + test('does not skip when now >= start_at', () => { + const r: ScheduleRule = { interval: 1, unit: 'hours', start_at: '2030-01-01T00:00:00Z' } + expect(shouldSkipFire(r, new Date('2030-01-02T00:00:00Z'))).toBe(false) + }) + + test('weekly interval>1 enforces cadence', () => { + const r: ScheduleRule = { interval: 2, unit: 'weeks', start_at: '2030-01-07T00:00:00Z' } // Mon + // 7 days later = 1 week → wrong week + expect(shouldSkipFire(r, new Date('2030-01-14T00:00:00Z'))).toBe(true) + // 14 days later = 2 weeks → right week + expect(shouldSkipFire(r, new Date('2030-01-21T00:00:00Z'))).toBe(false) + }) +}) + +describe('humanizeRule', () => { + test('singular noun for interval=1', () => { + const r: ScheduleRule = { interval: 1, unit: 'days', start_at: '2030-01-01T09:00:00Z' } + expect(humanizeRule(r)).toMatch(/^Every 1 day starting /) + }) + + test('plural for interval>1', () => { + const r: ScheduleRule = { interval: 3, unit: 'weeks', start_at: '2030-01-01T09:00:00Z' } + expect(humanizeRule(r)).toMatch(/^Every 3 weeks starting /) + }) +}) diff --git a/web/src/components/ScheduleEditor.tsx b/web/src/components/ScheduleEditor.tsx index 2e21499..ff9de64 100644 --- a/web/src/components/ScheduleEditor.tsx +++ b/web/src/components/ScheduleEditor.tsx @@ -5,7 +5,8 @@ import { useSessions } from '../hooks/useSessions' import { hubFetch, HubFetchError } from '../lib/api' import { nextRuns, validate as validateCron, browserTimezone } from '../lib/cron' import { PostRunActionsEditor } from './PostRunActionsEditor' -import { CronBuilder } from './CronBuilder' +import { ScheduleRulesBuilder } from './ScheduleRulesBuilder' +import { type ScheduleRule, ruleToCron, defaultRule, validateRule } from '../lib/schedule-rules' import { computeTaskAutoName } from '../lib/task-name' import { TASK_TEMPLATES, isReplaceableNotes } from '../lib/task-templates' @@ -80,8 +81,14 @@ export function ScheduleEditor({ token, existing, allSchedules, onClose, onSave setNotes(prev => isReplaceableNotes(prev) ? (TASK_TEMPLATES[taskType] ?? '') : prev) }, [taskType, existing]) - // Schedule — composed by ; the cron string is the source of truth. - const [cronExpr, setCronExpr] = useState(existing?.cron_expr ?? '0 9 * * *') + // Schedule — composed by . Hydrate from + // `schedule_rules` when present; otherwise start with one default rule. + // The legacy cron string is derived from rule[0] on submit. + const [scheduleRules, setScheduleRules] = useState(() => { + const r = existing?.schedule_rules + if (Array.isArray(r) && r.length > 0) return r as ScheduleRule[] + return [defaultRule()] + }) // Timezone const browserTz = browserTimezone() @@ -107,9 +114,21 @@ export function ScheduleEditor({ token, existing, allSchedules, onClose, onSave const [error, setError] = useState(null) const [cycleError, setCycleError] = useState<{ path: string[] } | null>(null) - const cronValidation = useMemo(() => validateCron(cronExpr), [cronExpr]) + // Derive cron from rule[0] for the auto-name + legacy back-compat. + const cronExpr = useMemo(() => { + if (!scheduleRules[0]) return '' + try { return ruleToCron(scheduleRules[0], tz) } catch { return '' } + }, [scheduleRules, tz]) + const rulesValid = useMemo( + () => scheduleRules.length > 0 && scheduleRules.every(r => validateRule(r) === null), + [scheduleRules], + ) + const cronValidation = useMemo(() => { + if (!rulesValid) return { ok: false, error: 'Pick a valid date/time for each rule' } as const + return validateCron(cronExpr) + }, [cronExpr, rulesValid]) - // Sub-15 warning for non-prompt (best-effort: derived from next 2 runs) + // Sub-15 warning for non-prompt (best-effort: derived from next 2 runs of rule[0]) const intervalMinutes = useMemo(() => { if (!cronValidation.ok) return null const r = nextRuns(cronExpr, tz, 2) @@ -226,7 +245,9 @@ export function ScheduleEditor({ token, existing, allSchedules, onClose, onSave target_kind: targetKind, target_id: targetKind === 'session' || targetKind === 'supervisor' ? targetId : null, payload, - cron_expr: cronExpr, + // New shape: send the structured rules. Server derives cron_expr from + // rule[0] and persists both for back-compat with the croner engine. + schedule_rules: scheduleRules, timezone: tz, catchup_policy: catchup, max_concurrent: maxConcurrent, @@ -328,7 +349,7 @@ export function ScheduleEditor({ token, existing, allSchedules, onClose, onSave {/* Schedule */} - + {/* Timezone */} diff --git a/web/src/components/ScheduleRulesBuilder.tsx b/web/src/components/ScheduleRulesBuilder.tsx new file mode 100644 index 0000000..5691f14 --- /dev/null +++ b/web/src/components/ScheduleRulesBuilder.tsx @@ -0,0 +1,203 @@ +import { useMemo } from 'react' +import { + type ScheduleRule, + type ScheduleUnit, + defaultRule, + ruleToCron, + humanizeRule, + validateRule, +} from '../lib/schedule-rules' +import { nextRuns } from '../lib/cron' + +/** + * ScheduleRulesBuilder — composes a list of structured schedule rules. + * + * UX: one row per rule — `Every [N] [hours|days|weeks] starting on [date] at [time]`. + * Plus-button appends another rule. Multiple rules per task = fires on any. + * + * Never exposes raw cron strings to the user. The cron string is derived + * from rule[0] for back-compat with the legacy croner engine; multi-rule + * dispatching is handled hub-side by registering one Cron per rule. + */ + +interface Props { + rules: ScheduleRule[] + timezone: string + onChange: (rules: ScheduleRule[]) => void +} + +const UNIT_OPTIONS: Array<{ value: ScheduleUnit; label: string }> = [ + { value: 'hours', label: 'hours' }, + { value: 'days', label: 'days' }, + { value: 'weeks', label: 'weeks' }, +] + +export function ScheduleRulesBuilder({ rules, timezone, onChange }: Props) { + const effectiveRules = rules.length > 0 ? rules : [defaultRule()] + + const next3 = useMemo(() => { + const all: number[] = [] + for (const r of effectiveRules) { + if (validateRule(r) !== null) continue + try { + const expr = ruleToCron(r, timezone) + const startMs = Date.parse(r.start_at) + const from = Number.isFinite(startMs) && startMs > Date.now() ? new Date(startMs) : new Date() + for (const d of nextRuns(expr, timezone, 3, from)) all.push(d.getTime()) + } catch {} + } + all.sort((a, b) => a - b) + return Array.from(new Set(all)).slice(0, 3).map(ms => new Date(ms)) + }, [effectiveRules, timezone]) + + const updateRule = (idx: number, patch: Partial) => { + onChange(effectiveRules.map((r, i) => (i === idx ? { ...r, ...patch } : r))) + } + const removeRule = (idx: number) => { + if (effectiveRules.length === 1) return + onChange(effectiveRules.filter((_, i) => i !== idx)) + } + const addRule = () => { + onChange([...effectiveRules, defaultRule()]) + } + + return ( +
+
+ {effectiveRules.map((rule, idx) => ( + 1} + onChange={(patch) => updateRule(idx, patch)} + onRemove={() => removeRule(idx)} + /> + ))} +
+ + + + {effectiveRules.length > 1 && ( +

+ Task fires on any rule. +

+ )} + + {next3.length > 0 && ( +
+
+ Next 3 fires +
+
    + {next3.map((d, i) => ( +
  • + {d.toLocaleString(undefined, { timeZone: timezone, hour12: false })} +
    • + ))} +
+
+ )} +
+ ) +} + +interface RowProps { + rule: ScheduleRule + canRemove: boolean + onChange: (patch: Partial) => void + onRemove: () => void +} + +export function ScheduleRuleRow({ rule, canRemove, onChange, onRemove }: RowProps) { + // Split start_at into the two native inputs (date + time, in browser tz). + const { dateStr, timeStr } = splitLocal(rule.start_at) + const err = validateRule(rule) + + const setDate = (next: string) => { + onChange({ start_at: combineLocal(next, timeStr) }) + } + const setTime = (next: string) => { + onChange({ start_at: combineLocal(dateStr, next) }) + } + + return ( +
+ Every + { + const n = parseInt(e.target.value || '1', 10) + onChange({ interval: Number.isFinite(n) && n >= 1 ? Math.min(n, 999) : 1 }) + }} + className="w-16 px-2 py-1.5 bg-[var(--bg-tertiary)] rounded-lg text-sm text-[var(--text-primary)] focus:outline-none focus:ring-2 focus:ring-indigo-500 font-mono" + /> + + starting on + setDate(e.target.value)} + className="px-2 py-1.5 bg-[var(--bg-tertiary)] rounded-lg text-sm text-[var(--text-primary)] focus:outline-none focus:ring-2 focus:ring-indigo-500" + /> + at + setTime(e.target.value)} + className="px-2 py-1.5 bg-[var(--bg-tertiary)] rounded-lg text-sm text-[var(--text-primary)] focus:outline-none focus:ring-2 focus:ring-indigo-500" + /> + {canRemove && ( + + )} + {err && ( +

{err}

+ )} +

+ {humanizeRule(rule)} +

+
+ ) +} + +function splitLocal(iso: string): { dateStr: string; timeStr: string } { + const d = new Date(iso) + if (isNaN(d.getTime())) return { dateStr: '', timeStr: '' } + const pad = (n: number) => String(n).padStart(2, '0') + return { + dateStr: `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}`, + timeStr: `${pad(d.getHours())}:${pad(d.getMinutes())}`, + } +} + +function combineLocal(dateStr: string, timeStr: string): string { + if (!dateStr) return new Date().toISOString() + const [hh, mm] = (timeStr || '00:00').split(':').map(n => parseInt(n, 10)) + const [y, mo, d] = dateStr.split('-').map(n => parseInt(n, 10)) + const dt = new Date(y, (mo || 1) - 1, d || 1, hh || 0, mm || 0, 0, 0) + return dt.toISOString() +} diff --git a/web/src/hooks/useSchedules.ts b/web/src/hooks/useSchedules.ts index 77e49f8..974e691 100644 --- a/web/src/hooks/useSchedules.ts +++ b/web/src/hooks/useSchedules.ts @@ -32,6 +32,7 @@ export interface ScheduledTask { target_id: string | null payload: Record cron_expr: string + schedule_rules?: Array<{ interval: number; unit: 'hours'|'days'|'weeks'; start_at: string }> | null timezone: string catchup_policy: CatchupPolicy max_concurrent: number @@ -58,7 +59,13 @@ export interface ScheduleCreateInput { target_kind: TargetKind target_id?: string | null payload?: Record - cron_expr: string + /** + * EITHER `cron_expr` (legacy) OR `schedule_rules` (new shape). Server + * derives the missing side: when `schedule_rules` is sent, `cron_expr` + * is built from `rules[0]` and stored alongside for the croner engine. + */ + cron_expr?: string + schedule_rules?: Array<{ interval: number; unit: 'hours'|'days'|'weeks'; start_at: string }> timezone: string catchup_policy?: CatchupPolicy max_concurrent?: number diff --git a/web/src/lib/schedule-rules.ts b/web/src/lib/schedule-rules.ts new file mode 100644 index 0000000..e3cfa53 --- /dev/null +++ b/web/src/lib/schedule-rules.ts @@ -0,0 +1,86 @@ +/** + * Browser mirror of `hub/src/scheduler/schedule-rules.ts`. Kept in sync so + * the editor can compute the "next 3 fires" preview locally. + */ + +export type ScheduleUnit = 'hours' | 'days' | 'weeks' + +export interface ScheduleRule { + interval: number + unit: ScheduleUnit + start_at: string +} + +export function validateRule(rule: Partial): string | null { + if (typeof rule.interval !== 'number' || !Number.isInteger(rule.interval) || rule.interval < 1 || rule.interval > 999) { + return 'Interval must be 1..999' + } + if (rule.unit !== 'hours' && rule.unit !== 'days' && rule.unit !== 'weeks') { + return 'Unit must be hours, days, or weeks' + } + if (typeof rule.start_at !== 'string' || Number.isNaN(Date.parse(rule.start_at))) { + return 'Pick a valid start date/time' + } + return null +} + +export function ruleToCron(rule: ScheduleRule, tz: string = 'UTC'): string { + const d = new Date(rule.start_at) + const parts = extractWallClock(d, tz) + const { mm, hh, dow } = parts + switch (rule.unit) { + case 'hours': + return rule.interval === 1 ? `${mm} * * * *` : `${mm} */${rule.interval} * * *` + case 'days': + return rule.interval === 1 ? `${mm} ${hh} * * *` : `${mm} ${hh} */${rule.interval} * *` + case 'weeks': + return `${mm} ${hh} * * ${dow}` + } +} + +function extractWallClock(d: Date, tz: string): { mm: number; hh: number; dow: number } { + try { + const fmt = new Intl.DateTimeFormat('en-US', { + timeZone: tz, hour: '2-digit', minute: '2-digit', weekday: 'short', hour12: false, + }) + const parts = fmt.formatToParts(d) + const get = (t: string) => parts.find(p => p.type === t)?.value ?? '' + let hh = parseInt(get('hour'), 10) + if (hh === 24) hh = 0 + const mm = parseInt(get('minute'), 10) + const dow = ['Sun','Mon','Tue','Wed','Thu','Fri','Sat'].indexOf(get('weekday')) + return { mm: Number.isFinite(mm) ? mm : 0, hh: Number.isFinite(hh) ? hh : 0, dow: dow >= 0 ? dow : 0 } + } catch { + return { mm: 0, hh: 0, dow: 0 } + } +} + +export function shouldSkipFire(rule: ScheduleRule, now: Date = new Date()): boolean { + const start = new Date(rule.start_at) + if (now.getTime() < start.getTime()) return true + if (rule.unit === 'weeks' && rule.interval > 1) { + const weekMs = 7 * 24 * 60 * 60 * 1000 + const weeksSince = Math.floor((now.getTime() - start.getTime()) / weekMs) + if (weeksSince % rule.interval !== 0) return true + } + return false +} + +export function humanizeRule(rule: ScheduleRule): string { + const unitWord = rule.interval === 1 + ? (rule.unit === 'hours' ? 'hour' : rule.unit === 'days' ? 'day' : 'week') + : rule.unit + const start = new Date(rule.start_at) + const dateStr = isNaN(start.getTime()) + ? '—' + : start.toLocaleString(undefined, { dateStyle: 'medium', timeStyle: 'short' }) + return `Every ${rule.interval} ${unitWord} starting ${dateStr}` +} + +/** Default rule when adding a new row in the builder. */ +export function defaultRule(): ScheduleRule { + // Round forward to next quarter hour + const d = new Date() + d.setMinutes(Math.ceil((d.getMinutes() + 1) / 15) * 15, 0, 0) + return { interval: 1, unit: 'days', start_at: d.toISOString() } +}