fix(cron): preserve isolated success on delivery failure

yu-xin-c · yu-xin-c · commit 291dc8e6896c · 2026-06-17T19:31:29.000+08:00
diff --git a/src/cron/isolated-agent/run.message-tool-policy.test.ts b/src/cron/isolated-agent/run.message-tool-policy.test.ts
@@ -1212,6 +1212,76 @@ describe("runCronIsolatedAgentTurn message tool policy", () => {
     });
   });
 
+  it("keeps the isolated run successful when post-run delivery fails", async () => {
+    mockRunCronFallbackPassthrough();
+    resolveCronDeliveryPlanMock.mockReturnValue(makeAnnounceDeliveryPlan());
+    resolveCronPayloadOutcomeMock.mockReturnValue({
+      summary: "Final cron report",
+      outputText: "Final cron report",
+      synthesizedText: "Final cron report",
+      deliveryPayload: { text: "Final cron report" },
+      deliveryPayloads: [{ text: "Final cron report" }],
+      deliveryPayloadHasStructuredContent: false,
+      hasFatalErrorPayload: false,
+      hasFatalStructuredErrorPayload: false,
+      embeddedRunError: undefined,
+    });
+    dispatchCronDeliveryMock.mockImplementationOnce(
+      (params: {
+        withRunSession: (result: {
+          status: "error";
+          summary: string;
+          outputText: string;
+          error: string;
+          deliveryAttempted: true;
+        }) => unknown;
+      }) => ({
+        result: params.withRunSession({
+          status: "error",
+          summary: "Final cron report",
+          outputText: "Final cron report",
+          error: "Message failed",
+          deliveryAttempted: true,
+        }),
+        delivered: false,
+        deliveryAttempted: true,
+        summary: "Final cron report",
+        outputText: "Final cron report",
+        synthesizedText: "Final cron report",
+        deliveryPayloads: [{ text: "Final cron report" }],
+      }),
+    );
+
+    const result = await runCronIsolatedAgentTurn({
+      ...makeParams(),
+      job: makeAnnounceMessageToolJob({
+        id: "delivery-failure-after-success",
+        name: "Delivery Failure After Success",
+      }),
+    });
+
+    expect(result.status).toBe("ok");
+    expect(result.error).toBe("Message failed");
+    expect(result.delivered).toBe(false);
+    expect(result.deliveryAttempted).toBe(true);
+    expectDeliveryFields(result.delivery, {
+      intended: { channel: "messagechat", to: "123", source: "explicit" },
+      resolved: { ok: true, channel: "messagechat", to: "123", source: "explicit" },
+      fallbackUsed: true,
+      delivered: false,
+    });
+    expect(result.diagnostics?.summary).toBe("Message failed");
+    expect(result.diagnostics?.entries).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          source: "delivery",
+          severity: "error",
+          message: "Message failed",
+        }),
+      ]),
+    );
+  });
+
   it("rewrites generic message provider to resolved channel in delivery trace", async () => {
     mockRunCronFallbackPassthrough();
     resolveCronDeliveryPlanMock.mockReturnValue(makeAnnounceDeliveryPlan());
diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts
@@ -1178,12 +1178,17 @@ async function finalizeCronRun(params: {
     delivered?: boolean;
     deliveryAttempted?: boolean;
     delivery?: CronDeliveryTrace;
-  }) =>
-    prepared.withRunSession({
+    deliveryError?: string;
+    deliveryDiagnostics?: RunCronAgentTurnResult["diagnostics"];
+  }) => {
+    const deliveryError = normalizeOptionalString(result?.deliveryError);
+    return prepared.withRunSession({
       status: hasFatalErrorPayload ? "error" : "ok",
       ...(hasFatalErrorPayload
         ? { error: embeddedRunError ?? "cron isolated run returned an error payload" }
-        : {}),
+        : deliveryError
+          ? { error: deliveryError }
+          : {}),
       summary,
       outputText,
       delivered: result?.delivered,
@@ -1192,14 +1197,16 @@ async function finalizeCronRun(params: {
       diagnostics: hasFatalErrorPayload
         ? mergeCronRunDiagnostics(
             agentDiagnostics,
+            result?.deliveryDiagnostics,
             createCronRunDiagnosticsFromError(
               "agent-run",
               embeddedRunError ?? "cron isolated run returned an error payload",
             ),
           )
-        : agentDiagnostics,
+        : mergeCronRunDiagnostics(agentDiagnostics, result?.deliveryDiagnostics),
       ...telemetry,
     });
+  };
   const failPendingPresentationWarningUnlessDelivered = (delivered?: boolean) => {
     if (pendingPresentationWarningError && delivered !== true) {
       hasFatalErrorPayload = true;
@@ -1295,13 +1302,26 @@ async function finalizeCronRun(params: {
     failPendingPresentationWarningUnlessDelivered(
       resultWithDeliveryMeta.delivered ?? deliveryResult.delivered,
     );
-    if (!hasFatalErrorPayload || deliveryResult.result.status !== "ok") {
+    if (!hasFatalErrorPayload) {
+      if (deliveryResult.result.status === "error" && !params.isAborted()) {
+        return resolveRunOutcome({
+          delivered: resultWithDeliveryMeta.delivered ?? deliveryResult.delivered,
+          deliveryAttempted: resultWithDeliveryMeta.deliveryAttempted,
+          delivery: deliveryTrace,
+          deliveryError: deliveryResult.result.error,
+          deliveryDiagnostics: resultWithDeliveryMeta.diagnostics,
+        });
+      }
+      return resultWithDeliveryMeta;
+    }
+    if (deliveryResult.result.status !== "ok") {
       return resultWithDeliveryMeta;
     }
     return resolveRunOutcome({
       delivered: deliveryResult.result.delivered,
       deliveryAttempted: resultWithDeliveryMeta.deliveryAttempted,
       delivery: deliveryTrace,
+      deliveryDiagnostics: resultWithDeliveryMeta.diagnostics,
     });
   }
   summary = deliveryResult.summary;
