fix(qqbot): use UTF-16-safe truncation for messaging reply and approval previews by wangmiao0668000666 · Pull Request #101421 · openclaw/openclaw

wangmiao0668000666 · 2026-07-07T06:35:15Z

Summary

AI-assisted with Codex.
QQBot reply dispatcher, streaming-c2c, streaming-media-send, sender, and approval-command-preview sites now use UTF-16-safe truncation instead of raw .slice(0, N) so emoji and CJK surrogate pairs in the user-visible approval command preview, TTS preview, payload-text preview, textBefore preview, media-path preview, and ApiError message are not split mid-pair.
Replaces raw UTF-16 code-unit truncation with the existing truncateUtf16Safe(text, N) helper from openclaw/plugin-sdk/text-utility-runtime.
Keeps the existing cap values; out of scope: outbound.ts / outbound-deliver.ts / outbound-media-send.ts (covered by PR fix(qqbot): use UTF-16-safe truncation for messaging outbound previews #101410) and gateway / api surfaces (PR 2 + PR 3).

Linked context

No linked issue.
Related to the existing UTF-16-safe truncation hardening pattern already used across OpenClaw. Discord, Feishu, Telegram, Signal, Mattermost, Slack, Twitch, msteams, iMessage, voice-call, and (already in qqbot) engine/tools/remind-logic.ts (llagy009 #96575) all use the same truncateUtf16Safe helper. QQBot reply dispatcher + streaming + approval command preview is the new addition. Continues PR fix(qqbot): use UTF-16-safe truncation for messaging outbound previews #101410 (messaging outbound-deliver / outbound-media-send).

Real behavior proof (required for external PRs)

Behavior or issue addressed: QQBot reply-dispatcher debugLog TTS: preview, streaming-c2c payload-text / textBefore / media-path previews, streaming-media-send executeSendQueue: sending debug, sender ApiError end-to-end propagation message, and the user-visible approval command preview (buildExecApprovalText) now truncate on a UTF-16 code-unit boundary instead of splitting emoji/CJK surrogate pairs.
Real environment tested: local OpenClaw source checkout on Node v22.22.0.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs extensions/qqbot/src/engine/messaging/reply-dispatcher.test.ts extensions/qqbot/src/engine/approval/index.test.ts --run

Evidence after fix: focused test passed, 16 tests (12 pre-existing + 4 new inline UTF-16 tests across the two touched files; the new tests cover TTS preview, streaming-c2c payload-text preview, ASCII negative control, and the user-visible buildExecApprovalText code-fence).
Observed result after fix: the touched QQBot paths no longer emit a lone surrogate when truncating text at an emoji or CJK boundary. Each new inline test asserts both length <= cap AND that the output contains no lone high or low surrogate code unit. The buildExecApprovalText test extracts the user-visible code-fence from the rendered approval text and verifies both the cap and the no-lone-surrogate invariant.
What was not tested: full end-to-end QQBot provider/channel delivery was not run; this is a focused text-boundary fix on internal debugLog previews, sender error returns, and the user-visible approval command preview.
Proof limitations or environment constraints: proof is scoped to the affected local runtime/test path and does not require external services.

Before evidence (optional but encouraged):

[emoji straddling cap 300 in user-visible approval command preview]
  BEFORE .slice(0, 300):        length=300, codeUnits=[0065 0063 0068 006f 0020 d83d de00 d83d de00 ... d83d]   ← last d83d at index 299, but input length 302, so the trailing low surrogate de00 is dropped → pair split
  AFTER  truncateUtf16Safe(cmd, 300): length=300, codeUnits=[0065 0063 0068 006f 0020 d83d de00 d83d de00 ... de00]   ← pair preserved as a unit, truncated by 2 if the next char would split a pair

[plain ASCII command, negative control]
  BEFORE .slice(0, 300):        length=300, codeUnits=[0065 0063 0068 006f ... 006f]    ← pass-through
  AFTER  truncateUtf16Safe(cmd, 300): length=300, codeUnits=[0065 0063 0068 006f ... 006f]

[CJK content, no surrogate pair issue]
  BEFORE .slice(0, 60):         length=60, codeUnits=[6d4b 8bd5 ...]        ← no change (CJK lives in BMP, no surrogate)
  AFTER  truncateUtf16Safe(s, 60): length=60, codeUnits=[6d4b 8bd5 ...]

The lone-surrogate detector is the production-style helper from packages/normalization-core/src/utf16-slice.test.ts (hasLoneSurrogate walk). When the source site is truncateUtf16Safe it returns 0 lone surrogates; if the source site is reverted to .slice(0, N) and the input contains a surrogate pair straddling index N-1 or N, the test fails.

Tests and validation

node scripts/run-vitest.mjs extensions/qqbot/src/engine/messaging/reply-dispatcher.test.ts extensions/qqbot/src/engine/approval/index.test.ts --run
Added focused regression coverage for a boundary emoji / surrogate-pair truncation case in the production-helper surface (4 new it() blocks across the two existing test files: reply-dispatcher.test.ts and approval/index.test.ts).
node scripts/run-oxlint-shards.mjs --threads=4 clean.
pnpm tsgo:extensions clean.
No known failures from this change.

Risk checklist

Did user-visible behavior change? Yes — malformed truncated text (lone surrogate in the user-visible approval command preview, TTS preview, payload-text preview, and ApiError message) is now avoided while preserving existing cap values.
Did config, environment, or migration behavior change? No.
Did security, auth, secrets, network, or tool execution behavior change? No.
What is the highest-risk area? approval/index.ts:buildExecApprovalText is directly user-visible in QQ approval cards; reply-dispatcher.ts:sanitizeForLog and the streaming / sender debugLog paths are operator-visible in logs.
How is that risk mitigated? The change is a 1-to-1 substitution of .slice(0, N) for truncateUtf16Safe(text, N); the helper preserves identical behavior for ASCII, CJK (BMP), and all inputs without surrogate pairs, and only ever trims at most 1 code unit when a surrogate pair straddles the cap. Inline tests assert both the cap and the absence of lone surrogates. The SHA-256 hex slice at reply-dispatcher.ts:301 and the positional slices at streaming-media-send.ts:208 / :429 are intentionally skipped (hex is ASCII; positional slices operate on dynamic indices).

Current review state

Next action: maintainer review and CI.
Nothing is waiting on the author.

…al previews

clawsweeper · 2026-07-07T06:40:03Z

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 3:19 AM ET / 07:19 UTC.

Summary
The PR replaces raw UTF-16 .slice(0, N) truncation in QQBot approval, reply, streaming, media-send, and token-retry preview paths with truncateUtf16Safe and keeps one focused approval regression test.

PR surface: Source +2, Tests +15. Total +17 across 6 files.

Reproducibility: yes. at the source-expression level: current main still uses raw .slice(0, N) at the touched QQBot preview sites, and a read-only Node probe reproduced a dangling surrogate at the approval cap. I did not reproduce the behavior through a live QQBot service.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
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 after-fix QQBot runtime proof from an approval, reply, streaming, media-send, or token-retry preview path.
Update the PR body so the proof and test counts match the current head; ClawSweeper should re-review automatically, or a maintainer can comment @clawsweeper re-review.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body provides local focused tests and string-level before/after examples, but no after-fix redacted QQBot runtime log, approval output, terminal transcript, or live run; add real proof with private data redacted, then update the PR body for re-review.

Risk before merge

[P1] The PR still lacks after-fix real QQBot approval/log/runtime output, so local tests do not prove the changed transport-visible paths in a real setup.
[P1] The PR body still describes broader test proof than the final diff leaves in place after the second commit, so the proof section should be refreshed when real evidence is added.

Maintainer options:

Decide the mitigation before merge
Land the focused helper substitution after redacted real QQBot approval/log/terminal proof is added or a maintainer explicitly overrides the proof gate; keep sibling QQBot UTF-16 surfaces in their separate PRs.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] Human proof handling remains: the contributor should add real QQBot behavior proof or a maintainer should explicitly override the proof gate; there is no narrow code repair for ClawSweeper to apply.

Maintainer decision needed

Question: Should this PR require redacted real QQBot runtime proof before merge, or should a maintainer explicitly override the proof gate for this low-level helper substitution?
Rationale: The patch shape is narrow and source-reproducible, but the external contributor has only provided local test/string evidence and ClawSweeper's external-PR gate requires real behavior proof unless a maintainer accepts the gap.
Likely owner: steipete — steipete is assigned to the PR and authored the latest test-focusing commit on the branch.
Options:
- Require real QQBot proof (recommended): Ask for redacted terminal logs, runtime output, or an approval-card/live-run transcript from one changed QQBot path before merge.
- Override proof gate: A maintainer may explicitly accept the proof gap because the code is a direct replacement with an existing helper and the failure is source-reproducible.

Security
Cleared: Cleared: the diff imports an existing SDK text helper and changes tests/source call sites only, with no dependency, workflow, secret, auth, network, permission, or persistence changes.

Review details

Best possible solution:

Land the focused helper substitution after redacted real QQBot approval/log/terminal proof is added or a maintainer explicitly overrides the proof gate; keep sibling QQBot UTF-16 surfaces in their separate PRs.

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

Yes at the source-expression level: current main still uses raw .slice(0, N) at the touched QQBot preview sites, and a read-only Node probe reproduced a dangling surrogate at the approval cap. I did not reproduce the behavior through a live QQBot service.

Is this the best way to solve the issue?

Yes for the code shape, but not yet for merge readiness. Reusing the existing exported truncateUtf16Safe helper at plugin-owned preview sites is the narrow maintainable fix; the missing part is real behavior proof from a changed QQBot path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: This is a normal-priority QQBot correctness fix for malformed truncated approval/log/error previews with limited blast radius.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish 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 provides local focused tests and string-level before/after examples, but no after-fix redacted QQBot runtime log, approval output, terminal transcript, or live run; add real proof with private data redacted, then update the PR body for re-review.

Evidence reviewed

PR surface:

Source +2, Tests +15. Total +17 across 6 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	5	13	11	+2
Tests	1	16	1	+15
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	6	29	12	+17

What I checked:

Repository policy read: Read the root and scoped extension AGENTS.md files; the review applied the extension boundary rule allowing bundled plugins to import public openclaw/plugin-sdk/* subpaths and the ClawSweeper rule requiring real behavior proof for user-visible PRs. (AGENTS.md:1, d09469c7a6b4)
Current main approval preview still uses raw slicing: Current main builds the QQBot exec approval code fence with cmd.slice(0, 300), so an emoji crossing that cap can leave a dangling surrogate in the approval text. (extensions/qqbot/src/engine/approval/index.ts:64, d09469c7a6b4)
Current main reply and streaming previews still use raw slicing: Current main uses raw .slice(0, N) in QQBot log/operator preview paths including sanitizeForLog, TTS preview, streaming onDeliver, media-tag previews, media-send queue logging, and token-retry warning text. (extensions/qqbot/src/engine/messaging/reply-dispatcher.ts:283, d09469c7a6b4)
Source-level repro: A read-only Node probe reproduced the string failure mode: slice(0, 300) on 299 ASCII code units plus an emoji returns length 300 ending in a lone high surrogate. (d09469c7a6b4)
Shared helper contract: truncateUtf16Safe delegates to sliceUtf16Safe, whose boundary checks avoid returning dangling surrogate halves at the slice edge. (packages/normalization-core/src/utf16-slice.ts:68, d09469c7a6b4)
PR diff applies existing SDK helper: The live PR diff imports truncateUtf16Safe from openclaw/plugin-sdk/text-utility-runtime and replaces the raw slices at the QQBot approval, reply-dispatcher, sender, streaming-c2c, and streaming-media-send preview sites. (extensions/qqbot/src/engine/approval/index.ts:62, 7424d627ac71)

Likely related people:

cxyhhhhh: GitHub path history shows the QQBot self-contained engine and later group/C2C streaming architecture commits introduced the central approval and messaging surfaces this PR touches. (role: feature introducer; confidence: medium; commits: 5e72e39c1852, 5ccf179a34a9; files: extensions/qqbot/src/engine/approval/index.ts, extensions/qqbot/src/engine/messaging/reply-dispatcher.ts, extensions/qqbot/src/engine/messaging/streaming-c2c.ts)
steipete: GitHub history shows repeated QQBot helper refactors and normalization helper work, and the live PR is assigned to steipete with a second commit from that account focusing the regression test. (role: recent area contributor and proof owner; confidence: high; commits: a607661a71d8, b22c36f1125c, 7424d627ac71; files: extensions/qqbot/src/engine/approval/index.ts, extensions/qqbot/src/engine/messaging/reply-dispatcher.ts, packages/normalization-core/src/utf16-slice.ts)
zhangguiping-xydt: Recent GitHub path history for reply-dispatcher and streaming-media-send includes scoped QQBot media-send work close to the touched messaging paths. (role: recent QQBot messaging/media contributor; confidence: medium; commits: cd6b67533d6d; files: extensions/qqbot/src/engine/messaging/reply-dispatcher.ts, extensions/qqbot/src/engine/messaging/streaming-c2c.ts, extensions/qqbot/src/engine/messaging/streaming-media-send.ts)
llagy009: Merged PR fix(qqbot): truncate reminder job name on a code-point boundary #96575 applied the same truncateUtf16Safe pattern to a QQBot reminder job-name surrogate-boundary bug. (role: related pattern contributor; confidence: medium; commits: d693ed4af3e7; files: extensions/qqbot/src/engine/tools/remind-logic.ts, extensions/qqbot/src/engine/tools/remind-logic.test.ts)
sliverp: CONTRIBUTING.md lists sliverp for Chinese channels including QQ, which is useful routing context for QQBot behavior review even though this review did not find recent commits from that account on these files. (role: listed QQ channel area contact; confidence: low; files: CONTRIBUTING.md, extensions/qqbot/src/engine/approval/index.ts, extensions/qqbot/src/engine/messaging/reply-dispatcher.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 (1 earlier review cycle)

reviewed 2026-07-07T06:44:15.844Z sha c7b6ef2 :: needs real behavior proof before merge. :: none

steipete · 2026-07-07T07:22:10Z

Merged via squash.

Prepared head SHA: 7424d627ac7161fc33dbb935d531621a31c8cc2c
Landed commit: 048fe0850edd0fca4d6dde573dea88f5fc4f5f9e

…al previews (openclaw#101421) * fix(qqbot): use UTF-16-safe truncation for messaging reply and approval previews * test(qqbot): focus UTF-16 approval regression --------- Co-authored-by: Peter Steinberger <[email protected]>

fix(qqbot): use UTF-16-safe truncation for messaging reply and approv…

c7b6ef2

…al previews

openclaw-barnacle Bot added channel: qqbot size: S labels Jul 7, 2026

wangmiao0668000666 mentioned this pull request Jul 7, 2026

fix(qqbot): use UTF-16-safe truncation for gateway log and queue messages #101426

Closed

steipete self-assigned this Jul 7, 2026

wangmiao0668000666 mentioned this pull request Jul 7, 2026

fix(qqbot): use UTF-16-safe truncation for api error and response body reads #101444

Closed

test(qqbot): focus UTF-16 approval regression

7424d62

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

steipete merged commit 048fe08 into openclaw:main Jul 7, 2026
158 of 161 checks passed

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

📡 Upstream Digest — 2026-07-07 07:31 UTC curtismercier/openclaw-mods#1228

Open

clawsweeper Bot mentioned this pull request Jul 8, 2026

[Bug]: QQ Bot exec approval command preview does not wrap on desktop QQ #101979

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(qqbot): use UTF-16-safe truncation for messaging reply and approval previews#101421

fix(qqbot): use UTF-16-safe truncation for messaging reply and approval previews#101421
steipete merged 2 commits into
openclaw:mainfrom
wangmiao0668000666:fix/qqbot-utf16-messaging-reply-and-approval

wangmiao0668000666 commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wangmiao0668000666 commented Jul 7, 2026

Summary

Linked context

Real behavior proof (required for external PRs)

Tests and validation

Risk checklist

Current review state

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading