terminal/snapshot: misc bugs (#13572)

Misc bugs related to snapshotting. Nothing critical. Each backed by a
failed test w/o the change that passes with it.

src/terminal/snapshot/grid.zig

@@ -1067,20 +1067,25 @@ fn decodeGraphemes(
             break :target .{ .row = row, .cell = cell };
         };
 
-        // Always consume every declared codepoint. Invalid scalars and
-        // suffixes that exceed the native capacity are dropped
-        // independently without affecting the rest of the grid.
+        // Always consume every declared codepoint. Invalid scalars and NUL are
+        // not meaningful grapheme suffix components and are ignored. If native
+        // capacity is exhausted, remove any prefix already attached so the
+        // cell never exposes a truncated cluster.
         var accept = target != null;
         for (0..cp_count) |_| {
             const cp = try io.readInt(reader, u32);
             if (!accept) continue;
-            if (!validScalar(cp)) continue;
+            if (cp == 0 or !validScalar(cp)) continue;
 
             page.appendGrapheme(
                 target.?.row,
                 target.?.cell,
                 @intCast(cp),
             ) catch {
+                if (target.?.cell.hasGrapheme()) {
+                    page.clearGrapheme(target.?.cell);
+                    page.updateRowGraphemeFlag(target.?.row);
+                }
                 accept = false;
             };
         }
@@ -1752,8 +1757,9 @@ test "grid drops undeliverable grapheme entries" {
     // Valid entry with one invalid scalar dropped from within it.
     try io.writeInt(&writer, u16, 0);
     try io.writeInt(&writer, u16, 0);
-    try io.writeInt(&writer, u16, 3);
+    try io.writeInt(&writer, u16, 4);
     try io.writeInt(&writer, u32, 0x0301);
+    try io.writeInt(&writer, u32, 0);
     try io.writeInt(&writer, u32, 0xD800);
     try io.writeInt(&writer, u32, 0x0302);
     // Duplicate entry for the same cell is consumed and dropped.
@@ -1794,6 +1800,48 @@ test "grid drops undeliverable grapheme entries" {
     try testing.expect(!page.getRowAndCell(3, 0).cell.hasGrapheme());
 }
 
+test "grid drops a complete grapheme when capacity fails mid-cluster" {
+    const testing = std.testing;
+    var page = try TerminalPage.init(.{
+        .cols = 1,
+        .rows = 1,
+        .grapheme_bytes = 16,
+    });
+    defer page.deinit();
+    var style_remap = try StyleRemap.init(testing.allocator);
+    defer style_remap.deinit(testing.allocator);
+    var hyperlink_remap = try HyperlinkRemap.init(testing.allocator);
+    defer hyperlink_remap.deinit(testing.allocator);
+
+    var payload: [1024]u8 = undefined;
+    var writer: std.Io.Writer = .fixed(&payload);
+    try writer.writeByte(@bitCast(Row{ .cell_width = .eight }));
+    try io.writeInt(&writer, u16, 1);
+    try io.writeInt(&writer, u64, @bitCast(Cell{ .kind = 1, .content = 'x' }));
+    try io.writeInt(&writer, u32, 1);
+    try io.writeInt(&writer, u16, 0);
+    try io.writeInt(&writer, u16, 0);
+    // BitmapAllocator rounds this capacity to 64 four-codepoint chunks. The
+    // 129th suffix needs 33 new chunks while the old 32-chunk slice is still
+    // live, forcing the append's atomic replacement allocation to fail.
+    try io.writeInt(&writer, u16, 129);
+    for (0..129) |i| try io.writeInt(
+        &writer,
+        u32,
+        @intCast(0x0300 + i),
+    );
+
+    var reader: std.Io.Reader = .fixed(writer.buffered());
+    try decode(&page, &reader, &style_remap, &hyperlink_remap);
+    try page.verifyIntegrity(testing.allocator);
+
+    const cell = page.getRowAndCell(0, 0);
+    try testing.expectEqual(@as(u21, 'x'), cell.cell.codepoint());
+    try testing.expect(!cell.cell.hasGrapheme());
+    try testing.expect(!cell.row.grapheme);
+    try testing.expectEqual(@as(usize, 0), page.graphemeCount());
+}
+
 test "grid encodes rows at their narrowest width" {
     const testing = std.testing;
     var page = try TerminalPage.init(.{

src/terminal/snapshot/page.zig

@@ -418,6 +418,18 @@ fn decodePayloadBody(
         &style_remap,
         &hyperlink_remap,
     );
+
+    // A newly inserted table value starts with one reference so grid decoding
+    // can safely attach it to any number of cells. Unlike organically built
+    // pages, that initial reference does not itself represent a cell. Release
+    // it once per distinct live style after every cell reference is installed;
+    // unused entries then become dead and disappear from canonical re-encoding.
+    for (1..@as(usize, page.styles.next_id)) |raw_id| {
+        const id: TerminalStyleId = @intCast(raw_id);
+        if (page.styles.refCount(page.memory, id) > 0) {
+            page.styles.release(page.memory, id);
+        }
+    }
 }
 
 /// The fixed logical dimensions, table counts, and allocation hints at the
@@ -751,6 +763,14 @@ test "framed PAGE encode and decode a sparse native page" {
         .bg_color = .{ .palette = 42 },
     }).eql(decoded_style_b.value_ptr.*));
     try std.testing.expectEqual(null, style_it.next());
+    try std.testing.expectEqual(
+        @as(u16, 2),
+        decoded.styles.refCount(decoded.memory, decoded_style_a.id),
+    );
+    try std.testing.expectEqual(
+        @as(u16, 1),
+        decoded.styles.refCount(decoded.memory, decoded_style_b.id),
+    );
 
     var hyperlink_it = decoded.hyperlink_set.iterator(decoded.memory);
     const decoded_link_a = hyperlink_it.next().?;
@@ -914,7 +934,7 @@ test "decode sparse page rejects every truncation" {
 
 test "decode accepts unordered sparse style IDs and ignores zero" {
     const header: Header = .{
-        .columns = 1,
+        .columns = 2,
         .rows = 1,
         .style_count = 2,
         .hyperlink_count = 0,
@@ -927,7 +947,7 @@ test "decode accepts unordered sparse style IDs and ignores zero" {
     var descending: [
         Header.len +
             2 * (2 + style.len) +
-            7
+            3 + 2 * 8 + 4
     ]u8 = undefined;
     var descending_writer: std.Io.Writer = .fixed(&descending);
     try header.encode(&descending_writer);
@@ -935,8 +955,16 @@ test "decode accepts unordered sparse style IDs and ignores zero" {
     try style.encode(.{ .flags = .{ .bold = true } }, &descending_writer);
     try io.writeInt(&descending_writer, TerminalStyleId, 2);
     try style.encode(.{ .flags = .{ .italic = true } }, &descending_writer);
-    try descending_writer.writeByte(0); // row flags
-    try io.writeInt(&descending_writer, u16, 0); // cell count
+    try descending_writer.writeByte(@bitCast(grid.Row{ .cell_width = .eight }));
+    try io.writeInt(&descending_writer, u16, 2); // cell count
+    try io.writeInt(&descending_writer, u64, @bitCast(grid.Cell{
+        .content = 'A',
+        .style_id = 3,
+    }));
+    try io.writeInt(&descending_writer, u64, @bitCast(grid.Cell{
+        .content = 'B',
+        .style_id = 2,
+    }));
     try io.writeInt(&descending_writer, u32, 0); // grapheme section
 
     var descending_reader: std.Io.Reader = .fixed(
@@ -948,6 +976,14 @@ test "decode accepts unordered sparse style IDs and ignores zero" {
     );
     defer decoded_descending.deinit();
     try std.testing.expectEqual(@as(usize, 2), decoded_descending.styles.count());
+    try std.testing.expectEqual(
+        @as(TerminalStyleId, 1),
+        decoded_descending.getRowAndCell(0, 0).cell.style_id,
+    );
+    try std.testing.expectEqual(
+        @as(TerminalStyleId, 2),
+        decoded_descending.getRowAndCell(1, 0).cell.style_id,
+    );
 
     const one_header: Header = .{
         .columns = 1,
@@ -1362,6 +1398,45 @@ test "decode normalizes duplicate default and invalid style entries" {
     );
 }
 
+test "decode releases unused style table references" {
+    const header: Header = .{
+        .columns = 1,
+        .rows = 1,
+        .style_count = 1,
+        .hyperlink_count = 0,
+        .style_capacity = 8,
+        .hyperlink_capacity_bytes = 0,
+        .grapheme_capacity_bytes = 0,
+        .string_capacity_bytes = 0,
+    };
+
+    var empty_page = try TerminalPage.init(.{ .cols = 1, .rows = 1 });
+    defer empty_page.deinit();
+    var grid_bytes: [32]u8 = undefined;
+    var grid_writer: std.Io.Writer = .fixed(&grid_bytes);
+    try grid.encode(&empty_page, &grid_writer);
+
+    var encoded: std.Io.Writer.Allocating = .init(std.testing.allocator);
+    defer encoded.deinit();
+    try header.encode(&encoded.writer);
+    try io.writeInt(&encoded.writer, TerminalStyleId, 7);
+    try style.encode(.{ .flags = .{ .bold = true } }, &encoded.writer);
+    try encoded.writer.writeAll(grid_writer.buffered());
+
+    var reader: std.Io.Reader = .fixed(encoded.written());
+    var decoded = try decodePayload(&reader, std.testing.allocator);
+    defer decoded.deinit();
+    try std.testing.expectEqual(@as(usize, 0), decoded.styles.count());
+
+    // The dead table-only entry is absent from the very first re-encode.
+    var canonical: [64]u8 = undefined;
+    var canonical_writer: std.Io.Writer = .fixed(&canonical);
+    try encodePayload(&decoded, &canonical_writer);
+    var canonical_reader: std.Io.Reader = .fixed(canonical_writer.buffered());
+    const canonical_header = try Header.decode(&canonical_reader);
+    try std.testing.expectEqual(@as(u16, 0), canonical_header.style_count);
+}
+
 test "decode reuses duplicate hyperlinks" {
     const header: Header = .{
         .columns = 1,

src/terminal/snapshot/screen.zig

@@ -233,7 +233,7 @@ const PayloadEncodeError = hyperlink.EncodeError || error{
 };
 
 /// Errors possible while encoding a SCREEN and its complete PAGE sequence.
-pub const EncodeError = PayloadEncodeError || page.EncodeError || error{
+pub const EncodeError = Allocator.Error || PayloadEncodeError || page.EncodeError || error{
     /// The active area spans more pages than the SCREEN header can declare.
     PageCountOverflow,
 };
@@ -273,13 +273,14 @@ pub fn encode(
         try destination.finish();
     }
 
-    // PageList never compresses the active-boundary page or any later page.
-    // Encoding this resident suffix therefore does not restore cold history or
-    // otherwise mutate the source screen.
+    // Active pages are resident today, but use the representation-safe access
+    // path so a future PageList compression-policy change cannot turn this
+    // wire encoder's optimization invariant into undefined behavior.
     node = first;
     while (node) |current| : (node = current.next) {
-        std.debug.assert(current.pageIfResident() != null);
-        try page.encode(current.pageAssumeResident(), destination);
+        var preserved = try current.pagePreservingState(screen.alloc);
+        defer preserved.deinit();
+        try page.encode(preserved.page(), destination);
     }
 }
 
@@ -411,7 +412,7 @@ pub fn decode(
                 .y = y,
                 .cursor_style = header.cursor_style,
                 .pending_wrap = header.cursor_flags.pending_wrap and
-                    x == options.cols - 1,
+                    x == row_pin.node.cols() - 1,
                 .protected = header.cursor_flags.protected,
                 .style = header.cursor_pen,
                 .hyperlink_implicit_id = header.hyperlink_implicit_id,
@@ -1963,6 +1964,62 @@ test "SCREEN encodes the minimal complete-page active suffix" {
     try std.testing.expectError(error.EndOfStream, restore_source.takeByte());
 }
 
+test "SCREEN encoding preserves a compressed suffix page" {
+    const testing = std.testing;
+    const compression = @import("../compress.zig");
+
+    var screen = try TerminalScreen.init(
+        testing.io,
+        testing.allocator,
+        .{ .cols = 8, .rows = 2, .max_scrollback_bytes = 0 },
+    );
+    defer screen.deinit();
+    screen.pages.getCell(.{ .active = .{} }).?.cell.* = .init('A');
+
+    // Force the active suffix into the representation that current PageList
+    // policy normally reserves for history. This models a future policy change
+    // and makes pageAssumeResident an invalid tagged-union access.
+    const node = screen.pages.getTopLeft(.active).node;
+    const resident = node.pageAssumeResident();
+    const scratch = try testing.allocator.alloc(
+        u8,
+        try compression.Page.requiredScratch(resident.memory.len),
+    );
+    defer testing.allocator.free(scratch);
+    var table: compression.lz4.HashTable = undefined;
+    const compressed = (try compression.Page.init(
+        testing.allocator,
+        resident,
+        scratch,
+        &table,
+    )).?;
+    node.data = .{ .compressed = compressed };
+    try testing.expectEqual(.compressed, node.storage());
+
+    var destination: std.Io.Writer.Allocating = .init(testing.allocator);
+    defer destination.deinit();
+    var stream: record.Writer = .init(testing.allocator, &destination.writer);
+    defer stream.deinit();
+    try encode(&screen, .primary, &stream);
+
+    // Encoding borrows or clones through PreservedPage and never changes the
+    // source node's storage representation.
+    try testing.expectEqual(.compressed, node.storage());
+
+    var source: std.Io.Reader = .fixed(destination.written());
+    var decoded = try decode(
+        &source,
+        testing.io,
+        testing.allocator,
+        .{ .cols = 8, .rows = 2, .max_scrollback_bytes = 0 },
+    );
+    defer decoded.deinit();
+    try testing.expectEqual(
+        @as(u21, 'A'),
+        decoded.screen.pages.getCell(.{ .active = .{} }).?.cell.codepoint(),
+    );
+}
+
 test "SCREEN restoration normalizes invalid cursor positions" {
     var screen = try TerminalScreen.init(
         std.testing.io,
@@ -2048,6 +2105,58 @@ test "SCREEN restoration normalizes invalid cursor positions" {
     }
 }
 
+test "SCREEN validates pending wrap against a mixed-width cursor page" {
+    var screen = try TerminalScreen.init(
+        std.testing.io,
+        std.testing.allocator,
+        .{ .cols = 8, .rows = 1, .max_scrollback_bytes = 0 },
+    );
+    defer screen.deinit();
+
+    // Model a lazily reflowed active page that is narrower than the terminal.
+    // Column three is its physical final column even though it is not column
+    // seven of the current terminal dimensions.
+    var narrow_page = try terminal_page.Page.init(.{ .cols = 4, .rows = 1 });
+    defer narrow_page.deinit();
+
+    var destination: std.Io.Writer.Allocating = .init(
+        std.testing.allocator,
+    );
+    defer destination.deinit();
+    var stream: record.Writer = .init(
+        std.testing.allocator,
+        &destination.writer,
+    );
+    defer stream.deinit();
+
+    var header = Header.init(&screen, .primary, 1);
+    header.cursor_x = 3;
+    header.cursor_y = 0;
+    header.cursor_flags.pending_wrap = true;
+    header.saved_cursor_present = false;
+
+    const screen_payload = stream.begin(.screen);
+    errdefer stream.cancel();
+    try header.encode(screen_payload);
+    try screen_payload.writeByte(0);
+    try stream.finish();
+    try page.encode(&narrow_page, &stream);
+
+    var source: std.Io.Reader = .fixed(destination.written());
+    var decoded = try decode(
+        &source,
+        std.testing.io,
+        std.testing.allocator,
+        .{ .cols = 8, .rows = 1, .max_scrollback_bytes = 0 },
+    );
+    defer decoded.deinit();
+
+    try std.testing.expectEqual(@as(u16, 4), decoded.screen.cursor.page_pin.node.cols());
+    try std.testing.expectEqual(@as(u16, 3), decoded.screen.cursor.x);
+    try std.testing.expect(decoded.screen.cursor.pending_wrap);
+    decoded.screen.assertIntegrity();
+}
+
 test "SCREEN restoration rejects invalid and incomplete sequences" {
     var screen = try TerminalScreen.init(
         std.testing.io,

src/terminal/snapshot/snapshot.zig

@@ -413,6 +413,12 @@ pub const Decoder = struct {
         page.DecodeError ||
         Allocator.Error ||
         error{
+            /// `next` was called before `ready` completed successfully.
+            DecoderNotReady,
+
+            /// A prior `next` call failed and invalidated the stream position.
+            DecoderFailed,
+
             /// A HISTORY names a key not declared by TERMINAL.
             UnexpectedHistoryKey,
 
@@ -457,7 +463,8 @@ pub const Decoder = struct {
         switch (self.state) {
             // `ready` must succeed before history exists to decode, and a
             // failed decoder no longer knows its stream position.
-            .start, .failed => unreachable,
+            .start => return error.DecoderNotReady,
+            .failed => return error.DecoderFailed,
             .finished => return null,
             .history => {},
         }
@@ -1774,6 +1781,23 @@ test "incremental decode restores a renderable terminal at READY" {
     try testing.expectEqualStrings(&test_complete_fixture, reencoded.written());
 }
 
+test "incremental next reports invalid decoder states" {
+    const testing = std.testing;
+    var source: std.Io.Reader = .fixed("");
+    var decoder: Decoder = .init(&source);
+    var terminal_value = try Terminal.init(
+        testing.io,
+        testing.allocator,
+        .{ .cols = 1, .rows = 1 },
+    );
+    defer terminal_value.deinit(testing.allocator);
+
+    try testing.expectError(
+        error.DecoderNotReady,
+        decoder.next(testing.allocator, &terminal_value),
+    );
+}
+
 test "incremental decode applies history below live PTY output" {
     const testing = std.testing;
 
@@ -2065,6 +2089,10 @@ test "incremental decode failure leaves applied history usable" {
         error.EndOfStream,
         decoder.next(testing.allocator, &restored),
     );
+    try testing.expectError(
+        error.DecoderFailed,
+        decoder.next(testing.allocator, &restored),
+    );
 
     // The transferred terminal was never owned by the decoder: it remains
     // valid with the contiguous history prefix that did apply, and only