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.goanddocs/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.gorather than theremote_serve.go11b creates, so it can be authored while 11b is still in review. internal/format/oci/remote_cache_store.goleaves 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 — theBumpLastDownloadedAtpromotion, thevar _ remote.CacheStoreanchor, and the doc comments counting the wrapper as three of four — alongside themanifest_max_payloadthreading 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.gonames four edits, not three doc comments. The fourth isresolveRemoteRepositorytaking its500'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
digestandblob_sha256hold 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 bytesblob_sha256addresses, where the manifest arm has to argue from every upstream fetch asking for the identity representation. resolveRepositorywas 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 readingoci 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_INVALIDon the blob arm,404 MANIFEST_UNKNOWNon the manifest arm — where the plan claimed400for 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:176becomes:213, which is wheremainhas 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.