a73x

94b89d3d

refactor: the replica measures the grid it holds, and a delta paint holds no row list

a73x   2026-09-04 18:04

Commit message
refactor: the replica measures the grid it holds, and a delta paint holds no row list

Two self-review findings.

The replica compared a snapshot's prefix against a size it copied at init,
so a grid resized by anyone else was written past its end: watched panicking
with an index-out-of-bounds on the first row. The grid's own dimensions are
the comparison now, and the remembered copy is gone — every reader of the
authoritative size already reads the grid.

The delta paint collected row indices into a fixed stack array and silently
dropped the surplus. It walks the frame once instead, painting each row from
the grid as it goes; the frame is read for the indices it names and nothing
else, because the cells it carried are already in the grid.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CsWfuJFQbTfGtKZLS5qw4q
src/engine/replica.zig
Old New
@@ -15,12 +15,12 @@ const proto = @import("protocol.zig");
15 15
16 pub const Replica = struct { 16 pub const Replica = struct {
17 alloc: std.mem.Allocator, 17 alloc: std.mem.Allocator,
18 /// Borrowed. The grid the frames are copied into. 18 /// Borrowed. The grid the frames are copied into, and the authoritative
19 /// size: under latest-wins another client's attach or resize can make it
20 /// differ from any local tty, and the replica follows the grid. Its own
21 /// dimensions are what a snapshot's prefix is compared against, so a grid
22 /// resized by anyone else is still one this can be indexed with.
19 grid: *Grid, 23 grid: *Grid,
20 /// The authoritative grid size, learned from snapshot prefixes. Under
21 /// latest-wins another client's attach or resize can make it differ
22 /// from any local tty; the replica follows the grid, not the tty.
23 grid_size: proto.Size,
24 /// The daemon instance we are talking to, learned from its snapshots, 24 /// The daemon instance we are talking to, learned from its snapshots,
25 /// and quoted back on reconnect so the daemon can tell whether the seq 25 /// and quoted back on reconnect so the daemon can tell whether the seq
26 /// we hold is one of its own (a restarted daemon counts from zero over 26 /// we hold is one of its own (a restarted daemon counts from zero over
@@ -48,11 +48,7 @@ pub const Replica = struct {
48 }; 48 };
49 49
50 pub fn init(alloc: std.mem.Allocator, g: *Grid) Replica { 50 pub fn init(alloc: std.mem.Allocator, g: *Grid) Replica {
51 return .{ 51 return .{ .alloc = alloc, .grid = g };
52 .alloc = alloc,
53 .grid = g,
54 .grid_size = .{ .cols = g.cols, .rows = g.rows },
55 };
56 } 52 }
57 53
58 /// Consume one replay frame; only `.snapshot` and `.delta` are replay 54 /// Consume one replay frame; only `.snapshot` and `.delta` are replay
@@ -82,10 +78,8 @@ pub const Replica = struct {
82 self.session_epoch = prefix.epoch; 78 self.session_epoch = prefix.epoch;
83 self.last_seq = prefix.seq; 79 self.last_seq = prefix.seq;
84 self.history_rows = prefix.history_rows; 80 self.history_rows = prefix.history_rows;
85 if (prefix.cols != self.grid_size.cols or prefix.rows != self.grid_size.rows) { 81 if (prefix.cols != self.grid.cols or prefix.rows != self.grid.rows)
86 try self.grid.resize(prefix.cols, prefix.rows); 82 try self.grid.resize(prefix.cols, prefix.rows);
87 self.grid_size = .{ .cols = prefix.cols, .rows = prefix.rows };
88 }
89 // Cleared, then every row written: a snapshot is the whole 83 // Cleared, then every row written: a snapshot is the whole
90 // screen, and a row the payload happens not to reach must not 84 // screen, and a row the payload happens not to reach must not
91 // keep what the previous grid had there. 85 // keep what the previous grid had there.
@@ -261,12 +255,38 @@ test "snapshot at a new grid size resizes the replica grid first" {
261 defer alloc.free(payload); 255 defer alloc.free(payload);
262 256
263 try std.testing.expectEqual(Replica.Applied.painted, try r.apply(.snapshot, payload)); 257 try std.testing.expectEqual(Replica.Applied.painted, try r.apply(.snapshot, payload));
264 try std.testing.expectEqual(@as(u16, 100), r.grid_size.cols);
265 try std.testing.expectEqual(@as(u16, 30), r.grid_size.rows);
266 try std.testing.expectEqual(@as(u16, 100), g.cols); 258 try std.testing.expectEqual(@as(u16, 100), g.cols);
267 try std.testing.expectEqual(@as(u16, 30), g.rows); 259 try std.testing.expectEqual(@as(u16, 30), g.rows);
268 } 260 }
269 261
262 test "snapshot: a grid resized by somebody else is measured as it is, not as it was" {
263 // The size a prefix is compared against is the GRID's own, not a copy the
264 // replica took at init: a client that resized its grid for its own reasons
265 // and then took a snapshot of the size the replica remembered would skip
266 // the resize and write rows off the end of the shorter grid.
267 const alloc = std.testing.allocator;
268 const g = try Grid.init(alloc, 8, 4);
269 defer g.deinit();
270 var r = Replica.init(alloc, g);
271 try g.resize(4, 2);
272
273 const payload = try testSnapshot(alloc, .{
274 .seq = 1,
275 .history_rows = 0,
276 .cols = 8,
277 .rows = 4,
278 .epoch = 1,
279 }, .{ .x = 0, .y = 0 }, &.{ "aaaa", "bbbb", "cccc", "dddd" });
280 defer alloc.free(payload);
281
282 try std.testing.expectEqual(Replica.Applied.painted, try r.apply(.snapshot, payload));
283 try std.testing.expectEqual(@as(u16, 8), g.cols);
284 try std.testing.expectEqual(@as(u16, 4), g.rows);
285 const dump = try g.dumpPlain(alloc);
286 defer alloc.free(dump);
287 try std.testing.expectEqualStrings("aaaa\nbbbb\ncccc\ndddd", dump);
288 }
289
270 test "short snapshot proves nothing: BadPayload, state_since_attach untouched" { 290 test "short snapshot proves nothing: BadPayload, state_since_attach untouched" {
271 const alloc = std.testing.allocator; 291 const alloc = std.testing.allocator;
272 const g = try Grid.init(alloc, 80, 24); 292 const g = try Grid.init(alloc, 80, 24);
src/tui/paint.zig
Old New
@@ -270,14 +270,6 @@ pub fn paintDeltaClipped(
270 out_fd: std.posix.fd_t, 270 out_fd: std.posix.fd_t,
271 ) !void { 271 ) !void {
272 const hdr = try proto.readDeltaHeader(payload); 272 const hdr = try proto.readDeltaHeader(payload);
273 var rows_buf: [max_delta_rows]u16 = undefined;
274 var n: usize = 0;
275 var it = proto.deltaRowIterator(payload);
276 while (try it.next()) |row| {
277 if (n == rows_buf.len) break;
278 rows_buf[n] = row.row;
279 n += 1;
280 }
281 var paint: std.ArrayList(u8) = .empty; 273 var paint: std.ArrayList(u8) = .empty;
282 defer paint.deinit(alloc); 274 defer paint.deinit(alloc);
283 try paint.appendSlice(alloc, sync_begin); 275 try paint.appendSlice(alloc, sync_begin);
@@ -286,23 +278,20 @@ pub fn paintDeltaClipped(
286 const view: grid.RowView = .{ .col_off = vp.left, .cols = vp.cols }; 278 const view: grid.RowView = .{ .col_off = vp.left, .cols = vp.cols };
287 var ech_buf: [16]u8 = undefined; 279 var ech_buf: [16]u8 = undefined;
288 const ech = std.fmt.bufPrint(&ech_buf, "\x1b[{d}X", .{vp.cols}) catch ""; 280 const ech = std.fmt.bufPrint(&ech_buf, "\x1b[{d}X", .{vp.cols}) catch "";
289 for (rows_buf[0..n]) |y| { 281 // The frame is walked for the row INDICES it names and nothing else: the
290 if (y >= limit) continue; 282 // cells it carried are already in the grid, put there by the replica.
291 try appendRowAt(&paint, alloc, y, vp, ech); 283 var it = proto.deltaRowIterator(payload);
292 const row = try dumpRow(alloc, g, y, hl, view); 284 while (try it.next()) |row| {
293 defer alloc.free(row); 285 if (row.row >= limit) continue;
294 try paint.appendSlice(alloc, row); 286 try appendRowAt(&paint, alloc, row.row, vp, ech);
287 const seg = try dumpRow(alloc, g, row.row, hl, view);
288 defer alloc.free(seg);
289 try paint.appendSlice(alloc, seg);
295 } 290 }
296 291
297 try finishPaint(&paint, alloc, .{ .x = hdr.cursor_x, .y = hdr.cursor_y }, vp, out_fd); 292 try finishPaint(&paint, alloc, .{ .x = hdr.cursor_x, .y = hdr.cursor_y }, vp, out_fd);
298 } 293 }
299 294
300 /// The row indices one delta paint will hold on the stack. A frame naming
301 /// more rows than this is a frame naming more rows than any grid mux serves
302 /// has, so the surplus is dropped rather than allocated for; the replica has
303 /// already taken every row, and the next full repaint carries them.
304 const max_delta_rows = 1024;
305
306 /// An inverse status marker parked in the top-right corner: `[scroll]` when 295 /// An inverse status marker parked in the top-right corner: `[scroll]` when
307 /// viewing history, `[reconnecting]` when the transport is being rebuilt. 296 /// viewing history, `[reconnecting]` when the transport is being rebuilt.
308 /// Text only, so a caller mid-repaint can append it into its own paint 297 /// Text only, so a caller mid-repaint can append it into its own paint