fix(workshop): keep approval target names UTF-16 safe by mushuiyu886 · Pull Request #102963 · openclaw/openclaw

mushuiyu886 · 2026-07-09T15:28:25Z

What Problem This Solves

Skill Workshop lifecycle approvals bound the Target skill: line to the 512-UTF-16-code-unit approval-description limit with raw JavaScript slice. If an astral character crossed that exact cut, the prompt contained a lone surrogate and could render � instead of a valid target name.

Why This Change Was Made

buildLifecycleApprovalDescription is the source formatter for apply, reject, and quarantine approval payloads. This change replaces only its raw target-name slice with the existing shared truncateUtf16Safe helper from @openclaw/normalization-core/utf16-slice.

That is the best owner boundary: proposal lookup, lifecycle policy, persistence, gateway limits, and renderers remain unchanged, while every affected approval payload becomes valid before it leaves the formatter. Downstream UI or gateway sanitization would be later, duplicated, and incomplete.

The regression exercises the real proposeCreateSkill → persisted proposal → resolveSkillWorkshopToolApproval path. It extends the existing metadata-bound test instead of adding a near-duplicate scenario, preserves every required approval fact, checks the shared 512-unit constant, and rejects lone high or low surrogates.

The docs correction changes the generic plugin approval description limit from the stale 256 value to the runtime and gateway-protocol value of 512.

User Impact

Operators see a clean ellipsis-truncated target name for long Unicode skill names. Short names, approval policy, allowed decisions, timeout behavior, proposal lookup/state, and apply/reject/quarantine behavior do not change.

Evidence

Frozen reviewed head: 3e7224efcbcfb8f23f16e4ba44ee10c4f78b8c49.

Untrusted contributor code was exercised only in sanitized AWS Crabbox leases with public networking, no Tailscale, no instance role, isolated HOME, pinned Node/pnpm, --fresh-pr, and --no-hydrate.

Exact-final-head non-test Node behavior proof on AWS lease cbx_94657e95da1f: created persisted boundary and short-control proposals, resolved the exported approval payload, and checked the behavior contract without Vitest. Boundary output was at most 512 UTF-16 code units, kept the complete prefix plus one ellipsis, contained every approval fact, and had no unpaired surrogate; the short control remained unchanged.
Focused GREEN: pnpm test src/skills/workshop/policy.test.ts — 1 file, 4/4 tests passed.
Old-slice RED: in a fresh checkout, restoring only the old raw slice made exactly the boundary regression fail; the received target line contained � before the ellipsis.
Full Workshop GREEN: pnpm test src/skills/workshop — 2 files, 40/40 tests passed.
Exact-final-head CI: success; 44 jobs succeeded and 11 skipped. The complete PR rollup is 65 success, 38 skipped, 1 neutral, 2 cancelled stale automation runs, and 0 pending/failing.
OpenGrep attempt 2: success after retrying the first attempt's GitHub-release lookup/SARIF infrastructure failure.
Source-blind behavior validation: PASS — 11/11 contract clauses, no accepted findings or evidence gaps.
Fresh full-branch autoreview: clean, no accepted/actionable findings, correctness confidence 0.88. Independent exact-head review also found no blocker.
node scripts/check-docs-mdx.mjs docs/plugins/plugin-permission-requests.md: passed.
git diff --check: clean.

No external service participates in this formatter path. Browser UI automation was not used; the live proof observes the exported approval payload consumed by the approval runtime/UI.

Root Cause and Provenance

The raw bounded slice was introduced in #100498 (f53103de72da09185b27a58618ece136a5667c18). JavaScript string indices are UTF-16 code units, so the cut could land between a valid high/low surrogate pair. This PR follows the same shared-helper fix class as #102816 but covers the distinct Skill Workshop approval formatter. A live related-PR search found no same-point open fix.

Risk / Compatibility

Risk is low and display-only. At a pair-splitting boundary, the visible prefix can be one code unit shorter so the astral character is omitted whole before the existing ellipsis. There are no config, schema, storage, protocol, dependency, provider, security-boundary, or operator-workflow changes.

Landed through the repository native wrapper as b451d8b5e59539c21009947c23e203df7d438943 after exact-head Testbox preparation and merge verification.

clawsweeper · 2026-07-09T15:43:43Z

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 1:20 PM ET / 17:20 UTC.

Summary
This PR changes Skill Workshop lifecycle approval target-name truncation to use truncateUtf16Safe, tightens the approval regression test, and updates the plugin approval docs limit to 512 characters.

PR surface: Source +1, Tests +23, Docs 0. Total +24 across 3 files.

Reproducibility: yes. Current main still raw-slices the computed target-name prefix, and the PR includes an old-slice RED control showing the boundary failure as a replacement character.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: vector/embedding metadata: src/skills/workshop/policy.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Risk before merge

[P1] GitHub reports this PR as behind the base, and the discussion says native landing is waiting on fix(pr): stop rebased syncs from losing ancestry #98765; that is a landing-process gate, not a code defect in this patch.

Maintainer options:

Decide the mitigation before merge
Land the focused formatter, regression-test, and docs patch after ordinary current-head merge and repository ancestry gates pass, keeping truncation centralized in Skill Workshop approval formatting.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No ClawSweeper repair job is indicated because the PR already contains the focused fix, tests, docs, and sufficient proof; the remaining action is normal maintainer landing once current-head and ancestry gates are satisfied.

Security
Cleared: The diff only changes display-string truncation, focused tests, and one docs value; it does not touch workflows, dependencies, secrets, package resolution, or code-execution surfaces.

Review details

Best possible solution:

Land the focused formatter, regression-test, and docs patch after ordinary current-head merge and repository ancestry gates pass, keeping truncation centralized in Skill Workshop approval formatting.

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

Yes. Current main still raw-slices the computed target-name prefix, and the PR includes an old-slice RED control showing the boundary failure as a replacement character.

Is this the best way to solve the issue?

Yes. The patch fixes the canonical approval-description formatter with the existing shared UTF-16 helper; downstream UI or Gateway sanitization would duplicate the policy and miss this source boundary.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a focused malformed-Unicode display bug in Skill Workshop approval text with limited blast radius and clear validation.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body and follow-up comment provide exact-head non-test Crabbox live-output proof for the exported approval payload, plus an old-slice RED control and focused/full green validation.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body and follow-up comment provide exact-head non-test Crabbox live-output proof for the exported approval payload, plus an old-slice RED control and focused/full green validation.

Evidence reviewed

PR surface:

Source +1, Tests +23, Docs 0. Total +24 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	2	1	+1
Tests	1	28	5	+23
Docs	1	1	1	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	31	7	+24

What I checked:

Repository policy read: Root AGENTS.md and docs/AGENTS.md were read fully; the applicable guidance required reviewing the whole touched path, dependency helper, docs contract, tests, and current-main behavior. (AGENTS.md:1, 921b6c1e2d79)
Current-main bug remains: Current main still builds the bounded Skill Workshop approval target line with requestedSkillName.slice(...), so a UTF-16 surrogate pair can be split at the truncation boundary. (src/skills/workshop/policy.ts:95, 921b6c1e2d79)
PR fixes the canonical formatter: The PR head imports truncateUtf16Safe and replaces only the target-name raw slice inside buildLifecycleApprovalDescription; proposal lookup, policy decisions, and approval routing are not changed. (src/skills/workshop/policy.ts:96, 3e7224efcbcf)
Helper contract inspected: truncateUtf16Safe clamps the limit and delegates to sliceUtf16Safe, whose end-boundary check backs off when the cut would leave a dangling high surrogate. (packages/normalization-core/src/utf16-slice.ts:44, 921b6c1e2d79)
Runtime approval path checked: resolveSkillWorkshopToolApproval feeds the generated description into the before-tool-call approval request, and the Gateway plugin-approval schema accepts descriptions up to 512 UTF-16 code units. (src/agents/agent-tools.before-tool-call.ts:980, 921b6c1e2d79)
Regression coverage checked: The PR test exercises proposeCreateSkill through resolveSkillWorkshopToolApproval, asserts the 512-unit bound, preserves required metadata, and rejects lone high or low surrogate halves. (src/skills/workshop/policy.test.ts:73, 3e7224efcbcf)

Likely related people:

steipete: The enriched Skill Workshop lifecycle approval formatter with the raw slice landed in the merged workshop approval work, current-main blame points the formatter at Peter Steinberger, and steipete added later test/docs commits plus the land-ready proof comment on this PR. (role: introduced behavior and recent reviewer; confidence: high; commits: f53103de72da, 2ff33f97ebc5, 2fec48e3e884; files: src/skills/workshop/policy.ts, src/skills/workshop/policy.test.ts, docs/plugins/plugin-permission-requests.md)
ToToKr: The closest same-class sibling fix on main is the gateway command-list UTF-16-safe truncation change merged in commit fe18ced. (role: adjacent UTF-16 truncation fix contributor; confidence: medium; commits: fe18ceda762c; files: src/gateway/server-methods/commands-list-result.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 (4 earlier review cycles)

reviewed 2026-07-09T15:47:45.635Z sha bb4a4c7 :: needs real behavior proof before merge. :: none
reviewed 2026-07-09T16:11:21.312Z sha 7268be0 :: needs real behavior proof before merge. :: none
reviewed 2026-07-09T16:38:40.822Z sha 3e7224e :: needs real behavior proof before merge. :: none
reviewed 2026-07-09T17:07:59.416Z sha 3e7224e :: needs maintainer review before merge. :: none

steipete · 2026-07-09T16:58:23Z

Land-ready evidence for frozen head 3e7224efcbcfb8f23f16e4ba44ee10c4f78b8c49:

Best-fix review: the invalid UTF-16 originates in the canonical Skill Workshop lifecycle approval formatter. The patch replaces only that raw slice with the existing shared surrogate-safe helper; no downstream fallback or parallel path was added.
Exact-final-head non-test Node behavior proof on sanitized AWS Crabbox cbx_94657e95da1f: persisted boundary/calibration/control proposals, resolved the exported approval payload, proved the old cutoff would leave a lone surrogate, then proved the fixed payload is within 512 UTF-16 units, contains exact metadata lines, has one complete-prefix ellipsis, and has no unpaired surrogate. The short control remained exact and untruncated.
Old-slice RED: restoring only the old raw slice made exactly the boundary regression fail and rendered � before the ellipsis.
Focused GREEN: 4/4. Full Workshop GREEN: 40/40.
Exact-head CI: 44 success, 11 skipped. Full rollup: 65 success, 38 skipped, 1 neutral, 2 stale automation cancellations, 0 pending/failing. OpenGrep retry succeeded after the first attempt's infrastructure-only failure.
Source-blind behavior validation: PASS — 11/11 contract clauses, no accepted findings or evidence gaps. Fresh full-branch autoreview: clean, no actionable findings, confidence 0.88. Independent exact-head review: clean.
Docs MDX check and git diff --check: passed.

The code/proof is ready. Native landing remains intentionally held only on the repository PR-wrapper ancestry gate in #98765.

@clawsweeper re-review

clawsweeper · 2026-07-09T16:58:26Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

steipete · 2026-07-09T17:26:14Z

Merged via squash.

Prepared head SHA: 3e7224efcbcfb8f23f16e4ba44ee10c4f78b8c49
Landed commit: b451d8b5e59539c21009947c23e203df7d438943
Verified the landed blobs for src/skills/workshop/policy.ts, src/skills/workshop/policy.test.ts, and docs/plugins/plugin-permission-requests.md exactly match the prepared head.
Verified the trusted checkout and origin/main are clean and exact at the landed commit.
No linked issue or open same-point issue/PR remains to close.

* fix(workshop): keep approval target names UTF-16 safe * test(workshop): tighten UTF-16 approval regression * test(workshop): mirror approval budget exactly * docs(plugins): correct approval description limit --------- Co-authored-by: Peter Steinberger <[email protected]>

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

steipete self-assigned this Jul 9, 2026

mushuiyu886 and others added 2 commits July 9, 2026 16:54

fix(workshop): keep approval target names UTF-16 safe

2c4d3ab

test(workshop): tighten UTF-16 approval regression

2fec48e

steipete force-pushed the fix/followup-102816 branch from bb4a4c7 to 2fec48e Compare July 9, 2026 15:54

steipete added 2 commits July 9, 2026 16:59

test(workshop): mirror approval budget exactly

7268be0

docs(plugins): correct approval description limit

3e7224e

openclaw-barnacle Bot added the docs Improvements or additions to documentation label Jul 9, 2026

steipete merged commit b451d8b into openclaw:main Jul 9, 2026
135 of 143 checks passed

github-actions Bot mentioned this pull request Jul 9, 2026

📡 Upstream Digest — 2026-07-09 17:48 UTC curtismercier/openclaw-mods#1244

Open

steipete mentioned this pull request Jul 9, 2026

docs(changelog): credit recent UTF-16 and display fixes #103102

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(workshop): keep approval target names UTF-16 safe#102963

fix(workshop): keep approval target names UTF-16 safe#102963
steipete merged 4 commits into
openclaw:mainfrom
mushuiyu886:fix/followup-102816

mushuiyu886 commented Jul 9, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 9, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

mushuiyu886 commented Jul 9, 2026 • edited by steipete 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

Root Cause and Provenance

Risk / Compatibility

Uh oh!

clawsweeper Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

clawsweeper Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mushuiyu886 commented Jul 9, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

steipete commented Jul 9, 2026 •

edited

Loading