fix(discord): keep thread-binding persona UTF-16 safe#103543
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 10, 2026, 10:53 AM ET / 14:53 UTC. Summary PR surface: Source +1, Tests +5, Docs 0. Total +6 across 3 files. Reproducibility: yes. Current main's raw 80-code-unit slice deterministically leaves a dangling high surrogate for the documented boundary input, and the result is used directly as a Discord webhook username. Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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: Land the focused shared-helper substitution with the exact boundary regression, preserving the existing Discord limit, prefix, fallback selection, and webhook delivery behavior. Do we have a high-confidence way to reproduce the issue? Yes. Current main's raw 80-code-unit slice deterministically leaves a dangling high surrogate for the documented boundary input, and the result is used directly as a Discord webhook username. Is this the best way to solve the issue? Yes. This is the narrowest maintainable fix because it applies the repository's established public SDK primitive at the single owner construction point without adding configuration, policy, compatibility logic, or a duplicate implementation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against b2c1db4b5eb6. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1, Tests +5, Docs 0. Total +6 across 3 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 (1 earlier review cycle)
|
03393af to
0d8308f
Compare
resolveThreadBindingPersona truncated the persona label with a raw .slice(0, THREAD_BINDING_PERSONA_MAX_CHARS). When an emoji or other astral code point straddles the 80-unit limit, the raw slice keeps a dangling high surrogate, so the Discord thread-binding persona (webhook display name) can carry a lone surrogate. Route the clamp through the shared truncateUtf16Safe primitive so the boundary code point is dropped whole. Adds a regression test.
0d8308f to
b07b6d5
Compare
|
Land-ready maintainer pass at exact head
Known proof gaps: no live Discord credentialed send was needed for this deterministic string-boundary change. Historical PR #101644 contained this omitted hunk and will receive a correction after the canonical merge; the changelog already credits both @MoerAI and @ly85206559. |
|
Merged via squash.
|
* fix(discord): keep thread-binding persona UTF-16 safe resolveThreadBindingPersona truncated the persona label with a raw .slice(0, THREAD_BINDING_PERSONA_MAX_CHARS). When an emoji or other astral code point straddles the 80-unit limit, the raw slice keeps a dangling high surrogate, so the Discord thread-binding persona (webhook display name) can carry a lone surrogate. Route the clamp through the shared truncateUtf16Safe primitive so the boundary code point is dropped whole. Adds a regression test. * test(discord): assert exact persona boundary * docs(changelog): credit Discord persona fix --------- Co-authored-by: Peter Steinberger <[email protected]>
What Problem This Solves
Discord thread-binding personas are built by
resolveThreadBindingPersona, which clamps the⚙️ <label>display string to 80 characters with a rawvalue.slice(0, 80). When an emoji or other astral code point straddles that limit, the raw slice keeps a dangling high surrogate, so the persona label used as the Discord thread webhook display name can carry a lone (invalid) surrogate.Why This Change Was Made
Route the clamp through the shared
truncateUtf16Safeprimitive (openclaw/plugin-sdk/text-utility-runtime), which backs off one code unit when the boundary would split a surrogate pair. The 80-character bound and the⚙️prefix are unchanged; the persona can only get one code unit shorter in the boundary case, never longer, and it is never emitted with a split surrogate.User Impact
Discord thread-binding persona names (webhook display names) stay valid Unicode. An emoji at the length boundary is dropped whole instead of surfacing as a broken/replacement character in Discord.
Evidence
Real behavior at the 80-unit boundary (persona label = 50x U+1F600), using the exported
resolveThreadBindingPersona:Before (raw slice): length 80, last unit
0xd83d,isWellFormed() = false(dangling high surrogate)After (
truncateUtf16Safe): length 79, last unit0xde00,isWellFormed() = true(complete pair)Added a regression test through the real
resolveThreadBindingPersonapath (an emoji straddling the 80-char limit), asserting no trailing high surrogate. RED before the fix (expected true to be false), GREEN after.pnpm test extensions/discord/src/monitor/thread-bindings.persona.test.ts— 4 passedpnpm tsgo:extensionsandpnpm tsgo:extensions:test— passedoxfmt --check,oxlint,git diff --check— cleanAI-assisted (Claude Code); I reviewed and understand every line.