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>
This commit is contained in:
Yuzuru Suzuki
2026-03-02 22:27:57 +09:00
committed by GitHub
parent 16e85360a1
commit 6513c42d2d
3 changed files with 71 additions and 9 deletions

View File

@@ -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",

View File

@@ -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;
};