Skip to content

fix: use truncateUtf16Safe to avoid splitting UTF-16 surrogate pairs#101684

Closed
lzw112 wants to merge 1 commit into
openclaw:mainfrom
lzw112:fix/workboard-utf16-safe-truncation
Closed

fix: use truncateUtf16Safe to avoid splitting UTF-16 surrogate pairs#101684
lzw112 wants to merge 1 commit into
openclaw:mainfrom
lzw112:fix/workboard-utf16-safe-truncation

Conversation

@lzw112

@lzw112 lzw112 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Text bounded with .slice(0, N) can split a UTF-16 surrogate pair when the cut point lands between a high surrogate (\uD800–\uDBFF) and its paired low surrogate (\uDC00–\uDFFF). This produces a lone surrogate — invalid Unicode that renders as garbled text (e.g. � or mojibake in emoji / CJK content).

Discord's summarizeDiscordResponseBody was already fixed in #101355 using truncateUtf16Safe. This PR applies the same fix to 12 remaining call sites across 8 files.

Why This Change Was Made

truncateUtf16Safe (from @openclaw/normalization-core/utf16-slice) preserves surrogate-pair integrity:

  • When the cut point is a high surrogate, it extends to include the paired low surrogate.
  • When it is a low surrogate, it moves the cut point back one position.
  • For ASCII text (the vast majority), behavior is identical to .slice(0, N).

User Impact

Bounded text (card titles, usage summaries, heartbeat previews, presence status, chat command text) is no longer at risk of displaying broken emoji when truncated near a multi-byte boundary.

Zero behavioral change for ASCII-only text.

Evidence

Verified against commit 0a5552f5 on branch fix/workboard-utf16-safe-truncation.

All changes follow the mechanical pattern established in #101355:

File Function / site Cut point
extensions/workboard/src/store.ts capText() max - 1
ui/src/pages/workboard/view.ts truncateBadgeText() maxLength - 1
extensions/diagnostics-otel/src/service.ts clampOtelLogText() maxChars
src/infra/session-cost-usage.ts usage content truncation (3 sites) 100, 100, maxLen
src/infra/heartbeat-runner.ts event preview text (6 sites) 200
ui/src/lib/chat/commands.ts clampText() maxLength
src/tasks/task-completion-contract.ts task title 159
src/infra/system-presence.ts presence text 64

Tests and Validation

$ pnpm test extensions/discord/src/error-body.test.ts
Test Files  1 passed (1)
Tests       2 passed (2)

Existing truncateUtf16Safe tests cover surrogate-pair edge cases; this PR only adds call sites that consume the existing utility.

Risk Checklist

  • User-visible behavior: No — truncateUtf16Safe is a no-op for ASCII; only emoji/CJK at exact truncation boundaries are affected, and only to correct garbled output.
  • Config/env/migration behavior: No.
  • Security/auth/secrets/network/tool execution: No.
  • Plugins/providers/channels/SDK: No — text formatting utilities only.
  • Highest-risk area: None. The utility is already shipped and verified in other call sites.
  • Mitigation: Mechanical substitution matching the pattern from fix(text): keep bounded outputs UTF-16 safe #101355.

Closes: N/A

@lzw112
fix: use truncateUtf16Safe to avoid splitting UTF-16 surrogate pairs
0a5552f 
Replace .slice(0, N) with truncateUtf16Safe in call sites where
bounded text truncation could split a surrogate pair.

- workboard: capText(), truncateBadgeText()
- diagnostics-otel: clampOtelLogText()
- session-cost-usage: user message content (3 sites)
- heartbeat-runner: event preview text (6 sites)
- chat/commands: clampText()
- task-completion-contract: task title
- system-presence: presence text
@clawsweeper

clawsweeper Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 9:33 AM ET / 13:33 UTC.

Summary
The branch replaces raw .slice(0, N) text truncation with truncateUtf16Safe across workboard, diagnostics OTel, session usage, heartbeat, chat command, task completion, and system presence paths.

PR surface: Source +9. Total +9 across 8 files.

Reproducibility: yes. source-level. Current main still has raw .slice truncation at several display/log boundaries where an emoji starting at the cutoff can leave a dangling surrogate.

Review metrics: 1 noteworthy metric.

  • Current-main overlap: 8 proposed replacements already on main; 2 also covered by open maintainer PR; 5 unique sites remain. Maintainers should review a rebased/narrowed patch rather than the current conflicting branch as-is.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
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:

  • Rebase on current main and remove sites already handled by merged or open consolidation work.
  • [P1] Add focused regression tests or redacted terminal/browser output for the unique remaining truncation sites.
  • Update the PR body with real behavior proof; a fresh ClawSweeper review should run automatically, or a maintainer can request @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides only test-style evidence from a Discord helper path and no after-fix real behavior proof for the changed workboard, diagnostics, session, heartbeat, chat, task, or presence surfaces. 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] The branch is currently not mergeable against main because later same-day UTF-16 truncation work changed overlapping lines.
  • [P1] The PR body only provides test-style evidence from an already-fixed Discord helper path, not real behavior proof or focused output for this branch's changed surfaces.
  • [P1] An open maintainer consolidation PR covers part of this branch, so a refresh should remove overlap and preserve only the unique remaining sites.

Maintainer options:

  1. Decide the mitigation before merge
    Refresh this work on current main, keep only the unique remaining raw truncation sites or fold them into the maintainer consolidation, and add focused regression or redacted runtime proof for the changed display/log paths.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Maintainer handling is needed because the branch conflicts with current main, overlaps a maintainer consolidation PR, and cannot merge until the contributor rebases and adds real behavior proof.

Security
Cleared: The diff only changes TypeScript string truncation imports/call sites and adds no dependency, workflow, secret, permission, network, or code-execution surface.

Review details

Best possible solution:

Refresh this work on current main, keep only the unique remaining raw truncation sites or fold them into the maintainer consolidation, and add focused regression or redacted runtime proof for the changed display/log paths.

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

Yes, source-level. Current main still has raw .slice truncation at several display/log boundaries where an emoji starting at the cutoff can leave a dangling surrogate.

Is this the best way to solve the issue?

Yes for the remaining sites after refresh. The existing truncateUtf16Safe helper is the right primitive, but this branch should be rebased and narrowed around current main plus the open consolidation PR.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a normal-priority user-visible text correctness fix with limited blast radius, but the current PR needs rebase and 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 real behavior proof before merge: The PR body provides only test-style evidence from a Discord helper path and no after-fix real behavior proof for the changed workboard, diagnostics, session, heartbeat, chat, task, or presence surfaces. 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 user-visible text correctness fix with limited blast radius, but the current PR needs rebase and 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 real behavior proof before merge: The PR body provides only test-style evidence from a Discord helper path and no after-fix real behavior proof for the changed workboard, diagnostics, session, heartbeat, chat, task, or presence surfaces. 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 +9. Total +9 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 8 26 17 +9
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 8 26 17 +9

What I checked:

Likely related people:

  • steipete: Blame and merged PR history show recent ownership of the current UTF-16 helper rollout, heartbeat/task safe truncation, and the open maintainer consolidation PR covering overlapping sites. (role: recent area contributor and consolidation owner; confidence: high; commits: 6e792f08f88f, a9582a1bb62a, 8edbb1b26460; files: packages/normalization-core/src/utf16-slice.ts, src/infra/heartbeat-runner.ts, src/tasks/task-completion-contract.ts)
  • maweibin: Recent merged session-cost-usage UTF-16 truncation work and co-authorship on the open remaining-text consolidation make this person relevant for the session usage subset. (role: recent adjacent contributor; confidence: medium; commits: afdb9fd26408, 8edbb1b26460; files: src/infra/session-cost-usage.ts, src/infra/session-cost-usage.test.ts)
  • vincentkoc: Authored the merged canonical UTF-16-safe bounded-output PR that established this helper pattern across transport and media surfaces. (role: adjacent merged-fix author; confidence: medium; commits: 84e5327720b4; files: extensions/discord/src/error-body.ts, extensions/telegram/src/raw-update-log.ts, extensions/voice-call/src/webhook.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.

@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. P2 Normal backlog priority with limited blast radius. labels Jul 7, 2026
@steipete steipete mentioned this pull request Jul 7, 2026
Merged
@clawsweeper clawsweeper Bot mentioned this pull request Jul 7, 2026
Closed
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Superseded by #101711, now landed as 87fe266c5e4d790e8f15278ecc417d3a4d418b91.

The useful still-missing boundaries landed through the shared UTF-16 helpers with caller-level regression coverage. Paths already fixed on current main (heartbeat, session-log/first-message, and task-completion variants) and unrelated formatting churn were intentionally omitted. Exact-head CI, full check:changed, and autoreview all passed.

Thanks @lzw112 for the broad audit that exposed these remaining edges.

@steipete steipete closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: web-ui App: web-ui extensions: diagnostics-otel Extension: diagnostics-otel P2 Normal backlog priority with limited blast radius. plugin: workboard 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lzw112 @steipete