Skip to content

Commit da27904

Browse files
steipetesteipete
authored
fix(discord): suppress recovered tool warnings (#87451)
1 parent 3f9d241 commit da27904

4 files changed

Lines changed: 113 additions & 32 deletions

File tree

‎extensions/discord/src/monitor/message-handler.process.test.ts‎

Lines changed: 59 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
import type { ReplyPayload } from "openclaw/plugin-sdk/reply-dispatch-runtime";
88
import * as runtimeEnvModule from "openclaw/plugin-sdk/runtime-env";
99
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
10+
import { setReplyPayloadMetadata } from "../../../../src/auto-reply/reply-payload.js";
1011
import type { DiscordMessagePreflightContext } from "./message-handler.preflight.js";
1112

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

55+
function createNonTerminalToolWarningPayload(): ReplyPayload {
56+
return setReplyPayloadMetadata(
57+
{
58+
text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
59+
isError: true,
60+
},
61+
{ nonTerminalToolErrorWarning: true },
62+
);
63+
}
64+
5465
vi.mock("../send.js", () => ({
5566
reactMessageDiscord: async (
5667
channelId: string,
@@ -2237,14 +2248,11 @@ describe("processDiscordMessage draft streaming", () => {
22372248
expect(deliverDiscordReply).toHaveBeenCalledTimes(1);
22382249
});
22392250

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

@@ -2257,6 +2265,44 @@ describe("processDiscordMessage draft streaming", () => {
22572265
expectPreviewEditContent("delivery survived");
22582266
expect(draftStream.clear).not.toHaveBeenCalled();
22592267
expect(draftStream.messageId()).toBe("preview-1");
2268+
expect(deliverDiscordReply).not.toHaveBeenCalled();
2269+
});
2270+
2271+
it("drops earlier tool warning finals when recovered replies arrive", async () => {
2272+
const draftStream = createMockDraftStreamForTest();
2273+
dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
2274+
await params?.dispatcher.sendFinalReply(createNonTerminalToolWarningPayload());
2275+
await params?.dispatcher.sendFinalReply({ text: "delivery recovered" });
2276+
return { queuedFinal: true, counts: { final: 2, tool: 0, block: 0 } };
2277+
});
2278+
2279+
const ctx = await createAutomaticSourceDeliveryContext({
2280+
discordConfig: { streamMode: "partial", maxLinesPerMessage: 5 },
2281+
});
2282+
2283+
await runProcessDiscordMessage(ctx);
2284+
2285+
expectPreviewEditContent("delivery recovered");
2286+
expect(draftStream.clear).not.toHaveBeenCalled();
2287+
expect(draftStream.messageId()).toBe("preview-1");
2288+
expect(deliverDiscordReply).not.toHaveBeenCalled();
2289+
});
2290+
2291+
it("delivers tool warning finals when no recovered reply is available", async () => {
2292+
const draftStream = createMockDraftStreamForTest();
2293+
dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
2294+
await params?.dispatcher.sendFinalReply(createNonTerminalToolWarningPayload());
2295+
return { queuedFinal: true, counts: { final: 1, tool: 0, block: 0 } };
2296+
});
2297+
2298+
const ctx = await createAutomaticSourceDeliveryContext({
2299+
discordConfig: { streamMode: "partial", maxLinesPerMessage: 5 },
2300+
});
2301+
2302+
await runProcessDiscordMessage(ctx);
2303+
2304+
expect(editMessageDiscord).not.toHaveBeenCalled();
2305+
expect(draftStream.clear).toHaveBeenCalledTimes(1);
22602306
expect(deliverDiscordReply).toHaveBeenCalledTimes(1);
22612307
expect(firstMockArg(deliverDiscordReply, "deliverDiscordReply")).toMatchObject({
22622308
replies: [
@@ -2268,14 +2314,14 @@ describe("processDiscordMessage draft streaming", () => {
22682314
});
22692315
});
22702316

2271-
it("keeps draft previews when tool warning finals arrive before recovered replies", async () => {
2317+
it("keeps mutating tool warning finals after successful-looking replies", async () => {
22722318
const draftStream = createMockDraftStreamForTest();
22732319
dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
2320+
await params?.dispatcher.sendFinalReply({ text: "Done." });
22742321
await params?.dispatcher.sendFinalReply({
2275-
text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
2322+
text: "⚠️ 🛠️ `write file (agent)` failed",
22762323
isError: true,
22772324
} as never);
2278-
await params?.dispatcher.sendFinalReply({ text: "delivery recovered" });
22792325
return { queuedFinal: true, counts: { final: 2, tool: 0, block: 0 } };
22802326
});
22812327

@@ -2285,14 +2331,13 @@ describe("processDiscordMessage draft streaming", () => {
22852331

22862332
await runProcessDiscordMessage(ctx);
22872333

2288-
expectPreviewEditContent("delivery recovered");
2334+
expectPreviewEditContent("Done.");
22892335
expect(draftStream.clear).not.toHaveBeenCalled();
2290-
expect(draftStream.messageId()).toBe("preview-1");
22912336
expect(deliverDiscordReply).toHaveBeenCalledTimes(1);
22922337
expect(firstMockArg(deliverDiscordReply, "deliverDiscordReply")).toMatchObject({
22932338
replies: [
22942339
{
2295-
text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
2340+
text: "⚠️ 🛠️ `write file (agent)` failed",
22962341
isError: true,
22972342
},
22982343
],
@@ -2448,17 +2493,14 @@ describe("processDiscordMessage draft streaming", () => {
24482493
expectPreviewEditContent("done");
24492494
});
24502495

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

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

@@ -2479,15 +2521,7 @@ describe("processDiscordMessage draft streaming", () => {
24792521
expectPreviewEditContent("delivery survived");
24802522
expect(draftStream.clear).not.toHaveBeenCalled();
24812523
expect(draftStream.messageId()).toBe("preview-1");
2482-
expect(deliverDiscordReply).toHaveBeenCalledTimes(1);
2483-
expect(firstMockArg(deliverDiscordReply, "deliverDiscordReply")).toMatchObject({
2484-
replies: [
2485-
{
2486-
text: "⚠️ 🛠️ `run openclaw definitely-not-a-real-subcommand (agent)` failed",
2487-
isError: true,
2488-
},
2489-
],
2490-
});
2524+
expect(deliverDiscordReply).not.toHaveBeenCalled();
24912525
});
24922526

24932527
it("uses raw tool-progress detail in Discord progress drafts", async () => {

‎extensions/discord/src/monitor/message-handler.process.ts‎

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import { createChannelHistoryWindow } from "openclaw/plugin-sdk/reply-history";
3737
import {
3838
buildTtsSupplementMediaPayload,
3939
getReplyPayloadTtsSupplement,
40+
isReplyPayloadNonTerminalToolErrorWarning,
4041
resolveSendableOutboundReplyParts,
4142
} from "openclaw/plugin-sdk/reply-payload";
4243
import type { ReplyDispatchKind, ReplyPayload } from "openclaw/plugin-sdk/reply-runtime";
@@ -104,6 +105,13 @@ function formatDiscordReplyDeliveryFailure(params: {
104105
return `discord ${params.kind} reply failed (${context}): ${String(params.err)}`;
105106
}
106107

108+
function isFallbackOnlyToolWarningFinal(payload: ReplyPayload): boolean {
109+
if (payload.isError !== true || !isReplyPayloadNonTerminalToolErrorWarning(payload)) {
110+
return false;
111+
}
112+
return !resolveSendableOutboundReplyParts(payload).hasMedia;
113+
}
114+
107115
type DiscordReplySkipReason = "aborted before delivery" | "reasoning payload";
108116

109117
export function formatDiscordReplySkip(params: {
@@ -537,6 +545,16 @@ export async function processDiscordMessage(
537545
draftPreview.markFinalReplyStarted();
538546
observer?.onFinalReplyStart?.();
539547
};
548+
let userFacingFinalDelivered = false;
549+
let pendingToolWarningFinal:
550+
| { payload: ReplyPayload; info: { kind: ReplyDispatchKind } }
551+
| undefined;
552+
const markUserFacingFinalDelivered = () => {
553+
userFacingFinalDelivered = true;
554+
pendingToolWarningFinal = undefined;
555+
draftPreview.markFinalReplyDelivered();
556+
observer?.onFinalReplyDelivered?.();
557+
};
540558
const beforeDiscordPayloadDelivery = (
541559
payload: ReplyPayload,
542560
info: { kind: ReplyDispatchKind },
@@ -570,7 +588,7 @@ export async function processDiscordMessage(
570588
return null;
571589
}
572590
}
573-
if (info.kind === "final") {
591+
if (info.kind === "final" && !isFallbackOnlyToolWarningFinal(payload)) {
574592
draftPreview.markFinalReplyStarted();
575593
}
576594
return payload;
@@ -579,6 +597,7 @@ export async function processDiscordMessage(
579597
const deliverDiscordPayload = async (
580598
payload: ReplyPayload,
581599
info: { kind: ReplyDispatchKind },
600+
options?: { allowFallbackOnlyToolWarning?: boolean },
582601
) => {
583602
if (isProcessAborted(abortSignal)) {
584603
// Surface so operators don't chase missing replies when an abort
@@ -606,6 +625,16 @@ export async function processDiscordMessage(
606625
);
607626
return { visibleReplySent: false };
608627
}
628+
if (
629+
isFinal &&
630+
!options?.allowFallbackOnlyToolWarning &&
631+
isFallbackOnlyToolWarningFinal(payload)
632+
) {
633+
if (!userFacingFinalDelivered) {
634+
pendingToolWarningFinal = { payload, info };
635+
}
636+
return { visibleReplySent: false };
637+
}
609638
if (isFinal) {
610639
draftPreview.markFinalReplyStarted();
611640
}
@@ -679,10 +708,9 @@ export async function processDiscordMessage(
679708
});
680709
},
681710
onPreviewFinalized: () => {
682-
draftPreview.markFinalReplyDelivered();
711+
markUserFacingFinalDelivered();
683712
draftPreview.markPreviewFinalized();
684713
replyReference.markSent();
685-
observer?.onFinalReplyDelivered?.();
686714
},
687715
buildSupplementalPayload: () =>
688716
ttsSupplement ? buildTtsSupplementMediaPayload(effectivePayload) : undefined,
@@ -759,9 +787,8 @@ export async function processDiscordMessage(
759787
return true;
760788
},
761789
onNormalDelivered: () => {
762-
draftPreview.markFinalReplyDelivered();
790+
markUserFacingFinalDelivered();
763791
replyReference.markSent();
764-
observer?.onFinalReplyDelivered?.();
765792
},
766793
});
767794
if (result.kind !== "normal-skipped") {
@@ -807,8 +834,7 @@ export async function processDiscordMessage(
807834
});
808835
replyReference.markSent();
809836
if (isFinal && payload.isError !== true) {
810-
draftPreview.markFinalReplyDelivered();
811-
observer?.onFinalReplyDelivered?.();
837+
markUserFacingFinalDelivered();
812838
}
813839
return { visibleReplySent: true };
814840
};
@@ -837,6 +863,21 @@ export async function processDiscordMessage(
837863
null;
838864
let dispatchError = false;
839865
let dispatchAborted = false;
866+
const deliverPendingToolWarningFinalIfNeeded = async () => {
867+
if (!pendingToolWarningFinal || userFacingFinalDelivered || isProcessAborted(abortSignal)) {
868+
return;
869+
}
870+
const pending = pendingToolWarningFinal;
871+
pendingToolWarningFinal = undefined;
872+
try {
873+
await deliverDiscordPayload(pending.payload, pending.info, {
874+
allowFallbackOnlyToolWarning: true,
875+
});
876+
} catch (err) {
877+
dispatchError = true;
878+
onDiscordDeliveryError(err, pending.info);
879+
}
880+
};
840881
try {
841882
if (isProcessAborted(abortSignal)) {
842883
dispatchAborted = true;
@@ -1026,6 +1067,7 @@ export async function processDiscordMessage(
10261067
dispatchAborted = true;
10271068
return;
10281069
}
1070+
await deliverPendingToolWarningFinalIfNeeded();
10291071
} catch (err) {
10301072
if (isProcessAborted(abortSignal)) {
10311073
dispatchAborted = true;

‎src/auto-reply/reply-payload.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,10 @@ export function getReplyPayloadMetadata(payload: object): ReplyPayloadMetadata |
184184
return replyPayloadMetadata.get(payload);
185185
}
186186

187+
export function isReplyPayloadNonTerminalToolErrorWarning(payload: object): boolean {
188+
return getReplyPayloadMetadata(payload)?.nonTerminalToolErrorWarning === true;
189+
}
190+
187191
export function copyReplyPayloadMetadata<T extends object>(source: object, payload: T): T {
188192
const metadata = getReplyPayloadMetadata(source);
189193
return metadata ? setReplyPayloadMetadata(payload, metadata) : payload;

‎src/plugin-sdk/reply-payload.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export type { ReplyPayloadTtsSupplement } from "../auto-reply/reply-payload.js";
1313
export {
1414
buildTtsSupplementMediaPayload,
1515
getReplyPayloadTtsSupplement,
16+
isReplyPayloadNonTerminalToolErrorWarning,
1617
isReplyPayloadTtsSupplement,
1718
markReplyPayloadAsTtsSupplement,
1819
} from "../auto-reply/reply-payload.js";

0 commit comments

Comments
 (0)