fix(http-error-body): keep emoji / surrogate pairs intact during error body truncation by wings1029 · Pull Request #101728 · openclaw/openclaw

wings1029 · 2026-07-07T14:30:49Z

What Problem This Solves

readResponseBodySnippet in src/infra/http-error-body.ts truncates HTTP error response body text with .slice(0, limits.maxChars), which counts UTF-16 code units. When an emoji straddles the cut boundary, .slice() produces a lone surrogate that renders as � in error diagnostics shown to users.

The sibling file src/infra/http-body.ts already imports and uses truncateUtf16Safe for the same kind of truncation (#101685), but http-error-body.ts was missed.

Why This Change Was Made

Replace .slice(0, limits.maxChars) with truncateUtf16Safe(text, limits.maxChars) at all three call sites (body-less path × 2 + stream path × 1). This matches src/infra/http-body.ts:317.

User Impact

Error diagnostics from MiniMax VLM and PDF native providers will no longer contain � when an emoji falls near the character truncation boundary.

Evidence

The table-driven regression test asserts the exact surrogate-safe result across
all three reachable branches:

body-less response below maxBytes
body-less response truncated by maxBytes
streamed response

Exact-head hosted CI is the execution gate for this contributor branch.

AI-assisted.

…r body truncation Replace .slice(0, limits.maxChars) with truncateUtf16Safe() so that emoji straddling the maxChars boundary don't produce lone surrogates that render as � in error diagnostics. Matches the sibling http-body.ts which already uses truncateUtf16Safe. Co-Authored-By: Claude <[email protected]>

clawsweeper · 2026-07-07T14:41:24Z

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 11:03 AM ET / 15:03 UTC.

Summary
The PR changes HTTP error-body truncation to use UTF-16-safe slicing and adds regression coverage for body-less and streamed response snippets.

PR surface: Source +6, Tests +26. Total +32 across 2 files.

Reproducibility: yes. from source inspection. Current main slices readResponseBodySnippet results by UTF-16 code units, so a value with an emoji crossing maxChars can leave a dangling surrogate.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
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 real behavior proof for an HTTP error-body diagnostic containing supplementary Unicode near the truncation boundary.
Rebase or refresh on current main before final exact-head checks.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR has regression-test and CI evidence only; it still needs redacted live output or logs showing an after-fix MiniMax/PDF-provider error diagnostic, with private details removed. 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] Contributor real-behavior proof is still missing: the PR shows test/CI evidence, but not a redacted MiniMax or native PDF provider error diagnostic after the fix.
[P1] The branch is behind current main, so final merge readiness should use a refreshed exact-head check after proof is added.

Maintainer options:

Decide the mitigation before merge
Land the narrow helper reuse after contributor proof shows a real HTTP error-body diagnostic with supplementary Unicode remains well-formed, then refresh exact-head checks on current main.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] Contributor real-behavior proof is required before merge, and there is no concrete code repair for automation to queue.

Security
Cleared: The diff only changes bounded string truncation and tests; it does not touch dependencies, workflows, secrets, package metadata, or code-execution surfaces.

Review details

Best possible solution:

Land the narrow helper reuse after contributor proof shows a real HTTP error-body diagnostic with supplementary Unicode remains well-formed, then refresh exact-head checks on current main.

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

Yes, from source inspection. Current main slices readResponseBodySnippet results by UTF-16 code units, so a value with an emoji crossing maxChars can leave a dangling surrogate.

Is this the best way to solve the issue?

Yes. Reusing the existing truncateUtf16Safe helper at all three truncation returns is the narrowest maintainable fix and matches the sibling response-snippet path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a narrow user-visible diagnostic rendering bug with limited blast radius and a clear focused fix path.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR has regression-test and CI evidence only; it still needs redacted live output or logs showing an after-fix MiniMax/PDF-provider error diagnostic, with private details removed. 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 +6, Tests +26. Total +32 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	12	6	+6
Tests	1	26	0	+26
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	38	6	+32

What I checked:

Current main bug source: Current main returns raw .slice(0, limits.maxChars) for both body-less and streamed readResponseBodySnippet results, which can split a surrogate pair at the character cap. (src/infra/http-error-body.ts:15, 87fe266c5e4d)
PR diff fixes all return sites: The live PR diff imports truncateUtf16Safe and applies it to all three truncating return paths in readResponseBodySnippet. (src/infra/http-error-body.ts:1, 3048532f531d)
Regression test coverage: The PR adds table-driven tests for body-less under byte limit, body-less byte truncation, and streamed response paths using supplementary Unicode at the truncation boundary. (src/infra/http-error-body.test.ts:96, 3048532f531d)
Sibling implementation pattern: readResponseTextSnippet already uses truncateUtf16Safe(collapsed, maxChars) for diagnostic body text truncation, supporting the PR’s reuse of the existing helper. (src/infra/http-body.ts:317, 87fe266c5e4d)
Helper contract: truncateUtf16Safe floors the limit and delegates to sliceUtf16Safe, whose boundary checks avoid returning dangling surrogate halves. (packages/normalization-core/src/utf16-slice.ts:44, 87fe266c5e4d)
Runtime callers: The helper feeds user-visible HTTP failure diagnostics for MiniMax VLM and native PDF provider responses. (src/agents/minimax-vlm.ts:134, 87fe266c5e4d)

Likely related people:

steipete: Blame and PR metadata tie the current error-body helper introduction, sibling http-body.ts UTF-16-safe truncation change, and merged related UTF-16 consolidation PRs to this account. (role: recent area contributor; confidence: high; commits: 942b44966107, a9582a1bb62a, 176fee5d071d; files: src/infra/http-error-body.ts, src/infra/http-body.ts, packages/normalization-core/src/utf16-slice.ts)
wings1029: This account authored the current PR and appears as a co-author on the merged related UTF-16 truncation consolidation, but the current-main code ownership trail is weaker than for steipete. (role: related contributor; confidence: low; commits: 176fee5d071d; files: src/infra/http-error-body.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.

Review history (1 earlier review cycle)

reviewed 2026-07-07T14:45:05.197Z sha 47502c6 :: needs real behavior proof before merge. :: none

steipete · 2026-07-07T15:06:01Z

Land-ready maintainer review complete on exact head 3048532f531da5abeb0c28d62ab171e8b3c90a03.

Replaced the original surrogate-inspection loops with exact-output, table-driven coverage for all three reachable reader branches: bodyless under the byte cap, bodyless over the byte cap, and streamed.
Reviewed the shared reader, both production callers (minimax-vlm and PDF native providers), the sibling bounded-body reader, and the normalization-core UTF-16 helper. This is the correct owner boundary and the smallest complete fix.
git diff --check: pass.
Fresh branch autoreview against origin/main: clean, 0.98 confidence.
Exact-head GitHub CI run 28875806869: success, including compile/lint/type gates, all test shards, QA matrix, and Telegram smoke.

No docs, config, migration, security, or changelog changes are required. No known proof gaps.

steipete · 2026-07-07T15:06:19Z

Merged via squash.

Prepared head SHA: 3048532f531da5abeb0c28d62ab171e8b3c90a03
Landed commit: d633a2df422971154f5f819072fed8ba6142cf75

…r body truncation (openclaw#101728) * fix(http-error-body): keep emoji / surrogate pairs intact during error body truncation Replace .slice(0, limits.maxChars) with truncateUtf16Safe() so that emoji straddling the maxChars boundary don't produce lone surrogates that render as � in error diagnostics. Matches the sibling http-body.ts which already uses truncateUtf16Safe. Co-Authored-By: Claude <[email protected]> * test: cover UTF-16 HTTP body branches --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

…cript truncation UTF-16 safe normalizeCliContextValue (cli-output.ts) and formatSkillExperienceReviewTranscript (experience-review-prompt.ts) both used raw .slice(0, N) / .slice(-N) to truncate user-facing strings. When a truncation boundary falls inside a surrogate pair (emoji), the resulting string contains a lone surrogate — encodeURIComponent throws URIError, JSON serialisers may reject it, and downstream display surfaces can render garbage. Replace .slice(0, 200) with truncateUtf16Safe in normalizeCliContextValue. Replace .slice(0, 6_000) with truncateUtf16Safe and full.slice(-tailBudget) with sliceUtf16Safe(full, tailStart) in formatSkillExperienceReviewTranscript. Follows the pattern established by openclaw#98644, openclaw#102470, openclaw#101728 and the broader UTF-16 safety sweep already applied across the codebase.

…cript truncation UTF-16 safe (#106370) * fix(agents,skills): keep session-identity and experience-review transcript truncation UTF-16 safe normalizeCliContextValue (cli-output.ts) and formatSkillExperienceReviewTranscript (experience-review-prompt.ts) both used raw .slice(0, N) / .slice(-N) to truncate user-facing strings. When a truncation boundary falls inside a surrogate pair (emoji), the resulting string contains a lone surrogate — encodeURIComponent throws URIError, JSON serialisers may reject it, and downstream display surfaces can render garbage. Replace .slice(0, 200) with truncateUtf16Safe in normalizeCliContextValue. Replace .slice(0, 6_000) with truncateUtf16Safe and full.slice(-tailBudget) with sliceUtf16Safe(full, tailStart) in formatSkillExperienceReviewTranscript. Follows the pattern established by #98644, #102470, #101728 and the broader UTF-16 safety sweep already applied across the codebase. * chore: re-trigger CI after PR body update * test(agents,skills): construct transcripts exceeding 60k chars, verify old slices dangle Both experience-review fixtures now exceed EXPERIENCE_REVIEW_MAX_TRANSCRIPT_CHARS so the truncation branch actually fires. Pre-condition assertions prove the old raw .slice(0,6000) / .slice(tailStart) produce isolated surrogates; the production functions (formatSkillExperienceReviewTranscript, formatCliOutputError) do not. cli-output test also adds pre-condition check showing normalizeCliContextValue with raw .slice(0,200) would split a surrogate pair. * test: tighten UTF-16 truncation coverage Co-authored-by: 毛宇豪0668001457 <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>

…cript truncation UTF-16 safe (openclaw#106370) * fix(agents,skills): keep session-identity and experience-review transcript truncation UTF-16 safe normalizeCliContextValue (cli-output.ts) and formatSkillExperienceReviewTranscript (experience-review-prompt.ts) both used raw .slice(0, N) / .slice(-N) to truncate user-facing strings. When a truncation boundary falls inside a surrogate pair (emoji), the resulting string contains a lone surrogate — encodeURIComponent throws URIError, JSON serialisers may reject it, and downstream display surfaces can render garbage. Replace .slice(0, 200) with truncateUtf16Safe in normalizeCliContextValue. Replace .slice(0, 6_000) with truncateUtf16Safe and full.slice(-tailBudget) with sliceUtf16Safe(full, tailStart) in formatSkillExperienceReviewTranscript. Follows the pattern established by openclaw#98644, openclaw#102470, openclaw#101728 and the broader UTF-16 safety sweep already applied across the codebase. * chore: re-trigger CI after PR body update * test(agents,skills): construct transcripts exceeding 60k chars, verify old slices dangle Both experience-review fixtures now exceed EXPERIENCE_REVIEW_MAX_TRANSCRIPT_CHARS so the truncation branch actually fires. Pre-condition assertions prove the old raw .slice(0,6000) / .slice(tailStart) produce isolated surrogates; the production functions (formatSkillExperienceReviewTranscript, formatCliOutputError) do not. cli-output test also adds pre-condition check showing normalizeCliContextValue with raw .slice(0,200) would split a surrogate pair. * test: tighten UTF-16 truncation coverage Co-authored-by: 毛宇豪0668001457 <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added the size: S label Jul 7, 2026

test: cover UTF-16 HTTP body branches

3048532

openclaw-barnacle Bot added size: XS and removed size: S labels Jul 7, 2026

steipete merged commit d633a2d into openclaw:main Jul 7, 2026
113 of 116 checks passed

wings1029 mentioned this pull request Jul 7, 2026

fix: keep emoji intact at remaining text truncation boundaries #101754

Merged

github-actions Bot mentioned this pull request Jul 7, 2026

📡 Upstream Digest — 2026-07-07 17:52 UTC curtismercier/openclaw-mods#1231

Open

Monkey-wusky mentioned this pull request Jul 13, 2026

fix(agents,skills): keep session-identity and experience-review transcript truncation UTF-16 safe #106370

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(http-error-body): keep emoji / surrogate pairs intact during error body truncation#101728

fix(http-error-body): keep emoji / surrogate pairs intact during error body truncation#101728
steipete merged 2 commits into
openclaw:mainfrom
wings1029:fix/utf16-safe-http-error-body

wings1029 commented Jul 7, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wings1029 commented Jul 7, 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 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wings1029 commented Jul 7, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading