fix: keep boo ui scrollback replay compatible with older daemons#75
Merged
Conversation
A boo ui view's scrollback lived only in its local terminal and was built from output streamed while attached. Switching sessions destroys the view (and its scrollback) and creates a fresh one, and the daemon's attach replay (repaint) deliberately sends only the visible screen, so a just-switched-to view had nothing above the current screen to scroll up into. Replay the window's history to ui views on attach: - window.historyReplay: VT bytes that reproduce the scrollback HISTORY (rows above the visible screen) as a styled, scrolling stream, plus a full-screen flush so every history row lands in the canvas's scrollback before the repaint's erase, with no blank gap. Fed before repaint(), a fresh terminal ends up holding the same scrollback, viewport at bottom. - protocol.AttachPayload: the attach handshake now carries a `ui` flag (decodes a bare 4-byte size payload as non-ui for slack). - The daemon sends the history (chunked across output frames, since it can exceed max_payload) before the repaint, but only to ui clients. A plain `boo attach` is raw passthrough, where a history dump would spam the user's real terminal, so it stays history-free. Verified end to end: a ui attach receives scrolled-off history; a plain attach receives only the visible screen. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The original change widened the attach handshake to a 5-byte AttachPayload (rows, cols, ui flag). An already-running daemon from before that change decodes attach as a strict 4-byte SizePayload and rejects the extra byte, so after upgrading the binary a new `boo ui`/`boo attach` could not attach to a pre-upgrade session (the client reported "lost connection"). Carry the ui-ness out of band instead: - protocol: drop AttachPayload; attach stays the 4-byte SizePayload it has always been. Add a client-to-daemon `ui` message (type 6) that marks the connection as a ui view. - ui: send the `ui` marker right before attaching. An older daemon ignores the unknown message type (its handler has a catch-all) and simply attaches the view with no history, so a new ui degrades gracefully against an old daemon instead of failing. - daemon: set conn.ui from the `ui` message; attach decodes SizePayload again and still replays history to ui views. - client: revert plain attach to the unchanged 4-byte handshake. Add PTY integration tests for both directions of the feature: a ui view attaching to a session that already had scrolled-off history pages it in on a wheel-up, and a plain attach receives only the visible screen. Verified all four upgrade combinations: new/new replays history; new ui against an old daemon attaches with no history (was "lost connection"); old client against a new daemon still attaches. Generated by Coder Agents on behalf of @kylecarbs.
510a3fa to
07045f7
Compare
A freshly focused ui view is seeded by the daemon with a scrollback history replay followed by a repaint. The ui renders on a ~15ms timer, so for a history large enough to span multiple output frames it could paint a half-applied frame, briefly flashing partial scrollback before the repaint snapped the viewport to the live screen. Hold the viewport blank from attach until the repaint's terminating `.screen` message arrives, then reveal it cleanly. The sidebar and status keep rendering during the load so the switch stays responsive, and the cursor stays hidden while the viewport is blank. Non-live views render their placard as before. Reveal incrementally rather than with a full repaint. The held frames clear libghostty's dirty bits while the viewport rows are painted blank, so the reveal invalidates the viewport cache to re-serialize the live rows instead of reusing stale bytes, but leaves the row-level diff in place so only rows that actually changed are written, exactly as a plain attach's first repaint does. A full-screen rewrite here would emit every row at once, and a ui whose terminal is momentarily undrained blocks on that larger write (the same Darwin PTY buffering restoreTty works around) and wedges the loop before it reads the next key or SIGWINCH. Generated by Coder Agents on behalf of @kylecarbs.
07045f7 to
af75c7b
Compare
Merged
BenLocal
added a commit
to BenLocal/boo
that referenced
this pull request
Jun 18, 2026
Brings in upstream coder#73 (boo ui sessions created at the viewport size), coder#74 (capture held prefix keys via kitty release events), coder#75 (scrollback replay on attach), and the v0.5.21/v0.5.22 releases. Conflict resolution: - main.zig / daemon.zig: union the createSession / runDaemon / Options params — self's state_dir/cwd/max_scrollback (restore + config) plus main's rows/cols (viewport-size fix). restoreOne passes null rows/cols. - scrollback replay (ui.zig, daemon.zig, protocol.zig, client.zig): self and main implemented the same feature two ways. Kept main's design (a separate `.ui` marker message before a SizePayload attach) and dropped self's redundant AttachPayload.ui flag, so client and daemon agree on the wire. - window.zig: kept self's added alt-screen historyReplay test. Verified: zig fmt, zig build, unit tests, and PTY integration tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Iterates on #72 (by @BenLocal) so we can land scrollback replay without the upgrade hazard, and adds the integration coverage that PR was missing. #72's original commit is preserved underneath; this branch adds fixes on top, so we can iterate here without blocking on the contributor.
What this changes on top of #72
AttachPayload(rows, cols, ui flag). A daemon started by a pre-upgrade binary decodes attach as a strict 4-byteSizePayloadand rejects the extra byte, so after upgrading, a newboo ui/boo attachcannot attach to a pre-upgrade session (the client reportslost connection). This swaps the flag-in-payload for a separateuimarker message (type 6) sent right before attach. Attach stays the original 4-byteSizePayload. An older daemon ignores the unknown message type (its handler already has a catch-all) and just attaches the view with no history, i.e. graceful degradation instead of a failed attach..screenarrives, then revealed. The reveal is incremental: it invalidates the viewport cache so the held-blank rows are re-serialized from the now-complete screen, but keeps the row-level diff, so only rows that actually changed are written — exactly like a plain attach's first repaint. A full-screen rewrite here would emit every row at once, and a ui whose terminal is momentarily undrained would block on that larger write (the same Darwin PTY buffering fix: capture held prefix command keys via kitty release events #74'srestoreTtyworks around) and wedge its event loop before it read the next key or SIGWINCH. The sidebar/status keep rendering so the switch stays responsive, and the cursor stays hidden while the viewport is blank.The core mechanism from #72 (
Window.historyReplay, the daemon replaying history to ui views before the repaint) is unchanged.Verification (Zig 0.15.2)
zig fmt --checkzig buildzig build testzig build test-integrationzig build test-all -Doptimize=ReleaseSafeCross-version matrix (manually driven over real PTYs):
uiuilost connectionattachFull review of #72 and detailed findings
Verdict
The feature is well built and correct. The history reconstruction (serialize scrollback history as styled VT, reset SGR, scroll a full screen so every history row lands in the canvas scrollback before the repaint's erase, then repaint the visible screen) is sound and unit-tested for full-dump equality + viewport-at-bottom. Plain
attachcorrectly stays history-free; alt-screen sessions correctly yield no history.Findings addressed here
uimarker so attach stays 4 bytes.ui: create and killandui: viewport resizetests wedged against a full-screen reveal write under Darwin PTY buffering.Findings left for a follow-up (non-blocking)
historyReplayserializes the whole history into one allocation per attach (bounded bymax_scrollback). Fine in practice; could stream if it ever matters.Notes
PageList.getTopLeft/getBottomRight(.history),formatter.Extra.none) confirmed against the pinned ghostty.historyTosplitting atmax_payloadis safe because the client feeds frames in order into a stateful VT parser, so a sequence split across the boundary still parses.Generated by Coder Agents on behalf of @kylecarbs.