From 6b38714cb9afa59f57cea85bc5b4afa1a53c269f Mon Sep 17 00:00:00 2001 From: Charles Dusek <38732970+cgdusek@users.noreply.github.com> Date: Sat, 25 Apr 2026 01:36:40 -0500 Subject: [PATCH] fix(agents): guard malformed tool result text blocks Harden context pruning and tool-result character estimation against malformed `{ type: "text" }` blocks created by void/undefined tool handler results. - Require text blocks to carry a string before using `.length` in the tool-result estimator. - Guard context-pruning text/image loops against malformed and null content entries. - Serialize malformed non-string text blocks for pruning size accounting so they cannot bypass trimming as zero-sized. - Add regression coverage for malformed text blocks, null entries, and non-string text payloads. Closes #34979. Maintainer verification: - `pnpm test src/agents/pi-embedded-runner/tool-result-char-estimator.test.ts src/agents/pi-hooks/context-pruning/pruner.test.ts` - `pnpm check:changed` - GitHub checks passed, including the OpenAI / Opus 4.6 parity gate. Based on prior work by #39331 and #34980. Co-authored-by: Charles Dusek Co-authored-by: alvinttang Co-authored-by: coffeexcoin Co-authored-by: Claude Opus 4.6 --- .../tool-result-char-estimator.test.ts | 69 +++++++++ .../tool-result-char-estimator.ts | 7 +- .../pi-hooks/context-pruning/pruner.test.ts | 135 ++++++++++++++++++ src/agents/pi-hooks/context-pruning/pruner.ts | 46 ++++-- 4 files changed, 247 insertions(+), 10 deletions(-) create mode 100644 src/agents/pi-embedded-runner/tool-result-char-estimator.test.ts diff --git a/src/agents/pi-embedded-runner/tool-result-char-estimator.test.ts b/src/agents/pi-embedded-runner/tool-result-char-estimator.test.ts new file mode 100644 index 00000000000..8e00c10b823 --- /dev/null +++ b/src/agents/pi-embedded-runner/tool-result-char-estimator.test.ts @@ -0,0 +1,69 @@ +import type { AgentMessage } from "@mariozechner/pi-agent-core"; +import { describe, expect, it } from "vitest"; +import { + createMessageCharEstimateCache, + estimateMessageCharsCached, + getToolResultText, +} from "./tool-result-char-estimator.js"; + +/** + * Regression tests for malformed tool result content blocks. + * See https://github.com/openclaw/openclaw/issues/34979 + * + * A plugin tool handler returning undefined produces {type: "text"} (no text + * property) in the session JSONL. Without guards, this crashes the char + * estimator with: TypeError: Cannot read properties of undefined (reading 'length') + */ +describe("tool-result-char-estimator", () => { + it("does not crash on toolResult with malformed text block (missing text string)", () => { + const malformed = { + role: "toolResult", + toolName: "sentinel_control", + content: [{ type: "text" }], + isError: false, + timestamp: Date.now(), + } as unknown as AgentMessage; + + const cache = createMessageCharEstimateCache(); + expect(() => estimateMessageCharsCached(malformed, cache)).not.toThrow(); + // Malformed block should be estimated via the unknown-block fallback, not zero + expect(estimateMessageCharsCached(malformed, cache)).toBeGreaterThan(0); + }); + + it("does not crash on toolResult with null content entries", () => { + const malformed = { + role: "toolResult", + toolName: "read", + content: [null, { type: "text", text: "ok" }], + timestamp: Date.now(), + } as unknown as AgentMessage; + + const cache = createMessageCharEstimateCache(); + expect(() => estimateMessageCharsCached(malformed, cache)).not.toThrow(); + }); + + it("getToolResultText skips malformed text blocks without crashing", () => { + const malformed = { + role: "toolResult", + toolName: "sentinel_control", + content: [{ type: "text" }, { type: "text", text: "valid" }], + timestamp: Date.now(), + } as unknown as AgentMessage; + + expect(() => getToolResultText(malformed)).not.toThrow(); + expect(getToolResultText(malformed)).toBe("valid"); + }); + + it("estimates well-formed toolResult correctly", () => { + const msg = { + role: "toolResult", + toolName: "read", + content: [{ type: "text", text: "hello world" }], + timestamp: Date.now(), + } as unknown as AgentMessage; + + const cache = createMessageCharEstimateCache(); + const chars = estimateMessageCharsCached(msg, cache); + expect(chars).toBeGreaterThanOrEqual(11); // "hello world".length + }); +}); diff --git a/src/agents/pi-embedded-runner/tool-result-char-estimator.ts b/src/agents/pi-embedded-runner/tool-result-char-estimator.ts index 6d022d62289..0b836a12f3b 100644 --- a/src/agents/pi-embedded-runner/tool-result-char-estimator.ts +++ b/src/agents/pi-embedded-runner/tool-result-char-estimator.ts @@ -7,7 +7,12 @@ const IMAGE_CHAR_ESTIMATE = 8_000; export type MessageCharEstimateCache = WeakMap; function isTextBlock(block: unknown): block is { type: "text"; text: string } { - return !!block && typeof block === "object" && (block as { type?: unknown }).type === "text"; + return ( + !!block && + typeof block === "object" && + (block as { type?: unknown }).type === "text" && + typeof (block as { text?: unknown }).text === "string" + ); } function isImageBlock(block: unknown): boolean { diff --git a/src/agents/pi-hooks/context-pruning/pruner.test.ts b/src/agents/pi-hooks/context-pruning/pruner.test.ts index e4ef978b018..0b334339650 100644 --- a/src/agents/pi-hooks/context-pruning/pruner.test.ts +++ b/src/agents/pi-hooks/context-pruning/pruner.test.ts @@ -152,6 +152,141 @@ describe("pruneContextMessages", () => { ).not.toThrow(); }); + it("does not crash on toolResult with malformed text block (missing text string)", () => { + // Regression: a plugin returning undefined produces {type: "text"} with no text property, + // which crashed estimateTextAndImageChars / collectTextSegments / collectPrunableToolResultSegments. + // See https://github.com/openclaw/openclaw/issues/34979 + const malformedToolResult = { + role: "toolResult", + toolName: "sentinel_control", + content: [{ type: "text" }], + isError: false, + timestamp: Date.now(), + } as unknown as AgentMessage; + + const messages: AgentMessage[] = [ + makeUser("remove sentinel"), + makeAssistant([ + { type: "toolCall", toolCallId: "call_1", toolName: "sentinel_control", arguments: {} }, + ] as unknown as AssistantContentBlock[]), + malformedToolResult, + makeUser("follow up"), + makeAssistant([{ type: "text", text: "done" }]), + ]; + + expect(() => + pruneContextMessages({ + messages, + settings: DEFAULT_CONTEXT_PRUNING_SETTINGS, + ctx: CONTEXT_WINDOW_1M, + }), + ).not.toThrow(); + }); + + it("does not crash on toolResult with malformed text block during soft-trim (image path)", () => { + // The collectPrunableToolResultSegments path is exercised when the tool result + // contains image blocks alongside a malformed text block. + const malformedToolResult = { + role: "toolResult", + toolName: "read", + content: [{ type: "text" }, { type: "image", data: "img", mimeType: "image/png" }], + timestamp: Date.now(), + } as unknown as AgentMessage; + + const messages: AgentMessage[] = [ + makeUser("show image"), + malformedToolResult, + makeAssistant([{ type: "text", text: "here it is" }]), + ]; + + expect(() => + pruneContextMessages({ + messages, + settings: { + ...DEFAULT_CONTEXT_PRUNING_SETTINGS, + keepLastAssistants: 1, + softTrimRatio: 0, + hardClear: { + ...DEFAULT_CONTEXT_PRUNING_SETTINGS.hardClear, + enabled: false, + }, + softTrim: { + maxChars: 5_000, + headChars: 2_000, + tailChars: 2_000, + }, + }, + ctx: CONTEXT_WINDOW_1M, + isToolPrunable: () => true, + contextWindowTokensOverride: 1, + }), + ).not.toThrow(); + }); + + it("counts malformed non-string text blocks when deciding to trim tool results", () => { + const malformedToolResult = { + role: "toolResult", + toolName: "read", + content: [{ type: "text", text: { payload: "X".repeat(5_000) } }], + timestamp: Date.now(), + } as unknown as AgentMessage; + + const result = pruneContextMessages({ + messages: [ + makeUser("show data"), + malformedToolResult, + makeAssistant([{ type: "text", text: "done" }]), + ], + settings: { + ...DEFAULT_CONTEXT_PRUNING_SETTINGS, + keepLastAssistants: 1, + softTrimRatio: 0, + hardClear: { + ...DEFAULT_CONTEXT_PRUNING_SETTINGS.hardClear, + enabled: false, + }, + softTrim: { + maxChars: 200, + headChars: 80, + tailChars: 40, + }, + }, + ctx: CONTEXT_WINDOW_1M, + isToolPrunable: () => true, + contextWindowTokensOverride: 1, + }); + + const toolResult = result.find((message) => message.role === "toolResult") as Extract< + AgentMessage, + { role: "toolResult" } + >; + const textBlock = toolResult.content[0] as { type: "text"; text: string }; + expect(textBlock.text).toContain("[Tool result trimmed:"); + }); + + it("does not crash on toolResult with null content entries", () => { + const malformedToolResult = { + role: "toolResult", + toolName: "read", + content: [null, { type: "text", text: "ok" }], + timestamp: Date.now(), + } as unknown as AgentMessage; + + const messages: AgentMessage[] = [ + makeUser("hello"), + malformedToolResult, + makeAssistant([{ type: "text", text: "done" }]), + ]; + + expect(() => + pruneContextMessages({ + messages, + settings: DEFAULT_CONTEXT_PRUNING_SETTINGS, + ctx: CONTEXT_WINDOW_1M, + }), + ).not.toThrow(); + }); + it("handles well-formed thinking blocks correctly", () => { const messages: AgentMessage[] = [ makeUser("hello"), diff --git a/src/agents/pi-hooks/context-pruning/pruner.ts b/src/agents/pi-hooks/context-pruning/pruner.ts index 6171480e2ac..c1b3c041169 100644 --- a/src/agents/pi-hooks/context-pruning/pruner.ts +++ b/src/agents/pi-hooks/context-pruning/pruner.ts @@ -13,11 +13,36 @@ function asText(text: string): TextContent { return { type: "text", text }; } +function serializeMalformedTextBlock(block: unknown): string { + try { + const serialized = JSON.stringify(block); + return typeof serialized === "string" ? serialized : "[malformed text block]"; + } catch { + return "[malformed text block]"; + } +} + +function coerceTextBlock(block: unknown): string | null { + if (!block || typeof block !== "object") { + return null; + } + if ((block as { type?: unknown }).type !== "text") { + return null; + } + const text = (block as { text?: unknown }).text; + return typeof text === "string" ? text : serializeMalformedTextBlock(block); +} + +function isImageBlock(block: unknown): boolean { + return !!block && typeof block === "object" && (block as { type?: unknown }).type === "image"; +} + function collectTextSegments(content: ReadonlyArray): string[] { const parts: string[] = []; for (const block of content) { - if (block.type === "text") { - parts.push(block.text); + const text = coerceTextBlock(block); + if (text !== null) { + parts.push(text); } } return parts; @@ -28,11 +53,12 @@ function collectPrunableToolResultSegments( ): string[] { const parts: string[] = []; for (const block of content) { - if (block.type === "text") { - parts.push(block.text); + const text = coerceTextBlock(block); + if (text !== null) { + parts.push(text); continue; } - if (block.type === "image") { + if (isImageBlock(block)) { parts.push(PRUNED_CONTEXT_IMAGE_MARKER); } } @@ -105,7 +131,7 @@ function takeTailFromJoinedText(parts: string[], maxChars: number): string { function hasImageBlocks(content: ReadonlyArray): boolean { for (const block of content) { - if (block.type === "image") { + if (isImageBlock(block)) { return true; } } @@ -119,10 +145,12 @@ function estimateWeightedTextChars(text: string): number { function estimateTextAndImageChars(content: ReadonlyArray): number { let chars = 0; for (const block of content) { - if (block.type === "text") { - chars += estimateWeightedTextChars(block.text); + const text = coerceTextBlock(block); + if (text !== null) { + chars += estimateWeightedTextChars(text); + continue; } - if (block.type === "image") { + if (isImageBlock(block)) { chars += IMAGE_CHAR_ESTIMATE; } }