fix(agents): keep tool-result context guard truncation UTF-16 safe by chengzhichao-xydt · Pull Request #102466 · openclaw/openclaw

chengzhichao-xydt · 2026-07-09T06:32:02Z

What Problem This Solves

truncateTextToBudget could split an emoji or another supplementary Unicode character when the tool-result context budget ended inside its UTF-16 surrogate pair. The resulting provider context contained a lone surrogate. The truncation notice could also undercount omitted code units if a safe boundary backed up by one.

Why This Change Was Made

The context guard now truncates through the shared truncateUtf16Safe primitive and computes the notice from the actual retained prefix length. This keeps both the serialized text and its omitted-character count accurate without changing the existing context-budget policy.

The focused regression asserts the exact provider-facing output, including the safe prefix and the corrected omitted count, and confirms the original live tool result remains unchanged.

User Impact

Oversized tool results containing emoji no longer inject malformed UTF-16 into provider replay context, and their truncation notices report the actual number of omitted UTF-16 code units.

Evidence

Exact head: 686f644a7c3844056f5b4327d5021986436a7180
node scripts/run-vitest.mjs src/agents/embedded-agent-runner/tool-result-context-guard.test.ts — 1 file, 46 tests passed.
Targeted oxfmt and git diff --check passed.
Fresh uncommitted autoreview: clean, no accepted/actionable findings (0.98).

Local narrow test fallback was used because fork-contributor code is not eligible for credential-hydrated Testbox execution before landing review.

AI-assisted; maintainer-reviewed.

Replace the raw text.slice(0, cutPoint) in truncateTextToBudget with truncateUtf16Safe so oversized tool results do not emit lone surrogates when an emoji falls on the truncation boundary. Adds a regression test that places an emoji exactly at the legacy cut point and asserts the truncated output contains no lone surrogates. Co-Authored-By: Claude Opus 4.8 <[email protected]>

clawsweeper · 2026-07-09T06:43:55Z

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 2:55 AM ET / 06:55 UTC.

Summary
The PR updates the embedded agent tool-result context guard to use truncateUtf16Safe for truncated provider replay text and adds a regression covering an emoji split boundary.

PR surface: Source +1, Tests +20. Total +21 across 2 files.

Reproducibility: yes. source-level reproduction is high confidence: current main's private truncation helper can cut the PR's emoji-boundary input into a lone high surrogate. I did not run the full OpenClaw agent loop because this review was read-only.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🧂 unranked krab
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add redacted terminal/live output or a short runtime transcript that directly drives the changed context guard after the fix and shows no lone surrogate plus the corrected omitted count.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body provides focused test/format/autoreview evidence, but no real after-fix runtime output or provider replay transcript showing the changed guard behavior outside tests. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] After-fix evidence is currently test-only; a maintainer should not treat the external PR proof gate as cleared until there is a redacted real runtime transcript, live output, or other direct behavior artifact.

Maintainer options:

Decide the mitigation before merge
Land the narrow UTF-16-safe context-guard fix after real behavior proof is added or a maintainer explicitly accepts the proof gap; keep the sibling truncation helper path unchanged.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] Do not queue automation; the remaining blocker is contributor-supplied real behavior proof, not a concrete code repair.

Security
Cleared: The diff imports an existing internal UTF-16 helper and adds a focused test; no concrete security or supply-chain regression was found.

Review details

Best possible solution:

Land the narrow UTF-16-safe context-guard fix after real behavior proof is added or a maintainer explicitly accepts the proof gap; keep the sibling truncation helper path unchanged.

Do we have a high-confidence way to reproduce the issue?

Yes, source-level reproduction is high confidence: current main's private truncation helper can cut the PR's emoji-boundary input into a lone high surrogate. I did not run the full OpenClaw agent loop because this review was read-only.

Is this the best way to solve the issue?

Yes, the proposed code is the best narrow fix: it uses the existing shared UTF-16-safe helper at the exact raw slice site and computes the notice from the actual retained prefix. The remaining issue is proof quality, not fix shape.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against b20b02a47633.

Label changes

Label changes:

add P2: This is a normal-priority agent runtime bug fix for malformed provider replay context with limited blast radius and a narrow patch.
add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🧂 unranked krab and patch quality is 🦞 diamond lobster.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides focused test/format/autoreview evidence, but no real after-fix runtime output or provider replay transcript showing the changed guard behavior outside tests. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

P2: This is a normal-priority agent runtime bug fix for malformed provider replay context with limited blast radius and a narrow patch.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🧂 unranked krab and patch quality is 🦞 diamond lobster.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides focused test/format/autoreview evidence, but no real after-fix runtime output or provider replay transcript showing the changed guard behavior outside tests. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +1, Tests +20. Total +21 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	3	2	+1
Tests	1	20	0	+20
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	23	2	+21

What I checked:

Repository policy read: Read the root AGENTS.md and scoped src/agents/AGENTS.md; the review applied the agents-path guidance to inspect callers, sibling truncation helpers, current main behavior, and proof quality. (AGENTS.md:1, b20b02a47633)
Current main source behavior: Current main still builds the provider replay truncation with text.slice(0, cutPoint), so this PR is not already implemented on main and the proposed change targets the implicated code path. (src/agents/embedded-agent-runner/tool-result-context-guard.ts:168, b20b02a47633)
Source-level reproduction: A direct reproduction of the current truncation algorithm with the PR's 439-a plus emoji input returns a lone high surrogate and reports 1001 omitted characters, matching the bug claim before the PR's retained-prefix count correction. (src/agents/embedded-agent-runner/tool-result-context-guard.ts:144, b20b02a47633)
Proposed fix shape: The PR replaces the raw slice with truncateUtf16Safe(text, cutPoint) and derives the truncation notice from the actual retained prefix length, which is the narrowest fix for this private cut point. (src/agents/embedded-agent-runner/tool-result-context-guard.ts:165, 686f644a7c38)
Regression coverage: The PR adds a focused test that drives installToolResultContextGuard, asserts the provider-facing text drops the split emoji, verifies the omitted count is 1002, and confirms the original tool result remains unchanged. (src/agents/embedded-agent-runner/tool-result-context-guard.test.ts:327, 686f644a7c38)
Shared helper contract: truncateUtf16Safe floors the limit and delegates to sliceUtf16Safe, whose edge checks avoid returning dangling surrogate halves; the import path is exported by packages/normalization-core/package.json. (packages/normalization-core/src/utf16-slice.ts:44, b20b02a47633)

Likely related people:

steipete: Blame shows Peter Steinberger introduced the current context-guard helper and the shared UTF-16 helper in eae3fc1a79; he also authored the follow-up commit on this PR that corrected the omitted count. (role: feature owner and recent area contributor; confidence: high; commits: eae3fc1a79dd, 686f644a7c38, d361eab4586b; files: src/agents/embedded-agent-runner/tool-result-context-guard.ts, src/agents/embedded-agent-runner/tool-result-context-guard.test.ts, packages/normalization-core/src/utf16-slice.ts)
chengzhichao-xydt: Beyond authoring this PR, this contributor authored merged PR fix(agents): keep tool-result truncation UTF-16 safe #102087, which hardened the sibling tool-result truncation path with the same UTF-16-safe helper pattern. (role: recent adjacent contributor; confidence: medium; commits: e050e018ff62; files: src/agents/embedded-agent-runner/tool-result-truncation.ts, src/agents/embedded-agent-runner/tool-result-truncation.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete · 2026-07-09T06:58:55Z

Land-ready maintainer pass complete at exact head 686f644a7c3844056f5b4327d5021986436a7180.

The original UTF-16 fix was useful. I improved it by deriving the truncation notice from the actual surrogate-safe prefix (the prior version undercounted omitted code units by one when it backed up) and replacing a verbose test scanner with an exact provider-context assertion.

Proof:

Focused fallback: node scripts/run-vitest.mjs src/agents/embedded-agent-runner/tool-result-context-guard.test.ts — 46 tests passed.
Exact-head CI run https://github.com/openclaw/openclaw/actions/runs/28999637545 — success.
Targeted oxfmt and git diff --check — passed.
Fresh full-branch autoreview — clean, no accepted/actionable findings (0.98).
Native review artifacts validated; OPENCLAW_TESTBOX=1 scripts/pr prepare-run 102466 passed at the same SHA.

The focused test ran locally because fork-contributor code is not eligible for credential-hydrated Testbox execution before landing review. Exact-head fork CI supplies the broad hosted proof. No remaining proof gap.

steipete · 2026-07-09T06:59:12Z

Merged via squash.

Prepared head SHA: 686f644a7c3844056f5b4327d5021986436a7180
Landed commit: bc6a4bdd94ab71faa1fc782b70e4c1374c98d041

…penclaw#102466) * fix(agents): keep tool-result context guard truncation UTF-16 safe Replace the raw text.slice(0, cutPoint) in truncateTextToBudget with truncateUtf16Safe so oversized tool results do not emit lone surrogates when an emoji falls on the truncation boundary. Adds a regression test that places an emoji exactly at the legacy cut point and asserts the truncated output contains no lone surrogates. Co-Authored-By: Claude Opus 4.8 <[email protected]> * fix(agents): report surrogate-safe omitted count --------- Co-authored-by: chengzhichao-xydt <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 9, 2026

fix(agents): report surrogate-safe omitted count

686f644

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 9, 2026

steipete merged commit bc6a4bd into openclaw:main Jul 9, 2026
128 of 134 checks passed

steipete mentioned this pull request Jul 9, 2026

fix(agents): keep truncateText and truncateTextToBudget UTF-16 safe #102482

Closed

This was referenced Jul 9, 2026

fix(text): keep five src-side text truncations UTF-16 safe #102630

Closed

fix(chat,cli): keep session snippet and CLI output digest truncation UTF-16 safe #102747

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): keep tool-result context guard truncation UTF-16 safe#102466

fix(agents): keep tool-result context guard truncation UTF-16 safe#102466
steipete merged 2 commits into
openclaw:mainfrom
chengzhichao-xydt:fix/agents-tool-result-utf16-safe

chengzhichao-xydt commented Jul 9, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

chengzhichao-xydt commented Jul 9, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chengzhichao-xydt commented Jul 9, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading