Skip to content

fix(split-pane): open Pane B's live queue after the response, keep the disconnected marker last - #524

Merged
Ark0N merged 1 commit into
Ark0N:masterfrom
timkjr:fix/split-pane-live-queue-timing
Oct 4, 2026
Merged

Ark0N merged 1 commit into
Ark0N:masterfrom
timkjr:fix/split-pane-live-queue-timing

Conversation

@timkjr

@timkjr timkjr commented Oct 2, 2026

Copy link
Copy Markdown

Follow-ups from the #506 review.

  • The history pull's live-frame queue opened before the fetch, so Pane B stopped painting for the whole round trip. It now opens when the response arrives, beside capturedAt. Frames from before it are replaced by the capture's \x1bc or written unchanged, so the final buffer is the same. The request uses the primary pane's CodemanFetchDeadline budget, and the body read gets 10 s once headers land, since that is the only window that holds live output.
  • A {t:'r'} refresh queued behind a pull ran its clear() after the marker was written and wiped it. A close during a refresh load also wrote the marker above the replay. The marker is now an owed flag (_markerOwed) that each load settles in its own finally, so it is the last thing on screen however the close, the pull and a trailing refresh interleave.

Two related items in one PR because they share the marker/queue state machine; happy to split if preferred.

Testing: unit tests drive each ordering. Checked in real Chrome against a beta instance with a delayed pull, old code vs new:

Scenario Old code New code
Output while the fetch is in flight held painted
Close mid-pull marker last marker last
Close mid-pull with a refresh queued marker wiped marker last

docs/architecture-invariants.md updated.

🤖 Generated with Claude Code

…e disconnected marker last

Follow-ups from the Ark0N#506 review.

The live-frame queue opened before the fetch, freezing Pane B for the
whole round trip. It now opens beside capturedAt; the request uses the
shared terminal fetch deadline and the body read a 10 s one.

A {t:'r'} refresh queued behind a pull ran its clear() after the
disconnected marker was written and wiped it, and a close during a
refresh load wrote the marker above the replay. The marker is now an
owed flag (_markerOwed) that each load settles in its own finally.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Ark0N
Ark0N merged commit ec2a036 into Ark0N:master Oct 4, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Oct 4, 2026
… what a pull's request phase holds (#524 review)

- Stale second marker above a trailing refresh's replay: xterm parses
  write() on a later tick while clear() is synchronous, so a marker stamped
  in a load's finally, just before _endBufferLoad() starts the trailing
  refresh, landed in the freshly cleared buffer above that refresh's replay.
  _stampMarkerIfOwed() now returns early while a refresh is pending; that
  refresh re-owes the marker on a closed socket and writes the one copy
  below its own replay. Pinned by marker-count assertions on the two
  existing trailing-refresh tests plus a new async-parse fake (writes
  parsed on a later tick, clear() synchronous) for back-to-back refreshes
  and a pull with a queued refresh and a close mid-pull; all four fail
  without the guard. Also checked against a real @xterm/headless 6.0.0.
- Marker withheld for up to the 45 s request budget: kept the behaviour and
  made the comment and the docs truthful. The pull's request phase holds no
  live output, but it holds the single-flight flag, so a coalesced {t:'r'}
  refresh and a close's owed marker wait for the response. Writing the
  marker at once during that phase would need a separate "awaiting
  response" state and, with a refresh pending, reopens the same
  write-vs-clear() race as above; a Codeman restart resets the in-flight
  request along with the socket, so that pull fails at once and stamps.
- Stale comments: _onSocketClosed() now says the deferral covers any load,
  _writeDisconnectedMarker() points at _stampMarkerIfOwed(), and the pull's
  finally comment describes the hand-off to a trailing refresh.
- Invariants doc: dropped "the initial load" from the loads a close can land
  in (connect() awaits it before creating the socket), reworded the
  "nested refresh stamps its own" sentence to describe the guard, and noted
  what the request phase holds.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Ark0N pushed a commit that referenced this pull request Oct 4, 2026
…#520, #521, #522, #523, #524, #530, #531)

One consolidated minor changeset with the Thanks block first; the four contributor changesets (#520, #521, #522, #523) are folded into it.

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

Ark0N commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Merged and shipped in 1.34.0. Thanks @timkjr! Turning the marker into an owed flag that each load settles in its own finally is a much sturdier model than the previous one, and your old-versus-new table in real Chrome showed exactly which interleaving it fixes.

I applied the review items while landing (36af183):

  • A refresh queued behind a pull no longer leaves a second, stale "disconnected" marker above its replay. _stampMarkerIfOwed() leaves the marker to the pending refresh, which writes the only copy below its own replay. That is needed because xterm's write() is asynchronous while clear() is not. New tests use a fake terminal that parses writes on a later tick, and it was also checked against @xterm/headless.
  • A close while the pull is still waiting for headers keeps the marker deferred (writing it there would bring the same race back with a queued refresh). The comment and the invariants doc now say plainly what the request phase holds and for how long.
  • The two stale comments are fixed, and the doc no longer lists the initial load as one a close can land in.

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