fix(chunk): keep surrogate pairs whole when hard-splitting an over-long line#96951
Merged
vincentkoc merged 1 commit intoJun 26, 2026
Merged
Conversation
openclaw-barnacle
Bot
added
size: XS
triage: needs-pr-context
Contributor
|
Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 5:01 PM ET / 21:01 UTC. Summary PR surface: Source +3, Tests +13. Total +16 across 2 files. Reproducibility: yes. Source inspection shows current main still slices the first Review metrics: none identified. 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. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land one proof-backed canonical fix that makes every Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main still slices the first Is this the best way to solve the issue? Yes. Reusing the existing AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9636bea901d0. Label changesLabel justifications:
Evidence reviewedPR surface: Source +3, Tests +13. Total +16 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
|
openclaw-barnacle
Bot
removed
the
triage: needs-pr-context
clawsweeper
Bot
added
proof: sufficient
…ng line chunkByNewline length-splits a single over-long line that has no usable break point. The first head cut used a raw UTF-16 slice (lineValue.slice(0, firstLimit)), so when firstLimit landed inside a surrogate pair it emitted a chunk ending in a lone high surrogate and a next chunk starting with a lone low surrogate, which render as U+FFFD on delivery. The recursive chunkText that handles the remainder is already surrogate-safe; only this first cut was raw. Route the head cut through avoidTrailingHighSurrogateBreak (the same helper chunkText and chunkMarkdownText already use) so the cut backs off to a code-point boundary. ASCII and non-surrogate cuts are unaffected. Reproduces at the production-default 4096 limit for emoji-dense lines; chunkByNewline is the published plugin-SDK channel.text helper, reachable with arbitrary outbound text. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
vincentkoc
force-pushed
the
salvage/96671-chunk-surrogate-firstcut
branch
from
June 26, 2026 20:57
c15254a to
6940d70
Compare
Member
|
Merged via squash.
|
wangmiao0668000666
pushed a commit
to wangmiao0668000666/openclaw
that referenced
this pull request
Jun 27, 2026
…ng line (openclaw#96951) chunkByNewline length-splits a single over-long line that has no usable break point. The first head cut used a raw UTF-16 slice (lineValue.slice(0, firstLimit)), so when firstLimit landed inside a surrogate pair it emitted a chunk ending in a lone high surrogate and a next chunk starting with a lone low surrogate, which render as U+FFFD on delivery. The recursive chunkText that handles the remainder is already surrogate-safe; only this first cut was raw. Route the head cut through avoidTrailingHighSurrogateBreak (the same helper chunkText and chunkMarkdownText already use) so the cut backs off to a code-point boundary. ASCII and non-surrogate cuts are unaffected. Reproduces at the production-default 4096 limit for emoji-dense lines; chunkByNewline is the published plugin-SDK channel.text helper, reachable with arbitrary outbound text. Co-authored-by: ly-wang19 <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
github-actions Bot
pushed a commit
to Desicool/openclaw
that referenced
this pull request
Jun 27, 2026
…ng line (openclaw#96951) chunkByNewline length-splits a single over-long line that has no usable break point. The first head cut used a raw UTF-16 slice (lineValue.slice(0, firstLimit)), so when firstLimit landed inside a surrogate pair it emitted a chunk ending in a lone high surrogate and a next chunk starting with a lone low surrogate, which render as U+FFFD on delivery. The recursive chunkText that handles the remainder is already surrogate-safe; only this first cut was raw. Route the head cut through avoidTrailingHighSurrogateBreak (the same helper chunkText and chunkMarkdownText already use) so the cut backs off to a code-point boundary. ASCII and non-surrogate cuts are unaffected. Reproduces at the production-default 4096 limit for emoji-dense lines; chunkByNewline is the published plugin-SDK channel.text helper, reachable with arbitrary outbound text. Co-authored-by: ly-wang19 <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
xydigit-zt
pushed a commit
to xydigit-zt/xydigit-zt-openclaw
that referenced
this pull request
Jun 28, 2026
…ng line (openclaw#96951) chunkByNewline length-splits a single over-long line that has no usable break point. The first head cut used a raw UTF-16 slice (lineValue.slice(0, firstLimit)), so when firstLimit landed inside a surrogate pair it emitted a chunk ending in a lone high surrogate and a next chunk starting with a lone low surrogate, which render as U+FFFD on delivery. The recursive chunkText that handles the remainder is already surrogate-safe; only this first cut was raw. Route the head cut through avoidTrailingHighSurrogateBreak (the same helper chunkText and chunkMarkdownText already use) so the cut backs off to a code-point boundary. ASCII and non-surrogate cuts are unaffected. Reproduces at the production-default 4096 limit for emoji-dense lines; chunkByNewline is the published plugin-SDK channel.text helper, reachable with arbitrary outbound text. Co-authored-by: ly-wang19 <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
QiuYuang
pushed a commit
to QiuYuang/openclaw
that referenced
this pull request
Jul 1, 2026
…ng line (openclaw#96951) chunkByNewline length-splits a single over-long line that has no usable break point. The first head cut used a raw UTF-16 slice (lineValue.slice(0, firstLimit)), so when firstLimit landed inside a surrogate pair it emitted a chunk ending in a lone high surrogate and a next chunk starting with a lone low surrogate, which render as U+FFFD on delivery. The recursive chunkText that handles the remainder is already surrogate-safe; only this first cut was raw. Route the head cut through avoidTrailingHighSurrogateBreak (the same helper chunkText and chunkMarkdownText already use) so the cut backs off to a code-point boundary. ASCII and non-surrogate cuts are unaffected. Reproduces at the production-default 4096 limit for emoji-dense lines; chunkByNewline is the published plugin-SDK channel.text helper, reachable with arbitrary outbound text. Co-authored-by: ly-wang19 <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Problem This Solves
chunkByNewlinesplits a surrogate pair when it hard-cuts an over-long line that has no break point, so an emoji-dense message can produce a chunk ending in a lone high surrogate (and the next starting with a lone low surrogate) — corrupting the emoji at every chunk boundary.Salvages #96671 by @ly-wang19 onto current
main— clean single-commit rebase, original authorship preserved. The original was auto-closed byopenclaw-barnaclefor the author's >20-active-PR cap, not on merits; the bug is still live onmain.Root Cause
Symptom — A long unbroken line of emoji (e.g.
"😀".repeat(30)) chunked at a small limit yields chunks that end in\uD800–\uDBFF(lone high surrogate) / start with\uDC00–\uDFFF(lone low surrogate). The emoji at the cut is mojibake.Root cause — In
src/auto-reply/chunk.ts,chunkByNewline()computes the head cut asfirstLimit = Math.max(1, maxLineLength - prefix.length)and slices the line at that raw UTF-16 code-unit offset. If that offset lands between the two code units of a surrogate pair, the pair is severed. The recursivechunkTextpath below already routes throughavoidTrailingHighSurrogateBreak; only this first cut was raw.Fix + why this level — Run the existing, already-imported
avoidTrailingHighSurrogateBreak(lineValue, 0, rawLimit)on the first cut, mirroring the contract the recursive path already honors. Fixing at the cut site is correct: it's the only place that produces a non-code-point-aligned boundary, and reusing the existing helper keeps both cut paths consistent.Scope / risk — Touches only the first-cut offset in
chunkByNewline. Lines with a natural break point and BMP-only content are unaffected (back-off is a no-op when the boundary is already aligned).chunks.join("")still reconstructs the input exactly.Evidence
src/auto-reply/chunk.test.ts→ "does not split surrogate pairs when hard-splitting an over-long line" — fails without the fix.WITH fix:
WITHOUT fix (test against current
mainsource forchunk.ts):Changes from original
main(clean single commit, no conflicts).Note: #96415 addressed the same
firstLimitcut; this salvage (#96671) is the cleaner of the two and supersedes it.Credit: Salvage of #96671 by @ly-wang19 — rebased onto current main. Original closed by the >20-active-PR queue cap, not on merits.