From 3a03e38378e2bb9ea17a71cdea5b6b30ae750a3c Mon Sep 17 00:00:00 2001 From: Tyler Yust Date: Tue, 3 Feb 2026 06:12:07 -0800 Subject: [PATCH] fix(cron): fix timeout, add timestamp validation, enable file sync Fixes #7667 Task 1: Fix cron operation timeouts - Increase default gateway tool timeout from 10s to 30s - Increase cron-specific tool timeout to 60s - Increase CLI default timeout from 10s to 30s - Prevents timeouts when gateway is busy with long-running jobs Task 2: Add timestamp validation - New validateScheduleTimestamp() function in validate-timestamp.ts - Rejects atMs timestamps more than 1 minute in the past - Rejects atMs timestamps more than 10 years in the future - Applied to both cron.add and cron.update operations - Provides helpful error messages with current time and offset Task 3: Enable file sync for manual edits - Track file modification time (storeFileMtimeMs) in CronServiceState - Check file mtime in ensureLoaded() and reload if changed - Recompute next runs after reload to maintain accuracy - Update mtime after persist() to prevent reload loop - Dashboard now picks up manual edits to ~/.openclaw/cron/jobs.json --- src/agents/tools/cron-tool.ts | 2 +- src/agents/tools/gateway.ts | 2 +- src/cli/gateway-rpc.ts | 2 +- src/cron/service/state.ts | 4 ++ src/cron/service/store.ts | 40 +++++++++++++++---- src/cron/validate-timestamp.ts | 64 ++++++++++++++++++++++++++++++ src/gateway/server-methods/cron.ts | 27 ++++++++++++- 7 files changed, 128 insertions(+), 13 deletions(-) create mode 100644 src/cron/validate-timestamp.ts diff --git a/src/agents/tools/cron-tool.ts b/src/agents/tools/cron-tool.ts index a3f8de89ee..13bbd8fa80 100644 --- a/src/agents/tools/cron-tool.ts +++ b/src/agents/tools/cron-tool.ts @@ -208,7 +208,7 @@ Use jobId as the canonical identifier; id is accepted for compatibility. Use con const gatewayOpts: GatewayCallOptions = { gatewayUrl: readStringParam(params, "gatewayUrl", { trim: false }), gatewayToken: readStringParam(params, "gatewayToken", { trim: false }), - timeoutMs: typeof params.timeoutMs === "number" ? params.timeoutMs : undefined, + timeoutMs: typeof params.timeoutMs === "number" ? params.timeoutMs : 60_000, }; switch (action) { diff --git a/src/agents/tools/gateway.ts b/src/agents/tools/gateway.ts index 03daee16f8..fc15c769d0 100644 --- a/src/agents/tools/gateway.ts +++ b/src/agents/tools/gateway.ts @@ -22,7 +22,7 @@ export function resolveGatewayOptions(opts?: GatewayCallOptions) { const timeoutMs = typeof opts?.timeoutMs === "number" && Number.isFinite(opts.timeoutMs) ? Math.max(1, Math.floor(opts.timeoutMs)) - : 10_000; + : 30_000; return { url, token, timeoutMs }; } diff --git a/src/cli/gateway-rpc.ts b/src/cli/gateway-rpc.ts index 568b1a0e2d..feac3abcd2 100644 --- a/src/cli/gateway-rpc.ts +++ b/src/cli/gateway-rpc.ts @@ -15,7 +15,7 @@ export function addGatewayClientOptions(cmd: Command) { return cmd .option("--url ", "Gateway WebSocket URL (defaults to gateway.remote.url when configured)") .option("--token ", "Gateway token (if required)") - .option("--timeout ", "Timeout in ms", "10000") + .option("--timeout ", "Timeout in ms", "30000") .option("--expect-final", "Wait for final response (agent)", false); } diff --git a/src/cron/service/state.ts b/src/cron/service/state.ts index ab094c20b7..64fd9cc9e0 100644 --- a/src/cron/service/state.ts +++ b/src/cron/service/state.ts @@ -48,6 +48,8 @@ export type CronServiceState = { running: boolean; op: Promise; warnedDisabled: boolean; + storeLoadedAtMs: number | null; + storeFileMtimeMs: number | null; }; export function createCronServiceState(deps: CronServiceDeps): CronServiceState { @@ -58,6 +60,8 @@ export function createCronServiceState(deps: CronServiceDeps): CronServiceState running: false, op: Promise.resolve(), warnedDisabled: false, + storeLoadedAtMs: null, + storeFileMtimeMs: null, }; } diff --git a/src/cron/service/store.ts b/src/cron/service/store.ts index d1d15ad045..cc27ec246d 100644 --- a/src/cron/service/store.ts +++ b/src/cron/service/store.ts @@ -1,20 +1,37 @@ +import fs from "node:fs"; import type { CronJob } from "../types.js"; import type { CronServiceState } from "./state.js"; import { migrateLegacyCronPayload } from "../payload-migration.js"; import { loadCronStore, saveCronStore } from "../store.js"; +import { recomputeNextRuns } from "./jobs.js"; import { inferLegacyName, normalizeOptionalText } from "./normalize.js"; -const storeCache = new Map(); +async function getFileMtimeMs(path: string): Promise { + try { + const stats = await fs.promises.stat(path); + return stats.mtimeMs; + } catch { + return null; + } +} export async function ensureLoaded(state: CronServiceState) { - if (state.store) { - return; - } - const cached = storeCache.get(state.deps.storePath); - if (cached) { - state.store = cached; + const fileMtimeMs = await getFileMtimeMs(state.deps.storePath); + + // Check if we need to reload: + // - No store loaded yet + // - File modification time has changed + // - File was modified after we last loaded (external edit) + const needsReload = + !state.store || + (fileMtimeMs !== null && + state.storeFileMtimeMs !== null && + fileMtimeMs > state.storeFileMtimeMs); + + if (!needsReload) { return; } + const loaded = await loadCronStore(state.deps.storePath); const jobs = (loaded.jobs ?? []) as unknown as Array>; let mutated = false; @@ -44,7 +61,12 @@ export async function ensureLoaded(state: CronServiceState) { } } state.store = { version: 1, jobs: jobs as unknown as CronJob[] }; - storeCache.set(state.deps.storePath, state.store); + state.storeLoadedAtMs = state.deps.nowMs(); + state.storeFileMtimeMs = fileMtimeMs; + + // Recompute next runs after loading to ensure accuracy + recomputeNextRuns(state); + if (mutated) { await persist(state); } @@ -69,4 +91,6 @@ export async function persist(state: CronServiceState) { return; } await saveCronStore(state.deps.storePath, state.store); + // Update file mtime after save to prevent immediate reload + state.storeFileMtimeMs = await getFileMtimeMs(state.deps.storePath); } diff --git a/src/cron/validate-timestamp.ts b/src/cron/validate-timestamp.ts new file mode 100644 index 0000000000..bb9751c4cd --- /dev/null +++ b/src/cron/validate-timestamp.ts @@ -0,0 +1,64 @@ +import type { CronSchedule } from "./types.js"; + +const ONE_MINUTE_MS = 60 * 1000; +const TEN_YEARS_MS = 10 * 365.25 * 24 * 60 * 60 * 1000; + +export type TimestampValidationError = { + ok: false; + message: string; +}; + +export type TimestampValidationSuccess = { + ok: true; +}; + +export type TimestampValidationResult = TimestampValidationSuccess | TimestampValidationError; + +/** + * Validates atMs timestamps in cron schedules. + * Rejects timestamps that are: + * - More than 1 minute in the past + * - More than 10 years in the future + */ +export function validateScheduleTimestamp( + schedule: CronSchedule, + nowMs: number = Date.now(), +): TimestampValidationResult { + if (schedule.kind !== "at") { + return { ok: true }; + } + + const atMs = schedule.atMs; + + if (typeof atMs !== "number" || !Number.isFinite(atMs)) { + return { + ok: false, + message: `Invalid atMs: must be a finite number (got ${String(atMs)})`, + }; + } + + const diffMs = atMs - nowMs; + + // Check if timestamp is in the past (allow 1 minute grace period) + if (diffMs < -ONE_MINUTE_MS) { + const nowDate = new Date(nowMs).toISOString(); + const atDate = new Date(atMs).toISOString(); + const minutesAgo = Math.floor(-diffMs / ONE_MINUTE_MS); + return { + ok: false, + message: `atMs is in the past: ${atDate} (${minutesAgo} minutes ago). Current time: ${nowDate}`, + }; + } + + // Check if timestamp is too far in the future + if (diffMs > TEN_YEARS_MS) { + const atDate = new Date(atMs).toISOString(); + const yearsAhead = Math.floor(diffMs / (365.25 * 24 * 60 * 60 * 1000)); + return { + ok: false, + message: `atMs is too far in the future: ${atDate} (${yearsAhead} years ahead). Maximum allowed: 10 years`, + }; + } + + return { ok: true }; +} diff --git a/src/gateway/server-methods/cron.ts b/src/gateway/server-methods/cron.ts index 82591dd35a..703103860f 100644 --- a/src/gateway/server-methods/cron.ts +++ b/src/gateway/server-methods/cron.ts @@ -2,6 +2,7 @@ import type { CronJobCreate, CronJobPatch } from "../../cron/types.js"; import type { GatewayRequestHandlers } from "./types.js"; import { normalizeCronJobCreate, normalizeCronJobPatch } from "../../cron/normalize.js"; import { readCronRunLogEntries, resolveCronRunLogPath } from "../../cron/run-log.js"; +import { validateScheduleTimestamp } from "../../cron/validate-timestamp.js"; import { ErrorCodes, errorShape, @@ -82,7 +83,17 @@ export const cronHandlers: GatewayRequestHandlers = { ); return; } - const job = await context.cron.add(normalized as unknown as CronJobCreate); + const jobCreate = normalized as unknown as CronJobCreate; + const timestampValidation = validateScheduleTimestamp(jobCreate.schedule); + if (!timestampValidation.ok) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, timestampValidation.message), + ); + return; + } + const job = await context.cron.add(jobCreate); respond(true, job, undefined); }, "cron.update": async ({ params, respond, context }) => { @@ -116,7 +127,19 @@ export const cronHandlers: GatewayRequestHandlers = { ); return; } - const job = await context.cron.update(jobId, p.patch as unknown as CronJobPatch); + const patch = p.patch as unknown as CronJobPatch; + if (patch.schedule) { + const timestampValidation = validateScheduleTimestamp(patch.schedule); + if (!timestampValidation.ok) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, timestampValidation.message), + ); + return; + } + } + const job = await context.cron.update(jobId, patch); respond(true, job, undefined); }, "cron.remove": async ({ params, respond, context }) => {