mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:00:43 +00:00
fix: reject invalid cron edits on disabled jobs (#74720)
* fix(cron): reject invalid disabled schedule updates * docs: add cron validation changelog entry --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -41,6 +41,7 @@ Docs: https://docs.openclaw.ai
|
||||
### Fixes
|
||||
|
||||
- Config: accept documented `browser.tabCleanup` keys in strict root config validation, so configured tab cleanup no longer fails before runtime reads it. Fixes #74577. Thanks @lonexreb and @ezdlp.
|
||||
- Cron: validate disabled job schedule edits before persisting updates, so invalid cron changes no longer partially mutate stored jobs. Fixes #74459. Thanks @yfge.
|
||||
- Channels/status: keep Telegram, Slack, and Google Chat read-only allowlist/default-target accessors on config-only paths, so status and channel summaries do not resolve SecretRef-backed runtime credentials. Thanks @eusine.
|
||||
- Active Memory: clarify the deprecated `modelFallbackPolicy` warning and config help so `modelFallback` is described as a chain-resolution last resort, not runtime failover. (#74602) Thanks @jeffrey701.
|
||||
- Channels/Discord: keep read-only allowlist/default-target accessors from resolving SecretRef-backed bot tokens, so status and channel summaries no longer fail when tokens are only available in gateway runtime. (#74737) Thanks @eusine.
|
||||
|
||||
@@ -260,6 +260,61 @@ describe("Cron issue regressions", () => {
|
||||
cron.stop();
|
||||
});
|
||||
|
||||
it("rejects invalid cron schedule updates without mutating disabled jobs", async () => {
|
||||
const store = cronIssueRegressionFixtures.makeStorePath();
|
||||
const cron = await startCronForStore({ storePath: store.storePath, cronEnabled: false });
|
||||
|
||||
const disabledJob = await cron.add({
|
||||
name: "disabled-cron",
|
||||
enabled: false,
|
||||
schedule: { kind: "cron", expr: "0 * * * *", tz: "UTC" },
|
||||
sessionTarget: "main",
|
||||
wakeMode: "next-heartbeat",
|
||||
payload: { kind: "systemEvent", text: "tick" },
|
||||
});
|
||||
|
||||
await expect(
|
||||
cron.update(disabledJob.id, {
|
||||
schedule: { kind: "cron", expr: "* * * 13 *", tz: "UTC" },
|
||||
}),
|
||||
).rejects.toThrow("CronPattern");
|
||||
|
||||
let persisted = await loadCronStore(store.storePath);
|
||||
let storedJob = persisted.jobs.find((job) => job.id === disabledJob.id);
|
||||
expect(storedJob?.enabled).toBe(false);
|
||||
expect(storedJob?.schedule).toEqual(
|
||||
expect.objectContaining({ kind: "cron", expr: "0 * * * *", tz: "UTC" }),
|
||||
);
|
||||
|
||||
await writeCronStoreSnapshot(store.storePath, [
|
||||
{
|
||||
id: "invalid-disabled-job",
|
||||
name: "invalid disabled job",
|
||||
createdAtMs: Date.parse("2026-02-06T10:00:00.000Z"),
|
||||
updatedAtMs: Date.parse("2026-02-06T10:00:00.000Z"),
|
||||
enabled: false,
|
||||
schedule: { kind: "cron", expr: "* * * 13 *", tz: "UTC" },
|
||||
sessionTarget: "main",
|
||||
wakeMode: "next-heartbeat",
|
||||
payload: { kind: "systemEvent", text: "tick" },
|
||||
state: {},
|
||||
},
|
||||
]);
|
||||
|
||||
const invalidCron = await startCronForStore({ storePath: store.storePath, cronEnabled: false });
|
||||
await expect(invalidCron.update("invalid-disabled-job", { enabled: true })).rejects.toThrow(
|
||||
"CronPattern",
|
||||
);
|
||||
|
||||
persisted = await loadCronStore(store.storePath);
|
||||
storedJob = persisted.jobs.find((job) => job.id === "invalid-disabled-job");
|
||||
expect(storedJob?.enabled).toBe(false);
|
||||
expect(storedJob?.state.nextRunAtMs).toBeUndefined();
|
||||
|
||||
invalidCron.stop();
|
||||
cron.stop();
|
||||
});
|
||||
|
||||
it("keeps telegram delivery target writeback after manual cron.run", async () => {
|
||||
const store = cronIssueRegressionFixtures.makeStorePath();
|
||||
const originalTarget = "https://t.me/obviyus";
|
||||
|
||||
@@ -353,19 +353,20 @@ export async function update(state: CronServiceState, id: string, patch: CronJob
|
||||
await ensureLoaded(state, { skipRecompute: true });
|
||||
const job = findJobOrThrow(state, id);
|
||||
const now = state.deps.nowMs();
|
||||
applyJobPatch(job, patch, { defaultAgentId: state.deps.defaultAgentId });
|
||||
if (job.schedule.kind === "every") {
|
||||
const anchor = job.schedule.anchorMs;
|
||||
const nextJob = structuredClone(job);
|
||||
applyJobPatch(nextJob, patch, { defaultAgentId: state.deps.defaultAgentId });
|
||||
if (nextJob.schedule.kind === "every") {
|
||||
const anchor = nextJob.schedule.anchorMs;
|
||||
if (typeof anchor !== "number" || !Number.isFinite(anchor)) {
|
||||
const patchSchedule = patch.schedule;
|
||||
const fallbackAnchorMs =
|
||||
patchSchedule?.kind === "every"
|
||||
? now
|
||||
: typeof job.createdAtMs === "number" && Number.isFinite(job.createdAtMs)
|
||||
? job.createdAtMs
|
||||
: typeof nextJob.createdAtMs === "number" && Number.isFinite(nextJob.createdAtMs)
|
||||
? nextJob.createdAtMs
|
||||
: now;
|
||||
job.schedule = {
|
||||
...job.schedule,
|
||||
nextJob.schedule = {
|
||||
...nextJob.schedule,
|
||||
anchorMs: Math.max(0, Math.floor(fallbackAnchorMs)),
|
||||
};
|
||||
}
|
||||
@@ -373,16 +374,27 @@ export async function update(state: CronServiceState, id: string, patch: CronJob
|
||||
const scheduleChanged = patch.schedule !== undefined;
|
||||
const enabledChanged = patch.enabled !== undefined;
|
||||
|
||||
job.updatedAtMs = now;
|
||||
if (scheduleChanged && nextJob.schedule.kind === "cron" && !isJobEnabled(nextJob)) {
|
||||
computeJobNextRunAtMs({ ...nextJob, enabled: true }, now);
|
||||
}
|
||||
|
||||
nextJob.updatedAtMs = now;
|
||||
if (scheduleChanged || enabledChanged) {
|
||||
if (isJobEnabled(job)) {
|
||||
job.state.nextRunAtMs = computeJobNextRunAtMs(job, now);
|
||||
if (isJobEnabled(nextJob)) {
|
||||
nextJob.state.nextRunAtMs = computeJobNextRunAtMs(nextJob, now);
|
||||
} else {
|
||||
job.state.nextRunAtMs = undefined;
|
||||
job.state.runningAtMs = undefined;
|
||||
nextJob.state.nextRunAtMs = undefined;
|
||||
nextJob.state.runningAtMs = undefined;
|
||||
}
|
||||
} else if (isJobEnabled(nextJob) && !hasScheduledNextRunAtMs(nextJob.state.nextRunAtMs)) {
|
||||
nextJob.state.nextRunAtMs = computeJobNextRunAtMs(nextJob, now);
|
||||
}
|
||||
|
||||
if (state.store) {
|
||||
const index = state.store.jobs.findIndex((entry) => entry.id === id);
|
||||
if (index >= 0) {
|
||||
state.store.jobs[index] = nextJob;
|
||||
}
|
||||
} else if (isJobEnabled(job) && !hasScheduledNextRunAtMs(job.state.nextRunAtMs)) {
|
||||
job.state.nextRunAtMs = computeJobNextRunAtMs(job, now);
|
||||
}
|
||||
|
||||
await persist(state);
|
||||
@@ -390,10 +402,10 @@ export async function update(state: CronServiceState, id: string, patch: CronJob
|
||||
emit(state, {
|
||||
jobId: id,
|
||||
action: "updated",
|
||||
job,
|
||||
nextRunAtMs: job.state.nextRunAtMs,
|
||||
job: nextJob,
|
||||
nextRunAtMs: nextJob.state.nextRunAtMs,
|
||||
});
|
||||
return job;
|
||||
return nextJob;
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user