fix(oc-path): use truncateUtf16Safe for error message path truncation by lsr911 · Pull Request #102527 · openclaw/openclaw

lsr911 · 2026-07-09T07:49:49Z

What Problem This Solves

oc-path parser truncates error context strings with naive .slice(0, 80) at three fail() call sites, risking surrogate pair splitting in error messages.

Files Changed

File	Change
extensions/oc-path/src/oc-path/oc-path.ts	+1 import; .slice(0,80) → truncateUtf16Safe() ×3

🤖 Generated with Claude Code

Replace naive .slice(0, 80) with truncateUtf16Safe() at three error-reporting call sites to prevent surrogate pair splitting in oc:// path validation error messages. Co-Authored-By: Claude <[email protected]>

clawsweeper · 2026-07-09T08:26:39Z

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 4:30 AM ET / 08:30 UTC.

Summary
This PR replaces three oc-path overlong error-context .slice(0, 80) truncations with truncateUtf16Safe via the plugin SDK and adds parse/post-NFC/format limit tests.

PR surface: Source +1, Tests +37. Total +38 across 2 files.

Reproducibility: yes. at source level: current main slices an overlong path context at UTF-16 code unit 80, and a prefix-plus-emoji probe shows that cut can leave a dangling high surrogate. I did not run the full oc-path test suite in this read-only review.

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 redacted terminal or live-output proof showing an overlong oc:// path with an emoji now reports a clean truncated context without a dangling surrogate.

Proof guidance:

[P1] Needs real behavior proof before merge: No after-fix terminal output, live logs, screenshot, recording, or linked artifact is present; the contributor should add redacted CLI/runtime proof and updating the PR body should trigger a fresh ClawSweeper 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 patch has source and test evidence but no after-fix real behavior proof from a real setup, so merge should wait for redacted terminal/live output or equivalent runtime evidence.

Maintainer options:

Decide the mitigation before merge
Keep the narrow SDK-helper parser fix with its regression tests, then merge only after redacted real behavior proof shows an overlong emoji-containing oc:// path error no longer exposes a dangling surrogate.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] No automated repair is indicated; the remaining blocker is contributor or maintainer-supplied real behavior proof before normal merge review continues.

Security
Cleared: The diff only uses an existing text utility facade and adds tests; it does not touch dependencies, lockfiles, scripts, secrets, permissions, or other supply-chain surfaces.

Review details

Best possible solution:

Keep the narrow SDK-helper parser fix with its regression tests, then merge only after redacted real behavior proof shows an overlong emoji-containing oc:// path error no longer exposes a dangling surrogate.

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

Yes, at source level: current main slices an overlong path context at UTF-16 code unit 80, and a prefix-plus-emoji probe shows that cut can leave a dangling high surrogate. I did not run the full oc-path test suite in this read-only review.

Is this the best way to solve the issue?

Yes, the current PR is the best narrow fix: it reuses the existing UTF-16-safe helper through the plugin SDK facade and covers all three overlong oc-path error sites. A direct @openclaw/normalization-core import would be the wrong extension boundary, and the PR already moved away from that.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P3: The PR fixes a low-blast-radius oc-path error-message truncation edge case without changing runtime routing, config, state, or plugin APIs.
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: No after-fix terminal output, live logs, screenshot, recording, or linked artifact is present; the contributor should add redacted CLI/runtime proof and updating the PR body should trigger a fresh ClawSweeper 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: The PR fixes a low-blast-radius oc-path error-message truncation edge case without changing runtime routing, config, state, or plugin APIs.
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: No after-fix terminal output, live logs, screenshot, recording, or linked artifact is present; the contributor should add redacted CLI/runtime proof and updating the PR body should trigger a fresh ClawSweeper 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 +37. Total +38 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	4	3	+1
Tests	1	37	0	+37
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	41	3	+38

What I checked:

Current main behavior: Current origin/main still truncates the three OC_PATH_TOO_LONG error inputs with raw .slice(0, 80), which can split a surrogate pair in the error-context string. (extensions/oc-path/src/oc-path/oc-path.ts:137, 240d350c7f66)
PR implementation: The PR changes those same three call sites to truncateUtf16Safe(..., 80) and imports it from the plugin SDK facade after the maintainer follow-up commit. (extensions/oc-path/src/oc-path/oc-path.ts:13, fa8c2a4e3467)
Helper contract: truncateUtf16Safe floors and clamps the limit, then delegates to sliceUtf16Safe, whose end-boundary logic avoids returning a dangling high surrogate. (packages/normalization-core/src/utf16-slice.ts:44, 240d350c7f66)
Extension boundary: openclaw/plugin-sdk/text-utility-runtime exports truncateUtf16Safe, and the package export/SDK entrypoint list includes that subpath, so the PR uses the public bundled-plugin boundary rather than a core-internal import. (src/plugin-sdk/text-utility-runtime.ts:3, 240d350c7f66)
Regression coverage: The PR adds focused tests for overlong parse input, post-NFC overlong input, and overlong formatted paths so the changed error input remains UTF-16 safe. (extensions/oc-path/src/oc-path/tests/scenarios/security-and-limits.test.ts:88, fa8c2a4e3467)
Source repro probe: A lightweight Node probe confirmed the current slice(0, 80) shape can return a string ending in the emoji high surrogate when the emoji starts at offset 79.

Likely related people:

vincentkoc: Recent main-branch oc-path refactors touched extensions/oc-path/src/oc-path/oc-path.ts, and shallow blame attributes the current overlong-error call sites to a later carried-forward commit by Vincent Koc. (role: recent area contributor; confidence: high; commits: 23cf16a7d551, 9bb946acc52d; files: extensions/oc-path/src/oc-path/oc-path.ts, extensions/oc-path/src/oc-path/universal.ts)
giodl73-repo: Prior oc-path grammar and security-test refactors touched the parser and security-and-limits.test.ts, including the same validation area this PR extends. (role: feature/refactor contributor; confidence: medium; commits: ac9418d20668, 8f26422840e2; files: extensions/oc-path/src/oc-path/oc-path.ts, extensions/oc-path/src/oc-path/tests/scenarios/security-and-limits.test.ts)
steipete: The oc-path plugin migration touched the full parser/test surface, and the current PR head includes a follow-up commit by steipete that adjusted the import to the plugin SDK and added tests. (role: adjacent owner; confidence: medium; commits: e3b33a26cdb1, fa8c2a4e3467; files: extensions/oc-path/src/oc-path/oc-path.ts, extensions/oc-path/src/oc-path/tests/scenarios/security-and-limits.test.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-09T08:35:05Z

Land-ready maintainer verification at exact head fa8c2a4e3467113380c9f0462b2bed608c7041c4:

The raw-input, post-NFC, and formatted-output overlength branches retain their limits, error messages/codes, and 80-code-unit preview suffix.
All three previews now use openclaw/plugin-sdk/text-utility-runtime, avoiding an undeclared direct package dependency.
Focused oc-path security/limits suite: 34 passed.
Fresh autoreview: clean (0.98).
Exact-head hosted CI/Testbox gates: passed.

Tests assert structured OcPathError.input for each path, including an actual NFC-expanding code point and a surrogate crossing the preview cutoff.

steipete · 2026-07-09T08:35:22Z

Merged via squash.

Prepared head SHA: fa8c2a4e3467113380c9f0462b2bed608c7041c4
Landed commit: 145b71f57efce9948a0615bb9e6793d78a0bcf82

…openclaw#102527) * fix(oc-path): use truncateUtf16Safe for error message path truncation Replace naive .slice(0, 80) with truncateUtf16Safe() at three error-reporting call sites to prevent surrogate pair splitting in oc:// path validation error messages. Co-Authored-By: Claude <[email protected]> * test(oc-path): cover utf16 error truncation --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

fix(oc-path): use truncateUtf16Safe for error message path truncation

b203180

Replace naive .slice(0, 80) with truncateUtf16Safe() at three error-reporting call sites to prevent surrogate pair splitting in oc:// path validation error messages. Co-Authored-By: Claude <[email protected]>

openclaw-barnacle Bot added extensions: oc-path size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 9, 2026

test(oc-path): cover utf16 error truncation

fa8c2a4

steipete merged commit 145b71f into openclaw:main Jul 9, 2026
112 of 118 checks passed

This was referenced Jul 9, 2026

fix(mistral): use truncateUtf16Safe for error text truncation #102539

Merged

fix(compaction): use truncateUtf16Safe for compaction summary truncation #102538

Closed

fix(memory-core): use truncateUtf16Safe for diary context truncation #102524

Merged

wangmiao0668000666 mentioned this pull request Jul 9, 2026

fix(text): keep five src-side text truncations UTF-16 safe #102630

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(oc-path): use truncateUtf16Safe for error message path truncation#102527

fix(oc-path): use truncateUtf16Safe for error message path truncation#102527
steipete merged 2 commits into
openclaw:mainfrom
lsr911:fix/utf16-oc-path

lsr911 commented Jul 9, 2026

Uh oh!

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

lsr911 commented Jul 9, 2026

What Problem This Solves

Files Changed

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!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading