fix(cron): truncate failure alert error text on UTF-16 boundary#97298
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 27, 2026, 2:58 PM ET / 18:58 UTC. Summary PR surface: Source +1, Tests +60. Total +61 across 2 files. Reproducibility: yes. Source inspection shows current main slices the trimmed cron alert error at 200 UTF-16 code units, and an error string with an emoji straddling that boundary reaches the alert text path. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Next step before merge
Security Review detailsBest possible solution: Land the narrow helper reuse and regression test after normal maintainer review and required checks remain green. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main slices the trimmed cron alert error at 200 UTF-16 code units, and an error string with an emoji straddling that boundary reaches the alert text path. Is this the best way to solve the issue? Yes. The alert text is assembled in AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ebf1ba70d590. Label changesLabel justifications:
Evidence reviewedPR surface: Source +1, Tests +60. Total +61 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
|
clawsweeper
Bot
added
proof: sufficient
Verify that emitFailureAlert does not produce dangling surrogates when truncating a cron failure error message with a non-BMP character (emoji) straddling the 200-code-unit truncation boundary.
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🦐 gold shrimp
The dangling surrogate check loop stopped at length-1, missing a high surrogate at the final position. Extend to i < alertText.length so charCodeAt(i+1) returns NaN for the last char, correctly failing the high-surrogate pair assertion.
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
- Use template literal instead of string concatenation - Add braces to type guard if-statement
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
Mantis Telegram Desktop ProofSummary: Mantis captured Telegram Desktop before/after GIFs for the cron failure alert text.
Motion-trimmed clips: |
…claw#97298) * fix(cron): truncate failure alert error text on UTF-16 boundary * test(cron): add regression test for UTF-16 safe failure alert truncation Verify that emitFailureAlert does not produce dangling surrogates when truncating a cron failure error message with a non-BMP character (emoji) straddling the 200-code-unit truncation boundary. * fix(test): cover last code unit in surrogate scan The dangling surrogate check loop stopped at length-1, missing a high surrogate at the final position. Extend to i < alertText.length so charCodeAt(i+1) returns NaN for the last char, correctly failing the high-surrogate pair assertion. * fix(test): address lint issues in regression test - Use template literal instead of string concatenation - Add braces to type guard if-statement
…claw#97298) * fix(cron): truncate failure alert error text on UTF-16 boundary * test(cron): add regression test for UTF-16 safe failure alert truncation Verify that emitFailureAlert does not produce dangling surrogates when truncating a cron failure error message with a non-BMP character (emoji) straddling the 200-code-unit truncation boundary. * fix(test): cover last code unit in surrogate scan The dangling surrogate check loop stopped at length-1, missing a high surrogate at the final position. Extend to i < alertText.length so charCodeAt(i+1) returns NaN for the last char, correctly failing the high-surrogate pair assertion. * fix(test): address lint issues in regression test - Use template literal instead of string concatenation - Add braces to type guard if-statement
…claw#97298) * fix(cron): truncate failure alert error text on UTF-16 boundary * test(cron): add regression test for UTF-16 safe failure alert truncation Verify that emitFailureAlert does not produce dangling surrogates when truncating a cron failure error message with a non-BMP character (emoji) straddling the 200-code-unit truncation boundary. * fix(test): cover last code unit in surrogate scan The dangling surrogate check loop stopped at length-1, missing a high surrogate at the final position. Extend to i < alertText.length so charCodeAt(i+1) returns NaN for the last char, correctly failing the high-surrogate pair assertion. * fix(test): address lint issues in regression test - Use template literal instead of string concatenation - Add braces to type guard if-statement
…claw#97298) * fix(cron): truncate failure alert error text on UTF-16 boundary * test(cron): add regression test for UTF-16 safe failure alert truncation Verify that emitFailureAlert does not produce dangling surrogates when truncating a cron failure error message with a non-BMP character (emoji) straddling the 200-code-unit truncation boundary. * fix(test): cover last code unit in surrogate scan The dangling surrogate check loop stopped at length-1, missing a high surrogate at the final position. Extend to i < alertText.length so charCodeAt(i+1) returns NaN for the last char, correctly failing the high-surrogate pair assertion. * fix(test): address lint issues in regression test - Use template literal instead of string concatenation - Add braces to type guard if-statement
2 participants
Summary
Replace
String.prototype.slice(0, 200)withtruncateUtf16Safe()in cronemitFailureAlertto prevent broken Unicode from surrogate-pair splitting when error messages contain emoji or non-BMP characters.What Problem This Solves
When
emitFailureAlertinsrc/cron/service/failure-alerts.ts:116truncates an error message for a cron failure notification, it uses.slice(0, 200)which can split a UTF-16 surrogate pair in half. The resulting alert text routes through chat channels (as noted by the code comment: "Keep alert bodies compact because they may route through chat channels with notification previews") — where users would see garbled�characters.Code evidence: The current code does
(params.error?.trim() || "unknown reason").slice(0, 200), operating on UTF-16 code units. Characters outside BMP (emoji, rare CJK, etc.) are encoded as two code units; slicing at an arbitrary index can cut between them.Root Cause
9d31cbbd6a(2026-05-31, refactor: split cron service timer helpers)String.prototype.slice(n)assumes all characters are single-code-unit BMP characters. Provider error messages can legitimately contain non-BMP characters.Why This Fix
truncateUtf16Safeutility fromsrc/shared/utf16-slice.ts(same utility used by the merged PR fix(shared): use UTF-16 safe truncation in assistant error formatting #97289 for the same class of bug).User Impact
�if the error contained an emoji near the 200-char boundaryEvidence
Terminal proof script demonstrating the bug pattern at the 200-code-unit boundary (matching the exact
.slice(0, 200)inemitFailureAlert):Before (
.slice(0, 200)on a 201-code-unit error with emoji at boundary):After (
truncateUtf16Safeat the same boundary):Real Behavior Proof
Before (on main)
After (with fix)
Tests and Validation
pnpm checkpassed