Skip to content

fix: preserve pre-TUI SSH shell output across alternate-screen tab restore (#6106)#6572

Closed
nwparker wants to merge 1 commit into
mainfrom
nwparker/fix-6106
Closed

fix: preserve pre-TUI SSH shell output across alternate-screen tab restore (#6106)#6572
nwparker wants to merge 1 commit into
mainfrom
nwparker/fix-6106

Conversation

@nwparker

@nwparker nwparker commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

🧑‍🤝‍🧑 Impact & ELI5

1) What's broken today (ELI5). If you use Orca over SSH (a remote machine), open a terminal, run some commands (like ls), and then start a full-screen tool like Codex or Claude in that same terminal — switching to another tab and back makes all your earlier command output vanish. The remote shell history you printed before launching the tool is silently thrown away; when you come back, you only see the current Codex/Claude screen and your prior ls results and prompts are gone. This hits anyone running a full-screen terminal app over SSH and switching tabs, which is a common workflow, so it's a real annoyance (lost context) rather than a crash.

2) What this PR does (ELI5). Think of the terminal as a notebook. When a full-screen app takes over, it flips to a fresh clean page (a "scratch page") to draw on, but your earlier notes are still in the notebook. The old code, when saving the terminal to restore it later, would tear out everything except that one scratch page. This PR teaches the desktop restore path to keep the earlier pages too — so when you switch back, your pre-tool command history is still there. It also re-attaches a small "mouse precision" setting that the save process used to drop, so clicking in big full-screen apps keeps working accurately. Oh, so it now keeps your earlier shell output when you restore a tab instead of wiping it the moment a full-screen app was running.

3) Tradeoffs / regressions — honest answer. No regressions for existing desktop or mobile behavior. The fix is a narrowly gated desktop opt-in: the new "keep the history" mode turns on for desktop tab restore, and now also for first desktop attach when the PTY is already inside an alternate-screen TUI. Ordinary desktop subscribe snapshots stay on the existing zero-row path unless a full-screen app is active, and mobile replay is still explicitly opted out because its xterm re-wrap path would duplicate prompts / flatten SGR. The remaining cost is inherent to the fix: desktop snapshots for active TUIs can carry the user's earlier shell history across SSH instead of only the current TUI frame. That transfer is bounded by the existing snapshot byte budget and row fallback, so it is not unbounded.


Summary

Fixes #6106.

When a desktop SSH terminal tab is hidden and then restored while an alternate-screen TUI (Codex/Claude/vim) is active, the restore snapshot dropped all the normal-buffer shell output that preceded the TUI — the user's pre-Codex ls output and prompts vanished and only the current TUI frame survived.

Two compounding daemon behaviors caused this on the hidden-restore path (SnapshotRequestserializeBudgetedRequestedSnapshotserializeTerminalBufferserializeHeadlessTerminalBufferHeadlessEmulator.getSnapshot):

  1. serializeHeadlessTerminalBuffer forced scrollbackRows=0 whenever the emulator was in alternate screen, so normal-buffer scrollback was never serialized.
  2. HeadlessEmulator.getSnapshot ran normalizeSnapshotAnsiForModes, which (for alt-screen) slices off everything before the last \x1b[?1049h, deliberately discarding the normal-buffer content SerializeAddon emits before the alt buffer.

Both behaviors are correct for the mobile replay path (mobile xterm re-wraps and would duplicate prompts / flatten SGR), but wrong for the desktop live SSH restore path.

Fix

Add a desktop-only opt-in (altScreenPreservesScrollback threaded through the RPC/runtime, preserveNormalBufferOnAltScreen at the emulator) that keeps the pre-TUI scrollback and returns the raw serializer output for desktop snapshots. The flag is used for desktop restore/requested snapshots, and for first desktop attach when the PTY is already in alternate screen. Mobile remains opted out, and ordinary desktop initial subscribe snapshots stay on the existing zero-row path unless an alternate-screen TUI is active.

Because the opt-in path returns the raw serializer.serialize() output, and SerializeAddon._serializeModes emits mouse tracking modes (?1000h/?1002h/?1003h) and bracketed paste but not SGR mouse encoding (?1006h/?1016h), the opt-in path now also re-appends the dropped encoding mode. Without this, a desktop TUI using SGR mouse encoding (common: Codex/Claude/OpenTUI) would lose ?1006h on restore and degrade to legacy X10 mouse reports (breaking clicks past col/row 223). The raw blob already enters alt screen via its embedded ?1049h, so only the encoding mode is appended — no duplicate alt-screen entry.

This supersedes community PR #6427 by @rodboev (its approach was adopted; credited via Co-authored-by). On top of that PR it adds: the SGR mouse-encoding restoration + regression test, a clearer serializeBudgetedRequestedSnapshot option-forwarding (no inverted default), and a WHY comment colocating the desktop-vs-mobile rationale. PR #6427 is left open.

Screenshots

No visual screenshot from the Electron app: the bug lives entirely in the daemon-side terminal serialization, and a faithful end-to-end repro needs a running SSH-backed runtime (sshd was not reachable on localhost in this environment and there was no packaged build). Instead, a Node-level reproduction exercising the exact HeadlessEmulator.getSnapshot path is attached as a PR comment, showing the before/after, alongside unit tests that assert the regression directly.

Testing

  • pnpm lint
  • pnpm typecheck
  • pnpm test (the three touched files: headless-emulator.test.ts, orca-runtime.test.ts, terminal-multiplex.test.ts)
  • pnpm build (not run — no source other than these modules changed; CI covers build)
  • Added/updated regression tests (see below)

Tests added:

  • headless-emulator.test.ts: opt-in preserves pre-alt-screen content; opt-in preserves SGR ?1006h encoding when active without duplicating ?1049h.
  • orca-runtime.test.ts: serializeTerminalBuffer drops pre-TUI output by default but preserves it with altScreenPreservesScrollback: true (asserts source: 'headless').
  • terminal-multiplex.test.ts: desktop SnapshotRequest forwards altScreenPreservesScrollback: true; mobile SnapshotRequest does NOT forward the flag; desktop multiplex subscribe and legacy binary subscribe preserve pre-TUI scrollback on first attach when alternate screen is active.

AI Review Report

Reviewed for correctness, edge cases, cross-platform behavior, and elegance.

  • Correctness: opt-in is gated on modes.alternateScreen, so normal-buffer behavior is untouched. Desktop requested snapshots and desktop first-attach snapshots use the opt-in only while an alternate-screen TUI is active; mobile and normal desktop initial subscribe keep the existing normalized/zero-row behavior. Verified no other serializeTerminalBuffer caller (serializeMainTerminalBuffer, serializeHiddenOutputRecoveryBuffer) sets the new flag, so their behavior is unchanged.
  • Cross-platform (macOS/Linux/Windows): no keyboard shortcuts, path separators, or shell-specific behavior introduced — only ANSI mode sequences and option threading. SSH/remote is the primary target of this fix (the bug is SSH-specific) and is handled via the runtime's headless emulator, not local-only assumptions.
  • Provider-agnostic: terminal serialization is unrelated to git providers; no GitHub-only assumptions.
  • Elegance: extracted a shared buildMouseEncodingSequence reused by both buildRehydrateSequences and the opt-in path (removes duplication, keeps the file under the max-lines limit without any disable). Replaced PR Preserve SSH pre-Codex scrollback on tab restore #6427's inverted-default altScreenPreservesScrollback === false ? ... : ... with a straightforward spread that forwards the caller's intent.

Security Audit

Treated the original community-PR author as potentially malicious and re-audited the adopted code.

  • No network calls, eval, child_process, filesystem writes, secret access, or new dependencies.
  • The change only threads optional booleans through existing serialization and emits well-known terminal mode sequences (?1006h/?1016h).
  • The opt-in returns MORE of the user's own terminal buffer (pre-TUI scrollback) only for desktop (non-mobile) restore — no exfiltration, injection, or path-traversal surface; the data already belongs to the user's session.
  • No IPC/auth surface touched. Clean.

Notes

  • Desktop first attach now preserves pre-TUI scrollback when the PTY is already in alternate screen, so the previous first-subscribe limitation is gone for the SSH desktop workflow.
  • Mobile behavior and normalizeSnapshotAnsiForModes default behavior are deliberately unchanged because the mobile replay path re-wraps snapshots and would duplicate prompts / flatten SGR if it used the desktop raw snapshot.

Made with Orca 🐋

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@nwparker, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5db597f9-5aaf-464b-9e20-e20a40574e30

📥 Commits

Reviewing files that changed from the base of the PR and between 8c49426 and 83a73f2.

📒 Files selected for processing (9)
  • src/main/daemon/headless-emulator.test.ts
  • src/main/daemon/headless-emulator.ts
  • src/main/runtime/orca-runtime.test.ts
  • src/main/runtime/orca-runtime.ts
  • src/main/runtime/rpc/methods/terminal.ts
  • src/main/runtime/rpc/streaming.test.ts
  • src/main/runtime/rpc/terminal-multiplex.test.ts
  • src/main/runtime/rpc/terminal-output-batching.test.ts
  • src/main/runtime/rpc/terminal-subscribe-buffer.test.ts

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

Copy link
Copy Markdown
Contributor Author

Evidence

Node-level reproduction of the exact bug path (before/after)

A faithful Electron + SSH-localhost repro was infeasible in this environment (sshd unreachable on localhost, no packaged build), so I reproduced the defect directly at the layer where it lives — HeadlessEmulator.getSnapshot, which is what serializeHeadlessTerminalBuffer calls on the hidden-restore path. The script writes pre-Codex shell output, enters alt screen with SGR mouse encoding (\x1b[?1049h\x1b[?1002;1006h), writes a TUI frame, then snapshots both ways:

=== BEFORE FIX (default snapshot path = what mobile/old desktop got) ===
  contains PRE_CODEX_START: false
  contains PRE_CODEX_END  : false
  contains ls output      : false
  contains Codex TUI frame : true
  preserves SGR ?1006h     : true
  alt-screen ?1049h count  : 1

=== AFTER FIX (desktop opt-in preserveNormalBufferOnAltScreen) ===
  contains PRE_CODEX_START: true
  contains PRE_CODEX_END  : true
  contains ls output      : true
  contains Codex TUI frame : true
  preserves SGR ?1006h     : true
  alt-screen ?1049h count  : 1

This is issue #6106 exactly: the default (pre-fix / mobile) path drops PRE_CODEX_START, PRE_CODEX_END, and the ls output, keeping only the live Codex frame. The desktop opt-in restores all pre-TUI shell output AND the live frame, preserves the SGR ?1006h encoding (the fix-item-1 mouse gap), and emits exactly one ?1049h (no duplicate alt-screen entry).

Unit tests (regression coverage)

$ ./node_modules/.bin/vitest run --config config/vitest.config.ts \
    src/main/daemon/headless-emulator.test.ts \
    src/main/runtime/rpc/terminal-multiplex.test.ts

 Test Files  2 passed (2)
      Tests  63 passed (63)
$ ./node_modules/.bin/vitest run --config config/vitest.config.ts \
    src/main/runtime/orca-runtime.test.ts

 Test Files  1 passed (1)
      Tests  487 passed (487)

New/updated assertions:

  • headless-emulator.test.ts: preserves pre-alternate-screen content with opt-in mode; preserves SGR mouse encoding in the opt-in alternate-screen snapshot.
  • orca-runtime.test.ts: preserves pre-alternate-screen shell output when explicitly requested for remote snapshots (default drops PRE_CODEX_START, opt-in keeps it, source: 'headless').
  • terminal-multiplex.test.ts: desktop SnapshotRequest forwards altScreenPreservesScrollback: true; new does not forward alternate-screen snapshot flag for mobile snapshot requests.

Lint / typecheck

  • pnpm run lint — passes (only a pre-existing unrelated warning in useGitStatusPolling.ts, not touched here). No max-lines disable was added; the file was kept under the limit by extracting buildMouseEncodingSequence.
  • pnpm run typecheck (full: node + cli + web) — clean.

Manual SSH/Electron note

The verification plan's /electron SSH-backed manual repro (hide/restore the Codex tab over ssh localhost, confirm scroll-up shows pre-Codex output and mouse still works) could not be executed here because Remote Login/sshd was disabled on the host and there was no built app. The Node repro above exercises the identical daemon serialization path and demonstrates the cursor/scrollback and ?1006h mouse-encoding outcomes that the manual steps would observe.

@nwparker
nwparker force-pushed the nwparker/fix-6106 branch from 7134f39 to f479019 Compare June 30, 2026 00:41
@nwparker

Copy link
Copy Markdown
Contributor Author

Bugbash update pushed as f479019.

What changed:

  • Rebased the PR cleanly onto current origin/main and resolved the emulator/runtime conflicts without dropping main's applied-size and alternate-screen metadata changes.
  • Removed the desktop first-attach tradeoff: desktop multiplex subscribe and legacy binary subscribe now use the same altScreenPreservesScrollback path when the PTY is already in alternate screen, so pre-TUI shell output is preserved on first attach as well as hidden-tab restore.
  • Tradeoff status: zero for the desktop SSH issue scope. Mobile remains deliberately unchanged because that replay path re-wraps snapshots and would duplicate prompts / flatten SGR if it used the desktop raw snapshot. No extra provider/API calls; the only added payload is the existing bounded snapshot data for active desktop TUIs.

Validation:

  • ./node_modules/.bin/vitest run --config config/vitest.config.ts src/main/daemon/headless-emulator.test.ts src/main/runtime/rpc/terminal-multiplex.test.ts src/main/runtime/orca-runtime.test.ts — 3 files, 578 tests passed.
  • pnpm run typecheck — passed.
  • pnpm run lint — passed; oxlint still prints the pre-existing unrelated useGitStatusPolling.ts exhaustive-deps warning.
  • ./node_modules/.bin/oxlint on the six touched files — passed.

…store

When a desktop SSH terminal tab is hidden and restored while an alternate-screen TUI (Codex) is active, the hidden-restore snapshot discarded all normal-buffer shell output that preceded the TUI. Two behaviors compounded: serializeHeadlessTerminalBuffer forced scrollbackRows=0 in alternate screen, and HeadlessEmulator.getSnapshot sliced off everything before the last ?1049h marker. Both are correct for mobile replay, but wrong for desktop live SSH snapshots.

Add a desktop-only opt-in (altScreenPreservesScrollback / preserveNormalBufferOnAltScreen) that keeps pre-TUI scrollback and returns the raw serializer output. Desktop requested snapshots and desktop first-subscribe snapshots for active alternate-screen terminals use this path, so first attach and tab restore both preserve the earlier shell output. Mobile replay remains unchanged, and ordinary desktop subscribe snapshots still use the existing zero-row path unless an alternate-screen TUI is active.

Because SerializeAddon emits mouse TRACKING modes but not SGR mouse ENCODING (?1006h/?1016h), the opt-in path also re-appends the dropped encoding so TUI clicks past col/row 223 keep using SGR instead of degrading to legacy X10.

Fixes #6106

Co-authored-by: Rod Boev <rod.boev@gmail.com>

Co-authored-by: Orca <help@stably.ai>
@nwparker
nwparker force-pushed the nwparker/fix-6106 branch from f479019 to 83a73f2 Compare June 30, 2026 00:58
@nwparker

Copy link
Copy Markdown
Contributor Author

Follow-up for failed hosted verify pushed as 83a73f2.

Root cause from run 28412304316: full-suite RPC tests used partial OrcaRuntimeService stubs that did not define isTerminalAlternateScreen. The production subscribe path now calls that method for desktop first-attach preservation, so those stubs threw before emitting subscribed. I added default non-alternate-screen behavior to the three affected RPC test stub helpers; production scrollback behavior is unchanged.

Local validation after the fix:

  • ./node_modules/.bin/vitest run --config config/vitest.config.ts src/main/runtime/rpc/streaming.test.ts src/main/runtime/rpc/terminal-output-batching.test.ts src/main/runtime/rpc/terminal-subscribe-buffer.test.ts src/main/runtime/rpc/terminal-multiplex.test.ts src/main/daemon/headless-emulator.test.ts src/main/runtime/orca-runtime.test.ts — 6 files, 598 tests passed.
  • pnpm run typecheck — passed.
  • targeted oxlint on touched runtime/RPC files — passed.

Watching the new hosted verify now.

@nwparker

Copy link
Copy Markdown
Contributor Author

Hosted verify is green on 83a73f2 (run 28412924365): lint, typecheck, full test suite, unpacked build, and CLI smoke all passed. The only annotation is the pre-existing unrelated useGitStatusPolling.ts exhaustive-deps warning.

@Jinwoo-H
Jinwoo-H self-requested a review June 30, 2026 02:29
@nwparker

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #7214: current main headless snapshots preserve normal scrollback separately from alternate-screen state and tests assert preserved shell history is restored before the TUI preamble. Porting this older raw-serializer path would bypass current model/view, mobile, and query-authority contracts.

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.

SSH terminal loses pre-TUI shell output after Codex tab restore

1 participant