mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-10 16:45:01 +00:00
fix(cron): treat missing enabled as true in update() (openclaw#15477) thanks @eternauta1337
Verified: - pnpm exec vitest src/cron/service.issue-regressions.test.ts Co-authored-by: eternauta1337 <550409+eternauta1337@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
a7b6555195
commit
9a344da298
@@ -66,6 +66,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Gateway/Agent: route bare `/new` and `/reset` through `sessions.reset` before running the fresh-session greeting prompt, so reset commands clear the current session in-place instead of falling through to normal agent runs. (#16732) Thanks @kdotndot and @vignesh07.
|
- Gateway/Agent: route bare `/new` and `/reset` through `sessions.reset` before running the fresh-session greeting prompt, so reset commands clear the current session in-place instead of falling through to normal agent runs. (#16732) Thanks @kdotndot and @vignesh07.
|
||||||
- Cron: prevent `cron list`/`cron status` from silently skipping past-due recurring jobs by using maintenance recompute semantics. (#16156) Thanks @zerone0x.
|
- Cron: prevent `cron list`/`cron status` from silently skipping past-due recurring jobs by using maintenance recompute semantics. (#16156) Thanks @zerone0x.
|
||||||
- Cron: repair missing/corrupt `nextRunAtMs` for the updated job without globally recomputing unrelated due jobs during `cron update`. (#15750)
|
- Cron: repair missing/corrupt `nextRunAtMs` for the updated job without globally recomputing unrelated due jobs during `cron update`. (#15750)
|
||||||
|
- Cron: treat persisted jobs with missing `enabled` as enabled by default across update/list/timer due-path checks, and add regression coverage for missing-`enabled` store records. (#15433) Thanks @eternauta1337.
|
||||||
- Cron: skip missed-job replay on startup for jobs interrupted mid-run (stale `runningAtMs` markers), preventing restart loops for self-restarting jobs such as update tasks. (#16694) Thanks @sbmilburn.
|
- Cron: skip missed-job replay on startup for jobs interrupted mid-run (stale `runningAtMs` markers), preventing restart loops for self-restarting jobs such as update tasks. (#16694) Thanks @sbmilburn.
|
||||||
- Heartbeat/Cron: treat cron-tagged queued system events as cron reminders even on interval wakes, so isolated cron announce summaries no longer run under the default heartbeat prompt. (#14947) Thanks @archedark-ada and @vignesh07.
|
- Heartbeat/Cron: treat cron-tagged queued system events as cron reminders even on interval wakes, so isolated cron announce summaries no longer run under the default heartbeat prompt. (#14947) Thanks @archedark-ada and @vignesh07.
|
||||||
- Discord: prefer gateway guild id when logging inbound messages so cached-miss guilds do not appear as `guild=dm`. Thanks @thewilloftheshadow.
|
- Discord: prefer gateway guild id when logging inbound messages so cached-miss guilds do not appear as `guild=dm`. Thanks @thewilloftheshadow.
|
||||||
|
|||||||
@@ -233,6 +233,109 @@ describe("Cron issue regressions", () => {
|
|||||||
await store.cleanup();
|
await store.cleanup();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("treats persisted jobs with missing enabled as enabled during update()", async () => {
|
||||||
|
const store = await makeStorePath();
|
||||||
|
const now = Date.parse("2026-02-06T10:05:00.000Z");
|
||||||
|
await fs.writeFile(
|
||||||
|
store.storePath,
|
||||||
|
JSON.stringify(
|
||||||
|
{
|
||||||
|
version: 1,
|
||||||
|
jobs: [
|
||||||
|
{
|
||||||
|
id: "missing-enabled-update",
|
||||||
|
name: "legacy missing enabled",
|
||||||
|
createdAtMs: now - 60_000,
|
||||||
|
updatedAtMs: now - 60_000,
|
||||||
|
schedule: { kind: "cron", expr: "0 */2 * * *", tz: "UTC" },
|
||||||
|
sessionTarget: "main",
|
||||||
|
wakeMode: "next-heartbeat",
|
||||||
|
payload: { kind: "systemEvent", text: "legacy" },
|
||||||
|
state: {},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
null,
|
||||||
|
2,
|
||||||
|
),
|
||||||
|
"utf-8",
|
||||||
|
);
|
||||||
|
|
||||||
|
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 listed = await cron.list();
|
||||||
|
expect(listed.some((job) => job.id === "missing-enabled-update")).toBe(true);
|
||||||
|
|
||||||
|
const updated = await cron.update("missing-enabled-update", {
|
||||||
|
schedule: { kind: "cron", expr: "0 */3 * * *", tz: "UTC" },
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(updated.state.nextRunAtMs).toBeTypeOf("number");
|
||||||
|
expect(updated.state.nextRunAtMs).toBeGreaterThan(now);
|
||||||
|
|
||||||
|
cron.stop();
|
||||||
|
await store.cleanup();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("treats persisted due jobs with missing enabled as runnable", async () => {
|
||||||
|
const store = await makeStorePath();
|
||||||
|
const now = Date.parse("2026-02-06T10:05:00.000Z");
|
||||||
|
const dueAt = now - 30_000;
|
||||||
|
await fs.writeFile(
|
||||||
|
store.storePath,
|
||||||
|
JSON.stringify(
|
||||||
|
{
|
||||||
|
version: 1,
|
||||||
|
jobs: [
|
||||||
|
{
|
||||||
|
id: "missing-enabled-due",
|
||||||
|
name: "legacy due job",
|
||||||
|
createdAtMs: dueAt - 60_000,
|
||||||
|
updatedAtMs: dueAt,
|
||||||
|
schedule: { kind: "at", at: new Date(dueAt).toISOString() },
|
||||||
|
sessionTarget: "main",
|
||||||
|
wakeMode: "now",
|
||||||
|
payload: { kind: "systemEvent", text: "missing-enabled-due" },
|
||||||
|
state: { nextRunAtMs: dueAt },
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
null,
|
||||||
|
2,
|
||||||
|
),
|
||||||
|
"utf-8",
|
||||||
|
);
|
||||||
|
|
||||||
|
const enqueueSystemEvent = vi.fn();
|
||||||
|
const cron = new CronService({
|
||||||
|
cronEnabled: false,
|
||||||
|
storePath: store.storePath,
|
||||||
|
log: noopLogger,
|
||||||
|
enqueueSystemEvent,
|
||||||
|
requestHeartbeatNow: vi.fn(),
|
||||||
|
runIsolatedAgentJob: vi.fn().mockResolvedValue({ status: "ok", summary: "ok" }),
|
||||||
|
});
|
||||||
|
await cron.start();
|
||||||
|
|
||||||
|
const result = await cron.run("missing-enabled-due", "due");
|
||||||
|
expect(result).toEqual({ ok: true, ran: true });
|
||||||
|
expect(enqueueSystemEvent).toHaveBeenCalledWith(
|
||||||
|
"missing-enabled-due",
|
||||||
|
expect.objectContaining({ agentId: undefined }),
|
||||||
|
);
|
||||||
|
|
||||||
|
cron.stop();
|
||||||
|
await store.cleanup();
|
||||||
|
});
|
||||||
|
|
||||||
it("caps timer delay to 60s for far-future schedules", async () => {
|
it("caps timer delay to 60s for far-future schedules", async () => {
|
||||||
const timeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
const timeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||||
const store = await makeStorePath();
|
const store = await makeStorePath();
|
||||||
|
|||||||
@@ -55,7 +55,7 @@ export function findJobOrThrow(state: CronServiceState, id: string) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function computeJobNextRunAtMs(job: CronJob, nowMs: number): number | undefined {
|
export function computeJobNextRunAtMs(job: CronJob, nowMs: number): number | undefined {
|
||||||
if (!job.enabled) {
|
if (job.enabled === false) {
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
if (job.schedule.kind === "every") {
|
if (job.schedule.kind === "every") {
|
||||||
@@ -102,7 +102,7 @@ function normalizeJobTickState(params: { state: CronServiceState; job: CronJob;
|
|||||||
changed = true;
|
changed = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!job.enabled) {
|
if (job.enabled === false) {
|
||||||
if (job.state.nextRunAtMs !== undefined) {
|
if (job.state.nextRunAtMs !== undefined) {
|
||||||
job.state.nextRunAtMs = undefined;
|
job.state.nextRunAtMs = undefined;
|
||||||
changed = true;
|
changed = true;
|
||||||
@@ -220,7 +220,9 @@ export function recomputeNextRunsForMaintenance(state: CronServiceState): boolea
|
|||||||
|
|
||||||
export function nextWakeAtMs(state: CronServiceState) {
|
export function nextWakeAtMs(state: CronServiceState) {
|
||||||
const jobs = state.store?.jobs ?? [];
|
const jobs = state.store?.jobs ?? [];
|
||||||
const enabled = jobs.filter((j) => j.enabled && typeof j.state.nextRunAtMs === "number");
|
const enabled = jobs.filter(
|
||||||
|
(j) => j.enabled !== false && typeof j.state.nextRunAtMs === "number",
|
||||||
|
);
|
||||||
if (enabled.length === 0) {
|
if (enabled.length === 0) {
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
@@ -480,7 +482,11 @@ export function isJobDue(job: CronJob, nowMs: number, opts: { forced: boolean })
|
|||||||
if (opts.forced) {
|
if (opts.forced) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
return job.enabled && typeof job.state.nextRunAtMs === "number" && nowMs >= job.state.nextRunAtMs;
|
return (
|
||||||
|
job.enabled !== false &&
|
||||||
|
typeof job.state.nextRunAtMs === "number" &&
|
||||||
|
nowMs >= job.state.nextRunAtMs
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
export function resolveJobPayloadTextForMain(job: CronJob): string | undefined {
|
export function resolveJobPayloadTextForMain(job: CronJob): string | undefined {
|
||||||
|
|||||||
@@ -84,7 +84,7 @@ export async function list(state: CronServiceState, opts?: { includeDisabled?: b
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
const includeDisabled = opts?.includeDisabled === true;
|
const includeDisabled = opts?.includeDisabled === true;
|
||||||
const jobs = (state.store?.jobs ?? []).filter((j) => includeDisabled || j.enabled);
|
const jobs = (state.store?.jobs ?? []).filter((j) => includeDisabled || j.enabled !== false);
|
||||||
return jobs.toSorted((a, b) => (a.state.nextRunAtMs ?? 0) - (b.state.nextRunAtMs ?? 0));
|
return jobs.toSorted((a, b) => (a.state.nextRunAtMs ?? 0) - (b.state.nextRunAtMs ?? 0));
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -151,7 +151,7 @@ export async function update(state: CronServiceState, id: string, patch: CronJob
|
|||||||
|
|
||||||
job.updatedAtMs = now;
|
job.updatedAtMs = now;
|
||||||
if (scheduleChanged || enabledChanged) {
|
if (scheduleChanged || enabledChanged) {
|
||||||
if (job.enabled) {
|
if (job.enabled !== false) {
|
||||||
job.state.nextRunAtMs = computeJobNextRunAtMs(job, now);
|
job.state.nextRunAtMs = computeJobNextRunAtMs(job, now);
|
||||||
} else {
|
} else {
|
||||||
job.state.nextRunAtMs = undefined;
|
job.state.nextRunAtMs = undefined;
|
||||||
|
|||||||
@@ -90,7 +90,7 @@ function applyJobResult(
|
|||||||
"cron: disabling one-shot job after error",
|
"cron: disabling one-shot job after error",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
} else if (result.status === "error" && job.enabled) {
|
} else if (result.status === "error" && job.enabled !== false) {
|
||||||
// Apply exponential backoff for errored jobs to prevent retry storms.
|
// Apply exponential backoff for errored jobs to prevent retry storms.
|
||||||
const backoff = errorBackoffMs(job.state.consecutiveErrors ?? 1);
|
const backoff = errorBackoffMs(job.state.consecutiveErrors ?? 1);
|
||||||
const normalNext = computeJobNextRunAtMs(job, result.endedAt);
|
const normalNext = computeJobNextRunAtMs(job, result.endedAt);
|
||||||
@@ -107,7 +107,7 @@ function applyJobResult(
|
|||||||
},
|
},
|
||||||
"cron: applying error backoff",
|
"cron: applying error backoff",
|
||||||
);
|
);
|
||||||
} else if (job.enabled) {
|
} else if (job.enabled !== false) {
|
||||||
job.state.nextRunAtMs = computeJobNextRunAtMs(job, result.endedAt);
|
job.state.nextRunAtMs = computeJobNextRunAtMs(job, result.endedAt);
|
||||||
} else {
|
} else {
|
||||||
job.state.nextRunAtMs = undefined;
|
job.state.nextRunAtMs = undefined;
|
||||||
@@ -129,10 +129,11 @@ export function armTimer(state: CronServiceState) {
|
|||||||
const nextAt = nextWakeAtMs(state);
|
const nextAt = nextWakeAtMs(state);
|
||||||
if (!nextAt) {
|
if (!nextAt) {
|
||||||
const jobCount = state.store?.jobs.length ?? 0;
|
const jobCount = state.store?.jobs.length ?? 0;
|
||||||
const enabledCount = state.store?.jobs.filter((j) => j.enabled).length ?? 0;
|
const enabledCount = state.store?.jobs.filter((j) => j.enabled !== false).length ?? 0;
|
||||||
const withNextRun =
|
const withNextRun =
|
||||||
state.store?.jobs.filter((j) => j.enabled && typeof j.state.nextRunAtMs === "number")
|
state.store?.jobs.filter(
|
||||||
.length ?? 0;
|
(j) => j.enabled !== false && typeof j.state.nextRunAtMs === "number",
|
||||||
|
).length ?? 0;
|
||||||
state.deps.log.debug(
|
state.deps.log.debug(
|
||||||
{ jobCount, enabledCount, withNextRun },
|
{ jobCount, enabledCount, withNextRun },
|
||||||
"cron: armTimer skipped - no jobs with nextRunAtMs",
|
"cron: armTimer skipped - no jobs with nextRunAtMs",
|
||||||
@@ -346,7 +347,7 @@ function findDueJobs(state: CronServiceState): CronJob[] {
|
|||||||
if (!j.state) {
|
if (!j.state) {
|
||||||
j.state = {};
|
j.state = {};
|
||||||
}
|
}
|
||||||
if (!j.enabled) {
|
if (j.enabled === false) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
if (typeof j.state.runningAtMs === "number") {
|
if (typeof j.state.runningAtMs === "number") {
|
||||||
@@ -370,7 +371,7 @@ export async function runMissedJobs(
|
|||||||
if (!j.state) {
|
if (!j.state) {
|
||||||
j.state = {};
|
j.state = {};
|
||||||
}
|
}
|
||||||
if (!j.enabled) {
|
if (j.enabled === false) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
if (skipJobIds?.has(j.id)) {
|
if (skipJobIds?.has(j.id)) {
|
||||||
@@ -409,7 +410,7 @@ export async function runDueJobs(state: CronServiceState) {
|
|||||||
if (!j.state) {
|
if (!j.state) {
|
||||||
j.state = {};
|
j.state = {};
|
||||||
}
|
}
|
||||||
if (!j.enabled) {
|
if (j.enabled === false) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
if (typeof j.state.runningAtMs === "number") {
|
if (typeof j.state.runningAtMs === "number") {
|
||||||
|
|||||||
Reference in New Issue
Block a user