fix(cron): keep recovered tool warnings diagnostic

abnershang · abnershang · commit a2c65f070e2d · 2026-05-19T15:57:28.000+08:00
diff --git a/src/agents/pi-embedded-runner/run/payloads.errors.test.ts b/src/agents/pi-embedded-runner/run/payloads.errors.test.ts
@@ -1,5 +1,6 @@
 import type { AssistantMessage } from "@earendil-works/pi-ai";
 import { describe, expect, it } from "vitest";
+import { getReplyPayloadMetadata } from "../../../auto-reply/reply-payload.js";
 import { formatBillingErrorMessage } from "../../pi-embedded-helpers.js";
 import { makeAssistantMessageFixture } from "../../test-helpers/assistant-message-fixtures.js";
 import {
@@ -457,6 +458,9 @@ describe("buildEmbeddedRunPayloads", () => {
     expect(payloads[1]?.isError).toBe(true);
     expect(payloads[1]?.text).toContain("Write");
     expect(payloads[1]?.text).not.toContain("missing");
+    expect(getReplyPayloadMetadata(payloads[1] as object)?.nonTerminalToolErrorWarning).toBe(
+      undefined,
+    );
   });
 
   it("shows exec tool errors when assistant output claims success", () => {
@@ -474,6 +478,9 @@ describe("buildEmbeddedRunPayloads", () => {
     expect(payloads[1]?.isError).toBe(true);
     expect(payloads[1]?.text).toContain("Exec");
     expect(payloads[1]?.text).not.toContain("python: command not found");
+    expect(getReplyPayloadMetadata(payloads[1] as object)?.nonTerminalToolErrorWarning).toBe(
+      undefined,
+    );
   });
 
   it("shows mutating tool errors when assistant output does not acknowledge the failure", () => {
diff --git a/src/agents/pi-embedded-runner/run/payloads.test.ts b/src/agents/pi-embedded-runner/run/payloads.test.ts
@@ -351,6 +351,28 @@ describe("buildEmbeddedRunPayloads tool-error warnings", () => {
     });
   });
 
+  it("marks middleware tool-error warnings after assistant output as non-terminal", () => {
+    const payloads = buildPayloads({
+      assistantTexts: ["Queued 3 topics."],
+      lastToolError: {
+        toolName: "exec",
+        error: "Tool output unavailable due to post-processing error",
+        middlewareError: true,
+      },
+      verboseLevel: "off",
+    });
+
+    expect(payloads).toHaveLength(2);
+    expect(payloads[0]?.text).toBe("Queued 3 topics.");
+    expect(payloads[1]).toMatchObject({
+      isError: true,
+    });
+    expect(payloads[1]?.text).toContain("Exec failed");
+    expect(getReplyPayloadMetadata(payloads[1] as object)).toMatchObject({
+      nonTerminalToolErrorWarning: true,
+    });
+  });
+
   it("surfaces concise bash tool errors when verbose mode is off", () => {
     const payloads = buildPayloads({
       lastToolError: { toolName: "bash", error: "command failed" },
diff --git a/src/agents/pi-embedded-runner/run/payloads.ts b/src/agents/pi-embedded-runner/run/payloads.ts
@@ -136,6 +136,10 @@ function shouldIncludeToolErrorDetails(params: {
   );
 }
 
+function shouldMarkNonTerminalToolErrorWarning(lastToolError: ToolErrorSummary): boolean {
+  return lastToolError.middlewareError === true;
+}
+
 function resolveToolErrorWarningPolicy(params: {
   lastToolError: ToolErrorSummary;
   hasUserFacingReply: boolean;
@@ -221,6 +225,7 @@ export function buildEmbeddedRunPayloads(params: {
     presentation?: ReplyPayload["presentation"];
     interactive?: ReplyPayload["interactive"];
     channelData?: Record<string, unknown>;
+    nonTerminalToolErrorWarning?: boolean;
     sourceReplyMirror?: {
       idempotencyKey?: string;
     };
@@ -509,6 +514,9 @@ export function buildEmbeddedRunPayloads(params: {
         replyItems.push({
           text: warningText,
           isError: true,
+          nonTerminalToolErrorWarning:
+            hasUserFacingAssistantReply &&
+            shouldMarkNonTerminalToolErrorWarning(params.lastToolError),
         });
       }
     }
@@ -530,6 +538,11 @@ export function buildEmbeddedRunPayloads(params: {
       if (item.isError !== undefined) {
         payload.isError = item.isError;
       }
+      if (item.nonTerminalToolErrorWarning) {
+        setReplyPayloadMetadata(payload, {
+          nonTerminalToolErrorWarning: true,
+        });
+      }
       if (item.replyToId) {
         payload.replyToId = item.replyToId;
       }
diff --git a/src/agents/pi-embedded-subscribe.handlers.tools.test.ts b/src/agents/pi-embedded-subscribe.handlers.tools.test.ts
@@ -281,6 +281,47 @@ describe("handleToolExecutionEnd cron.add commitment tracking", () => {
 });
 
 describe("handleToolExecutionEnd mutating failure recovery", () => {
+  it("marks middleware failures on the last tool error", async () => {
+    const { ctx } = createTestContext();
+
+    await handleToolExecutionStart(
+      ctx as never,
+      {
+        type: "tool_execution_start",
+        toolName: "exec",
+        toolCallId: "tool-exec-middleware-error",
+        args: { cmd: "echo ok" },
+      } as never,
+    );
+
+    await handleToolExecutionEnd(
+      ctx as never,
+      {
+        type: "tool_execution_end",
+        toolName: "exec",
+        toolCallId: "tool-exec-middleware-error",
+        isError: false,
+        result: {
+          content: [
+            {
+              type: "text",
+              text: "Tool output unavailable due to post-processing error.",
+            },
+          ],
+          details: {
+            status: "error",
+            middlewareError: true,
+          },
+        },
+      } as never,
+    );
+
+    expect(ctx.state.lastToolError).toMatchObject({
+      toolName: "exec",
+      middlewareError: true,
+    });
+  });
+
   it("clears edit failure when the retry succeeds through common file path aliases", async () => {
     const { ctx } = createTestContext();
 
diff --git a/src/agents/pi-embedded-subscribe.handlers.tools.ts b/src/agents/pi-embedded-subscribe.handlers.tools.ts
@@ -67,6 +67,19 @@ const beforeToolCallModuleLoader = createLazyImportLoader<BeforeToolCallModule>(
 const LIVE_EXEC_OUTPUT_MAX_CHARS = 8000;
 const LIVE_EXEC_UPDATE_MIN_INTERVAL_MS = 250;
 
+function isMiddlewareToolResultError(result: unknown): boolean {
+  if (!result || typeof result !== "object") {
+    return false;
+  }
+  const details = (result as { details?: unknown }).details;
+  return Boolean(
+    details &&
+    typeof details === "object" &&
+    !Array.isArray(details) &&
+    (details as { middlewareError?: unknown }).middlewareError === true,
+  );
+}
+
 function loadExecApprovalReply(): Promise<ExecApprovalReplyModule> {
   return execApprovalReplyModuleLoader.load();
 }
@@ -939,6 +952,7 @@ export async function handleToolExecutionEnd(
       meta,
       error: errorMessage,
       timedOut: isToolResultTimedOut(sanitizedResult) || undefined,
+      middlewareError: isMiddlewareToolResultError(sanitizedResult) || undefined,
       mutatingAction: callSummary?.mutatingAction,
       actionFingerprint: callSummary?.actionFingerprint,
       fileTarget: callSummary?.fileTarget,
diff --git a/src/agents/tool-error-summary.ts b/src/agents/tool-error-summary.ts
@@ -6,6 +6,7 @@ export type ToolErrorSummary = {
   meta?: string;
   error?: string;
   timedOut?: boolean;
+  middlewareError?: boolean;
   mutatingAction?: boolean;
   actionFingerprint?: string;
   fileTarget?: FileTarget;
diff --git a/src/auto-reply/reply-payload.ts b/src/auto-reply/reply-payload.ts
@@ -163,6 +163,8 @@ export type ReplyPayloadMetadata = {
     idempotencyKey?: string;
   };
   beforeAgentRunBlocked?: boolean;
+  /** Warning synthesized from an observed tool error after the run produced assistant output. */
+  nonTerminalToolErrorWarning?: boolean;
 };
 
 const replyPayloadMetadata = new WeakMap<object, ReplyPayloadMetadata>();
diff --git a/src/cron/isolated-agent.helpers.test.ts b/src/cron/isolated-agent.helpers.test.ts
@@ -1,4 +1,5 @@
 import { describe, expect, it } from "vitest";
+import { setReplyPayloadMetadata } from "../auto-reply/reply-payload.js";
 import { detectCronDenialToken, resolveCronPayloadOutcome } from "./isolated-agent/helpers.js";
 
 describe("detectCronDenialToken", () => {
@@ -65,6 +66,51 @@ describe("resolveCronPayloadOutcome", () => {
     expect(result.summary).toBe("Write completed successfully.");
   });
 
+  it("keeps non-terminal tool warnings diagnostic when final assistant output succeeded", () => {
+    const toolWarning = setReplyPayloadMetadata(
+      {
+        text: "⚠️ Exec failed",
+        isError: true,
+      },
+      { nonTerminalToolErrorWarning: true },
+    );
+
+    const result = resolveCronPayloadOutcome({
+      payloads: [{ text: "Queued 3 topics." }, toolWarning],
+      finalAssistantVisibleText: "Queued 3 topics.",
+      preferFinalAssistantVisibleText: true,
+    });
+
+    expect(result.hasFatalErrorPayload).toBe(false);
+    expect(result.embeddedRunError).toBeUndefined();
+    expect(result.summary).toBe("Queued 3 topics.");
+    expect(result.outputText).toBe("Queued 3 topics.");
+    expect(result.deliveryPayloads).toEqual([{ text: "Queued 3 topics." }]);
+  });
+
+  it("keeps marked middleware warnings diagnostic after structured cron output", () => {
+    const mediaPayload = { mediaUrl: "file:///tmp/cron-report.png" };
+    const toolWarning = setReplyPayloadMetadata(
+      {
+        text: "⚠️ Exec failed",
+        isError: true,
+      },
+      { nonTerminalToolErrorWarning: true },
+    );
+
+    const result = resolveCronPayloadOutcome({
+      payloads: [mediaPayload, toolWarning],
+    });
+
+    expect(result.hasFatalErrorPayload).toBe(false);
+    expect(result.embeddedRunError).toBeUndefined();
+    expect(result.summary).toBeUndefined();
+    expect(result.outputText).toBeUndefined();
+    expect(result.synthesizedText).toBeUndefined();
+    expect(result.deliveryPayloads).toEqual([mediaPayload]);
+    expect(result.deliveryPayloadHasStructuredContent).toBe(true);
+  });
+
   it("treats trailing message delivery warnings as non-fatal when final assistant text exists", () => {
     const result = resolveCronPayloadOutcome({
       payloads: [{ text: "Draft output" }, { text: "⚠️ ✉️ Message failed", isError: true }],
diff --git a/src/cron/isolated-agent/helpers.ts b/src/cron/isolated-agent/helpers.ts
@@ -1,5 +1,6 @@
 import { hasOutboundReplyContent } from "openclaw/plugin-sdk/reply-payload";
 import { DEFAULT_HEARTBEAT_ACK_MAX_CHARS } from "../../auto-reply/heartbeat.js";
+import { getReplyPayloadMetadata } from "../../auto-reply/reply-payload.js";
 import type { ReplyPayload } from "../../auto-reply/reply-payload.js";
 import { normalizeOptionalString } from "../../shared/string-coerce.js";
 import { truncateUtf16Safe } from "../../utils.js";
@@ -151,6 +152,9 @@ export function pickSummaryFromPayloads(
     }
   }
   for (let i = payloads.length - 1; i >= 0; i--) {
+    if (isNonTerminalToolErrorWarning(payloads[i])) {
+      continue;
+    }
     const summary = pickSummaryFromOutput(payloads[i]?.text);
     if (summary) {
       return summary;
@@ -172,6 +176,9 @@ export function pickLastNonEmptyTextFromPayloads(
     }
   }
   for (let i = payloads.length - 1; i >= 0; i--) {
+    if (isNonTerminalToolErrorWarning(payloads[i])) {
+      continue;
+    }
     const clean = (payloads[i]?.text ?? "").trim();
     if (clean) {
       return clean;
@@ -249,14 +256,26 @@ function isCronMessagePresentationWarning(text: string | undefined): boolean {
   );
 }
 
+function isNonTerminalToolErrorWarning(payload: object | undefined): boolean {
+  return Boolean(payload && getReplyPayloadMetadata(payload)?.nonTerminalToolErrorWarning);
+}
+
+function isSuccessfulCronPayload(payload: DeliveryPayload | undefined): boolean {
+  return (
+    payload?.isError !== true &&
+    (isDeliverablePayload(payload) || payloadHasStructuredDeliveryContent(payload))
+  );
+}
+
 export function resolveCronPayloadOutcome(params: {
   payloads: DeliveryPayload[];
   runLevelError?: unknown;
   failureSignal?: CronFailureSignal | undefined;
   finalAssistantVisibleText?: string | undefined;
   preferFinalAssistantVisibleText?: boolean;
 }): CronPayloadOutcome {
-  const firstText = params.payloads[0]?.text ?? "";
+  const firstText =
+    params.payloads.find((payload) => !isNonTerminalToolErrorWarning(payload))?.text ?? "";
   const fallbackSummary =
     pickSummaryFromPayloads(params.payloads) ?? pickSummaryFromOutput(firstText);
   const fallbackOutputText = pickLastNonEmptyTextFromPayloads(params.payloads);
@@ -277,23 +296,33 @@ export function resolveCronPayloadOutcome(params: {
   const hasSuccessfulPayloadAfterLastError =
     !params.runLevelError &&
     lastErrorPayloadIndex >= 0 &&
-    params.payloads
-      .slice(lastErrorPayloadIndex + 1)
-      .some((payload) => payload?.isError !== true && Boolean(payload?.text?.trim()));
+    params.payloads.slice(lastErrorPayloadIndex + 1).some(isSuccessfulCronPayload);
   const hasSuccessfulPayloadBeforeLastError =
     !params.runLevelError &&
     lastErrorPayloadIndex > 0 &&
-    params.payloads
-      .slice(0, lastErrorPayloadIndex)
-      .some((payload) => payload?.isError !== true && Boolean(payload?.text?.trim()));
+    params.payloads.slice(0, lastErrorPayloadIndex).some(isSuccessfulCronPayload);
+  const lastErrorPayload =
+    lastErrorPayloadIndex >= 0 ? params.payloads[lastErrorPayloadIndex] : undefined;
+  const hasRecoveringTerminalOutput =
+    normalizedFinalAssistantVisibleText !== undefined ||
+    hasSuccessfulPayloadAfterLastError ||
+    hasSuccessfulPayloadBeforeLastError;
+  const hasNonTerminalToolErrorWarning =
+    !params.runLevelError &&
+    params.failureSignal?.fatalForCron !== true &&
+    hasRecoveringTerminalOutput &&
+    isNonTerminalToolErrorWarning(lastErrorPayload);
   const hasPendingPresentationWarning =
     !params.runLevelError &&
     params.failureSignal?.fatalForCron !== true &&
     lastErrorPayloadIndex >= 0 &&
     isCronMessagePresentationWarning(lastErrorPayloadText) &&
     (normalizedFinalAssistantVisibleText !== undefined || hasSuccessfulPayloadBeforeLastError);
   const hasFatalStructuredErrorPayload =
-    hasErrorPayload && !hasSuccessfulPayloadAfterLastError && !hasPendingPresentationWarning;
+    hasErrorPayload &&
+    !hasSuccessfulPayloadAfterLastError &&
+    !hasPendingPresentationWarning &&
+    !hasNonTerminalToolErrorWarning;
   const hasStructuredDeliveryPayloads = selectedDeliveryPayloads.some((payload) =>
     payloadHasStructuredDeliveryContent(payload),
   );
@@ -324,10 +353,13 @@ export function resolveCronPayloadOutcome(params: {
     { field: "synthesizedText", text: synthesizedText },
     { field: "fallbackSummary", text: fallbackSummary },
     { field: "fallbackOutputText", text: fallbackOutputText },
-    ...params.payloads.map((payload, index) => ({
-      field: `payloads[${index}].text`,
-      text: payload?.text,
-    })),
+    ...params.payloads
+      .map((payload, index) => ({ payload, index }))
+      .filter(({ payload }) => !isNonTerminalToolErrorWarning(payload))
+      .map(({ payload, index }) => ({
+        field: `payloads[${index}].text`,
+        text: payload?.text,
+      })),
   ]);
   const failureSignal = normalizeCronFailureSignal(params.failureSignal);
   const runLevelError = formatCronRunLevelError(params.runLevelError);
diff --git a/src/cron/run-diagnostics.test.ts b/src/cron/run-diagnostics.test.ts
diff --git a/src/cron/run-diagnostics.ts b/src/cron/run-diagnostics.ts
