Skip to content

Commit 3a03e38

Browse files
tyler6204steipete
authored andcommitted
fix(cron): fix timeout, add timestamp validation, enable file sync
Fixes openclaw#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
1 parent a749db9 commit 3a03e38

File tree

7 files changed

+128
-13
lines changed

7 files changed

+128
-13
lines changed

src/agents/tools/cron-tool.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ Use jobId as the canonical identifier; id is accepted for compatibility. Use con
208208
const gatewayOpts: GatewayCallOptions = {
209209
gatewayUrl: readStringParam(params, "gatewayUrl", { trim: false }),
210210
gatewayToken: readStringParam(params, "gatewayToken", { trim: false }),
211-
timeoutMs: typeof params.timeoutMs === "number" ? params.timeoutMs : undefined,
211+
timeoutMs: typeof params.timeoutMs === "number" ? params.timeoutMs : 60_000,
212212
};
213213

214214
switch (action) {

src/agents/tools/gateway.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export function resolveGatewayOptions(opts?: GatewayCallOptions) {
2222
const timeoutMs =
2323
typeof opts?.timeoutMs === "number" && Number.isFinite(opts.timeoutMs)
2424
? Math.max(1, Math.floor(opts.timeoutMs))
25-
: 10_000;
25+
: 30_000;
2626
return { url, token, timeoutMs };
2727
}
2828

src/cli/gateway-rpc.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export function addGatewayClientOptions(cmd: Command) {
1515
return cmd
1616
.option("--url <url>", "Gateway WebSocket URL (defaults to gateway.remote.url when configured)")
1717
.option("--token <token>", "Gateway token (if required)")
18-
.option("--timeout <ms>", "Timeout in ms", "10000")
18+
.option("--timeout <ms>", "Timeout in ms", "30000")
1919
.option("--expect-final", "Wait for final response (agent)", false);
2020
}
2121

src/cron/service/state.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ export type CronServiceState = {
4848
running: boolean;
4949
op: Promise<unknown>;
5050
warnedDisabled: boolean;
51+
storeLoadedAtMs: number | null;
52+
storeFileMtimeMs: number | null;
5153
};
5254

5355
export function createCronServiceState(deps: CronServiceDeps): CronServiceState {
@@ -58,6 +60,8 @@ export function createCronServiceState(deps: CronServiceDeps): CronServiceState
5860
running: false,
5961
op: Promise.resolve(),
6062
warnedDisabled: false,
63+
storeLoadedAtMs: null,
64+
storeFileMtimeMs: null,
6165
};
6266
}
6367

src/cron/service/store.ts

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,37 @@
1+
import fs from "node:fs";
12
import type { CronJob } from "../types.js";
23
import type { CronServiceState } from "./state.js";
34
import { migrateLegacyCronPayload } from "../payload-migration.js";
45
import { loadCronStore, saveCronStore } from "../store.js";
6+
import { recomputeNextRuns } from "./jobs.js";
57
import { inferLegacyName, normalizeOptionalText } from "./normalize.js";
68

7-
const storeCache = new Map<string, { version: 1; jobs: CronJob[] }>();
9+
async function getFileMtimeMs(path: string): Promise<number | null> {
10+
try {
11+
const stats = await fs.promises.stat(path);
12+
return stats.mtimeMs;
13+
} catch {
14+
return null;
15+
}
16+
}
817

918
export async function ensureLoaded(state: CronServiceState) {
10-
if (state.store) {
11-
return;
12-
}
13-
const cached = storeCache.get(state.deps.storePath);
14-
if (cached) {
15-
state.store = cached;
19+
const fileMtimeMs = await getFileMtimeMs(state.deps.storePath);
20+
21+
// Check if we need to reload:
22+
// - No store loaded yet
23+
// - File modification time has changed
24+
// - File was modified after we last loaded (external edit)
25+
const needsReload =
26+
!state.store ||
27+
(fileMtimeMs !== null &&
28+
state.storeFileMtimeMs !== null &&
29+
fileMtimeMs > state.storeFileMtimeMs);
30+
31+
if (!needsReload) {
1632
return;
1733
}
34+
1835
const loaded = await loadCronStore(state.deps.storePath);
1936
const jobs = (loaded.jobs ?? []) as unknown as Array<Record<string, unknown>>;
2037
let mutated = false;
@@ -44,7 +61,12 @@ export async function ensureLoaded(state: CronServiceState) {
4461
}
4562
}
4663
state.store = { version: 1, jobs: jobs as unknown as CronJob[] };
47-
storeCache.set(state.deps.storePath, state.store);
64+
state.storeLoadedAtMs = state.deps.nowMs();
65+
state.storeFileMtimeMs = fileMtimeMs;
66+
67+
// Recompute next runs after loading to ensure accuracy
68+
recomputeNextRuns(state);
69+
4870
if (mutated) {
4971
await persist(state);
5072
}
@@ -69,4 +91,6 @@ export async function persist(state: CronServiceState) {
6991
return;
7092
}
7193
await saveCronStore(state.deps.storePath, state.store);
94+
// Update file mtime after save to prevent immediate reload
95+
state.storeFileMtimeMs = await getFileMtimeMs(state.deps.storePath);
7296
}

src/cron/validate-timestamp.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import type { CronSchedule } from "./types.js";
2+
3+
const ONE_MINUTE_MS = 60 * 1000;
4+
const TEN_YEARS_MS = 10 * 365.25 * 24 * 60 * 60 * 1000;
5+
6+
export type TimestampValidationError = {
7+
ok: false;
8+
message: string;
9+
};
10+
11+
export type TimestampValidationSuccess = {
12+
ok: true;
13+
};
14+
15+
export type TimestampValidationResult = TimestampValidationSuccess | TimestampValidationError;
16+
17+
/**
18+
* Validates atMs timestamps in cron schedules.
19+
* Rejects timestamps that are:
20+
* - More than 1 minute in the past
21+
* - More than 10 years in the future
22+
*/
23+
export function validateScheduleTimestamp(
24+
schedule: CronSchedule,
25+
nowMs: number = Date.now(),
26+
): TimestampValidationResult {
27+
if (schedule.kind !== "at") {
28+
return { ok: true };
29+
}
30+
31+
const atMs = schedule.atMs;
32+
33+
if (typeof atMs !== "number" || !Number.isFinite(atMs)) {
34+
return {
35+
ok: false,
36+
message: `Invalid atMs: must be a finite number (got ${String(atMs)})`,
37+
};
38+
}
39+
40+
const diffMs = atMs - nowMs;
41+
42+
// Check if timestamp is in the past (allow 1 minute grace period)
43+
if (diffMs < -ONE_MINUTE_MS) {
44+
const nowDate = new Date(nowMs).toISOString();
45+
const atDate = new Date(atMs).toISOString();
46+
const minutesAgo = Math.floor(-diffMs / ONE_MINUTE_MS);
47+
return {
48+
ok: false,
49+
message: `atMs is in the past: ${atDate} (${minutesAgo} minutes ago). Current time: ${nowDate}`,
50+
};
51+
}
52+
53+
// Check if timestamp is too far in the future
54+
if (diffMs > TEN_YEARS_MS) {
55+
const atDate = new Date(atMs).toISOString();
56+
const yearsAhead = Math.floor(diffMs / (365.25 * 24 * 60 * 60 * 1000));
57+
return {
58+
ok: false,
59+
message: `atMs is too far in the future: ${atDate} (${yearsAhead} years ahead). Maximum allowed: 10 years`,
60+
};
61+
}
62+
63+
return { ok: true };
64+
}

src/gateway/server-methods/cron.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { CronJobCreate, CronJobPatch } from "../../cron/types.js";
22
import type { GatewayRequestHandlers } from "./types.js";
33
import { normalizeCronJobCreate, normalizeCronJobPatch } from "../../cron/normalize.js";
44
import { readCronRunLogEntries, resolveCronRunLogPath } from "../../cron/run-log.js";
5+
import { validateScheduleTimestamp } from "../../cron/validate-timestamp.js";
56
import {
67
ErrorCodes,
78
errorShape,
@@ -82,7 +83,17 @@ export const cronHandlers: GatewayRequestHandlers = {
8283
);
8384
return;
8485
}
85-
const job = await context.cron.add(normalized as unknown as CronJobCreate);
86+
const jobCreate = normalized as unknown as CronJobCreate;
87+
const timestampValidation = validateScheduleTimestamp(jobCreate.schedule);
88+
if (!timestampValidation.ok) {
89+
respond(
90+
false,
91+
undefined,
92+
errorShape(ErrorCodes.INVALID_REQUEST, timestampValidation.message),
93+
);
94+
return;
95+
}
96+
const job = await context.cron.add(jobCreate);
8697
respond(true, job, undefined);
8798
},
8899
"cron.update": async ({ params, respond, context }) => {
@@ -116,7 +127,19 @@ export const cronHandlers: GatewayRequestHandlers = {
116127
);
117128
return;
118129
}
119-
const job = await context.cron.update(jobId, p.patch as unknown as CronJobPatch);
130+
const patch = p.patch as unknown as CronJobPatch;
131+
if (patch.schedule) {
132+
const timestampValidation = validateScheduleTimestamp(patch.schedule);
133+
if (!timestampValidation.ok) {
134+
respond(
135+
false,
136+
undefined,
137+
errorShape(ErrorCodes.INVALID_REQUEST, timestampValidation.message),
138+
);
139+
return;
140+
}
141+
}
142+
const job = await context.cron.update(jobId, patch);
120143
respond(true, job, undefined);
121144
},
122145
"cron.remove": async ({ params, respond, context }) => {

0 commit comments

Comments
 (0)