mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 02:06:43 +00:00
fix(ui): preserve non-minute cron intervals on edit (#110034)
The Control UI rounded any every-schedule interval that is not a whole number of minutes up to the next minute when reading a job into the edit form (parseEverySchedule ceil): 30s and 90s and 4m6s all became a whole number of minutes. Because addCronJob always rebuilds the schedule into the cron.update patch, any save — including metadata-only edits that never touch the schedule — silently rewrote the cadence (e.g. 30s → 60s, 90s → 120s) and re-anchored the job. CLI, API, and docs accept these intervals, and the UI itself can create them, so the editor corrupted cadences supported everywhere else. Add a Seconds interval unit and read everyMs back into the largest unit that divides it exactly (days → hours → minutes → seconds), rendering sub-second remainders as exact decimal seconds built with BigInt quotient/remainder. Every integer millisecond up to Number.MAX_SAFE_INTEGER now round-trips losslessly, so a resave with the same everyMs preserves the gateway's anchor inheritance. Gateway, protocol, storage, and cron runtime are unchanged. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
9f25b0dc12
commit
bbaaac4b44
@@ -4190,9 +4190,11 @@ export const en: TranslationMap = {
|
||||
expression: "Expression",
|
||||
expressionPlaceholder: "0 7 * * *",
|
||||
everyAmountPlaceholder: "30",
|
||||
summaryEverySecondOne: "Runs every second",
|
||||
summaryEveryMinuteOne: "Runs every minute",
|
||||
summaryEveryHourOne: "Runs every hour",
|
||||
summaryEveryDayOne: "Runs every day",
|
||||
summaryEverySeconds: "Runs every {amount} seconds",
|
||||
summaryEveryMinutes: "Runs every {amount} minutes",
|
||||
summaryEveryHours: "Runs every {amount} hours",
|
||||
summaryEveryDays: "Runs every {amount} days",
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
const CRON_POSITIVE_DECIMAL_RE = /^(?:\d+(?:\.\d*)?|\.\d+)$/u;
|
||||
|
||||
const CRON_EVERY_UNIT_MS = {
|
||||
seconds: 1_000,
|
||||
minutes: 60_000,
|
||||
hours: 3_600_000,
|
||||
days: 86_400_000,
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
// Control UI tests cover cron behavior.
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { CronJob } from "../../api/types.ts";
|
||||
import { parseCronEveryMs } from "../../lib/cron/decimal.ts";
|
||||
import {
|
||||
addCronJob,
|
||||
cancelCronEdit,
|
||||
@@ -2180,6 +2182,135 @@ describe("cron controller", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("cron every-interval lossless round-trip", () => {
|
||||
function everyJob(everyMs: number): CronJob {
|
||||
return {
|
||||
id: "job-interval",
|
||||
name: "Interval",
|
||||
enabled: true,
|
||||
createdAtMs: 0,
|
||||
updatedAtMs: 0,
|
||||
schedule: { kind: "every", everyMs },
|
||||
sessionTarget: "isolated",
|
||||
wakeMode: "next-heartbeat",
|
||||
payload: { kind: "agentTurn", message: "tick" },
|
||||
delivery: { mode: "none" },
|
||||
state: {},
|
||||
} as unknown as CronJob;
|
||||
}
|
||||
|
||||
function captureUpdateState(job: CronJob) {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.update") {
|
||||
return { id: job.id };
|
||||
}
|
||||
if (method === "cron.list") {
|
||||
return { jobs: [{ id: job.id }] };
|
||||
}
|
||||
if (method === "cron.status") {
|
||||
return { enabled: true, jobs: 1, nextWakeAtMs: null };
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const state = createState({
|
||||
client: { request } as unknown as CronState["client"],
|
||||
cronJobs: [job],
|
||||
});
|
||||
return { request, state };
|
||||
}
|
||||
|
||||
// Each everyMs the editable form must reproduce exactly: reading a job into the
|
||||
// form and rebuilding the schedule may never change the cadence. Legal everyMs
|
||||
// spans 1ms..MAX_SAFE_INTEGER (gateway schema minimum 1, no sub-minute floor).
|
||||
const cases: ReadonlyArray<{ everyMs: number; amount: string; unit: string }> = [
|
||||
{ everyMs: 1, amount: "0.001", unit: "seconds" },
|
||||
{ everyMs: 450, amount: "0.45", unit: "seconds" },
|
||||
{ everyMs: 1_000, amount: "1", unit: "seconds" },
|
||||
{ everyMs: 30_000, amount: "30", unit: "seconds" },
|
||||
{ everyMs: 90_000, amount: "90", unit: "seconds" },
|
||||
{ everyMs: 246_000, amount: "246", unit: "seconds" },
|
||||
{ everyMs: 60_000, amount: "1", unit: "minutes" },
|
||||
{ everyMs: 7_200_000, amount: "2", unit: "hours" },
|
||||
{ everyMs: 86_400_000, amount: "1", unit: "days" },
|
||||
{ everyMs: Number.MAX_SAFE_INTEGER, amount: "9007199254740.991", unit: "seconds" },
|
||||
];
|
||||
|
||||
it("reads every job back into the most natural exact unit", () => {
|
||||
for (const { everyMs, amount, unit } of cases) {
|
||||
const state = createState();
|
||||
startCronEdit(state, everyJob(everyMs));
|
||||
expect(state.cronForm.everyUnit).toBe(unit);
|
||||
expect(state.cronForm.everyAmount).toBe(amount);
|
||||
// The rebuilt millisecond value must equal the original, not a rounded one.
|
||||
expect(parseCronEveryMs(state.cronForm.everyAmount, state.cronForm.everyUnit)).toBe(everyMs);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps everyMs unchanged on a metadata-only edit", async () => {
|
||||
for (const everyMs of [30_000, 90_000, 450, Number.MAX_SAFE_INTEGER]) {
|
||||
const { request, state } = captureUpdateState(everyJob(everyMs));
|
||||
startCronEdit(state, state.cronJobs[0] as CronJob);
|
||||
state.cronForm.name = "Renamed only";
|
||||
await addCronJob(state);
|
||||
|
||||
const updateCall = findRequestCall(request.mock.calls, "cron.update");
|
||||
const patch = requestPatch(updateCall);
|
||||
expect(patch.schedule).toEqual({ kind: "every", everyMs });
|
||||
}
|
||||
});
|
||||
|
||||
it("sends the edited interval when the seconds unit is changed", async () => {
|
||||
const wholeSeconds = captureUpdateState(everyJob(60_000));
|
||||
startCronEdit(wholeSeconds.state, wholeSeconds.state.cronJobs[0] as CronJob);
|
||||
wholeSeconds.state.cronForm.everyUnit = "seconds";
|
||||
wholeSeconds.state.cronForm.everyAmount = "45";
|
||||
await addCronJob(wholeSeconds.state);
|
||||
expect(
|
||||
requestPatch(findRequestCall(wholeSeconds.request.mock.calls, "cron.update")).schedule,
|
||||
).toEqual({ kind: "every", everyMs: 45_000 });
|
||||
|
||||
const subSecond = captureUpdateState(everyJob(60_000));
|
||||
startCronEdit(subSecond.state, subSecond.state.cronJobs[0] as CronJob);
|
||||
subSecond.state.cronForm.everyUnit = "seconds";
|
||||
subSecond.state.cronForm.everyAmount = "0.45";
|
||||
await addCronJob(subSecond.state);
|
||||
expect(
|
||||
requestPatch(findRequestCall(subSecond.request.mock.calls, "cron.update")).schedule,
|
||||
).toEqual({ kind: "every", everyMs: 450 });
|
||||
});
|
||||
|
||||
it("clones a sub-minute job without rounding its interval", async () => {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.add") {
|
||||
return { id: "job-clone" };
|
||||
}
|
||||
if (method === "cron.list") {
|
||||
return { jobs: [] };
|
||||
}
|
||||
if (method === "cron.status") {
|
||||
return { enabled: true, jobs: 0, nextWakeAtMs: null };
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const sourceJob = everyJob(30_000);
|
||||
const state = createState({
|
||||
client: { request } as unknown as CronState["client"],
|
||||
cronJobs: [sourceJob],
|
||||
});
|
||||
|
||||
startCronClone(state, sourceJob);
|
||||
expect(state.cronForm.everyUnit).toBe("seconds");
|
||||
expect(state.cronForm.everyAmount).toBe("30");
|
||||
await addCronJob(state);
|
||||
|
||||
const addCall = findRequestCall(request.mock.calls, "cron.add");
|
||||
expect((addCall[1] as { schedule?: unknown }).schedule).toEqual({
|
||||
kind: "every",
|
||||
everyMs: 30_000,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("loadCronFailingCount", () => {
|
||||
it("queries the unfiltered enabled+error total and stores it", async () => {
|
||||
const request = vi.fn(async () => ({ jobs: [], total: 4, offset: 0, limit: 1 }));
|
||||
|
||||
@@ -44,7 +44,7 @@ export type CronFormState = {
|
||||
scheduleKind: "at" | "every" | "cron" | "on-exit";
|
||||
scheduleAt: string;
|
||||
everyAmount: string;
|
||||
everyUnit: "minutes" | "hours" | "days";
|
||||
everyUnit: "seconds" | "minutes" | "hours" | "days";
|
||||
cronExpr: string;
|
||||
cronTz: string;
|
||||
scheduleExact: boolean;
|
||||
@@ -693,15 +693,32 @@ function formatDateTimeLocal(input: string): string {
|
||||
return `${year}-${month}-${day}T${hour}:${minute}`;
|
||||
}
|
||||
|
||||
// Render everyMs back to the largest unit that divides it exactly, falling through
|
||||
// to decimal seconds. Sub-second remainders are built from BigInt quotient/remainder,
|
||||
// not float division, so every integer millisecond up to Number.MAX_SAFE_INTEGER
|
||||
// round-trips losslessly through parseCronEveryMs when the job is resaved.
|
||||
function parseEverySchedule(everyMs: number): Pick<CronFormState, "everyAmount" | "everyUnit"> {
|
||||
if (everyMs % 86_400_000 === 0) {
|
||||
return { everyAmount: String(Math.max(1, everyMs / 86_400_000)), everyUnit: "days" };
|
||||
return { everyAmount: String(everyMs / 86_400_000), everyUnit: "days" };
|
||||
}
|
||||
if (everyMs % 3_600_000 === 0) {
|
||||
return { everyAmount: String(Math.max(1, everyMs / 3_600_000)), everyUnit: "hours" };
|
||||
return { everyAmount: String(everyMs / 3_600_000), everyUnit: "hours" };
|
||||
}
|
||||
const minutes = Math.max(1, Math.ceil(everyMs / 60_000));
|
||||
return { everyAmount: String(minutes), everyUnit: "minutes" };
|
||||
if (everyMs % 60_000 === 0) {
|
||||
return { everyAmount: String(everyMs / 60_000), everyUnit: "minutes" };
|
||||
}
|
||||
return { everyAmount: everyMsToSecondsString(everyMs), everyUnit: "seconds" };
|
||||
}
|
||||
|
||||
function everyMsToSecondsString(everyMs: number): string {
|
||||
const value = BigInt(everyMs);
|
||||
const whole = value / 1_000n;
|
||||
const remainder = value % 1_000n;
|
||||
if (remainder === 0n) {
|
||||
return String(whole);
|
||||
}
|
||||
const fractional = remainder.toString().padStart(3, "0").replace(/0+$/u, "");
|
||||
return `${whole}.${fractional}`;
|
||||
}
|
||||
|
||||
function parseStaggerSchedule(
|
||||
|
||||
@@ -618,6 +618,57 @@ describe("cron view editor", () => {
|
||||
expect(onceText).toContain("2026");
|
||||
});
|
||||
|
||||
it("offers a Seconds interval unit so sub-minute cadences stay editable", () => {
|
||||
const container = renderView({
|
||||
createOpen: true,
|
||||
form: {
|
||||
...DEFAULT_CRON_FORM,
|
||||
scheduleKind: "every",
|
||||
everyAmount: "30",
|
||||
everyUnit: "seconds",
|
||||
},
|
||||
});
|
||||
const unitSelect = getElement(container, 'select[aria-label="Unit"]', HTMLSelectElement);
|
||||
const values = Array.from(unitSelect.querySelectorAll("option")).map((option) => option.value);
|
||||
expect(values).toEqual(["seconds", "minutes", "hours", "days"]);
|
||||
});
|
||||
|
||||
it("summarizes seconds intervals, including singular and decimal amounts", () => {
|
||||
const singular = renderView({
|
||||
createOpen: true,
|
||||
form: { ...DEFAULT_CRON_FORM, scheduleKind: "every", everyAmount: "1", everyUnit: "seconds" },
|
||||
});
|
||||
expect(singular.querySelector(".cron-schedule-summary")?.textContent).toContain(
|
||||
"Runs every second",
|
||||
);
|
||||
|
||||
const plural = renderView({
|
||||
createOpen: true,
|
||||
form: {
|
||||
...DEFAULT_CRON_FORM,
|
||||
scheduleKind: "every",
|
||||
everyAmount: "30",
|
||||
everyUnit: "seconds",
|
||||
},
|
||||
});
|
||||
expect(plural.querySelector(".cron-schedule-summary")?.textContent).toContain(
|
||||
"Runs every 30 seconds",
|
||||
);
|
||||
|
||||
const decimal = renderView({
|
||||
createOpen: true,
|
||||
form: {
|
||||
...DEFAULT_CRON_FORM,
|
||||
scheduleKind: "every",
|
||||
everyAmount: "0.45",
|
||||
everyUnit: "seconds",
|
||||
},
|
||||
});
|
||||
expect(decimal.querySelector(".cron-schedule-summary")?.textContent).toContain(
|
||||
"Runs every 0.45 seconds",
|
||||
);
|
||||
});
|
||||
|
||||
it("hides the schedule summary for recurring amounts that cannot produce safe milliseconds", () => {
|
||||
for (const everyAmount of ["0x10", "1e3", "+1", String(Number.MAX_SAFE_INTEGER), "0.000001"]) {
|
||||
const container = renderView({
|
||||
|
||||
+15
-10
@@ -1241,19 +1241,23 @@ function describeFormSchedule(form: CronFormState): string | null {
|
||||
}
|
||||
if (Number(amount) === 1) {
|
||||
const singularKey =
|
||||
form.everyUnit === "minutes"
|
||||
? "cron.form.summaryEveryMinuteOne"
|
||||
: form.everyUnit === "hours"
|
||||
? "cron.form.summaryEveryHourOne"
|
||||
: "cron.form.summaryEveryDayOne";
|
||||
form.everyUnit === "seconds"
|
||||
? "cron.form.summaryEverySecondOne"
|
||||
: form.everyUnit === "minutes"
|
||||
? "cron.form.summaryEveryMinuteOne"
|
||||
: form.everyUnit === "hours"
|
||||
? "cron.form.summaryEveryHourOne"
|
||||
: "cron.form.summaryEveryDayOne";
|
||||
return t(singularKey);
|
||||
}
|
||||
const key =
|
||||
form.everyUnit === "minutes"
|
||||
? "cron.form.summaryEveryMinutes"
|
||||
: form.everyUnit === "hours"
|
||||
? "cron.form.summaryEveryHours"
|
||||
: "cron.form.summaryEveryDays";
|
||||
form.everyUnit === "seconds"
|
||||
? "cron.form.summaryEverySeconds"
|
||||
: form.everyUnit === "minutes"
|
||||
? "cron.form.summaryEveryMinutes"
|
||||
: form.everyUnit === "hours"
|
||||
? "cron.form.summaryEveryHours"
|
||||
: "cron.form.summaryEveryDays";
|
||||
return t(key, { amount });
|
||||
}
|
||||
if (form.scheduleKind === "at") {
|
||||
@@ -1361,6 +1365,7 @@ function renderScheduleSection(props: CronProps) {
|
||||
.value as CronFormState["everyUnit"],
|
||||
})}
|
||||
>
|
||||
<option value="seconds">${t("cron.form.seconds")}</option>
|
||||
<option value="minutes">${t("cron.form.minutes")}</option>
|
||||
<option value="hours">${t("cron.form.hours")}</option>
|
||||
<option value="days">${t("cron.form.days")}</option>
|
||||
|
||||
Reference in New Issue
Block a user