fix(acp): keep session update text truncation surrogate-safe by zw-xysk · Pull Request #102378 · openclaw/openclaw

zw-xysk · 2026-07-09T03:06:21Z

What Problem This Solves

ACP session updates and streamed output were truncated with raw UTF-16 code-unit slices. A boundary through an emoji or another supplementary Unicode character could emit a lone surrogate. The streamed-output path also needed to preserve a contiguous prefix after a split code point was dropped.

Why This Change Was Made

Use the shared truncateUtf16Safe primitive for bounded session-update text.
Use the same primitive at the total streamed-output budget boundary.
Exhaust the output budget whenever text is dropped, preventing later deltas from skipping past an omitted code point.
Consolidate regression coverage into the existing ACP projector production-path suite with exact delivered-text assertions.

User Impact

ACP status updates and streamed replies remain valid Unicode at truncation boundaries. Output still honors the configured UTF-16-unit limits and remains a contiguous prefix of the original stream.

Evidence

Blacksmith Testbox run: corepack pnpm test src/auto-reply/reply/acp-projector.test.ts
- 1 file passed
- 28 tests passed
- Run: https://github.com/openclaw/openclaw/actions/runs/28993269628
git diff --check
Fresh structured autoreview: clean, no accepted/actionable findings (confidence 0.96)

AI-assisted: Yes

clawsweeper · 2026-07-09T03:08:14Z

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 12:18 AM ET / 04:18 UTC.

Summary
The PR routes ACP session-update and streamed-output truncation through truncateUtf16Safe and updates the ACP projector regression tests for surrogate-boundary truncation.

PR surface: Source +4, Tests +26. Total +30 across 2 files.

Reproducibility: yes. from source inspection: current main slices ACP session-update and streamed-output text at raw UTF-16 code-unit boundaries, so a limit can cut through an emoji surrogate pair. I did not execute tests because this was a read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup 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 that demonstrates an ACP truncation boundary through an emoji no longer emits a lone surrogate; updating the PR body should trigger a fresh ClawSweeper review.

Proof guidance:

[P1] Needs real behavior proof before merge: The current visible evidence is test/CI-style validation, which is useful but not real behavior proof for an external PR; add redacted terminal/live output or a runtime transcript showing the surrogate-boundary truncation after the fix. 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] Current visible proof is test/CI-style evidence only; if the external-PR proof gate is enforced, the PR still needs redacted live output, terminal output, or a runtime transcript showing the surrogate-boundary behavior after the fix.

Maintainer options:

Decide the mitigation before merge
Land the focused ACP projector truncation fix after maintainer review, exact-head CI, and the required proof gate are satisfied, keeping the behavior in the existing private projector helper.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] No repair lane is needed because I found no actionable code defect; the remaining handling is maintainer review plus visible proof or an explicit maintainer proof decision.

Security
Cleared: No concrete security or supply-chain concern was found; the diff imports an existing internal helper and adds focused tests without touching dependencies, workflows, secrets, or permissions.

Review details

Best possible solution:

Land the focused ACP projector truncation fix after maintainer review, exact-head CI, and the required proof gate are satisfied, keeping the behavior in the existing private projector helper.

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

Yes from source inspection: current main slices ACP session-update and streamed-output text at raw UTF-16 code-unit boundaries, so a limit can cut through an emoji surrogate pair. I did not execute tests because this was a read-only review.

Is this the best way to solve the issue?

Yes for the code shape: reusing truncateUtf16Safe in the private ACP projector truncation points is the narrow maintainable fix, and changing the shared helper or adding a parallel ACP-specific utility is unnecessary.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The current visible evidence is test/CI-style validation, which is useful but not real behavior proof for an external PR; add redacted terminal/live output or a runtime transcript showing the surrogate-boundary truncation after the fix. 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.
remove proof: sufficient: Current real behavior proof status is mock_only, not sufficient.
remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.
remove status: 👀 ready for maintainer look: Current PR status label is status: 📣 needs proof.

Label justifications:

P2: This is a normal-priority ACP correctness fix with limited blast radius and no blocking code finding.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The current visible evidence is test/CI-style validation, which is useful but not real behavior proof for an external PR; add redacted terminal/live output or a runtime transcript showing the surrogate-boundary truncation after the fix. 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 +4, Tests +26. Total +30 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	7	3	+4
Tests	1	29	3	+26
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	36	6	+30

What I checked:

Current main still slices at UTF-16 code-unit boundaries: truncateText and the streamed-output budget branch on current main still use raw slice, so the central bug is not already implemented on main. (src/auto-reply/reply/acp-projector.ts:48, 6e4670f9c147)
PR head uses the shared safe truncation helper: The PR imports truncateUtf16Safe, uses it inside the private ACP truncation helper, and uses it at the streamed-output budget boundary. (src/auto-reply/reply/acp-projector.ts:49, 21f9a8103a35)
Helper contract is appropriate for this fix: truncateUtf16Safe floors and clamps the limit, then delegates to sliceUtf16Safe, which backs off when the requested end would split a surrogate pair. (packages/normalization-core/src/utf16-slice.ts:44, 6e4670f9c147)
Regression coverage exercises the ACP production path: The PR adds a status-update test at the effective 64-code-unit clamp and changes the streamed-output budget test to hit an emoji split boundary while asserting contiguous delivered text and one truncation notice. (src/auto-reply/reply/acp-projector.test.ts:764, 21f9a8103a35)
Config clamp explains the test boundary: resolveAcpProjectionSettings clamps maxSessionUpdateChars to a minimum of 64, matching the updated regression test's effective truncation boundary. (src/auto-reply/reply/acp-stream-settings.ts:96, 6e4670f9c147)
Import path is an existing internal export: @openclaw/normalization-core/utf16-slice is already exported by packages/normalization-core, so the PR does not add a new package surface. (packages/normalization-core/package.json:47, 6e4670f9c147)

Likely related people:

osolmaz: ACP projector and stream-setting history traces through the original thread-bound ACP feature plus later projector dedupe and stream-limit commits. (role: feature-history contributor; confidence: high; commits: a7d56e3554d0, f4538b22f7f3, 8292401719e7; files: src/auto-reply/reply/acp-projector.ts, src/auto-reply/reply/acp-stream-settings.ts, src/auto-reply/reply/acp-projector.test.ts)
steipete: Recent history includes the normalization-core extraction that provides truncateUtf16Safe, ACP test refactors, and the latest PR-head commit shaping this fix. (role: recent adjacent contributor; confidence: medium; commits: 062f88e3e3af, 9617ac9dd57f, 21f9a8103a35; files: packages/normalization-core/src/utf16-slice.ts, packages/normalization-core/package.json, src/auto-reply/reply/acp-projector.test.ts)
Pavan Kumar Gondhi: Current-main blame for the ACP projector and UTF-16 helper lines points to a recent broad commit that added these files in this checkout, though the deeper ACP feature trail is stronger for routing. (role: current-line provenance; confidence: low; commits: e25fa79c5d19; files: src/auto-reply/reply/acp-projector.ts, src/auto-reply/reply/acp-projector.test.ts, packages/normalization-core/src/utf16-slice.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 (3 earlier review cycles)

reviewed 2026-07-09T03:22:19.009Z sha 8f9f3bc :: needs changes before merge. :: [P2] Make the truncation test hit the clamped boundary
reviewed 2026-07-09T03:28:30.630Z sha 8f9f3bc :: needs changes before merge. :: [P2] Make the truncation test hit the clamped boundary
reviewed 2026-07-09T03:35:59.592Z sha e9a2f78 :: needs maintainer review before merge. :: none

acp-projector's truncateText function used String.prototype.slice(0, N) on session update text that is sent to AI providers in the prompt context. When the truncation boundary falls inside a UTF-16 surrogate pair (emoji, CJK extended), the resulting string contains a dangling surrogate that can corrupt the prompt. Replace .slice(0, maxChars-1) with truncateUtf16Safe(input, maxChars-1) so the truncation point always falls on a complete code-point boundary.

steipete · 2026-07-09T04:22:06Z

Land-ready review complete for exact head 21f9a8103a35e7ef367552c53aa1de9b4caae62e.

What changed during maintainer review:

covered the second unsafe boundary in the same ACP projector: the total streamed-output budget;
preserved contiguous-prefix semantics by exhausting the budget after any dropped text;
replaced the standalone generic surrogate test with exact delivery assertions in the existing projector suite.

Proof:

Blacksmith Testbox: corepack pnpm test src/auto-reply/reply/acp-projector.test.ts — 28/28 passed (run 28993269628);
exact-head hosted CI — passed (run 28993365600);
git diff --check — passed;
fresh structured autoreview after the final edits — clean, no accepted/actionable findings (confidence 0.96).

Known proof gaps: none for this bounded owner path.

steipete · 2026-07-09T04:22:25Z

Merged via squash.

Prepared head SHA: 21f9a8103a35e7ef367552c53aa1de9b4caae62e
Landed commit: ba5a212c8f078b93659b05ff14107030e8f05779

…w#102378) * fix(acp): keep session update text truncation surrogate-safe acp-projector's truncateText function used String.prototype.slice(0, N) on session update text that is sent to AI providers in the prompt context. When the truncation boundary falls inside a UTF-16 surrogate pair (emoji, CJK extended), the resulting string contains a dangling surrogate that can corrupt the prompt. Replace .slice(0, maxChars-1) with truncateUtf16Safe(input, maxChars-1) so the truncation point always falls on a complete code-point boundary. * fix: make ACP projection truncation UTF-16 safe --------- Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added extensions: tts-local-cli size: S labels Jul 9, 2026

zw-xysk force-pushed the fix/acp-projector-truncation branch from 20c7741 to bd60126 Compare July 9, 2026 03:10

openclaw-barnacle Bot removed the extensions: tts-local-cli label Jul 9, 2026

zw-xysk force-pushed the fix/acp-projector-truncation branch from bd60126 to 8f9f3bc Compare July 9, 2026 03:17

zw-xysk force-pushed the fix/acp-projector-truncation branch from 8f9f3bc to e9a2f78 Compare July 9, 2026 03:30

fix: make ACP projection truncation UTF-16 safe

21f9a81

openclaw-barnacle Bot added size: XS and removed size: S labels Jul 9, 2026

steipete merged commit ba5a212 into openclaw:main Jul 9, 2026
130 of 142 checks passed

lsr911 mentioned this pull request Jul 9, 2026

fix(tool-policy-audit): use truncateUtf16Safe for audit field truncation #102464

Merged

xydt-tanshanshan mentioned this pull request Jul 9, 2026

fix(talk): use truncateUtf16Safe for LLM-prompt-facing text truncation #102477

Merged

wangmiao0668000666 mentioned this pull request Jul 9, 2026

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

Closed

9 tasks

SunnyShu0925 mentioned this pull request Jul 9, 2026

fix(llm-slug): use truncateUtf16Safe for LLM-prompt-facing session content truncation #102672

Closed

wangmiao0668000666 mentioned this pull request Jul 9, 2026

fix(chat,cli): keep session snippet and CLI output digest truncation UTF-16 safe #102747

Closed

9 tasks

This was referenced Jul 17, 2026

fix(discord): keep voice participant label truncation UTF-16 safe #109531

Closed

fix(agents): stop inflating subagent spawn depth from /new rollover #111246

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(acp): keep session update text truncation surrogate-safe#102378

fix(acp): keep session update text truncation surrogate-safe#102378
steipete merged 2 commits into
openclaw:mainfrom
zw-xysk:fix/acp-projector-truncation

zw-xysk 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!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

zw-xysk 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

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

zw-xysk commented Jul 9, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading