mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(agents): keep exact NO_REPLY silent instead of mirroring messaging-tool text (#119463)
* [AI] fix(agents): keep exact NO_REPLY silent instead of mirroring messaging-tool text Exact final NO_REPLY used to be rewritten to the latest messaging-tool text (messagingToolSentTexts.at(-1)). sessions_send bodies are recorded in that global list on success, but carry no channel route target, so an internal escalation note could become the final user-facing payload and be delivered to the originating user channel (#119383). Remove the obsolete generic rewrite: the Doctor migration contract already states exact NO_REPLY is never rewritten to visible fallback text, and both the block-reply delivery path and the final payload normalizer treat exact NO_REPLY as silence. Messaging-tool sent-text/target evidence recording is unchanged so dedupe and lifecycle evidence keep working. Fixes #119383 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * [AI] refactor(agents): document NO_REPLY invariant without report reference ClawSweeper P3: the inline comment in handleMessageEnd referenced the specific report (#119383). State the durable invariant instead: global messaging-tool send evidence is not a user-route reply and must never be mirrored into the final payload. Also apply oxfmt formatting to the new regression test fixture lines. Related to #119383 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -39,10 +39,6 @@ type EmbeddedSubscribeMessagesTestApi = {
|
||||
visibleDelta: string;
|
||||
},
|
||||
): { hold: boolean; text: string };
|
||||
resolveSilentReplyFallbackText(params: {
|
||||
text: unknown;
|
||||
messagingToolSentTexts: string[];
|
||||
}): string;
|
||||
};
|
||||
|
||||
function getTestApi(): EmbeddedSubscribeMessagesTestApi {
|
||||
@@ -79,10 +75,3 @@ export function resolveCurrentSourceMessagingToolPartial(
|
||||
): { hold: boolean; text: string } {
|
||||
return getTestApi().resolveCurrentSourceMessagingToolPartial(state, params);
|
||||
}
|
||||
|
||||
export function resolveSilentReplyFallbackText(params: {
|
||||
text: unknown;
|
||||
messagingToolSentTexts: string[];
|
||||
}): string {
|
||||
return getTestApi().resolveSilentReplyFallbackText(params);
|
||||
}
|
||||
|
||||
@@ -16,7 +16,6 @@ import {
|
||||
buildAssistantStreamData,
|
||||
recordPendingAssistantReplyDirectives,
|
||||
resolveCurrentSourceMessagingToolPartial,
|
||||
resolveSilentReplyFallbackText,
|
||||
} from "./embedded-agent-subscribe.handlers.messages.test-support.js";
|
||||
import type { EmbeddedAgentSubscribeContext } from "./embedded-agent-subscribe.handlers.types.js";
|
||||
import {
|
||||
@@ -237,50 +236,6 @@ function createMessageToolEnvelope(message: string, args: Record<string, unknown
|
||||
});
|
||||
}
|
||||
|
||||
describe("resolveSilentReplyFallbackText", () => {
|
||||
it("replaces NO_REPLY with latest messaging tool text when available", () => {
|
||||
expect(
|
||||
resolveSilentReplyFallbackText({
|
||||
text: "NO_REPLY",
|
||||
messagingToolSentTexts: ["first", "final delivered text"],
|
||||
}),
|
||||
).toBe("final delivered text");
|
||||
});
|
||||
|
||||
it("keeps original text when response is not NO_REPLY", () => {
|
||||
expect(
|
||||
resolveSilentReplyFallbackText({
|
||||
text: "normal assistant reply",
|
||||
messagingToolSentTexts: ["final delivered text"],
|
||||
}),
|
||||
).toBe("normal assistant reply");
|
||||
});
|
||||
|
||||
it("keeps NO_REPLY when there is no messaging tool text to mirror", () => {
|
||||
expect(
|
||||
resolveSilentReplyFallbackText({
|
||||
text: "NO_REPLY",
|
||||
messagingToolSentTexts: [],
|
||||
}),
|
||||
).toBe("NO_REPLY");
|
||||
});
|
||||
|
||||
it("tolerates malformed text payloads without throwing", () => {
|
||||
expect(
|
||||
resolveSilentReplyFallbackText({
|
||||
text: undefined,
|
||||
messagingToolSentTexts: ["final delivered text"],
|
||||
}),
|
||||
).toBe("");
|
||||
expect(
|
||||
resolveSilentReplyFallbackText({
|
||||
text: "NO_REPLY",
|
||||
messagingToolSentTexts: [42 as unknown as string],
|
||||
}),
|
||||
).toBe("42");
|
||||
});
|
||||
});
|
||||
|
||||
describe("hasAssistantVisibleReply", () => {
|
||||
it("treats audio-only payloads as visible", () => {
|
||||
expect(hasAssistantVisibleReply({ audioAsVoice: true })).toBe(true);
|
||||
@@ -1705,6 +1660,71 @@ describe("handleMessageEnd", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it("keeps exact NO_REPLY silent after a user-facing message send followed by sessions_send (#119383)", () => {
|
||||
const emitBlockReply = vi.fn();
|
||||
const finalizeAssistantTexts = vi.fn();
|
||||
const ctx = createMessageEndContext({
|
||||
emitBlockReply,
|
||||
finalizeAssistantTexts,
|
||||
consumeReplyDirectives: vi.fn((text: string) => ({ text })),
|
||||
state: {
|
||||
blockBuffer: "",
|
||||
deltaBuffer: "",
|
||||
messagingToolSentTexts: ["<user-facing reply>", "<internal escalation note>"],
|
||||
messagingToolSentTextsNormalized: ["<user-facing reply>", "<internal escalation note>"],
|
||||
messagingToolSentTargets: [
|
||||
{
|
||||
tool: "message",
|
||||
provider: "whatsapp",
|
||||
to: "user:123",
|
||||
text: "<user-facing reply>",
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
void endMessage(ctx, {
|
||||
message: { role: "assistant", content: [{ type: "text", text: "NO_REPLY" }] },
|
||||
});
|
||||
|
||||
// The exact silent token must never be rewritten to the sessions_send body:
|
||||
// the final assistant text keeps NO_REPLY and no block reply carries the note.
|
||||
expect(finalizeAssistantTexts).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ text: "NO_REPLY" }),
|
||||
);
|
||||
for (const call of emitBlockReply.mock.calls) {
|
||||
expect(JSON.stringify(call)).not.toContain("<internal escalation note>");
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps exact NO_REPLY silent when only sessions_send delivered (#119383)", () => {
|
||||
const emitBlockReply = vi.fn();
|
||||
const finalizeAssistantTexts = vi.fn();
|
||||
const ctx = createMessageEndContext({
|
||||
emitBlockReply,
|
||||
finalizeAssistantTexts,
|
||||
consumeReplyDirectives: vi.fn((text: string) => ({ text })),
|
||||
state: {
|
||||
blockBuffer: "",
|
||||
deltaBuffer: "",
|
||||
messagingToolSentTexts: ["<internal escalation note>"],
|
||||
messagingToolSentTextsNormalized: ["<internal escalation note>"],
|
||||
messagingToolSentTargets: [],
|
||||
},
|
||||
});
|
||||
|
||||
void endMessage(ctx, {
|
||||
message: { role: "assistant", content: [{ type: "text", text: "NO_REPLY" }] },
|
||||
});
|
||||
|
||||
expect(finalizeAssistantTexts).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ text: "NO_REPLY" }),
|
||||
);
|
||||
for (const call of emitBlockReply.mock.calls) {
|
||||
expect(JSON.stringify(call)).not.toContain("<internal escalation note>");
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: "counts a completed provider assistant message",
|
||||
|
||||
@@ -446,23 +446,6 @@ function copyPartialBlockState(
|
||||
target.pendingTagFragment = source.pendingTagFragment;
|
||||
}
|
||||
|
||||
/** Replaces a silent-reply token with the latest sent messaging-tool text when available. */
|
||||
function resolveSilentReplyFallbackText(params: {
|
||||
text: unknown;
|
||||
messagingToolSentTexts: string[];
|
||||
}): string {
|
||||
const text = coerceChatContentText(params.text);
|
||||
const trimmed = text.trim();
|
||||
if (trimmed !== SILENT_REPLY_TOKEN) {
|
||||
return text;
|
||||
}
|
||||
const fallback = coerceChatContentText(params.messagingToolSentTexts.at(-1)).trim();
|
||||
if (!fallback) {
|
||||
return text;
|
||||
}
|
||||
return fallback;
|
||||
}
|
||||
|
||||
function clearPendingToolMedia(
|
||||
state: Pick<
|
||||
EmbeddedAgentSubscribeState,
|
||||
@@ -1257,7 +1240,6 @@ if (process.env.VITEST || process.env.NODE_ENV === "test") {
|
||||
buildAssistantStreamData,
|
||||
recordPendingAssistantReplyDirectives,
|
||||
resolveCurrentSourceMessagingToolPartial,
|
||||
resolveSilentReplyFallbackText,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1357,10 +1339,10 @@ export function handleMessageEnd(
|
||||
? ctx.stripBlockTags(visibleText, { thinking: false, final: false }, { final: true })
|
||||
: visibleText;
|
||||
|
||||
const text = resolveSilentReplyFallbackText({
|
||||
text: finalVisibleText,
|
||||
messagingToolSentTexts: ctx.state.messagingToolSentTexts,
|
||||
});
|
||||
// Exact NO_REPLY stays silent. The legacy rewrite (silentReplyRewrite) was
|
||||
// removed by contract; global messaging-tool send evidence is not a
|
||||
// user-route reply and must never be mirrored into the final payload.
|
||||
const text = finalVisibleText;
|
||||
const rawThinking =
|
||||
ctx.state.includeReasoning || ctx.state.streamReasoning
|
||||
? extractAssistantThinking(assistantMessage) || extractThinkingFromTaggedText(rawText)
|
||||
|
||||
Reference in New Issue
Block a user