detached: a big screen can attach, and the frame it asks for fits the wire

Two defects, and either fix alone makes the other one worse.

No frontend ever clamped its window to the protocol's grid ceiling — the hello
carried it raw — so a 4K display at a small font, already past max_rows 128,
had its geometry refused by the session's decoder as BadValue. That path
answers with close(.protocol) and no refuse behind it, so the frontend was told
only that the session "hung up on the connect": at a session with all 32 slots
free. client.zig now asks for the largest grid the wire carries, which is what
its own GEOMETRY note already promises a frontend gets — the session is drawn
at its own size in the corner of a bigger window, exactly as when another
frontend is the smaller one. A ZERO geometry is dropped rather than clamped,
because the session grid is the smallest common one and a frontend reporting 1
would collapse everybody else; TIOCGWINSZ answers 0x0 during a teardown and the
tty shell forwarded it, which was the same mute hangup by another route.

That clamp alone would have replaced one bug with a worse one. max_cols *
max_rows is 65536 and a run's length prefix is a u16, so the single grid legal
at both bounds is the one grid whose full frame — and an attach always produces
a full frame — cannot be described by one run. encodeFrame's @intCast panicked
in a safe build and was illegal behaviour in a fast one. The encoder splits the
run instead, bounding the CURSOR rather than the run because the gap lookahead
runs ahead of it, and frameBound had already paid for the extra header.

wire.version 1 -> 2 for the same reason: the geometry a v2 frontend now asks
for is one a v1 daemon panics encoding, and `zig build` replacing the binary
under a running session is exactly what that field exists for. A v1 daemon
answers Refusal.version instead of dying with every pane shell it owns.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_016Q4RATpafkwahrovHQLKRf

1 files changed, 90 insertions, 5 deletions

diff --git a/src/detached/wire.zig b/src/detached/wire.zig
index 23a85158..8281fc63 100644
--- a/src/detached/wire.zig
+++ b/src/detached/wire.zig
@@ -84,7 +84,17 @@ const pardes = @import("../pardes.zig");
 /// machine — `zig build` replaces the binary under a running session — and a
 /// frontend decoding another version's frame layout would paint garbage and
 /// blame the terminal.
-pub const version: u16 = 1;
+///
+/// 2: the layout did not move, but what a frontend is ALLOWED TO ASK FOR did.
+/// A frontend now clamps its window to `max_cols` x `max_rows` instead of
+/// sending it raw (client.zig), and a 512x128 grid is a full frame a v1 daemon
+/// PANICS encoding — its run length overflowed a u16 by exactly one cell, see
+/// `run_max`. A v1 session refused that geometry outright, so nothing was ever
+/// lost by refusing the connection instead; a v1 daemon meeting a v2 frontend
+/// answers `Refusal.version`, which says so, rather than dying with every pane
+/// shell it owns. This is the case the paragraph above is about: `zig build`
+/// replaces the binary under a running session.
+pub const version: u16 = 2;
 
 pub const Error = error{
     /// The message ended inside a field.
@@ -111,9 +121,17 @@ pub const Error = error{
 /// The largest grid this protocol carries. `Surface.cols`/`rows` are u16, so
 /// these are protocol bounds rather than type bounds, and they exist because
 /// `max_payload` below is derived from them: a decoder that accepts 65535
-/// columns accepts a 25 GiB frame prefix. A 4K display at a 6-pixel font is
-/// about 340 columns and 110 rows, so this is roughly 1.5x the largest grid
-/// any real terminal has, and the board's own is 56x14.
+/// columns accepts a 25 GiB frame prefix. The board's own grid is 56x14 and a
+/// terminal's is usually near 200x50.
+///
+/// NOT a ceiling above every real display, which is what this comment used to
+/// claim: a 4K window at the SDL shell's minimum 8-pixel font is around 768
+/// columns by 216 rows, and a tty on the same screen passes 128 rows at any
+/// ordinary line height. Those windows attach at 512x128 and letterbox the
+/// rest (client.zig clamps), rather than being refused as they were. Raising
+/// the pair instead would have been a bigger change than it looks: `frameBound`
+/// stays well inside `max_payload`, but a themed full frame at 512x128 is
+/// already ~0.9 MiB against server.zig's 1 MiB `out_backlog`.
 pub const max_cols: u16 = 512;
 pub const max_rows: u16 = 128;
 
@@ -127,6 +145,24 @@ const cell_max = 1 + 1 + 7 + 4 + 4 + 1 + 1 + 1;
 /// encoder coalesces against (see `encodeFrame`).
 const run_header = 4 + 2;
 
+/// ...and the longest run that `count:u16` can describe, which is EXACTLY ONE
+/// SHORT of the largest grid this protocol carries: `max_cols * max_rows` is
+/// 512*128 = 65536, and `maxInt(u16)` is 65535.
+///
+/// A full frame of that grid is ONE run over all of it whenever the theme has
+/// a background: `render` fills the surface and every pane then repaints its
+/// text area, and `Surface.set` clears `default`, so `sendCell`'s `!default`
+/// holds for every cell. (Under a theme with `bg = null` — `dark` — untouched
+/// body cells stay default and a stretch of six of them breaks the run, so the
+/// overflow was theme-dependent as well as geometry-dependent, which is the
+/// worst kind of latent.) `encodeFrame`'s `@intCast(run_end - start)` then
+/// panicked in a safe build and was illegal behaviour in a fast one — LLVM
+/// happens to truncate to zero, which the far side refuses as `BadValue`, but
+/// nothing promises that. That grid is what a frontend with a big window now
+/// asks for (client.zig clamps to it), so the meeting point went from
+/// unreachable to routine, and the encoder splits the run instead.
+const run_max = std.math.maxInt(u16);
+
 /// `kind:u8 + cols:u16 + rows:u16 + cursor(6) + nruns:u32`.
 const frame_head = 1 + 2 + 2 + 6 + 4;
 
@@ -791,7 +827,11 @@ pub fn encodeFrame(
         const start = i;
         var run_end = i + 1;
         i += 1;
-        while (i < cells.len) {
+        // `i - start` and not `run_end - start`, because the gap lookahead
+        // below moves `i` ahead of `run_end` by up to `run_header - 1` before
+        // the next iteration adopts it: bounding the cursor bounds the run,
+        // and bounding the run afterwards would not.
+        while (i < cells.len and i - start < run_max) {
             if (sendCell(cells, prev, full, i)) {
                 run_end = i + 1;
                 i += 1;
@@ -1382,6 +1422,51 @@ test "detached wire: a full frame carries the grid, a diff carries the change" {
     }
 }
 
+test "detached wire: the largest grid is one cell past a run, and survives it" {
+    // `max_cols * max_rows` is 65536 and a run's `count` is a u16, so THE ONE
+    // grid this protocol calls legal at both bounds is the one grid whose full
+    // frame cannot be described by a single run. It reached nobody while a
+    // frontend sent its window raw — a display that big had its hello refused
+    // before any frame was composed — and it is the ordinary case now that a
+    // frontend clamps to exactly this. In a safe build the `@intCast` panicked
+    // and took the session down; in a fast one it wrote a zero-length run the
+    // frontend answered with `BadValue`.
+    const cols = max_cols;
+    const rows = max_rows;
+    const n = @as(usize, cols) * rows;
+    const gpa = testing.allocator;
+
+    const cells = try gpa.alloc(pardes.Cell, n);
+    defer gpa.free(cells);
+    const mirror = try gpa.alloc(pardes.Cell, n);
+    defer gpa.free(mirror);
+    const buf = try gpa.alloc(u8, frameBound(cols, rows));
+    defer gpa.free(buf);
+
+    // Every cell painted, which is what `render` produces and therefore what a
+    // full frame always is: one run, if a run could be that long.
+    for (cells) |*c| c.* = .{ .text = "a".* ++ @as([6]u8, @splat(0)), .len = 1, .default = false };
+    @memset(mirror, .{});
+
+    const bytes = try encodeFrame(buf, cols, rows, null, cells, &.{});
+    const f = (try framed(bytes)).?;
+    const msg = (try decodeServer(f.tag, f.payload)).frame;
+    try testing.expectEqual(FrameKind.full, msg.kind);
+    // Split, and split as late as it can be: 65535 cells then 1.
+    try testing.expectEqual(@as(u32, 2), msg.nruns);
+    try msg.apply(mirror);
+    try expectGridEqual(cells, mirror);
+    // ...at exactly the two headers the split costs and not one byte more.
+    // `bytes.len <= frameBound(...)` would prove nothing here — the buffer IS
+    // `frameBound` bytes, so an over-run is `NoSpace` above, not a false
+    // assertion here — and what wants pinning is that the second run is the
+    // whole of the cost.
+    try testing.expectEqual(
+        @as(usize, header_len + frame_head + 2 * run_header + n * 8),
+        bytes.len,
+    );
+}
+
 test "detached wire: the diff is worth having, in bytes, on the board's grid" {
     // The numbers quoted in `encodeFrame`'s comment, asserted so the claim
     // cannot rot. 56x14 is the ESP32-P4 board's default grid (esp32p4.zig).