fix(audit): keep emoji / surrogate pairs intact during audit text truncation by maweibin · Pull Request #101628 · openclaw/openclaw

maweibin · 2026-07-07T11:49:21Z

What Problem This Solves

the audit text shortener in src/commands/audit.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 78-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(78) + '🚀tail';
const before = content.slice(0, 78 + 1);
const after = truncateUtf16Safe(content, 78 + 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/commands/audit.test.ts --run

AI-assisted.

…ncation Use truncateUtf16Safe() instead of .slice() to count full code points, preventing lone surrogates in audit CLI output. Co-Authored-By: Claude Opus 4.8 <[email protected]>

clawsweeper · 2026-07-07T12:09:39Z

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

Summary
The PR replaces audit CLI text truncation with truncateUtf16Safe and adds a surrogate-pair regression test.

PR surface: Source +3, Tests +13. Total +16 across 2 files.

Reproducibility: yes. by source inspection: current main sanitizes audit text and then truncates with UTF-16 .slice(), which can split a surrogate pair at the boundary. I did not run the audit CLI path because this read-only review forbids test/artifact-producing validation in the checkout.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
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:

[P1] Add real behavior proof that runs the audit formatter or CLI with an emoji at the truncation boundary, redacting private data.
Update the regression test to call testApi.formatAuditRows() so it protects the changed code path.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes terminal output for truncateUtf16Safe itself, but it does not show the changed audit formatter or audit CLI output after the fix; add redacted terminal output/logs for that path before merge. 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 posted proof and added regression test exercise truncateUtf16Safe directly rather than the audit formatter or CLI output, so the user-visible path is still relying on code inspection before merge.

Maintainer options:

Decide the mitigation before merge
Land the narrow audit truncation fix after the regression proof exercises testApi.formatAuditRows() or the audit CLI output with an emoji at the truncation boundary.
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 repair while the external-contributor proof gate is unmet; the contributor should add audit-path proof and the small test adjustment.

Security
Cleared: The diff only changes an internal CLI formatter and colocated test; it adds no dependency, workflow, credential, package, or code-execution surface.

Review findings

[P3] Exercise the audit formatter in the regression test — src/commands/audit.test.ts:47-54

Review details

Best possible solution:

Land the narrow audit truncation fix after the regression proof exercises testApi.formatAuditRows() or the audit CLI output with an emoji at the truncation boundary.

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

Yes, by source inspection: current main sanitizes audit text and then truncates with UTF-16 .slice(), which can split a surrogate pair at the boundary. I did not run the audit CLI path because this read-only review forbids test/artifact-producing validation in the checkout.

Is this the best way to solve the issue?

Yes for the implementation shape: using the existing truncateUtf16Safe helper inside short() is the narrowest maintainable fix for the audit formatter. The regression test and real-behavior proof should be adjusted to exercise the audit formatter rather than the helper alone.

Full review comments:

[P3] Exercise the audit formatter in the regression test — src/commands/audit.test.ts:47-54
The new test imports truncateUtf16Safe directly, so it would still pass if short() or formatAuditRows() kept using .slice() or regressed later. Please feed an audit event with an emoji at the toolName, agentId, or runId boundary through testApi.formatAuditRows() and assert the rendered row has no lone surrogate or replacement character.
Confidence: 0.88

Overall correctness: patch is correct
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P2: This is a focused user-visible CLI text correctness fix with limited blast radius and no evidence of broader runtime breakage.
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 stronger real behavior proof before merge: The PR body includes terminal output for truncateUtf16Safe itself, but it does not show the changed audit formatter or audit CLI output after the fix; add redacted terminal output/logs for that path before merge. 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 focused user-visible CLI text correctness fix with limited blast radius and no evidence of broader runtime breakage.
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 stronger real behavior proof before merge: The PR body includes terminal output for truncateUtf16Safe itself, but it does not show the changed audit formatter or audit CLI output after the fix; add redacted terminal output/logs for that path before merge. 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 +3, Tests +13. Total +16 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	4	1	+3
Tests	1	13	0	+13
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	17	1	+16

What I checked:

Current main still has the audit truncation bug: short() sanitizes audit fields and then truncates with sanitized.slice(0, maxChars - 1), so an emoji at the boundary can leave a dangling surrogate in text output. (src/commands/audit.ts:66, 2ba622ca3019)
PR changes the implicated formatter: The PR imports truncateUtf16Safe and replaces the .slice() truncation in short() with the UTF-16-safe helper. (src/commands/audit.ts:64, 97cdf88e8297)
Helper contract is appropriate for the fix: truncateUtf16Safe delegates to sliceUtf16Safe and avoids returning dangling surrogate halves when truncating a UTF-16 string. (packages/normalization-core/src/utf16-slice.ts:43, 2ba622ca3019)
Import path is exported: @openclaw/normalization-core/utf16-slice is a declared package export, so the new import follows the package contract used elsewhere in the repo. (packages/normalization-core/package.json:47, 2ba622ca3019)
Added test does not cover the changed audit path: The new regression imports and asserts truncateUtf16Safe directly, so it would pass even if formatAuditRows() or short() kept using .slice(). (src/commands/audit.test.ts:47, 97cdf88e8297)
Audit formatter provenance: git blame ties the current short() helper and .slice() truncation to commit 6b76a306d4725a826093dd259bf1eb6bc42c3de4, which introduced the audit command/test surface while landing fix(zalo): accept opaque string chat IDs. (src/commands/audit.ts:58, 6b76a306d472)

Likely related people:

steipete: git blame shows the audit command, its short() helper, and the truncateUtf16Safe helper were introduced in commit 6b76a306d4725a826093dd259bf1eb6bc42c3de4, merged by the same GitHub user on https://github.com/openclaw/openclaw/pull/101548. (role: introduced behavior / recent area contributor; confidence: high; commits: 6b76a306d472; files: src/commands/audit.ts, src/commands/audit.test.ts, packages/normalization-core/src/utf16-slice.ts)
maweibin: The same contributor has recent merged main history applying UTF-16-safe truncation to neighboring surfaces, including session-cost usage and control UI assets, so they are relevant to this truncation sweep beyond only authoring this PR. (role: recent adjacent contributor; confidence: medium; commits: afdb9fd26408, f6ffe89df654; files: src/infra/session-cost-usage.ts, src/infra/session-cost-usage.test.ts, src/infra/control-ui-assets.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-07T12:46:05Z

Thanks @maweibin. The useful behavior from this PR was consolidated with the sibling UTF-16 boundary fixes into #101654 and landed on main in a9582a1bb62aa70bb0b0ceb72536d0d76f08eab8.

The landed fix uses the existing truncateUtf16Safe helper, preserves this caller's prior code-unit limit and output shape, and adds caller-level surrogate-boundary coverage. Closing this PR as superseded by the canonical landed change.

fix(audit): keep emoji / surrogate pairs intact during audit text tru…

97cdf88

…ncation Use truncateUtf16Safe() instead of .slice() to count full code points, preventing lone surrogates in audit CLI output. Co-Authored-By: Claude Opus 4.8 <[email protected]>

openclaw-barnacle Bot added commands Command implementations size: XS labels Jul 7, 2026

maweibin closed this Jul 7, 2026

maweibin reopened this Jul 7, 2026

steipete mentioned this pull request Jul 7, 2026

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

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(audit): keep emoji / surrogate pairs intact during audit text truncation#101628

fix(audit): keep emoji / surrogate pairs intact during audit text truncation#101628
maweibin wants to merge 1 commit into
openclaw:mainfrom
maweibin:fix/audit-emoji-truncation

maweibin commented Jul 7, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

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!

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