fix(acp): keep background-task summaries UTF-16 safe at truncation boundaries by MoerAI · Pull Request #101976 · openclaw/openclaw

MoerAI · 2026-07-08T02:42:46Z

What Problem This Solves

Fixes an issue where operators watching a child ACP background run would see a corrupted replacement character (U+FFFD) at the end of the task label or progress summary when the truncation boundary landed in the middle of an emoji or other astral character. Both requester-facing strings for detached ACP tasks were affected.

Part of the UTF-16 safety sweep (same defect class as the landed acp/event-mapper, http-error-body, tasks, and control-ui-assets fixes).

Why This Change Was Made

src/acp/control-plane/manager.background-task.ts bounded both summaries with raw String.slice: summarizeBackgroundTaskText() cut at ACP_BACKGROUND_TASK_TEXT_MAX_LENGTH - 1 (159) and appendBackgroundTaskProgressSummary() at ACP_BACKGROUND_TASK_PROGRESS_MAX_LENGTH - 1 (239). A surrogate pair astride either index leaves a lone high surrogate before the … marker. Both sites now use truncateUtf16Safe() from @openclaw/normalization-core/utf16-slice, the same helper the sibling ACP event-mapper.ts already uses for its argument previews.

This module owns the bounded requester-facing task strings; the PR changes only these two truncation expressions and leaves chunk normalization, terminal-result inference, and the task-registry calls intact.

Non-goals: no change to the length limits themselves, to whitespace normalization, or to any other truncation site in the codebase.

User Impact

ACP background-task labels and progress lines now always end with well-formed text before the ellipsis — no more � in task status when the summary happens to cut through an emoji. Plain-ASCII truncation output is byte-for-byte unchanged (covered by regression).

Evidence

All proof from current head 8c96243892 (branched from upstream/main 6765eb0166), Node v24.4.0.

Real module behavior — the exported appendBackgroundTaskProgressSummary() invoked via tsx, before/after:

--- unpatched main ---
tail of progress summary: "xx\ud83e…"     ← lone high surrogate (renders as U+FFFD)
well-formed UTF-16: false
--- with this fix ---
tail of progress summary: "xxx…"
well-formed UTF-16: true
length: 239 (max 240)

Negative control — with the new regression tests present but the source fix reverted, exactly the two surrogate-boundary tests fail and the plain-ASCII behavior tests still pass (confirming no behavior change outside the boundary):

× keeps surrogate pairs intact at the progress truncation boundary
× keeps surrogate pairs intact in the bounded task label
Tests  2 failed | 3 passed (5)

After fix:

Test Files  1 passed (1)
Tests       5 passed (5)

node scripts/run-vitest.mjs src/acp/control-plane/manager.background-task.test.ts — 5 passed
pnpm exec oxlint <changed files> — clean
pnpm exec oxfmt --check --threads=1 <changed files> — "All matched files use the correct format"
pnpm tsgo — passed
Not run: pnpm check:changed delegates to the Blacksmith Testbox remote sandbox, which contributor machines cannot dispatch; the local lanes above cover lint, format, types, and the changed test file.

AI-assisted (Claude Code); I have reviewed and understand every line.

…undaries The bounded task label and progress summary shown for child ACP background runs used raw String.slice, which can split a surrogate pair at the 160/240-char boundaries and surface U+FFFD in requester-facing task status. Part of the UTF-16 safety sweep.

clawsweeper · 2026-07-08T02:47:06Z

Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 10:50 PM ET / 02:50 UTC.

Summary
The PR replaces raw ACP background-task truncation slices with UTF-16-safe truncation and adds regression coverage for surrogate-pair boundaries.

PR surface: Source +1, Tests +67. Total +68 across 2 files.

Reproducibility: yes. from source inspection: current main uses raw String.slice at the 160- and 240-code-unit ACP background-task status boundaries, so a surrogate pair at either boundary can produce ill-formed text. I did not run tests in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

none.

Risk before merge

[P1] The live PR is behind current main, so landing should use the normal exact-head refresh and required-check flow rather than assuming the current check set covers the final merge result.

Maintainer options:

Decide the mitigation before merge
Land this focused helper swap with the regression tests after exact-head merge prep; broader unrelated raw-slice cleanup should stay in separate surface-specific PRs.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No automated repair is needed; this PR is a normal review-and-land candidate once branch freshness and required checks are handled by the maintainer workflow.

Security
Cleared: The diff only changes string truncation and adds focused tests; it does not touch dependencies, workflows, credentials, permissions, package metadata, or code-execution surfaces.

Review details

Best possible solution:

Land this focused helper swap with the regression tests after exact-head merge prep; broader unrelated raw-slice cleanup should stay in separate surface-specific PRs.

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

Yes from source inspection: current main uses raw String.slice at the 160- and 240-code-unit ACP background-task status boundaries, so a surrogate pair at either boundary can produce ill-formed text. I did not run tests in this read-only review.

Is this the best way to solve the issue?

Yes; the patch uses the existing shared UTF-16-safe truncation helper already used by sibling ACP/event and runtime display surfaces, preserving the existing limits and normalization behavior. A broader sweep of unrelated raw-slice sites would be a less focused fix for this PR.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P2: This fixes a bounded ACP status-text display bug with limited blast radius and no evidence of data loss or runtime unavailability.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes before/after terminal output from invoking the exported progress-summary helper, plus negative-control and passing regression-test output; this is sufficient real behavior proof for a non-visual text runtime fix.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes before/after terminal output from invoking the exported progress-summary helper, plus negative-control and passing regression-test output; this is sufficient real behavior proof for a non-visual text runtime fix.

Label justifications:

P2: This fixes a bounded ACP status-text display bug with limited blast radius and no evidence of data loss or runtime unavailability.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes before/after terminal output from invoking the exported progress-summary helper, plus negative-control and passing regression-test output; this is sufficient real behavior proof for a non-visual text runtime fix.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes before/after terminal output from invoking the exported progress-summary helper, plus negative-control and passing regression-test output; this is sufficient real behavior proof for a non-visual text runtime fix.

Evidence reviewed

PR surface:

Source +1, Tests +67. Total +68 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	3	2	+1
Tests	1	67	0	+67
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	70	2	+68

What I checked:

Current-main defect remains: Current main still truncates both ACP background-task requester-facing strings with raw String.slice, so a surrogate pair at the cut point can leave ill-formed UTF-16. (src/acp/control-plane/manager.background-task.ts:35, 4bdf22444203)
Caller path confirmed: ACP turn output text deltas are accumulated through appendBackgroundTaskProgressSummary before task status updates are written, so the changed helper is on the reported runtime path. (src/acp/control-plane/manager.turn-runner.ts:242, 4bdf22444203)
Helper contract and export path verified: truncateUtf16Safe floors the limit and delegates to sliceUtf16Safe, which backs off from a dangling high surrogate; the package exports the ./utf16-slice subpath used by the PR. (packages/normalization-core/src/utf16-slice.ts:43, 4bdf22444203)
Sibling ACP pattern: ACP event mapping already imports the same UTF-16-safe helper for bounded argument previews, supporting this as the established local pattern rather than a new truncation mechanism. (src/acp/event-mapper.ts:17, 4bdf22444203)
PR diff reviewed: The PR changes only the two raw truncation expressions and adds a focused test file for the progress summary and bounded task label surrogate-boundary cases. (src/acp/control-plane/manager.background-task.ts:35, 8c962438927b)
Real behavior proof present: The PR body includes before/after terminal output from invoking the exported progress-summary helper, a negative-control failure with the source fix reverted, and passing focused regression-test output after the fix. (8c962438927b)

Likely related people:

steipete: GitHub history shows this author split ACP manager task helpers and has repeated ACP control-plane refactor/documentation work around the affected files. (role: feature/refactor owner; confidence: medium; commits: 7b78941ea521, 118b9cacf677; files: src/acp/control-plane/manager.background-task.ts, src/acp/control-plane/manager.turn-runner.ts)
vincentkoc: Recent history shows ACP helper refactors and coauthored adjacent background-turn cancellation work connected to the same control-plane helper surface. (role: recent area contributor; confidence: medium; commits: c0c1a929670f, f9439715e99b, c0b6b846ac29; files: src/acp/control-plane/manager.background-task.ts)
masatohoshino: Recent merged ACP background-turn cancellation work touched the same background-task helper and turn-runner path. (role: recent area contributor; confidence: medium; commits: c0b6b846ac29; files: src/acp/control-plane/manager.background-task.ts, src/acp/control-plane/manager.turn-runner.ts)
lsr911: A recent merged ACP event-mapper UTF-16 truncation fix introduced the same helper pattern in a sibling ACP event formatting surface. (role: adjacent UTF-16 ACP contributor; confidence: medium; commits: 42fc9e8a92f9; files: src/acp/event-mapper.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-09T04:02:10Z

Land-ready review complete on exact head 8c962438927b68dc40eead5793880fea2b88ee99.

Reviewed both ACP background-task summary owners, their task-registry callers, the normalization-core helper contract, and the separate ACP projector sibling surface.
Exact tests exercise task-label and progress-summary cuts through the exported manager behavior, including unchanged short text and ASCII truncation.
git diff --check and targeted formatting passed.
Testbox run 28993033940: 5/5 focused tests passed.
Fresh autoreview is clean at confidence 0.98.
Exact-head hosted CI run 28913623010 passed.

No remaining findings or proof gaps.

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

Merged via squash.

Prepared head SHA: 8c962438927b68dc40eead5793880fea2b88ee99
Landed commit: 6e4670f9c14790264516f70acb5c20dab71eadd5

…undaries (openclaw#101976) The bounded task label and progress summary shown for child ACP background runs used raw String.slice, which can split a surrogate pair at the 160/240-char boundaries and surface U+FFFD in requester-facing task status. Part of the UTF-16 safety sweep.

openclaw-barnacle Bot added the size: S label Jul 8, 2026

steipete merged commit 6e4670f into openclaw:main Jul 9, 2026
138 of 144 checks passed

clawsweeper Bot mentioned this pull request Jul 9, 2026

fix: scope ACP search path approvals [AI] #102416

Merged

This was referenced Jul 9, 2026

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

Closed

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

Closed

MoerAI deleted the fix/acp-background-task-utf16 branch July 13, 2026 02:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(acp): keep background-task summaries UTF-16 safe at truncation boundaries#101976

fix(acp): keep background-task summaries UTF-16 safe at truncation boundaries#101976
steipete merged 1 commit into
openclaw:mainfrom
MoerAI:fix/acp-background-task-utf16

MoerAI commented Jul 8, 2026

Uh oh!

clawsweeper Bot commented Jul 8, 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

MoerAI commented Jul 8, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 8, 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 8, 2026 •

edited

Loading