Skip to content

fix(terminal): preserve answered snapshots across fit deadlines - #33474

Merged
nwparker merged 1 commit into
mainfrom
nwparker/police-car-term-regression
Oct 25, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/police-car-term-regression

Conversation

@nwparker

@nwparker nwparker commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​108 $\color{#cf222e}{\Huge{\mathbf{−}}}$​48 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​60
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​28 $\color{#cf222e}{\Huge{\mathbf{−}}}$​8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​20

ELI5

Returning to a terminal could insert a false recovery warning into a running TUI, leaving duplicated “Working” rows. A snapshot that already arrived now resumes output quietly when pane sizing takes too long.

What Changed

  • Recognize an answered snapshot at the 750 ms foreground deadline regardless of pending-output size.
  • Drain intact queued output using the existing snapshot sequence reconciliation. Request a later repaint only for overflow, a known uncovered drop, or an unmappable chunk, avoiding repeated snapshots during slow sizing.
  • Extend the existing test and terminal-output.scrollback-restore gate with six cases, exact headless-buffer comparisons, and request counts during continued output.

Why

The old deadline recognized an answered snapshot only after the live queue overflowed. Small queues instead received the unavailable-recovery banner, shifting the cursor position underneath cursor-relative TUI redraws. The existing baseline/drain and quiet-abandon paths already supply the recovery behavior needed here.

Source history traces the awaited-fit path to #8674, committed July 16, 2026 UTC, first stable release v1.4.144 on July 17. The affected file is identical in v1.4.239 and v1.4.240. This dates the vulnerable code from source history; an old-release runtime bisect was not performed.

Linked Issue

Slack report

Visual Proof

Same finite cursor-redraw fixture and forced 1.8-second sizing delay, using a real PTY and main-owned snapshot in a hidden macOS Electron app. The old code leaves stale FRAME_08 beside final FRAME_23; the fix leaves one final row. The exact warning was captured in terminal writes and overwritten by subsequent redraws.

Before After
Before: persistent duplicated Working rows After: one final Working row

Continuous 20 Hz redraw with sizing unavailable for 12 seconds, observed for 19 seconds: one target snapshot request, advancing output, no warning or duplicated rows. A real shell command still worked after recovery and another hide/reveal.

Continuous redraw after prolonged sizing delay

Testing

  • Manually exercised the rendered behavior through an independent adversarial visual agent, including old-code failure, no-delay control, final fix, prolonged sizing failure, and shell input after recovery.
  • Automated tests updated: 212 tests across six terminal replay suites passed, including exact deadline/final buffers, partial sequence overlap, unmappable chunks, uncovered unhide/fresh-snapshot markers, overflow, and continued output without repeated fetching.
  • Passed ORCA_BACKGROUND_LAUNCH=1 pnpm tc:web, pnpm run check:code-quality:changed, pnpm run check:reliability-gates, and git diff --check.

Review

Self-review and independent Claude CLI review found no remaining blocking defects. Review feedback eliminated repeated repaints for intact queues while preserving repaint recovery for real gaps. Checked sequence ordering, stale generation/PTY guards, fit continuation, timer cleanup, true recovery failures, and remote behavior.

A fresh adversarial Claude CLI review of the exact published commit 39486521ccd14896747393cf06ce4743d8d27add independently traced all three changed files and surrounding code. Verdict: no new must-fix defect; recommends merging. It challenged snapshot paint ordering, generation/PTY identity, fit cancellation and skipped alt frames, sequence/raw-length overlap, true fetch failures, mixed-version remote behavior, and bounded work. This review was read-only and did not run tests.

Nonblocking inherited behaviors remain: the existing quiet-abandon gap reset can cancel a partial escape sequence or reset styling until the TUI redraws; the quiet path does not ACK the painted snapshot, so a later reveal may fetch again until a restore completes normally. These were not expanded into this fix.

Agent skill upstream boundary

  • Not applicable: no upstream skill-installer content is copied or translated.

Notes

  • Invariant/gate: terminal-output.scrollback-restore; an answered snapshot must not become an unavailable-recovery banner solely because pane fitting is delayed. Exact buffers and bounded request counts are the oracle.
  • Coverage: local/daemon PTY rendering on macOS; deterministic remote/SSH recovery suites. No protocol or workspace-path changes. Native Linux, Windows/WSL, mobile, and a live remote host were not exercised.
  • Performance/diagnostics: reuse existing recovery timers and byte caps; no added polling or subprocesses. The additional scan is confined to the bounded pending queue at the deadline. Screenshots, terminal-write capture, and registered tests provide failure evidence.
  • Limits: this proves a controlled delayed-sizing mechanism, not Vincent's exact trigger. Actual Pi execution and Activity disappearance remain unverified. Existing overflow recovery behavior is retained; an unmappable chunk can remain temporarily duplicated until its repaint, and the quiet path can fetch again on a later reveal because the existing drop latch remains armed.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d566cfe1-5902-494b-88de-da12029c3324
Commits

Reviewing files that changed from the base of the PR and between 1e221ce and 3948652.

Files selected for processing: 3
  • config/reliability-gates.jsonc
  • src/renderer/src/components/terminal-pane/pty-connection-hidden-restore-fit-overflow.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-drain.ts
  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker
nwparker merged commit 346e2ac into main Oct 25, 2026
44 checks passed
@nwparker
nwparker deleted the nwparker/police-car-term-regression branch October 25, 2026 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant