From b91d513720bbd72aeb1d797c3fd9ba1e23317160 Mon Sep 17 00:00:00 2001 From: zzstoatzz Date: Tue, 18 Aug 2026 01:27:49 -0500 Subject: [PATCH] release: v0.4.3 mst: collectBlocks emits empty-node blocks instead of stranding them. An empty node and an unloaded stub are structurally identical once nodeCid has serialized and cleaned the node, so collectNodeBlocks skipped it and the block was never stored. Content addressing tells them apart: only an empty node hashes to the empty-node cid. Found in production on pds.zat.dev, where two repos carried an empty subtree node minted by a pre-0.3.19 writer; the first walk to reach it returned PartialTree and took all writes down. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 23 +++++++ build.zig.zon | 2 +- src/internal/repo/mst.zig | 131 +++++++++++++++++++++++++++++++++++++- 3 files changed, 154 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfccf3c..50ab8d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,28 @@ # changelog +## 0.4.3 + +- **fix**: `Mst.collectBlocks` now emits the block for an empty node instead of + skipping it. `collectNodeBlocks` bails on `isUnloadedStub`, and after `nodeCid` + serializes a node it clears `dirty` and caches the encoding without emitting — + so a genuinely empty node ends up not-dirty, cid-set, no left, no entries, which + is byte-for-byte the shape of an unloaded stub. Skipping a stub is right (its + block is already stored); skipping an empty node strands it, because nothing else + ever writes one. Content addressing separates the two: only an empty node hashes + to the empty-node cid. Ordinary nodes were never at risk — they reach the emit + path and reuse the cached encoding, which is what that cache is for. Empty nodes + are the one shape that cannot, and the same stub/empty ambiguity `0.3.19` guarded + in the delete path was still open here. Found in production: two repos on + pds.zat.dev carried an empty subtree node minted by a pre-`0.3.19` writer, its + block was never stored, and the first tree walk that reached it returned + `PartialTree` — every `createRecord` 500, `getRepo` 404, for any collection. The + fix is self-healing: `nodeCid` returns a stub's cid without loading it, so a write + that does not descend into the stranded branch still commits, and the collect pass + puts the missing block back. New coverage: the collect-closure invariant (every + node cid a collected block points at is itself collected) and the production + repair path (store missing the block, lazy load, write, assert it returns). See + `docs/incident-2026-08-18-empty-mst-node.md` in zds. + ## 0.4.2 - `zat.cbor` accepts finite 64-bit floats, matching the reference diff --git a/build.zig.zon b/build.zig.zon index 415140f..d547021 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -1,6 +1,6 @@ .{ .name = .zat, - .version = "0.4.2", + .version = "0.4.3", .fingerprint = 0x8da9db57ee82fbe4, .minimum_zig_version = "0.16.0-dev.3070+b22eb176b", .dependencies = .{ diff --git a/src/internal/repo/mst.zig b/src/internal/repo/mst.zig index 5a60887..5f40ff7 100644 --- a/src/internal/repo/mst.zig +++ b/src/internal/repo/mst.zig @@ -516,7 +516,24 @@ pub const Mst = struct { } fn collectNodeBlocks(self: *Mst, node: *Node, gpa: Allocator, out: *std.ArrayList(car.Block)) MstError!void { - if (isUnloadedStub(node)) return; + if (isUnloadedStub(node)) { + // A stub and a serialized empty node are structurally identical (see + // `isUnloadedStub`), so emptiness cannot be judged by inspection. Content + // addressing separates them: only the empty node hashes to the empty-node + // cid. Skipping a stub is right — its block is already in the store — but + // skipping an empty node strands it, because nothing else ever writes one. + // Emitting it costs a 7-byte encode and heals a tree that inherited a + // stranded node from a writer that predates `pruneIfEmpty`. + const cid = node.cid.?; + const encoded = try self.serializeEmptyNode(); + const empty_cid = try cbor.Cid.forDagCbor(self.allocator, encoded); + if (!std.mem.eql(u8, cid.raw, empty_cid.raw)) { + self.allocator.free(encoded); + return; + } + try out.append(gpa, .{ .cid_raw = cid.raw, .data = encoded }); + return; + } const loaded = self.ensureNodeLoaded(node) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, @@ -2311,3 +2328,115 @@ test "collectBlocks emits consistent (cid, data) pairs across repeated commit cy } } } + +test "collectBlocks emits a stranded empty node inherited from a pre-prune writer" { + const alloc = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(alloc); + defer arena.deinit(); + const a = arena.allocator(); + + // Rebuild the shape zat <= v0.3.18 produced: deleting the sole occupant of a + // subtree left the emptied node in place instead of dropping the pointer. + // See docs/incident-2026-08-18-empty-mst-node.md in zds. + const parent_key = "com.atproto.lexicon.schema/place.birds.speciesPhoto"; + const child_key = "place.birds.sighting/3msvg56t43skv"; + try std.testing.expectEqual(@as(u32, 1), keyHeight(parent_key)); + try std.testing.expectEqual(@as(u32, 0), keyHeight(child_key)); + + const value = try cbor.Cid.forDagCbor(a, "record"); + + var tree = Mst.init(a); + try tree.put(parent_key, value); + try tree.put(child_key, value); + + // strand the subtree the way the old delete did: empty it, but keep the pointer + const root = tree.root.?; + const idx = Mst.entryLowerBound(root.entries.items, parent_key); + const stranded = root.entries.items[idx].right.?; + stranded.entries.clearRetainingCapacity(); + stranded.left = null; + const empty_encoded = try tree.serializeEmptyNode(); + stranded.cid = try cbor.Cid.forDagCbor(a, empty_encoded); + stranded.encoded = null; + stranded.dirty = false; + root.dirty = true; + + var blocks: std.ArrayList(car.Block) = .empty; + defer blocks.deinit(a); + _ = try tree.rootCid(); + try tree.collectBlocksInto(a, &blocks); + + // the closure invariant: every MST node cid a collected block points at must + // itself be collected, or the tree references a block nobody stored + var have: std.StringHashMapUnmanaged(void) = .empty; + defer have.deinit(a); + for (blocks.items) |b| try have.put(a, b.cid_raw, {}); + + for (blocks.items) |b| { + const nd = decodeMstNode(a, b.data) catch continue; + if (nd.left) |l| try std.testing.expect(have.contains(l)); + for (nd.entries) |e| if (e.tree) |t| try std.testing.expect(have.contains(t)); + } + + // and specifically: the stranded empty node's block is emitted + try std.testing.expect(have.contains(stranded.cid.?.raw)); +} + +test "a lazily loaded tree heals a stranded empty node whose block is absent" { + const alloc = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(alloc); + defer arena.deinit(); + const a = arena.allocator(); + + // the production repair path: the repo's store is missing the empty node's + // block entirely (that is the outage), and the next write must put it back. + const parent_key = "com.atproto.lexicon.schema/place.birds.speciesPhoto"; + const child_key = "place.birds.sighting/3msvg56t43skv"; + const value = try cbor.Cid.forDagCbor(a, "record"); + + var staged = Mst.init(a); + try staged.put(parent_key, value); + try staged.put(child_key, value); + const root = staged.root.?; + const idx = Mst.entryLowerBound(root.entries.items, parent_key); + const stranded = root.entries.items[idx].right.?; + stranded.entries.clearRetainingCapacity(); + stranded.left = null; + const empty_encoded = try staged.serializeEmptyNode(); + const empty_cid = try cbor.Cid.forDagCbor(a, empty_encoded); + stranded.cid = empty_cid; + stranded.encoded = null; + stranded.dirty = false; + root.dirty = true; + + // store every block EXCEPT the empty node's, exactly as production looked + var store = TestBlockStore{}; + var staged_blocks: std.ArrayList(car.Block) = .empty; + defer staged_blocks.deinit(a); + const staged_root = try staged.rootCid(); + try staged.collectBlocksInto(a, &staged_blocks); + for (staged_blocks.items) |b| { + if (std.mem.eql(u8, b.cid_raw, empty_cid.raw)) continue; + try store.blocks.put(a, b.cid_raw, b.data); + } + try std.testing.expect(store.blocks.get(empty_cid.raw) == null); + + // a write that does not descend into the stranded branch must still succeed... + var tree = try Mst.loadLazy(a, staged_root.raw, store.reader()); + try tree.put("app.bsky.feed.post/3lsync00001zz", value); + + var blocks: std.ArrayList(car.Block) = .empty; + defer blocks.deinit(a); + _ = try tree.rootCid(); + try tree.collectBlocksInto(a, &blocks); + + // ...and must re-emit the stranded block, healing the store + var healed = false; + for (blocks.items) |b| { + if (std.mem.eql(u8, b.cid_raw, empty_cid.raw)) { + try std.testing.expectEqualSlices(u8, empty_encoded, b.data); + healed = true; + } + } + try std.testing.expect(healed); +} -- 2.51.2