feat(remote): Fetch cache-fill pipeline (S13 Step 7, part 2/4)

David Fernandezrequested to merge
10io/s13-step7-part2of4 into main

📦 What this MR does

Part 2 of 4 of S13 Step 7 (Fetch cache-fill pipeline): the production pipeline itself — one indivisible unit.

  • Fetcher/NewFetcher, scoped to one remote repository, reaching upstream only through part 1's UpstreamDoer seam.
  • Fetch: GET via the per-format RequestBuilder (conditional If-None-Match only when a validator exists), dispatching on the response — 200 → streamAndFill, legitimate 304 → revalidated (bump upstream_checked_at, no rewrite; legitimacy is keyed on the If-None-Match the request actually carried, from either the option or the builder, and a bare 304 with no validator sent falls through to FetchError), anything else → FetchError with a drained-then-closed body (preserves upstream keep-alive) and the fail-closed SafeHeaders copy.
  • Two responses that arrive intact but are unusable are rejected in that same FetchError class rather than cached: a 200 whose body still carries a content coding the upstream client did not unwrap (br, deflate, zstd, multi-coding lists — the bytes are compressed while the caller and the cache row would both describe them as identity; the spec-level decision on volunteered codings is tracked in #397 (closed)), and a legitimate 304 whose cache row retention deleted between the Lookup and the revalidation, which degrades to one unconditional re-fetch instead of reporting a vanished entry as fresh.
  • streamAndFill + the cacheFillTee terminal-state machine: a client-driven tee of the upstream body into an S06 staging session (buffered writes capped at 1MiB, shrunk to a shorter declared Content-Length), commit-before-upsert ordering on clean EOF, discard on early Close/read failure (Close reports an abandoned fill as ErrFillAbandoned rather than nil, so a consumer that never reads through EOF cannot leave the cache silently unfilled), exactly-once terminal transitions, upstream body closed on every exit. The session and the cache-maintenance writes run on a cancellation-detached context (client disconnect must not corrupt server-side bookkeeping); the two single-query cache calls carry a detached 30s bound. Upstream ETags are dropped to empty at capture — on the cache row, the result, and the client-safe header copy — when they are over-long (>255 chars, the cache tables' CHECK limit), carry a CR/LF/NUL byte the upstream client would reject on the next conditional GET, or are not valid UTF-8: the spec-sanctioned re-fetch-in-full degradation. UpstreamCheckedAt is stamped when the upstream response arrives, not at fill completion.

⚠️ Tests arrive in parts 3-4

This part ships production code whose test floors land directly above it in the same stack: part 3 carries the nine acceptance-path unit tests, part 4 the sixteen error/edge-case unit tests plus the two real-Postgres integration tests. Splitting the tests out was a deliberate operator decision at the split gate: the 26-test suite is welded to the single Fetcher.Fetch entry point, so colocating it would have produced one ~2k LoC MR. Review parts 3-4 together with this one before merging the stack top.

⚙️ Why stacked

Step 7's full diff is ~2.7k reviewable LoC, so it ships as 4 stacked MRs, reviewed and merged bottom-up. Part 1 (seams, sentinels, fakes) is below this MR; parts 3-4 (test floors) sit above.

🔗 References

  • Plan: docs/plans/2026-07-16-s13-virtual-remote-foundation.md — Step 7
  • Spec: docs/specs/S13-virtual-remote-foundation.md — Fetch, cache freshness model

🔬 e2e scenarios

No scenario added or affected: S13 remote fetch has no e2e catalog yet, and this code is not reachable from a request path until Step 11 wires the production adapter.

Related to #328 (closed)

📚 Stacked MRs (review/merge bottom-up)

Edited by David Fernandez

Merge request reports