fix(line): truncate action label/data on code-point boundaries#96493
fix(line): truncate action label/data on code-point boundaries#96493ly-wang19 wants to merge 2 commits into
Conversation
The LINE action builders truncated label (20) and data/displayText (300) with a raw `String.prototype.slice`, which cuts an emoji apart when the limit falls inside a surrogate pair. A 19-char label followed by 😀 (U+1F600) yielded a label ending in a lone high surrogate (\uD83D), which LINE renders as a replacement glyph or rejects. Truncate with `truncateUtf16Safe` instead so a dangling half-emoji is dropped, matching how this plugin already truncates LINE template titles. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 1:08 PM ET / 17:08 UTC. Summary PR surface: Source +1, Tests +67. Total +68 across 2 files. Reproducibility: yes. for source-level reproduction: current main uses raw UTF-16 slicing in the LINE action builders, and a Node probe shows the reported input leaves a dangling high surrogate. I did not run the PR's tests in this read-only review. 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:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this owner-local helper-based fix after the contributor adds real after-fix proof for the changed LINE builders; handle broader LINE grapheme-counting semantics as a separate follow-up if desired. Do we have a high-confidence way to reproduce the issue? Yes for source-level reproduction: current main uses raw UTF-16 slicing in the LINE action builders, and a Node probe shows the reported input leaves a dangling high surrogate. I did not run the PR's tests in this read-only review. Is this the best way to solve the issue? Yes for the narrow surrogate-safety bug: reusing the existing exported SDK helper inside the LINE plugin boundary is the maintainable fix. It is not a full grapheme-cluster length-counting solution for all LINE-visible fields. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4ae0a5d958a0. Label changesLabel justifications:
Evidence reviewedPR surface: Source +1, Tests +67. Total +68 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
|
- drop the unnecessary template literal (no-unnecessary-template-expression)
- cast messageAction/uriAction results to { label: string } so loneHighSurrogate.test()
receives a string (matches the postback/datetime test casts)
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
Closing to stay under the repo's active-PR limit and keep my queue focused on changes I can fully prove headlessly. This channel fix needs live-channel proof I can't generate in my current environment; the code change itself stands and I'll resubmit with live proof when I can run the channel. |
What Problem This Solves
The LINE action builders in
extensions/line/src/actions.tsenforce LINE'sfield limits by calling
String.prototype.slicedirectly:messageAction/uriAction/postbackAction/datetimePickerAction→label.slice(0, 20)postbackAction/datetimePickerAction→data.slice(0, 300)anddisplayText?.slice(0, 300)A raw UTF-16 slice cuts an emoji in half whenever the limit lands inside a
surrogate pair. The returned
labelthen ends in an unpaired highsurrogate, which LINE renders as a replacement character (or rejects the
payload).
Repro:
messageAction("1234567890123456789😀")— 19 ASCII chars + 😀(U+1F600, 2 UTF-16 code units) = 21 code units.
labelreturned"1234567890123456789\uD83D"— trailing lone high surrogate"1234567890123456789"— half-emoji dropped, no lone surrogateThe fix swaps the raw slices for
truncateUtf16Safe(value, limit)fromopenclaw/plugin-sdk/text-utility-runtime, which drops a dangling surrogateinstead of emitting it. This matches how this plugin already truncates LINE
template titles (
truncateTemplateTextintemplate-messages.ts). Normalinputs (short labels, exactly-at-limit ASCII, fully-fitting emoji) are
unchanged, and
displayTextstaysundefinedwhen omitted.Evidence
Standalone red→green proof (replicates
truncateUtf16Safe+ the old vs. newaction logic):
The script asserts:
label.slice(0, 20)keeps a lone high surrogate (\uD83D)."1234567890123456789"(no lonesurrogate) for
label, and surrogate-safedata/displayText."Yes"→"Yes", 20-char ASCII unchanged, 25-char ASCIItruncated to 20, fully-fitting
"hi😀"preserved, anddisplayTextstaysundefinedwhen not passed.New unit tests in
extensions/line/src/message-cards.test.tscovermessageAction,uriAction,postbackAction(label, data, displayText), anddatetimePickerAction, asserting no/[\uD800-\uDBFF](?![\uDC00-\uDFFF])/lone surrogate survives truncation.
🤖 Generated with Claude Code