Skip to content

fix(terminal): keep the output a pane capture could not contain - #436

Merged
Ark0N merged 4 commits into
Ark0N:masterfrom
irisitymichaelgrundberg:fix/replay-output-that-arrived-after-the-capture
Sep 18, 2026
Merged

Ark0N merged 4 commits into
Ark0N:masterfrom
irisitymichaelgrundberg:fix/replay-output-that-arrived-after-the-capture

Conversation

@irisitymichaelgrundberg

@irisitymichaelgrundberg irisitymichaelgrundberg commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Live terminal events are queued while a buffer load runs, and the load discards that queue when it ends. That is right for the server's accumulated byte history and wrong for a tmux pane capture, which is current only up to capture time. Output emitted during the rest of the load was dropped with nothing to recover it.

The problem

batchTerminalWrite queues live events while _isLoadingBuffer is true, and _finishBufferLoad discards that queue unless the caller passes flushQueued. The reasoning in its doc comment is sound for one case only:

For an established session the loaded buffer from the API is the source of truth up to the response timestamp; SSE events queued during the fetch+write overlap already appear in that buffer.

That holds for the byte history, which the route keeps appending to right up to the moment it serializes the response. A pane capture is a photograph, current only as of the instant capture-pane ran. Everything printed afterwards is queued and then dropped, and nothing schedules a re-fetch to recover it: _onSessionNeedsRefresh is wired only to the 128KB overflow path. The CLI's next partial redraw then lands on a frame the terminal never received.

How much is lost depends on which capture the route served, and the two differ:

  • ?full=1 returns the capture alone, with no byte history in front of it. Nothing in the payload covers the gap, so the loss runs from the capture to the end of the chunked write.
  • ?tail= returns byteHistory + clear + capture, and the route reads that history after the capture. The history therefore covers up to the response, and the loss runs from the response to the end of the chunked write.

The chunked write dominates either way. It spreads across rAF frames after a fetch that measured 25–90ms against live sessions here, and the load has already sat through a 400ms redraw settle whenever the dimensions changed.

The response already distinguishes the sources. source reads mux-visible or mux-full-history for a capture, and history for the byte stream, so the client has what it needs to tell them apart.

Changes

  • src/web/public/terminal-ui.js — queue entries carry their arrival time, and _finishBufferLoad takes a since cutoff. A capture load passes the response's arrival time. The pre-capture events then stay dropped and only the tail replays. Without since, a flush would duplicate everything the capture already holds.
  • src/web/public/terminal-ui.jschunkedTerminalWrite takes the finish options and applies them at its own three finish sites. It is what ends the load for every non-empty buffer, so a policy set only in selectSession never ran on the path that matters.
  • src/web/public/terminal-ui.js_beginBufferLoad keeps the queue when one load re-enters it. selectSession opens the load before its fetch and chunkedTerminalWrite opens it again under the same owner, and the reset on that second call discarded the entire fetch window before anything could replay it. A genuinely new load still starts empty.
  • src/web/public/app.js — one _bufferLoadFinishOpts helper decides the policy from a response's source, and all four paths that fetch a terminal buffer and write it go through it: selectSession, _onSessionNeedsRefresh, _onSessionClearTerminal and _maybeRefetchFullHistory. They previously differed, and only selectSession got the fix. _onSessionNeedsRefresh 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 call that remains at the end of selectSession runs only when the write was skipped, which is where COD-144 lives: a new session's first prompt predates the response, so an empty paint replays its queue whole rather than from the header timestamp. The queued-byte total reads w.data.length to match the queue's new entry shape.
  • The cache-hit write inside selectSession deliberately stays on discard. It runs before the fetch, so its queue holds only events the capture that follows already contains.
  • Tests: the since cutoff and the re-entry rule in test/terminal-buffer-flush.test.ts, plus a new browser suite.

Verification

Every CI gate passes locally:

  • npm run typecheck, npm run lint, npm run format:check, npm run check:frontend-syntax
  • npm run check:lockfile
  • npm test, green at 7014 tests across 369 files

The baseline on master is 7009 tests, so that is the added tests and no regressions.

test/capture-load-window.browser.test.ts drives the real client in chromium. It injects one live event from inside the response's own json() call, which is the one place guaranteed to land after the headers and before the chunked write. Run before and after the change:

buffer from a pane capture buffer from the byte history
on master fails: the event is dropped passes: the event is dropped
with this change passes: the event reaches the terminal passes: the event is dropped

The second column matters as much as the first. The byte history already contains everything up to the response, so replaying the queue on top of it would duplicate output, most visibly Ink's cursor-up redraws. That discard had to survive.

What this does not cover

The cutoff is headersReceivedAt, because that is the only clock the client holds. It is exactly right for the ?tail= path, where the byte history covers everything up to the response. On the ?full=1 path it is conservative: events between the capture and the response are in no payload and still go missing, which is the server's own prepare phase plus one network leg.

Closing that last gap means estimating the capture instant from the Server-Timing header the route already emits, and selectSession already reads that header. I did not do it. Halving an unmeasured round trip to date-stamp a frame felt like more guesswork than the remaining milliseconds justify, and a cutoff that guesses too early duplicates output instead of dropping it. Happy to add it if you would rather have the whole window.

Worth saying plainly: the first version of this change put the policy only at the _finishBufferLoad call in selectSession, and its unit tests passed. The browser test above is what showed it never ran, because chunkedTerminalWrite had already ended the load. The two extra changes in terminal-ui.js came out of that. A later review pass found the same fix still covered only one of the four load paths, which is where the shared helper came from.

Running the browser suite in a tree that borrows another checkout's node_modules needs node scripts/prepare-test-vendor.mjs first. Without the vendored xterm bundles the page never defines Terminal, and the test fails on its wait for app.terminal.

Related

Follows #395, #396 and #397, which fixed the ways the replayed frame itself could disagree with the terminal. This one is about output that no frame contained.

Independent of #435, which reports the height a capture was taken at. The two touch selectSession but not the same lines, and merge cleanly in either order.

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
irisitymichaelgrundberg force-pushed the fix/replay-output-that-arrived-after-the-capture branch from fb1b526 to c9515b1 Compare September 15, 2026 16:15
@irisitymichaelgrundberg
irisitymichaelgrundberg marked this pull request as ready for review September 15, 2026 16:23
@Ark0N

Ark0N commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Same as on #435: both jobs are green on this head, and the reviewer here skips drafts, so this is currently outside the review queue rather than at the back of it. If the draft flag marks something unfinished, tell me what and I will wait. Otherwise please mark it ready for review.

I am putting 1.29.2 together and this is the kind of change I would want in it, but not unreviewed.

Worth knowing: master moved an hour ago (1.29.1, carrying #376's auto-naming, which touches app.js). This still merges clean over it and clean alongside the other open PRs, in any order, so no rebase needed. I verified your "independent of #435, merge cleanly in either order" note directly and it holds.

Two remarks ahead of the actual review, neither of them blocking:

The paragraph about your first version passing its unit tests while never running is the most useful thing in this description, and I would rather you kept writing them than trimmed them out. chunkedTerminalWrite ending the load before the policy in selectSession could apply is exactly the failure mode that makes a green suite worthless, and "a later review pass found the same fix still covered only one of the four load paths" is why the shared helper is the right shape rather than a fourth copy. That is the invariant worth pinning, and it is close to something CLAUDE.md already says about single resolvers for call sites that must not disagree.

On the ?full=1 window you deliberately left open: I agree with the call. A cutoff that guesses too early duplicates output rather than dropping it, and duplicated Ink redraws are more visible and more confusing than a few missing milliseconds. Leave it. If it ever turns out to matter, the Server-Timing route is there and the PR description already explains it better than a comment would.

Mark it ready and it goes into the queue.

@Ark0N

Ark0N commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Thanks for this, and for the write-up. The distinction you drew (the byte history is current up to the response, a pane capture only up to capture time) is right, and the browser test that showed the first version of the fix never ran because chunkedTerminalWrite had already ended the load is exactly the kind of evidence that makes a terminal change reviewable. I ran that test against master here and it does fail on the capture case and pass on the history case, as you said.

One thing needs fixing before I can take it.

The queue replay latches the sticky-scroll flag, so a refresh now drags a scrolled-up reader to the bottom (src/web/public/terminal-ui.js:3900). The replay goes through batchTerminalWrite, which re-samples this._wasAtBottomBeforeWrite = this.isTerminalAtBottom() at terminal-ui.js:3281. That happens inside chunkedTerminalWrite before its promise resolves, at a moment when the terminal has just been reset and rewritten, so isTerminalAtBottom() is true and the flag latches true. The caller then restores the user's position (_onSessionNeedsRefresh at app.js:2559, _maybeRefetchFullHistory at app.js:5843), and the next flushPendingWrites() restores preserveViewportY and immediately calls scrollToBottom() off that latched flag (terminal-ui.js:3596), undoing the restore. The only thing in the way is _hasRecentUserScrollUp(), a 1500ms window that a server-triggered needsRefresh will usually be past. Those two paths are the ones #259 and #205 exist for, and they are also where a non-empty queue is most likely, since a needsRefresh fires precisely when output is flooding.

I confirmed it by driving the real _finishBufferLoad in the vm harness test/terminal-flush-budget.test.ts already uses: with isTerminalAtBottom() returning true, a one-entry flush leaves _wasAtBottomBeforeWrite === true. Simplest fix I see is to re-sync the baseline after each caller restores the viewport (this._wasAtBottomBeforeWrite = this.isTerminalAtBottom(); right after the scrollToLine in both), or to stop the replay loop sampling at all. A case in test/terminal-buffer-flush.test.ts would cover it.

Smaller things, any of which I am happy to fold in at merge or take as a follow-up:

  • _bufferLoadFinishOpts (app.js:1928) is the new policy and nothing in npm test touches it: the only coverage is the browser suite, which CI does not run. It drops straight into the vm harness in test/terminal-flush-budget.test.ts, four assertions (both mux sources flush, history does not, a missing source does not).
  • headersReceivedAt is not quite duplicate-proof on a busy session. Output is appended to the byte buffer and emitted in the same tick (session.ts:1936), but the broadcast is batched: 8ms over WebSocket (ws-routes.ts:42) and 16 to 50ms over SSE (sse-stream-manager.ts:371). The terminal route is fully synchronous from capture-pane to its return, so the batch that was pending when the capture ran is already in the payload and still goes out after the reply, which means it now replays. It is bounded and much smaller than what you are recovering, so I am not asking you to solve it, but it belongs in the "what this does not cover" list. If you ever want to close it, the fix is server side: flush that session's pending batch before taking the capture.
  • The second browser test does not assert sessionId is truthy the way the first does (test/capture-load-window.browser.test.ts:167), so a failed session create would pass it with zero hits instead of failing it.
  • docs/architecture-invariants.md:119 still says the replay releases the gate "without extending the pre-existing queued-event discard window". The JSDoc you updated is good; that line is where the next person will look.

Everything else checks out on my machine: typecheck, lint, format:check, check:frontend-syntax and the full npm test are green at 7014 tests, the changeset is right, no version or CHANGELOG edits, and the new suite is registered in config/test-suites.ts the way the repo expects. Fix the scroll latch and I will merge.

A capture load now replays its queued tail, and that replay runs through
`batchTerminalWrite`, which samples `_wasAtBottomBeforeWrite` before it queues.
It runs inside `chunkedTerminalWrite`, before that promise resolves, with the
terminal freshly reset and rewritten — so the sample is always true. The caller
then restored the reader's position and the next `flushPendingWrites` scrolled
straight back to the bottom off the latched flag, undoing it. The only thing in
the way was `_hasRecentUserScrollUp()`, a 1500ms window a server-triggered
refresh is usually past.

`_syncStickyScrollBaseline()` re-takes the flag from wherever the viewport now
sits, and the two paths that restore a position call it right after doing so:
`_onSessionNeedsRefresh` and `_maybeRefetchFullHistory`. Those are the paths
Ark0N#259 and Ark0N#205 exist for, and they are also where a non-empty queue is most
likely, since a needsRefresh fires when output is flooding. Re-taking rather
than suppressing the sampling: suppressing leaves whatever stale value the flag
held from before the load, which on the full-history re-pull has no reason to
be false. `selectSession` and `_onSessionClearTerminal` deliberately end at the
bottom, so the sampled true is already the truth there and they do not call it.

`_bufferLoadFinishOpts` gains the coverage the CI gate can see: both mux
sources flush, `history` does not, and a payload naming no source does not.
Its only coverage was the browser suite, which CI does not run.

The JSDoc and the changeset now record the one duplicate window this cutoff
cannot close. The server appends output to the byte buffer in the same tick it
emits, but broadcasts on a batch timer — 8ms over WebSocket, 16 to 50ms over
SSE — so a batch pending when `capture-pane` ran leaves the server after the
reply and is replayed although the capture holds it. It is one batch interval
wide against a recovery window spanning the whole chunked write, and closing it
means flushing that batch server side before the capture.

The second browser test asserts its session was created, so a failed create
fails it instead of passing with zero hits.

docs/architecture-invariants.md no longer claims the replay leaves the
queued-event discard window alone. That clause now describes what decides how a
load ends, the baseline rule, the batch window, and the three covering tests.

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

Ark0N commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Thanks for this, and for coming back with all four items from the last round in one head.

For anyone reading later: a buffer load parks live terminal events in a queue and drops that queue when it ends, which is right for the server's byte history and wrong for a tmux pane capture, because a capture is only current as of the instant capture-pane ran. Everything printed after the shutter was queued, dropped, and never re-fetched. This PR stamps each queue entry with its arrival time, gives _finishBufferLoad a since cutoff, and routes all four paths that fetch a terminal buffer and write it through one _bufferLoadFinishOpts helper so they cannot drift apart again.

I re-ran the before and after here rather than taking the table on trust. With app.js and terminal-ui.js from the merge base, test/capture-load-window.browser.test.ts fails the capture case with expected +0 to be 1 and passes the history case. With your change both pass. The full gate is green at 7021 tests across 369 files, and typecheck, lint, format:check, check:frontend-syntax, check:lockfile and check:public-assets all pass. The browser suite has four red files on this machine and none of them are yours: codex-predictive-echo needs a real codex binary, opencode-resize and inline-rename fail identically with the merge-base frontend swapped in, and terminal-copy-shortcut is flaky at the same rate on both sides (3 failures in 15 runs each way) and never enters any code this diff touches.

Nothing blocks. One thing I would like from you, and a few I will fix myself at merge.

The ask:

  • Pin the four load paths to the helper (test/terminal-buffer-flush.test.ts:327). Your own write-up says the first version covered one of four paths and a later pass found it still covered one of four. Nothing in the gate stops a fifth path, or an inlined { flushQueued: true }, from reintroducing that. The static-scan idiom you already wrote for _syncStickyScrollBaseline sits right there: the same slice over selectSession, _onSessionNeedsRefresh, _onSessionClearTerminal and _maybeRefetchFullHistory, asserting each contains this._bufferLoadFinishOpts(, and it runs in CI where the browser suite does not.

What I will fold in at merge, listed so you know they are not oversights on your side:

  • src/web/public/terminal-ui.js:3147 says selectSession deliberately ends at the bottom. It ends at scrollToLastNonEmptyLine() (app.js:6512), which targets lastNonEmptyLine - rows + 2 and so sits above baseY whenever the replayed frame keeps trailing blank rows, which a full capture does on purpose. Driving your own methods in a vm harness against a 100 row buffer with 10 trailing blanks: the flag latches true, the viewport lands at 67 against a baseY of 76, and isTerminalAtBottom() is false. It has no visible effect once this merges, because de864e7d landed on master after you cut this branch and now gates the sticky snap on preserveViewportY === null, so I will just correct the sentence.
  • src/web/public/app.js:1924 bounds the remaining duplicate window at one batch interval. There is a second contributor: captureActivePaneBuffer is execSync, so the event loop is blocked for the whole capture and anything tmux had already painted into the pane but the server had not yet read from the attach PTY is in the capture, is broadcast only after the reply, and replays too. The trade is still clearly worth it, but one batch interval is the number the next person will reason from.
  • test/terminal-buffer-flush.test.ts:1 still describes the file as COD-144 only, and says the replay happens only when the load painted nothing, which since is exactly the counterexample to.
  • src/web/public/terminal-ui.js:3855, pre-existing: the block headed "Complete a buffer load" sits above _beginBufferLoad, so the @param list you extended documents the function 15 lines further down. Cheap to move while it is being edited.

On the second commit, for your information rather than as a change request: the sticky-scroll latch was real against your merge base, but master gained de864e7d eight hours later and that already prevents the snap over a restore. Your version is defence in depth now, it costs nothing, and its unit test pins a true property of the replay, so it stays.

Send the drift test and I will merge.

…lper

The first version of this fix decided the flush policy in `selectSession`
alone, and a later pass found it still covering one path of four. Nothing in
the CI gate stops a fifth path, or an inlined `{ flushQueued: true }`, from
splitting that policy up again — the browser suite that would notice is
excluded from `npm test`.

A static scan over `selectSession`, `_onSessionNeedsRefresh`,
`_onSessionClearTerminal` and `_maybeRefetchFullHistory` asserts each one asks
`_bufferLoadFinishOpts`, reusing the `methodBody` slice the sticky-scroll guard
already needed. Verified by inlining the policy back into
`_onSessionClearTerminal`, which fails it by name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The JSDoc on `_syncStickyScrollBaseline` said `selectSession` deliberately ends
at the bottom, so the baseline the replay samples is already true there. It
does not. `selectSession` calls `scrollToBottom()` after the write and then
ends at `scrollToLastNonEmptyLine()` (app.js:6512), which targets
`lastNonEmptyLine - rows + 2` and therefore parks ABOVE `baseY` whenever the
replayed frame keeps trailing blank rows — which a full capture does on
purpose, since no transform that can delete a line may run over one.

Its baseline really is a stale true. What covers it is the sticky snap itself:
since de864e7 that snap fires only when the flush found the viewport already
at the bottom (`preserveViewportY === null`), which a parked selectSession
viewport is not. That commit landed on master after this branch was cut, so
the guard arrives with the merge rather than being present here.

`_onSessionClearTerminal` is unchanged in the comment and was correct: it
resets and rewrites with no scroll afterwards, so it does end at the bottom.

Comment only; no behaviour change.

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

Copy link
Copy Markdown
Contributor Author

Drift test is in (cfd771d1). Static scan over selectSession, _onSessionNeedsRefresh, _onSessionClearTerminal and _maybeRefetchFullHistory, each asserted to contain this._bufferLoadFinishOpts(, reusing the methodBody slice the sticky-scroll guard already needed. I checked it bites rather than assuming it: inlining { flushQueued: true, since: headersReceivedAt } back into _onSessionClearTerminal fails it by name.

I also took your first fold-in item, so you do not do it twice (3730bc7d, comment only). You were right and I had it backwards: selectSession calls scrollToBottom() after the write, which is what misled me, but it ends at scrollToLastNonEmptyLine() and parks above baseY on any frame with trailing blanks. The comment now says that, says the baseline there is a stale true, and points at the snap guard as the thing that covers it.

One thing on that. The guard is de864e7d, which is on master but not on this branch, so the coverage arrives with the merge rather than being present here. Pre-merge, a capture load through selectSession can snap past the scrollToLastNonEmptyLine() anchor and show the blank tail. Post-merge it cannot, exactly as you said. If you would rather not carry that gap between now and the merge, this._syncStickyScrollBaseline() in selectSession closes it in one line and is a no-op afterwards — your call, and I have left it out because you had already decided the other way.

Your other three fold-ins I have left alone. Green here: typecheck, lint, format:check, check:frontend-syntax and the full gate at 7022 tests.

@Ark0N

Ark0N commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Thanks for the drift test, and for taking the comment fix yourself so I did not do it twice.

On the one-liner: leave it out. The guard is on master and arrives with this merge, so the gap only exists on the branch, and a _syncStickyScrollBaseline() call in selectSession that turns into a no-op the moment it lands is a line the next reader has to reason about for nothing.

Merging now. The remaining comment and changeset edits from the last round (the _bufferLoadFinishOpts comment, the test file header, the doc block above _beginBufferLoad) go in as a follow-up commit on master right after, so they do not hold this up. Thanks for the whole series, the four-paths-one-helper shape is the right one and the browser test that proved the first version never ran is the part I will point people at.

@Ark0N
Ark0N merged commit 492f8d8 into Ark0N:master Sep 18, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Sep 18, 2026
The four edits the review said would be folded in at merge, none of them
code: the changeset becomes one user-facing paragraph, since it is what
CHANGELOG.md and the release notes print; the `_bufferLoadFinishOpts` comment
now names the second contributor to the duplicate window (`captureActivePaneBuffer`
is `execSync`, so anything painted into the pane before the server read it is
in the capture and is broadcast after the reply) and says why a `history`
payload keeps the pre-existing discard when its exposure is the same; the
`_finishBufferLoad` doc block moves from above `_beginBufferLoad` onto the
function it documents; and the test file's header describes both rules the
file now pins instead of only COD-144.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
irisitymichaelgrundberg added a commit to irisitymichaelgrundberg/Codeman that referenced this pull request Sep 19, 2026
`resizeRetry` caps the recursion inside one select and says nothing about the
next one, so a pane this browser cannot size reported the same mismatch on
every select and bought the same failed repair each time: two fetches per tab
switch for the life of the page, measured as a running count of 2, 4, 6 across
three selects. That is the case this branch describes as happening every time
rather than occasionally, a phone whose resize `Session.resize` declines while a
desktop claim is live, and it is not the only one — any pane Codeman cannot size
lands there, including one a second tmux client is also holding. Each wasted
pass costs another `capture-pane`, which is `execSync` and blocks the server's
event loop, plus a reset and chunked rewrite, a discarded snapshot and cache
entry, and a dropped and reopened WebSocket.

`_geometryRetryUseless` mirrors the existing `_fullHistoryRepullUseless`: a
retry pass whose frame still does not fit adds the session, geometry that fits
removes it, and the replay gate consults it. The proof has to come from a retry
pass rather than a first one, because the retry ran at the size that stuck and
the pane ignored it. Clearing on a fitting frame is what stops a pane that
becomes sizeable again, once the desktop tab closes or its claim goes idle, from
staying permanently unrepaired. The race case never reaches the latch, since it
converges on its first attempt.

The new browser case walks all of that: three selects reading 2, 3, 4 instead of
2, 4, 6, then a fitting frame, then a mismatch diagnosed afresh. Without the
gate it fails on the second switch with `expected 4 to be 3`.

Rebased onto master, which has moved to 1.30.0 and taken Ark0N#436. The one conflict
was `config/test-suites.ts`, where both branches appended a glob to
`BROWSER_TEST_GLOBS`; both are kept. Everything else merged clean, Ark0N#436's own
changes to the same buffer-load path included.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot mentioned this pull request Sep 19, 2026
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