fix: recompute all cron next-run times after job update (openclaw#15905) thanks @echoVic
Verified: - pnpm check - pnpm vitest src/cron/service.issue-regressions.test.ts src/cron/service.issue-13992-regression.test.ts Co-authored-by: echoVic <16428813+echoVic@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
@@ -41,6 +41,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Security/BlueBubbles: reject ambiguous shared-path webhook routing when multiple webhook targets match the same guid/password.
|
||||
- Security/BlueBubbles: require explicit `mediaLocalRoots` allowlists for local outbound media path reads to prevent local file disclosure. (#16322) Thanks @mbelinky.
|
||||
- Cron/Slack: preserve agent identity (name and icon) when cron jobs deliver outbound messages. (#16242) Thanks @robbyczgw-cla.
|
||||
- Cron: repair missing/corrupt `nextRunAtMs` for the updated job without globally recomputing unrelated due jobs during `cron update`. (#15750)
|
||||
- Discord: prefer gateway guild id when logging inbound messages so cached-miss guilds do not appear as `guild=dm`. Thanks @thewilloftheshadow.
|
||||
- TUI: refactor searchable select list description layout and add regression coverage for ANSI-highlight width bounds.
|
||||
|
||||
|
||||
@@ -152,6 +152,84 @@ describe("Cron issue regressions", () => {
|
||||
await store.cleanup();
|
||||
});
|
||||
|
||||
it("repairs missing nextRunAtMs on non-schedule updates without touching other jobs", async () => {
|
||||
const store = await makeStorePath();
|
||||
const cron = new CronService({
|
||||
cronEnabled: true,
|
||||
storePath: store.storePath,
|
||||
log: noopLogger,
|
||||
enqueueSystemEvent: vi.fn(),
|
||||
requestHeartbeatNow: vi.fn(),
|
||||
runIsolatedAgentJob: vi.fn().mockResolvedValue({ status: "ok", summary: "ok" }),
|
||||
});
|
||||
await cron.start();
|
||||
|
||||
const created = await cron.add({
|
||||
name: "repair-target",
|
||||
schedule: { kind: "cron", expr: "0 * * * *", tz: "UTC" },
|
||||
sessionTarget: "main",
|
||||
payload: { kind: "systemEvent", text: "tick" },
|
||||
});
|
||||
const updated = await cron.update(created.id, {
|
||||
payload: { kind: "systemEvent", text: "tick-2" },
|
||||
state: { nextRunAtMs: undefined },
|
||||
});
|
||||
|
||||
expect(updated.payload.kind).toBe("systemEvent");
|
||||
expect(typeof updated.state.nextRunAtMs).toBe("number");
|
||||
expect(updated.state.nextRunAtMs).toBe(created.state.nextRunAtMs);
|
||||
|
||||
cron.stop();
|
||||
await store.cleanup();
|
||||
});
|
||||
|
||||
it("does not advance unrelated due jobs when updating another job", async () => {
|
||||
const store = await makeStorePath();
|
||||
const now = Date.parse("2026-02-06T10:05:00.000Z");
|
||||
vi.setSystemTime(now);
|
||||
const cron = new CronService({
|
||||
cronEnabled: false,
|
||||
storePath: store.storePath,
|
||||
log: noopLogger,
|
||||
enqueueSystemEvent: vi.fn(),
|
||||
requestHeartbeatNow: vi.fn(),
|
||||
runIsolatedAgentJob: vi.fn().mockResolvedValue({ status: "ok", summary: "ok" }),
|
||||
});
|
||||
await cron.start();
|
||||
|
||||
const dueJob = await cron.add({
|
||||
name: "due-preserved",
|
||||
schedule: { kind: "every", everyMs: 60_000, anchorMs: now },
|
||||
sessionTarget: "main",
|
||||
payload: { kind: "systemEvent", text: "due-preserved" },
|
||||
});
|
||||
const otherJob = await cron.add({
|
||||
name: "other-job",
|
||||
schedule: { kind: "cron", expr: "0 * * * *", tz: "UTC" },
|
||||
sessionTarget: "main",
|
||||
payload: { kind: "systemEvent", text: "other" },
|
||||
});
|
||||
|
||||
const originalDueNextRunAtMs = dueJob.state.nextRunAtMs;
|
||||
expect(typeof originalDueNextRunAtMs).toBe("number");
|
||||
|
||||
// Make dueJob past-due without running timer callbacks.
|
||||
vi.setSystemTime(now + 5 * 60_000);
|
||||
|
||||
await cron.update(otherJob.id, {
|
||||
payload: { kind: "systemEvent", text: "other-updated" },
|
||||
});
|
||||
|
||||
const storeData = JSON.parse(await fs.readFile(store.storePath, "utf8")) as {
|
||||
jobs: Array<{ id: string; state?: { nextRunAtMs?: number } }>;
|
||||
};
|
||||
const persistedDueJob = storeData.jobs.find((job) => job.id === dueJob.id);
|
||||
expect(persistedDueJob?.state?.nextRunAtMs).toBe(originalDueNextRunAtMs);
|
||||
|
||||
cron.stop();
|
||||
await store.cleanup();
|
||||
});
|
||||
|
||||
it("caps timer delay to 60s for far-future schedules", async () => {
|
||||
const timeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
const store = await makeStorePath();
|
||||
|
||||
@@ -119,7 +119,7 @@ export async function add(state: CronServiceState, input: CronJobCreate) {
|
||||
export async function update(state: CronServiceState, id: string, patch: CronJobPatch) {
|
||||
return await locked(state, async () => {
|
||||
warnIfDisabled(state, "update");
|
||||
await ensureLoaded(state);
|
||||
await ensureLoaded(state, { skipRecompute: true });
|
||||
const job = findJobOrThrow(state, id);
|
||||
const now = state.deps.nowMs();
|
||||
applyJobPatch(job, patch);
|
||||
@@ -150,6 +150,13 @@ export async function update(state: CronServiceState, id: string, patch: CronJob
|
||||
job.state.nextRunAtMs = undefined;
|
||||
job.state.runningAtMs = undefined;
|
||||
}
|
||||
} else if (job.enabled) {
|
||||
// Non-schedule edits should not mutate other jobs, but still repair a
|
||||
// missing/corrupt nextRunAtMs for the updated job.
|
||||
const nextRun = job.state.nextRunAtMs;
|
||||
if (typeof nextRun !== "number" || !Number.isFinite(nextRun)) {
|
||||
job.state.nextRunAtMs = computeJobNextRunAtMs(job, now);
|
||||
}
|
||||
}
|
||||
|
||||
await persist(state);
|
||||
|
||||
Reference in New Issue
Block a user