feat(oci): revalidate a stale tag against the upstream (S16 plan: 14/26)

Reviewable size

12 files, +1,619 −132, measured three-dot against main. Past the 500-line ceiling in development-model.md, so Justify a diff past 500 reviewable LOC in the MR description wants the split written out:

Group	Lines
Tests	+1,185 −46 — remote_revalidate_test.go 1,078/0, remote_manifest_test.go 96/46, remote_serve_fixtures_test.go 11/0
Production Go	+290 −76 — remote_revalidate.go 155/0, remote_manifest.go 95/36, remote_serve.go 17/23, standalone.go 14/17, remote_operations.go 6/0, remote_errors.go 3/0
Run recipe (.claude/skills/)	+143 −9
Configuration reference	+1 −1

Stripped of blanks and comment-only lines, the Go is 847 reviewable added lines.

Why not split further. Seven tenths of the diff is coverage, and the new file is one function plus its fallback helper. Four of the six production files change nothing but comments — this MR is what makes the "a stale tag takes the interim 501" and "the revalidation has no caller" sentences 14-1 through 14-3 left behind false, and A comment must describe the code as the same change leaves it wants those corrected in the change that falsifies them. The behavior itself has one seam, between the conditional request and the fill it delegates to on a 200, and the fill is already 14-3's.

Why

Step 14-4 of the S16 container remote plan, the last of the step's four MRs. 14-1 landed the refusal surface, 14-2 the by-digest fill and the composition root, 14-3 the by-tag fill. This part closes the one read still on the interim 501: a tag past its cache_validity_hours. After it, the manifest arm answers every read shape it owns.

What

The conditional request. A stale tag sends a GET upstream carrying the row's upstream_etag on If-None-Match. The stored bytes are replayed rather than an entity-tag re-derived, so an upstream comparing them byte for byte matches.

304 — the copy is confirmed. upstream_checked_at bumps, the tag pointer and the manifest row are untouched, nothing is fetched and nothing is committed, and the cached manifest is served through the same path a fresh hit takes. Negotiation still runs: an Accept excluding the stored media type is 404 MANIFEST_UNKNOWN on this path too.

200 — the upstream re-tagged. The tag moves to the new manifest through 14-3's fill rather than a copy of it, so the row, the ETag, the digest and the download signal all come from one implementation. The manifest the tag used to name stays resolvable by its own digest; the re-point deletes nothing, because reclaiming that row is a retention concern.

A null upstream_etag re-resolves in full. No validator goes out, and age alone never serves a stale copy. A 304 answered to a request that sent no If-None-Match confirms nothing, so it is treated as an unusable answer rather than as a confirmation.

cache_validity_hours = 0 pins the tag. The freshness verdict is the store's, computed in SQL, so the arm adds no second rule — a pinned tag never reaches this branch at all.

A transport failure serves the stale copy. So does a body-cap overrun, which arrives the same way. A status the upstream deliberately chose — 401, 403, 404, 429, 5xx — is propagated instead, because it is an answer rather than an outage.

The unhealthy-remote gate covers this arm. 14-1 placed the refusal on the miss path; it now sits ahead of the branch between a miss and a stale tag, so both take it and neither is served from cache.

Worth a reviewer's attention

The cache-fallback route reads the row back, and that is the one non-obvious piece. remote.CacheFallback is the seam every format shares, so what it reports is that a copy exists, not which one — remote.LookupResult carries none of the columns a manifest is served from. Serving the row the arm was already holding would answer with a manifest a concurrent fill may have superseded, since that row was resolved before the upstream attempt ran. So the row is read back through RemoteReadCache and the read-back's row is what is served. Three failures fall out of that and are each covered: a row that vanishes between the two reads takes the same 503 an empty fallback earns, a read-back that fails outright is this service's own 500, and a client that left during the upstream attempt is answered 499.

The digest cross-check stays gated, and the seam that lifts it is now on main. !1807 (merged) merged while this MR was open, putting a fourth parameter on RemoteManifestFiller.FetchManifest. commitUpstreamManifest is the one call site, shared by the miss and the revalidation, and it passes false — which is what main's own callers do: the seam ships wired and switched off. TestRemoteManifestRevalidate_DisagreeingUpstreamDigestHeaderIsNeverCached therefore still skips.

What is left is container-side — the fetch opting in, and remote.ErrUpstreamDigestMismatch on isRefusedUpstreamContent, which have to land together because the second is what gives the first its status. That is #918, filed for it, and commitUpstreamManifest's obligations banner names it.

The revalidation suite gains the pin main added for the miss: that the read passes false. It earns a line of its own rather than being inherited by proximity — a tag names no digest, so the upstream's header is the only reference value that read could ever be held to, which makes the flag's value the whole of what protects it. Mutation-checked by flipping the argument.

It is gated the way 14-3 gates the miss half rather than left as a bare skip: arrange, act and assertions are all written, the skip sits between the act and the assertions, and a Fatalf above the skip fails the moment the arm starts refusing these bytes. Lifting the gate is then a one-line deletion instead of a test that passes asserting nothing. The revalidation half is the sharper one — a 200 re-points a tag the cache already holds, so the exposure covers warm tags and not only tags nobody has pulled — and commitUpstreamManifest's Obligations this arm leaves open banner is where that is written down.

A 304 restarts the freshness window on a representation the request never negotiated. fillCacheMiss said a tag's negotiated media type lasts one window and that the window closes on its own. It does not. The conditional request goes out with this client's Accept lines and the row's stored validator, so an upstream that answers 304 on the validator alone confirms the stored representation and bumps upstream_checked_at — including for the client that is then refused 404 MANIFEST_UNKNOWN over the media type it asked for. Only a 200 re-negotiates. The comment now says that and TestRemoteManifestRevalidate_NotModified_AcceptExcludingTheStoredTypeStillBumps pins it; the existing 404 test stopped at the status, which reads as if the refusal happened before the upstream was asked.

writeRefusedFill gains a third route. Its census covered the two mappers only. confirmCacheFallback composes its own respondUnavailable and hands over the failed attempt's cause. That cause is nil whenever the upstream answered with a status this slice cannot use — an unsolicited 304, an undecodable 200, a 405/407/426 — because those reach the fallback route through mapUpstreamStatus with no Go error, so both log sites attach error_message only when there is one. respondUnavailable is also SelfDecided, so the two calls render through writeUnavailable rather than the relayed-refusal arm.

The degraded serve's own log line redacts that cause. It is the one site in this arm where a fetch failure is logged by a read that still answers 200, so the fill suite's three redaction rows, all of them refusals, cannot reach it. remote.RedactedForLog rewrites the *url.Error, whose Error() serializes its URL field raw, so a stored url column holding userinfo would otherwise put the credential in the line that records the failure behind a served manifest. TestRemoteManifestRevalidate_DegradedServeLogRedactsTheFetchFailure pins it, mutation-checked by dropping the wrapper.

The upstream_etag write is the store's, and the format-level test says so. TestRemoteManifestRevalidate_Modified_StoresTheNewUpstreamEtag asserts the half that is production here — that the next conditional request replays the validator the re-point stored — and requires the fixture's own write as a precondition rather than dressing it up as coverage. The column write is asserted against the real table in container_remote_cache_write_integration_test.go:1211,1254.

Two tests assert that nothing was added. The unhealthy gate and the pinned tag both produce answers no code in remote_revalidate.go writes. They are worth their lines as guards against a second gate and a second freshness rule being written into this arm later.

The concurrency note keeps one claim, and the recurring burst is it. The unbounded-concurrency comment on remote_operations.go was written when the cache-miss fill was the only manifest read that fetched, so it said the herd is one burst per uncached tag and nothing after. A stale tag fetches again once per freshness window, and a fallback-eligible failure deliberately leaves upstream_checked_at unmoved — so against a failing upstream the burst recurs per read for as long as the failure lasts. That is the one sentence this MR adds to the note. The longer version this branch carried before the merge — the session-size upper bound, the image-row lock that serializes fills under one image, and the held-connection cost of a blackholing upstream — is gone, and main is what removed it: the comment-cap gate compressed the whole block to a banner and a line, and re-adding those claims would put the block back over its cap. Issue #752 is still what closes it; RemoteBlobFill is where main now carries the pointer.

The merge down from main adopts main's wording rather than re-deriving it. The comment-cap gate landed on main after this branch was written, and the parent's own caps commit rewrote the same blocks this arm had edited, so every file the parent touched conflicted. The base git picks for a squash-merged parent predates that work, so the merge uses a virtual base whose tree is main's and whose parents are main and the parent branch's tip. git diff --stat main...HEAD is the check that it was honest: 12 files, all of them this arm's, and no main-side file reverted.

Three resolutions are worth a look, because taking either side whole would have shipped a false claim:

• commitUpstreamManifest keeps this branch's free-function form, which the revalidation reuses; main still has the method that writes its own refusal.
• remote_serve.go and remote_operations.go say a blob miss still takes the interim 501. main has the blob miss fill, but WithRemoteBlobFill has no caller under cmd/, so no boot reaches it.
• remote_download.go takes main's version whole. The paragraph this branch had edited is one the compression drops, and its "only the manifest arm settles a read against an upstream" claim does not survive Step 15 anyway.

The gate is at zero for this diff, and pre-commit run comment-caps --from-ref origin/main --to-ref HEAD passes. The merge commit itself was made with SKIP=comment-caps, not --no-verify. The hook resolves its base as merge-base origin/main HEAD, and on the pre-merge HEAD that base predates the parent's squash, so it reports every block main brought in, across ten packages. Once the merge commit exists the base is main itself — which is what the CI job uses too, HEAD^1 on a merged-results pipeline and the MR diff base on a detached one — and nothing is left.

The 4 MB / 250 KB gap 14-2 and 14-3 flagged is unchanged. ociRemoteManifestBodyBounds still resolves to a 4 MB default against ADR-004's 250 KB. Inherited, not introduced; the remediation is a handbook amendment or a config correction, neither of which belongs on this branch.

Divergence from the plan's Files list

Nine of twelve paths sit outside 14-4's Files cell, which names remote_revalidate.go (Create) and remote_manifest.go (Modify), with remote_revalidate_test.go under Tests. Four are comment-only staleness repair that A comment must describe the code as the same change leaves it forces here — remote_serve.go, remote_errors.go, remote_operations.go and standalone.go, and the plan never lists the last one under Step 14 at all. remote_manifest_test.go and remote_serve_fixtures_test.go carry fixture changes the revalidation suite reads through: the fill fake now models upstream_etag and the cache fake gains a row getter.

docs/dev/configuration-reference.md appears in no Step 14 cell. Its one-line change is a claim correction, not a schema change: the server.timeouts.write row said the container read path re-arms the write deadline on the cache-miss fill only, and the revalidation arms it too, through the same call site.

.claude/skills/run-artifact-registry/SKILL.md is annotated "Modify in 14-1 and 14-2" only, and Keep the run recipe current alongside build and launch changes requires it here — this MR changes what a stale-tag route answers. driver.sh appears nowhere in Step 14. Its change is not a comment: seed-remote's container_remote_blobs INSERT joined blob_storage_attachments plainly, so when a namespace already held the layer the join matched every attachment row, the SELECT repeated, and the rows produced collided with each other on unique_container_remote_blobs_ns_id_image_id_digest — a failure NOT EXISTS cannot prevent, and one that reads as a broken fixture rather than as a re-run. A CROSS JOIN LATERAL … LIMIT 1 takes one attachment per blob and fixes it; the completeness check below already handled the same multiplicity with EXISTS. It is here because it blocked the smoke run this MR owes.

Plan changes are in a separate MR

Step MRs do not edit the plan file. This branch carries none.

Review this against the Step 14 entry in !1831, not main's. main's entry still costs Step 14 as one MR and has no per-MR table, so it gives no Files list and no criteria allocation for this part. The SKILL.md and driver.sh attributions above want a follow-up there rather than a change here.

Merge order

Every parent has merged, and this branch now targets main. 14-1 (!1869 (merged)), 14-2 (!1872 (merged)) and 14-3 (!1873 (merged)) are all in, so nothing gates this part any more. The parents' review rounds are folded in, resolved toward keeping both sides' facts: the response-grace tail's arming site corrected to buildUpstreamFill, the #433 obligation census on commitUpstreamManifest, the Close-refusal fixture case, the npm-tarball paragraph in standalone.go, and namespace.Resolution dropped from the answer helpers, which this arm was still threading.

main moved under the branch while the parents merged, so this branch merges main down rather than rebasing — it was cut from a squash-merged parent, and a rebase would replay this arm's commits over a history that already contains the parent's work in one commit. It has been merged down twice: once when the parents landed, and again for !1807 (merged), whose fourth parameter on FetchManifest conflicted with this branch's own rewrite of the same declarations.

Open MRs overlapping these files:

• !1904 (SKILL.md, driver.sh, configuration-reference.md, remote_errors.go, remote_operations.go, remote_serve.go) — Step 15 part 3, which wires the blob fill into the root and so falsifies the "no composition root wires the blob fill" sentence this MR leaves in remote_serve.go and remote_operations.go. Whichever lands second owes that correction.
• !1903 (remote_errors.go) — Step 15 part 2.
• !1895, !1916 and !1885 (SKILL.md).
• !1997 (SKILL.md) and !1011 (configuration-reference.md) — both Draft.

Whichever lands second rebases; no pipeline reports it.

Guardrails with nothing to add

• Conformance tests. The OCI conformance suite runs against hosted repositories and cannot reach a kind=2 route, so it cannot cover this behavior.
• e2e scenario catalogs. docs/testing/e2e/oci.md puts virtual and remote repositories out of scope until the capability ships, so no scenario is added or affected.
• Bruno / OpenAPI. No operation added, renamed, or removed.
• Database review. No query changed; the revalidation reads through the existing RemoteReadCache lookup.

Run recipe

SKILL.md gains a stale-tag revalidation section: how to force staleness by backdating upstream_checked_at rather than pinning cache_validity_hours to 0, what each upstream answer makes the row do, and which two of them Docker Hub can produce without a controlled upstream. The unhealthy-refusal paragraph is corrected to say the refusal runs ahead of the branch between a miss and a stale tag, and the negotiation row now says the refused read still bumps upstream_checked_at, so it has to be backdated again like every row above it. driver.sh smoke passes 72/72 on this branch, matching the tail SKILL.md itself states; it asserts none of the revalidation rows, which need a live upstream, and the section says so.

Related to #288