Display::new_frame's scroll-detection can false-positive on indistinguishable blank rows, corrupting client-side display state

Summary

Display::new_frame()'s "did the screen just scroll?" fast path
(src/terminal/terminaldisplay.cc, roughly lines 166–250) can misfire on a
perfectly ordinary redraw that never scrolled at all, causing mosh-client to
physically scroll the real terminal by a bogus amount and mis-render content
onto the wrong row. The corruption isn't a one-frame flicker: mosh's
internal row bookkeeping is left wrong, and stays wrong — content on the
affected row is skipped on subsequent diffs — until an unrelated later full
screen clear happens to reset it.

Reproduced against mosh 1.4.0, both as: a real client/server pair over the
network, and a local loopback session (mosh-server/mosh-client on the
same machine) — same result both ways, so this isn't network-timing
specific, just timing-of-writes specific (see "why chunking matters"
below). The identical byte stream replayed directly under a real pty (no
mosh) or through a from-scratch VT100 emulator
(charmbracelet/x/vt)
renders correctly every time.

Root cause

Before diffing row by row, new_frame() tries a fast path: does the new
frame's row 0 match some row K in the previously-displayed frame? If so,
it assumes the screen scrolled up by K lines, emits a real scroll-region +
\n sequence to the physical terminal, and re-indexes its own bookkeeping
(rows.at(i) = rows.at(i + K)) instead of diffing those rows normally:

for ( int row = 0; row < f.ds.get_height(); row++ ) {
  const Row* new_row = f.get_row( 0 );
  const Row* old_row = &*rows.at( row );
  if ( !( new_row == old_row || *new_row == *old_row ) ) {
    continue;
  }
  if ( row == 0 ) { break; }
  lines_scrolled = row;
  scroll_height = 1;
  /* ... extend scroll_height while subsequent rows also match ... */

The match test is pointer identity, or (Row::operator==) a generation
counter plus cell content. Rows that are still blank because they were
never individually written to (freshly constructed, resized, or produced
by insert_line/delete_line — all of which fill multiple row slots from
one shared Row object; see Framebuffer::newrow() and its callers in
terminalframebuffer.cc) are literally the same object, so they trivially
satisfy this check. The loop takes the first row (scanning from the top)
that matches, with no minimum region size and no exclusion for
still-untouched filler rows, so it cannot distinguish "this content actually
scrolled up from row K" from "row 0 and row K both just happen to still be
blank, for unrelated reasons." A screen with more than one indistinguishable
blank region — extremely common in any TUI with sparse content (empty
scrollback, an empty status area, etc.) — is enough to produce a false
match.

Once that fires, mosh has physically scrolled the real terminal by an
amount that doesn't correspond to anything that happened, and re-indexed
its own model of "what's on screen" to match — so it now believes rows are
already correctly positioned when they aren't, and skips redrawing them
(Display::put_row trusts the bookkeeping). That's why the corruption
persists rather than self-correcting on the very next frame.

I confirmed Row::reset() (used by Erase-in-Line/Erase-in-Display) always
assigns a fresh, unique generation counter on clear, so an explicitly
erased row can never spuriously match another this way — the false match
specifically requires never-individually-touched blank rows sharing one
Row object's identity, which is the ordinary, unavoidable result of how
mosh allocates blank rows on construction, resize, and scrolling.

Minimal reproduction (389 bytes)

minimal-repro.bin isolates the mechanism directly, independent of any
originating application. Structurally it is just:

1. ESC[H ESC[2J — clear screen (every row now blank, but individually
   reset via Erase-in-Display, each with its own unique generation — not
   yet the bug's precondition, since EL/ED never share identity).
2. A short line of unique text on row 0, and a short run of highlighted
   NBSP cells near the bottom — enough real content that some rows are
   non-blank while most of the 27-row screen (never touched by either
   write) remains blank and mutually identity-sharing.
3. A DECSTBM scroll-region change followed immediately by a bare \n,
   then ESC M (reverse-index), and a handful of small, separately-flushed
   writes to the bottom row — ordinary cost-optimized redraw traffic, no
   different in kind from what any raw-mode TUI emits.

Removing either the row-0 text or the NBSP run (keeping everything else)
stops it from reproducing — both pieces of real content are needed, we
suspect because with only one, there's no longer a second, independent
blank region at the right depth for the false match to land on. Shortening
either one also stops it, consistent with the mechanism being about exact
row/generation identity rather than content shape.

Watching row 25 specifically (which this candidate never writes to at all)
is the cleanest demonstration: it should be — and on a direct pty, or in a
from-scratch VT100 emulator, always is — blank for the entire session. Over
mosh, it briefly develops a spurious highlighted band and a stray character
partway through, then goes blank again a moment later:

Before (blank, correct)	Corruption appears	Settles blank again	Direct pty, no mosh (reference)

Reproduce with (in a 93x27 terminal):

go build -o chunkreplay chunkreplay.go
mosh-server new -- ./chunkreplay ./minimal-repro.bin
# prints: MOSH CONNECT <port> <key>
MOSH_KEY=<key> mosh-client <host> <port>   # a local loopback session reproduces it too

chunkreplay.go:

// Command chunkreplay replays a captured terminal-output file to stdout in
// small chunks with delays between them, instead of writing it all at once.
//
// Feeding mosh-server's command the whole file in one write does NOT
// reproduce the corruption below — mosh's client-side state-sync protocol
// coalesces an instantaneous write straight to the final state, skipping the
// intermediate frames where the bug is visible. It only reproduces when the
// bytes arrive as multiple separate writes over time, matching how a real
// interactive program actually produces output (many small render+flush
// cycles), not one giant write.
package main

import (
	"os"
	"time"
)

func main() {
	data, err := os.ReadFile(os.Args[1])
	if err != nil {
		panic(err)
	}
	chunk := 40
	delay := 15 * time.Millisecond
	for i := 0; i < len(data); i += chunk {
		end := i + chunk
		if end > len(data) {
			end = len(data)
		}
		os.Stdout.Write(data[i:end])
		time.Sleep(delay)
	}
}

Original, larger reproduction (from a real application)

This is where the bug was first found: zlily-sanitized.bin (1,625 bytes)
is real terminal output from an in-development TUI chat client's own
internal tap on its terminal writes, for a 93x27 session. All visible text
has been replaced with a deterministic same-length placeholder (the same
source word always maps to the same placeholder); every escape sequence is
untouched.

byte  750: r25="ukuyllulueleulueluelueluyelyuelyuelyuelueolueoxiexkjlereyueyaueyueluxluluyueyyueluylxuexyuluu"  (settled)
byte  890: r24=""     r25="  uyllulueleulueluelueluyelyuelyuelyuelueolueoxiexkjlereyueyaueyueluxluluyueyyueluylxuexyuluu"
byte  930: r25="                                       ueolueoxiexkjlereyueyaueyueluxluluyueyyueluylxuexyuluu"
byte  960: r25="                                       ␠␠␠␠␠␠␠␠␠␠␠␠␠␠␠"

At byte 750 the input text has correctly settled onto row 25, all 94
characters. Over the next ~200 bytes it's eaten away from the left,
replaced by blank space and then NBSP padding, until row 25 is completely
blank. Nothing in the source stream at that point instructs row 25 to be
touched — the NBSP padding is, byte-for-byte, the leading edge of a
different row's fill pattern, bleeding onto this one.

Before (byte 750)	Corruption starts (byte 890)	Midway (byte 930)	Wiped blank (byte 960)

Direct pty, no mosh, for comparison — row 25 settles correctly and stays
that way:

Environment

• mosh 1.4.0 on both ends (confirmed matching versions)
• Client: macOS (arm64)
• Server: Ubuntu 24.04.4 LTS, Linux 6.8.0-111-generic
• TERM=xterm-256color
• Terminal size: 93x27 (the minimal repro was also confirmed at other sizes; 93x27 is just what carried over from the original capture)

Attachments

All reproduction files are in this repo: https://github.com/joshw/mosh-scroll-bug-repro

• minimal-repro.bin — 389-byte minimal reproduction targeting the root
  cause directly (recommended starting point)
• zlily-sanitized.bin — original, larger (1,625-byte) reproduction from a
  real application
• chunkreplay.go — the replay driver (also inlined above)
• BUG_REPORT.md — this report in full, plus a byte-by-byte trace of the
  original reproduction
• screenshots for both reproductions

Start in src/terminal/terminaldisplay.cc around Display::new_frame(), then inspect Framebuffer::newrow() and its callers in terminalframebuffer.cc. Build and run chunkreplay.go with minimal-repro.bin in a 93x27 mosh session, and compare against the direct-pty behavior. Done means the reproduction no longer causes a false physical scroll or corrupts the untouched row 25 across subsequent frames.