From 4c182d5fdfd72cfe0fe693d8cce1d5d2d2e8d175 Mon Sep 17 00:00:00 2001 From: zzstoatzz Date: Fri, 24 Jul 2026 16:27:57 -0500 Subject: [PATCH] listSegments: answer on encode failure instead of dropping the request Found while auditing planBackfill. listSegments built its JSON with `catch return` throughout, so an allocation failure mid-encode returned without ever calling respond -- the request was dropped rather than answered. A client pages listSegments to discover the archive, so the same path could also have returned a truncated 200 that silently hides sealed segments. Rendering moves into renderSegments, which propagates; the handler answers 500. Adds the same fail-index sweep used for planBackfill. Audit: listSegments/getSegment/getBlock partial -> verified again. zig build test (297), ReleaseSafe, archive-contract against pinned client. Co-Authored-By: Claude Opus 5 (1M context) --- docs/semantic-parity.md | 2 +- src/internal/xrpcapi.zig | 164 +++++++++++++++++++++++++++------------ 2 files changed, 116 insertions(+), 50 deletions(-) diff --git a/docs/semantic-parity.md b/docs/semantic-parity.md index 123fd1a..d69e3b4 100644 --- a/docs/semantic-parity.md +++ b/docs/semantic-parity.md @@ -56,7 +56,7 @@ blockers remain. The detailed bootstrap audit is in | Failed-repository healing | **blocked** | A real global/per-host worker gate exists, final redirect hosts are recorded, and retry state is stored. | Pass-level RocksDB/archive failures are logged and hidden from service health. Every failed DID and host gate is materialized in RAM before work. Persisted host parking is not upstream behavior, and park-read errors mean “not parked.” | | JSS sealed format interoperability | **verified** | Upstream-produced sealed fixtures are parsed; Stream-produced sealed files are consumed by the pinned Go reader; header/footer, block index, blooms, collections, compression, and checksums have reciprocal fixtures. | This verifies sealed-format compatibility only. It does not verify startup recovery or replay completeness. | | Archive startup recovery | **verified** | Startup validates the active header, walks complete frames, propagates read/decode/allocation failures, truncates and fsyncs only a framing-torn suffix, reconstructs block/event/sequence state, and resumes the same active file and segment index. Sealed high-water floors are accepted only through the checksum-verifying parser, including the empty-active/lower-sealed case. Physical tests prove repeated same-file restart, exact torn-tail truncation, byte-preserving failure on a complete corrupt frame, and rejection of a forged sealed `max_seq`. | This row establishes startup recovery. Cold replay and post-startup rewrite paths remain separate surfaces. | -| `listSegments`, `getSegment`, `getBlock` | **partial** | The pinned Go client and direct HTTP tests cover normal responses, byte identity, checksums/ETags, conditional requests, ranges, cache headers, and storage-error responses through the production server. | Downgraded from verified on 2026-07-24: `listSegments` builds its JSON with `catch return`, so an allocation failure mid-encode returns without ever calling `respond` and drops the request instead of answering `500`. The same pattern was just removed from `planBackfill`. Needs the identical treatment plus a fail-index sweep. | +| `listSegments`, `getSegment`, `getBlock` | **verified** | The pinned Go client and direct HTTP tests cover normal responses, byte identity, checksums/ETags, conditional requests, ranges, cache headers, and storage-error responses through the production server. `listSegments` rendering was extracted so encode failure propagates; a fail-index sweep over the production handler proves every response is the exact baseline list or `5xx`, never a truncated 200 and never a dropped request. | The claim is limited to these three methods and the tested file states. | | `planBackfill` | **verified** | Normal DID/collection/sequence filtering, whole-segment versus block mode, pagination, and configured limits match the pinned client fixtures. The planner is one-sided like upstream: a block whose collection bitmask row is absent, or a segment carrying no collection index, fails open instead of being pruned (upstream `blockHasAnyCollection`). Internal failure can no longer become a truthful-looking short plan — a fail-index sweep over the whole production handler proves every response is either the exact baseline plan or `5xx`, never a 200 with fewer segments, never a 400, and never a dropped request. | Exact collection reporting (`inspect-segment`) deliberately stays fail-closed; that split matches upstream and is asserted separately. | | Subscribe v1/v2 handshake and wire formats | **partial** | Cursor parsing, v1/v2 filter shapes, WebSocket framing, v1 deflate, v2 dictionary zstd, dictionary validators, size caps, pings, write deadlines, and shutdown close frames have production-path tests. | These tests do not establish gap-free cold replay or remote-data encoding isolation. | | Cold archive replay | **blocked** | Normal sealed and active-prefix replay reaches the hot tail in order in existing fixtures. Timestamp cursor corruption is surfaced for the selected block. | Both cold walkers skip a manifest-listed segment when `openFile` fails. They may then advance beyond durable events never delivered. Upstream returns an error and enforces progress/hole invariants below the readable-log floor. | diff --git a/src/internal/xrpcapi.zig b/src/internal/xrpcapi.zig index ddbf2b0..f3329bd 100644 --- a/src/internal/xrpcapi.zig +++ b/src/internal/xrpcapi.zig @@ -192,55 +192,8 @@ pub const Api = struct { return respond.err(500, "InternalError", "oom"); var out: Io.Writer.Allocating = .init(alloc); - var s: std.json.Stringify = .{ .writer = &out.writer }; - s.beginObject() catch return; - s.objectField("segments") catch return; - s.beginArray() catch return; - var emitted: usize = 0; - var last_idx: u64 = 0; - var remaining = false; - for (summaries) |meta| { - const idx = meta.idx; - if (after != null and idx <= after.?) continue; - if (emitted >= limit) { - remaining = true; - break; - } - var name_buf: [64]u8 = undefined; - const name = archive_mod.formatSegmentName(&name_buf, idx); - s.beginObject() catch return; - s.objectField("name") catch return; - s.write(name) catch return; - s.objectField("index") catch return; - s.write(idx) catch return; - s.objectField("sizeBytes") catch return; - s.write(meta.size) catch return; - s.objectField("checksum") catch return; - var hex_buf: [16]u8 = undefined; - s.write(checksumHex(&hex_buf, meta.header.checksum)) catch return; - s.objectField("eventCount") catch return; - s.write(meta.header.event_count) catch return; - s.objectField("minSeq") catch return; - s.write(meta.header.min_seq) catch return; - s.objectField("maxSeq") catch return; - s.write(meta.header.max_seq) catch return; - s.objectField("minWitnessedAt") catch return; - s.write(meta.header.min_witnessed_at) catch return; - s.objectField("maxWitnessedAt") catch return; - s.write(meta.header.max_witnessed_at) catch return; - s.objectField("blockCount") catch return; - s.write(meta.header.block_count) catch return; - s.endObject() catch return; - emitted += 1; - last_idx = idx; - } - s.endArray() catch return; - if (remaining) { - s.objectField("cursor") catch return; - var cur_buf: [24]u8 = undefined; - s.write(std.fmt.bufPrint(&cur_buf, "{d}", .{last_idx}) catch return) catch return; - } - s.endObject() catch return; + renderSegments(summaries, limit, after, &out) catch + return respond.err(500, "InternalError", "oom"); respond.json(200, out.written()); } @@ -929,6 +882,65 @@ pub const PlanRequest = struct { before_seq: u64 = std.math.maxInt(u64), }; +/// Render the listSegments body. Failures propagate so the caller answers +/// 500; returning without responding would drop the request entirely. +fn renderSegments( + summaries: []const @import("manifest.zig").Summary, + limit: usize, + after: ?u64, + out: *Io.Writer.Allocating, +) !void { + var s: std.json.Stringify = .{ .writer = &out.writer }; + try s.beginObject(); + try s.objectField("segments"); + try s.beginArray(); + var emitted: usize = 0; + var last_idx: u64 = 0; + var remaining = false; + for (summaries) |meta| { + const idx = meta.idx; + if (after != null and idx <= after.?) continue; + if (emitted >= limit) { + remaining = true; + break; + } + var name_buf: [64]u8 = undefined; + const name = archive_mod.formatSegmentName(&name_buf, idx); + try s.beginObject(); + try s.objectField("name"); + try s.write(name); + try s.objectField("index"); + try s.write(idx); + try s.objectField("sizeBytes"); + try s.write(meta.size); + try s.objectField("checksum"); + var hex_buf: [16]u8 = undefined; + try s.write(checksumHex(&hex_buf, meta.header.checksum)); + try s.objectField("eventCount"); + try s.write(meta.header.event_count); + try s.objectField("minSeq"); + try s.write(meta.header.min_seq); + try s.objectField("maxSeq"); + try s.write(meta.header.max_seq); + try s.objectField("minWitnessedAt"); + try s.write(meta.header.min_witnessed_at); + try s.objectField("maxWitnessedAt"); + try s.write(meta.header.max_witnessed_at); + try s.objectField("blockCount"); + try s.write(meta.header.block_count); + try s.endObject(); + emitted += 1; + last_idx = idx; + } + try s.endArray(); + if (remaining) { + try s.objectField("cursor"); + var cur_buf: [24]u8 = undefined; + try s.write(try std.fmt.bufPrint(&cur_buf, "{d}", .{last_idx})); + } + try s.endObject(); +} + pub fn planBackfill(self: *Api, respond: anytype, body: []const u8) void { var arena = std.heap.ArenaAllocator.init(self.allocator); defer arena.deinit(); @@ -1439,6 +1451,60 @@ test "planBackfill never answers 200 with a short plan under allocation failure" try testing.expect(unanswered == 0); } +// Same contract as planBackfill: allocation failure must answer, and must +// never answer 200 with a truncated segment list. A client pages listSegments +// to discover the archive, so a short list silently hides sealed segments. +test "listSegments never answers 200 with a short list under allocation failure" { + const testing = std.testing; + var threaded: Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buffer: [Io.Dir.max_path_bytes]u8 = undefined; + const path_len = try tmp.dir.realPath(io, &path_buffer); + var archive = try archive_mod.Archive.init(testing.allocator, io, path_buffer[0..path_len]); + defer archive.deinit(); + + for (0..6) |i| { + _ = try archive.append(.{ + .seq = i, + .witnessed_at = @intCast(i + 1), + .indexed_at = 0, + .kind = .create, + .did = "did:plc:listfault", + .collection = "app.bsky.feed.post", + .rkey = "3l3qo2vuowo2b", + .rev = "3l3qo2vutsw2b", + .payload = "\xa1\x64\x74\x65\x78\x74\x62\x68\x69", + }, 1); + if (i == 2) try archive.rotate(); + } + try archive.rotate(); + + var baseline_api: Api = .{ .allocator = testing.allocator, .io = io, .archive = &archive }; + var baseline: PlanResponder = .{}; + baseline_api.listSegments(&baseline, ""); + try testing.expectEqual(@as(u16, 200), baseline.code); + + var unanswered: usize = 0; + for (0..256) |fail_index| { + var failing = testing.FailingAllocator.init(testing.allocator, .{ .fail_index = fail_index }); + var api: Api = .{ .allocator = failing.allocator(), .io = io, .archive = &archive }; + var response: PlanResponder = .{}; + api.listSegments(&response, ""); + if (response.code == 200) { + try testing.expectEqualStrings(baseline.written(), response.written()); + } else if (response.code == 0) { + unanswered += 1; + } else { + try testing.expect(response.code >= 500); + } + } + try testing.expect(unanswered == 0); +} + test "plan request rejects invalid filters and windows" { var arena = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena.deinit(); -- 2.54.0