docs(plans): correct S16's Step 11 entries and its criteria map

What this records

This MR corrects Step 11's entries in docs/plans/2026-07-30-container-remote.md. Most of the corrections were authored on the Step 11b and 11c branches, alongside the work that found them; the rest come from this MR's own review. CLAUDE.md guardrail 5 keeps plan edits — the Status table included — out of step MRs, so they land here instead. One file. It also fills 11c's Status row with !1764 (merged), per guardrail 5 — the row is recorded when the step MR opens, and an empty cell means "not yet recorded" rather than "not started".

Criteria numbering

The Manifest and blob proxy map now reads at 45 criteria. Three were inserted into that section of the spec after the map was written, in two rounds: the suspended-namespace write 403 and the manifest/blob DELETE 405 at 28 and 29, then the envelope-decode refusal at 35. Every citation from 28 on was low, so the map pointed Step 8 at the delivery-mode criteria instead of the cache-fill transaction ones, and pointed the shared header writer one short of nosniff.

Each shifted number was read back against the spec's own text rather than derived from the arithmetic, so a fourth insertion nobody noticed would have shown up as a mismatch. The note below the map carries both rounds and a worked example.

Criteria ownership

Criteria Was Now
28, 29 no owning row, recorded as a gap Step 10's row — its merged tests assert both
17 (range on a freshly cached blob) 11b 11c, the arm that ships it
21, 32, 37 allocated whole to one MR split across the two arms, the way 16 already was
10, 41, 42 unstated manifest halves 11b asserts while the rows sit elsewhere
40 Step 11's row, and 11c's, whole one clause in Step 11; the other two land in no step

The 10/41/42 row is the one that matters at review time. Left unstated, those assertions read as scope creep against 11b's row, and 11c's and Step 14's reviewers write them again. Criterion 39 (nosniff) is explicitly not one of them — 11b owns it outright, which is the other half of that correction.

Criterion 40 is the last row and the only one that removes coverage rather than moving it. Two of its three clauses need an upstream 200 — the omitted Docker-Content-Digest answered with the sha256 the fill computed, and a disagreeing header losing to it — and Step 11 serves cache hits without issuing an upstream call. Only "No proxied manifest or blob read omits the header" is reachable there, and both arms assert it, which puts 40 alongside 10 and 42 rather than whole in 11c's row. Steps 14 and 15 do not pick the other two up; their nearest rows are 36 and 43. The map now records that half as a gap in this plan. Allocating it is left as its own call rather than folded in here.

Criterion 41 stays in 11c's row, but not for the reason the first draft gave. That draft said the GET-versus-HEAD equality clause is blob-only; the criterion says "a GET and a HEAD for the same cached artifact", and 11b ships the manifest HEAD. The row placement now rests on the delivery difference — BlobInfo on a HEAD against the opened BlobContent on a GET — and says it is a choice about which reviewer should be looking.

File sets

  • 11b's Files cell named three files where its own prose required a good many more, among them internal/format/oci/remote_download.go and docs/dev/observability.md, whose comments describe a dispatcher with no caller and a counter not yet emitted — both false once the arm lands.
  • 11c is pointed at a new internal/format/oci/remote_blob_serve.go rather than the remote_serve.go 11b creates, so it can be authored while 11b is still in review.
  • internal/format/oci/remote_cache_store.go leaves Step 11 entirely. Completing that wrapper is Step 13's, agreed with Step 13's owner and written up in !1714 (closed) — closed rather than merged on purpose, because its corrections fold into the MRs they constrain. No arm of Step 11 reads the wrapper, and Step 13 is what stops compiling without the method. Dependency rows 11 and 13, Step 8's entry and Step 11's merge-order bullet follow. Step 13's own Files entry carries the enumeration of that edit — the BumpLastDownloadedAt promotion, the var _ remote.CacheStore anchor, and the doc comments counting the wrapper as three of four — alongside the manifest_max_payload threading it already had, so Step 11's pointer at it resolves to something an implementer can read.
  • 11c's entry for internal/format/oci/remote_serve.go names four edits, not three doc comments. The fourth is resolveRemoteRepository taking its 500's log message from the caller: a signature change on a function both arms call, which is what makes the file a shared edit rather than bookkeeping, and what 11c's reviewer checks the diff against.

The second review round on 11b

The entries were first written against 11b as it stood after one review round. A second round landed five more changes before 11b merged, and the rule this plan states two paragraphs earlier — that each round on 11b ends by asking whether the blob arm shares the shape just fixed — is the one the entries then failed. This is the correction that most changes what a reader does next.

Two were shapes, and both are now discharged on the blob side:

  • The manifest arm gained a guard refusing a cache row whose digest and blob_sha256 hold different sums. The blob arm reads the same two columns the same way and had no such guard. It refuses the row now, on its own justification rather than the manifest arm's borrowed: a layer has one representation, so its OCI digest is the sha256 of the bytes blob_sha256 addresses, where the manifest arm has to argue from every upstream fetch asking for the identity representation.
  • resolveRepository was lifted to package level explicitly so the blob arm would not copy it, so the blob arm calls it. The lift left the 500's log message reading oci remote manifest read: …; it takes the message from the caller now, so an operator filtering by route gets the route they filtered for.

Three were file moves, and they are what the Files cells had wrong. The manifest builder and the staleness-window selector left wire_oci.go for a new wire_oci_remote.go, that selector's test moved with it, and remote_serve_test.go split its fixtures and header-writer cases out. 11c's cell named wire_oci_test.go for a rename that now happens in wire_oci_remote_test.go; 11b's named neither the file it created nor two of the four test files it shipped.

Stacking

The bullet saying 11c is not stacked was wrong: 11c carries 11b's commits plus its own. The accepted-smell paragraph resting on that claim said whichever of the two merged second resolves a one-line wire_oci.go conflict; stacked, 11c inherits that line, so the shared file costs a rebase if the two are ever unstacked, not a conflict.

That paragraph also said the option slice is the only file the two arms share, which the Files cell four rows down contradicts by listing remote_serve.go as an 11c edit. Three files are shared. The paragraph names them and says which sharing is the smell worth flagging: 11c is reviewed against remote_serve.go, whose arm it is not changing, so a reviewer has to tell a comment correction from a behavior change by reading each hunk.

The counter seam

Four places still told Step 11 to wire and assert a repositories.downloads_count increment. S16 ships no such seam; both the emission and the pipeline arrive with S18.

Also corrected

  • The malformed-digest refusal differs per arm — 400 DIGEST_INVALID on the blob arm, 404 MANIFEST_UNKNOWN on the manifest arm — where the plan claimed 400 for both.
  • The Tests entry pointed at npm for the dropped-subtest diff. npm ships no remote read suite to diff against; it points at Maven now.
  • internal/remote/fetch.go:176 becomes :213, which is where main has it.
  • 11b's Status row is filled.

Why it is separate

This is a plan MR, not a step MR, so it carries no (S16 plan: <step>/<total>) title marker and no step number in the title.

Merge order

None. This and !1764 (merged) can merge in either order.

It needed !1707 (merged) to merge first, because the counter-seam corrections describe a spec that says S16 ships no counter seam and that amendment travelled in !1707 (merged). !1707 (merged) has merged, the amendment is on main, and this branch is rebased onto it.

It was briefly marked blocked by !1764 (merged), the Step 11c blob-arm MR, and that block is removed. The reason it looked necessary was this MR's own phrasing, not the ordering: the section on what 11b's review rounds cost the blob arm was written in review-report register — "all five are discharged", "now refuses it too" — which are claims about an unmerged branch and false on main until it lands. A plan describes steps that have not merged; this file already does that for Steps 12 through 19. Those paragraphs now say what 11c does rather than what has been done, so they read correctly whichever MR lands first and do not decay into a stale snapshot afterwards.

Two reasons to prefer no dependency over the ordering. !1764 (merged)'s reviewer reads the Step 11 entry on main to know what 11c is meant to do, and that entry is the uncorrected one — it says 11c modifies remote_serve.go (it creates remote_blob_serve.go) and hands it criteria 38 and 39, which 11b owns outright. And holding this MR widens the drift it exists to fix: the reason it needed a second pass at all is that a review round on 11b moved files out from under its Files cells, and every further round on !1764 (merged) is another chance for that.

The two touch disjoint files — !1764 (merged) changes nothing under docs/plans/, this changes nothing else — so they cannot conflict.

Verification

Every renumbered citation was read back against ## Manifest and blob proxy on main rather than derived from the arithmetic: 45 criteria, each covered exactly once across the map's rows, no gaps. The two Step 10 test names the note cites exist at internal/format/oci/remote_dispatch_test.go. internal/remote/fetch.go:213 is func NewFetcher( and lookup.go:18 is func NewLookuper(.

The file sets are read off the rebased 11c branch rather than predicted: it now sits on main past 11b's merge, builds, and is green on ./internal/format/oci/ and ./cmd/artifact-registry/, with golangci-lint reporting 0 issues on both packages. The blob arm's new disagreeing-pair row was mutation-checked — without the guard it serves a 200 carrying the wrong Docker-Content-Digest.

Edited by Radamanthus Batnag

Merge request reports

Loading
Loading