Repository navigation
Conversation
…rk delay SSH PTYs are excluded from hidden-view parking because they have no daemon snapshot for re-hydration. Their xterm.js instances therefore remain in the renderer heap indefinitely while hidden, scrollback buffers intact. At the user's configured limit (1k–5k rows) a typical multi-tab SSH workflow grows the renderer heap to 100–250 MB; logs show a session reaching 257 MB in under 3 h with zero unhandled rejections — confirming the parking exclusion as the dominant residual leak (see issue #8652). This commit mirrors the cold-park hysteresis (30 s) with a matching scrollback-trim timer: * After a tab has been hidden for ≥30 s, all SSH panes have their xterm.js scrollback trimmed to SSH_HIDDEN_SCROLLBACK_TRIM_ROWS (200). xterm.js applies the trim synchronously, freeing the cell data immediately. * When the tab is revealed the buffer is restored to the user's configured limit so agent output can accumulate normally again. * A quick tab flip (hide→show in <30 s) never trims. The logic lives in a new `useHiddenSshScrollbackTrim` hook (pure- function helpers + timer effect) wired into `useTerminalPaneLifecycle` alongside the existing scrollback-rows application path. The unit tests cover trim, restore, timer cancellation on re-reveal, re-trim after restore, and unmount cleanup. Closes #8652
📝 WalkthroughWalkthroughAdds delayed scrollback trimming for hidden SSH terminal panes, restoring configured scrollback when panes become visible. The new hook is integrated into the terminal pane lifecycle with visibility, pane manager, transport, and scrollback configuration inputs. Tests cover SSH-only identification, trimming and restoration guards, delayed timers, cancellation, re-trimming, visibility changes, and unmount cleanup. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f12787b7-1362-4aea-9e2c-026587feb40e
📒 Files selected for processing (3)
src/renderer/src/components/terminal-pane/terminal-ssh-scrollback-trim.test.tssrc/renderer/src/components/terminal-pane/terminal-ssh-scrollback-trim.tssrc/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts
| export function trimHiddenSshScrollback( | ||
| manager: TrimPaneManager, | ||
| paneTransports: Map<number, PtyIdProvider>, | ||
| trimRows: number | ||
| ): void { | ||
| for (const pane of manager.getPanes()) { | ||
| const ptyId = paneTransports.get(pane.id)?.getPtyId() ?? null | ||
| if (ptyId !== null && parseAppSshPtyId(ptyId) !== null) { | ||
| pane.terminal.options.scrollback = trimRows | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export function restoreHiddenSshScrollback( | ||
| manager: TrimPaneManager, | ||
| paneTransports: Map<number, PtyIdProvider>, | ||
| configuredRows: number | ||
| ): void { | ||
| for (const pane of manager.getPanes()) { | ||
| const ptyId = paneTransports.get(pane.id)?.getPtyId() ?? null | ||
| if (ptyId !== null && parseAppSshPtyId(ptyId) !== null) { | ||
| // Only bump up — never reduce a scrollback that is already at or above | ||
| // the configured value (e.g. the pane was never trimmed). | ||
| if ((pane.terminal.options.scrollback ?? configuredRows) < configuredRows) { | ||
| pane.terminal.options.scrollback = configuredRows | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Trim can raise scrollback above the configured limit and get stuck there.
trimHiddenSshScrollback unconditionally sets scrollback = trimRows with no check against the pane's current value, while restoreHiddenSshScrollback only ever bumps scrollback up to configuredRows, never down. If a user's configured scrollback is below SSH_HIDDEN_SCROLLBACK_TRIM_ROWS (200), hiding the tab actually increases memory usage, and revealing it afterward never restores the smaller configured value — the pane is stuck at 200 for the rest of the session since restore's guard only fires when current < configuredRows.
Give trimHiddenSshScrollback the same directional guard as restore, so trim is a strict no-op when the pane is already at/below the target:
🐛 Proposed fix
export function trimHiddenSshScrollback(
manager: TrimPaneManager,
paneTransports: Map<number, PtyIdProvider>,
trimRows: number
): void {
for (const pane of manager.getPanes()) {
const ptyId = paneTransports.get(pane.id)?.getPtyId() ?? null
if (ptyId !== null && parseAppSshPtyId(ptyId) !== null) {
- pane.terminal.options.scrollback = trimRows
+ // Only reduce — a pane whose configured limit is already below
+ // trimRows must never be pushed up by "trimming".
+ if ((pane.terminal.options.scrollback ?? trimRows) > trimRows) {
+ pane.terminal.options.scrollback = trimRows
+ }
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function trimHiddenSshScrollback( | |
| manager: TrimPaneManager, | |
| paneTransports: Map<number, PtyIdProvider>, | |
| trimRows: number | |
| ): void { | |
| for (const pane of manager.getPanes()) { | |
| const ptyId = paneTransports.get(pane.id)?.getPtyId() ?? null | |
| if (ptyId !== null && parseAppSshPtyId(ptyId) !== null) { | |
| pane.terminal.options.scrollback = trimRows | |
| } | |
| } | |
| } | |
| export function restoreHiddenSshScrollback( | |
| manager: TrimPaneManager, | |
| paneTransports: Map<number, PtyIdProvider>, | |
| configuredRows: number | |
| ): void { | |
| for (const pane of manager.getPanes()) { | |
| const ptyId = paneTransports.get(pane.id)?.getPtyId() ?? null | |
| if (ptyId !== null && parseAppSshPtyId(ptyId) !== null) { | |
| // Only bump up — never reduce a scrollback that is already at or above | |
| // the configured value (e.g. the pane was never trimmed). | |
| if ((pane.terminal.options.scrollback ?? configuredRows) < configuredRows) { | |
| pane.terminal.options.scrollback = configuredRows | |
| } | |
| } | |
| } | |
| } | |
| export function trimHiddenSshScrollback( | |
| manager: TrimPaneManager, | |
| paneTransports: Map<number, PtyIdProvider>, | |
| trimRows: number | |
| ): void { | |
| for (const pane of manager.getPanes()) { | |
| const ptyId = paneTransports.get(pane.id)?.getPtyId() ?? null | |
| if (ptyId !== null && parseAppSshPtyId(ptyId) !== null) { | |
| // Only reduce — a pane whose configured limit is already below | |
| // trimRows must never be pushed up by "trimming". | |
| if ((pane.terminal.options.scrollback ?? trimRows) > trimRows) { | |
| pane.terminal.options.scrollback = trimRows | |
| } | |
| } | |
| } | |
| } | |
| export function restoreHiddenSshScrollback( | |
| manager: TrimPaneManager, | |
| paneTransports: Map<number, PtyIdProvider>, | |
| configuredRows: number | |
| ): void { | |
| for (const pane of manager.getPanes()) { | |
| const ptyId = paneTransports.get(pane.id)?.getPtyId() ?? null | |
| if (ptyId !== null && parseAppSshPtyId(ptyId) !== null) { | |
| // Only bump up — never reduce a scrollback that is already at or above | |
| // the configured value (e.g. the pane was never trimmed). | |
| if ((pane.terminal.options.scrollback ?? configuredRows) < configuredRows) { | |
| pane.terminal.options.scrollback = configuredRows | |
| } | |
| } | |
| } | |
| } |
| useHiddenSshScrollbackTrim({ | ||
| managerRef, | ||
| paneTransportsRef, | ||
| isVisible, | ||
| configuredScrollbackRows: terminalScrollbackRows | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate applyTerminalScrollbackRowsToMountedPanes to check whether it is
# SSH-aware or unconditionally overwrites every mounted pane's scrollback.
rg -n "applyTerminalScrollbackRowsToMountedPanes" -A 15 src/renderer/srcRepository: stablyai/orca
Length of output: 9378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure, then inspect the hidden-scrollback hook and the nearby effects.
ast-grep outline src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts --view expanded
printf '\n--- hook implementation ---\n'
sed -n '260,420p' src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts
printf '\n--- effect region ---\n'
sed -n '1920,1975p' src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.tsRepository: stablyai/orca
Length of output: 9429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the tests around hidden SSH scrollback to see whether this interaction is already covered.
sed -n '1,260p' src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.tsRepository: stablyai/orca
Length of output: 8098
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for the hidden SSH scrollback hook and any trim/reschedule logic.
rg -n "useHiddenSshScrollbackTrim|scrollback.*trim|cold-park|trim" src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.tsRepository: stablyai/orca
Length of output: 445
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/renderer/src/components/terminal-pane/terminal-ssh-scrollback-trim.ts --view expanded
printf '\n--- implementation ---\n'
cat -n src/renderer/src/components/terminal-pane/terminal-ssh-scrollback-trim.tsRepository: stablyai/orca
Length of output: 6036
Keep hidden SSH panes out of the global scrollback update applyTerminalScrollbackRowsToMountedPanes still overwrites every mounted pane, so a hidden SSH pane trimmed to 200 gets restored to the full configured depth on a scrollback-setting change. useHiddenSshScrollbackTrim only reschedules its timeout in the hidden branch, so the pane stays untrimmed until that delay elapses again.
|
Field data update — v1.4.144 through v1.4.161, Jul 23–30 Heap growth continues; Jul 29 hit the highest peak we've recorded. Daily heap stats (renderer_memory interval samples)
Jul 29 607 MB peak is the highest we've seen. Jul 25–26 are low because there were few active sessions. Session load contextFrom Jul 29
The SSH remote worktrees drive PTY scrollback buffers that are the primary leak source identified in this PR. With that many concurrent SSH panes, growth rates of 25–30 MB/hr align with the leak model. Comparison to pre-fix baselineOur earlier data (before PR #8279 merged): ~62 MB/hr with only macOS local sessions. The fix in this PR should bring that down significantly. Would appreciate a review when there's bandwidth. |
|
Status update: relationship to PR #10625 (merged in v1.4.162) PR #10625 shipped the official fix for Issue #8652 and is now in v1.4.162. Its approach is a fleet-level retention budget (slice B): hidden un-parkable worktrees — including remote-runtime and SSH-without-parking — are force-parked once the count exceeds 12 or the TTL hits 45 min. The PR explicitly notes that slice C (scrollback demotion/trim) was intentionally removed in favour of the worktree-count ceiling. This PR (#8655) is complementary, not redundant:
With slice B in place, the number of mounted remote/SSH worktrees is now bounded. But the panes inside those worktrees — including local panes inside a remote worktree — are not scrollback-trimmed. On high-load days (Jul 27, Jul 29) the growth rate on our setup was 25–30 MB/hr with 7–9 live panes and 350 sidebar activations over the week; trimming the scrollback of hidden SSH panes is the next lever available. If there's appetite for a narrower change than the full eviction approach in #7448, this PR is that change. Happy to rebase on main and address the CodeRabbit comments if it's useful. |
|
Closing as superseded — the linked issue(s) for this PR were fixed by another change that has already been merged.
Superseding merge: #10625 Thank you for the contribution! If anything remains unaddressed, please open a new issue or PR against current |
Problem
SSH PTYs are excluded from hidden-view parking (
canParkTerminalTabRenderer()returnsfalsefor anyptyIdthat matchesparseAppSshPtyId()) because parking relies on a daemon snapshot for re-hydration and SSH PTYs have no such snapshot in the current phase.As a result their xterm.js instances — and the full scrollback buffers they hold — remain in the renderer heap for as long as the tab stays hidden. At the default 5 000-row limit each SSH terminal occupies roughly 20–25 MB; a workflow with five or more SSH tabs can push the renderer heap past 100–250 MB without any unhandled rejections or other visible errors.
Log analysis from a production session (issue #8652) shows the renderer heap growing from 23 MB to 257 MB over 3 h in a session with zero unhandled rejections and
terminalHiddenViewParkingenabled — confirming the parking exclusion as the dominant residual leak.Solution
Mirror the existing cold-park hysteresis (30 s,
TERMINAL_TAB_COLD_PARK_DELAY_MS) with a matching scrollback-trim timer:SSH_HIDDEN_SCROLLBACK_TRIM_ROWS = 200rows. xterm.js applies this synchronously by trimming the circular buffer in place, freeing cell data immediately without discarding the process or terminal instance.200 rows is intentionally generous — enough to cover a full viewport plus a short scroll — while releasing > 95 % of the accumulated history that agent runs fill.
Files changed
terminal-ssh-scrollback-trim.tstrimHiddenSshScrollback,restoreHiddenSshScrollback(pure helpers, testable without React),useHiddenSshScrollbackTrimhookterminal-ssh-scrollback-trim.test.tsuse-terminal-pane-lifecycle.tsExpected outcome
In a session with 5 × SSH terminals each at 1 000-row scrollback the trimmed heap footprint per hidden terminal drops from ~5 MB to ~1 MB, reducing the total contribution of hidden terminals from ~25 MB to ~5 MB. At the previous 5 000-row default the saving is proportionally larger (~100 MB → ~5 MB for 5 terminals).
Related
Test plan
ELI5
Hidden SSH terminals kept huge scrollback in memory forever. After a delay, Orca trims that scrollback so idle SSH tabs use much less RAM.