feat(oci): OCI content discovery, repeat read and refusal, Steps 34-36 (S08 remote, batch 11/12)
Summary
OCI content discovery, repeat read and refusal batch per the S08 remote
goal-run's annex §7 MR clusters ("OCI content discovery, repeat read, refusal").
Lands Steps 34-36: oci.remote.tag-list-proxy, oci.remote.referrers-proxy,
oci.remote.repeat-read, oci.remote.not-found,
oci.remote.write-refused-manifest, oci.remote.write-refused-blob-upload.
That completes the OCI band's thirteen ## Remote rows; only Step 37's
strict-equality flip is left.
Related to #47 (closed). Divergence index: #54. Spec-amendment candidates: #70.
!271 (merged) and !266 (merged) have merged, and main is merged in here. The branch no
longer stacks on anything. The merge was not textual and its commit message
records what each conflict was decided on; §The merge below has the short
version.
Steps
Step 34: the two listing rows
remote_listings.go, remote_listings_test.go, plus a computed-listing seam in
remotefake. Rows: oci.remote.tag-list-proxy, oci.remote.referrers-proxy.
AC #21 (closed), #29 (closed).
Each row asserts only that this run's own entry appears, because the upstream may hold other operators' content. A listing carrying unrelated entries alongside this run's must pass, and that is a control rather than a comment: the reference matrix injects a foreign tag and a foreign referrer and both rows pass with the mutation confirmed fired. A row asserting an exact listing fails those two and passes every other case in the matrix.
No finding echoes an entry the repository under test chose. The echo was
removed rather than scrubbed, on servedBytesClause's doctrine, and the tests
kill both an unscrubbed echo and a scrubbed one.
oci.remote.referrers-proxy seeds its own referrer through a second
SeedAndSettle rather than a fourth artifact on the shared fixture, which would
have reopened seven committed rows. The cost is filed as plan Open 9.
Step 35: repeat read and absence
remote_repeat_read.go, remote_not_found.go and their tests, plus an
AbsenceAdapter seam. Rows: oci.remote.repeat-read, oci.remote.not-found.
Only the absence row carries NeedsUpstream: false. AC #16 (closed), #17 (closed), #29 (closed).
The repeat read covers both artifacts, twice each, and asserts byte
identity and the premise that comparison needs: a first read that served no
bytes is a finding, because bytes.Equal(nil, nil) is true and a relay
answering 200 with an empty body would otherwise satisfy both pairs. Whether
the bytes are this run's stays the pull-through rows' finding. It judges no
Content-Length framing: see §What the review changed.
oci.remote.not-found is the one row of this pair that runs under
--upstream-free-only, and the run exits 0 there, measured end to end.
Step 36: the two write-refusal rows
remote_write_refused.go and its tests. Rows:
oci.remote.write-refused-manifest (NeedsUpstream: true, Seeded write
refusal class, so it skips under --upstream-free-only) and
oci.remote.write-refused-blob-upload (NeedsUpstream: false). AC #18 (closed), #22 (closed),
#29 (closed).
Both no-trace shapes, with non-interchangeable coordinates. The manifest
PUT goes at a coordinate no row seeds and a read afterwards returns 404;
the DELETEs go at the row's seeded coordinates and a read afterwards
returns the seeded bytes. Pointing the PUT at the seeded tag would make
"creates nothing" vacuous, and the mutation that does so kills eight tests.
The empty-Allow assertion is not vacuous, and there is a mutation proving
it. Upload routes carry a present-and-empty Allow; empty compared against
empty passes anything, so the row separates absent from present-and-empty.
(*HTTPError).Allow() returns (value, ok) off resp.Header[headerAllow]
rather than Header.Get, which is what makes that expressible. Dropping the
presence bit kills both refusal matrices.
The merge
main moved under this branch in three ways that mattered.
0d6b385 replaced remoteFixture.RepoName with the paired
UnderTest / Upstream targets, so 23 call sites in this batch's four row
files had to be read rather than renamed. remoteFixture's own doc says why:
a mechanical rename "seeds into the repository being tested, which then relays
green having proxied nothing, and no assertion a relay row makes can see that."
The readings: every relay read, refused write and no-trace read addresses
fx.UnderTest; the referrer's probe, PUT and settle read-back address
fx.Upstream; the two rows that seed nothing resolve only oe.UnderTest().
TestSeedReferrer_... and
TestTestRemoteReferrersProxy_SeedsItsReferrerUpstreamBeforeReadingTheListing
are the backstop, and crossing the pair kills five tests.
!271 (merged)'s last ten commits are on main and were not on this branch, so ten
conflicted files Steps 34-36 never touched take main's version whole rather
than reverting ccf28f3, 6f7a267, 6083b88 and 75bfd03.
module.go's remoteCatalog came out of the auto-merge with six descriptors
twice, because both sides added the same rows.
TestInventory_moduleSlugsMatchCatalogPerKind and
TestCatalog_bucketsHoldTheirOwnSection caught it, which is what those guards
are for.
What the review changed
/review-branch found two blocking defects and twelve warnings. All are fixed;
the five commits after the merge carry the detail.
oci.remote.referrers-proxy took its v1.1 arm through conformance.Skip
after settling twice. That leaves TestCase's seeded marker unset, so
countSeedingRows reads the row as having established nothing and AC #36 (closed) exits
2 blaming --run-id reuse or the credential's write scope. A full band run
hides it; --filter=oci.remote.referrers-proxy against a v1.0-only registry
does not, and AC #29 (closed) makes that invocation a requirement. Plan Step 34's
Acceptance said in bold that this arm reports through SkipAfterSeeding, and
four other places said the same. It went the other way because a comment blamed
anySeedingRowDidNotSkip, a function that does not exist, and attributed it to
Open 6; the defect described is Filed 1, which shipped in !275 (merged). Nothing pinned
the old behaviour, because both constructors agree on every exported field.
A cancelled run rendered a conformance failure in all six rows. !271 (merged) added
relayReadRefusal to write that rule once, after this branch forked, so each
row grew its own refusal renderer without it. A --timeout expiry inside any
read arrived as a transport error with StatusCode 0 and became a JUnit
<failure> for a registry nothing was learned about.
Allow was compared as a raw string. S08 §Write refusal per format states
the rule as "lists exactly the read verbs" and says it asserts "a rule rather
than a table of literals", but the row compared against GET, HEAD byte for
byte, so GET,HEAD and HEAD, GET failed a conformant target. Nothing could
catch it: the double emits the row's own spelling and the negative direction
only staged wrong verb sets. The finding now also names the value that arrived.
repeatReadFraming is gone. It failed a relay that declared no
Content-Length on either blob read, while passing one that declared a length
cold and chunked warm. Neither expresses a cache-state transition, so the
asymmetry was the defect rather than the choice between S08's two readings, and
framing depends on cache state so either reading flakes.
contentLengthUndeclared, oci.remote.head-parity and oci.remote.blob-range
each decline the same judgement in the same words. The clause is now asserted by
no row in any band, which is a real coverage gap recorded in Open 10 rather than
a row that flakes.
Four claims had no assertion: seedReferrer's three reported fields, the
manifest PUT's body, the upload PATCH's Content-Range, and
acceptedReadDetail's verb at three sites. Each survived being falsified.
Open 11 is new: oci.remote.not-found reads a repository no row creates,
which S07 §Resolutions said no row does. Its accepted code set may be narrower
than the protocol admits for that condition, and hosted AC #41 (closed) accepts
NAME_UNKNOWN there. The row is not loosened here, because widening an
accepted code set is loosening a conformance assertion and CLAUDE.md puts that
in its own MR. S07's now-false sentence is corrected either way.
Also: three Files lists were missing files their steps changed, which is the
defect this branch's base commit fixed for Steps 31-33; inventory_test.go's
two recorded blockers for Step 37 and plan Open 8's replacement example were all
falsified by this batch's own catalog rows; and nine plan citations in code named
the wrong entry after the plan renumbered.
Spec coverage
Merged across the three steps. The per-step tables, including each step's
mutation matrix, are in the test(oci): commit bodies (2a3ab6b, ee61845,
85d7fa1).
Acceptance criteria
| # | Criterion | Step | Covered by |
|---|---|---|---|
| S08 AC-13 | Seed through --upstream-url, read the same coordinate through --registry-url, pass only on identical bytes |
34 | TestRemoteListingRows_PassAgainstALinkedPair, ..._SettleOnTheOriginBeforeTouchingTheProxy. For a listing the byte comparison is AC #21 (closed)'s membership question; the byte-exact half is the four relay rows before them |
| S08 AC-14 | The seeding write is confirmed readable upstream within SettleTimeout first |
34, 35, 36 | requireSettledOnTheOriginBeforeTheProxy in each row's suite; the referrers row's second settle is TestTestRemoteReferrersProxy_SeedsItsReferrerUpstreamBeforeReadingTheListing |
| S08 AC-15 | A residue-status refusal skips with a reason naming --upstream-url and the status |
34, 35, 36 | ..._RouteASeedingFailureThroughNewSeedFailureCase (the 409), plus TestTestRemoteReferrersProxy_RoutesAReferrerSeedingFailureThroughNewSeedFailureCase for the second write |
| S08 AC-16 | A repeat read passes only when the response is byte-identical to the first | 35 | TestTestRemoteRepeatRead_PassesAgainstALinkedPair, the two near-miss arms (same-length substitution, re-serialised document), ..._FailsARelayThatServesNothingTwice for the non-empty premise, ..._PassesUnderEveryFramingArrangement |
| S08 AC-17 | An absence row passes only on the format's not-found status and error shape | 35 | TestTestRemoteNotFound_FailsWhenEitherArtifactIsNotReportedAbsent (status and code as separate branches, both routes, both codes non-interchangeable), and remotefake's AbsenceBody seam, without which the shape half was inexpressible |
| S08 AC-18 | Each write verb answers 405 with an Allow listing exactly the route's read verbs; the no-trace check differs by verb shape |
36 | TestRemoteWriteRefusalMatrix_* (3x6 and 5x6 cross products), TestWriteRefusal_AcceptsEveryConformantSpellingOfTheAllowVerbs, ..._StillRejectsAWrongVerbSetHoweverItIsSpelled, both no-trace suites |
| S08 AC-21 | The two listing rows pass on this run's own entry and assert nothing about any other | 34 | ..._PassWhenTheListingCarriesEntriesThisRunDidNotCreate, ..._FailWhenTheListingDoesNotCarryThisRunsEntry, TestTestRemoteTagListProxy_MatchesTheSeededTagExactly |
| S08 AC-22 | Every target-specific assertion is carried by a catalog row whose Notes name its spec | 36 | Both write-refusal catalog rows name AR S16 §Write operations for the code |
| S08 AC-24 | The preflight is first among a remote run's descriptors | 34, 35, 36 | The three TestRemoteCatalog_Registers* tests plus the pre-existing preflight-first guard; all fail on a swapped order |
| S08 AC-25 / AC-26 | Under --upstream-free-only a row needing a fixture skips with the pinned reason and its Fn is not called |
34, 35, 36 | TestRemoteRows_UnderUpstreamFreeOnlyTheAbsenceRowRunsAndTheRepeatReadSkips, ..._TheBlobUploadRowRunsAndTheManifestRowSkips, both asserting the whole run's request log by equality |
| S08 AC-27 / AC-28 | The per-kind inventory guard, and a hosted run registering no remote slug |
34, 35, 36 | TestInventory_moduleSlugsMatchCatalogPerKind, ..._registeredSlugsRespectThePerKindKeepList, requireRelayRegistration's hosted half |
| S08 AC-29 | Each row passes alone under a --filter; a row reading seeded content seeds it |
34, 35, 36 | Every row's Fn is driven in isolation with no other row having run. Distinct repository segments make it falsifiable (repo_segment_test.go's literal table); for a listing row the repository is the coordinate |
| S08 AC-30 | --timeout's remote default counts settling descriptors |
35, 36 | The counting is pkg/conformance's. The referrers row settles twice, which the formula does not model: plan Open 9 |
| S08 AC-33 | A run-attributable seeding failure ends the run with exit 2 carrying its detail |
34, 35, 36 | The 401 cases in both seeding-failure tests, asserting SetupFailure is the *SeedError NewSeedFailureCase set |
| S08 AC-35 | Relay, Repeat read and Seeded write refusal rows carry NeedsUpstream; Absence and Write refusal do not |
34, 35, 36 | The three registration tests, with the false halves stated separately from requireRelayRegistration's true half. The band-wide guard is Step 37's |
| S08 AC-36 | A remote run that established no fixture exits 2 |
35, 36 | pkg/conformance's. This batch's contribution is oci.remote.referrers-proxy reporting its post-seed skip through SkipAfterSeeding, pinned by whole-value comparison against the constructor |
| S08 AC-37 | A resumed settle succeeds only on a byte-identical read-back | 34 | Step 30's TestTestRemotePreflight_ResumedSettleRequiresByteIdenticalContent, which these rows call unchanged. Not covered for the referrer's second seed: the row that would cover it is a resumed-settle row this band does not have |
Per-format and protocol sections
| Section | Step | Covered by |
|---|---|---|
S08 §Per-format rows, tag-list-proxy |
34 | TestTestRemoteTagListProxy_MatchesTheSeededTagExactly (prefix and suffix near misses), ..._TreatsA404AsAFindingRatherThanASkip |
S08 §Per-format rows, referrers-proxy |
34 | ..._SeedsItsReferrerUpstreamBeforeReadingTheListing, ..._SkipsWhenTheRepositoryDoesNotImplementV11 |
S08 §Per-format rows, repeat-read |
35 | Both artifacts, twice each, with the request log pinned by equality |
S08 §Per-format rows, not-found |
35 | Both routes, both codes, and the --upstream-free-only composition end to end |
| S08 §Per-format rows, both write-refusal rows | 36 | The two matrices, the two no-trace shapes, and TestRemoteWriteRefusedRows_SendWellFormedRequests |
| S08 §Per-format behavior a hosted run does not exercise | 34 | ..._PassWhenTheListingCarriesEntriesThisRunDidNotCreate, plus TestOCIAdapter_ComputeListingListsReferrersBySubject which makes a foreign referrer expressible at all |
S08 §Write refusal per format, Allow derivation |
36 | The two spelling tables above; the value is compared as a verb set, which is what the section asks for |
| S08 §Read surface per format | 34, 35, 36 | Each row's request log by equality, and tags/list's "available, not optional" half |
| S08 §Fixture seeding model | 34, 35, 36 | Step 30's purity test, plus per-row repository derivation and TestRemoteListingFixtures_StageDistinctDigests |
| S08 §Assertion sources | 35, 36 | The 405 and UNSUPPORTED pair is target-specific and named in both catalog rows' Notes |
| S07 §Referrers API (v1.1) | 34 | TestOCIAdapter_ComputeListingListsReferrersBySubject, ..._AnswersReferrers (a subject with no referrers is 200 with []), ..._ReferrersIsOrderedByStoreKey |
| S07 §Pluggable behavior the spec does not pin | 34 | ..._SkipsWhenTheRepositoryDoesNotImplementV11, which also pins the reason as probe.go's constant verbatim. The inheritance is inference, filed as Open 5 |
| S07 §Operation: manifest GET / blob GET | 35, 36 | The not-found matrix and the post-PUT read |
| S07 §Operation: blob upload complete | 36 | TestRemoteWriteRefusedRows_SendWellFormedRequests (the chunk, its Content-Range, and the digest over those bytes) |
| S07 §Repository naming | 34, 35, 36 | repo_segment_test.go's literal table, all six segments, plus pairwise distinctness and the naming shape |
| RFC 9110 §15.5.6 | 36 | The absent-versus-present-and-empty separation, resting on (*HTTPError).Allow's presence bit |
| reference-quirks R4, R5 finding 2 | 35 | The framing pair is modelled by the double and asserted of it; the row itself asserts no framing, per Open 10 |
Error cases and security considerations
E-1 through E-10 and S-1 through S-7 are tabulated per step in the three
test(oci): commit bodies. Two gaps are explicit there and repeated here:
- E-4, a
2xxthe write callback reports as not-means-stored, is covered for the shared write and not for the referrer write: reaching it needs an origin fault answering200to a manifestPUTat a digest, which no arrangement needs for any other reason. - AC-37 for the second seed, as above.
The security rows are all covered. No finding in any of the six rows echoes what
the repository under test chose: TestRemoteListingRows_DoNotEchoTheServedListing,
TestTestRemoteNotFound_DoesNotEchoTheResponse and
TestRemoteWriteRefusedRows_DoNotEchoTheResponse each require the absence of a
credential-shaped value, the absence of an ordinary one with nothing for
ScanContent to match, the absence of redact.Sentinel, and the
over-omission direction. The one value deliberately named is the Allow that
arrived, which the verb-set finding names so an operator can tell a wrong verb
set from a re-spelling of the right one. It is scrubbed before it reaches the
message: that branch fires precisely when the value is not the method tokens the
route expects, so "a closed vocabulary of method tokens" is not a property of
what it renders, and the response body arriving scrubbed on Detail.HTTP is not
a reason to leave the header undefended.
TestWriteRefusal_RedactsASecretTheAllowHeaderCarried stages a
credential-shaped Allow and asserts the sentinel replaces the credential while
the verbs before it survive. internal/messagescan reported clean through the
gap, so Allow is now in its OCI selector list and allow in the OCI and npm
identifier lists.
Reference validation
validation/evidence/step{34,35,36}/, runners at
validation/ref-env/mutation-matrix-step{34,35,36}.sh. 14, 11 and 13 cases; all
behaved as designed at the time they ran.
Those runs predate the review fixes and no longer cover the current code.
Ten things changed behaviour after the evidence was gathered: the merge
reassigned 23 call sites between the two repository sides, the referrers row's
skip constructor changed, four refusal paths now skip a cancelled run,
repeatReadFraming was deleted, the Allow comparison became a set comparison
and then a real one (a repeated verb no longer fails a conformant target), the
Allow trim set narrowed to SP and HTAB, the Allow value the finding echoes
is now scrubbed, oci.remote.repeat-read now fails a first read that served no
bytes, the tag-list provenance clause now names the upstream side, and the
cancellation skip reason was reworded to name no fixture.
The matrices need re-running before the evidence claim in this description
should be read as current. The in-repo suite is green and the mutations quoted
above were verified by hand against the current tree, but that is a weaker
statement than the matrices make.
Same addressing caveat as !266 (merged) and !271 (merged): every verdict was gathered through the R20 path-prefix addressing shim rather than the tool reaching Artifact Registry as a client would. R20 is in #54 as a usability defect in its own right.
Two harness faults, both caught by the harness rather than by review, and both the same class:
- Step 35's path patterns inserted
.*/between the repository segment and the route, where the real path has nothing, so every mutation fired zero times and every case passed. - Step 36 then reached the same doubled slash by a different route, concatenating two strings that each carried a separator, in a file whose header already warned about the first form. A warning phrased as one specific wrong pattern does not cover the class.
Both were visible only because the matrices print a per-request "mutated"
count, added at Step 31. In Step 36's case the two cases lost were the two
no-trace shapes, the highest-value controls in that matrix.
Size
9,655 reviewable LOC against main across 32 files: 9,452 insertions and 203
deletions, with nothing vendored or generated to subtract. That is past the 500
LOC ceiling and the overage is not only table-test volume, so it is a
deliberate choice rather than a default. It has grown during review, which is
expected: eight of the commits on this branch are review fixes.
The departure is recorded in the plan beside the npm band's, at §The OCI band
ships Steps 34 to 36 as one MR, with its cost. There is no written size rule to
grant an exemption against: docs/dev/conventions.md and CLAUDE.md carry
none, and the plan grants the OCI band no exemption, so this is precedent
rather than a rule and the record belongs in the plan where the next band's
author will read it.
The three-way split the plan's own structure offers is Step 34 (~2.4k LOC), Step 35 (~3.0k) and Step 36 (~2.6k). Each would still exceed the ceiling, at roughly 1.6x rather than 5x on the production share, and each is a coherent two-row unit. Batching is this plan's established pattern: !266 (merged) shipped Steps 26-30 and !271 (merged) shipped Steps 31-33, and the rows in a batch share a preamble, a fixture contract and a test harness, so splitting them duplicates the review of that shared surface three times.
Reviewing per commit is the practical route. Each step is a test(oci): commit
carrying its own spec-coverage table and mutation matrix, followed by its
feat(oci): implementation, and the commits after the merge are the review
fixes, each scoped to one finding class.
Two things this MR changes that the diff does not announce
The blob framing assertion was removed, and the row now asserts byte
identity only. repeatReadFraming and declaresContentLength existed at
758f5c38 and are gone. They were removed inside merge commit 450afe03,
whose body enumerates the eighteen conflicted files and never mentions the
repeat-read row, and the main tip that merge brought in does not contain the
file, so this was not a conflict resolution. The scope decision stands: S08's
Repeat read class adds "any cache-state-dependent headers reflect the hit" to
AC #16 (closed), both readings of that clause fail a relay that is behaving (the plan's
Open 10), so the clause is left unasserted rather than read one of its two
ways. TestTestRemoteRepeatRead_PassesUnderEveryFramingArrangement is what
stops it being re-introduced in either direction. Recorded here because MRs
squash on merge, so the commit history is not the durable place for it.
An --upstream-free-only OCI remote run stops being all-skip.
oci.remote.not-found and oci.remote.write-refused-blob-upload register
without NeedsUpstream, so a job pinned to --repository-kind=remote --upstream-free-only --format=oci moves from exit 0 with everything skipped
to two judged priority::critical rows that can fail. That is the rows'
purpose and the README documents the invocation, but it is the one
consumer-visible behaviour change in this MR and an operator with such a job
pinned should expect the first run after merge to actually assert something.
It compounds with the accepted-code question the plan's Open 11 holds: the
absence row accepts only the route-specific code today, and if Artifact
Registry answers NAME_UNKNOWN for a repository that was never created, that
job red-lights on a conformant target.
Test plan
-
go build ./...,go vet ./...clean. -
golangci-lint run ./...clean, with the cache cleaned first. The three findings the review's own edits introduced (funlen,thelper,modernize) are fixed on their merits, notnolint'd. -
goimports -l -localclean over all 125 changed.gofiles. -
go test -race -count=1 ./...green. -
pre-commit run --all-filesgreen, including the four mise-backed hooks. That is the real gate: this clone has no installed git pre-commit hook, sogit commitruns nothing of its own (validation/deviations.mdD2). - Each row passes alone under a
--filterselecting only it (AC #29 (closed)), and both--upstream-free-onlycompositions are asserted end to end. - Local Go is 1.27.1, matching
.tool-versions, so no CI toolchain skew. - Reference matrices re-run against the post-review code. See §Reference validation.
Process
From a goal run whose annex §7 defines this batch's scope. Each step followed
the repo's authorship contract: a test-author commit landing tests plus a
panic skeleton, then an implementation-author commit, separately visible and
never squashed. The only --no-verify commits are the three test-phase ones,
one per step.
Deferred findings are logged in validation/decisions.md and the plan's
§Spec-amendment candidates rather than fixed silently or dropped. This batch
filed Opens 9 and 10, corrected Open 10 twice after mutations disproved it, and
the review added Open 11. Opens 9, 10 and 11 belong to #70 with the rest of the
OCI band's; #70 needs the three appended.