fix(onboard-skills): keep emoji / surrogate pairs intact during skill description truncation by maweibin · Pull Request #101619 · openclaw/openclaw

maweibin · 2026-07-07T11:43:50Z

What Problem This Solves

summarizeInstallFailure / formatSkillHint in src/commands/onboard-skills.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 reaches user-facing CLI, status, or log output, so any content containing emoji near the 139-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.ts) and the canonical 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. Existing column width / character caps are preserved — the broken surrogate is dropped cleanly rather than rendered as a replacement character.

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 = 'y'.repeat(139) + '\u{1F680}xx';
const before = content.slice(0, 139 + 1) + '…';
const after = truncateUtf16Safe(content, 139 + 1) + '…';
const t = (s) => /[\uD800-\uDBFF](?![\uDC00-\uDFFF])/.test(s);
console.log('BEFORE (.slice 139):', JSON.stringify(before.slice(-12)), 'lone:', t(before));
console.log('AFTER  (truncateUtf16Safe 139):', JSON.stringify(after.slice(-12)), 'lone:', t(after));
"

Evidence after fix:

BEFORE (.slice 139):           "yyyyyyyyyy\ud83d…"  lone: true
AFTER  (truncateUtf16Safe 139): "yyyyyyyyyyy…"       lone: false

Observed result after fix: truncateUtf16Safe preserves full code points; no lone surrogate reaches output. The broken surrogate pair is dropped cleanly rather than producing ``.

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

Regression Test Plan

node scripts/run-vitest.mjs src/commands/onboard-skills.test.ts --run

Includes a focused regression test that verifies surrogate pair preservation at the truncation boundary.

AI-assisted.

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

clawsweeper · 2026-07-07T12:02:36Z

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

Summary
Replaces two onboarding skill text truncations with truncateUtf16Safe and adds a surrogate-pair truncation assertion in the onboarding skills test file.

PR surface: Source +1, Tests +9. Total +10 across 2 files.

Reproducibility: yes. at source level: current main uses .slice() at both onboarding truncation sites, and a focused Node boundary check reproduced a lone surrogate at the reported emoji boundary. I did not run the full interactive onboarding flow.

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 redacted terminal output, logs, or a screenshot/video showing the onboarding skills output path after the fix, with private paths, endpoints, keys, and other sensitive details removed.
Replace or supplement the helper-only assertion with a setupSkills-level regression case for a long emoji-bearing install message or skill description.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes terminal output for the helper, but not redacted terminal output, logs, screenshot, video, or live output from the changed onboarding path; after adding that proof to the PR body, a fresh ClawSweeper review should run automatically or can be requested with @clawsweeper re-review. 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 proof currently exercises the normalization helper directly, not the onboarding output path that this PR changes.
[P1] The added regression assertion bypasses setupSkills, so it would not catch a future reversion of the production onboarding truncation helpers.

Maintainer options:

Decide the mitigation before merge
Land the existing-helper production fix after the PR adds redacted real onboarding output proof and a regression test that drives setupSkills through a long emoji-bearing install message or skill description.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] The remaining blocker is contributor-supplied changed-surface proof plus a setupSkills-level regression assertion; ClawSweeper should not supply the contributor’s real-environment proof.

Security
Cleared: The diff uses an existing internal normalization helper in command output formatting and adds a test; no security or supply-chain surface change was found.

Review findings

[P3] Exercise the onboarding truncation path — src/commands/onboard-skills.test.ts:517

Review details

Best possible solution:

Land the existing-helper production fix after the PR adds redacted real onboarding output proof and a regression test that drives setupSkills through a long emoji-bearing install message or skill description.

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

Yes at source level: current main uses .slice() at both onboarding truncation sites, and a focused Node boundary check reproduced a lone surrogate at the reported emoji boundary. I did not run the full interactive onboarding flow.

Is this the best way to solve the issue?

Yes for the production change: using the existing truncateUtf16Safe helper is the narrow maintainable fix. The supporting proof and test are not yet the best shape because they bypass the changed onboarding path.

Full review comments:

[P3] Exercise the onboarding truncation path — src/commands/onboard-skills.test.ts:517
This assertion imports truncateUtf16Safe directly and rebuilds the expected expression, so it would still pass if summarizeInstallFailure or formatSkillHint kept using .slice(). Please drive setupSkills through a long emoji-bearing install message or skill description so the test protects the changed code path.
Confidence: 0.89

Overall correctness: patch is correct
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a focused user-visible CLI/onboarding text bug with limited blast radius and a narrow existing-helper fix.
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 the helper, but not redacted terminal output, logs, screenshot, video, or live output from the changed onboarding path; after adding that proof to the PR body, a fresh ClawSweeper review should run automatically or can be requested with @clawsweeper re-review. 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 +1, Tests +9. Total +10 across 2 files.

View PR surface stats

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

What I checked:

Current main still has unsafe onboarding truncation: summarizeInstallFailure and formatSkillHint both truncate user-visible onboarding text with .slice(0, maxLen - 1), and setupSkills renders those strings through notes/spinner output. (src/commands/onboard-skills.ts:50, 2ba622ca3019)
Helper contract fits the proposed production change: truncateUtf16Safe clamps to a UTF-16 limit and delegates to sliceUtf16Safe, which avoids returning dangling surrogate halves; the package also exports the ./utf16-slice subpath used by the PR. (packages/normalization-core/src/utf16-slice.ts:44, 2ba622ca3019)
Current-main source reproduction: A focused Node boundary check showed current .slice() behavior can produce a trailing high surrogate before the ellipsis for the reported 139-character boundary case. (2ba622ca3019)
PR test still bypasses the changed code path: The added test imports truncateUtf16Safe directly and reconstructs the expected expression, so it would pass even if the production onboarding helpers still used .slice(). (src/commands/onboard-skills.test.ts:517, 76fa2dc67cba)
Previous ClawSweeper finding remains unresolved: The latest completed ClawSweeper review already asked for changed-surface proof and a setupSkills-level regression assertion; the live PR head is still the same reviewed SHA. (76fa2dc67cba)
History provenance for touched behavior: git blame ties the current onboarding helper code and existing .slice() truncations to commit 6b76a306d4725a826093dd259bf1eb6bc42c3de4. (src/commands/onboard-skills.ts:44, 6b76a306d472)

Likely related people:

steipete: git blame shows the current onboarding skills module, tests, and normalization helper entered current main through commit 6b76a306d4725a826093dd259bf1eb6bc42c3de4, and live PR metadata ties that merge to steipete. (role: introduced behavior / recent area contributor; confidence: high; commits: 6b76a306d472; files: src/commands/onboard-skills.ts, src/commands/onboard-skills.test.ts, packages/normalization-core/src/utf16-slice.ts)
maweibin: Recent merged current-main fixes by maweibin addressed the same surrogate-safe truncation pattern in other command/CLI output paths, so they are relevant to this recurring behavior beyond authoring this PR. (role: adjacent contributor; confidence: medium; commits: afdb9fd26408, 4d5cd05a64bf; files: src/infra/session-cost-usage.ts, src/commands/tasks.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-07T12:06:53.305Z sha 76fa2dc :: needs real behavior proof before merge. :: [P3] Exercise the onboarding truncation path

steipete · 2026-07-07T12:46:09Z

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(onboard-skills): keep emoji / surrogate pairs intact during skill…

76fa2dc

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

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

steipete mentioned this pull request Jul 7, 2026

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

Merged

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix(onboard-helpers): keep emoji / surrogate pairs intact during workspace path truncation #101639

Closed

steipete closed this Jul 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(onboard-skills): keep emoji / surrogate pairs intact during skill description truncation#101619

fix(onboard-skills): keep emoji / surrogate pairs intact during skill description truncation#101619
maweibin wants to merge 1 commit into
openclaw:mainfrom
maweibin:fix/onboard-skills-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