From e81b63106b83bce66641e8dd27d6aa348252baf1 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Thu, 3 Sep 2026 13:11:14 -0300 Subject: detached: a big screen can attach, and the frame it asks for fits the wire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_016Q4RATpafkwahrovHQLKRf --- src/detached/client.zig | 85 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 76 insertions(+), 9 deletions(-) (limited to 'src/detached/client.zig') diff --git a/src/detached/client.zig b/src/detached/client.zig index fc39164b..f317b2e2 100644 --- a/src/detached/client.zig +++ b/src/detached/client.zig @@ -156,7 +156,10 @@ pub const Client = struct { server.setNonblock(fd); var c: Client = .{ .gpa = gpa, .fd = fd }; errdefer c.deinit(); - try c.send(.{ .hello = .{ .cols = cols, .rows = rows } }); + try c.send(.{ .hello = .{ + .cols = @min(cols, wire.max_cols), + .rows = @min(rows, wire.max_rows), + } }); return c; } @@ -214,8 +217,33 @@ pub const Client = struct { /// Tell the session this frontend's window changed. Not a promise about the /// next frame: with other frontends attached the session grid is the /// smallest common one. + /// + /// Clamped, like the hello in `open`: a window past `max_cols`/`max_rows` + /// is a geometry the protocol cannot carry, and the decoder on the far end + /// answers one with `BadValue` — which `apply` turns into `close(.protocol)` + /// with no `refuse` behind it, so the frontend was told only that the + /// session "hung up on the connect". A 4K display at a small font is + /// already past `max_rows`, which made an ordinary big screen unable to + /// attach at all. Asking for the largest grid the wire carries is what this + /// file's GEOMETRY note already promises a frontend gets: the session is + /// drawn at ITS size wherever the window is bigger, exactly as it is when + /// another frontend is the smaller one. + /// + /// A ZERO is dropped rather than clamped, and that asymmetry is the whole + /// point of `getCols` refusing zero in the first place: the session grid is + /// the smallest common one, so a frontend that reported 1 would collapse + /// every other frontend to a single cell. `TIOCGWINSZ` answers 0x0 while a + /// terminal is being torn down and tty.zig forwards a `winsize` verbatim + /// (its ATTACH path already refuses a zero one, its resize path did not), + /// so this is reachable without a hostile peer — and it reached the session + /// as the same mute `close(.protocol)` the oversize geometry did. Keeping + /// the last real window is what a momentary zero means. pub fn resize(c: *Client, cols: u16, rows: u16) (Error || wire.Error)!void { - return c.send(.{ .event = .{ .resize = .{ .cols = cols, .rows = rows } } }); + if (cols == 0 or rows == 0) return; + return c.send(.{ .event = .{ .resize = .{ + .cols = @min(cols, wire.max_cols), + .rows = @min(rows, wire.max_rows), + } } }); } /// Wait up to `timeout_ms` for the session to say something, and push @@ -663,10 +691,20 @@ const Harness = struct { host.vtable.push_present.?(host.ctx, surface); } - /// Pump until this client has the message we are waiting for. Bounded, so a - /// broken transport fails a test rather than hanging the suite. + /// The round budget every `pumpUntil*` below shares. A round moves at most + /// one socket buffer, because nothing here is concurrent: the session + /// flushes until `EAGAIN`, and only then does the client read. That buffer + /// is 8 KiB on darwin (`net.local.stream.sendspace`) against linux's 208 + /// KiB, which is the same asymmetry the slow-frontend test at the bottom of + /// this file already had to say out loud — and a full frame of the largest + /// grid the protocol carries is near a megabyte, so 64 rounds is a linux- + /// only number. High enough for that frame on the smaller buffer, and still + /// a bound: a broken transport fails a test rather than hanging the suite. + const rounds = 256; + + /// Pump until this client has the message we are waiting for. fn pumpUntil(h: *Harness, c: *Client, comptime want: std.meta.Tag(wire.ServerMsg)) !wire.ServerMsg { - for (0..64) |_| { + for (0..rounds) |_| { try h.pump(); try c.wait(5); while (try c.next()) |msg| if (std.meta.activeTag(msg) == want) return msg; @@ -679,7 +717,7 @@ const Harness = struct { /// input queued (a `Look` on a directory emits a spawn) lands a frame /// later, and the frame in between legitimately says nothing. fn pumpUntilChange(h: *Harness, c: *Client) !wire.Frame { - for (0..64) |_| { + for (0..rounds) |_| { const msg = try h.pumpUntil(c, .frame); if (msg.frame.nruns > 0) return msg.frame; } @@ -691,7 +729,7 @@ const Harness = struct { /// because a resize is announced when the session settles it, which may be /// one empty frame after the pump that caused it. fn pumpUntilGrid(h: *Harness, c: *Client, cols: u16, rows: u16) !void { - for (0..64) |_| { + for (0..rounds) |_| { try h.pump(); try c.wait(5); while (try c.next()) |_| {} @@ -706,7 +744,7 @@ const Harness = struct { /// what a shared session promises is that they CONVERGE, which is what this /// waits for. fn pumpUntilSameScreen(h: *Harness, a: *Client, b: *Client) !void { - for (0..64) |_| { + for (0..rounds) |_| { try h.pump(); try a.wait(5); try b.wait(5); @@ -733,7 +771,7 @@ const Harness = struct { /// a transport that really does drop one must fail as a wrong screen, not /// as a timeout. fn pumpUntilShowsCore(h: *Harness, c: *Client) !void { - for (0..64) |_| { + for (0..rounds) |_| { _ = h.arena.reset(.retain_capacity); if (sameScreen((try h.core.render(h.arena.allocator())).cells, c.grid.items)) return; try h.pump(); @@ -835,6 +873,35 @@ test "detached session: two frontends share one screen at the smallest common gr try h.pumpUntilSameScreen(&a, &b); } +test "detached session: a window past the protocol attaches at the largest grid it carries" { + var h: Harness = undefined; + try h.init(80, 24); + defer h.deinit(); + + // A 4K display at a small font is already past `max_rows`, and until the + // clamp in `open` that hello was a geometry the session's decoder refused: + // `apply` answered `BadValue` with `close(.protocol)` and no `refuse` + // behind it, so a frontend on a big screen was told the session "hung up on + // the connect" — at a session with every slot free. It attaches now, at the + // biggest grid the wire has. + var c = try h.attach(wire.max_cols + 400, wire.max_rows + 70); + defer c.deinit(); + try testing.expectEqual(wire.max_cols, c.cols); + try testing.expectEqual(wire.max_rows, c.rows); + + // ...and the full frame that follows is the 65536-cell one whose single run + // does not fit a u16 count (wire.zig `run_max`), which is the half of this + // the clamp alone would have made universal rather than fixed. + const frame = (try h.pumpUntil(&c, .frame)).frame; + try testing.expectEqual(wire.FrameKind.full, frame.kind); + try testing.expectEqual(@as(usize, @as(usize, wire.max_cols) * wire.max_rows), c.grid.items.len); + // TWO runs and not one, which is the assertion that fails first if the + // encoder's bound goes: this grid's full frame is 65536 painted cells and + // a run counts to 65535. + try testing.expectEqual(@as(u32, 2), frame.nruns); + try h.pumpUntilShowsCore(&c); +} + test "detached session: a frontend that dies takes nothing with it" { var h: Harness = undefined; try h.init(60, 16); -- cgit v1.3