fix(session): stop restored scrollback from garbling in split-pane workspaces - #242
Merged
Conversation
Extracts the plain-tail fallback's concat+cap+decode step out of the
singleton store into SessionWALCore.decodeFallbackScrollbackText (pure,
unit-testable; behavior unchanged) and adds the regression test: a
suffix-cap cut landing mid-CSI or mid-UTF-8 must not leak orphaned
escape/continuation bytes as literal replayed text, and uncut data must
survive untouched even when its genuine leading bytes look like a torn
CSI tail.
readFrameAndDelta already guards its cut sites this way; the fallback
branch (no usable frame: session younger than the capture interval, or
walGeneration mismatch after rotation) was missing the guard. This
commit is test-only by design — the trim lands next, so CI shows the
test failing without it.
Symptom: corrupted characters in restored scrollback ("t*inking",
"eff5rt") reported by the team.
Apply trimTornLeadingBytes when — and only when — the fallback path's suffix cap actually cut the byte stream. Turns the previous commit's regression test green. Unconditional trimming would be wrong the other way: at a genuine stream beginning the torn-CSI heuristic cannot distinguish legitimate leading text like "196mVISIBLE" from an orphaned parameter run, so uncut data skips the trim entirely (covered by the same test).
Multi-pane restore raced the one-shot scrollback seed against divider restoration: restoreSessionSnapshot creates every pane at bonsplit's default 50/50 fractions and applies persisted divider positions only afterwards, while the revive seed fired on the FIRST post-creation updateSize (or a 0.15s timer) — i.e. at the wrong width for any non-50/50 split. Replaying \r-overwritten spinner output at a narrower-than-capture width wraps every frame, the in-place overwrite misses its row, and every frame stays visible — the scattered repeated "thinking with high effort" garbage the team screenshotted. Single-pane workspaces never hit it, which matches who reported it. Each restore pass now opens a generation-tagged settle gate before creating panels: surfaces revived under the gate hold their seed until the pass has applied divider positions plus one main-queue turn, then seed immediately (the surface is already at final width by then — the gate blocks seed consumption, never resizing). The 0.15s fallback timer re-arms while gated, bounded at 6s as a force-through safety net. Settling is a pass counter and each pass clears exactly its own generation, so back-to-back workspace restores at app relaunch cannot strand an earlier pass's seeds until the ceiling. No unit test: driving createSurface's revive path plus updateSize requires a live ghostty surface; there is no seam for it today. The fallback-trim half of this branch carries the regression test.
3 tasks
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
1 participant
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.
What this does
Fixes the scattered garbled spinner output after session restore (the repeated corrupted 'thinking with high effort' screenshots from the team thread). Two remaining holes after #240, both worse or exclusive to split-pane workspaces:
Divider race: restore creates every pane at bonsplit's default 50/50 fractions and applies persisted divider positions only afterwards, but the one-shot scrollback seed fired on the first layout pass, i.e. at the wrong width for any non-50/50 split. Spinner output rewrites itself in place with carriage returns; replayed at a narrower width every frame wraps, the overwrite misses its row, and every frame stays visible. Single-pane workspaces never hit this, which matches exactly who does and does not see the bug. Each restore pass now opens a generation-tagged gate that holds revive seeds until divider positions are applied plus one main-queue turn. The gate blocks seed consumption only, never resizing, so surfaces are at final width when the seed fires. A bounded (6s) re-arming fallback timer remains as a force-through safety net, and each pass clears exactly its own generation so back-to-back workspace restores at relaunch cannot strand each other.
Torn escapes on the fallback path: the no-frame fallback (session younger than the capture interval, or walGeneration mismatch after rotation) capped the WAL tail with a raw suffix cut and decoded it without trimming, leaking orphaned CSI/UTF-8 fragments as literal text (the 't*inking' / 'eff5rt' corruption flavor). The cut is now trimmed like readFrameAndDelta already does, and only when a cut actually occurred, so genuine leading text that merely looks like a torn escape survives.
Review order
Test plan
Gate itself has no unit test: driving createSurface's revive path plus updateSize needs a live ghostty surface and there is no seam for it today.