Skip to content

Commit 2896107

Browse files
vincentkocvincentkoc
authored
fix(cron): tolerate malformed legacy jobs
1 parent a7604f8 commit 2896107

7 files changed

Lines changed: 103 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai
1717
### Fixes
1818

1919
- Google Chat: preserve reply text when a typing indicator message is deleted or can no longer be updated, so media captions and first text chunks are resent instead of silently disappearing. (#71498) Thanks @colin-lgtm.
20+
- Cron: tolerate malformed legacy job rows in startup, main-session system-event payloads, and human-readable `cron list` output so missing `state`, `payload.text`, or display fields no longer crash the scheduler or CLI. Fixes #66016, #65916, #64137, #57872, #59968, #63813, #52804, and #43163.
2021
- Heartbeat: clamp oversized scheduler delays through the shared safe timer helper, preventing `every` values over Node's timeout cap from becoming a 1 ms crash loop. Fixes #71414. (#71478) Thanks @hclsys.
2122
- Control UI/chat: collapse assistant token/model context details behind an explicit Context disclosure and show full dates in message footers, making historical transcript timing clear without noisy default metadata. (#71337) Thanks @BunsDev.
2223
- OpenAI/Codex OAuth: explain `unsupported_country_region_territory` token-exchange failures with a proxy/region hint instead of surfacing a generic OAuth error. Fixes #51175.

‎src/cli/cron-cli/shared.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,22 @@ describe("printCronList", () => {
7878
expect(logs.some((line) => line.includes("isolated"))).toBe(true);
7979
});
8080

81+
it("tolerates malformed rows in human-readable output", () => {
82+
const { logs, runtime } = createRuntimeLogCapture();
83+
const malformedJob = {
84+
id: "malformed-job",
85+
name: undefined,
86+
enabled: true,
87+
sessionTarget: undefined,
88+
payload: undefined,
89+
schedule: undefined,
90+
state: undefined,
91+
} as unknown as CronJob;
92+
93+
expect(() => printCronList([malformedJob], runtime)).not.toThrow();
94+
expect(logs.some((line) => line.includes("malformed-job"))).toBe(true);
95+
});
96+
8197
it("shows stagger label for cron schedules", () => {
8298
const { logs, runtime } = createRuntimeLogCapture();
8399
const job = createBaseJob({

‎src/cli/cron-cli/shared.ts‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,17 @@ const CRON_DELIVERY_PAD = 64;
156156
const CRON_AGENT_PAD = 10;
157157
const CRON_MODEL_PAD = 20;
158158

159-
const pad = (value: string, width: number) => value.padEnd(width);
159+
const stringifyCell = (value: unknown, fallback = "-") => {
160+
if (typeof value === "string") {
161+
return value;
162+
}
163+
if (typeof value === "number" || typeof value === "boolean") {
164+
return String(value);
165+
}
166+
return fallback;
167+
};
168+
169+
const pad = (value: unknown, width: number) => stringifyCell(value).padEnd(width);
160170

161171
const truncate = (value: string, width: number) => {
162172
if (value.length <= width) {
@@ -200,13 +210,16 @@ const formatRelative = (ms: number | null | undefined, nowMs: number) => {
200210
return delta >= 0 ? `in ${label}` : `${label} ago`;
201211
};
202212

203-
const formatSchedule = (schedule: CronSchedule) => {
204-
if (schedule.kind === "at") {
213+
const formatSchedule = (schedule: CronSchedule | undefined) => {
214+
if (schedule?.kind === "at") {
205215
return `at ${formatIsoMinute(schedule.at)}`;
206216
}
207-
if (schedule.kind === "every") {
217+
if (schedule?.kind === "every") {
208218
return `every ${formatDurationHuman(schedule.everyMs)}`;
209219
}
220+
if (schedule?.kind !== "cron") {
221+
return "-";
222+
}
210223
const base = schedule.tz ? `cron ${schedule.expr} @ ${schedule.tz}` : `cron ${schedule.expr}`;
211224
const staggerMs = resolveCronStaggerMs(schedule);
212225
if (staggerMs <= 0) {
@@ -219,10 +232,11 @@ const formatStatus = (job: CronJob) => {
219232
if (!job.enabled) {
220233
return "disabled";
221234
}
222-
if (job.state.runningAtMs) {
235+
const state = job.state ?? {};
236+
if (state.runningAtMs) {
223237
return "running";
224238
}
225-
return job.state.lastStatus ?? "idle";
239+
return state.lastStatus ?? "idle";
226240
};
227241

228242
export function coerceCronDeliveryPreviews(value: unknown): Map<string, CronDeliveryPreview> {
@@ -275,17 +289,18 @@ export function printCronList(
275289
const now = Date.now();
276290

277291
for (const job of jobs) {
292+
const state = job.state ?? {};
278293
const idLabel = pad(job.id, CRON_ID_PAD);
279-
const nameLabel = pad(truncate(job.name, CRON_NAME_PAD), CRON_NAME_PAD);
294+
const nameLabel = pad(truncate(stringifyCell(job.name), CRON_NAME_PAD), CRON_NAME_PAD);
280295
const scheduleLabel = pad(
281296
truncate(formatSchedule(job.schedule), CRON_SCHEDULE_PAD),
282297
CRON_SCHEDULE_PAD,
283298
);
284299
const nextLabel = pad(
285-
job.enabled ? formatRelative(job.state.nextRunAtMs, now) : "-",
300+
job.enabled ? formatRelative(state.nextRunAtMs, now) : "-",
286301
CRON_NEXT_PAD,
287302
);
288-
const lastLabel = pad(formatRelative(job.state.lastRunAtMs, now), CRON_LAST_PAD);
303+
const lastLabel = pad(formatRelative(state.lastRunAtMs, now), CRON_LAST_PAD);
289304
const statusRaw = formatStatus(job);
290305
const statusLabel = pad(statusRaw, CRON_STATUS_PAD);
291306
const targetLabel = pad(job.sessionTarget ?? "-", CRON_TARGET_PAD);
@@ -297,7 +312,7 @@ export function printCronList(
297312
const agentLabel = pad(truncate(job.agentId ?? "-", CRON_AGENT_PAD), CRON_AGENT_PAD);
298313
const modelLabel = pad(
299314
truncate(
300-
(job.payload.kind === "agentTurn" ? job.payload.model : undefined) ?? "-",
315+
(job.payload?.kind === "agentTurn" ? job.payload.model : undefined) ?? "-",
301316
CRON_MODEL_PAD,
302317
),
303318
CRON_MODEL_PAD,
@@ -339,7 +354,7 @@ export function printCronList(
339354
? colorize(rich, theme.info, deliveryLabel)
340355
: colorize(rich, theme.muted, deliveryLabel),
341356
coloredAgent,
342-
job.payload.kind === "agentTurn" && job.payload.model
357+
job.payload?.kind === "agentTurn" && job.payload.model
343358
? colorize(rich, theme.info, modelLabel)
344359
: colorize(rich, theme.muted, modelLabel),
345360
].join(" ");

‎src/cron/service.jobs.test.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { describe, expect, it } from "vitest";
2-
import { applyJobPatch, createJob, recomputeNextRuns } from "./service/jobs.js";
2+
import {
3+
applyJobPatch,
4+
createJob,
5+
recomputeNextRuns,
6+
resolveJobPayloadTextForMain,
7+
} from "./service/jobs.js";
38
import type { CronServiceState } from "./service/state.js";
49
import { DEFAULT_TOP_OF_HOUR_STAGGER_MS } from "./stagger.js";
510
import type { CronJob, CronJobPatch } from "./types.js";
@@ -683,6 +688,20 @@ describe("createJob delivery defaults", () => {
683688
});
684689
expect(job.delivery).toBeUndefined();
685690
});
691+
692+
it("uses legacy systemEvent message text without throwing", () => {
693+
const state = createMockState(now, { defaultAgentId: "main" });
694+
const job = createJob(state, {
695+
name: "legacy system event",
696+
enabled: true,
697+
schedule: { kind: "every", everyMs: 60_000 },
698+
sessionTarget: "main",
699+
wakeMode: "now",
700+
payload: { kind: "systemEvent", message: "legacy text" } as never,
701+
});
702+
703+
expect(resolveJobPayloadTextForMain(job)).toBe("legacy text");
704+
});
686705
});
687706

688707
describe("recomputeNextRuns", () => {

‎src/cron/service/normalize.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,12 @@ export function inferLegacyName(job: {
6363

6464
export function normalizePayloadToSystemText(payload: CronPayload) {
6565
if (payload.kind === "systemEvent") {
66-
return payload.text.trim();
66+
const text = (payload as { text?: unknown }).text;
67+
if (typeof text === "string") {
68+
return text.trim();
69+
}
70+
const legacyMessage = (payload as { message?: unknown }).message;
71+
return typeof legacyMessage === "string" ? legacyMessage.trim() : "";
6772
}
68-
return payload.message.trim();
73+
return typeof payload.message === "string" ? payload.message.trim() : "";
6974
}

‎src/cron/service/ops.regression.test.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
waitForActiveTasks,
1717
} from "../../process/command-queue.js";
1818
import { CommandLane } from "../../process/lanes.js";
19-
import { enqueueRun, run } from "./ops.js";
19+
import { enqueueRun, run, start } from "./ops.js";
2020
import type { CronEvent } from "./state.js";
2121
import { createCronServiceState } from "./state.js";
2222
import { onTimer } from "./timer.js";
@@ -27,6 +27,37 @@ const opsRegressionFixtures = setupCronRegressionFixtures({
2727
});
2828

2929
describe("cron service ops regressions", () => {
30+
it("does not crash startup when a loaded job is missing state", async () => {
31+
const scheduledAt = Date.now() + 60_000;
32+
const store = opsRegressionFixtures.makeStorePath();
33+
const state = createCronServiceState({
34+
cronEnabled: true,
35+
storePath: store.storePath,
36+
log: noopLogger,
37+
enqueueSystemEvent: vi.fn(),
38+
requestHeartbeatNow: vi.fn(),
39+
runIsolatedAgentJob: vi.fn(),
40+
});
41+
state.store = {
42+
version: 1,
43+
jobs: [
44+
{
45+
...createIsolatedRegressionJob({
46+
id: "missing-state-startup",
47+
name: "missing-state-startup",
48+
scheduledAt,
49+
schedule: { kind: "at", at: new Date(scheduledAt).toISOString() },
50+
payload: { kind: "agentTurn", message: "noop" },
51+
}),
52+
state: undefined as never,
53+
},
54+
],
55+
};
56+
57+
await expect(start(state)).resolves.toBeUndefined();
58+
expect(state.store.jobs[0]?.state).toEqual(expect.any(Object));
59+
});
60+
3061
it("skips forced manual runs while a timer-triggered run is in progress", async () => {
3162
const store = opsRegressionFixtures.makeStorePath();
3263
const dueAt = Date.now() - 1;

‎src/cron/service/ops.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ export async function start(state: CronServiceState) {
9696
await ensureLoaded(state, { skipRecompute: true });
9797
const jobs = state.store?.jobs ?? [];
9898
for (const job of jobs) {
99+
job.state ??= {};
99100
if (typeof job.state.runningAtMs === "number") {
100101
state.deps.log.warn(
101102
{ jobId: job.id, runningAtMs: job.state.runningAtMs },

0 commit comments

Comments
 (0)