feat(oci): Step 15 part 3 - wire the fill into the root (S16 plan: 15/26)

Why

Last of the three MRs that make up Step 15, and the one that turns the arm on. The two below it build the blob fill path and leave it inert: oci.NewRemoteBlobHandler takes the fill seam as an option, so an arm built without it answers every cache miss with the interim 501.

The three-MR chain

Step 15 ships as three MRs. Merge bottom-up; each targets the one below it.

Part MR LOC What
part 3 !1904 837 Wires the arm into the composition root — the MR that turns it on
part 2 !1903 1,422 The two guards a review found, plus the comment and coverage fixes
part 1 !1894 (merged) 2,414 The fill path itself: three answers and the refusal's framing
main

The first two are inert in production. oci.NewRemoteBlobHandler takes the fill seam as an option, so an arm built without it answers every cache miss with the interim 501. Nothing reaches the fill path until !1904 supplies the seam, which is what makes it safe to land the arm and its guards separately.

Two MRs sit alongside rather than in the chain: !1893 (merged) (the S16 spec amendment this arm implements — merging it is the approval signal) and !1896 (merged) (a WriteError fix this work surfaced). Both target main and neither is a code dependency of any of the three.

What

ociRemoteWiring carries what wireRemote built — the security-hardened upstream client as a Doer, the metadata body cap a manifest fetch is bounded by, and the request_total_timeout the arm sizes its response write deadline from. Its zero value wires no fill, which is what keeps the DB-less unit stubs and every boot guard on the 501 rather than nil-panicking on the first cold pull; npmRemoteWiring holds the same shape for the same reason.

The guard in ociRemoteWiringFrom is on the client, not on the Doer that wraps it: upstreamhttp.NewDoer wraps a nil client in a non-nil Doer that reads as wired and then fails on the first request.

buildOCIRemoteBlobFill is the per-miss factory, and what is built once versus per request is the whole of it. The token cache is why the shared half exists: oci.RemoteTokenCache coalesces concurrent exchanges for one key, so one instance per process is what makes it a cache — a per-request one would exchange a fresh token on every cold pull with no signature able to detect it. The URL builder, request builder and cache store are per request because each captures this request's own read of the container_remote_repositories row.

Two config values reach two constructors through one selector each. remoteManifestPayloadCeiling feeds the manifest ceiling to both NewRemoteCacheStore and RemoteOperationsDeps — unequal values there make every manifest between them fetch, commit, and then be refused by the read-back, so the image never caches and the refusal blames the upstream. TestOCIRemoteWiringFrom_ReadsEachConfigFieldFromItsOwnBlock pins which http_client field lands where, because MetadataBodyCap and BodySizeCapBlob are both int64 in the same struct and a swap compiles, validates, and silently lifts the manifest bound to the blob cap.

The operator docs come with the behaviour they describe

max_concurrent_fills_per_repo is enforced inside remote.SingleFlight and nowhere else, which Maven and npm compose and this arm does not. The configuration-reference row and config.example.yaml now scope it to those two formats and say container blob fills are unbounded — tracked in #752. The request_total_timeout guidance one paragraph down explained the cost of raising it in terms of fill slots and queueing that container has neither of; it now states container's actual cost, including that a layer which cannot transfer inside the timeout never caches.

A driver assertion that had never passed

cmd_smoke's uncached-digest row asserted 404 against a reachable registry, while the OCI_UPSTREAM_URL comment in the same file says a real registry's answer "is not assertable in advance". Following the documented recipe against Docker Hub returns 401, so that branch had never run — the green runs were all the unresolvable-upstream path. It now asserts only that the answer is not the interim 501, which is the regression the check exists for, through a new check_not helper.

Verification

Run against a real local instance, both ways:

Upstream Result Uncached digest
https://upstream.invalid (default) 71 passed, 0 failed 503
https://registry-1.docker.io 71 passed, 0 failed 401 (not 501)

The count is unchanged from main because the blob rows were already there; what moved is the last of them. go vet -tags=integration is clean — it is what caught a boot-guard fixture that would have panicked in CI, since the integration suite is not part of a local go test ./... run.

mise run conformance could not run locally: it needs /etc/labkit and /secrets writable, both root-owned. This MR touches no hosted path, so the hosted-route risk lives in !1896 (merged) rather than here.

Overlap with the S16 Step 14 stack

Step 14 and Step 15 are sibling-parallel in the plan and share the same composition glue on purpose — remote_operations.go's own comment says keeping them on one composition "is what stops the manifest, blob, and list arms drifting apart". The cost of that sharing is that parallel development collides.

!1869 (merged) has merged and this branch is rebased onto it. !1872 (merged) is still open and further along, so it lands first and this chain rebases after. Measured with git merge-tree, the conflict is concentrated rather than spread:

conflicts with !1872 (merged)
!1894 (merged) (this chain's base) ~31 LOC
!1903 ~150 LOC
!1904 ~603 LOC

Related to #288

🤖 Generated with Claude Code

Edited by Sylvia Shen

Merge request reports

Loading
Loading