fix: guard against malformed tool result text blocks crashing context truncation by coffeexcoin · Pull Request #34980 · openclaw/openclaw

coffeexcoin · 2026-03-04T20:56:14Z

Summary

Problem: A plugin tool handler returning undefined produced a content block {type: "text"} with no text property. This got persisted to session JSONL, and every subsequent context truncation attempt crashed with TypeError: Cannot read properties of undefined (reading 'length') at estimateMessageChars, causing an unresponsive crash loop.
Why it matters: Any plugin that returns void/undefined from a tool handler can trigger this, bricking the session until manual reset.
What changed: Hardened isTextBlock type guard to require typeof text === "string", and added sanitizeToolResultContentBlocks at the persistence boundary to fix malformed blocks before they reach the session JSONL.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Fixes #34979

User-visible / Behavior Changes

Sessions no longer enter a crash loop when a plugin returns a malformed tool result with missing text property.
Malformed {type: "text"} content blocks are silently normalized to {type: "text", text: ""} before persistence.

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: macOS (Darwin 25.3.0)
Runtime/container: Node 22+ / Bun
Model/provider: Any
Integration/channel (if any): Any plugin returning void from a tool handler
Relevant config (redacted): N/A

Steps

Register a tool handler that returns undefined (e.g. return; with no value)
Invoke that tool in an agent session
Send any follow-up message (triggers context truncation)

Expected

Session continues normally; malformed content block is treated as empty text.

Actual (before fix)

TypeError: Cannot read properties of undefined (reading 'length') on every subsequent message, crash loop until session reset.

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

New tests (15 total across 3 files) reproduce the exact crash scenario — {type: "text"} with no text key passed through estimateMessageChars, context guard pipeline, and persistence guard. All 45 tests pass (15 new + 30 existing).

Human Verification (required)

Verified scenarios: Ran pnpm vitest run on all 3 changed test files — 45/45 pass. Ran pnpm tsgo — no new type errors.
Edge cases checked: {type: "text"} (no key), {type: "text", text: undefined}, {type: "text", text: null}, {type: "text", text: 42}, mixed valid+malformed blocks in same content array.
What I did not verify: End-to-end with a real plugin returning void (no sentinel plugin in this repo). Did not verify the full pnpm test suite.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

Failure Recovery (if this breaks)

How to disable/revert: Revert the 2 source file changes (tool-result-char-estimator.ts, session-tool-result-guard.ts). The old behavior returns (crashes on malformed blocks).
Files/config to restore: src/agents/pi-embedded-runner/tool-result-char-estimator.ts, src/agents/session-tool-result-guard.ts
Known bad symptoms: If the sanitization wrongly modifies valid text blocks, tool results would appear empty. The isTextBlock change could cause valid blocks to be estimated via estimateUnknownChars (slightly different char count, no crash).

Risks and Mitigations

Risk: isTextBlock now rejects blocks where text is not a string, which changes the estimation path from block.text.length to estimateUnknownChars(block) (JSON serialization). This could produce slightly different char estimates for blocks that previously had non-string text values.
- Mitigation: Such blocks were already invalid and would have crashed before. The new path produces a reasonable estimate instead of crashing.

greptile-apps · 2026-03-04T20:58:59Z

Greptile Summary

This PR fixes a crash loop caused by plugin tool handlers returning undefined, which produced malformed {type: "text"} content blocks (with no text property) that were persisted to session JSONL. On every subsequent message, estimateMessageChars would dereference block.text.length on undefined, crashing the context-truncation pipeline and bricking the session.

Changes:

tool-result-char-estimator.ts: Hardened isTextBlock to require typeof text === "string". Malformed blocks now fall through to estimateUnknownChars, which serializes them safely via JSON.stringify — no crash, reasonable estimate.
session-tool-result-guard.ts: Added sanitizeToolResultContentBlocks at the persistence boundary (capToolResultSize) to normalise any {type:"text"} block without a string text to {type:"text", text:""} before it is written to JSONL. This prevents malformed data from accumulating in new sessions.
Tests (3 files, 15 new cases): Reproduce the exact crash scenario end-to-end through the estimator, context guard pipeline, and persistence guard.

Minor gap worth noting: sanitizeToolResultContentBlocks is called inside capToolResultSize, which runs before the transformToolResultForPersistence and beforeMessageWriteHook hooks. If either of these hooks re-introduces a malformed block, it would bypass the sanitization guard and be written to JSONL in a malformed state. This is a low-probability scenario but is the one residual path not covered by the fix.

Confidence Score: 4/5

Safe to merge — fixes a confirmed crash-loop bug with well-targeted, minimal changes and comprehensive test coverage; one low-risk residual gap in the persistence hook ordering.
The two core changes (hardened isTextBlock predicate and sanitizeToolResultContentBlocks at the persistence boundary) are correct, minimal, and backward-compatible. 15 new tests directly reproduce the failure scenario and pass. The one-point deduction reflects the residual gap where transformToolResultForPersistence and beforeMessageWriteHook run after sanitization, meaning a misbehaving plugin hook could still persist a malformed block — but this is unlikely in practice and does not affect the fix for the described crash.
Minor attention to src/agents/session-tool-result-guard.ts around the hook ordering (lines 143–231) — persistToolResult and applyBeforeWriteHook run after sanitizeToolResultContentBlocks, leaving a residual gap for hooks that re-introduce malformed blocks.

_{Last reviewed commit: 6e2b387}

greptile-apps · 2026-03-04T20:59:07Z

Additional Comments (1)

src/agents/session-tool-result-guard.ts, line 149
Sanitization gap: persistToolResult and applyBeforeWriteHook run after sanitization

sanitizeToolResultContentBlocks is applied inside capToolResultSize, but both persistToolResult (i.e. transformToolResultForPersistence) and applyBeforeWriteHook (i.e. beforeMessageWriteHook) execute after that sanitization step. If either of those hooks returns a message with a malformed {type: "text"} block — e.g. a plugin that wraps or rewrites content — the block will bypass the guard and be written to JSONL in the malformed state.

const capped = capToolResultSize(persistMessage(normalizedToolResult));
const persisted = applyBeforeWriteHook(
  persistToolResult(capped, { ... }),
);

For full coverage, consider calling sanitizeToolResultContentBlocks on the result just before the final originalAppend, e.g.:

const finalToWrite = sanitizeToolResultContentBlocks(persisted);
return originalAppend(finalToWrite as never);

This is a low-probability gap (plugins would have to deliberately produce malformed content through these hooks), but worth noting given the PR's stated security-hardening goal.

chatgpt-codex-connector

💡 Codex Review

https://github.com/openclaw/openclaw/blob/4571f245377e9dd85b4f094487a95a7322711660/src/agents/session-tool-result-guard.ts#L227-L229
Re-sanitize tool results after persistence hooks rewrite them

In deployments that use tool_result_persist or before_message_write, this ordering still lets malformed {type:"text"} blocks reach JSONL. capToolResultSize() is the only place that now sanitizes tool-result content, but it runs before both hook chains here, and the hook contracts in src/plugins/types.ts:1725-1750 explicitly allow either hook to replace the message entirely. If a hook returns content: [{type: "text"}], the malformed block bypasses the new guard and the next context-pruning pass will still hit block.text.length in src/agents/pi-extensions/context-pruning/pruner.ts:114-160, recreating the same crash loop for those sessions.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@alvinttang

…ar estimation A plugin tool handler returning undefined produces {type: "text"} (no text property) in the session JSONL. This crashes the context truncation pipeline with TypeError on every subsequent message, locking the session into an unrecoverable crash loop. Guard all four crash sites: - isTextBlock type guard in tool-result-char-estimator.ts - collectTextSegments in pruner.ts - collectPrunableToolResultSegments in pruner.ts - estimateTextAndImageChars in pruner.ts Add regression tests for both the char estimator and the context pruner covering malformed toolResult content blocks. Closes openclaw#34979 Based on prior work by @alvinttang (openclaw#39331) and @coffeexcoin (openclaw#34980). 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>

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>

@vincentkoc

… tool-result pruning entries Three entries were missing co-credits I should have preserved: - Diagnostics/OTEL exec-process spans (#71451): @vincentkoc implemented, but @jlapenna's #70424 proposed the broader tracing work this entry builds on. Now credits both. - Diagnostics/OTEL preloaded SDK (#71450): same pattern — credits @vincentkoc and @jlapenna. - Agents/tool-result pruning (#51267): @cgdusek's PR explicitly built on prior work in #39331 by @alvinttang and #34980 by @coffeexcoin. Now credits all three.

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 openclaw#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 openclaw#39331 and openclaw#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>

@vincentkoc

… tool-result pruning entries Three entries were missing co-credits I should have preserved: - Diagnostics/OTEL exec-process spans (openclaw#71451): @vincentkoc implemented, but @jlapenna's openclaw#70424 proposed the broader tracing work this entry builds on. Now credits both. - Diagnostics/OTEL preloaded SDK (openclaw#71450): same pattern — credits @vincentkoc and @jlapenna. - Agents/tool-result pruning (openclaw#51267): @cgdusek's PR explicitly built on prior work in openclaw#39331 by @alvinttang and openclaw#34980 by @coffeexcoin. Now credits all three.

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 openclaw#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 openclaw#39331 and openclaw#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>

@vincentkoc

… tool-result pruning entries Three entries were missing co-credits I should have preserved: - Diagnostics/OTEL exec-process spans (openclaw#71451): @vincentkoc implemented, but @jlapenna's openclaw#70424 proposed the broader tracing work this entry builds on. Now credits both. - Diagnostics/OTEL preloaded SDK (openclaw#71450): same pattern — credits @vincentkoc and @jlapenna. - Agents/tool-result pruning (openclaw#51267): @cgdusek's PR explicitly built on prior work in openclaw#39331 by @alvinttang and openclaw#34980 by @coffeexcoin. Now credits all three.

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 openclaw#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 openclaw#39331 and openclaw#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>

@vincentkoc

… tool-result pruning entries Three entries were missing co-credits I should have preserved: - Diagnostics/OTEL exec-process spans (openclaw#71451): @vincentkoc implemented, but @jlapenna's openclaw#70424 proposed the broader tracing work this entry builds on. Now credits both. - Diagnostics/OTEL preloaded SDK (openclaw#71450): same pattern — credits @vincentkoc and @jlapenna. - Agents/tool-result pruning (openclaw#51267): @cgdusek's PR explicitly built on prior work in openclaw#39331 by @alvinttang and openclaw#34980 by @coffeexcoin. Now credits all three.

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 openclaw#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 openclaw#39331 and openclaw#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>

@vincentkoc

… tool-result pruning entries Three entries were missing co-credits I should have preserved: - Diagnostics/OTEL exec-process spans (openclaw#71451): @vincentkoc implemented, but @jlapenna's openclaw#70424 proposed the broader tracing work this entry builds on. Now credits both. - Diagnostics/OTEL preloaded SDK (openclaw#71450): same pattern — credits @vincentkoc and @jlapenna. - Agents/tool-result pruning (openclaw#51267): @cgdusek's PR explicitly built on prior work in openclaw#39331 by @alvinttang and openclaw#34980 by @coffeexcoin. Now credits all three.

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 openclaw#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 openclaw#39331 and openclaw#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>

@vincentkoc

… tool-result pruning entries Three entries were missing co-credits I should have preserved: - Diagnostics/OTEL exec-process spans (openclaw#71451): @vincentkoc implemented, but @jlapenna's openclaw#70424 proposed the broader tracing work this entry builds on. Now credits both. - Diagnostics/OTEL preloaded SDK (openclaw#71450): same pattern — credits @vincentkoc and @jlapenna. - Agents/tool-result pruning (openclaw#51267): @cgdusek's PR explicitly built on prior work in openclaw#39331 by @alvinttang and openclaw#34980 by @coffeexcoin. Now credits all three.

openclaw-barnacle Bot added agents Agent runtime and tooling size: M labels Mar 4, 2026

lynnzc mentioned this pull request Mar 7, 2026

fix(agents): guard context truncation against malformed text blocks #35876

Closed

2 tasks

coffeexcoin force-pushed the fix/malformed-tool-result-crash branch from 6e2b387 to 3ffd367 Compare March 10, 2026 18:48

chatgpt-codex-connector Bot reviewed Mar 19, 2026

View reviewed changes

coffeexcoin closed this Mar 19, 2026

coffeexcoin force-pushed the fix/malformed-tool-result-crash branch from 4571f24 to 7fb142d Compare March 19, 2026 16:51

openclaw-barnacle Bot added size: XS and removed size: M labels Mar 19, 2026

This was referenced Mar 20, 2026

fix: guard against malformed text blocks in context truncation #51267

Merged

[Bug]: Malformed tool result content block crashes context truncation in a session-permanent loop #34979

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: guard against malformed tool result text blocks crashing context truncation#34980

fix: guard against malformed tool result text blocks crashing context truncation#34980
coffeexcoin wants to merge 0 commit intoopenclaw:mainfrom
coffeexcoin:fix/malformed-tool-result-crash

coffeexcoin commented Mar 4, 2026

Uh oh!

greptile-apps Bot commented Mar 4, 2026

Uh oh!

greptile-apps Bot commented Mar 4, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

coffeexcoin commented Mar 4, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual (before fix)

Evidence

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Mar 4, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps Bot commented Mar 4, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant