Skip to content

fix(auto-reply): keep conversation label truncation surrogate-safe#102697

Closed
zw-xysk wants to merge 2 commits into
openclaw:mainfrom
zw-xysk:fix/conversation-label-safe
Closed

fix(auto-reply): keep conversation label truncation surrogate-safe#102697
zw-xysk wants to merge 2 commits into
openclaw:mainfrom
zw-xysk:fix/conversation-label-safe

Conversation

@zw-xysk

@zw-xysk zw-xysk commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

conversation-label-generator.ts truncates LLM-generated conversation labels using .slice(0, N) before returning them for UI display. When the truncation boundary falls inside a UTF-16 surrogate pair (emoji, CJK extended), the resulting label contains a dangling surrogate that renders as U+FFFD (�) in the UI.

Changes

src/auto-reply/reply/conversation-label-generator.ts (+2, −1):

  • Import truncateUtf16Safe from @openclaw/normalization-core/utf16-slice
  • text.slice(0, maxLength) → truncateUtf16Safe(text, maxLength)

Evidence

Unit tests (7/7 passed):

 Test Files  1 passed (1)
      Tests  7 passed (7)

Real behavior proof (platinum):

[+0.001s] ═══ conversation-label-generator proof ═══
[+0.001s] OLD .slice(0,50): ✗ BROKEN
[+0.001s] NEW truncateUtf16Safe: ✓ OK
[+0.001s] RESULT: PASS (platinum)

Review

  • Manually reviewed and verified
  • AI-assisted: Yes

@zw-xysk

zw-xysk commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: current main still has the conversation-label UTF-16 truncation bug, but the open consolidation PR already carries this same fix with stronger empty-result handling, broader related coverage, sufficient proof, and a mergeable canonical landing path.

Root-cause cluster
Relationship: superseded
Canonical: #102823
Summary: This PR is a small conversation-label instance of the broader bounded UTF-16 truncation cleanup now owned by the open consolidation PR.

Members:

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

Canonical path: Review and land #102823 as the canonical UTF-16 boundary fix, preserving this contributor's useful conversation-label change through that branch.

So I’m closing this here and keeping the remaining discussion on #102823.

Review details

Best possible solution:

Review and land #102823 as the canonical UTF-16 boundary fix, preserving this contributor's useful conversation-label change through that branch.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main returns text.slice(0, maxLength) for generated labels, so placing a surrogate pair across the cap can produce a dangling surrogate; the helper tests establish the boundary behavior without needing a full UI run.

Is this the best way to solve the issue?

No for this branch as the landing path. The helper swap is the right primitive, but the canonical consolidation PR includes the same fix plus || null handling and stronger regression coverage, so this PR is superseded.

Security review:

Security review cleared: The current PR only uses an existing internal string helper and adds a focused unit test; it does not touch dependencies, workflows, secrets, permissions, or code-execution surfaces.

AGENTS.md: found and applied where relevant.

What I checked:

  • Repository policy read: Root AGENTS.md was read fully; no scoped AGENTS.md owns src/auto-reply/reply, and the review applied the whole-path, proof, and supersession guidance. (AGENTS.md:1, f6b990124353)
  • Current main still has the raw truncation: generateConversationLabel currently returns text.slice(0, maxLength), so an emoji or supplementary-plane character split at the cap can leave a dangling UTF-16 surrogate in the generated label. (src/auto-reply/reply/conversation-label-generator.ts:117, f6b990124353)
  • Shared helper contract: truncateUtf16Safe delegates to sliceUtf16Safe, which backs away from split surrogate-pair boundaries; its tests also show the helper can return an empty string when the first emoji cannot fit. (packages/normalization-core/src/utf16-slice.ts:44, f6b990124353)
  • This PR's useful change: The branch imports truncateUtf16Safe and swaps the raw return to truncateUtf16Safe(text, maxLength), with a focused test for a split rocket emoji at the label cap. (src/auto-reply/reply/conversation-label-generator.ts:115, 84f00bebb9b0)
  • Canonical replacement is same-or-better: The open consolidation PR explicitly supersedes this PR and changes the same line to truncateUtf16Safe(text, maxLength) || null, adding tests for both a split emoji and the empty-label edge. (src/auto-reply/reply/conversation-label-generator.ts:115, 7e666f6b73cd)
  • Canonical PR proof and viability: The canonical PR is open, mergeable, labeled proof: sufficient and status: 👀 ready for maintainer look, and its body links focused Testbox/GitHub Actions runs for the conversation-label and broader UTF-16 regressions. (7e666f6b73cd)

Likely related people:

  • obviyus: Git blame and the commit-to-PR lookup show the current conversation-label generator, dashboard title caller, and UTF-16 helper came in through the merged context-map PR authored by obviyus. (role: introduced current behavior; confidence: high; commits: 34d257713ea3; files: src/auto-reply/reply/conversation-label-generator.ts, src/gateway/dashboard-session-title.ts, packages/normalization-core/src/utf16-slice.ts)
  • steipete: steipete is the official maintainer listed in CONTRIBUTING.md, is assigned to this PR, added the branch's regression-test commit, and authored the open consolidation PR that explicitly supersedes this branch. (role: canonical follow-up owner; confidence: high; commits: 84f00bebb9b0, 13b4d8bf8751, 7e666f6b73cd; files: src/auto-reply/reply/conversation-label-generator.ts, src/auto-reply/reply/conversation-label-generator.test.ts)

Codex review notes: model internal, reasoning high; reviewed against f6b990124353.

@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 9, 2026
@steipete steipete self-assigned this Jul 9, 2026
@steipete steipete mentioned this pull request Jul 9, 2026
Merged
zw-xysk and others added 2 commits July 9, 2026 09:11
@zw-xysk @steipete
fix(auto-reply): keep conversation label truncation surrogate-safe
068c124 
Conversation label text generated by LLM is truncated using .slice(0, N)
which can split a UTF-16 surrogate pair when the boundary falls inside
an emoji, producing invalid Unicode in UI-displayed session labels.

Replace .slice(0, maxLength) with truncateUtf16Safe(text, maxLength).
@steipete
test(auto-reply): cover label surrogate boundary
84f00be
@steipete
steipete force-pushed the fix/conversation-label-safe branch from 2a4a11f to 84f00be Compare July 9, 2026 13:15
@clawsweeper clawsweeper Bot closed this Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🦞✅
ClawSweeper autoclose is complete.

Reason: structured ClawSweeper close marker: close-required (sha=84f00bebb9b0f7aa5b59ee4c18e267902096be00)

Closed:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zw-xysk @steipete