fix(discord): suppress recovered tool warnings (#87451)

2026-08-05 17:41:27 +00:00 · 2026-05-28 00:32:28 +01:00
parent 3f9d2415ac
commit da279041ab
4 changed files with 121 additions and 40 deletions
--- a/extensions/discord/src/monitor/message-handler.process.test.ts
+++ b/extensions/discord/src/monitor/message-handler.process.test.ts
@@ -7,6 +7,7 @@ import {
 import type { ReplyPayload } from "openclaw/plugin-sdk/reply-dispatch-runtime";
 import * as runtimeEnvModule from "openclaw/plugin-sdk/runtime-env";
 import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
+import { setReplyPayloadMetadata } from "../../../../src/auto-reply/reply-payload.js";
 import type { DiscordMessagePreflightContext } from "./message-handler.preflight.js";

 const sendMocks = vi.hoisted(() => ({
@@ -51,6 +52,16 @@ const editMessageDiscord = deliveryMocks.editMessageDiscord;
 const deliverDiscordReply = deliveryMocks.deliverDiscordReply;
 const createDiscordDraftStream = deliveryMocks.createDiscordDraftStream;

+function createNonTerminalToolWarningPayload(): ReplyPayload {
+  return setReplyPayloadMetadata(
+    {
+      text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
+      isError: true,
+    },
+    { nonTerminalToolErrorWarning: true },
+  );
+}
+
 vi.mock("../send.js", () => ({
  reactMessageDiscord: async (
    channelId: string,
@@ -2237,14 +2248,11 @@ describe("processDiscordMessage draft streaming", () => {
    expect(deliverDiscordReply).toHaveBeenCalledTimes(1);
  });

-  it("keeps finalized previews when later tool warning finals are delivered", async () => {
+  it("drops later tool warning finals after preview final replies", async () => {
    const draftStream = createMockDraftStreamForTest();
    dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
      await params?.dispatcher.sendFinalReply({ text: "delivery survived" });
-      await params?.dispatcher.sendFinalReply({
-        text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
-        isError: true,
-      } as never);
+      await params?.dispatcher.sendFinalReply(createNonTerminalToolWarningPayload());
      return { queuedFinal: true, counts: { final: 2, tool: 0, block: 0 } };
    });

@@ -2257,24 +2265,13 @@ describe("processDiscordMessage draft streaming", () => {
    expectPreviewEditContent("delivery survived");
    expect(draftStream.clear).not.toHaveBeenCalled();
    expect(draftStream.messageId()).toBe("preview-1");
-    expect(deliverDiscordReply).toHaveBeenCalledTimes(1);
-    expect(firstMockArg(deliverDiscordReply, "deliverDiscordReply")).toMatchObject({
-      replies: [
-        {
-          text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
-          isError: true,
-        },
-      ],
-    });
+    expect(deliverDiscordReply).not.toHaveBeenCalled();
  });

-  it("keeps draft previews when tool warning finals arrive before recovered replies", async () => {
+  it("drops earlier tool warning finals when recovered replies arrive", async () => {
    const draftStream = createMockDraftStreamForTest();
    dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
-      await params?.dispatcher.sendFinalReply({
-        text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
-        isError: true,
-      } as never);
+      await params?.dispatcher.sendFinalReply(createNonTerminalToolWarningPayload());
      await params?.dispatcher.sendFinalReply({ text: "delivery recovered" });
      return { queuedFinal: true, counts: { final: 2, tool: 0, block: 0 } };
    });
@@ -2288,6 +2285,24 @@ describe("processDiscordMessage draft streaming", () => {
    expectPreviewEditContent("delivery recovered");
    expect(draftStream.clear).not.toHaveBeenCalled();
    expect(draftStream.messageId()).toBe("preview-1");
+    expect(deliverDiscordReply).not.toHaveBeenCalled();
+  });
+
+  it("delivers tool warning finals when no recovered reply is available", async () => {
+    const draftStream = createMockDraftStreamForTest();
+    dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
+      await params?.dispatcher.sendFinalReply(createNonTerminalToolWarningPayload());
+      return { queuedFinal: true, counts: { final: 1, tool: 0, block: 0 } };
+    });
+
+    const ctx = await createAutomaticSourceDeliveryContext({
+      discordConfig: { streamMode: "partial", maxLinesPerMessage: 5 },
+    });
+
+    await runProcessDiscordMessage(ctx);
+
+    expect(editMessageDiscord).not.toHaveBeenCalled();
+    expect(draftStream.clear).toHaveBeenCalledTimes(1);
    expect(deliverDiscordReply).toHaveBeenCalledTimes(1);
    expect(firstMockArg(deliverDiscordReply, "deliverDiscordReply")).toMatchObject({
      replies: [
@@ -2299,6 +2314,36 @@ describe("processDiscordMessage draft streaming", () => {
    });
  });

+  it("keeps mutating tool warning finals after successful-looking replies", async () => {
+    const draftStream = createMockDraftStreamForTest();
+    dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
+      await params?.dispatcher.sendFinalReply({ text: "Done." });
+      await params?.dispatcher.sendFinalReply({
+        text: "⚠️ 🛠️ `write file (agent)` failed",
+        isError: true,
+      } as never);
+      return { queuedFinal: true, counts: { final: 2, tool: 0, block: 0 } };
+    });
+
+    const ctx = await createAutomaticSourceDeliveryContext({
+      discordConfig: { streamMode: "partial", maxLinesPerMessage: 5 },
+    });
+
+    await runProcessDiscordMessage(ctx);
+
+    expectPreviewEditContent("Done.");
+    expect(draftStream.clear).not.toHaveBeenCalled();
+    expect(deliverDiscordReply).toHaveBeenCalledTimes(1);
+    expect(firstMockArg(deliverDiscordReply, "deliverDiscordReply")).toMatchObject({
+      replies: [
+        {
+          text: "⚠️ 🛠️ `write file (agent)` failed",
+          isError: true,
+        },
+      ],
+    });
+  });
+
  it("suppresses reasoning payload delivery to Discord", async () => {
    mockDispatchSingleBlockReply({ text: "thinking...", isReasoning: true });
    await processStreamOffDiscordMessage();
@@ -2448,17 +2493,14 @@ describe("processDiscordMessage draft streaming", () => {
    expectPreviewEditContent("done");
  });

-  it("keeps finalized progress previews when later tool warning finals are delivered", async () => {
+  it("drops later tool warning finals after progress preview final replies", async () => {
    const draftStream = createMockDraftStreamForTest();

    dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
      await params?.replyOptions?.onToolStart?.({ name: "exec", phase: "start" });
      await params?.replyOptions?.onItemEvent?.({ progressText: "exec done" });
      await params?.dispatcher.sendFinalReply({ text: "delivery survived" });
-      await params?.dispatcher.sendFinalReply({
-        text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
-        isError: true,
-      } as never);
+      await params?.dispatcher.sendFinalReply(createNonTerminalToolWarningPayload());
      return { queuedFinal: true, counts: { final: 2, tool: 0, block: 0 } };
    });

@@ -2479,15 +2521,7 @@ describe("processDiscordMessage draft streaming", () => {
    expectPreviewEditContent("delivery survived");
    expect(draftStream.clear).not.toHaveBeenCalled();
    expect(draftStream.messageId()).toBe("preview-1");
-    expect(deliverDiscordReply).toHaveBeenCalledTimes(1);
-    expect(firstMockArg(deliverDiscordReply, "deliverDiscordReply")).toMatchObject({
-      replies: [
-        {
-          text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
-          isError: true,
-        },
-      ],
-    });
+    expect(deliverDiscordReply).not.toHaveBeenCalled();
  });

  it("uses raw tool-progress detail in Discord progress drafts", async () => {
--- a/extensions/discord/src/monitor/message-handler.process.ts
+++ b/extensions/discord/src/monitor/message-handler.process.ts
@@ -37,6 +37,7 @@ import { createChannelHistoryWindow } from "openclaw/plugin-sdk/reply-history";
 import {
  buildTtsSupplementMediaPayload,
  getReplyPayloadTtsSupplement,
+  isReplyPayloadNonTerminalToolErrorWarning,
  resolveSendableOutboundReplyParts,
 } from "openclaw/plugin-sdk/reply-payload";
 import type { ReplyDispatchKind, ReplyPayload } from "openclaw/plugin-sdk/reply-runtime";
@@ -104,6 +105,13 @@ function formatDiscordReplyDeliveryFailure(params: {
  return `discord ${params.kind} reply failed (${context}): ${String(params.err)}`;
 }

+function isFallbackOnlyToolWarningFinal(payload: ReplyPayload): boolean {
+  if (payload.isError !== true || !isReplyPayloadNonTerminalToolErrorWarning(payload)) {
+    return false;
+  }
+  return !resolveSendableOutboundReplyParts(payload).hasMedia;
+}
+
 type DiscordReplySkipReason = "aborted before delivery" | "reasoning payload";

 export function formatDiscordReplySkip(params: {
@@ -537,6 +545,16 @@ export async function processDiscordMessage(
    draftPreview.markFinalReplyStarted();
    observer?.onFinalReplyStart?.();
  };
+  let userFacingFinalDelivered = false;
+  let pendingToolWarningFinal:
+    | { payload: ReplyPayload; info: { kind: ReplyDispatchKind } }
+    | undefined;
+  const markUserFacingFinalDelivered = () => {
+    userFacingFinalDelivered = true;
+    pendingToolWarningFinal = undefined;
+    draftPreview.markFinalReplyDelivered();
+    observer?.onFinalReplyDelivered?.();
+  };
  const beforeDiscordPayloadDelivery = (
    payload: ReplyPayload,
    info: { kind: ReplyDispatchKind },
@@ -570,7 +588,7 @@ export async function processDiscordMessage(
        return null;
      }
    }
-    if (info.kind === "final") {
+    if (info.kind === "final" && !isFallbackOnlyToolWarningFinal(payload)) {
      draftPreview.markFinalReplyStarted();
    }
    return payload;
@@ -579,6 +597,7 @@ export async function processDiscordMessage(
  const deliverDiscordPayload = async (
    payload: ReplyPayload,
    info: { kind: ReplyDispatchKind },
+    options?: { allowFallbackOnlyToolWarning?: boolean },
  ) => {
    if (isProcessAborted(abortSignal)) {
      // Surface so operators don't chase missing replies when an abort
@@ -606,6 +625,16 @@ export async function processDiscordMessage(
      );
      return { visibleReplySent: false };
    }
+    if (
+      isFinal &&
+      !options?.allowFallbackOnlyToolWarning &&
+      isFallbackOnlyToolWarningFinal(payload)
+    ) {
+      if (!userFacingFinalDelivered) {
+        pendingToolWarningFinal = { payload, info };
+      }
+      return { visibleReplySent: false };
+    }
    if (isFinal) {
      draftPreview.markFinalReplyStarted();
    }
@@ -679,10 +708,9 @@ export async function processDiscordMessage(
            });
          },
          onPreviewFinalized: () => {
-            draftPreview.markFinalReplyDelivered();
+            markUserFacingFinalDelivered();
            draftPreview.markPreviewFinalized();
            replyReference.markSent();
-            observer?.onFinalReplyDelivered?.();
          },
          buildSupplementalPayload: () =>
            ttsSupplement ? buildTtsSupplementMediaPayload(effectivePayload) : undefined,
@@ -759,9 +787,8 @@ export async function processDiscordMessage(
          return true;
        },
        onNormalDelivered: () => {
-          draftPreview.markFinalReplyDelivered();
+          markUserFacingFinalDelivered();
          replyReference.markSent();
-          observer?.onFinalReplyDelivered?.();
        },
      });
      if (result.kind !== "normal-skipped") {
@@ -807,8 +834,7 @@ export async function processDiscordMessage(
    });
    replyReference.markSent();
    if (isFinal && payload.isError !== true) {
-      draftPreview.markFinalReplyDelivered();
-      observer?.onFinalReplyDelivered?.();
+      markUserFacingFinalDelivered();
    }
    return { visibleReplySent: true };
  };
@@ -837,6 +863,21 @@ export async function processDiscordMessage(
    null;
  let dispatchError = false;
  let dispatchAborted = false;
+  const deliverPendingToolWarningFinalIfNeeded = async () => {
+    if (!pendingToolWarningFinal || userFacingFinalDelivered || isProcessAborted(abortSignal)) {
+      return;
+    }
+    const pending = pendingToolWarningFinal;
+    pendingToolWarningFinal = undefined;
+    try {
+      await deliverDiscordPayload(pending.payload, pending.info, {
+        allowFallbackOnlyToolWarning: true,
+      });
+    } catch (err) {
+      dispatchError = true;
+      onDiscordDeliveryError(err, pending.info);
+    }
+  };
  try {
    if (isProcessAborted(abortSignal)) {
      dispatchAborted = true;
@@ -1026,6 +1067,7 @@ export async function processDiscordMessage(
      dispatchAborted = true;
      return;
    }
+    await deliverPendingToolWarningFinalIfNeeded();
  } catch (err) {
    if (isProcessAborted(abortSignal)) {
      dispatchAborted = true;
--- a/src/auto-reply/reply-payload.ts
+++ b/src/auto-reply/reply-payload.ts
@@ -184,6 +184,10 @@ export function getReplyPayloadMetadata(payload: object): ReplyPayloadMetadata |
  return replyPayloadMetadata.get(payload);
 }

+export function isReplyPayloadNonTerminalToolErrorWarning(payload: object): boolean {
+  return getReplyPayloadMetadata(payload)?.nonTerminalToolErrorWarning === true;
+}
+
 export function copyReplyPayloadMetadata<T extends object>(source: object, payload: T): T {
  const metadata = getReplyPayloadMetadata(source);
  return metadata ? setReplyPayloadMetadata(payload, metadata) : payload;
--- a/src/plugin-sdk/reply-payload.ts
+++ b/src/plugin-sdk/reply-payload.ts
@@ -13,6 +13,7 @@ export type { ReplyPayloadTtsSupplement } from "../auto-reply/reply-payload.js";
 export {
  buildTtsSupplementMediaPayload,
  getReplyPayloadTtsSupplement,
+  isReplyPayloadNonTerminalToolErrorWarning,
  isReplyPayloadTtsSupplement,
  markReplyPayloadAsTtsSupplement,
 } from "../auto-reply/reply-payload.js";