fix(tui): keep emoji / surrogate pairs intact during abort diagnostic truncation by maweibin · Pull Request #101641 · openclaw/openclaw

maweibin · 2026-07-07T11:59:23Z

What Problem This Solves

the TUI abort diagnostic truncator in src/tui/tui-event-handlers.ts truncates text with .slice() which counts UTF-16 code units. When a surrogate-pair character (emoji) straddles the cut boundary, .slice() produces a lone surrogate (e.g. \ud83d) that renders as � in terminal output.

This is user-visible: the truncated text comes from real user-facing output, so any content containing emoji near the 159-character boundary can hit this.

Why This Change Was Made

Replace .slice() with truncateUtf16Safe() from @openclaw/normalization-core/utf16-slice so the truncation counts full code points instead of UTF-16 code units. This matches the already-merged PR #101517 (session-cost-usage) and the existing shortenText helper in src/commands/text-format.ts:5.

User Impact

Users will no longer see � replacement characters in this output when emoji appear near truncation boundaries.

Evidence

Real environment tested: local OpenClaw source checkout, Node v24.13.1, PR head.

Exact steps after this patch:

node --import tsx -e "
const { truncateUtf16Safe } = await import('@openclaw/normalization-core/utf16-slice');
const content = 'x'.repeat(159) + '🚀tail';
const before = content.slice(0, 159 + 1);
const after = truncateUtf16Safe(content, 159 + 1);
const t = (s) => /[\uD800-\uDBFF](?![\uDC00-\uDFFF])/.test(s);
console.log('BEFORE (.slice):', JSON.stringify(before.slice(-10)), 'lone:', t(before));
console.log('AFTER  (truncateUtf16Safe):', JSON.stringify(after.slice(-10)), 'lone:', t(after));
"

Evidence after fix:

BEFORE (.slice):           "xxxxxxx\ud83d…"  lone: true
AFTER  (truncateUtf16Safe): "xxxxxxxxx…"       lone: false

Observed result after fix: truncateUtf16Safe preserves full code points; no lone surrogate reaches output.

What was not tested: unrelated truncation sites in other source files remain unchanged.

Regression Test Plan

node scripts/run-vitest.mjs src/tui/tui-event-handlers.test.ts --run

AI-assisted.

… truncation Use truncateUtf16Safe() instead of .slice() to count full code points. Co-Authored-By: Claude Opus 4.8 <[email protected]>

clawsweeper · 2026-07-07T12:21:57Z

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 8:25 AM ET / 12:25 UTC.

Summary
The PR changes TUI abort diagnostic truncation from raw .slice() to truncateUtf16Safe() and adds a surrogate-pair truncation test.

PR surface: Source +1, Tests +11. Total +12 across 2 files.

Reproducibility: yes. source-reproducible: current main slices a long abort diagnostic at a UTF-16 code-unit boundary before rendering run aborted: ..., so an emoji whose high surrogate lands at the cut point can render as a replacement character. I did not run the TUI test locally because this cleanup review is read-only.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

Replace the helper-only test with an abort-handler regression through createEventHandlers().handleChatEvent.
[P1] Add changed-path real behavior proof showing the rendered abort line no longer contains a dangling surrogate or replacement character.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes terminal output for the helper versus .slice(), but not for the changed TUI abort path; add terminal output, logs, or a small live transcript for the rendered abort line after redacting private details, then update the PR body to trigger re-review.

Risk before merge

[P1] The new regression test can pass while the TUI abort formatter remains broken, because it validates the helper directly instead of routing through handleChatEvent.
[P1] The PR body proof shows helper behavior, not the changed user-visible abort diagnostic output, so the external-PR real-behavior proof gate is not met yet.

Maintainer options:

Decide the mitigation before merge
Keep the helper-based production change, but make the regression and proof drive createEventHandlers().handleChatEvent with an emoji-boundary errorMessage and assert the rendered abort line stays surrogate-safe.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] Contributor action is needed for changed-path real behavior proof, and the concrete test repair should come with that proof rather than a ClawSweeper repair marker.

Security
Cleared: The diff only changes TypeScript string truncation and a focused test, with no dependency, workflow, secret, permission, network, or code-execution surface added.

Review findings

[P2] Exercise the abort handler, not the helper — src/tui/tui-event-handlers.test.ts:2324-2329

Review details

Best possible solution:

Keep the helper-based production change, but make the regression and proof drive createEventHandlers().handleChatEvent with an emoji-boundary errorMessage and assert the rendered abort line stays surrogate-safe.

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

Yes, source-reproducible: current main slices a long abort diagnostic at a UTF-16 code-unit boundary before rendering run aborted: ..., so an emoji whose high surrogate lands at the cut point can render as a replacement character. I did not run the TUI test locally because this cleanup review is read-only.

Is this the best way to solve the issue?

No, not merge-ready as submitted. The production change is the right layer, but the test and real-behavior proof should exercise the TUI abort handler instead of the helper in isolation.

Full review comments:

[P2] Exercise the abort handler, not the helper — src/tui/tui-event-handlers.test.ts:2324-2329
This test imports truncateUtf16Safe directly, so it would still pass if formatAbortDiagnostic kept using raw .slice() and the TUI continued emitting a lone surrogate. Please route an emoji-boundary errorMessage through createEventHandlers().handleChatEvent and assert the chatLog.addSystem abort line contains no dangling surrogate or replacement character.
Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P2: This is a narrow user-visible TUI formatting bug with limited blast radius and a focused PR fix, but it still needs changed-path proof before merge.
add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal output for the helper versus .slice(), but not for the changed TUI abort path; add terminal output, logs, or a small live transcript for the rendered abort line after redacting private details, then update the PR body to trigger re-review.

Label justifications:

P2: This is a narrow user-visible TUI formatting bug with limited blast radius and a focused PR fix, but it still needs changed-path proof before merge.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal output for the helper versus .slice(), but not for the changed TUI abort path; add terminal output, logs, or a small live transcript for the rendered abort line after redacting private details, then update the PR body to trigger re-review.

Evidence reviewed

PR surface:

Source +1, Tests +11. Total +12 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	3	2	+1
Tests	1	11	0	+11
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	14	2	+12

What I checked:

Current main bug path: Current main still truncates sanitized abort diagnostics with raw UTF-16 .slice(0, MAX_ABORT_DIAGNOSTIC_LENGTH - 1), which can leave a dangling high surrogate before the ellipsis. (src/tui/tui-event-handlers.ts:56, 60f0749b7fb3)
Abort diagnostic call site: Aborted chat events pass evt.errorMessage through formatAbortDiagnostic and render it via chatLog.addSystem, so the malformed truncation reaches visible TUI output. (src/tui/tui-event-handlers.ts:767, 60f0749b7fb3)
PR production change: The proposed merge result replaces the raw slice with truncateUtf16Safe(diagnostic, MAX_ABORT_DIAGNOSTIC_LENGTH - 1) while preserving the existing ellipsis behavior. (src/tui/tui-event-handlers.ts:57, 259d8a3729c2)
Regression test gap: The added test imports truncateUtf16Safe directly and would still pass if formatAbortDiagnostic reverted to raw .slice(), so it does not guard the changed TUI behavior. (src/tui/tui-event-handlers.test.ts:2324, 73b475bc0c81)
Helper contract: truncateUtf16Safe delegates to sliceUtf16Safe, which adjusts slice boundaries to avoid returning dangling surrogate halves. (packages/normalization-core/src/utf16-slice.ts:44, 60f0749b7fb3)
History provenance: Blame ties the current abort diagnostic formatter and raw slice line to the TUI event-handler extraction in commit 395fbb8, merged from the busy approval queue fix. (src/tui/tui-event-handlers.ts:56, 395fbb8eb631)

Likely related people:

steipete: Authored and merged the current TUI event-handler extraction that introduced the abort diagnostic formatter and visible abort line. (role: recent area contributor and merger; confidence: high; commits: 395fbb8eb631; files: src/tui/tui-event-handlers.ts, src/tui/tui-event-handlers.test.ts)
vincentkoc: Co-authored the same TUI event-handler extraction commit that now owns the abort diagnostic path. (role: adjacent contributor; confidence: medium; commits: 395fbb8eb631; files: src/tui/tui-event-handlers.ts, src/tui/tui-event-handlers.test.ts)
maweibin: Authored the recently merged related PR that applied the same surrogate-safe truncation pattern to session cost usage output. (role: adjacent UTF-16 truncation contributor; confidence: medium; commits: afdb9fd26408; files: src/infra/session-cost-usage.ts, src/infra/session-cost-usage.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.

maweibin · 2026-07-07T13:00:14Z

Hi @steipete, thanks for consolidating the emoji surrogate-pair truncation fixes into #101654!

This PR (#101641) fixes the same class of UTF-16 truncation bug in src/tui/tui-event-handlers.ts (abort diagnostic formatter), which was not covered by #101654.

Would you consider folding this remaining location into the same consolidated fix approach? I'm happy to adjust the PR in whatever way you prefer — either as a standalone or integrated with #101654's pattern.

Let me know your thoughts, happy to adapt.

steipete · 2026-07-07T13:50:36Z

Closed as superseded by #101685, landed in 176fee5d071dd03f6cbbc54b029f23380b04e911. The replacement carries this useful UTF-16 boundary fix on current main, uses the existing shared helper, expands caller-level coverage, and preserves your commit credit in the consolidated commit. Thank you, @maweibin, for finding and fixing this.

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

fix(tui): keep emoji / surrogate pairs intact during abort diagnostic…

73b475b

… truncation Use truncateUtf16Safe() instead of .slice() to count full code points. Co-Authored-By: Claude Opus 4.8 <[email protected]>

maweibin force-pushed the fix/tui-event-emoji-truncation branch from cac4835 to 73b475b Compare July 7, 2026 12:09

maweibin mentioned this pull request Jul 7, 2026

fix: keep bounded text truncation UTF-16 safe #101654

Merged

maweibin mentioned this pull request Jul 7, 2026

fix(agents): keep emoji / surrogate pairs intact during error observation truncation #101642

Closed

steipete mentioned this pull request Jul 7, 2026

fix: prevent garbled emoji at remaining text limits #101685

Merged

steipete closed this Jul 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(tui): keep emoji / surrogate pairs intact during abort diagnostic truncation#101641

fix(tui): keep emoji / surrogate pairs intact during abort diagnostic truncation#101641
maweibin wants to merge 1 commit into
openclaw:mainfrom
maweibin:fix/tui-event-emoji-truncation

maweibin commented Jul 7, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

maweibin commented Jul 7, 2026

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

maweibin commented Jul 7, 2026 • edited 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

Regression Test Plan

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!

maweibin commented Jul 7, 2026

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

maweibin commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading