fix(session): stop restored scrollback from leaking terminal state - #236
Merged
Conversation
…restart Restart replays saved scrollback into a fresh terminal. Those bytes are a raw tail of live pty output, so any DECSET in them is unbalanced: the program that would have sent the matching DECRST was killed by the restart. Replaying re-arms the mode permanently. These three tests describe what the replay text has to do about that. They fail on this commit and pass on the next one. - mouse tracking left armed reports motion at the prompt - alt screen, autowrap and charset left armed break the layout, and a window resize clears none of them - the cursor-moving resets have to be bracketed so the prompt does not land on top of the restored output
Three things went wrong when the app restarted. The replayed transcript re-armed whatever modes the killed program left set. Mouse tracking was the visible one: the prompt filled with literal motion reports. The rest broke the layout. A window resize clears the scrolling region but not the active screen, the wraparound bit or the charset, which is why resizing rescued some people and did nothing at all for others. The replay also ran before the surface was sized, so the transcript was parsed against ghostty's 800x600 placeholder grid instead of the real pane. Wrapped lines recover on a later resize. Absolute cursor addressing in a captured TUI redraw does not. Content scale was read once at surface creation. When an external display enumerates late, that read lands on the wrong screen and the grid is computed with the wrong DPI while the pixel size stays correct, so resizing cannot fix it. Nothing watched for the display configuration changing, so it never corrected itself. Size the surface before replaying, disarm the modes the transcript leaves behind, and re-assert display and scale when the screen configuration changes. Two notes on the reset sequence, both load-bearing: 1047l is used instead of 1049l. Ghostty restores the cursor unconditionally when 1049 is disabled, and restoring with nothing saved homes it to 0,0. Most restarts never touch the alternate screen, so 1049l would have put the prompt on top of the restored output every time. 1047's switch is skipped when the screen does not change. ESC[r homes the cursor, so it is bracketed in DECSC/DECRC. The charset and origin resets come before the save, because DECRC restores both from it. None of it runs when a program survived the restart and still owns the terminal. Resetting a live vim's alt screen would be as wrong as killing its mouse.
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
When Programa restarts, the terminal comes back wrong. Two ways people have seen it:
35;16;54M35;33;53M...Both are the same bug. On restart we replay saved scrollback into a brand new terminal. Those saved bytes are a raw tail of live output, so if a program was killed mid-run its "turn mouse reporting on" is in there but its "turn it back off" never happened. Replaying switches the mode on for good.
Mouse reporting is the one you can see. The rest wreck the layout, and that is where the split between people comes from: resizing the window clears the scrolling region but not the alternate screen, the wraparound flag or the charset. tmux leans on the scrolling region, so tmux users drag the window and it fixes itself. If your vim or htop died on the alternate screen, you can resize all day and nothing happens.
Two more problems found along the way:
Nothing resets if a program survived the restart and is still using the terminal. Turning off a live vim's alt screen would be just as broken as turning off its mouse.
Review order
Sources/SessionPersistence.swift- the reset sequence. Worth reading the comments, the order of those bytes is the fixSources/TerminalSurface.swift- size the terminal before replaying, and the check for whether a program is still using itSources/GhosttyNSView.swift- watch for the displays changingprogramaTests/SessionPersistenceTests.swift- the testsTwo commits on purpose: the tests land first and go red, the fix turns them green.
Two things in here that nearly shipped as new bugs
Both found in review, both worth a second pair of eyes:
1049lto leave the alternate screen. Ghostty restores the cursor whenever 1049 is switched off, and restoring with nothing saved sends it to the top left. Most restarts never touch the alternate screen, so that would have dropped the prompt on top of your restored scrollback every single time. Uses1047lnow, which does nothing when there is no alternate screen to leave.ESC[rmoves the cursor too. It is wrapped in a save and restore now, with the charset and origin cleaned up first, because the restore puts those back the way they were saved.Test plan
35;16;54MjunkNot verified
I have not reproduced any of this end to end. Tests run on CI, not locally, and the tests here only cover the fresh shell path because the other one needs a live terminal. The external monitor fix is reasoned from the source, not from a repro, so that last test plan item is the one that matters most.