Skip to content

fix(discord): keep thread-binding persona UTF-16 safe#103543

Merged
steipete merged 3 commits into
openclaw:mainfrom
MoerAI:fix/discord-persona-utf16-safe
Jul 10, 2026
Merged

fix(discord): keep thread-binding persona UTF-16 safe#103543
steipete merged 3 commits into
openclaw:mainfrom
MoerAI:fix/discord-persona-utf16-safe

Conversation

@MoerAI

@MoerAI MoerAI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Discord thread-binding personas are built by resolveThreadBindingPersona, which clamps the ⚙️ <label> display string to 80 characters with a raw value.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 truncateUtf16Safe primitive (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 unit 0xde00, isWellFormed() = true (complete pair)

  • Added a regression test through the real resolveThreadBindingPersona path (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 passed

  • pnpm tsgo:extensions and pnpm tsgo:extensions:test — passed

  • oxfmt --check, oxlint, git diff --check — clean

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

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: XS labels Jul 10, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jul 10, 2026
@clawsweeper

clawsweeper Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 10, 2026, 10:53 AM ET / 14:53 UTC.

Summary
The PR uses the shared UTF-16-safe truncator for Discord thread-binding webhook persona names, adds exact surrogate-boundary coverage, and updates release credit.

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
Persistent data-model change detected: vector/embedding metadata: CHANGELOG.md. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #103543
Summary: This focused PR is the viable implementation for the residual unsafe thread-binding persona path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Next step before merge

  • The PR is review-clean and needs no ClawSweeper repair job; ordinary maintainer landing review can proceed.

Security
Cleared: The patch changes only local string truncation, regression coverage, and release credit; it introduces no dependency, permission, secret, package-resolution, workflow, or code-execution concern.

Review details

Best 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 changes

Label changes:

  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P2: The patch fixes malformed Unicode in Discord webhook persona names at a narrow non-BMP length boundary.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR provides exact before-and-after output from the exported resolver at the malformed-surrogate boundary, a red-to-green focused regression, and green refreshed-head validation.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR provides exact before-and-after output from the exported resolver at the malformed-surrogate boundary, a red-to-green focused regression, and green refreshed-head validation.
Evidence reviewed

PR surface:

Source +1, Tests +5, Docs 0. Total +6 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 1 +1
Tests 1 5 0 +5
Docs 1 1 1 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 8 2 +6

What I checked:

Likely related people:

  • steipete: Introduced the current thread-binding persona surface, merged the related cross-surface UTF-16 consolidation, and refined this branch's boundary test and release credit. (role: recent area contributor and merger; confidence: high; commits: f1b75bf5fa34, 176fee5d071d, 68accbc74a5e; files: extensions/discord/src/monitor/thread-bindings.persona.ts, extensions/discord/src/monitor/thread-bindings.persona.test.ts, extensions/discord/src/monitor/thread-bindings.discord-api.ts)
  • vincentkoc: Recent merged work refactored and repaired the bound-thread reply-delivery path that consumes Discord identity information. (role: recent adjacent contributor; confidence: medium; commits: 99b24e741609, 0c210e5e524d; files: extensions/discord/src/monitor/reply-delivery.ts, extensions/discord/src/monitor/reply-delivery.test.ts)
  • scoootscooob: Carried the Discord channel implementation, including the persona files, into the bundled-plugin boundary during the channel migration. (role: feature-history contributor; confidence: medium; commits: 5682ec37fada; files: extensions/discord/src/monitor/thread-bindings.persona.ts, extensions/discord/src/monitor/thread-bindings.persona.test.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-10T11:25:06.022Z sha 03393af :: needs maintainer review before merge. :: none

@steipete
steipete force-pushed the fix/discord-persona-utf16-safe branch from 03393af to 0d8308f Compare July 10, 2026 14:31
MoerAI and others added 3 commits July 10, 2026 15:34
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.
@steipete
steipete force-pushed the fix/discord-persona-utf16-safe branch from 0d8308f to b07b6d5 Compare July 10, 2026 14:34
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 10, 2026
@steipete

Copy link
Copy Markdown
Contributor

Land-ready maintainer pass at exact head b07b6d5dda82876755132f505dd7287cff15ae73:

  • Kept the resolver-level fix at the shared thread-binding persona owner and replaced manual surrogate-range inspection with an exact output regression.
  • Verified the complete path from resolveThreadBindingPersonaFromRecord through maybeSendBindingMessage to the webhook username, plus the two already-safe sibling persona paths.
  • Sanitized secretless AWS Crabbox cbx_85471544b9d7, run run_02fbf79ccaae: persona, Discord API, reply-delivery, and outbound-adapter tests — 4 files, 79 tests passed.
  • Targeted oxfmt and git diff --check passed.
  • Fresh structured autoreview: clean, no accepted/actionable findings, correctness confidence 0.99.
  • Native OPENCLAW_TESTBOX=1 scripts/pr prepare-run 103543 accepted exact-head hosted evidence; live rollup is 61 successful checks, zero failures/pending (two cancelled superseded/noise runs and one neutral result).

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.

@steipete
steipete merged commit b745862 into openclaw:main Jul 10, 2026
100 of 102 checks passed
@steipete

Copy link
Copy Markdown
Contributor

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 11, 2026
* 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]>
@MoerAI
MoerAI deleted the fix/discord-persona-utf16-safe branch July 13, 2026 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: discord Channel integration: discord P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants