Skip to content

fix(terminal): keep row alignment in the full-history pane replay - #395

Merged
Ark0N merged 2 commits into
Ark0N:masterfrom
irisitymichaelgrundberg:fix/full-history-replay-row-alignment
Sep 10, 2026
Merged

Ark0N merged 2 commits into
Ark0N:masterfrom
irisitymichaelgrundberg:fix/full-history-replay-row-alignment

Conversation

@irisitymichaelgrundberg

@irisitymichaelgrundberg irisitymichaelgrundberg commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Part of #398, which describes the symptom and the other causes found with it.

Fixes the caret landing on the composer's border instead of its input line
after switching to a session, and the frame shifting a moment after it paints.
Any output from the CLI repaired it, because the CLI then repainted everything.

Two defects in the full-history replay, and they compound.

The capture never restored the cursor. The visible-frame path ends with an
absolute cursor move back to the pane's position. The linear path returned its
text and left the caret wherever the last character landed, which for an agent
CLI is the bottom-most row carrying text — the status line. Every
cursor-relative update the CLI sent afterwards was then measured from the wrong
row.

Its rows did not line up with the pane's rows. Four transforms ran over the
capture and each can delete a line: trailing blank rows were stripped,
redraw-bloat stripping ran, the trim that cuts everything above the Claude
banner ran, and leading whitespace was removed. All four are right for a byte
stream of successive frames. A capture is the rendered pane, one line per
screen row, so each deletion shifted the frame out from under the restored
cursor.

The full-history path now appends the pane's own cursor position and keeps
every row, so row N of the reply is row N of the pane. The visible-frame and
tail paths are untouched
— they still strip, still trim, and still repaint
absolutely, because for them none of that moves a row.

The guard, corrected

The first revision of this PR keyed the three skips on ?full=1, which is only
what the client ASKED FOR. When the capture comes back null — ENOBUFS, a
timeout, a vanished pane, or a session with no mux at all — the reply falls back
to session.terminalBuffer, which IS a byte stream of successive frames and
still needs stripping. Gating on the request returned it whole: 82,028 bytes
against 4,128 for the same buffer without full=1, and a direct-PTY session
takes that path on every first selection rather than only during an outage. The
skips now key on a capture having actually arrived.

The same revision left three more defects on this path, all fixed here:

The cursor move is relative, not CUP. Absolute row addressing is correct
only while the browser's row count equals the pane's, and resizeWindow issues
its tmux resize without waiting, so a capture can be taken before a requested
resize applies. Measured against real tmux with a browser four rows shorter than
the pane: the absolute move lands on a blank row, the relative one lands on the
caret's row.

Trailing rows and the cursor move now travel together. Keeping the rows is
only sound because a move follows to count back up from them; on the two
branches where the cursor query fails there is no move, so the caret was left at
the bottom of the pane — worse than before. The cursor is read first and settles
both decisions.

An all-blank pane returns '' again. Retaining trailing rows and appending a
move made it non-empty, and the caller reads non-empty as "replay this", so a
blank screen would have replaced real history — the downgrade
_replayWouldShrinkBuffer refuses, arriving from the server side where that
guard cannot see it.

Documentation

Restoring the cursor is what makes row alignment load-bearing on this path,
and neither CLAUDE.md nor docs/architecture-invariants.md said so — which
is how four line-deleting transforms accumulated there. Both now carry the rule,
in the "Full-scrollback replay" entry each already has.

One claim in the first revision of those entries was wrong and is corrected:
"one line per screen row" holds only where no row was hard-wrapped, since -J
joins a wrapped row into its logical line. Measured: a 100-character line in a
40-column pane captures as 10 lines against a 12-row pane. The counts reconcile
only once the browser xterm re-wraps at the same width, which is the assumption
_estimateReplayRows already documents. The stale "NOT repositioned" contract
in the mux interface is updated too.

Verification

Against a live 315×59 pane: the reply carries 59 rows, its row 55 is the
composer's input line matching tmux capture-pane, and it ends with
\x1b[56;3H, which lands there. Before, row 55 held the border.

Route tests cover a surviving blank first row, an unstripped byte-history
fallback, and an empty capture leaving history intact. The extracted helpers
(formatCursorRestore, hasVisibleContent) are unit-tested directly rather
than through source-text matching, and the slice window in the capture test is
bounded at the next method, having overrun into its neighbours. Every assertion
was checked by reverting the fix it covers.

npm test is green (6770 passing), along with typecheck, lint, format:check and
check:frontend-syntax.

Notes

One of three independent PRs for the same report; the others cover a fit taken
before the terminal font has loaded, and two windows sizing one pane. This one
stands alone.

Switching to a session left the caret one row below the composer's input
line, on the box border, and every cursor-relative update the CLI sent
afterwards was measured from the wrong row. Any fresh output repaired it,
because the CLI then repainted the whole frame.

Two things were wrong with the full-history replay, and they compound.

The capture never restored the cursor. The visible-frame path ends with an
absolute cursor move back to the pane's position; the linear path returned
its text and left the caret wherever the last character landed, which for an
agent CLI is the bottom-most row carrying text — the status line.

The rows it addressed did not line up with the pane's rows either. Four
transforms ran over the capture and each can delete a line: the trailing
blank rows were stripped, redraw-bloat stripping ran, the trim that cuts
everything above the Claude banner ran, and leading whitespace was removed.
All four are right for a byte stream of successive frames. A capture is the
rendered pane, one line per screen row, so each deletion shifted the frame
out from under the restored cursor.

The full-history path now appends the pane's own cursor position and keeps
every row, so row N of the reply is row N of the pane. The visible-frame and
tail paths are untouched.

Restoring the cursor is what makes row alignment load-bearing here, and
neither CLAUDE.md nor the architecture invariants said so — which is how
four line-deleting transforms accumulated on the path. Both now record it.

Verified against a live 315x59 pane: the reply carries 59 rows, its row 55
is the composer's input line matching tmux, and it ends with the cursor move
that lands there.
@irisitymichaelgrundberg
irisitymichaelgrundberg force-pushed the fix/full-history-replay-row-alignment branch from 47380a3 to 323730a Compare September 9, 2026 12:20
…ery flag

Review of the previous commit found the guard inverted: the three skips keyed
on `?full=1`, which is only what the client asked for. When the capture comes
back null — ENOBUFS, a timeout, a vanished pane, or a session with no mux at
all — the reply falls back to the byte history, which IS a stream of
successive frames and still needs stripping. Gating on the request returned it
whole: measured at 82KB against 4KB for the same buffer without `full=1`. A
direct-PTY session takes that path on every first selection, not only during
an outage. The skips now key on `isFullCapture`, meaning a capture arrived.

Three further defects the same review surfaced, all on this path:

Keeping the trailing rows is only sound when a cursor move follows to count
back up from them. On the two branches where the cursor query fails there is
no move, so the caret was left at the bottom of the pane — worse than before.
The cursor is now read first and settles both decisions together.

The move is relative rather than absolute. `CUP` numbers rows from the top of
the browser's screen, so it is only right while the browser's row count equals
the pane's, and `resizeWindow` does not wait for tmux, so a capture can be
taken before a requested resize applies. Measured against real tmux with a
browser four rows shorter than the pane: the absolute move lands on a blank
row, the relative one lands on the caret's row.

An all-blank pane no longer reads as content. Retaining trailing rows and
appending a move made it non-empty, and the caller treats non-empty as "replay
this", so a blank screen would have replaced real history — the downgrade
`_replayWouldShrinkBuffer` refuses, arriving from the server side where that
guard cannot see it.

The documentation claimed one line per screen row. `-J` joins a hard-wrapped
row into its logical line, so that is false whenever any row wrapped: measured
at 10 lines for a 12-row pane. Both entries now say what actually holds, and
the stale "NOT repositioned" contract in the mux interface is updated too.

Tests: the byte-history fallback is stripped, an empty capture leaves history
intact, and the extracted helpers are unit-tested directly rather than through
source-text matching. The slice window in the capture test is bounded at the
next method, having overrun into its neighbours.
@irisitymichaelgrundberg
irisitymichaelgrundberg force-pushed the fix/full-history-replay-row-alignment branch from 1a7a556 to 2b57c59 Compare September 9, 2026 13:35
@irisitymichaelgrundberg
irisitymichaelgrundberg marked this pull request as ready for review September 9, 2026 15:11
@Ark0N
Ark0N merged commit 890a1b0 into Ark0N:master Sep 10, 2026
2 checks passed
@Ark0N

Ark0N commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Merged and shipped in v1.26.2, together with #396 and #397. Thank you.

The part I want to call out is that you kept the visible-frame and tail paths untouched. Those four transforms are correct for a byte stream of successive frames and wrong for a single rendered frame, and the tempting fix was to "fix" the stripping globally, which would have broken the paths that depend on it. Scoping the row-preserving behaviour to the capture, and gating it on a capture rather than on the query flag in the follow-up commit, is the version that does not create a second bug somewhere else.

Restoring the cursor from the pane's own position rather than computing where it ought to be is the other thing that makes this durable.

irisitymichaelgrundberg added a commit to irisitymichaelgrundberg/Codeman that referenced this pull request Sep 15, 2026
Live terminal events are queued while a buffer load runs, and the load discards
that queue when it ends. That is right when the loaded buffer is the server's
accumulated byte history. The history is current up to the response, so the
queued events already appear in it and replaying them would duplicate output,
most visibly Ink's cursor-up redraws.

A tmux pane capture is current only up to CAPTURE time, which is part-way
through the fetch. Everything arriving between the capture and the end of the
chunked write was dropped, and nothing scheduled a re-fetch to recover it:
`_onSessionNeedsRefresh` is wired only to the 128KB overflow path. The CLI's
next partial redraw then landed on a frame the terminal never received. The
window covers the whole load, not a sliver of it — a 400ms redraw settle after a
real resize, the fetch, and the chunked write after that.

Queue entries now carry their arrival time, and `_finishBufferLoad` takes a
`since` cutoff, so a capture load replays exactly the tail that arrived after
the response headers. The pre-capture events stay dropped, because the capture
does hold those.

Two further things had to change for that tail to still exist when the load
ends, and a browser test is what found both. `chunkedTerminalWrite` is what ends
the load for every non-empty buffer, so the flush policy travels to its own
finish calls; the call in `selectSession` runs only when the write was skipped.
`_beginBufferLoad` no longer empties the queue when one load re-enters it, which
it does on every write, because that reset discarded the whole fetch window
before anything could replay it.

The response already distinguishes the two cases. `source` reads `mux-visible`
or `mux-full-history` for a capture and `history` for the byte stream.

Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the ways the replayed frame itself
could disagree with the terminal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
irisitymichaelgrundberg added a commit to irisitymichaelgrundberg/Codeman that referenced this pull request Sep 15, 2026
Live terminal events are queued while a buffer load runs, and the load discards
that queue when it ends. That is right when the loaded buffer is the server's
accumulated byte history. The route appends to that history right up to the
moment it serializes the response, so a queued event already appears in it and
replaying it would duplicate output, most visibly Ink's cursor-up redraws.

A tmux pane capture is a photograph, current only as of the instant
`capture-pane` ran. Output printed afterwards was queued and then dropped, and
nothing scheduled a re-fetch to recover it: `_onSessionNeedsRefresh` is wired
only to the 128KB overflow path. The CLI's next partial redraw then landed on a
frame the terminal never received.

How much went missing depended on which capture the route served. A `?full=1`
load returns the capture alone, with no history in front of it, so it lost
everything from the capture to the end of the chunked write. A `?tail=` load
returns history, a clear, and then the capture, and the route reads that history
after the capture, so it lost everything from the response to the end of that
write. The chunked write dominates either way. An agent CLI hides the loss on
its next full redraw; a shell session does not, because its output is linear and
nothing repaints it.

Queue entries now carry their arrival time, and `_finishBufferLoad` takes a
`since` cutoff, so a capture load replays exactly the tail that arrived after
the response headers. The earlier events stay dropped, because a payload that
carries history does hold those.

Two further things had to change for that tail to still exist when the load
ends, and a browser test is what found both. `chunkedTerminalWrite` is what ends
the load for every non-empty buffer, so the flush policy travels to its own
finish calls; the call in `selectSession` runs only when the write was skipped.
`_beginBufferLoad` no longer empties the queue when one load re-enters it, which
it does on every write, because that reset discarded the whole fetch window
before anything could replay it.

The response already distinguishes the sources. `source` reads `mux-visible` or
`mux-full-history` for a capture and `history` for the byte stream.

Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the ways the replayed frame itself
could disagree with the terminal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
irisitymichaelgrundberg added a commit to irisitymichaelgrundberg/Codeman that referenced this pull request Sep 15, 2026
A visible-frame capture repaints each row at an absolute position, counting up
to the pane's height. A terminal shorter than that clamps every address past its
own height onto its last line. The overflow rows then overwrite one another, and
the rows underneath are lost. Replaying a real 50-row capture into a 30-row
terminal rendered 28 lines of a 45-line command and drew the frame twice.

Nothing in the response said what height the frame was built for, so the client
could not detect this. A capture now reports the geometry it was really taken at
through `capturedGeometry` on `PaneCaptureOptions`, and the terminal response
carries it as `captureCols` and `captureRows`. When the captured pane is taller
than the terminal, or the size that produced the capture did not survive the
load, `selectSession` replays once at the size that stuck. `resizeRetry` caps
that at one attempt, so two competing fits cannot trade replays forever.

The retry re-arms the full-history flag only when the pass that ran had consumed
it. A tab switch takes the bounded tail, so its retry takes the tail too:
clearing the flag unconditionally would upgrade that switch into a fresh
scrollback capture the user never asked for, which the route's own comments put
at tens of megabytes.

What this repairs is a capture that won a race against the resize meant to
precede it. It does not repair a capture whose pane was too tall because
`Session.resize` declined the resize outright, which it does for a small
viewport while a desktop viewport's size claim is live. The retry re-sends the
same declined resize and captures the same pane, and `resizeRetry` then stops
it. Repairing that means changing who owns the pane size, which is a policy
question this does not touch. The reported geometry still helps there, because
the client can see the mismatch at all rather than being blind to it.

Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the other ways the replayed frame and
the terminal could disagree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
irisitymichaelgrundberg added a commit to irisitymichaelgrundberg/Codeman that referenced this pull request Sep 15, 2026
Live terminal events are queued while a buffer load runs, and the load discards
that queue when it ends. That is right when the loaded buffer is the server's
accumulated byte history. The route appends to that history right up to the
moment it serializes the response, so a queued event already appears in it and
replaying it would duplicate output, most visibly Ink's cursor-up redraws.

A tmux pane capture is a photograph, current only as of the instant
`capture-pane` ran. Output printed afterwards was queued and then dropped, and
nothing scheduled a re-fetch to recover it: `_onSessionNeedsRefresh` is wired
only to the 128KB overflow path. The CLI's next partial redraw then landed on a
frame the terminal never received.

How much went missing depended on which capture the route served. A `?full=1`
load returns the capture alone, with no history in front of it, so it lost
everything from the capture to the end of the chunked write. A `?tail=` load
returns history, a clear, and then the capture, and the route reads that history
after the capture, so it lost everything from the response to the end of that
write. The chunked write dominates either way. An agent CLI hides the loss on
its next full redraw; a shell session does not, because its output is linear and
nothing repaints it.

Queue entries now carry their arrival time, and `_finishBufferLoad` takes a
`since` cutoff, so a capture load replays exactly the tail that arrived after
the response headers. The earlier events stay dropped, because a payload that
carries history does hold those.

All four paths that fetch a terminal buffer and write it now decide this the
same way, through one `_bufferLoadFinishOpts` helper, so they cannot drift
apart: `selectSession`, `_onSessionNeedsRefresh`, `_onSessionClearTerminal` and
`_maybeRefetchFullHistory`. The second of those is the one that stings. It
exists to restore output the client already dropped once under backpressure, and
it was dropping more output while performing that recovery. The cache-hit write
inside `selectSession` stays on discard deliberately: it runs before the fetch,
so its queue holds only events the capture that follows already contains.

Two further things had to change for that tail to still exist when the load
ends, and a browser test is what found both. `chunkedTerminalWrite` is what ends
the load for every non-empty buffer, so the flush policy travels to its own
finish calls; the call in `selectSession` runs only when the write was skipped.
`_beginBufferLoad` no longer empties the queue when one load re-enters it, which
it does on every write, because that reset discarded the whole fetch window
before anything could replay it.

The response already distinguishes the sources. `source` reads `mux-visible` or
`mux-full-history` for a capture and `history` for the byte stream.

Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the ways the replayed frame itself
could disagree with the terminal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
irisitymichaelgrundberg added a commit to irisitymichaelgrundberg/Codeman that referenced this pull request Sep 19, 2026
A visible-frame capture repaints each row at an absolute position, counting up
to the pane's height. A terminal shorter than that clamps every address past its
own height onto its last line. The overflow rows then overwrite one another, and
the rows underneath are lost. Replaying a real 50-row capture into a 30-row
terminal rendered 28 lines of a 45-line command and drew the frame twice.

Nothing in the response said what height the frame was built for, so the client
could not detect this. A capture now reports the geometry it was really taken at
through `capturedGeometry` on `PaneCaptureOptions`, and the terminal response
carries it as `captureCols` and `captureRows`. When the captured pane is taller
than the terminal, or the size that produced the capture did not survive the
load, `selectSession` replays once at the size that stuck. `resizeRetry` caps
that at one attempt, so two competing fits cannot trade replays forever.

The retry re-arms the full-history flag only when the pass that ran had consumed
it. A tab switch takes the bounded tail, so its retry takes the tail too:
clearing the flag unconditionally would upgrade that switch into a fresh
scrollback capture the user never asked for, which the route's own comments put
at tens of megabytes.

What this repairs is a capture that won a race against the resize meant to
precede it. It does not repair a capture whose pane was too tall because
`Session.resize` declined the resize outright, which it does for a small
viewport while a desktop viewport's size claim is live. The retry re-sends the
same declined resize and captures the same pane, and `resizeRetry` then stops
it. Repairing that means changing who owns the pane size, which is a policy
question this does not touch. The reported geometry still helps there, because
the client can see the mismatch at all rather than being blind to it.

Follows Ark0N#395, Ark0N#396 and Ark0N#397, which fixed the other ways the replayed frame and
the terminal could disagree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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