Skip to content

fix(terminal): certify replay wedges only after a quiet window and re-kick recovery on reveal#8691

Merged
brennanb2025 merged 9 commits into
mainfrom
brennanb2025/terminal-reveal-self-heal
Jul 14, 2026
Merged

fix(terminal): certify replay wedges only after a quiet window and re-kick recovery on reveal#8691
brennanb2025 merged 9 commits into
mainfrom
brennanb2025/terminal-reveal-self-heal

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

A hidden Claude Code split pane on 1.4.138-rc.7 returned with a fossilized mid-Ink frame: transcript intact, but the input box, task list, and status line were erased. A split resize repaired it by delivering SIGWINCH and forcing the TUI to repaint.

Production breadcrumbs showed 1,483 terminal_replay_guard_wedged_release events and zero recoveries, including tight bursts on the affected pane. The pane later parsed successfully, so those wedge verdicts were false positives against a slow-but-alive xterm parser.

This PR makes replay wedge certification progress-aware, separates rejected writes from parse progress, stops output work once an xterm is certified dead, and makes pane recovery self-heal across reveal and cooldown races without restarting the live PTY.

Root cause

engageReplayGuard released wedged after a flat 2 × stallCheckMs, even when sibling write completions proved the parser was still draining a hidden-restore backlog. That opened the replay guard while replay bytes could still emit xterm auto-replies, leaked CPR/DA/ESC fragments into the agent's stdin, permanently certified a healthy xterm instance dead, and handed it to pane recovery.

Review then found seven adjacent correctness holes before merge:

  • Date.now() progress tracking missed a completion in the same millisecond;
  • discardTerminalOutput() reused the stall-watch settle function, so cleanup could impersonate parse progress;
  • a synchronous terminal.write() rejection invoked completion-shaped callbacks, recording progress that never occurred and leaving detached scheduler tails retryable;
  • a replacement xterm that wedged during the 15-second recovery cooldown could be declined permanently, while replay certification could leave already-queued scheduler entries and ACK credits behind;
  • an in-flight renderer serializer could outlive cold parking or remount and publish a fossil snapshot from the replaced xterm;
  • successful auxiliary FIFO probes were invisible to the shared parse-progress generation, while a synchronous parsed-output probe rejection was swallowed instead of certifying recovery;
  • a cooldown timer evaluated only the first current split request, so a failed remote/input liveness probe could silently abandon a probe-certified dead sibling renderer.

The final implementation uses monotonic generations for both parse progress and per-tab recovery ownership, keeps successful parse settlement distinct from cancellation/failure, and treats certified-dead xterms as terminal output sinks until remount.

Reliability contract

  • Invariant (terminal-output.replay-guard-safety): replay-generated terminal replies remain suppressed until the replay is known parsed or the parser has produced no real completion for a full certification window; cleanup and rejected writes are never evidence of parser liveness; certified-dead xterms receive no further queued/new output; recovery remains bounded and only the current xterm generation may retry/remount.
  • Failure source: 2026-07-13 fossil-pane incident; [Bug]: Terminal intermittently stops accepting text input #8104 symptom family; recovery lineage fix(terminal): self-heal panes whose renderer dies while the PTY stays alive #8630 and fix(terminal): let pane recovery reach input stuck behind a never-settling connect #8673; independent exact-head review findings on rejected writes, certified-dead output queues, and cooldown recovery.
  • Oracle: deterministic tests hold the guard while sibling completions advance, detect a completion in the same wall-clock millisecond, certify after a truly quiet window, prove rejected/discarded writes do not advance progress, credit every discarded delivery exactly once, reject stale-generation recovery, and retry a fresh replacement that wedges during cooldown. Rebuilt Electron tests prove restored pixels/output plus post-recovery input delivery.
  • Gate: terminal-output.scrollback-restore (experimental/partial) and terminal-performance.output-backpressure-budget (experimental/partial). The registered restore command is the focused pty-connection.test.ts suite; scheduler/health/replay suites provide the narrower output-pipeline oracle.
  • Diagnostics: existing terminal_replay_guard_wedged_release, terminal_replay_guard_lost_completion, and terminal_pane_recovery_remount breadcrumbs remain; terminal_pane_recovery_remount_unavailable covers the previously silent missing-tab outcome.

Changes

  1. Generation-based quiet-window certification (replay-guard.ts, terminal-write-pipeline-health.ts)

    • Scheduler parse completions and replay releases increment a WeakMap-scoped generation per xterm instance.
    • A replay guard captures the generation when its probe is queued and extends one quiet window whenever the generation advances.
    • A true wedge with no progress certifies on the original schedule.
    • No wall-clock comparison remains, so same-millisecond completions and clock adjustments cannot create false wedges.
  2. Rejected writes are not parse progress (pane-terminal-output-scheduler.ts, pane-terminal-foreground-render-settle.ts)

    • Successful parse callbacks record progress and settle the watch.
    • Cleanup only cancels the watch.
    • Synchronous write rejection follows a separate failure callback: it credits delivery, certifies recovery, and never runs onParsed or advances the progress generation.
    • Detached scheduler tails are abandoned after rejection instead of retried.
  3. Certified-dead output suppression and cleanup (pty-connection.ts, pane-terminal-output-scheduler.ts)

    • New PTY deliveries short-circuit before xterm and still credit their ACKs while recovery owns the dead renderer.
    • Certification immediately discards map-owned scheduler work.
    • Drain, explicit flush, parsed-output waits, renderer serialization, and the Ctrl+C Kitty reset all respect certification; dead renderers issue zero xterm writes and publish no fossil snapshot.
    • Serializer responses revalidate registration ownership after their parse wait, so cold parking or remount returns null instead of publishing an old xterm buffer.
    • Successful health/serializer/startup FIFO probes advance the shared parse-progress generation; synchronous probe rejection certifies the concrete xterm dead.
  4. Generation-owned cooldown recovery (terminal-pane-recovery.ts, pty-connection.ts)

    • Each connected xterm captures the current per-tab recovery epoch and includes it in every recovery reason.
    • Each binding also registers a concrete xterm lifecycle token; every dispose invalidates its requests and removes them from pending retries, canceling the timer when no requesting split remains.
    • A successful remount increments the generation.
    • Cooldown declines schedule a bounded retry only for a fresh current-generation replacement; requests/timers from the replaced xterm are ignored.
    • Generation and concrete lifecycle ownership are rechecked after asynchronous liveness probes and at timer fire, so cold parking, reconnect, or any ordinary remount cannot let an old request hit its successor.
    • Tab-wide retry coalescing retains one bounded request per split, so disposing one requester does not strand another dead split.
    • When cooldown expires, every current split request starts; a failed or hung liveness probe cannot block a certified-dead sibling, and the first successful tab remount invalidates the rest.
    • Existing five-minute/three-remount window caps and deduped retry behavior remain intact.
  5. Reveal re-kick and diagnostics (pty-connection.ts, terminal-pane-recovery.ts)

    • A restore requested on a certified-dead pipeline re-kicks recovery with reason restore-blocked once per xterm instance.
    • A failed remountTerminalTabForRecovery records terminal_pane_recovery_remount_unavailable without consuming recovery budget.

Provider and platform coverage

  • Local/daemon PTY: covered by renderer units and rebuilt Electron recovery/hidden-restore tests.
  • SSH/remote runtime: recovery generation is renderer/tab scoped and preserves the existing authoritative-liveness gate for remote input recovery; no local filesystem/process assumptions were added. Live SSH was not rerun.
  • WSL: provider/path/launch contracts are unaffected; no live WSL run.
  • Mobile/relay: subscription, snapshot, pagination, and provider ownership contracts are unchanged; delivery ACKs remain credited on every discard path. No mobile run.
  • macOS/Linux/Windows: implementation uses platform-neutral Map/WeakMap/WeakSet/timer state. Fresh Electron evidence is macOS; fresh post-push CI passed verify, Linux/macOS golden E2E, and Wayland input.

Verification

Final pushed head 467d01acc:

  • 584 focused tests across PTY connection, pane recovery, replay guard, scheduler, pipeline health, serializer registry, lifecycle, and write-callback guard: passed.
  • Recovery-generation tests prove:
    • old xterm requests do not retry after remount;
    • a fresh replacement wedged during cooldown retries at 15 seconds and remounts;
    • an awaited stale retry cannot remount after a newer recovery;
    • cold parking/reconnect/ordinary disposal cancels the stale owner without canceling a live sibling split retry;
    • a failed authoritative liveness retry cannot abandon a certified-dead sibling split;
    • serializers, parsed-output waits, and interrupt resets issue zero writes to certified-dead xterms;
    • an in-flight serializer replaced by cold parking/remount returns null;
    • successful auxiliary FIFO probes advance parse progress and synchronous probe rejection certifies recovery;
    • the existing recovery-window cap still heals when its window reopens.
  • Output-pipeline tests prove queued/background output and explicit flushes issue zero writes after replay certification, with every ACK credited exactly once; PTY integration proves certification discards its queue immediately.
  • Rebuilt Electron E2E: terminal-hidden-tui-visual-restore.spec.ts + terminal-wedged-write-pipeline-recovery.spec.ts: 6/6 passed.
    • Hidden full-screen restore, newer live output, rich synchronized TUI restore, and hidden side effects: 4/4.
    • Escaping-callback wedge recovery and disposed-xterm zombie recovery, including post-remount input: 2/2.
  • pnpm run typecheck: passed.
  • pnpm run build:electron-vite: passed.
  • Full pnpm run lint, switch exhaustiveness, reliability-gate manifest, max-lines ratchet, and localization checks: passed.
  • Changed-file formatting and git diff --check: passed.
  • Fresh GitHub PR checks: verify, Linux golden E2E, macOS golden E2E, and Wayland terminal input: 4/4 passed.

Performance budget

  • No polling, provider/session inventory, subprocess, scan, startup await, or background wake loop was added.
  • Parse progress remains one terminal-scoped WeakMap operation per parsed batch/replay release or rare auxiliary FIFO probe.
  • Serializer ownership validation adds one Map lookup per snapshot response; it is not on the PTY output hot path.
  • Recovery ownership adds one monotonic instance id and Set/Map registration per mounted xterm. Disposal removes it and cancels an ownerless timer; one tab-wide timer retains at most one request per active split; expiry fans out only across that bounded active-split set, and the first successful remount cancels the whole retry.
  • Certified-dead scheduler checks are constant-time WeakSet lookups and delete queued work, preventing renderer wakeups and retained ACK/backlog state.
  • Scheduler ceiling remains 117.6 MB/s foreground and 1.9 MB/s background (simulated time, instant parser), unchanged from the reviewed branch.

Residual gaps

  • The production load-dependent slow parse was not reproduced live; the oracle is the incident breadcrumb shape plus deterministic regression tests.
  • Live SSH, WSL, Windows, Linux, and mobile runs were not collected for this renderer-only follow-up; fresh post-push CI passed.
  • Profiling why a hidden replay can fall more than 20 seconds behind remains separate work.

Not in this PR

Refs #8630, #8673, and the #8104 symptom family.

…-kick recovery on reveal

A slow-but-alive xterm parse (hidden-restore backlogs) could be certified
"wedged" by the replay guard's flat probe deadline, opening the guard while
replay bytes were still parsing (auto-replies leak into the agent's stdin)
and handing healthy panes to recovery. The wedge deadline now extends while
parse progress is observed and certifies only after a fully quiet window.

Two self-heal holes in the same family: a restore requested on a
certified-dead pipeline silently returned false — if certification's own
recovery request was budget-declined, a revealed dead pane kept its stale
pre-death frame forever; it now re-kicks pane recovery once per xterm
instance. And remountTerminalTabForRecovery returning false was the one
unbreadcrumbed recovery outcome; it now leaves a trace.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Terminal write health now records parse progress and distinguishes successful parsing, cancellation, and write failure. Output scheduling propagates write failures while preserving acknowledgement handling and preventing discarded output from recording progress. Replay stall handling uses quiet-window checks before releasing a replay as wedged. Pane recovery adds a restore-blocked reason and unavailable-remount breadcrumbs, while hidden restoration can re-kick recovery once for a certified-dead pipeline.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description is detailed, but it does not follow the required template and omits Screenshots, Testing checklist, AI Review Report, Security Audit, and Notes sections. Rewrite the PR description to match the repository template, including all required sections and the requested testing, review, security, and notes details.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the main change: quiet-window replay certification and recovery re-kick on reveal.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

@brennanb2025
brennanb2025 merged commit 5b90c42 into main Jul 14, 2026
4 checks passed
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