mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:50:43 +00:00
fix(cron): tolerate malformed legacy jobs
This commit is contained in:
@@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai
|
||||
### Fixes
|
||||
|
||||
- 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.
|
||||
- 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.
|
||||
- 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.
|
||||
- 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.
|
||||
- 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.
|
||||
|
||||
@@ -78,6 +78,22 @@ describe("printCronList", () => {
|
||||
expect(logs.some((line) => line.includes("isolated"))).toBe(true);
|
||||
});
|
||||
|
||||
it("tolerates malformed rows in human-readable output", () => {
|
||||
const { logs, runtime } = createRuntimeLogCapture();
|
||||
const malformedJob = {
|
||||
id: "malformed-job",
|
||||
name: undefined,
|
||||
enabled: true,
|
||||
sessionTarget: undefined,
|
||||
payload: undefined,
|
||||
schedule: undefined,
|
||||
state: undefined,
|
||||
} as unknown as CronJob;
|
||||
|
||||
expect(() => printCronList([malformedJob], runtime)).not.toThrow();
|
||||
expect(logs.some((line) => line.includes("malformed-job"))).toBe(true);
|
||||
});
|
||||
|
||||
it("shows stagger label for cron schedules", () => {
|
||||
const { logs, runtime } = createRuntimeLogCapture();
|
||||
const job = createBaseJob({
|
||||
|
||||
@@ -156,7 +156,17 @@ const CRON_DELIVERY_PAD = 64;
|
||||
const CRON_AGENT_PAD = 10;
|
||||
const CRON_MODEL_PAD = 20;
|
||||
|
||||
const pad = (value: string, width: number) => value.padEnd(width);
|
||||
const stringifyCell = (value: unknown, fallback = "-") => {
|
||||
if (typeof value === "string") {
|
||||
return value;
|
||||
}
|
||||
if (typeof value === "number" || typeof value === "boolean") {
|
||||
return String(value);
|
||||
}
|
||||
return fallback;
|
||||
};
|
||||
|
||||
const pad = (value: unknown, width: number) => stringifyCell(value).padEnd(width);
|
||||
|
||||
const truncate = (value: string, width: number) => {
|
||||
if (value.length <= width) {
|
||||
@@ -200,13 +210,16 @@ const formatRelative = (ms: number | null | undefined, nowMs: number) => {
|
||||
return delta >= 0 ? `in ${label}` : `${label} ago`;
|
||||
};
|
||||
|
||||
const formatSchedule = (schedule: CronSchedule) => {
|
||||
if (schedule.kind === "at") {
|
||||
const formatSchedule = (schedule: CronSchedule | undefined) => {
|
||||
if (schedule?.kind === "at") {
|
||||
return `at ${formatIsoMinute(schedule.at)}`;
|
||||
}
|
||||
if (schedule.kind === "every") {
|
||||
if (schedule?.kind === "every") {
|
||||
return `every ${formatDurationHuman(schedule.everyMs)}`;
|
||||
}
|
||||
if (schedule?.kind !== "cron") {
|
||||
return "-";
|
||||
}
|
||||
const base = schedule.tz ? `cron ${schedule.expr} @ ${schedule.tz}` : `cron ${schedule.expr}`;
|
||||
const staggerMs = resolveCronStaggerMs(schedule);
|
||||
if (staggerMs <= 0) {
|
||||
@@ -219,10 +232,11 @@ const formatStatus = (job: CronJob) => {
|
||||
if (!job.enabled) {
|
||||
return "disabled";
|
||||
}
|
||||
if (job.state.runningAtMs) {
|
||||
const state = job.state ?? {};
|
||||
if (state.runningAtMs) {
|
||||
return "running";
|
||||
}
|
||||
return job.state.lastStatus ?? "idle";
|
||||
return state.lastStatus ?? "idle";
|
||||
};
|
||||
|
||||
export function coerceCronDeliveryPreviews(value: unknown): Map<string, CronDeliveryPreview> {
|
||||
@@ -275,17 +289,18 @@ export function printCronList(
|
||||
const now = Date.now();
|
||||
|
||||
for (const job of jobs) {
|
||||
const state = job.state ?? {};
|
||||
const idLabel = pad(job.id, CRON_ID_PAD);
|
||||
const nameLabel = pad(truncate(job.name, CRON_NAME_PAD), CRON_NAME_PAD);
|
||||
const nameLabel = pad(truncate(stringifyCell(job.name), CRON_NAME_PAD), CRON_NAME_PAD);
|
||||
const scheduleLabel = pad(
|
||||
truncate(formatSchedule(job.schedule), CRON_SCHEDULE_PAD),
|
||||
CRON_SCHEDULE_PAD,
|
||||
);
|
||||
const nextLabel = pad(
|
||||
job.enabled ? formatRelative(job.state.nextRunAtMs, now) : "-",
|
||||
job.enabled ? formatRelative(state.nextRunAtMs, now) : "-",
|
||||
CRON_NEXT_PAD,
|
||||
);
|
||||
const lastLabel = pad(formatRelative(job.state.lastRunAtMs, now), CRON_LAST_PAD);
|
||||
const lastLabel = pad(formatRelative(state.lastRunAtMs, now), CRON_LAST_PAD);
|
||||
const statusRaw = formatStatus(job);
|
||||
const statusLabel = pad(statusRaw, CRON_STATUS_PAD);
|
||||
const targetLabel = pad(job.sessionTarget ?? "-", CRON_TARGET_PAD);
|
||||
@@ -297,7 +312,7 @@ export function printCronList(
|
||||
const agentLabel = pad(truncate(job.agentId ?? "-", CRON_AGENT_PAD), CRON_AGENT_PAD);
|
||||
const modelLabel = pad(
|
||||
truncate(
|
||||
(job.payload.kind === "agentTurn" ? job.payload.model : undefined) ?? "-",
|
||||
(job.payload?.kind === "agentTurn" ? job.payload.model : undefined) ?? "-",
|
||||
CRON_MODEL_PAD,
|
||||
),
|
||||
CRON_MODEL_PAD,
|
||||
@@ -339,7 +354,7 @@ export function printCronList(
|
||||
? colorize(rich, theme.info, deliveryLabel)
|
||||
: colorize(rich, theme.muted, deliveryLabel),
|
||||
coloredAgent,
|
||||
job.payload.kind === "agentTurn" && job.payload.model
|
||||
job.payload?.kind === "agentTurn" && job.payload.model
|
||||
? colorize(rich, theme.info, modelLabel)
|
||||
: colorize(rich, theme.muted, modelLabel),
|
||||
].join(" ");
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { applyJobPatch, createJob, recomputeNextRuns } from "./service/jobs.js";
|
||||
import {
|
||||
applyJobPatch,
|
||||
createJob,
|
||||
recomputeNextRuns,
|
||||
resolveJobPayloadTextForMain,
|
||||
} from "./service/jobs.js";
|
||||
import type { CronServiceState } from "./service/state.js";
|
||||
import { DEFAULT_TOP_OF_HOUR_STAGGER_MS } from "./stagger.js";
|
||||
import type { CronJob, CronJobPatch } from "./types.js";
|
||||
@@ -683,6 +688,20 @@ describe("createJob delivery defaults", () => {
|
||||
});
|
||||
expect(job.delivery).toBeUndefined();
|
||||
});
|
||||
|
||||
it("uses legacy systemEvent message text without throwing", () => {
|
||||
const state = createMockState(now, { defaultAgentId: "main" });
|
||||
const job = createJob(state, {
|
||||
name: "legacy system event",
|
||||
enabled: true,
|
||||
schedule: { kind: "every", everyMs: 60_000 },
|
||||
sessionTarget: "main",
|
||||
wakeMode: "now",
|
||||
payload: { kind: "systemEvent", message: "legacy text" } as never,
|
||||
});
|
||||
|
||||
expect(resolveJobPayloadTextForMain(job)).toBe("legacy text");
|
||||
});
|
||||
});
|
||||
|
||||
describe("recomputeNextRuns", () => {
|
||||
|
||||
@@ -63,7 +63,12 @@ export function inferLegacyName(job: {
|
||||
|
||||
export function normalizePayloadToSystemText(payload: CronPayload) {
|
||||
if (payload.kind === "systemEvent") {
|
||||
return payload.text.trim();
|
||||
const text = (payload as { text?: unknown }).text;
|
||||
if (typeof text === "string") {
|
||||
return text.trim();
|
||||
}
|
||||
const legacyMessage = (payload as { message?: unknown }).message;
|
||||
return typeof legacyMessage === "string" ? legacyMessage.trim() : "";
|
||||
}
|
||||
return payload.message.trim();
|
||||
return typeof payload.message === "string" ? payload.message.trim() : "";
|
||||
}
|
||||
|
||||
@@ -16,7 +16,7 @@ import {
|
||||
waitForActiveTasks,
|
||||
} from "../../process/command-queue.js";
|
||||
import { CommandLane } from "../../process/lanes.js";
|
||||
import { enqueueRun, run } from "./ops.js";
|
||||
import { enqueueRun, run, start } from "./ops.js";
|
||||
import type { CronEvent } from "./state.js";
|
||||
import { createCronServiceState } from "./state.js";
|
||||
import { onTimer } from "./timer.js";
|
||||
@@ -27,6 +27,37 @@ const opsRegressionFixtures = setupCronRegressionFixtures({
|
||||
});
|
||||
|
||||
describe("cron service ops regressions", () => {
|
||||
it("does not crash startup when a loaded job is missing state", async () => {
|
||||
const scheduledAt = Date.now() + 60_000;
|
||||
const store = opsRegressionFixtures.makeStorePath();
|
||||
const state = createCronServiceState({
|
||||
cronEnabled: true,
|
||||
storePath: store.storePath,
|
||||
log: noopLogger,
|
||||
enqueueSystemEvent: vi.fn(),
|
||||
requestHeartbeatNow: vi.fn(),
|
||||
runIsolatedAgentJob: vi.fn(),
|
||||
});
|
||||
state.store = {
|
||||
version: 1,
|
||||
jobs: [
|
||||
{
|
||||
...createIsolatedRegressionJob({
|
||||
id: "missing-state-startup",
|
||||
name: "missing-state-startup",
|
||||
scheduledAt,
|
||||
schedule: { kind: "at", at: new Date(scheduledAt).toISOString() },
|
||||
payload: { kind: "agentTurn", message: "noop" },
|
||||
}),
|
||||
state: undefined as never,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
await expect(start(state)).resolves.toBeUndefined();
|
||||
expect(state.store.jobs[0]?.state).toEqual(expect.any(Object));
|
||||
});
|
||||
|
||||
it("skips forced manual runs while a timer-triggered run is in progress", async () => {
|
||||
const store = opsRegressionFixtures.makeStorePath();
|
||||
const dueAt = Date.now() - 1;
|
||||
|
||||
@@ -96,6 +96,7 @@ export async function start(state: CronServiceState) {
|
||||
await ensureLoaded(state, { skipRecompute: true });
|
||||
const jobs = state.store?.jobs ?? [];
|
||||
for (const job of jobs) {
|
||||
job.state ??= {};
|
||||
if (typeof job.state.runningAtMs === "number") {
|
||||
state.deps.log.warn(
|
||||
{ jobId: job.id, runningAtMs: job.state.runningAtMs },
|
||||
|
||||
Reference in New Issue
Block a user