Skip to content

fix(signal): truncate verbose inbound preview on code-point boundaries#97982

Closed
solodmd wants to merge 1 commit into
openclaw:mainfrom
solodmd:fix/signal-truncate-preview-utf16
Closed

fix(signal): truncate verbose inbound preview on code-point boundaries#97982
solodmd wants to merge 1 commit into
openclaw:mainfrom
solodmd:fix/signal-truncate-preview-utf16

Conversation

@solodmd

@solodmd solodmd commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Signal verbose inbound message previews truncate the message body with
body.slice(0, 200). When an emoji or other astral character straddles
position 200, the raw slice keeps only the high-surrogate half, producing
a lone \uD83D in the debug log — malformed UTF-16.

Why This Change Was Made

The plugin SDK provides truncateUtf16Safe for this exact pattern. Other
channel plugins (Discord, Feishu, Telegram, Tlon, Mattermost, Slack)
already use it for the same purpose.

User Impact

No user-facing behavior change. The verbose debug log now emits valid
UTF-16 previews regardless of where emoji fall in the truncated message.

Evidence

  • git diff --check origin/main...HEAD
  • pnpm test extensions/signal — 30 files, 375 tests passed
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

After-fix Proof

Boundary probe on this PR branch:

input length: 251 code units
unit at 199: U+D83D (high surrogate of 😀)

BEFORE .slice(0,200):
  length: 200  hasLoneSurrogate: true
  tail: [61 61 d83d]   ← broken

AFTER truncateUtf16Safe(body, 200):
  length: 199  hasLoneSurrogate: false
  tail: [61 61 61]   ← clean, emoji dropped whole

✅ All paths produce well-formed UTF-16.

Tests

extensions/signal/src/monitor/event-handler.truncation.test.ts — 5 tests
covering the straddling-emoji boundary, emoji-before-boundary, plain ASCII
truncation, position-0 edge case, and short-body pass-through.

@openclaw-barnacle openclaw-barnacle Bot added channel: signal Channel integration: signal size: S labels Jun 30, 2026
@clawsweeper

clawsweeper Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 10:43 PM ET / 02:43 UTC.

Summary
The PR changes Signal verbose inbound logging to truncate previews with truncateUtf16Safe and adds focused boundary tests for surrogate-safe truncation.

PR surface: Source 0, Tests +52. Total +52 across 2 files.

Reproducibility: yes. at source level: current main calls body.slice(0, 200) before verbose Signal logging, which can leave a dangling surrogate when an emoji straddles the code-unit boundary. I did not run tests because this review is read-only, but the PR body includes copied branch output for the exact boundary probe.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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.

Next step before merge

  • No ClawSweeper repair is needed; this PR is already a focused implementation awaiting maintainer review and required checks.

Security
Cleared: No security or supply-chain concern found; the diff only changes Signal TypeScript logging behavior and adds a local Vitest test.

Review details

Best possible solution:

Land the focused Signal logging fix after ordinary maintainer review and required-check gating; any broader sibling-channel preview cleanup should be handled separately.

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

Yes, at source level: current main calls body.slice(0, 200) before verbose Signal logging, which can leave a dangling surrogate when an emoji straddles the code-unit boundary. I did not run tests because this review is read-only, but the PR body includes copied branch output for the exact boundary probe.

Is this the best way to solve the issue?

Yes. Reusing the existing Plugin SDK truncateUtf16Safe helper at the Signal preview call site is the narrowest maintainable fix for the stated bug, and sibling channel precedent supports that shape.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P3: The PR fixes a low-risk Signal verbose debug-log encoding bug without changing message delivery, config, auth, security, or compatibility behavior.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied branch output showing raw slice creates a lone surrogate while truncateUtf16Safe returns well-formed UTF-16 for the same boundary input.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied branch output showing raw slice creates a lone surrogate while truncateUtf16Safe returns well-formed UTF-16 for the same boundary input.
Evidence reviewed

PR surface:

Source 0, Tests +52. Total +52 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 2 0
Tests 1 52 0 +52
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 54 2 +52

What I checked:

Likely related people:

  • LiLan0125: git blame attributes the current Signal verbose preview body.slice(0, 200) line to merge commit 685b95b from fix(cron): warn when allowed web_search has no provider #97677. (role: introduced current behavior; confidence: high; commits: 685b95b607bb; files: extensions/signal/src/monitor/event-handler.ts)
  • scoootscooob: Commit 4540c6b moved the Signal implementation into extensions/signal/src/, creating the current bundled-plugin ownership surface. (role: feature-history contributor; confidence: medium; commits: 4540c6b3bc12; files: extensions/signal/src/monitor.ts, extensions/signal/src/monitor/event-handler.ts)
  • steipete: Commit 5967709 added the original Signal provider support before later moves and refactors carried the monitor path forward. (role: introduced Signal channel support; confidence: medium; commits: 596770942adb; files: src/signal/monitor.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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jun 30, 2026
@solodmd
solodmd force-pushed the fix/signal-truncate-preview-utf16 branch from 49f054c to 23b73da Compare June 30, 2026 02:28
@solodmd @claude
fix(signal): truncate verbose inbound preview on code-point boundaries
442322f 
Signal verbose inbound message previews truncate with body.slice(0, 200),
which can split a UTF-16 surrogate pair when an emoji straddles position
200, producing a lone high surrogate in the debug log.

Use truncateUtf16Safe from openclaw/plugin-sdk/text-utility-runtime so the
preview always lands on a valid code-point boundary.

Tests: 5 new tests covering the straddling-emoji boundary, emoji-before-
boundary, plain ASCII truncation, position-0 edge case, and short-body
pass-through.

Co-Authored-By: Claude <[email protected]>
@solodmd
solodmd force-pushed the fix/signal-truncate-preview-utf16 branch from 23b73da to 442322f Compare June 30, 2026 02:31
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 30, 2026
This was referenced Jun 30, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@vincentkoc vincentkoc self-assigned this Jun 30, 2026
@vincentkoc

vincentkoc commented Jun 30, 2026

Copy link
Copy Markdown
Member

Closing this as superseded by the broader UTF-16-safe truncation fix now on main: c16bb87.

I kept this as one canonical pass instead of landing the one-off PRs separately so Browser, Discord, Feishu, iMessage, MS Teams, Signal, Twitch, and Voice Call preview/error truncation all use the shared UTF-16 helpers consistently. Thanks for flagging this surface.

Proof for the landed commit:

  • focused Vitest: shared UTF helper plus Browser, Discord, Feishu, iMessage, MS Teams, Signal, Twitch, and Voice Call touched suites
  • autoreview: clean on the UTF commit
  • Crabbox/Azure changed gate: run_33aba0edad9a, lease cbx_09cf3426b424, passed OPENCLAW_TESTBOX=1 node scripts/crabbox-wrapper.mjs run -- env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 corepack pnpm check:changed

@vincentkoc vincentkoc closed this Jun 30, 2026
@wangmiao0668000666 wangmiao0668000666 mentioned this pull request Jul 1, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

channel: signal Channel integration: signal P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@solodmd @vincentkoc