docs(plans): add S16 container remote implementation plan

What

The implementation plan for S16 Container Remote Repositories, decomposed into 18 MR-sized steps that ship as 22 MRs. This is the plan MR: it lands before any step MR opens, and merging it is the approval gate for the research, approach, and step breakdown.

It also carries two amendments to the merged S16 spec, folded in here rather than sent as a separate MR to save a review round trip. Both are described under Revisions since the first draft.

Decomposition

The plan follows S15's step spine rather than deriving its own shape — S15 is the closer sibling at five cache tables to S16's six, and the more compressed of the two breakdowns at 14 steps. Migrations in two groups, every pure building block in one step, read queries in one step, cache-fill writes in another, credentials separate from the finder, a kind-dispatch seam, an operations-glue step, one step per handler family, then observability and a hermetic harness.

Three differences — the first two are S16 surface the siblings do not have, the third is surface none of the three has:

  • The auth primitives take S15's config slot, since all four ContainerRemoteConfig fields are token-flow durations that the token cache reads.
  • Two steps S15's spine has no slot for: the cache-hit serve path (which S14 does have, and which ADR-005 delivery mode and Range earn on their own) and the token-exchange flow.
  • No health-probe step, because neither sibling has one — see the research findings below.

Two of S15's slots then split in two, because S16's version of each came to roughly 1,000 lines against S14's 280–400 band: the challenge parser separates from the token cache, and the repository finder separates from the cache lookup. S14 already splits the second of those, into its Steps 6 and 8.

The split is at the first upstream byte, which is where both siblings' landed commits actually stop:

  • Track A (Steps 1–11), openable today — the six container_remote_* migrations, HTTP-free upstream URL construction with per-segment escaping and the pull-reference grammar gate, the streaming tags/list name transform, the RFC 7235 challenge parser, the TTL-aware bearer-token cache with ContainerRemoteConfig, the datastore read side (finder and health seams; cache lookup and freshness), cache-fill writes with the by-digest gate, credential and auth_url/auth_status writes, the kind-dispatch seam with write-verb 405s and 501 read slots, and the warm-cache serve path with download signals.
  • Track B (Steps 12–18), gated on S13 Step 11 — the container request builder and token-exchange flow; operations glue with the upstream-outcome error map; the manifest and blob proxies; the two live-proxied lists; observability; and a hermetic end-to-end harness.

Sibling-parallel groups, mergeable in any order: 3/4/5, 6/7/9, 8/10, 14/15. Four independent roots (1, 3, 4, 5).

Criterion coverage

The spec carries 110 acceptance criteria across 8 groups — three times S15's. An Acceptance-criterion ownership table maps every one to its owning step, so the coverage claim is checkable against the spec without reading all 18 steps, and marks the ones shipping gated. Two criteria (NAME_UNKNOWN on an unresolvable slug, and malformed-digest rejection) are satisfied by shipped S12 code and are listed as owned outside any step rather than left unclaimed.

Building that table is what surfaced the header-discipline gap: upstream headers are not blanket-copied and nosniff on every proxied read had no single owner, so Set-Cookie, WWW-Authenticate, and X-* suppression was asserted nowhere. Step 11 now owns a shared response-header writer that Steps 14, 15, and 16 compose, each asserting the suppression on its own path.

Research findings the decomposition rests on

  • S13 is partly built, and its plan's Status table is not the signalgit log is. Landed: the contract seams, the SSRF-hardened upstreamhttp client, Lookup + freshness + CacheFallback, the teed Fetch pipeline, Probe, credential management, single-flight coalescing, and the health monitor. Not landed, and claimed by S13's own plan rather than unowned: the standalone request flow plus the UpstreamDoer adapter (S13 Step 11, #332 (closed)) and the scheduled health sweep (S13 Step 15, #336 (closed)). The findings were read on 2026-07-30 and re-verified on 2026-08-03, and that gap is not cosmetic — three things the first reading recorded as absent merged in between, each landing on a step rather than only on the inventory. See Re-verified against a moved main below.
  • S13 Step 11 carries the UpstreamDoer adapter and the #269 transfer-deadline fix, not S16. internal/remote/interfaces.go assigns both to "the composition that first wires Fetch end-to-end," and S13's plan reads that same sentence and answers "which is this step" for its Step 11 — whose Files list creates internal/remote/upstreamhttp/doer.go and its test, resolves #269 before wiring the adapter into Fetch, and adds a shared remotetest/conformance.go seam assertion every UpstreamDoer implementation runs through. S16 therefore writes no adapter and edits no package under internal/remote; it consumes all of it, with S13 Step 11 as Step 12's external prerequisite. The ordering works: S13 Step 11 needs only S13 Steps 6 and 8, both of which are on main, while S16 does not reach Track B until fifteen Track A MRs have merged — Track A is 11 steps, but Steps 1 and 2 ship three MRs each.
  • The spec's expected-digest follow-up overstates the gap. It says the by-digest never-commit-on-mismatch guarantee "cannot be met on the manifest path." The row-level half can be met today: remote.CacheEntry.BlobRef carries the session-computed sha256 and an UpsertCacheEntry error is wrapped as ErrCacheFill, so a per-request store that knows the requested digest can refuse the row. What cannot be prevented is the already-committed CAS blob being left unreferenced — which internal/remote/fetch.go's own comment calls "the benign direction to fail in," reclaimed by ADR-011. Step 8 carries the comparison and that criterion is not gated; the follow-up narrows to discarding the committed object.
  • The six migrations cannot be parallelized. internal/datastore/migrations/migrations.go sets goose.WithAllowOutofOrder(false), and a timestamp is fixed at authoring time, so two parallel branches yield one merging second with the earlier timestamp — refused on any database that already applied the later. FK parentage forces most of the order independently. Steps 1 and 2 stay two plan rows and name their six sequential MRs explicitly (1a–1c, 2a–2c).
  • kind already reaches the dispatcher through namespace.Resolution. Resolution carries Kind, ContainerRepositoryFinder.FindContainerRepositoryByName returns it in a 5-tuple, and FindByNameInNamespace selects repositories.kind on the JOIN that already carried format — all landed on this branch's base, with the OCI usage-data emission as the first consumer. Step 10 is therefore a handler-only step: it branches on a populated field, and the widening its file list used to name is gone.
  • S17, not S16, owns the single transaction around a url change. CredentialManager.ApplyURLChange's doc says the CredentialRepo seam "exposes no url write, so the clear here is not co-located with the url column update in one transaction. The management layer (S17) … must compose that update and this clear." S16 ships the store methods and the token-cache eviction.
  • Neither sibling gives the health probe a step, because none contributes a probe path — S14 folds the health seams into its store step, S15 has none. Both probe criteria are verifiable today against remotetest.FakeHealthStatusStore; what waits on S13 is the scheduled sweep.
  • ADR-023 rests on review here, not CI. .golangci.yaml's format-isolation allow entries are unscoped by file, so internal/format/oci importing internal/format/npm/npmremote passes the linter, and no depguard:negative-control probe exercises a top-level format package importing a sibling — the job's OCI probe covers ociroute-stays-a-leaf, a rule about what ociroute may import rather than about who may import npmremote. Filed as #431; scoping the allow entries is a separate chore(ci): MR.
  • auth_status is new in the merged spec — a three-state memoized discovery verdict with no ADR-007 equivalent and no sibling precedent. Reviewers diffing Steps 1a/1c against ADR-007 will flag it, and container_remote_manifests.last_downloaded_at, as divergent until the handbook amendment lands.

Reference implementation review (Steps 4 and 5)

Read per code-reuse-policy.md: the Container Registry's authchallenge.go and session.go, and the monolith's container/upstream.rb. The findings block lives in Step 4 and covers the whole auth surface — parser patterns in Step 4, TTL arithmetic in Step 5, the exchange in Step 12.

Worth keeping: the reference's escaped-quote unescaping, its lowercasing of both scheme token and parameter keys, its walk over every WWW-Authenticate header, and its fail-closed handling of an unterminated quote. The monolith independently corroborates the 10-second token-request timeout and the 404 re-discovery rule.

Three divergences where copying the reference would fail a criterion, all in session.go:

  1. It floors a short lifetime up (minimumTokenLifetimeSeconds = 60). Token caching criterion 2 rejects exactly that — expires_in: 45 is honored at 45s.
  2. It lets access_token overwrite token when both are present, above a comment calling the choice "undefined". Auth-challenge criterion 13 requires token to win.
  3. It computes expiry from the response's issued_at. S16 computes from receipt, recorded as deliberate.

The review also added two acceptance criteria Step 4 was missing: uppercase parameter names (Realm=), and selecting the Bearer challenge when a Basic one precedes it or arrives in a second header.

Reachability

No step in this plan delivers a user-reachable capability until S17 Phase 6 (#314) lands. internal/managementapi/create.go answers 422 to any non-hosted kindTestCreateHandler_NonHostedKind_Returns422 pins it — so no API creates a kind=2 container repository, and every fixture here seeds rows directly. That is the established pattern rather than a workaround: docs/dev/conformance.md says the shipping conformance harness seeds its target in the database too, because "AR does not auto-create repositories." The slice can merge complete and still serve no traffic. Nothing below waits on it; it is a scheduling fact for the S16 and S17 DRIs, and a wider consequence than the four auth criteria #314 also gates.

Conformance

Step 18's hermetic httptest harness is this slice's conformance deliverable for a proxy path, matching S14's Step 18 and S15's Step 14, which make the same call in the same words. The external opencontainers/distribution-spec suite behind conformance:oci:s3-garage stays on the hosted path, unchanged: it pushes its own fixtures before pulling them, and a remote repository answers 405 to every write verb, so its workflow does not run against kind=2 without a fixture upstream serving suite-specific content. scripts/conformance/provision.sh seeds kind = 0 and is untouched.

Gated criteria

Four S13-side gaps, S18, and S17 gate named criteria. Each ships skipped and named in its step's Gated entry rather than silently absent — a test written against a gated criterion that passes anyway asserts the gap, not the rule. The plain-gzip fixture is the clearest case: S13 decodes it and strips the header before Fetch sees the response, so a correct implementation cannot reject what it is never shown.

The four are the coalescing-composition mismatch described below (#477 (closed)), the no-transparent-decode switch (#397 (closed), widened to carry both halves of the content-coding decision), and gaps 3 and 2 of #433 — the Content-Length allowlist exemption and the readable Docker-Content-Digest. Every bullet in Dependencies cites a tracker: #91, #314, #320 (closed) item 8, #332 (closed), #336 (closed), #431, #433 gap 1, #434, #477 (closed), and the merged-and-closed #335 (closed) and #445 (closed).

The scope call worth flagging: S16 carries the gzip gate rather than blocking on it, matching how merged Maven code already ships the identical gap in its upstream builder, and matching how the spec itself is written. Neither half serves wrong bytes to a client that checks digests.

Review notes — step sizes

Estimates follow S14's convention — one number per step, hand-written reviewable code including tests, against the development model's 200-400 target and 500 ceiling. They now sit in an Est. column on the dependency table so the whole distribution reads in one place, as S14's plan does.

Most of S16's non-schema steps run larger than S14's, every one of which sits between 280 and 400. The difference is volume rather than step design: S16's non-schema surface totals roughly 10,000 lines against S14's ~4,700, because 27 of the 110 criteria are an upstream auth handshake neither sibling has. Splitting the challenge parser from the token cache, and the repository finder from the cache lookup, brings the largest pure-logic step down from ~990 to ~590. Three steps land inside S14's own band — Step 10 at ~280, Step 4 at ~400, and Step 6 at ~450 — and the other thirteen run from ~520 to ~920. Each stays one concern, and in every case the excess is table-driven cases rather than branching. Per docs/dev/development-model.md, an MR over 500 lines is a warning the operator answers by splitting or justifying the size in the MR description; each step's Est. and Seam interrogation entries are that justification. Steps 1 and 2 name how much of their total is mechanical partition DDL so a reviewer can net it out.

Revisions since the first draft

None of it is new implementation scope.

Scope handed back or narrowed. The UpstreamDoer adapter and #269 go to S13 Step 11: two plans were creating internal/remote/upstreamhttp/doer.go and its test from opposite sides of the platform/slice line, and both quoted the same interfaces.go doc comment to justify it. Dropping the transfer-deadline step and the adapter MR removes ~880 lines, leaves the development-model.md boundary uncrossed, and retires the "accepted deliberately" carve-out. The final step is scoped to the hermetic harness rather than claiming the external distribution-spec run it listed no files for — matching both siblings, with the External prerequisite: line their harness steps carry. And Step 10 is handler-only, because the kind widening it planned to do is already on main.

Step sizes. The plan claimed most steps sit above the ceiling "as S14's do"; S14's non-schema steps are all 280–400, with only its schema steps over. Corrected, with the real reason stated, and the two ~1,000-line steps split — the challenge parser from the token cache, and the repository finder from the cache lookup.

Spec amendments folded in. The Docker-Content-Digest rule is scoped to by-digest reads: the service-generated criterion said an upstream sending a different value than the fill computed always produces the computed value, while the disagreeing-digest criterion says a tag resolution discards the fill and fails 503 UNAVAILABLE, so for a tag read the two pointed opposite ways. Digest verification settles which is meant — a read that names a digest already holds the reference value and does not consult the upstream header, and a tag read holds none, which is why the header is the only cross-check there. Nothing shipping changes: the tag cross-check cannot run until Docker-Content-Digest is readable. Separately, the "needs a work item; not yet filed" lines across the spec's Follow-ups and the plan's Dependencies now cite #397 (closed), #433 gaps 1–3, #320 (closed) item 8, and #434.

Re-verified against a moved main

main advanced 43 commits between the 2026-07-30 reading and 2026-08-03, and three things the findings recorded as absent merged in that window: single-flight coalescing, the S13 health monitor (#335 (closed), closed — probe, status transitions, failure counter, on-demand entry), and the namespace partition key on the health and credential seams (#445 (closed), closed). Each lands on a step, so the operative text moved with it, not just the inventory.

Fetch coalescing is deferred out of this slice deliberately, and that is the one call worth a reviewer's attention. The layer exists on main now, so the code Step 15's three coalescing criteria need is there. S16 still does not compose it. NewSingleFlight captures the RequestBuilder and the CacheStore and holds them for the repository's lifetime, and its own doc makes one long-lived instance per remote repository a hard usage contract, because a per-request one coalesces nothing while compiling and passing every test. Both seams are request-scoped here on purpose: the container builder holds a bearer token it must not outlive, and the cache store has to know the digest the request named to run Step 8's by-digest gate. Reconciling them means either hoisting the token read into BuildRequest and moving the expected digest onto Fetch (#433 gap 1), or holding the layer per repository with request state threaded through it. Neither belongs in this slice, so the three criteria still ship skipped — now against that shape mismatch rather than against absent code, and Step 15 says not to reach for the landed layer, since an un-skip that looked green would assert the opposite of the rule.

The rest follows from the same re-read. #320 (closed)'s follower item turns out to be spec text, and the behavior it asks S13 to document is what the landed layer already does, so nothing waits on a behavior question there. Steps 6 and 9 implement already-widened signatures, where the instruction that still matters is pinning namespace_id in the predicate rather than merely accepting it. Two pointers had rotted: the Maven swap-marker invariant moved to internal/datastore and is now TestInvariant_MavenCounterUpdatesCarrySwapMarker, and since its AST walk is scoped to the package it sits in, S16's mirror belongs beside the store methods rather than in the OCI package; and the negative-control job does have an OCI probe now, for a different rule.

Three counts were also wrong against the plan's own tables: Step 10's estimate stayed ~560 in the dependency table after the step body dropped to ~280; the auth handshake is 27 of the 110 criteria, not 26, which the ownership table and Step 12 already had right; and three non-schema steps sit inside S14's 280–400 band, so "the rest sit between 520 and 920" misdescribed the distribution it summarizes.

Net across all of it: 17 steps / 22 MRs → 18 steps / 22 MRs.

Earlier GitLab Duo review is still addressed: Step 16 declares Step 10 (it deletes the last two stubs, and with them remote_stub.go), the Mermaid node IDs use ST<n> so they no longer collide with the spec identifiers this plan cites throughout, and the Step 4 reference review above replaces the earlier "not yet done" marker.

S16: Container remote implementation (#288)

Edited by Radamanthus Batnag

Merge request reports

Loading
Loading