mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:30:42 +00:00
fix: keep cron last delivery sentinel runtime-only (#68829) (thanks @tianhaocui)
* fix(cron): stop persisting "last" as literal delivery channel value The UI controller writes the sentinel value "last" into jobs.json when the delivery channel field is empty. This overwrites user-configured channels (e.g. "telegram") because the form populates with "last" as the default fallback, and saving the form materializes it as a literal persisted value. "last" is a runtime-only sentinel meaning "use whatever channel was last used in the session" and should never be written to jobs.json. When the channel field is empty, write `undefined` instead so the runtime delivery plan resolver applies the "last" fallback at execution time without polluting the persisted state. Fixes #68760 * fix(cron): keep last delivery sentinel runtime-only * fix: keep cron last delivery sentinel runtime-only (#68829) (thanks @tianhaocui) * fix: preserve clear-to-last cron updates (#68829) (thanks @tianhaocui) --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
@@ -8,6 +8,8 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Control UI/cron: keep the runtime-only `last` delivery sentinel from being materialized into persisted cron delivery and failure-alert channel configs when jobs are created or edited. (#68829) Thanks @tianhaocui.
|
||||
|
||||
## 2026.4.19-beta.2
|
||||
|
||||
### Fixes
|
||||
|
||||
@@ -163,6 +163,48 @@ describe("cron controller", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('omits delivery.channel when the form still uses the "last" sentinel', async () => {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.add") {
|
||||
return { id: "job-last-add" };
|
||||
}
|
||||
if (method === "cron.list") {
|
||||
return { jobs: [] };
|
||||
}
|
||||
if (method === "cron.status") {
|
||||
return { enabled: true, jobs: 0, nextWakeAtMs: null };
|
||||
}
|
||||
return {};
|
||||
});
|
||||
|
||||
const state = createState({
|
||||
client: { request } as unknown as CronState["client"],
|
||||
cronForm: {
|
||||
...DEFAULT_CRON_FORM,
|
||||
name: "implicit channel",
|
||||
scheduleKind: "cron",
|
||||
cronExpr: "0 * * * *",
|
||||
sessionTarget: "isolated",
|
||||
wakeMode: "next-heartbeat",
|
||||
payloadKind: "agentTurn",
|
||||
payloadText: "run this",
|
||||
deliveryMode: "announce",
|
||||
deliveryChannel: "last",
|
||||
},
|
||||
});
|
||||
|
||||
await addCronJob(state);
|
||||
|
||||
const addCall = request.mock.calls.find(([method]) => method === "cron.add");
|
||||
expect(addCall).toBeDefined();
|
||||
expect(addCall?.[1]).toMatchObject({
|
||||
delivery: { mode: "announce" },
|
||||
});
|
||||
expect(
|
||||
(addCall?.[1] as { delivery?: { channel?: string } } | undefined)?.delivery?.channel,
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it("forwards lightContext in cron payload", async () => {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.add") {
|
||||
@@ -485,6 +527,97 @@ describe("cron controller", () => {
|
||||
expect(state.cronForm.deliveryAccountId).toBe("bot-2");
|
||||
});
|
||||
|
||||
it('keeps implicit announce delivery implicit when editing a job that shows "last" in the form', async () => {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.update") {
|
||||
return { id: "job-implicit-delivery" };
|
||||
}
|
||||
if (method === "cron.list") {
|
||||
return { jobs: [{ id: "job-implicit-delivery" }] };
|
||||
}
|
||||
if (method === "cron.status") {
|
||||
return { enabled: true, jobs: 1, nextWakeAtMs: null };
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const job = {
|
||||
id: "job-implicit-delivery",
|
||||
name: "Implicit delivery",
|
||||
enabled: true,
|
||||
createdAtMs: 0,
|
||||
updatedAtMs: 0,
|
||||
schedule: { kind: "cron" as const, expr: "0 * * * *" },
|
||||
sessionTarget: "isolated" as const,
|
||||
wakeMode: "next-heartbeat" as const,
|
||||
payload: { kind: "agentTurn" as const, message: "run" },
|
||||
delivery: { mode: "announce" as const, to: "123" },
|
||||
state: {},
|
||||
};
|
||||
const state = createState({
|
||||
client: { request } as unknown as CronState["client"],
|
||||
cronJobs: [job],
|
||||
});
|
||||
|
||||
startCronEdit(state, job);
|
||||
await addCronJob(state);
|
||||
|
||||
const updateCall = request.mock.calls.find(([method]) => method === "cron.update");
|
||||
expect(updateCall).toBeDefined();
|
||||
expect(updateCall?.[1]).toMatchObject({
|
||||
id: "job-implicit-delivery",
|
||||
patch: {
|
||||
delivery: { mode: "announce", to: "123" },
|
||||
},
|
||||
});
|
||||
expect(
|
||||
(updateCall?.[1] as { patch?: { delivery?: { channel?: string } } } | undefined)?.patch
|
||||
?.delivery?.channel,
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('sends delivery.channel="last" when editing clears an explicit channel back to implicit-last', async () => {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.update") {
|
||||
return { id: "job-clear-delivery-channel" };
|
||||
}
|
||||
if (method === "cron.list") {
|
||||
return { jobs: [{ id: "job-clear-delivery-channel" }] };
|
||||
}
|
||||
if (method === "cron.status") {
|
||||
return { enabled: true, jobs: 1, nextWakeAtMs: null };
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const job = {
|
||||
id: "job-clear-delivery-channel",
|
||||
name: "Clear delivery channel",
|
||||
enabled: true,
|
||||
createdAtMs: 0,
|
||||
updatedAtMs: 0,
|
||||
schedule: { kind: "cron" as const, expr: "0 * * * *" },
|
||||
sessionTarget: "isolated" as const,
|
||||
wakeMode: "next-heartbeat" as const,
|
||||
payload: { kind: "agentTurn" as const, message: "run" },
|
||||
delivery: { mode: "announce" as const, channel: "telegram", to: "123" },
|
||||
state: {},
|
||||
};
|
||||
const state = createState({
|
||||
client: { request } as unknown as CronState["client"],
|
||||
cronJobs: [job],
|
||||
});
|
||||
|
||||
startCronEdit(state, job);
|
||||
state.cronForm.deliveryChannel = "last";
|
||||
await addCronJob(state);
|
||||
|
||||
const updateCall = request.mock.calls.find(([method]) => method === "cron.update");
|
||||
expect(updateCall).toBeDefined();
|
||||
expect(
|
||||
(updateCall?.[1] as { patch?: { delivery?: { channel?: string } } } | undefined)?.patch
|
||||
?.delivery?.channel,
|
||||
).toBe("last");
|
||||
});
|
||||
|
||||
it("includes model/thinking/stagger/bestEffort in cron.update patch", async () => {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.update") {
|
||||
@@ -684,6 +817,103 @@ describe("cron controller", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps implicit failure alert delivery implicit when editing a job that shows "last" in the form', async () => {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.update") {
|
||||
return { id: "job-alert-implicit-channel" };
|
||||
}
|
||||
if (method === "cron.list") {
|
||||
return { jobs: [{ id: "job-alert-implicit-channel" }] };
|
||||
}
|
||||
if (method === "cron.status") {
|
||||
return { enabled: true, jobs: 1, nextWakeAtMs: null };
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const job = {
|
||||
id: "job-alert-implicit-channel",
|
||||
name: "Implicit failure alert",
|
||||
enabled: true,
|
||||
createdAtMs: 0,
|
||||
updatedAtMs: 0,
|
||||
schedule: { kind: "cron" as const, expr: "0 * * * *" },
|
||||
sessionTarget: "isolated" as const,
|
||||
wakeMode: "next-heartbeat" as const,
|
||||
payload: { kind: "agentTurn" as const, message: "run" },
|
||||
delivery: { mode: "announce" as const, channel: "telegram", to: "123" },
|
||||
failureAlert: { after: 2, to: "123" },
|
||||
state: {},
|
||||
};
|
||||
const state = createState({
|
||||
client: { request } as unknown as CronState["client"],
|
||||
cronJobs: [job],
|
||||
});
|
||||
|
||||
startCronEdit(state, job);
|
||||
await addCronJob(state);
|
||||
|
||||
const updateCall = request.mock.calls.find(([method]) => method === "cron.update");
|
||||
expect(updateCall).toBeDefined();
|
||||
expect(updateCall?.[1]).toMatchObject({
|
||||
id: "job-alert-implicit-channel",
|
||||
patch: {
|
||||
failureAlert: {
|
||||
after: 2,
|
||||
to: "123",
|
||||
mode: "announce",
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(
|
||||
(updateCall?.[1] as { patch?: { failureAlert?: { channel?: string } } } | undefined)?.patch
|
||||
?.failureAlert?.channel,
|
||||
).toBeUndefined();
|
||||
});
|
||||
|
||||
it('sends failureAlert.channel="last" when editing clears an explicit failure channel back to implicit-last', async () => {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.update") {
|
||||
return { id: "job-clear-failure-channel" };
|
||||
}
|
||||
if (method === "cron.list") {
|
||||
return { jobs: [{ id: "job-clear-failure-channel" }] };
|
||||
}
|
||||
if (method === "cron.status") {
|
||||
return { enabled: true, jobs: 1, nextWakeAtMs: null };
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const job = {
|
||||
id: "job-clear-failure-channel",
|
||||
name: "Clear failure channel",
|
||||
enabled: true,
|
||||
createdAtMs: 0,
|
||||
updatedAtMs: 0,
|
||||
schedule: { kind: "cron" as const, expr: "0 * * * *" },
|
||||
sessionTarget: "isolated" as const,
|
||||
wakeMode: "next-heartbeat" as const,
|
||||
payload: { kind: "agentTurn" as const, message: "run" },
|
||||
delivery: { mode: "announce" as const, channel: "telegram", to: "123" },
|
||||
failureAlert: { after: 2, channel: "telegram", to: "123" },
|
||||
state: {},
|
||||
};
|
||||
const state = createState({
|
||||
client: { request } as unknown as CronState["client"],
|
||||
cronJobs: [job],
|
||||
});
|
||||
|
||||
startCronEdit(state, job);
|
||||
state.cronForm.failureAlertChannel = "last";
|
||||
await addCronJob(state);
|
||||
|
||||
const updateCall = request.mock.calls.find(([method]) => method === "cron.update");
|
||||
expect(updateCall).toBeDefined();
|
||||
expect(
|
||||
(updateCall?.[1] as { patch?: { failureAlert?: { channel?: string } } } | undefined)?.patch
|
||||
?.failureAlert?.channel,
|
||||
).toBe("last");
|
||||
});
|
||||
|
||||
it("omits failureAlert.cooldownMs when custom cooldown is left blank", async () => {
|
||||
const request = vi.fn(async (method: string, _payload?: unknown) => {
|
||||
if (method === "cron.update") {
|
||||
|
||||
@@ -597,7 +597,21 @@ export function buildCronPayload(form: CronFormState) {
|
||||
return payload;
|
||||
}
|
||||
|
||||
function buildFailureAlert(form: CronFormState) {
|
||||
function normalizePersistedDeliveryChannel(
|
||||
value: string,
|
||||
options: { preserveLastOnUpdate?: boolean } = {},
|
||||
) {
|
||||
const channel = value.trim();
|
||||
if (!channel) {
|
||||
return undefined;
|
||||
}
|
||||
if (channel === CRON_CHANNEL_LAST) {
|
||||
return options.preserveLastOnUpdate ? CRON_CHANNEL_LAST : undefined;
|
||||
}
|
||||
return channel;
|
||||
}
|
||||
|
||||
function buildFailureAlert(form: CronFormState, existingChannel?: string | undefined) {
|
||||
if (form.failureAlertMode === "disabled") {
|
||||
return false as const;
|
||||
}
|
||||
@@ -615,15 +629,15 @@ function buildFailureAlert(form: CronFormState) {
|
||||
const accountId = form.failureAlertAccountId.trim();
|
||||
const patch: Record<string, unknown> = {
|
||||
after: after > 0 ? Math.floor(after) : undefined,
|
||||
channel: form.failureAlertChannel.trim() || CRON_CHANNEL_LAST,
|
||||
channel: normalizePersistedDeliveryChannel(form.failureAlertChannel, {
|
||||
preserveLastOnUpdate: Boolean(existingChannel),
|
||||
}),
|
||||
to: form.failureAlertTo.trim() || undefined,
|
||||
...(cooldownMs !== undefined ? { cooldownMs } : {}),
|
||||
};
|
||||
// Always include mode and accountId so users can switch/clear them
|
||||
if (deliveryMode) {
|
||||
patch.mode = deliveryMode;
|
||||
}
|
||||
// Include accountId if explicitly set, or send undefined to allow clearing
|
||||
patch.accountId = accountId || undefined;
|
||||
return patch;
|
||||
}
|
||||
@@ -663,7 +677,9 @@ export async function addCronJob(state: CronState) {
|
||||
mode: selectedDeliveryMode,
|
||||
channel:
|
||||
selectedDeliveryMode === "announce"
|
||||
? form.deliveryChannel.trim() || "last"
|
||||
? normalizePersistedDeliveryChannel(form.deliveryChannel, {
|
||||
preserveLastOnUpdate: Boolean(editingJob?.delivery?.channel),
|
||||
})
|
||||
: undefined,
|
||||
to: form.deliveryTo.trim() || undefined,
|
||||
accountId:
|
||||
@@ -673,7 +689,12 @@ export async function addCronJob(state: CronState) {
|
||||
: selectedDeliveryMode === "none"
|
||||
? ({ mode: "none" } as const)
|
||||
: undefined;
|
||||
const failureAlert = buildFailureAlert(form);
|
||||
const failureAlert = buildFailureAlert(
|
||||
form,
|
||||
editingJob?.failureAlert && typeof editingJob.failureAlert === "object"
|
||||
? editingJob.failureAlert.channel
|
||||
: undefined,
|
||||
);
|
||||
const agentId = form.clearAgent ? null : form.agentId.trim();
|
||||
const sessionKeyRaw = form.sessionKey.trim();
|
||||
const sessionKey = sessionKeyRaw || (editingJob?.sessionKey ? null : undefined);
|
||||
|
||||
Reference in New Issue
Block a user