Compare commits

..
2 Commits
Author SHA1 Message Date
archvillainetteandClaude Opus 5 9b7be2c76e fix(nwsync): close the review findings on the framing fix and verify
ci / ci (pull_request) Successful in 3m38s
- compressBlob now asserts its own output declares a content size, instead of
  trusting that the only way to lose the field is the one #86 found. An encoder
  upgrade that finds another way would otherwise republish the same undecodable
  blob in silence, and a blob is skipped by every later emit once written.
- inspectBlob asserts the frame whenever one is present, rather than only when
  the payload is non-empty.
- dirSink stats instead of reading when not verifying. Reading every existing
  blob back on the default path was a plain regression, and it contradicted the
  documented promise that verifying is a repair pass, not the default.
- The manifest sha1 is now checked as hex before it is interpolated into a URL
  path, rather than only measured.
- One blobKey rule for where a blob lives, replacing three copies of the
  fanout-path expression.
- Renamed frameSizeThreshold to oneByteContentSizeCeiling, which says whose
  threshold it is.
- The read-back in zoneSink reads the storage API on purpose; said so, and
  corrected a comment that claimed a failed read-back re-uploads when it
  aborts.
- Tests derive their expected blob counts from the run instead of hard-coding
  fixture arithmetic, and the size-mismatch category has a test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 00:26:22 +02:00
archvillainetteandClaude Opus 5 56d4054118 fix(nwsync): declare Frame_Content_Size, and verify what is published (#86, #85)
These two land together on purpose. Fixing the encoder alone changes nothing
for the blobs already in the zone, because emit skips whatever is already
present.

#86 — klauspost/compress omits the zstd Frame_Content_Size field for inputs
under 256 bytes, which the format permits. Reference libzstd never does, so the
NWN client — which sizes its output buffer from ZSTD_getFrameContentSize and has
therefore never met a frame without one — rejected roughly 6% of our blobs
outright. Any single one stops a sync dead, so no client could complete a sync
of the live manifest.

No encoder option changes this, so emit re-headers the affected frames into the
shape libzstd itself emits: Single_Segment_flag set, Window_Descriptor dropped,
and the freed byte spent on a one-byte Frame_Content_Size. Same length in, same
length out. Verified against the zstd CLI end to end: a real emitted 230-byte
blob now reports its decompressed size where it previously reported none.
emitter_version goes to 2, so assemble refuses to merge an index written by the
encoder that omitted the field.

#85 — a published blob was verified by exactly one thing, a player's client, at
download time. `nwsync verify <manifest-sha1>` now reads a manifest and its
blobs back through the public pull zone with no credential, decompresses and
hashes every one, and reports missing / malformed framing / size mismatch / hash
mismatch per blob. `--sample N` makes a routine check cheap against a manifest
that is ~69,000 blobs and 15 GB. `emit --verify` applies the same check where
emit would otherwise trust presence, and replaces a stored blob that is not what
its name claims — which is what makes the #86 blobs repairable.

Both new checks assert the frame property, not just a round trip. Round-tripping
cannot see this class of fault: both the zstd CLI and Go's decoder stream such a
frame happily, being more capable decoders than the client's, which is how the
defect reached production and survived an audit.

Refs #54, #55, #59, #75, sow-platform#79.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 00:20:15 +02:00
2 changed files with 0 additions and 14 deletions
-12
View File
@@ -97,18 +97,6 @@ broken — is skipped by every later run forever and no backfill repairs it. Wit
and replaced when it does not match. It costs a full GET per existing blob, so and replaced when it does not match. It costs a full GET per existing blob, so
it is a repair pass, not the default. it is a repair pass, not the default.
**After a repair, purge the pull zone before believing `verify`.** A repair is
the one thing that makes a key serve different bytes than it did before, and the
edge caches these objects for 30 days precisely because that normally cannot
happen. The two commands therefore look at different copies on purpose: `emit
--verify` repairs the **origin**, `verify` reads the **edge**, and in between a
warm PoP still answers with the old bytes while a cold one answers with the new.
Until the zone is purged `verify`'s verdict is per-PoP and settles nothing — a
pass is not proof, and a failure is not the repair having failed. The purge is
one call against the pull zone; it belongs in the repair procedure rather than
in `emit`, which holds a storage credential and no CDN one (sow-tools#89, and
the procedure itself is in sow-platform's NWSync runbook).
`emit` uploads blobs first and the index last, so the presence of an index is `emit` uploads blobs first and the index last, so the presence of an index is
the publication marker: an artifact whose emit died halfway leaves real blobs in the publication marker: an artifact whose emit died halfway leaves real blobs in
the zone and no index. Blob names are content hashes, so re-running skips the zone and no index. Blob names are content hashes, so re-running skips
-2
View File
@@ -67,8 +67,6 @@ check on a published blob upstream of a player's client.
--verify makes emit hash what it would otherwise skip. emit normally treats a --verify makes emit hash what it would otherwise skip. emit normally treats a
blob's presence as proof of its contents, so without this an object written blob's presence as proof of its contents, so without this an object written
truncated, or written by an emitter since found broken, is skipped forever. truncated, or written by an emitter since found broken, is skipped forever.
--verify repairs the storage zone, while verify reads the edge in front of it,
so purge the pull zone after a repair or verify answers differently per PoP.
--out DIR writes to a local repository tree instead of uploading, which is the --out DIR writes to a local repository tree instead of uploading, which is the
conformance path against upstream nwn_nwsync_write. Without it, the zone comes conformance path against upstream nwn_nwsync_write. Without it, the zone comes