From be5a035d97e58902deca4e88a435ebb185bfd3ae Mon Sep 17 00:00:00 2001 From: Morrow Date: Wed, 1 Apr 2026 04:10:49 +0300 Subject: [PATCH] fix: harden embedded text normalization (#58555) Co-authored-by: Morrow <271612559+agent-morrow@users.noreply.github.com> --- ...ded-helpers.sanitizeuserfacingtext.test.ts | 5 +++++ src/agents/pi-embedded-helpers/errors.ts | 22 ++++++++++++------- ...bedded-subscribe.handlers.messages.test.ts | 15 +++++++++++++ ...pi-embedded-subscribe.handlers.messages.ts | 18 +++++++++------ src/shared/chat-content.test.ts | 18 +++++++++++++++ src/shared/chat-content.ts | 17 +++++++++----- 6 files changed, 74 insertions(+), 21 deletions(-) diff --git a/src/agents/pi-embedded-helpers.sanitizeuserfacingtext.test.ts b/src/agents/pi-embedded-helpers.sanitizeuserfacingtext.test.ts index 71ccde3f509..4c0951e204e 100644 --- a/src/agents/pi-embedded-helpers.sanitizeuserfacingtext.test.ts +++ b/src/agents/pi-embedded-helpers.sanitizeuserfacingtext.test.ts @@ -178,6 +178,11 @@ describe("sanitizeUserFacingText", () => { it.each(["\n\n", " \n "])("returns empty for whitespace-only input: %j", (input) => { expect(sanitizeUserFacingText(input)).toBe(""); }); + + it("tolerates non-string input without throwing", () => { + expect(sanitizeUserFacingText(undefined as unknown as string)).toBe(""); + expect(sanitizeUserFacingText(42 as unknown as string)).toBe("42"); + }); }); describe("stripThoughtSignatures", () => { diff --git a/src/agents/pi-embedded-helpers/errors.ts b/src/agents/pi-embedded-helpers/errors.ts index f55483027d4..67cd9b40438 100644 --- a/src/agents/pi-embedded-helpers/errors.ts +++ b/src/agents/pi-embedded-helpers/errors.ts @@ -544,11 +544,16 @@ export function classifyFailoverReasonFromHttpStatus( return null; } -function stripFinalTagsFromText(text: string): string { - if (!text) { - return text; +function coerceText(value: unknown): string { + return typeof value === "string" ? value : value == null ? "" : String(value); +} + +function stripFinalTagsFromText(text: unknown): string { + const normalized = coerceText(text); + if (!normalized) { + return normalized; } - return text.replace(FINAL_TAG_RE, ""); + return normalized.replace(FINAL_TAG_RE, ""); } function collapseConsecutiveDuplicateBlocks(text: string): string { @@ -755,12 +760,13 @@ export function formatAssistantErrorText( return raw.length > 600 ? `${raw.slice(0, 600)}…` : raw; } -export function sanitizeUserFacingText(text: string, opts?: { errorContext?: boolean }): string { - if (!text) { - return text; +export function sanitizeUserFacingText(text: unknown, opts?: { errorContext?: boolean }): string { + const raw = coerceText(text); + if (!raw) { + return raw; } const errorContext = opts?.errorContext ?? false; - const stripped = stripFinalTagsFromText(text); + const stripped = stripFinalTagsFromText(raw); const trimmed = stripped.trim(); if (!trimmed) { return ""; diff --git a/src/agents/pi-embedded-subscribe.handlers.messages.test.ts b/src/agents/pi-embedded-subscribe.handlers.messages.test.ts index 843856b53d7..8f33fec61d1 100644 --- a/src/agents/pi-embedded-subscribe.handlers.messages.test.ts +++ b/src/agents/pi-embedded-subscribe.handlers.messages.test.ts @@ -34,6 +34,21 @@ describe("resolveSilentReplyFallbackText", () => { }), ).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", () => { diff --git a/src/agents/pi-embedded-subscribe.handlers.messages.ts b/src/agents/pi-embedded-subscribe.handlers.messages.ts index aa51437cf1d..d3a35013efc 100644 --- a/src/agents/pi-embedded-subscribe.handlers.messages.ts +++ b/src/agents/pi-embedded-subscribe.handlers.messages.ts @@ -38,6 +38,9 @@ const stripTrailingDirective = (text: string): string => { return text.slice(0, openIndex); }; +const coerceText = (value: unknown): string => + typeof value === "string" ? value : value == null ? "" : String(value); + function isTranscriptOnlyOpenClawAssistantMessage(message: AgentMessage | undefined): boolean { if (!message || message.role !== "assistant") { return false; @@ -56,16 +59,17 @@ function emitReasoningEnd(ctx: EmbeddedPiSubscribeContext) { } export function resolveSilentReplyFallbackText(params: { - text: string; + text: unknown; messagingToolSentTexts: string[]; }): string { - const trimmed = params.text.trim(); + const text = coerceText(params.text); + const trimmed = text.trim(); if (trimmed !== SILENT_REPLY_TOKEN) { - return params.text; + return text; } - const fallback = params.messagingToolSentTexts.at(-1)?.trim(); + const fallback = coerceText(params.messagingToolSentTexts.at(-1)).trim(); if (!fallback) { - return params.text; + return text; } return fallback; } @@ -357,7 +361,7 @@ export function handleMessageEnd( } promoteThinkingTagsToBlocks(assistantMessage); - const rawText = extractAssistantText(assistantMessage); + const rawText = coerceText(extractAssistantText(assistantMessage)); appendRawStream({ ts: Date.now(), event: "assistant_message_end", @@ -382,7 +386,7 @@ export function handleMessageEnd( let { mediaUrls, hasMedia } = resolveSendableOutboundReplyParts(parsedText ?? {}); if (!cleanedText && !hasMedia && !ctx.params.enforceFinalTag) { - const rawTrimmed = rawText.trim(); + const rawTrimmed = coerceText(rawText).trim(); const rawStrippedFinal = rawTrimmed.replace(/<\s*\/?\s*final\s*>/gi, "").trim(); const rawCandidate = rawStrippedFinal || rawTrimmed; if (rawCandidate) { diff --git a/src/shared/chat-content.test.ts b/src/shared/chat-content.test.ts index 0131865cef8..e6e72aa1c8f 100644 --- a/src/shared/chat-content.test.ts +++ b/src/shared/chat-content.test.ts @@ -61,4 +61,22 @@ describe("shared/chat-content", () => { }), ).toBeNull(); }); + + it("tolerates sanitize and normalize hooks that return non-string values", () => { + expect( + extractTextFromChatContent("hello", { + sanitizeText: () => undefined as unknown as string, + }), + ).toBeNull(); + expect( + extractTextFromChatContent([{ type: "text", text: "hello" }], { + sanitizeText: () => 42 as unknown as string, + }), + ).toBe("42"); + expect( + extractTextFromChatContent("hello", { + normalizeText: () => undefined as unknown as string, + }), + ).toBeNull(); + }); }); diff --git a/src/shared/chat-content.ts b/src/shared/chat-content.ts index c052e457ebd..b87cf53e136 100644 --- a/src/shared/chat-content.ts +++ b/src/shared/chat-content.ts @@ -6,11 +6,19 @@ export function extractTextFromChatContent( normalizeText?: (text: string) => string; }, ): string | null { - const normalize = opts?.normalizeText ?? ((text: string) => text.replace(/\s+/g, " ").trim()); + const normalizeText = opts?.normalizeText ?? ((text: string) => text.replace(/\s+/g, " ").trim()); const joinWith = opts?.joinWith ?? " "; + const coerceText = (value: unknown): string => + typeof value === "string" ? value : value == null ? "" : String(value); + const sanitize = (text: unknown): string => { + const raw = coerceText(text); + const sanitized = opts?.sanitizeText ? opts.sanitizeText(raw) : raw; + return coerceText(sanitized); + }; + const normalize = (text: unknown): string => coerceText(normalizeText(coerceText(text))); if (typeof content === "string") { - const value = opts?.sanitizeText ? opts.sanitizeText(content) : content; + const value = sanitize(content); const normalized = normalize(value); return normalized ? normalized : null; } @@ -28,10 +36,7 @@ export function extractTextFromChatContent( continue; } const text = (block as { text?: unknown }).text; - if (typeof text !== "string") { - continue; - } - const value = opts?.sanitizeText ? opts.sanitizeText(text) : text; + const value = sanitize(text); if (value.trim()) { chunks.push(value); }