fix(slack): keep inbound message preview truncation UTF-16 safe#101784
fix(slack): keep inbound message preview truncation UTF-16 safe#101784hugenshen wants to merge 1 commit into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 8, 2026, 12:45 AM ET / 04:45 UTC. Summary PR surface: Source +1, Tests +9. Total +10 across 2 files. Reproducibility: yes. Current main's Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Next step before merge
Security Review detailsBest possible solution: Merge the focused producer-boundary fix after maintainer review and exact-head validation, leaving downstream Slack dispatch behavior unchanged. Do we have a high-confidence way to reproduce the issue? Yes. Current main's Is this the best way to solve the issue? Yes. The malformed string is created in AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against bdc1ce8dede0. Label changesLabel justifications:
Evidence reviewedPR surface: Source +1, Tests +9. Total +10 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (6 earlier review cycles)
|
831d277 to
cac8da9
Compare
|
Squashed to a single commit and rebased on latest Test run @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
Replace raw .slice(0, 160) inbound preview truncation with truncateUtf16Safe so emoji straddling the cap are dropped whole instead of leaving unpaired surrogates in Slack inbound message previews. Co-authored-by: Cursor <[email protected]>
eda74e2 to
a9b3c61
Compare
|
Closing because The reachable Slack UTF-16 case is being handled in the wider canonical cleanup. Thanks @hugenshen for the contribution. |
What Problem This Solves
Fixes an issue where Slack messages containing emoji near the 160-character boundary produce an invalid
previewstring in thePreparedSlackMessageshape returned byprepareSlackMessage.When an inbound Slack message is prepared for dispatch, a 160-character preview is built from the raw message body by collapsing whitespace and calling
.slice(0, 160). Emoji and supplementary-plane characters use two UTF-16 code units (a surrogate pair). A cut that lands between the two halves of a surrogate pair produces a lone high surrogate — invalid Unicode — in thepreviewfield that is stored on thePreparedSlackMessageobject and passed to the dispatch pipeline.Concrete example: Inbound body
"a".repeat(159) + "🐱"(161 code units). Old path:previewlength 160, ends onU+D83D(unpaired high surrogate). Consumers ofPreparedSlackMessage.preview— system event labels, monitoring, debug output — receive malformed Unicode.Why This Change Was Made
Replace
.slice(0, 160)withtruncateUtf16Safe(..., 160), which detects and avoids splitting surrogate pairs. The preceding.replace(/\s+/g, " ")is unchanged.truncateUtf16Safefromopenclaw/plugin-sdk/text-utility-runtimeis the established helper for this class of fix throughout the codebase.User Impact
Before: Long Slack messages with emoji straddling the 160-character preview boundary leave
PreparedSlackMessage.previewwith invalid Unicode.After:
previewis always a valid Unicode string; boundary emoji are dropped whole instead of split.Evidence
extensions/slack/src/monitor/message-handler/prepare.ts:1182—const preview = truncateUtf16Safe(rawBody.replace(/\s+/g, " "), 160);PreparedSlackMessage.previewtyped intypes.ts:38and returned fromprepareSlackMessagepackages/normalization-core/src/utf16-slice.ts—truncateUtf16Safe/sliceUtf16Safesurrogate backoffprepare.ts; surrogate-boundary regression inprepare.test.tsBehavior proof
1) Node runtime — before/after on exact
prepare.tsexpression (no Vitest)Replays the shipped preview pipeline on the surrogate boundary input (
159 × 'a' + 🐱):2) Node runtime — production
prepareSlackMessage(no Vitest, no vi.mock)Imports PR-head
prepareSlackMessageand calls it with the same boundary input viacreateInboundSlackTestContext(real monitor context shape, no Vitest mocks):This exercises the full changed production path — not just the helper expression — and shows
PreparedSlackMessage.previewis surrogate-safe at the 160 code-unit cap.3) Call-site regression through production
prepareSlackMessageRegression flow:
message.text="a".repeat(159) + "🐱".prepareSlackMessagereturns non-nullPreparedSlackMessage.prepared.preview==="a".repeat(159)(159 code units)./[\uD800-\uDBFF](?![\uDC00-\uDFFF])/.test(prepared.preview)=== false.Proof gap: No redacted live Slack workspace transcript in this validation environment. Node
prepareSlackMessageproof + regression cover the production preview builder; optional supplemental proof is a redacted inbound Slack message log showing a long emoji-near-boundary preview stays well-formed.