From 32f0b6c4a5c3902321bef31742eefaf871d6534d Mon Sep 17 00:00:00 2001 From: zzstoatzz Date: Tue, 18 Aug 2026 01:30:50 -0500 Subject: [PATCH] fix: bump zat to v0.4.3 for the stranded empty-MST-node fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit zat 0.4.3 makes collectBlocks emit the block for an empty MST node instead of mistaking it for an unloaded stub and skipping it. Two repos on this PDS carried an empty subtree node minted by a pre-0.3.19 writer whose block was never stored; the first tree walk to reach it returned PartialTree, taking every createRecord to 500 and getRepo to 404 regardless of collection. The fix is self-healing — a write that does not descend into the stranded branch still commits, and the collect pass restores the missing block. Also handles zat.cbor.Value.float, new in 0.4.2, in the CBOR -> ATProto JSON writer. Adds docs/incident-2026-08-18-empty-mst-node.md with the full investigation. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 5 +- build.zig.zon | 4 +- docs/incident-2026-08-18-empty-mst-node.md | 298 +++++++++++++++++++++ src/internal/cbor_json.zig | 24 ++ 4 files changed, 327 insertions(+), 4 deletions(-) create mode 100644 docs/incident-2026-08-18-empty-mst-node.md diff --git a/README.md b/README.md index 8558995..e652453 100644 --- a/README.md +++ b/README.md @@ -16,8 +16,9 @@ Start with the document that matches the work: - Operators: [operator guide](docs/operations.md), [production deployment](docs/deployment.md), [invite codes](docs/invite-codes.md), - [Comail](docs/comail.md), and the - [account takedown runbook](docs/account-takedown-runbook.md) + [Comail](docs/comail.md), the + [account takedown runbook](docs/account-takedown-runbook.md), and the + [2026-08-18 empty-MST-node incident](docs/incident-2026-08-18-empty-mst-node.md) - Account work: [create-account flow](docs/create-account-flow.md), [account security](docs/account-security.md), and [passkeys](docs/passkeys.md) - Protocol work: [architecture](docs/architecture.md), diff --git a/build.zig.zon b/build.zig.zon index 9d837fb..9342e7e 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -5,8 +5,8 @@ .minimum_zig_version = "0.16.0", .dependencies = .{ .zat = .{ - .url = "https://tangled.org/zat.dev/zat/archive/v0.3.29.tar.gz", - .hash = "zat-0.3.29-5PuC7vruDAD--Cox4-pdVRHS7QaLBDWjmUPlmuV1Ze9_", + .url = "https://tangled.org/zat.dev/zat/archive/v0.4.3.tar.gz", + .hash = "zat-0.4.3-5PuC7jGfDAAWck7re9WVY2M0YCionu5ymA0ewyto_Ooy", }, .zqlite = .{ .url = "git+https://github.com/karlseguin/zqlite.zig?ref=master#05a88d6758753e1c63fdd45b211dde2057094b0c", diff --git a/docs/incident-2026-08-18-empty-mst-node.md b/docs/incident-2026-08-18-empty-mst-node.md new file mode 100644 index 0000000..7a70fe9 --- /dev/null +++ b/docs/incident-2026-08-18-empty-mst-node.md @@ -0,0 +1,298 @@ +# incident: the 7-byte block that took down writes + +**2026-08-18** — every `com.atproto.repo.createRecord` on `pds.zat.dev` returned 500 for +two accounts, and `com.atproto.sync.getRepo` returned 404 for the same two. The cause was +a single missing 7-byte block, planted six days earlier by a bug that had already been +fixed upstream before it ever ran here. + +Two defects compose. Neither is dangerous alone. + +| | defect | where | status | +|---|---|---|---| +| **A** | delete leaves an emptied subtree node in the tree instead of pruning the pointer | zat ≤ v0.3.18 | fixed in [`f7d0816`](#citations), shipped v0.3.19 | +| **B** | `collectBlocks` never emits an empty node's block, so it is referenced but never stored | zat, current | **still live** | + +A *created* the bad node. B *hid* it, then turned it into an outage. + +--- + +## symptoms + +``` +error failed to serve /xrpc/com.atproto.repo.createRecord: PartialTree +``` + +- `createRecord` → 500 for **any** collection, including unknown ones +- `getRepo` → 404 `RepoNotFound` +- `listRepos`, `getLatestCommit`, `describeRepo` → **fine** + +That split is the first clue. The working endpoints read the `records` and `commits` +tables. The failing ones walk the MST. The repo metadata was intact; the tree was not. + +--- + +## the tree + +birds.place, commit `8730` → `8732`, deleting `place.birds.sighting/3msvg56t43skv` +— the sole occupant of the subtree hanging off the `speciesPhoto` entry. + +``` + BEFORE (8730) AFTER (8732, zat v0.3.10) + root, layer 1 root, layer 1 + ├── h1 app.bsky.actor.profile/self ├── h1 app.bsky.actor.profile/self + │ └── node (4 entries) │ └── node (4 entries) + ├── h1 …/place.birds.speciesPhoto ├── h1 …/place.birds.speciesPhoto + │ └── node (1 entry) │ └── {"e":[],"l":null} ← empty node, + │ └── place.birds.sighting/… │ left behind + ├── h1 sh.tangled.actor.profile/self ├── h1 sh.tangled.actor.profile/self + │ └── node (1 entry) │ └── node (1 entry) + └── h1 sh.tangled.repo/birds.place └── h1 sh.tangled.repo/birds.place + + CANONICAL: the pointer should be null, + and the node should not exist at all. +``` + +The MST is content-addressed, so a node that should not exist changes every ancestor CID. +From `8732` onward the repo served a root no other implementation would compute for that +keyset. + +--- + +## defect A — the missing prune + +zat v0.3.10, `deleteFromNode`, the descent path. The entry is removed from the child, the +parent is marked dirty, and **nothing drops the now-empty child**: + +```zig +// v0.3.10 src/internal/repo/mst.zig:389-393 +const child_ref = childAtIndex(node, entryLowerBound(node.entries.items, key)); +if (try self.ensureChildNode(child_ref)) |sub| { + const prev = try self.deleteFromNode(sub, layer - 1, key); + if (prev != null) node.dirty = true; // ← and that's all + return prev; +} +``` + +Current zat adds one line, plus a comment that states the stakes exactly: + +```zig +// HEAD src/internal/repo/mst.zig:382-386 +const prev = try self.deleteFromNode(sub, layer - 1, key); +if (prev != null) { + node.dirty = true; + pruneIfEmpty(child_ref); // ← f7d0816 +} +``` + +> Drop a subtree pointer whose node no longer holds anything. MST nodes are +> content-addressed, so an emptied node left in place serializes as a real block and +> changes every ancestor CID — the tree would no longer match the one that never +> contained the deleted key, breaking commit-proof inversion. +> +> — `pruneIfEmpty` doc comment, mst.zig:391-394 + +### the subtlety worth keeping + +`pruneIfEmpty` does **not** prune every childless node: + +```zig +// mst.zig:395-398 +fn pruneIfEmpty(child_ref: *?*Node) void { + const child = child_ref.* orelse return; + if (child.entries.items.len == 0 and child.left == null) child_ref.* = null; +} +``` + +The `and child.left == null` matters. A node with no entries but a surviving left child is +a legitimate **pass-through** that bridges a height gap; dropping it would promote the leaf +a level and produce a non-canonical root. zat tests both directions — that the emptied +subtree is pruned (mst.zig:1992) and that the height-gap pass-through is kept (mst.zig:2079). + +So the fully-empty encoding `{"e":[],"l":null}` — the 7 bytes `A2 6165 80 616C F6`, +CID `bafyreie5737gdxlw5i64vzichcalba3z2v5n6icifvx5xytvske7mr3hpm` — is canonical in +exactly one position: **the root of an empty repo**. Anywhere else it is corruption. + +--- + +## defect B — the block that never gets written + +zds derives the root CID and *then* collects blocks: + +```zig +// zds src/storage/store.zig:2140-2141 +const data_cid = try tree.rootCid(); +try writeMstBlocks(&tree, &mst_blocks); +``` + +`rootCid()` serializes the root, which recurses through `nodeCid` on every child. `nodeCid` +caches the encoding, **clears `dirty`, and does not emit a block** (mst.zig:492-516). That +is fine for ordinary nodes: `collectNodeBlocks` visits them afterwards and emits from the +cached encoding. The cache exists precisely to make that safe. + +An empty node is the one shape that never reaches the emit path: + +```zig +// mst.zig:518-519 +fn collectNodeBlocks(self: *Mst, node: *Node, …) MstError!void { + if (isUnloadedStub(node)) return; // ← bails here + +// mst.zig:786-788 +fn isUnloadedStub(node: *const Node) bool { + return !node.dirty and node.cid != null and node.left == null and node.entries.items.len == 0; +} +``` + +Once `nodeCid` has cleaned it, an empty node satisfies every clause: not dirty, has a CID, +no left, no entries. It is now **structurally indistinguishable from an unloaded stub** — +and skipping stubs is correct, because a stub's block is already in the store. This one's +never was. + +zat names the ambiguity in a test title of its own — *"An unloaded stub is indistinguishable +from an empty node by inspection … emptiness may only be judged after loading"* +(mst.zig:2102-2108). The delete path was hardened against it. `collectNodeBlocks` was not. + +``` + nodeCid() → empty node: dirty=false, cid set, encoded cached, NO BLOCK EMITTED + collectNodeBlocks() → isUnloadedStub() == true → return + repo_blocks → block absent + next tree walk → reader.get(cid) == null → error.PartialTree +``` + +`PartialTree` propagates out of `applyWrites` through the `else => return err` arm of +`createRecord` (src/atproto/repo.zig:49) as a 500 — which is why the collection in the +request was irrelevant. The tree could not be loaded at all. + +--- + +## timeline + +``` +2026-07-02 zat v0.3.10 released ← no pruneIfEmpty +2026-07-25 f7d0816 "mst: prune subtree nodes emptied by delete" + → shipped in v0.3.19; zds still pinned v0.3.10 +2026-08-12 15:13:07 commit 8730 + place.birds.sighting/3msvg56t43skv + 15:13:37 commit 8732 − same key → EMPTY NODE CREATED (defect A) + ... zat.dev acquires one the same way, under io.atcr.manifest/… +2026-08-13 04:54 zds 4cb15aa bumps zat v0.3.10 → v0.3.29 + defect A fixed — 13h41m too late. the bad node persists. +2026-08-18 05:14 / 05:16 last successful commits on both repos + ~05:37 first `PartialTree` — a walk finally reaches the empty node + → all writes down on zat.dev and birds.place +``` + +The gap is the interesting part. The bad node sat inert for six days because nothing had to +traverse that particular branch. Defect B guarantees the block is missing; it just takes a +walk that reaches it to convert that into an outage. + +--- + +## reproduction + +Feeding the exact 10-key set and the single delete into **zat v0.3.10** reproduces the +production commit bit-for-bit: + +``` +8732 stored (production) 830b8e14f23565d4029bb079a46be67188d81d1264a4e695301f03b963c99207 +8732 rebuilt on v0.3.10 830b8e14f23565d4029bb079a46be67188d81d1264a4e695301f03b963c99207 ✓ +canonical (fresh build) 6b68cca937ce9cf58e6047b4fa2d4ad8b831f444e5ebb157f5980bcac027bd5d +``` + +The same input on current zat produces `canonical`. The base tree at `8730` was verified +canonical first, so the delete is the sole cause. + +A replay through the **full zds write path** — real SQLite, `loadLazy`, `RepoBlockReader`, +`applyWrites` — on current code produces zero missing blocks and zero empty nodes. Defect A +is genuinely fixed here. + +--- + +## mitigation + +The empty node is a repo-independent constant, so restoring availability needed no commit +rewrite, no re-signing, and no firehose replay: + +```sql +INSERT INTO repo_blocks (did, cid, data, repo_rev) +VALUES (?, 'bafyreie5737gdxlw5i64vzichcalba3z2v5n6icifvx5xytvske7mr3hpm', + x'A2616580616CF6', NULL); +``` + +Applied to both DIDs. Both repos served again immediately. + +birds.place then **healed itself**: its bot's normal create/delete cycle on +`place.birds.sighting/*` lands in exactly that subtree, and the delete now runs through the +fixed `pruneIfEmpty`. Its served root is now identical to a canonical fresh build. + +zat.dev has not written since, so it still carries its empty node under +`io.atcr.manifest/…/l` and still serves a non-canonical root. + +### post-incident sweep + +All nine repos, walked from their served roots: + +``` +did:plc:b64lsctzqnzpv6vd4ry3qktw records=54 missing=0 empty=0 +did:plc:siv7zbedip4vcet4v67piibr records=84 missing=0 empty=0 +did:plc:mkqt76xvfgxuemlwlx6ruc3w records=384 missing=0 empty=1 ← zat.dev, non-canonical +did:plc:zbq37dlgttwafiqadqikhjgk records=2 missing=0 empty=0 +did:plc:whuetkjivofavwkqtmry5vpm records=1 missing=0 empty=0 +did:plc:w4p4bumx22vgnjmctoqjj7sy records=24 missing=0 empty=0 ← birds.place, healed +did:plc:lm5j7vhcqvmmalcosgujgkrh records=1 missing=0 empty=0 +did:plc:n3qjsl7p5xfgvirr5t7tnvdm records=0 missing=0 empty=1 ← empty repo, canonical +did:plc:lho2vo4lk3pqey4vl7zlvvvh records=0 missing=0 empty=1 ← empty repo, canonical +``` + +The two `records=0` repos have the empty node **at their root**, which is the one canonical +position for it. `collectBlocksInto` emits it there through a dedicated `self.root == null` +branch (mst.zig:466-479) — which is why those two DIDs had the block stored all along, and +why the same 7 bytes were present under other DIDs while missing under the broken ones. + +--- + +## outstanding + +- **defect B fix written, not yet released.** `collectNodeBlocks` now separates a stub from + an empty node by comparing the node's cid to the empty-node cid, and emits the 7 bytes when + they match. Two regression tests cover it — one on the closure invariant, one on the + production repair path — and both fail without the fix. Needs a zat release and a zds bump. + 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 succeeds, and the collect pass puts + the missing block back. Any repo carrying a pre-v0.3.19 artifact repairs itself on its + next write. +- **zat.dev needs a canonical rebuild** — new root, new rev, re-signed commit. The defect B + fix restores availability but preserves the non-canonical shape; only a rebuild (or a + delete that trips `pruneIfEmpty`, which is how birds.place healed) makes the root agree + with what other implementations compute. +- Unrelated, found in the same logs: an httpz worker panic, + `access of union field 'http' while field 'retired' is active` in `collectTimedOut` + (worker.zig:1076), SIGABRT'd the process at 05:16:47. Separate availability bug, upstream. + +--- + +## citations + +Everything below was read at the versions named, not recalled. + +**zat** (`~/tangled.org/zat.dev/zat`) +- `f7d08169b9dd5afd34dbb8c6016d42ebafad763e` — *"mst: prune subtree nodes emptied by delete"*, 2026-07-25 01:56:38 -0500 +- first tag containing it: `v0.3.19`. `git merge-base --is-ancestor f7d0816 v0.3.10` → **NO** +- `git show v0.3.10:src/internal/repo/mst.zig | grep -c pruneIfEmpty` → **0** +- HEAD `src/internal/repo/mst.zig`: `pruneIfEmpty` call 385, definition 395-398; + `nodeCid` 492; `collectNodeBlocks` 518-519; `isUnloadedStub` 786-788; + `collectBlocksInto` 466-479 +- tests: empty-root reference CID 1564; subtree prune 1992; height-gap pass-through 2079; + stub-vs-empty ambiguity 2102 + +**zds** (`~/tangled.org/zat.dev/zds`) +- `src/storage/store.zig`: `loadLazy` 2111, `deleteReturn` 2134, `rootCid` 2140, + `writeMstBlocks` 2141, `repoBlockDataFrom` (DID-scoped reader) 485 +- `src/atproto/repo.zig:48` — the `else => return err` that surfaces `PartialTree` as 500 +- `build.zig.zon` pinned `zat v0.3.10` from `29fd5e2` (2026-07-02) until `4cb15aa` + (2026-08-12 23:54:39 -0500 = 2026-08-13 04:54 UTC), which bumped it to `v0.3.29` + +**production** (`zds-pds`, fly.io, `/data/zds.sqlite3`) +- birds.place `did:plc:w4p4bumx22vgnjmctoqjj7sy`, commits 8730 / 8732, revs + `3msvg56t43sku` / `3msvg63musrkw` +- stored roots `bafyreibaq3sxp4vuib5g7xzt63lua7b3okpy53hu4ds6znbfa2wpjhhd6a` (8730) and + `bafyreiedbohbj4rvmxkafg5qpgsgxztrrdmb2eteuttjkma7ao4whsmsa4` (8732) +- the missing block, verbatim from `repo_blocks`: `A2616580616CF6`, 7 bytes diff --git a/src/internal/cbor_json.zig b/src/internal/cbor_json.zig index d97aadb..d768986 100644 --- a/src/internal/cbor_json.zig +++ b/src/internal/cbor_json.zig @@ -19,6 +19,9 @@ fn writeValue(allocator: std.mem.Allocator, writer: anytype, value: zat.cbor.Val try writer.print("{{\"$bytes\":{f}}}", .{std.json.fmt(encoded, .{})}); }, .text => |text| try writer.print("{f}", .{std.json.fmt(text, .{})}), + // zat rejects NaN/Infinity at decode and asserts them unreachable at + // encode, so anything reaching here is finite and renders as JSON. + .float => |v| try writer.print("{f}", .{std.json.fmt(v, .{})}), .array => |items| { try writer.writeByte('['); for (items, 0..) |item, idx| { @@ -44,3 +47,24 @@ fn writeValue(allocator: std.mem.Allocator, writer: anytype, value: zat.cbor.Val ), } } + +test "float values render as JSON numbers" { + const a = std.testing.allocator; + const cases = [_]struct { v: f64, want: []const u8 }{ + .{ .v = 1.5, .want = "1.5" }, + .{ .v = -0.25, .want = "-0.25" }, + .{ .v = 2.0, .want = "2e0" }, + }; + for (cases) |c| { + const out = try writeAlloc(a, .{ .float = c.v }); + defer a.free(out); + // whatever the spelling, it must parse back as the same number + const parsed = try std.json.parseFromSlice(std.json.Value, a, out, .{}); + defer parsed.deinit(); + try std.testing.expectEqual(c.v, switch (parsed.value) { + .float => |f| f, + .integer => |i| @as(f64, @floatFromInt(i)), + else => return error.NotANumber, + }); + } +} -- 2.51.2