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); +}