Repository navigation
fix(terminal): preserve escape tails during quiet snapshot handoff - #33677
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
[#33608] requires interactive TUI panes to retain PTY raw mode (-icanon -echo) and accept Enter. The reviewed changes only alter hidden output snapshot replay and ANSI/parser-state handling in `hidd… |
Implement and test the PTY input-mode behavior required by [#33608], including raw-mode retention and Enter handling, or remove [#33608] as a direct link if this PR does not intend to address that issue. |
| Out of Scope Changes check | |
The production changes address output replay state, escape-sequence continuity, and repaint-gap handling. These changes do not address [#33608]'s input-side failure in which the PTY returns to canonic… |
Remove [#33608] from the direct issue links or add implementation that addresses its raw-mode and Enter requirements. Keep the current output-replay changes linked only to the issue that defines that scope. |
| Docstring Coverage | |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (1 skipped: 1 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: preserving escape tails during terminal snapshot handoff. Description check The description is detailed and covers the user impact, implementation, rationale, linked issue context, visual proof, testing, scope, and residual risks. It omits the template's final Checklist secti… Full details: Description check
-
Explanation
The description is detailed and covers the user impact, implementation, rationale, linked issue context, visual proof, testing, scope, and residual risks. It omits the template's final Checklist section and does not use the exact
Fixes #format, but the required information is otherwise substantially present.
-
Full details: Linked Issues check
-
Explanation
[
#33608] requires interactive TUI panes to retain PTY raw mode (-icanon -echo) and accept Enter. The reviewed changes only alter hidden output snapshot replay and ANSI/parser-state handling inhidden-output-restore-drain.tsandhidden-output-restore-abandon.ts. The supplied PR evidence states that kernel TTY flags are unchanged. No implementation or test establishes compliance with the issue requirements.Resolution
Implement and test the PTY input-mode behavior required by [
#33608], including raw-mode retention and Enter handling, or remove [#33608] as a direct link if this PR does not intend to address that issue.
Full details: Out of Scope Changes check
-
Explanation
The production changes address output replay state, escape-sequence continuity, and repaint-gap handling. These changes do not address [
#33608]'s input-side failure in which the PTY returns to canonical mode and echoes input. The PR description also explicitly states that it does not resolve [#33608]. Therefore the reviewed implementation is outside the scope of the directly linked issue.Resolution
Remove [
#33608] from the direct issue links or add implementation that addresses its raw-mode and Enter requirements. Keep the current output-replay changes linked only to the issue that defines that scope.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
ELI5
A slow terminal restore could turn a split color sequence into visible text such as
214mWorking, or erase the color of ordinary output. Preserve the terminal's parser and styling when the snapshot and queued output form an intact stream.What Changed
Reuse the deadline's existing gap/repaint decision to preserve replay state only when a snapshot answered and the queued bytes can drain without a known gap. Other abandonment paths keep their existing parser, styling, and Kitty resets. No acknowledgement, protocol, timer, or retry-budget changes.
Extend the existing fit-deadline test and reliability gate with split-escape, styled-output, and genuine-gap controls. The tests compare full terminal buffers and actual cell color attributes before any later repaint.
Why
The quiet handoff still wrote CAN plus a rendition reset between the snapshot's
ESC[38;5;tail and the live214mWorkingcontinuation. Those bytes cancel a valid sequence. An intact handoff needs no gap reset; genuine byte loss still does.Linked Issue
Follow-up to merged #33474, addressing the escape-sequence finding from its adversarial review.
This does not claim to resolve #33608: that report measures kernel PTY
icanon echoand failed input. This change concerns output replay/parser state and does not alter kernel TTY flags.Visual Proof
Same real PTY fixture and main-owned snapshot, with pane fitting held unavailable for 1.8 seconds during reveal. The snapshot was verified to contain the incomplete escape tail. Screenshots are from a hidden macOS Electron renderer.
Testing
214mWorkingand lost foreground color. The genuine-gap control passed before and after.Workingin palette color 214. Real shell input/output still works after recovery.ORCA_BACKGROUND_LAUNCH=1 pnpm tc:web, changed-code quality checks, reliability-gate validation, andgit diff --check.Review
Independent adversarial Claude CLI review found no must-fix defects and recommends merging. It checked parser/write ordering, gap classification, Kitty and synchronized-output state, timer/generation/PTY guards, other abandonment callers, remote compatibility, and test branch coverage. It confirmed that an intact deadline drain now matches normal successful replay, which also writes no additional gap reset. The review was read-only; test and rendered evidence above were run separately.
Agent skill upstream boundary
Notes
terminal-output.scrollback-restore; intact snapshot/live bytes preserve escape parsing and rendition through a fit deadline, while genuine gaps still ground state.