feat(oci): serve cached container remote manifest reads (S16 plan: 11b/26)

Why

This is the first remote read that actually serves an artifact. It ships the shared response-header writer every later remote read path composes, the digest gate each arm has to run for itself, and the cached-manifest serve arm wired onto the dispatcher's remote manifest slot. A cache miss still answers the interim 501, which Step 14 replaces.

Step 11b of the S16 container remote plan, specified by S16.

What

A new internal/format/oci/remote_serve.go holds three things: writeRemoteReadHeaders, which builds the whole response header set from the cache row, the stored payload's length, and this package's own constants; the ^sha256:[a-fA-F0-9]{64}$ reference gate, which runs on the arm ahead of the cache lookup; and the cached-manifest serve arm itself, which reads the payload through S06 storage, bumps the download timestamps, and answers the interim 501 on a miss. The arm refuses a row whose digest and blob_sha256 hold different sums with a 500: both are taken over the same upstream bytes and container_remote_manifests pairs them with nothing, so serving such a row would advertise one sum in Docker-Content-Digest and stream the bytes the other addresses. Wiring threads the operator's last_downloaded_at_staleness_window from config through to the store — wireOCI gains a ContainerRemoteConfig parameter, which forces a one-line wire.go call-site update — so the store's own 1h default is deleted rather than left to diverge. The MR also amends docs/specs/S16-container-remote.md on two points the code contradicts: five places still described a downloads_count store seam that no longer exists, and the service-generated header list claimed both verbs source Content-Length from BlobInfo.

The plan changes are in a separate MR

This branch carried twelve commits' worth of edits to docs/plans/2026-07-30-container-remote.md. CLAUDE.md guardrail 5 keeps plan edits — the Status table included — out of step MRs, so they were lifted out into !1762 (merged) and this branch was rewritten without them. The Go, the spec amendment, and the other docs are byte-identical to what was here before; ten plan-only commits are gone and two mixed commits lost their plan hunk.

!1762 (merged) merges after this one. Its counter-seam corrections describe a spec that says S16 ships no counter seam, and that spec amendment is in this MR. Merged first, main would carry a plan saying there is no seam beside a spec that still specifies one — the same inconsistency, inverted rather than fixed.

Where this description says a plan claim is corrected, the correction is in !1762 (merged) unless the sentence says otherwise.

Reviewable size

1,801 reviewable Go against the plan's Est. ~1,400 per MR — 1.29x. Counted the house way (added .go lines, blanks and //-only lines stripped):

Group	Reviewable	Raw added
Production Go	350	985
Test Go	1,451	2,547
Docs (.md)	—	82
Total	1,801	3,614 across 25 files

Production reviewable has moved three times from this MR's first measurement at 321. The first branch-review round added one test and comment corrections only, so it moved the raw production column and left the reviewable one alone. The round after it landed two production behavior changes — a short-copy check on the cached-manifest stream, and the parent repositories.id added to the read arm's failure attributes.

The third is the reviewer round on this MR. It lands one behavior change, the digest-versus-blob_sha256 refusal above, and two moves that add no logic: resolveRepository and streamCachedManifest become package-level functions taking the one seam each uses, and the remote manifest wiring leaves the thousand-line wire_oci.go for wire_oci_remote.go. The serve suite also splits three ways — fixtures, the shared response writers, the arm's own request rows.

The file count is 25, up from 21: the wiring file, its test, and the two files the serve suite split into. Both splits move lines between files this MR already adds, so neither shows up in the reviewable columns — the 52 lines they gained are the new refusal, its table row, and the four files' doc comments.

Not split further because the plan's Scope bullet is the argument: the header writer, the digest gate, and the manifest arm are one review surface. Splitting the writer out would leave it in neither arm's review and leave the two header criteria without an owner. Step 11's two natural seams were already taken — this MR is one of the three that split produced.

Test plan

go test -count=1 ./internal/format/oci/ ./internal/config/ ./cmd/artifact-registry/ ./internal/datastore/
golangci-lint run --uniq-by-line=false ./internal/format/oci/... ./cmd/artifact-registry/...
golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 ./cmd/artifact-registry/... ./internal/datastore/...

# The staleness-window arm is integration-tagged. It needs a database at CI's
# max_locks_per_transaction=1024; the local dev container ships 64, which fails
# this package with SQLSTATE 53200 rather than on any assertion.
ARTIFACT_REGISTRY_DATABASE_TEST_DSN=... go test -count=1 -tags=integration -run TestContainerRemote ./internal/datastore/

The arm is driven through real requests against seeded cache rows. The digest gate is asserted ahead of the cache lookup on all three statuses, so a regression that moved it below the lookup fails rather than returning a plausible 500. The header set is asserted as a closed set on both a cache hit and an error envelope, which is what makes "nothing an upstream sent reaches the client" falsifiable on a path where no upstream response exists yet. Download signals are asserted on the timestamp bumps alone — there is no counter seam left to assert against.

The one integration-tagged addition is the staleness-window arm in internal/datastore. It is the only test in that package whose verdict depends on the configured window reaching the SQL, so it is the one that has to run against a real database rather than a rendered statement.

No e2e scenario is added or affected. Both docs/testing/e2e/oci.md and docker.md list "Virtual and remote (proxy/cache) repositories" as out of scope until the capability ships, and it has not: nothing fills the container remote cache yet, so a remote manifest read still answers the interim 501 on a miss. Container/OCI conformance is unreached for the same reason. driver.sh smoke re-run against this branch: 22 passed, 0 failed.

For LLM Agents

Full detail for an automated reviewer or a follow-up agent — a human reviewer does not need it

Merge order and branch state

Nothing merges before this any more. Both blockers landed on 2026-08-19 — 11a (!1694 (merged)) and Step 8's oci MR (!1641 (merged)). This MR opened stacked on 11a's branch and GitLab retargeted it to main on that merge, so the stacking the plan's Step 11 entry describes is historical for this MR rather than live. The branch is rebased onto main, so its diff equals its own change and the base for /validate-step or /review-branch is main. 11c stacks on this MR in turn.

The rebase surfaced two things worth knowing about, both dealt with and neither visible while the merge base was stale:

• A name collision that stops the package compiling. !1641 (merged)'s cache-store suite defines a test helper remoteCacheSum(payload []byte) that hashes a payload to its content address; this arm defined a production remoteCacheSum(raw []byte, column string) that validates a cache row's 32-byte sha256 column. Same package, same name, neither branch saw the other. The arm's is renamed remoteCacheSumFromColumn, which puts it next to the existing remoteCacheSumFromDigest — the two read a sum from the two places a serve can get one.
• The plan's criteria map was one short. !1641 (merged) inserted the envelope-decode refusal at Manifest and blob proxy 35, so the section holds 45 criteria and every citation from 35 on moved up by one. The map had been re-derived against 44. Two paragraphs also disagreed about whether the group could be trusted at all — the one above the table withdrew its numbers, the note below claimed they were current. Both are corrected in !1762 (merged) rather than here; the criteria numbers this description quotes are the 45-criterion ones.

What (the non-obvious parts)

• The header writer takes no upstream response headers, and that is the mechanism rather than an omission. writeRemoteReadHeaders is handed a remoteReadHeaders struct built entirely from the cache row, the stored payload's length, and this package's own constants. A writer that is never given an upstream response cannot forward one, so Steps 14, 15, and 16 get the suppression by composing it rather than by each restating the rule. Giving it one owner is what keeps the "upstream headers are not blanket-copied" and "nosniff on every proxied read" criteria from becoming four partial implementations.
• The digest gate runs on the arm, and its status is per route. S12's ^sha256:[a-fA-F0-9]{64}$ check sits inside the hosted per-action handlers, which are below where the dispatcher branches on repository kind — so no remote arm reaches it, and without the gate a malformed digest reaches the cache key parser and comes back a 500. A bad hex portion under sha256: is 400 DIGEST_INVALID; a sha512: prefix is 404 MANIFEST_UNKNOWN on the manifest arm, because readReferenceDigestInvalid routes any non-sha256: reference as a tag attempt and hosted answers an unresolvable tag the same way. The plan and spec both read 400 for both arms until that was checked against the shipped hosted code. The spec is corrected here, with a Resolutions entry recording the call and its arbiter; the plan's half is in !1762 (merged).
• The configured staleness window is threaded end to end here, closing the gap 11a documented. 11a shipped last_downloaded_at_staleness_window at the config boundary, but the store held its own 1h default because 11a constructs no store. This MR adds the first NewContainerRemoteCacheStore call on a production wiring path, so the constant is deleted rather than left to diverge, and wireOCI gains the ContainerRemoteConfig parameter that carries the operator's value (which forces the one-line wire.go call-site update). Three tests hold different links of that chain, because no single one proves it: which field the arm reads, that the arm forwards it on every build, and that the slot is filled at boot.
• The 500 arm logs the tenant, which Maven's equivalent does not. Step 7's cache lookup returns identifier-free errors by house rule, which relocates namespace_id and the remote repository id to whoever logs them — and this arm is that frame. internal/format/maven/remote_serve.go logs the cause with neither field, so it is a site to fix rather than a pattern to copy: a container 500 shipped to its shape is unattributable to a tenant. Six failure sites route through one remoteReadSubject, and the fixture gives the remote repository id a different value from the parent repositories.id so an arm logging the wrong row fails.
• Content-Length comes from a different S06 call per verb, and the spec said otherwise. A GET reads the length off the BlobContent its own open already returns; only a HEAD calls BlobInfo. The spec's service-generated header list claimed both verbs call BlobInfo. S12's blob response-header table already writes the split down ("from BlobContent.Size() (GET) or BlobInfo (HEAD)") and its manifest pull follows it, so the hosted spec settles the shape and this arm matches it. Corrected in the spec with a Resolutions entry; the observable contract — a GET and a HEAD returning the same length, never container_remote_manifests.size — is unchanged, which is why the criterion and the schema note needed no edit.
• The cache-wrapper edit is Step 13's. Completing internal/format/oci/remote_cache_store.go — adding BumpLastDownloadedAt to the embedded remoteCachePromoted, the var _ remote.CacheStore anchor, and the doc comments that count the promoted methods as a pair — was agreed with Step 13's owner and written up in !1714 (closed). That MR is closed rather than merged on purpose: its corrections fold into the MRs they constrain, and !1762 (merged) carries 11b's half of that fold. The 11b wording it replaces is newer than !1714 (closed) — it rode in on 11a's merge, an hour after !1714 (closed) closed — so nothing was reversed, it was just never reconciled. Two reasons the edit is not Step 11's: no arm of Step 11 reads the wrapper (the header writer, the digest gate, and both serve paths compose the datastore store directly), and Step 13 is what stops compiling without it. So remote_cache_store.go is deliberately absent from this diff. Step 13's own Files list and scope are left for that step's MR, so its entry and the code satisfying it land together.
• Two accepted smells, named per CLAUDE.md. 11c modifies remote_serve.go too, so the file is reviewed twice rather than once — the alternative, one MR per file, would put the header writer in neither arm's review and leave the two header criteria without an owner. And 11b and 11c both edit the option slice in wire_oci.go, one hunk each; the sequential merge order is what keeps that from being a conflict.
• Three criteria are asserted here whose plan rows sit elsewhere. Criteria 41 and 42 (Content-Length on every arm; a manifest read never redirected in either mode) sit in 11c's row, and criterion 10's warm half is on Step 14 — but each has a manifest half only this MR can assert. !1762 (merged) says so, so once it merges those assertions do not read as scope creep against 11b's row, and 11c's and Step 14's reviewers do not write them twice. Criterion 39 (nosniff on every proxied read) is not one of these — 11b owns it outright.
• Keep the shared helpers package-level. 11c creates internal/format/oci/remote_blob_serve.go and composes the header writer, the error-envelope writers, and remoteReadSubject from this file. Narrowing any of them to the manifest arm's own flow under review pressure breaks the sibling arm.

This MR carries a spec amendment

docs/specs/S16-container-remote.md is amended here, not just followed. The counter leg was removed from the container remote code, and five places in the spec still described the store seam it used to reach — so on main the spec promises a downloads_count path no implementer can build and no suite can assert. Corrected: the Download signals opening sentence, the mechanism paragraph, the gating note under the acceptance criteria, the S18 follow-up, and the dependency-table row. The mapping row itself stays, because the event is still what a container remote read is specified to produce; what changes is who emits it, which is S18 rather than S16.

Two of those sites cited S14 as landing the identical requirement "exactly this way". That held while S16 shipped a no-op stub behind its own seam and is now the opposite of the decision — Maven ships the stub, S16 ships nothing — so both name the difference instead of claiming parity. The Resolutions entry records what the stub cost and why it came out.

Flagged separately because a spec amendment is a different review class from the arm this MR builds, and it is the one change here that alters what the spec promises rather than what the code does.

Reviewable size — the rest of the argument

Tests at 81% sit inside the 72–85% band the sibling building-block MRs run at, so "most of it is tests" is not the justification for the size. The Not split further paragraph above is.

Test plan — the rest of it

Set-Cookie and WWW-Authenticate are named explicitly in the closed-set assertion, so a reviewer grepping the rule's own header names lands on an assertion. The no-relay rule is pinned on the error envelope as well as on success: mutation-checked by setting WWW-Authenticate in writeRemoteReadError, which turns it red.

The download signals are asserted on the timestamp bumps alone. There is no counter assertion and no counter seam to assert against: the whole downloads_count leg is gone from the container remote code, so both the emission and the pipeline belong to S18 now, and a suite pinning a read to a counter seam would fail a correct implementation. A repeat read inside a configured window records neither timestamp, which is what distinguishes a threaded value from the constant this MR deleted.

Diffed against the sibling cache-hit serve suite, internal/format/maven/remote_serve_integration_test.go, for subtests dropped rather than absent — all 21 of its functions are accounted for in the suite header, as a counterpart, an inverted answer, or a documented non-transfer. Three answers are deliberately inverted: a manifest answers 200 to a Range where Maven answers 206, a container HEAD records a download where Maven's does not, and a nil seam is refused at construction where Maven falls through to its fill stub. The plan's Tests entry pointed at npm for this diff; npm ships no remote read suite, and !1762 (merged) corrects it.

Every //nolint token in the touched files was measured rather than inherited — six of the seven fire, and the one that did not (contextcheck on the boot suite) is removed. Four identical dead contextcheck directives already in that file are left alone; they are not this change's to remove. Integration-tagged lint on cmd/artifact-registry reports 11 findings, all in wire_npm_* files: none on a line this MR added and none of them contextcheck.

The run recipe needs no edit — it never names wireOCI and seeds kind=hosted only, so nothing in it goes stale against the new wireOCI parameter or the filled manifest slot.

Review passes

/implement-step, /simplify, two /validate-step passes, and one /review-branch pass.

The second validation pass found one blocker, in text rather than behavior: the spec claimed both verbs source Content-Length from BlobInfo, which the code contradicts on a GET and which the plan repeated in two places. Resolved on the sibling ladder rather than by picking a side — S12 already writes the split down, so it is a hosted-spec resolution with the code matching, not a code-wins call. S14 and S15 were checked too: Maven's code splits it identically but its spec never says so, and npm names no length source at all.

It also found one coverage gap and several text inaccuracies. The no-relay rule was pinned on success responses only, so a fill path that grew an upstream copy loop onto the envelope writer would have stayed green on exactly the response an upstream 401 makes most dangerous — fixed here, and so are the spec's two stale claims the per-route digest split had corrected everywhere else. The plan's Files cell was missing two paths this MR has to touch again and its criteria note quoted a phrase only one of three criteria carries; both are in !1762 (merged).

The branch review raised no blocker in behavior. It found one test gap and a set of claims that had drifted behind the branch's own late edits, all dealt with — here where the bullet does not say otherwise, in !1762 (merged) where it does:

• The configured staleness window had no end-to-end guard. Every suite in internal/datastore built its store with time.Hour, which is both the config default and the value the store's deleted package constant carried, so nothing distinguished a store that honors its window parameter from one that ignores it — the hazard NewContainerRemoteCacheStore's own doc comment names. The new arm uses a three-hour window against a two-hour-old manifest. Replacing the field with time.Hour in the bump statement's caller fails that arm and nothing else in the family, including the pre-existing throttle test, which is what shows the gap was real.
• remote_repository_id is overloaded service-wide, and the comment said the opposite. It claimed every other emitter of the column uses the name for the same entity, naming internal/remote's health and standalone paths and cmd's audit sink; those carry the parent repositories.id, which remote_audit_sink.go warns about explicitly. The name stays, because S14, S15, and S16 each fix it in their proxy wide-event field lists and internal/format/oci already ships it for the remote row at four sites — so this arm matches both its spec and its package, and inherits a collision rather than creating one. docs/dev/observability.md now carries the split as its own section, with maven_remote_proxy and npm_remote_proxy recorded as unsettled rather than as agreeing: both are spec-only with no emitter, and neither field list says which id it means.
• Three documents contradicted the code this MR ships; two are fixed here. The config reference's own row still called the staleness window unread, nine lines under an intro saying the opposite. And the read half's header comment said the oci cache wrapper does not exist on this branch — it is on main, carrying three of remote.CacheStore's four methods. The third is the plan, which said 11c branches off main when it in fact stacks on this MR; that matters because the plan is the artifact a reader trusts for merge order, and it is corrected in !1762 (merged).
• Three stale symbol names. Two comment leaders still named TestRemoteCacheSum_* after the rename to remoteCacheSumFromColumn, and the mirrored-suite paragraph cited a Maven test that does not exist. The same round found two plan criteria numbers wrong — the prose naming criteria "21, 32, and 36" wanted 37, and criterion 39 sits in 11b's own row rather than 11c's — and those are in !1762 (merged).

Four items are deferred to the blob arm's MR rather than fixed here: the serve path's three integrity 500s are log-only and want a decision in S16's Follow-ups that shapes Step 17's metric set; two failure paths log a field set no test asserts; the shared response writer has no allocation ceiling, which starts to matter once Steps 14 and 15 build on it; and the intermittent test:race failure below wants its own issue.

test:race may flake red on this branch and it is not this MR's. pg.StringExp(pg.NULL) and pg.TimestampzExp(pg.NULL) in container_remote_repositories_update.go and its npm sibling hand jet's package-level NULL singleton to a wrapper that writes to it, which races under parallel subtests. Neither file is in this diff.

Download-signal quota buckets in the serve tests

Every serve fixture takes its own parent repositories.id from uuid.New(), so parallel tests sit in separate buckets of 11a's per-repository download-signal sub-quota rather than contending on one entry. That is what awaitDownloadSignals needs: a shed dispatch never reaches the seam, so a test sharing a full bucket would time out rather than fail on its own subject.

The package-level ids are the namespace, the container repository, the remote repository row, the image, and the manifest — fixed and mutually distinct so a swapped one is visible in a failure. The parent repositories.id is deliberately not among them.

Related to #288