Fix/panel scrollback leak 457

[ilijachrchev]

Description

Fixes #457 — new Claude Code tab receives another panel's mangled screen buffer as its first prompt.

Panel-scrollback readers sourced text from the raw PTY append log (terminal.scrollbackBuffer) and merely ANSI-stripped it. The append log accumulates every repaint frame — agents and tools update in place via cursor-motion escapes (\x1b[nD, \x1b[K, column addressing), not carriage returns. sanitizeTerminalOutput only collapses \r-based overwrites, so cursor-motion fragments concatenate into the reported garbage ("Workingorking•rking•king•ingngg"). The default 500-line copy preset ≈ the reported 9–12 KB.

The codebase already knew the raw log is unreliable — getTerminalState and agent-status detection both use the screen emulator instead. Three consumers shared the defective raw-log path:

1. runpane panels output → getPanelScrollback (main/src/ipc/runpane.ts) — the context-capture path; its output flows into --initial-input-file, prepended to the typed "continue"
2. terminal:getScrollbackClean (main/src/ipc/panels.ts) — the UI @-terminal raw-copy feature
3. terminal:save-scrollback (main/src/ipc/panels.ts) — the @-terminal embed-copy feature

The fix routes all three through a new getScrollbackText() method on the screen emulator, which renders clean plain text from the already-correct rendered buffer.

Changes:

• TerminalStateEmulator.getScrollbackText(maxLines?): clean plain text from the rendered buffer (scrollback history + viewport), trimming trailing blanks before applying the line limit
• TerminalPanelManager.getCleanTerminalScrollback(panelId, maxLines): awaits emulator idle, returns clean text or null (no live emulator → caller falls back to persisted state)
• Routed both panels.ts IPC handlers through the emulator, keeping the sanitized persisted-state fallback for lazy/inactive terminals
• Routed runpane panels output through the emulator (made panelScrollbackOutput async)
• Tests: emulator renders in-place cursor repaints as clean text, line-limit trimming, and a runpane integration test asserting the emulator source wins over the raw log

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Performance improvement
• Code refactoring

Checklist

• I have read the CONTRIBUTING.md guidelines
• My code follows the code style of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• I have run pnpm typecheck and pnpm lint locally
• I have tested the Electron app locally with pnpm electron-dev

Critical Areas Modified

• Session output handling (requires explicit permission)
• Timestamp handling
• State management/IPC events
• Diff viewer CSS

Screenshots (if applicable)

N/A — the bug produces no visible UI change; the symptom is in the agent's session JSONL (first user message contains another panel's screen dump). The fix changes what text the scrollback readers return internally.

Additional Notes

• pnpm run typecheck passes. OxLint exit 0, ESLint 0 errors in touched files.
• terminalStateEmulator.test.ts (9), runpane.test.ts (60), terminalPanelManager.test.ts — all pass.
• Two non-code failures (Knip's zod/mini resolution; daemonRegistryBindings minimatch CJS/ESM) reproduce identically on the base commit — pre-existing, not from this change.
• Manual end-to-end testing was not performed — the dev Electron app shows "Open Pane from the desktop app" on both this branch and plain main (v2.4.62), a pre-existing Windows dev environment issue unrelated to this @fix.
• One pre-existing typecheck error on the base commit (runpane.ts:475 PaneCommandValue) was fixed as part of the rebase cleanup (cast request as PaneCommandValue).

Automated QA

Status: passed at 33adc2d7. An isolated Electron dev app and local RunPane wrapper returned clean Working... and qa-sentinel text from both panels screen and panels output, with no repaint fragments. Typecheck, lint, and 116 focused tests pass. No screenshots were needed because this PR changes terminal text plumbing, not layout. Remaining human check: optional raw and embed @ terminal copy in a packaged app.

[parsakhaz]

Verdict: Request changes: preserve bounded scrollback when disposal races a read.
Counts: Must Fix: 1 (security: 0) · Should Fix: 0 · pass 1/1

Must Fix

• MF-1: getScrollbackText(maxLines) falls back to viewport-only finalScreenText after disposal and ignores maxLines · main/src/services/terminalStateEmulator.ts:121 · snapshot full rendered scrollback before disposal and apply the same limit on both live and disposed paths · violates the new method contract
  • Evidence: TerminalPanelManager.getCleanTerminalScrollback captures the emulator and then awaits waitForIdle() at main/src/services/terminalPanelManager.ts:2004-2010. dispose() resolves those idle waiters during terminal teardown, after storing only getScreenText(). The resumed reader therefore enters the disposed branch at line 122 and receives only the viewport, even when older history exists; a smaller requested limit is also ignored.
  • Failure scenario: a panel exits while pending writes drain and runpane panels output, raw @ copy, or embed-copy is reading it. The read resumes after disposal and silently returns only the last viewport instead of the requested recent scrollback, or returns more rows than requested when the terminal viewport is taller than the limit.

Praise

• The new emulator-backed path correctly avoids ANSI-stripped repaint fragments, and the limit + 1 integration in main/src/ipc/runpane.ts:1610-1614 preserves hasMore for live reads.

Checks

• Inspected all six changed files against PR #471 and issue #457 intent.
• Security review covered IPC input boundaries, output handling, filesystem effects, and terminal escape processing. No PR-introduced security defect found.
• Targeted main Vitest suite passed through the changed terminal emulator, RunPane IPC, and terminal panel manager tests.

[parsakhaz]

Re-review: MF-1 is fixed in 1d3ef4b with disposed-scrollback regression coverage. The final head also removes the remaining raw live-log fallback and validates line bounds. Typecheck, lint, 116 focused tests, and the isolated RunPane repaint probe pass.

[parsakhaz]

All three passes are complete and pushed at 33adc2d7.

REVIEW

• Rebased the PR onto current origin/main first.
• Found one Must-Fix lifecycle defect: a terminal disposed during an in-flight read returned viewport-only text and ignored the requested limit.
• Fixed it in 1d3ef4ba (fix(review): preserve scrollback across terminal disposal) and added disposed-scrollback regression coverage.
• Posted the initial request-changes review, then approved the corrected final head.
• Security review found no PR-introduced vulnerability.

SIMPLIFY

• Consolidated the duplicate live-emulator and persisted-state fallback used by both terminal copy handlers.
• Applied line bounding once in the RunPane output path and once in the emulator path.
• Reduced repeated implementation comments while retaining the detailed rationale at the emulator boundary.
• Commit: 9488825f (refactor(simplify): consolidate clean scrollback reads).

REFACTOR

• Removed getTerminalScrollback and the remaining raw live append-log fallback, leaving one authoritative live source: the rendered emulator.
• Added a finite, safe, non-negative integer boundary for maxLines.
• Removed duplicate raw-log tests and replaced them with persisted-fallback and invalid-bound proofs.
• Commit: 33adc2d7 (refactor(deep): enforce clean scrollback boundaries).

Verification

• Before simplify: pnpm typecheck passed; pnpm lint passed; full main Vitest suite passed, 627/627.
• After simplify and final refactor: pnpm typecheck passed; pnpm lint passed; focused terminal emulator, terminal panel manager, daemon registry, and RunPane IPC suite passed, 116/116.
• The later full main-suite rerun reached 626/627; only the unrelated network-backed skillCacheManager fallback test exceeded its fixed 5-second timeout. It also timed out in isolation after the network slowed. The touched-file suite remained fully green.
• Built and launched the Electron app with isolated Pane data, then drove the local RunPane wrapper against repaint marker qa-sentinel. Both panels screen and panels output returned clean Working... text with no overlapping fragments.
• No screenshot evidence was needed because there is no visual layout change.

Follow-ups

• Decide whether panels output should read normal-buffer history or the active alternate screen while a TUI is running. The current PR preserves active-buffer behavior.
• Decide whether copied terminal text should preserve physical xterm rows or reconstruct soft-wrapped logical lines. The current PR preserves visual-row behavior.

Left for parsa

• Optional packaged-app check of raw and embed @ terminal copy.
• Review CI and merge when satisfied. I did not merge.

Automated manual QA passed on isolated Pane data at 33adc2d7. Test pane 5c312480-49dd-473f-866b-5f179639ef19, panel 60fcc965-ceaf-4f8a-a871-9897af7121ca, marker qa-sentinel. The isolated Pane was archived and its temporary artifacts were cleaned up after the run.