mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:30:42 +00:00
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 <cgdusek@gmail.com>
Co-authored-by: alvinttang <alvinttang@users.noreply.github.com>
Co-authored-by: coffeexcoin <coffeexcoin@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
});
|
||||
});
|
||||
@@ -7,7 +7,12 @@ const IMAGE_CHAR_ESTIMATE = 8_000;
|
||||
export type MessageCharEstimateCache = WeakMap<AgentMessage, number>;
|
||||
|
||||
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 {
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -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<TextContent | ImageContent>): 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<TextContent | ImageContent>): 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<TextContent | ImageContent>): 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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user