From 6513c42d2d638091f1789a595fbac2e18d9306d7 Mon Sep 17 00:00:00 2001 From: Yuzuru Suzuki Date: Mon, 2 Mar 2026 22:27:57 +0900 Subject: [PATCH] fix(cron): treat announce delivery failure as ok when execution succeeded (#31082) * cron: treat announce delivery failure as ok when agent execution succeeded * fix: set delivered:false and error on announce delivery failure paths * Changelog: note cron announce delivery status handling (#31082) --------- Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> --- CHANGELOG.md | 1 + ...p-recipient-besteffortdeliver-true.test.ts | 63 +++++++++++++++++-- src/cron/isolated-agent/delivery-dispatch.ts | 16 +++-- 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4aaaf4be..793880a17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -261,6 +261,7 @@ Docs: https://docs.openclaw.ai - Cron/Isolated model defaults: resolve isolated cron `subagents.model` (including object-form `primary`) through allowlist-aware model selection so isolated cron runs honor subagent model defaults unless explicitly overridden by job payload model. (#11474) Thanks @AnonO6. - Cron/Isolated sessions list: persist the intended pre-run model/provider on isolated cron session entries so `sessions_list` reflects payload/session model overrides even when runs fail before post-run telemetry persistence. (#21279) Thanks @altaywtf. - Cron tool/update flat params: recover top-level update patch fields when models omit the `patch` wrapper, and allow flattened update keys through tool input schema validation so `cron.update` no longer fails with `patch required` for valid flat payloads. (#23221) +- Cron/Announce delivery status: keep isolated cron runs in `ok` state when execution succeeds but announce delivery fails (for example transient `pairing required`), while preserving `delivered=false` and delivery error context for visibility. (#31082) Thanks @YuzuruS. - Agents/Message tool scoping: include other configured channels in scoped `message` tool action enum + description so isolated/cron runs can discover and invoke cross-channel actions without schema validation failures. Landed from contributor PR #20840 by @altaywtf. Thanks @altaywtf. - Web UI/Chat sessions: add a cron-session visibility toggle in the session selector, fix cron-key detection across `cron:*` and `agent:*:cron:*` formats, and localize the new control labels/tooltips. (#26976) Thanks @ianderrington. - Web UI/Cron jobs: add schedule-kind and last-run-status filters to the Jobs list, with reset control and client-side filtering over loaded results. (#9510) Thanks @guxu11. diff --git a/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts b/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts index 265b89a22..6879d37d0 100644 --- a/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts +++ b/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts @@ -317,11 +317,33 @@ describe("runCronIsolatedAgentTurn", () => { }); }); - it("fails when announce delivery reports false and best-effort is disabled", async () => { - const { res, deps } = await runAnnounceFlowResult(false); - expect(res.status).toBe("error"); - expect(res.error).toContain("cron announce delivery failed"); - expect(deps.sendMessageTelegram).not.toHaveBeenCalled(); + it("returns ok when announce delivery reports false and best-effort is disabled", async () => { + await withTempCronHome(async (home) => { + const storePath = await writeSessionStore(home, { lastProvider: "webchat", lastTo: "" }); + const deps = createCliDeps(); + mockAgentPayloads([{ text: "hello from cron" }]); + vi.mocked(runSubagentAnnounceFlow).mockResolvedValueOnce(false); + + const res = await runTelegramAnnounceTurn({ + home, + storePath, + deps, + delivery: { + mode: "announce", + channel: "telegram", + to: "123", + bestEffort: false, + }, + }); + + // Announce delivery failure should not mark a successful agent execution + // as error. The execution succeeded; only delivery failed. + expect(res.status).toBe("ok"); + expect(res.delivered).toBe(false); + expect(res.deliveryAttempted).toBe(true); + expect(res.error).toBe("cron announce delivery failed"); + expect(deps.sendMessageTelegram).not.toHaveBeenCalled(); + }); }); it("marks attempted when announce delivery reports false and best-effort is enabled", async () => { @@ -333,6 +355,37 @@ describe("runCronIsolatedAgentTurn", () => { expect(deps.sendMessageTelegram).not.toHaveBeenCalled(); }); + it("returns ok when announce flow throws and best-effort is disabled", async () => { + await withTempCronHome(async (home) => { + const storePath = await writeSessionStore(home, { lastProvider: "webchat", lastTo: "" }); + const deps = createCliDeps(); + mockAgentPayloads([{ text: "hello from cron" }]); + vi.mocked(runSubagentAnnounceFlow).mockRejectedValueOnce( + new Error("gateway closed (1008): pairing required"), + ); + + const res = await runTelegramAnnounceTurn({ + home, + storePath, + deps, + delivery: { + mode: "announce", + channel: "telegram", + to: "123", + bestEffort: false, + }, + }); + + // Even when announce throws (e.g. "pairing required"), the agent + // execution succeeded so the job status should be ok. + expect(res.status).toBe("ok"); + expect(res.delivered).toBe(false); + expect(res.deliveryAttempted).toBe(true); + expect(res.error).toContain("pairing required"); + expect(deps.sendMessageTelegram).not.toHaveBeenCalled(); + }); + }); + it("ignores structured direct delivery failures when best-effort is enabled", async () => { await expectBestEffortTelegramNotDelivered({ text: "hello from cron", diff --git a/src/cron/isolated-agent/delivery-dispatch.ts b/src/cron/isolated-agent/delivery-dispatch.ts index 2c6748a99..db8f03d38 100644 --- a/src/cron/isolated-agent/delivery-dispatch.ts +++ b/src/cron/isolated-agent/delivery-dispatch.ts @@ -326,31 +326,39 @@ export async function dispatchCronDelivery( if (didAnnounce) { delivered = true; } else { + // Announce delivery failed but the agent execution itself succeeded. + // Return ok so the job isn't penalized for a transient delivery issue + // (e.g. "pairing required" when no active client session exists). + // Delivery failure is tracked separately via delivered/deliveryAttempted. const message = "cron announce delivery failed"; + logWarn(`[cron:${params.job.id}] ${message}`); if (!params.deliveryBestEffort) { return params.withRunSession({ - status: "error", + status: "ok", summary, outputText, error: message, + delivered: false, deliveryAttempted, ...params.telemetry, }); } - logWarn(`[cron:${params.job.id}] ${message}`); } } catch (err) { + // Same as above: announce delivery errors should not mark a successful + // agent execution as failed. + logWarn(`[cron:${params.job.id}] ${String(err)}`); if (!params.deliveryBestEffort) { return params.withRunSession({ - status: "error", + status: "ok", summary, outputText, error: String(err), + delivered: false, deliveryAttempted, ...params.telemetry, }); } - logWarn(`[cron:${params.job.id}] ${String(err)}`); } return null; };