From 0c444ff5ba5e003c4ddd3c21909a3a9585498ca3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 20 Apr 2026 14:09:13 +0100 Subject: [PATCH] fix: classify no-delivery cron runs correctly (#69285) --- CHANGELOG.md | 4 ++ .../service.persists-delivered-status.test.ts | 43 ++++++++++++++++--- src/cron/service/timer.ts | 22 ++++++---- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9601ec56a73..4b2d2bbf564 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Docs: https://docs.openclaw.ai ## Unreleased +### Fixes + +- Cron/delivery: treat explicit `delivery.mode: "none"` runs as not requested even if the runner reports `delivered: false`, so no-delivery cron jobs no longer persist false delivery failures or errors. (#69285) Thanks @matsuri1987. + ## 2026.4.20 ### Changes diff --git a/src/cron/service.persists-delivered-status.test.ts b/src/cron/service.persists-delivered-status.test.ts index dab021731c7..3a957037ff8 100644 --- a/src/cron/service.persists-delivered-status.test.ts +++ b/src/cron/service.persists-delivered-status.test.ts @@ -26,6 +26,13 @@ function buildIsolatedAgentTurnJob(name: string): CronAddInput { }; } +function buildAnnounceIsolatedAgentTurnJob(name: string): CronAddInput { + return { + ...buildIsolatedAgentTurnJob(name), + delivery: { mode: "announce", channel: "telegram", to: "123" }, + }; +} + function buildMainSessionSystemEventJob(name: string): CronAddInput { return { name, @@ -40,6 +47,7 @@ function buildMainSessionSystemEventJob(name: string): CronAddInput { function createIsolatedCronWithFinishedBarrier(params: { storePath: string; delivered?: boolean; + error?: string; onFinished?: (evt: { jobId: string; delivered?: boolean; deliveryStatus?: string }) => void; }) { const finished = createFinishedBarrier(); @@ -52,6 +60,7 @@ function createIsolatedCronWithFinishedBarrier(params: { runIsolatedAgentJob: vi.fn(async () => ({ status: "ok" as const, summary: "done", + ...(params.error === undefined ? {} : { error: params.error }), ...(params.delivered === undefined ? {} : { delivered: params.delivered }), })), onEvent: (evt) => { @@ -117,12 +126,14 @@ function expectDeliveryNotRequested( async function runIsolatedJobAndReadState(params: { job: CronAddInput; delivered?: boolean; + error?: string; onFinished?: (evt: { jobId: string; delivered?: boolean; deliveryStatus?: string }) => void; }) { const store = await makeStorePath(); const { cron, finished } = createIsolatedCronWithFinishedBarrier({ storePath: store.storePath, ...(params.delivered !== undefined ? { delivered: params.delivered } : {}), + ...(params.error !== undefined ? { error: params.error } : {}), ...(params.onFinished ? { onFinished: params.onFinished } : {}), }); @@ -142,7 +153,7 @@ async function runIsolatedJobAndReadState(params: { describe("CronService persists delivered status", () => { it("persists lastDelivered=true when isolated job reports delivered", async () => { const updated = await runIsolatedJobAndReadState({ - job: buildIsolatedAgentTurnJob("delivered-true"), + job: buildAnnounceIsolatedAgentTurnJob("delivered-true"), delivered: true, }); expectSuccessfulCronRun(updated); @@ -153,7 +164,7 @@ describe("CronService persists delivered status", () => { it("persists lastDelivered=false when isolated job explicitly reports not delivered", async () => { const updated = await runIsolatedJobAndReadState({ - job: buildIsolatedAgentTurnJob("delivered-false"), + job: buildAnnounceIsolatedAgentTurnJob("delivered-false"), delivered: false, }); expectSuccessfulCronRun(updated); @@ -162,6 +173,27 @@ describe("CronService persists delivered status", () => { expect(updated?.state.lastDeliveryError).toBeUndefined(); }); + it("suppresses delivered=false when delivery.mode none opts out of delivery", async () => { + const updated = await runIsolatedJobAndReadState({ + job: buildIsolatedAgentTurnJob("delivery-none-delivered-false"), + delivered: false, + error: "Message failed", + }); + expectDeliveryNotRequested(updated); + }); + + it("preserves delivery errors when requested delivery reports not delivered", async () => { + const updated = await runIsolatedJobAndReadState({ + job: buildAnnounceIsolatedAgentTurnJob("delivery-requested-error"), + delivered: false, + error: "Message failed", + }); + expectSuccessfulCronRun(updated); + expect(updated?.state.lastDelivered).toBe(false); + expect(updated?.state.lastDeliveryStatus).toBe("not-delivered"); + expect(updated?.state.lastDeliveryError).toBe("Message failed"); + }); + it("persists not-requested delivery state when delivery is not configured", async () => { const updated = await runIsolatedJobAndReadState({ job: buildIsolatedAgentTurnJob("no-delivery"), @@ -171,10 +203,7 @@ describe("CronService persists delivered status", () => { it("persists unknown delivery state when delivery is requested but the runner omits delivered", async () => { const updated = await runIsolatedJobAndReadState({ - job: { - ...buildIsolatedAgentTurnJob("delivery-unknown"), - delivery: { mode: "announce", channel: "telegram", to: "123" }, - }, + job: buildAnnounceIsolatedAgentTurnJob("delivery-unknown"), }); expectSuccessfulCronRun(updated); expect(updated?.state.lastDelivered).toBeUndefined(); @@ -205,7 +234,7 @@ describe("CronService persists delivered status", () => { it("emits delivered in the finished event", async () => { let capturedEvent: { jobId: string; delivered?: boolean; deliveryStatus?: string } | undefined; await runIsolatedJobAndReadState({ - job: buildIsolatedAgentTurnJob("event-test"), + job: buildAnnounceIsolatedAgentTurnJob("event-test"), delivered: true, onFinished: (evt) => { capturedEvent = evt; diff --git a/src/cron/service/timer.ts b/src/cron/service/timer.ts index 29605100ca7..890b7574370 100644 --- a/src/cron/service/timer.ts +++ b/src/cron/service/timer.ts @@ -253,14 +253,20 @@ function resolveRetryConfig(cronConfig?: CronConfig) { }; } -function resolveDeliveryStatus(params: { job: CronJob; delivered?: boolean }): CronDeliveryStatus { +function resolveDeliveryState(params: { job: CronJob; delivered?: boolean }): { + delivered?: boolean; + status: CronDeliveryStatus; +} { + if (!resolveCronDeliveryPlan(params.job).requested) { + return { status: "not-requested" }; + } if (params.delivered === true) { - return "delivered"; + return { delivered: true, status: "delivered" }; } if (params.delivered === false) { - return "not-delivered"; + return { delivered: false, status: "not-delivered" }; } - return resolveCronDeliveryPlan(params.job).requested ? "unknown" : "not-requested"; + return { status: "unknown" }; } function normalizeCronMessageChannel(input: unknown): CronMessageChannel | undefined { @@ -416,11 +422,11 @@ export function applyJobResult( result.status === "error" && typeof result.error === "string" ? (resolveFailoverReasonFromError(result.error) ?? undefined) : undefined; - job.state.lastDelivered = result.delivered; - const deliveryStatus = resolveDeliveryStatus({ job, delivered: result.delivered }); - job.state.lastDeliveryStatus = deliveryStatus; + const deliveryState = resolveDeliveryState({ job, delivered: result.delivered }); + job.state.lastDelivered = deliveryState.delivered; + job.state.lastDeliveryStatus = deliveryState.status; job.state.lastDeliveryError = - deliveryStatus === "not-delivered" && result.error ? result.error : undefined; + deliveryState.status === "not-delivered" && result.error ? result.error : undefined; job.updatedAtMs = result.endedAt; // Track consecutive errors for backoff / auto-disable.