fix(agents,skills): keep session-identity and experience-review transcript truncation UTF-16 safe#106370
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 15, 2026, 11:54 PM ET / July 16, 2026, 03:54 UTC. Summary PR surface: Source +4, Tests +108. Total +112 across 4 files. Reproducibility: yes. —the raw current behavior is deterministically exposed by placing a surrogate pair across the 200-, 6,000-, or retained-tail boundary, and the submitted fixtures demonstrate those exact conditions. The review did not independently execute current main, so the structured status remains source-reproducible rather than reproduced. 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. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the shared-helper substitutions and boundary tests without broadening the change, after refreshing against current main and retaining the existing character budgets and transcript layout. Do we have a high-confidence way to reproduce the issue? Yes—the raw current behavior is deterministically exposed by placing a surrogate pair across the 200-, 6,000-, or retained-tail boundary, and the submitted fixtures demonstrate those exact conditions. The review did not independently execute current main, so the structured status remains source-reproducible rather than reproduced. Is this the best way to solve the issue? Yes—the shared normalization-core helpers are the narrowest maintainable fix and match recently merged handling on sibling truncation surfaces; duplicating custom surrogate logic would be worse. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against de3f7925c4c3. Label changesLabel justifications:
Evidence reviewedPR surface: Source +4, Tests +108. Total +112 across 4 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 (5 earlier review cycles)
|
17a1e9b to
4e64571
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
4e64571 to
4db3c3b
Compare
4db3c3b to
7c101c4
Compare
…cript truncation UTF-16 safe normalizeCliContextValue (cli-output.ts) and formatSkillExperienceReviewTranscript (experience-review-prompt.ts) both used raw .slice(0, N) / .slice(-N) to truncate user-facing strings. When a truncation boundary falls inside a surrogate pair (emoji), the resulting string contains a lone surrogate — encodeURIComponent throws URIError, JSON serialisers may reject it, and downstream display surfaces can render garbage. Replace .slice(0, 200) with truncateUtf16Safe in normalizeCliContextValue. Replace .slice(0, 6_000) with truncateUtf16Safe and full.slice(-tailBudget) with sliceUtf16Safe(full, tailStart) in formatSkillExperienceReviewTranscript. Follows the pattern established by openclaw#98644, openclaw#102470, openclaw#101728 and the broader UTF-16 safety sweep already applied across the codebase.
…y old slices dangle Both experience-review fixtures now exceed EXPERIENCE_REVIEW_MAX_TRANSCRIPT_CHARS so the truncation branch actually fires. Pre-condition assertions prove the old raw .slice(0,6000) / .slice(tailStart) produce isolated surrogates; the production functions (formatSkillExperienceReviewTranscript, formatCliOutputError) do not. cli-output test also adds pre-condition check showing normalizeCliContextValue with raw .slice(0,200) would split a surrogate pair.
7c101c4 to
13b37f0
Compare
Co-authored-by: 毛宇豪0668001457 <[email protected]>
|
Merged via squash.
|
…cript truncation UTF-16 safe (openclaw#106370) * fix(agents,skills): keep session-identity and experience-review transcript truncation UTF-16 safe normalizeCliContextValue (cli-output.ts) and formatSkillExperienceReviewTranscript (experience-review-prompt.ts) both used raw .slice(0, N) / .slice(-N) to truncate user-facing strings. When a truncation boundary falls inside a surrogate pair (emoji), the resulting string contains a lone surrogate — encodeURIComponent throws URIError, JSON serialisers may reject it, and downstream display surfaces can render garbage. Replace .slice(0, 200) with truncateUtf16Safe in normalizeCliContextValue. Replace .slice(0, 6_000) with truncateUtf16Safe and full.slice(-tailBudget) with sliceUtf16Safe(full, tailStart) in formatSkillExperienceReviewTranscript. Follows the pattern established by openclaw#98644, openclaw#102470, openclaw#101728 and the broader UTF-16 safety sweep already applied across the codebase. * chore: re-trigger CI after PR body update * test(agents,skills): construct transcripts exceeding 60k chars, verify old slices dangle Both experience-review fixtures now exceed EXPERIENCE_REVIEW_MAX_TRANSCRIPT_CHARS so the truncation branch actually fires. Pre-condition assertions prove the old raw .slice(0,6000) / .slice(tailStart) produce isolated surrogates; the production functions (formatSkillExperienceReviewTranscript, formatCliOutputError) do not. cli-output test also adds pre-condition check showing normalizeCliContextValue with raw .slice(0,200) would split a surrogate pair. * test: tighten UTF-16 truncation coverage Co-authored-by: 毛宇豪0668001457 <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>
…cript truncation UTF-16 safe (openclaw#106370) * fix(agents,skills): keep session-identity and experience-review transcript truncation UTF-16 safe normalizeCliContextValue (cli-output.ts) and formatSkillExperienceReviewTranscript (experience-review-prompt.ts) both used raw .slice(0, N) / .slice(-N) to truncate user-facing strings. When a truncation boundary falls inside a surrogate pair (emoji), the resulting string contains a lone surrogate — encodeURIComponent throws URIError, JSON serialisers may reject it, and downstream display surfaces can render garbage. Replace .slice(0, 200) with truncateUtf16Safe in normalizeCliContextValue. Replace .slice(0, 6_000) with truncateUtf16Safe and full.slice(-tailBudget) with sliceUtf16Safe(full, tailStart) in formatSkillExperienceReviewTranscript. Follows the pattern established by openclaw#98644, openclaw#102470, openclaw#101728 and the broader UTF-16 safety sweep already applied across the codebase. * chore: re-trigger CI after PR body update * test(agents,skills): construct transcripts exceeding 60k chars, verify old slices dangle Both experience-review fixtures now exceed EXPERIENCE_REVIEW_MAX_TRANSCRIPT_CHARS so the truncation branch actually fires. Pre-condition assertions prove the old raw .slice(0,6000) / .slice(tailStart) produce isolated surrogates; the production functions (formatSkillExperienceReviewTranscript, formatCliOutputError) do not. cli-output test also adds pre-condition check showing normalizeCliContextValue with raw .slice(0,200) would split a surrogate pair. * test: tighten UTF-16 truncation coverage Co-authored-by: 毛宇豪0668001457 <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>
What Problem This Solves
normalizeCliContextValue(cli-output.ts) andformatSkillExperienceReviewTranscript(experience-review-prompt.ts) use raw.slice(0, N)/.slice(-N)to truncate user-visible strings. When a truncation boundary falls inside a UTF-16 surrogate pair, the result contains a lone surrogate —encodeURIComponent()throwsURIError, JSON serialisers may reject the string, and downstream display surfaces render broken characters.The codebase already applies
truncateUtf16Safe/sliceUtf16Safein 30+ locations. These two truncation sites were missed.Why This Change Was Made
Replace raw
.slice(0, N)withtruncateUtf16Safe, andfull.slice(-tailBudget)withsliceUtf16Safe(full, tailStart). Same approach as #98644 and #102470.User Impact
formatCliOutputErrorno longer produce broken characters at emoji boundariesEvidence
Environment: Node.js v24.13.1, @openclaw/normalization-core/utf16-slice, Linux x86_64.
Production-function proof (through
formatCliOutputErrorandformatSkillExperienceReviewTranscript, not raw helpers):All three scenarios: old code produces isolated surrogates, production functions are clean.
Unit tests (each pre-verifies the old
.slice()dangles, then calls the production function):Both fixtures now exceed
EXPERIENCE_REVIEW_MAX_TRANSCRIPT_CHARS(60,000), so the truncation branch fires:rendered[0]has high surrogate at index 5,999 →.slice(0,6000)dangles →formatSkillExperienceReviewTranscriptis cleanfull.slice(tailStart)starts at low surrogate →formatSkillExperienceReviewTranscriptis cleannormalizeCliContextValueraw.slice(0,200)dangles →formatCliOutputErroris cleanDiff: 4 files, +87/-14. No pnpm-lock.yaml. Branch from upstream/main.
AI-assisted.