Skip to content

fix(ui-skillworkshop): use truncateUtf16Safe for session label truncation#102737

Closed
lzyyzznl wants to merge 1 commit into
openclaw:mainfrom
lzyyzznl:fix/truncateUtf16Safe-skillworkshop
Closed

fix(ui-skillworkshop): use truncateUtf16Safe for session label truncation#102737
lzyyzznl wants to merge 1 commit into
openclaw:mainfrom
lzyyzznl:fix/truncateUtf16Safe-skillworkshop

Conversation

@lzyyzznl

@lzyyzznl lzyyzznl commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

One .slice(0, 80) truncation site in the Skill Workshop UI may cut UTF-16 surrogate pairs in half when the session label contains multi-byte characters such as emoji.

Why This Change Was Made

ui/src/pages/skill-workshop/skill-workshop-page.ts uses raw .slice(0, 80) for session label truncation. Replacing with truncateUtf16Safe ensures truncated output is always valid Unicode.

User Impact

Truncated session labels in Skill Workshop remain valid Unicode at existing size limits. No behavioral changes for ASCII-only content.

Evidence

Real behavior proof — terminal output

Test scenario: a session label that exceeds the 80-char limit with emoji near the cut boundary.

Scenario A: Cut falls in the middle of surrogate pair
Test string: 'A'x79 + 🦪 + 'X' (len=82)
.slice(0,80) produces dangling high surrogate (0xd83e) -> invalid Unicode
truncateUtf16Safe backs up one code unit -> valid Unicode

Scenario B: emoji entirely beyond cut point
Both .slice and truncateUtf16Safe produce identical valid output

Scenario C: Actual Skill Workshop label near 80-char boundary
Both produce valid output

The critical case is Scenario A: when the 80-char cut falls inside a surrogate pair, .slice(0, 80) produces a dangling high surrogate (0xd83e), which is invalid Unicode. truncateUtf16Safe correctly steps back one code unit to avoid breaking the pair.

Full terminal output and reproduction script are available in the PR worktree.

Generated with Claude Code

@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Summary
The PR replaces the Skill Workshop revision session label's raw .slice(0, 80) truncation with truncateUtf16Safe(..., 80).

PR surface: Source +1. Total +1 across 1 file.

Reproducibility: yes. Current main uses .slice(0, 80) for the Skill Workshop revision session label, and a focused Node check with the same prefix plus a boundary emoji produced a dangling high surrogate.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until 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 after-fix proof for the Skill Workshop revision flow, such as a screenshot or terminal/live output showing a long emoji-containing proposal label no longer produces a dangling surrogate.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body contains mechanical evidence only; please add a screenshot, terminal/live output, redacted logs, or another real after-fix run showing the Skill Workshop revision session label path, with private details redacted. Updating the PR body should trigger a fresh ClawSweeper review; if not, ask a maintainer to comment @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] The contributor has not posted real after-fix behavior proof for the Skill Workshop revision flow, so the PR should remain a human-only merge candidate until proof is added or explicitly overridden.

Maintainer options:

  1. Decide the mitigation before merge
    Land this shared-helper swap after real behavior proof shows the Skill Workshop revision session label path no longer produces a dangling surrogate, or after a maintainer explicitly overrides that proof gate.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The next action is to ask for contributor-supplied real behavior proof or a maintainer proof override; there is no concrete code repair for ClawSweeper to queue.

Security
Cleared: The diff only imports an existing internal normalization helper and changes one UI string truncation call; no security or supply-chain concern was found.

Review details

Best possible solution:

Land this shared-helper swap after real behavior proof shows the Skill Workshop revision session label path no longer produces a dangling surrogate, or after a maintainer explicitly overrides that proof gate.

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

Yes. Current main uses .slice(0, 80) for the Skill Workshop revision session label, and a focused Node check with the same prefix plus a boundary emoji produced a dangling high surrogate.

Is this the best way to solve the issue?

Yes for patch shape. The existing truncateUtf16Safe helper is already tested, exported, aliased into the Control UI, and used by sibling UI truncation paths; the remaining gap is proof, not a better code location.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P3: This is a narrow UI text-validity fix with limited blast radius and no evidence of a blocked user-facing setup or core workflow.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🧂 unranked krab 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: The PR body contains mechanical evidence only; please add a screenshot, terminal/live output, redacted logs, or another real after-fix run showing the Skill Workshop revision session label path, with private details redacted. Updating the PR body should trigger a fresh ClawSweeper review; if not, ask a maintainer to comment @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.

Label justifications:

  • P3: This is a narrow UI text-validity fix with limited blast radius and no evidence of a blocked user-facing setup or core workflow.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🧂 unranked krab 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: The PR body contains mechanical evidence only; please add a screenshot, terminal/live output, redacted logs, or another real after-fix run showing the Skill Workshop revision session label path, with private details redacted. Updating the PR body should trigger a fresh ClawSweeper review; if not, ask a maintainer to comment @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. Total +1 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
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 1 2 1 +1

What I checked:

Likely related people:

  • vincentkoc: Current-main blame attributes both the Skill Workshop page helper containing this label path and the UTF-16-safe helper implementation to the same recent commit. (role: recent area contributor; confidence: medium; commits: 45d15aabf9e9; files: ui/src/pages/skill-workshop/skill-workshop-page.ts, packages/normalization-core/src/utf16-slice.ts)
  • steipete: Merged adjacent Skill Workshop lifecycle work in fix(skills): make Skill Workshop lifecycle approvals decidable and non-wedging #100498, which is relevant for routing but did not touch this UI session-label truncation site. (role: adjacent feature contributor; confidence: low; commits: e3bd9da2261e; files: src/skills/workshop/policy.ts, docs/tools/skill-workshop.md)
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. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jul 9, 2026
@steipete steipete mentioned this pull request Jul 9, 2026
Merged
@lzyyzznl

lzyyzznl 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 picked this up.

Command router queued. I will update this comment with the next step.

@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thanks @lzyyzznl. #102823 landed the same UTF-16-safe Skill Workshop session-label cap in 5ef269c. No independent behavior remains on this branch, so it is fully superseded and I’m closing it.

@steipete steipete closed this Jul 9, 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 P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. 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

@lzyyzznl @steipete