fix(auto-reply): keep ACP steer output UTF-16 safe#102583
Conversation
…output truncation Replace naive .slice(0, N) with truncateUtf16Safe() in: - bash-command.ts: shell command name preview - lifecycle.ts: ACP steer output text Co-Authored-By: Claude <[email protected]>
|
Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 5:22 AM ET / 09:22 UTC. Summary PR surface: Source +2. Total +2 across 2 files. Reproducibility: yes. by source inspection: current main can cut a UTF-16 surrogate pair when bash session snippets or ACP steer output land exactly on the raw 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:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the helper-based truncation fix after the PR body includes the user-visible symptom, redacted after-fix proof, and preferably a focused surrogate-boundary regression check. Do we have a high-confidence way to reproduce the issue? Yes by source inspection: current main can cut a UTF-16 surrogate pair when bash session snippets or ACP steer output land exactly on the raw Is this the best way to solve the issue? Yes, using the existing AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3f2466c4c378. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +2. Total +2 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
|
|
Land-ready at exact head Maintainer review removed the original bash-session edit after tracing session slugs to fixed ASCII word lists, leaving only the real arbitrary ACP response-text boundary. Command-level regression proof passed at the 800-code-unit cutoff (1 selected, 60 skipped), targeted oxfmt and Best-fix verdict: the narrowed patch fixes the owned delivered-reply boundary and avoids dead defensive code in the ASCII-only slug path. |
|
Merged via squash.
|
* fix(auto-reply): use truncateUtf16Safe in bash command and ACP steer output truncation Replace naive .slice(0, N) with truncateUtf16Safe() in: - bash-command.ts: shell command name preview - lifecycle.ts: ACP steer output text Co-Authored-By: Claude <[email protected]> * test(auto-reply): prove safe ACP steer output bounds --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
What Problem This Solves
ACP steer acknowledgements buffer model-provided output up to 800 UTF-16 code units. The previous raw prefix slice could leave an unpaired surrogate when that limit fell inside an emoji or other supplementary character.
Why This Change Was Made
The steer-output owner now uses the shared UTF-16-safe truncation helper before appending its existing ellipsis. Maintainer review removed the proposed bash-session change because bash session IDs come from a fixed ASCII slug alphabet, so that path cannot hit this bug.
User Impact
Long
/acp steeroutput remains well-formed while preserving the existing output limit and acknowledgement format.Evidence
/acp steer: 1 passed, 60 skipped.git diff --checkpass.AI-assisted
The original patch was generated with Claude Code and then narrowed and tested during maintainer review.