Skip to content

fix(terminal): let a Shell pane's scroll-up reach tmux history - #494

Open
timkjr wants to merge 2 commits into
Ark0N:masterfrom
timkjr:fix/shell-scroll-history
Open

timkjr wants to merge 2 commits into
Ark0N:masterfrom
timkjr:fix/shell-scroll-history

Conversation

@timkjr

@timkjr timkjr commented Sep 26, 2026

Copy link
Copy Markdown

Summary

  • In a Shell pane, output that arrives in a burst (cat of a file longer than the screen) leaves the browser with about one screen of scrollback: tmux repaints the burst instead of scrolling it (the mechanism measured in Scrollback in terminal not working #205). tmux still holds every line. Other modes recover it by re-pulling ?full=1 when the wheel reaches the top; Shell declines that gesture outright (fix(terminal): bound shell history replay #331) to keep a multi-megabyte replay off xterm's main thread, leaving Load full history as the only path. That button renders only once a replay was truncated, so a Shell tab with under 1 MiB of output has no way to scroll back at all.
  • The Shell scroll gesture now pulls a bounded window, ?full=1&tail=${TERMINAL_TAIL_SIZE}: the same 1 MiB a tab switch already loads, but of the scrollback instead of the visible frame. The route already applies tail after the capture and marks the cut truncationReason: 'tail', so a longer history still gets the banner and its unbounded pull. No server change, and the fix(terminal): bound shell history replay #331 bound holds: the worst case is the payload a tab switch already carries.
  • A bounded window with no more rows than the browser already holds is not rewritten, so the viewport does not jump on every scroll that outlasts the cooldown at the top. It is not latched as useless, since the next burst can put more history in tmux.
  • Known gap, not in this PR: a split view's second pane (terminal-split.js) has no scroll-to-top pull at all, so the same symptom reproduces there.

Test plan

  • npm run typecheck, npm run lint, npm run format:check, npm run check:frontend-syntax
  • npm test (full suite, 8303 passed): new test/shell-scroll-history-pull.test.ts runs the real method extracted from app.js with the real row estimators (its two new-behaviour cases fail against master); the static guard in test/history-truncation-notice.test.ts now pins the bounded URL
  • Live against a beta instance: cat of a 160-line file in a 38-row Shell pane left 45 browser rows; one wheel-up pulled the window and the buffer held 164, and scrolling kept working through new output. Two Shell tabs switched back and forth twice: each switch drops to the bounded select replay (unchanged), and the first wheel-up restores all 164 rows.

🤖 Generated with Claude Code

A burst of output leaves a Shell pane with about one screen of browser
scrollback, because tmux repaints the burst instead of scrolling it,
while tmux itself keeps every line. Shell declined the scroll-to-top
re-pull other modes use, and the Load full history button renders only
once a replay was truncated, so a Shell tab under 1 MiB could not
scroll back at all.

The scroll gesture now pulls ?full=1&tail=TERMINAL_TAIL_SIZE, the same
bound a tab switch loads; the route's existing tail cut marks longer
histories 'tail', so the banner still offers the unbounded pull. A
window no longer than the browser's buffer is not rewritten.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ark0N

Ark0N commented Sep 26, 2026

Copy link
Copy Markdown
Owner

Thanks for this, @timkjr. It fixes a real gap: after a burst, a Shell pane kept about one screen of browser scrollback, and the scroll gesture could not reach the history tmux still held. Your change makes scroll-to-top in a Shell pull a bounded window of it (?full=1&tail=<1 MiB>) and leaves the unbounded pull behind Load full history. The direction is good, the write-up is clear, and the new test fails on master the way you said it does. There is one bug to fix before merge, plus a few small things.

1. A bounded window can mark the session "exhausted" and hide Load full history (src/web/public/app.js:6370)

The downgrade guard runs before your new bounded check. It was written for an unbounded capture, where "smaller than the browser" means tmux has nothing more to give. A bounded window is smaller than the browser whenever tmux holds more than 1 MiB and the browser already has more rows than the window. Two ways to get there: press Load full history and then scroll to the top of what it loaded, or keep a Shell tab open while it prints more than about 1 MiB. In both cases the guard runs _setHistoryTruncation(sessionId, { ...payload, exhausted: true }) on a payload with truncationReason: 'tail'. The banner then says "Showing all 1.0 MB of retained history. Earlier output is no longer kept for this session." and removes the Load full history button, even though tmux still holds the rest (fullSize in the same payload says so). I reproduced it with your test harness: bufferRows: 5000, a 300-line capture with truncated: true, truncationReason: 'tail', and the real computeHistoryTruncationNotice returns canLoadMore: false.

The ask:

  • Move the boundedShellPull && _estimateReplayRows(...) <= buffer.active.length block above the _replayWouldShrinkBuffer check, so a bounded window never reaches the exhausted/useless path. I tried that reorder locally and your four tests still pass.
  • Add a test for it in test/shell-scroll-history-pull.test.ts: a shell with far more browser rows than a tail-cut window. Assert that nothing is replayed, that _setHistoryTruncation never receives exhausted: true, and that the banner still offers Load full history.
  • While you are in that branch: once the check is above the guard, a skip right after a successful Load full history will relabel the banner as "Showing the most recent 1.0 MB", even though the terminal holds all of it. Nothing was written on that path, so leave an entry that came from an unbounded pull as it is.

2. Every Shell scroll-to-top now pays for a full tmux capture on the server (src/web/public/app.js:6346, minor)

tail is applied after the capture, so each bounded pull still runs a synchronous capture-pane -S -100000 (about 0.3 s at 100k lines, per the #331 measurements). That blocks the event loop for every client, and it happens every 4 s while the user wheels at the top, including when the client then skips the result. A tab switch captures only the visible frame, so "the worst case is the payload a tab switch already carries" holds for the client, not the server. The ask: when the skipped window came back truncationReason: 'tail', the gesture cannot reach anything older than what the browser shows, so put the session on the slow 60 s cooldown (_fullHistoryRepullUseless). Leave the untruncated skip on 4 s, where your "the next burst can add history" reasoning applies.

3. CLAUDE.md still says the opposite (CLAUDE.md:268, minor)

The Full-scrollback replay entry still reads "Shell loads the rest only via Load full history, never on ordinary scroll." You updated docs/architecture-invariants.md, so the summary line needs the same change, for example: "a Shell scroll-to-top pulls a bounded ?full=1&tail= window; the unbounded pull stays behind Load full history".

4. Nit (src/web/public/app.js:6390): with the reorder, the 1 MiB string goes through _estimateReplayRows twice (once in your check, once inside _replayWouldShrinkBuffer). Compute it once and reuse it.

Once 1 to 3 are in, this is ready to merge. The split-pane gap you called out is fine to leave for a follow-up.

A window cut at the tail size can be smaller than the browser's buffer
while tmux still holds more. The downgrade guard reads that as "tmux has
nothing more to give", which is true of an unbounded capture only, so a
bounded window reaching it marked the session exhausted and removed Load
full history from the banner.

The bounded skip now runs first, so such a window never reaches the
exhausted path, and it no longer writes banner state: relabelling it from
the bounded payload would call a terminal holding all of a Load full
history pull "the most recent 1 MiB".

A skipped window that came back truncated cannot reach anything older
than the browser shows, and every ask costs the server a synchronous
capture-pane of the whole history (tail is applied after the capture), so
it puts the session on the 60 s cooldown. An untruncated one keeps 4 s.

_replayWouldShrinkBuffer takes optional pre-estimated rows so a megabyte
capture is not scanned twice. CLAUDE.md's Full-scrollback replay entry no
longer says Shell never pulls on ordinary scroll.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@timkjr

timkjr commented Sep 26, 2026

Copy link
Copy Markdown
Author

Thanks for the careful read, and for reproducing it with the harness. All three are in f6aa502, plus the nit.

1. Bounded window marking the session exhausted. You're right: the guard read a tail-cut window smaller than the browser as "tmux has nothing more to give". The bounded skip now runs before the downgrade guard and no longer writes banner state, so a window cut at the tail size can't reach the exhausted path, and a skip right after Load full history leaves the banner as that load set it. The new tests use the real _setHistoryTruncation and the real computeHistoryTruncationNotice. Your scenario (5000 browser rows, a 300-row tail-cut window) asserts nothing is replayed, _setHistoryTruncation is never called, and canLoadMore stays true. A second case covers a skip right after Load full history. Both fail on the previous commit. The guard's own tests still pass: a one-frame capture from a repaint-mode pane and the shell Load full history button are still refused and marked exhausted.

2. Server capture cost. You're right that "the worst case is what a tab switch carries" holds for the client only, since tail is applied after the capture. A skipped window that came back truncated now puts the session on the 60 s cooldown; an untruncated skip keeps 4 s. One deviation: I keyed it on payload.truncated rather than truncationReason === 'tail', because a 'capped' window (the capture itself exceeded the byte ceiling, then was tail-cut) can't reach anything older than the browser holds either. One trade-off, noted in a comment: only a successful replay clears that latch, so a tab switch that shrinks the browser buffer below the window can leave scroll-to-top inert for up to a minute. Load full history bypasses the cooldown.

3. CLAUDE.md. The Full-scrollback replay entry now describes the bounded scroll pull. I also updated the matching paragraph in docs/architecture-invariants.md.

4. Nit. _replayWouldShrinkBuffer(capture, estimatedRows) takes the rows the caller already estimated, so the 1 MiB string is scanned once. Callers that pass nothing behave as before.

Full suite is green (8309 passed), with typecheck, lint and format. I'll send the split-pane gap as a separate PR.

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.

2 participants