ShadowsOverWestgate/sow-tools
7
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases 38 Activity

The emit upload path and NWCompressedBuffer round-trip #60

New Issue
Closed
opened 2026-07-27 18:51:21 +00:00 by archvillainette · 5 comments
archvillainette commented 2026-07-27 18:51:21 +00:00
Owner
Copy Link
Copy Source

Question

What does emit's upload path look like, and does the zstd framing round-trip against upstream?

Part of #54. The decision is already made that emit uploads directly with no local blob tree — the working set is one resource, since a blob name is the SHA-1 of the uncompressed bytes and no resource exceeds 15 MiB. This ticket makes that concrete enough to build.

Prototype far enough to answer:

  • The sink shape. Direct upload is the production path, and a local --out tree still has to exist for conformance comparison against upstream. One interface, two implementations — or something simpler.
  • The zstd dependency. This module has exactly two dependencies (x/text, yaml.v3) and stdlib has no zstd. Confirm the choice and that it can produce the exact NWCompressedBuffer framing: uint32 magic NSYC, version 3, algorithm 2, uncompressed size, zstd header version 1, dictionary 0, then a raw zstd frame.
  • Round-trip proof: a blob written by the prototype decompresses to bytes identical to the input, and upstream's blob for the same resource decompresses to the same bytes. Compressed bytes will differ between Nim's libzstd and Go's — that is expected and must not change the blob name.
  • The 15 MiB limit as a fail-closed check, matching upstream's quit(1).
  • What happens mid-hak when an upload fails. A partially emitted hak has real blobs in the zone and no NSYM, which is the state the presence-of-NSYM publication marker is supposed to make unambiguous.
## Question What does `emit`'s upload path look like, and does the zstd framing round-trip against upstream? Part of #54. The decision is already made that `emit` uploads directly with no local blob tree — the working set is one resource, since a blob name is the SHA-1 of the uncompressed bytes and no resource exceeds 15 MiB. This ticket makes that concrete enough to build. Prototype far enough to answer: - The sink shape. Direct upload is the production path, and a local `--out` tree still has to exist for conformance comparison against upstream. One interface, two implementations — or something simpler. - The zstd dependency. This module has exactly two dependencies (`x/text`, `yaml.v3`) and stdlib has no zstd. Confirm the choice and that it can produce the exact NWCompressedBuffer framing: `uint32` magic `NSYC`, version 3, algorithm 2, uncompressed size, zstd header version 1, dictionary 0, then a raw zstd frame. - Round-trip proof: a blob written by the prototype decompresses to bytes identical to the input, and upstream's blob for the same resource decompresses to the same bytes. Compressed bytes will differ between Nim's libzstd and Go's — that is expected and must not change the blob name. - The 15 MiB limit as a fail-closed check, matching upstream's `quit(1)`. - What happens mid-hak when an upload fails. A partially emitted hak has real blobs in the zone and no NSYM, which is the state the presence-of-NSYM publication marker is supposed to make unambiguous.
archvillainette added the wayfinder:prototype label 2026-07-27 18:51:21 +00:00
archvillainette commented 2026-07-27 19:00:31 +00:00
Author
Owner
Copy Link
Copy Source

Inherited from #55 and #57 — settled, build against these

#55 (Bunny API) and #57 (restypes) both landed and pushed work into this ticket rather than leaving it open. Folding it in here so it is not re-decided:

Reuse internal/depot's httpBackend, extended — do not write a new client. It already has IPv4-pinned transport, retry, a tri-state probe where Unconfirmed is distinct from Absent, and skip-if-present upload. Three sha256 assumptions become per-instance fields: the key layout (BlobKey(), manifest.go:26-28), the verify hash in Get() (remote.go:209-221), and the Checksum header (remote.go:171) — which Bunny defines as SHA-256 and validates, so sending an uppercase SHA-1 there will make the upload fail. Compute both hashes in one io.MultiWriter pass and send the SHA-256, keeping server-side integrity checking.

PutReader(ctx, key string, r io.Reader, size int64) is the new method this needs. Put takes a filesystem path, and NWSync blobs come from inside a hak and must never be staged to disk. Reimplement Put on top of it.

Concurrency ceiling stays at ProbeJobs (16) with the same discipline. The edge throttles with 428/000 under load; a throttled probe must never be read as "missing, re-upload" or as "present, skip".

Mirror upstream's restype table, not Crucible's. Crucible registers six extensions upstream does not have (lyt, vis, mdx, wlk, xml, gr2 — erf.go:128-133); upstream would quit(1) on them. Matching the reference implementation is the point of the conformance exercise, and the corpus contains none of the six. Same for the hard-error rule: an unresolvable restype stops the run, and the nss/ndb/gic skip list is applied after resolution, not instead of it.

Lowercase the resref in emit, and reject len > 16. erf.Read preserves original case (erf.go:347, no ToLower on the read path) and does not length-check. Upstream lowercases on both read and write (nwsync.nim:93-99). Crucible-built haks are already lowercase, foreign ones are not.

No table lookup is needed on the emit path at all — erf.Read keeps the raw uint16 from the ERF key list in Resource.Type (erf.go:58-63,339-356), which is exactly what the manifest entry wants.

## Inherited from #55 and #57 — settled, build against these #55 (Bunny API) and #57 (restypes) both landed and pushed work into this ticket rather than leaving it open. Folding it in here so it is not re-decided: **Reuse `internal/depot`'s `httpBackend`, extended — do not write a new client.** It already has IPv4-pinned transport, retry, a tri-state probe where `Unconfirmed` is distinct from `Absent`, and skip-if-present upload. Three sha256 assumptions become per-instance fields: the key layout (`BlobKey()`, `manifest.go:26-28`), the verify hash in `Get()` (`remote.go:209-221`), and the `Checksum` header (`remote.go:171`) — which Bunny defines as **SHA-256** and validates, so sending an uppercase SHA-1 there will make the upload fail. Compute both hashes in one `io.MultiWriter` pass and send the SHA-256, keeping server-side integrity checking. **`PutReader(ctx, key string, r io.Reader, size int64)` is the new method this needs.** `Put` takes a filesystem path, and NWSync blobs come from inside a hak and must never be staged to disk. Reimplement `Put` on top of it. **Concurrency ceiling stays at `ProbeJobs` (16) with the same discipline.** The edge throttles with `428`/`000` under load; a throttled probe must never be read as "missing, re-upload" or as "present, skip". **Mirror upstream's restype table, not Crucible's.** Crucible registers six extensions upstream does not have (`lyt`, `vis`, `mdx`, `wlk`, `xml`, `gr2` — `erf.go:128-133`); upstream would `quit(1)` on them. Matching the reference implementation is the point of the conformance exercise, and the corpus contains none of the six. Same for the hard-error rule: an unresolvable restype stops the run, and the `nss`/`ndb`/`gic` skip list is applied *after* resolution, not instead of it. **Lowercase the resref in `emit`, and reject `len > 16`.** `erf.Read` preserves original case (`erf.go:347`, no `ToLower` on the read path) and does not length-check. Upstream lowercases on both read and write (`nwsync.nim:93-99`). Crucible-built haks are already lowercase, foreign ones are not. **No table lookup is needed on the emit path at all** — `erf.Read` keeps the raw `uint16` from the ERF key list in `Resource.Type` (`erf.go:58-63,339-356`), which is exactly what the manifest entry wants.
archvillainette commented 2026-07-30 17:31:05 +00:00
Author
Owner
Copy Link
Copy Source

Verification sweep — half of this ticket is already answered by PR #71

Landed in a131b25, so do not re-decide:

  • zstd dependency chosen and proven. github.com/klauspost/compress v1.19.1 in go.mod. internal/nwsync/compressedbuf.go writes the exact framing (NSYC, version 3, algorithm 2, uncompressed size, zstd header version 1, dictionary 0, raw frame); TestBlobFramingRoundTrips proves round-trip.
  • 15 MiB fail-closed — emit.go:22 fileSizeLimit, TestEmitFailsClosedOnOversizeResource.
  • Lowercased resref — emit.go:144; unresolvable restype is a hard error (emit.go:135).
  • Sink shape, partly. The local --out tree exists and is the only sink today.

What is left in this ticket:

  1. The upload sink itself. PutReader(ctx, key, r io.Reader, size int64) does not exist on internal/depot's httpBackend yet — build it per the inherited #55 note above (per-instance key layout / verify hash / SHA-256 Checksum header, ProbeJobs=16 ceiling, probe-then-skip).
  2. The CLI reshape. PR #71 shipped #53's old surface, not the settled one: emit <artifact> --out DIR with --out required, and assemble --order NAMES --entries DIR. #56/#62/#65 settled positional artifact keys, upload as the default sink, and NSYM keys derived by Crucible from the artifact key. Detail in the sweep comment on #53. Doing this with the upload sink is one change, not two.
  3. Mid-hak upload failure — unchanged, still the real question here.

The blob-name-is-unchanged-by-compressed-bytes point is settled by construction (sha1 over uncompressed bytes) but still unproven against upstream — that proof is #59, not this ticket.

## Verification sweep — half of this ticket is already answered by PR #71 Landed in `a131b25`, so do not re-decide: - **zstd dependency chosen and proven.** `github.com/klauspost/compress` v1.19.1 in `go.mod`. `internal/nwsync/compressedbuf.go` writes the exact framing (`NSYC`, version 3, algorithm 2, uncompressed size, zstd header version 1, dictionary 0, raw frame); `TestBlobFramingRoundTrips` proves round-trip. - **15 MiB fail-closed** — `emit.go:22` `fileSizeLimit`, `TestEmitFailsClosedOnOversizeResource`. - **Lowercased resref** — `emit.go:144`; unresolvable restype is a hard error (`emit.go:135`). - **Sink shape, partly.** The local `--out` tree exists and is the only sink today. What is left in this ticket: 1. **The upload sink itself.** `PutReader(ctx, key, r io.Reader, size int64)` does not exist on `internal/depot`'s `httpBackend` yet — build it per the inherited #55 note above (per-instance key layout / verify hash / SHA-256 `Checksum` header, `ProbeJobs`=16 ceiling, probe-then-skip). 2. **The CLI reshape.** PR #71 shipped #53's old surface, not the settled one: `emit <artifact> --out DIR` with `--out` required, and `assemble --order NAMES --entries DIR`. #56/#62/#65 settled positional artifact keys, upload as the default sink, and NSYM keys derived by Crucible from the artifact key. Detail in the sweep comment on #53. Doing this with the upload sink is one change, not two. 3. **Mid-hak upload failure** — unchanged, still the real question here. The blob-name-is-unchanged-by-compressed-bytes point is settled by construction (sha1 over uncompressed bytes) but still unproven against upstream — that proof is #59, not this ticket.
archvillainette commented 2026-07-30 17:38:22 +00:00
Author
Owner
Copy Link
Copy Source

From #59: while reshaping the CLI, fix the flag order too — emit hak/x.hak --out DIR fails with "exactly one artifact is required" because Go's flag stops parsing at the first non-flag argument. With positionals becoming the norm (#56), this bites every caller.

From #59: while reshaping the CLI, fix the flag order too — `emit hak/x.hak --out DIR` fails with "exactly one artifact is required" because Go's `flag` stops parsing at the first non-flag argument. With positionals becoming the norm (#56), this bites every caller.
archvillainette self-assigned this 2026-07-30 18:07:43 +00:00
archvillainette commented 2026-07-30 18:17:34 +00:00
Author
Owner
Copy Link
Copy Source

Prototype landed — PR #73

The upload path is concrete now, so the parts of this ticket that were design questions are answered by working code rather than argument.

Sink shape. One sink interface, two implementations — zoneSink (production) and dirSink (--out DIR, the conformance path). Nothing simpler works, because both verbs need blob put, index put and index get, and the local tree has to stay diffable against upstream.

Reuse, per #55, with one refinement. depot.KeyStore adds ProbeKey / PutReader / GetKey to the existing httpBackend, and the sha-addressed Backend is now rewritten on top of it — same transport, same retry, same tri-state probe. #55 called for three sha256 assumptions becoming per-instance fields; passing the key and the checksum as arguments turned out simpler and left Put a four-line wrapper. Bunny's Checksum is sha256 of the uploaded body, verified by the fake zone in the tests.

Compression is now paid only for blobs that are actually uploaded — the body is a thunk behind the probe. On a re-run or a backfill pass that is the whole cost of the run.

Round-trip and framing were already proven by #59 against upstream (byte-identical manifest, identical blob headers, identical payloads after decompression). Re-checked through the reshaped CLI in this PR: still byte-identical.

Mid-hak failure — the mechanism. Blobs go up as produced, the index lands last, so a half-emitted artifact is real blobs plus no index, and that state is unambiguous. Blob names are content hashes, so a re-run skips whatever landed and costs one probe per resource. assemble fails closed on an artifact with no index rather than publishing a manifest missing a hak.

What still needs a human, and it is the last thing on this ticket

The policy above the mechanism: when emit fails inside a producer repo's release job — say hak 7 of 11 — what happens to that release?

  1. The producing release fails outright. No catalog entry, nothing downstream can pin it, and the operator fixes it there and then.
  2. The release publishes anyway, and the missing index surfaces later, when sow-module CI runs assemble and fails closed.
  3. The release publishes and a retry job re-runs emit, on the theory that most failures are transient edge throttling.

My recommendation is (1). A hak release whose blobs are only partly published is not a release anyone should be able to pin, and under (2) the failure surfaces in a different repo, days later, pointing at a hak whose emit failed rather than at the run that failed — the worst place to learn it. (3) is (1) plus a retry, and a re-run is already cheap and safe; CI re-run is the retry.

Not settled by me: (1) means a Bunny hiccup can fail an otherwise good hak build, and the blobs already uploaded stay in the zone as orphans until the next successful run adopts them. That is the cost.

## Prototype landed — [PR #73](https://git.westgate.pw/ShadowsOverWestgate/sow-tools/pulls/73) The upload path is concrete now, so the parts of this ticket that were design questions are answered by working code rather than argument. **Sink shape.** One `sink` interface, two implementations — `zoneSink` (production) and `dirSink` (`--out DIR`, the conformance path). Nothing simpler works, because both verbs need blob put, index put and index get, and the local tree has to stay diffable against upstream. **Reuse, per #55, with one refinement.** `depot.KeyStore` adds `ProbeKey` / `PutReader` / `GetKey` to the existing `httpBackend`, and the sha-addressed `Backend` is now rewritten on top of it — same transport, same retry, same tri-state probe. #55 called for three sha256 assumptions becoming per-instance fields; passing the key and the checksum as arguments turned out simpler and left `Put` a four-line wrapper. Bunny's `Checksum` is sha256 of the uploaded body, verified by the fake zone in the tests. **Compression is now paid only for blobs that are actually uploaded** — the body is a thunk behind the probe. On a re-run or a backfill pass that is the whole cost of the run. **Round-trip and framing** were already proven by #59 against upstream (byte-identical manifest, identical blob headers, identical payloads after decompression). Re-checked through the reshaped CLI in this PR: still byte-identical. **Mid-hak failure — the mechanism.** Blobs go up as produced, the index lands last, so a half-emitted artifact is real blobs plus no index, and that state is unambiguous. Blob names are content hashes, so a re-run skips whatever landed and costs one probe per resource. `assemble` fails closed on an artifact with no index rather than publishing a manifest missing a hak. ### What still needs a human, and it is the last thing on this ticket The *policy* above the mechanism: when `emit` fails inside a producer repo's release job — say hak 7 of 11 — what happens to **that release**? 1. **The producing release fails outright.** No catalog entry, nothing downstream can pin it, and the operator fixes it there and then. 2. **The release publishes anyway**, and the missing index surfaces later, when sow-module CI runs `assemble` and fails closed. 3. **The release publishes and a retry job re-runs emit**, on the theory that most failures are transient edge throttling. My recommendation is **(1)**. A hak release whose blobs are only partly published is not a release anyone should be able to pin, and under (2) the failure surfaces in a different repo, days later, pointing at a hak whose emit failed rather than at the run that failed — the worst place to learn it. (3) is (1) plus a retry, and a re-run is already cheap and safe; CI re-run is the retry. Not settled by me: (1) means a Bunny hiccup can fail an otherwise good hak build, and the blobs already uploaded stay in the zone as orphans until the next successful run adopts them. That is the cost.
archvillainette commented 2026-07-30 18:19:13 +00:00
Author
Owner
Copy Link
Copy Source

Resolved — failure posture is (1): a failed emit fails the producing release

The mechanism shipped in PR #73; the policy above it is now settled.

When emit fails inside a producer repo's release job, that release fails. No catalog entry, nothing downstream can pin it, and the operator fixes it in the run that broke. Not "publish and let sow-module's assemble catch it later": that surfaces the failure in a different repo days later, pointing at a hak rather than at the run that failed. Not a retry job either — a re-run is already cheap and safe, because blob names are content hashes and a re-run costs one probe per resource, so CI re-run is the retry.

Accepted costs, stated so nobody re-litigates them on the day it happens:

  • A Bunny hiccup can fail an otherwise good hak build. The fix is to re-run the job.
  • Blobs uploaded before the failure stay in the zone as orphans, unreferenced by any index, until the next successful run adopts them. They are harmless — content-addressed, deduped, and cheap — and no cleanup pass is built for them.

assemble's fail-closed check on a missing index stays as the second line of defence, not the first.

Everything else on this ticket, for the record

  • Sink shape — one sink interface, two implementations: zoneSink (production) and dirSink (--out DIR, the conformance path).
  • Reuse — depot.KeyStore (ProbeKey/PutReader/GetKey) extends internal/depot's httpBackend; the sha-addressed Backend now rides on it. #55's "three sha256 assumptions become per-instance fields" became "the caller passes key and checksum", which is smaller.
  • zstd — github.com/klauspost/compress, framing proven byte-identical against upstream in #59. Compression is now paid only for blobs that are actually uploaded.
  • 15 MiB limit — fail-closed, matching upstream's quit(1).
  • CLI — reshaped to #56/#62/#65's surface; flags may follow positionals.

Live upload against the real zone is still unproven: the zone and its credential are #61.

## Resolved — failure posture is (1): a failed emit fails the producing release The mechanism shipped in [PR #73](https://git.westgate.pw/ShadowsOverWestgate/sow-tools/pulls/73); the policy above it is now settled. **When `emit` fails inside a producer repo's release job, that release fails.** No catalog entry, nothing downstream can pin it, and the operator fixes it in the run that broke. Not "publish and let sow-module's `assemble` catch it later": that surfaces the failure in a different repo days later, pointing at a hak rather than at the run that failed. Not a retry job either — a re-run is already cheap and safe, because blob names are content hashes and a re-run costs one probe per resource, so CI re-run *is* the retry. Accepted costs, stated so nobody re-litigates them on the day it happens: - A Bunny hiccup can fail an otherwise good hak build. The fix is to re-run the job. - Blobs uploaded before the failure stay in the zone as orphans, unreferenced by any index, until the next successful run adopts them. They are harmless — content-addressed, deduped, and cheap — and no cleanup pass is built for them. `assemble`'s fail-closed check on a missing index stays as the second line of defence, not the first. ### Everything else on this ticket, for the record - **Sink shape** — one `sink` interface, two implementations: `zoneSink` (production) and `dirSink` (`--out DIR`, the conformance path). - **Reuse** — `depot.KeyStore` (`ProbeKey`/`PutReader`/`GetKey`) extends `internal/depot`'s `httpBackend`; the sha-addressed `Backend` now rides on it. #55's "three sha256 assumptions become per-instance fields" became "the caller passes key and checksum", which is smaller. - **zstd** — `github.com/klauspost/compress`, framing proven byte-identical against upstream in #59. Compression is now paid only for blobs that are actually uploaded. - **15 MiB limit** — fail-closed, matching upstream's `quit(1)`. - **CLI** — reshaped to #56/#62/#65's surface; flags may follow positionals. Live upload against the real zone is still unproven: the zone and its credential are #61.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
wayfinder:grilling
wayfinder:map
wayfinder:prototype
wayfinder:research
wayfinder:task
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No labels wayfinder:prototype
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
Chass FallenDabus WinterZen XeroTwo Zariu archvillainette gitea-bot xtul
No Assignees archvillainette
1 Participants
Notifications
Due Date
No due date set.
Depends on
#55 Bunny zone: how a blob is uploaded and how existence is checked without mounting
ShadowsOverWestgate/sow-tools
Reference: ShadowsOverWestgate/sow-tools#60