Skip to content

fix(gateway): keep chat history display text truncation surrogate-safe#102470

Merged
steipete merged 2 commits into
openclaw:mainfrom
zw-xysk:fix/chat-display-safe
Jul 9, 2026
Merged

fix(gateway): keep chat history display text truncation surrogate-safe#102470
steipete merged 2 commits into
openclaw:mainfrom
zw-xysk:fix/chat-display-safe

Conversation

@zw-xysk

@zw-xysk zw-xysk commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Chat-history display projection truncated text with a raw UTF-16 slice. When the configured display boundary fell inside an emoji or another supplementary character, the Control UI/history payload could contain a lone surrogate that rendered as a replacement character.

Why This Change Was Made

The owning truncateChatHistoryText helper now uses the shared normalization-core truncateUtf16Safe primitive. This covers assistant text, message previews, thinking text, and other projected display fields while preserving the existing code-unit cap and truncation marker.

A production-path regression drives assistant content through sanitizeChatHistoryMessages at a split surrogate boundary and asserts the exact projected payload.

User Impact

Control UI and chat-history previews no longer display a replacement glyph when emoji or other supplementary Unicode characters cross a display truncation boundary.

Evidence

  • Exact head: b9e92b96b6015fbb5a464e76d4126cfb9b9f2e23
  • node scripts/run-vitest.mjs src/gateway/server-methods/server-methods.test.ts — 1 file, 171 tests passed.
  • Targeted oxfmt and git diff --check passed.
  • Fresh uncommitted autoreview: clean, no accepted/actionable findings (0.99).

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.

truncateChatHistoryText uses slice(0, N) to truncate chat history text
for the Control UI. When the truncation boundary falls inside a UTF-16
surrogate pair (emoji, CJK extended), the resulting string contains a
dangling surrogate that browsers render as U+FFFD (�).

Replace slice(0, maxChars) with truncateUtf16Safe(text, maxChars) so
the truncation point always falls on a complete code-point boundary.
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Jul 9, 2026
@zw-xysk

zw-xysk commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

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

Summary
The branch replaces raw chat-history display text slicing with truncateUtf16Safe and adds a regression test for assistant text at a surrogate boundary.

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

Reproducibility: yes. source-reproducible: current main and the latest release use raw text.slice(0, maxChars) in truncateChatHistoryText, so a supplementary character crossing the display cap can leave a lone surrogate. I did not run tests in this read-only review.

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 after-fix real behavior proof, such as standalone terminal output from the projection/gateway path or a short Control UI chat-history recording showing the boundary text without a replacement glyph.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Mock-only: the PR body reports a Vitest run, formatting, and autoreview but no after-fix real gateway/UI output; add redacted terminal output, logs, or a short UI/gateway recording and update the PR body to trigger a fresh review, or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] The PR body currently provides test, formatting, and autoreview evidence only; external-PR merge confidence would be stronger with redacted after-fix real behavior proof from the gateway/UI path or an equivalent standalone terminal run.

Maintainer options:

  1. Decide the mitigation before merge
    Land the helper substitution after exact-head gates once the PR includes redacted real behavior proof showing the gateway/UI projection no longer emits a replacement glyph at the boundary.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] No repair lane is needed because there is no actionable code finding; the remaining blocker is human/contributor proof review rather than an automatable code change.

Security
Cleared: The diff only imports an existing workspace helper and adds a focused test; it does not change dependencies, workflows, secrets, permissions, packages, or code-execution surfaces.

Review details

Best possible solution:

Land the helper substitution after exact-head gates once the PR includes redacted real behavior proof showing the gateway/UI projection no longer emits a replacement glyph at the boundary.

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

Yes, source-reproducible: current main and the latest release use raw text.slice(0, maxChars) in truncateChatHistoryText, so a supplementary character crossing the display cap can leave a lone surrogate. I did not run tests in this read-only review.

Is this the best way to solve the issue?

Yes for the code shape: reusing the existing truncateUtf16Safe helper at the shared chat-history display truncation helper is the narrowest maintainable fix. A UI-side workaround or new helper would duplicate the existing normalization-core contract.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Mock-only: the PR body reports a Vitest run, formatting, and autoreview but no after-fix real gateway/UI output; add redacted terminal output, logs, or a short UI/gateway recording and update the PR body to trigger a fresh review, or ask a maintainer to comment @clawsweeper re-review.
  • remove proof: sufficient: Current real behavior proof status is mock_only, not sufficient.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: 📣 needs proof.

Label justifications:

  • P2: This is a normal-priority user-visible text correctness fix for Control UI chat-history display with a narrow gateway blast radius.
  • 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: Mock-only: the PR body reports a Vitest run, formatting, and autoreview but no after-fix real gateway/UI output; add redacted terminal output, logs, or a short UI/gateway recording and update the PR body to trigger a fresh review, or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

PR surface:

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

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
Tests 1 22 1 +21
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 24 2 +22

What I checked:

Likely related people:

  • steipete: Peter Steinberger authored and merged adjacent UTF-16 truncation cleanup PRs and added the current PR's gateway regression test commit. (role: recent UTF-16 truncation consolidation owner and current PR test contributor; confidence: high; commits: b9e92b96b601, 87fe266c5e4d, e5bab02257c8; files: src/gateway/server-methods/server-methods.test.ts, packages/normalization-core/src/utf16-slice.ts, ui/src/lib/format.ts)
  • vincentkoc: Current blame for the chat-display helper and shared UTF-16 helper points at commit 6630138, and adjacent backend UTF-16 truncation work was merged under this area history. (role: current gateway baseline contributor and adjacent backend UTF-16 merger; confidence: medium; commits: 6630138598b3, 66081c09ee50; files: src/gateway/chat-display-projection.ts, packages/normalization-core/src/utf16-slice.ts, src/agents/acp-spawn-parent-stream.ts)
  • xialonglee: Authored the merged backend UTF-16-safe truncation PR that established the same helper pattern on nearby backend display and relay boundaries. (role: adjacent merged-fix author; confidence: medium; commits: 66081c09ee50, c0843bf40267; files: src/agents/acp-spawn-parent-stream.ts, extensions/active-memory/index.ts)
  • zw-xysk: Beyond this PR, the same contributor authored the merged UI truncation PR that replaced raw slices with truncateUtf16Safe on user-visible text boundaries. (role: adjacent current-main UTF-16 truncation contributor; confidence: medium; commits: b89a49051cf1, aaa985845d5d; files: ui/src/lib/format.ts, src/gateway/chat-display-projection.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-09T06:52:12.289Z sha aaa9858 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jul 9, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 9, 2026
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Land-ready maintainer pass complete at exact head b9e92b96b6015fbb5a464e76d4126cfb9b9f2e23.

The shared gateway fix was correct but untested in the diff. I added a production-path regression through sanitizeChatHistoryMessages that asserts the exact Control UI/history payload at a split surrogate boundary.

Proof:

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
steipete merged commit a6d45c4 into openclaw:main Jul 9, 2026
124 of 132 checks passed
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 9, 2026
openclaw#102470)

* fix(gateway): keep chat history display text truncation surrogate-safe

truncateChatHistoryText uses slice(0, N) to truncate chat history text
for the Control UI. When the truncation boundary falls inside a UTF-16
surrogate pair (emoji, CJK extended), the resulting string contains a
dangling surrogate that browsers render as U+FFFD (�).

Replace slice(0, maxChars) with truncateUtf16Safe(text, maxChars) so
the truncation point always falls on a complete code-point boundary.

* test(gateway): cover chat display UTF-16 boundary

---------

Co-authored-by: Peter Steinberger <[email protected]>
Monkey-wusky added a commit to Monkey-wusky/openclaw that referenced this pull request Jul 14, 2026
…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.
Monkey-wusky added a commit to Monkey-wusky/openclaw that referenced this pull request Jul 14, 2026
…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.
Monkey-wusky added a commit to Monkey-wusky/openclaw that referenced this pull request Jul 14, 2026
…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.
Monkey-wusky added a commit to Monkey-wusky/openclaw that referenced this pull request Jul 16, 2026
…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.
steipete added a commit that referenced this pull request Jul 16, 2026
…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]>
wangmiao0668000666 pushed a commit to wangmiao0668000666/openclaw that referenced this pull request Jul 16, 2026
…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]>
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 17, 2026
…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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants