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 2xx the 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 answering 200 to a manifest PUT at 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, not nolint'd.
  • goimports -l -local clean over all 125 changed .go files.
  • go test -race -count=1 ./... green.
  • pre-commit run --all-files green, including the four mise-backed hooks. That is the real gate: this clone has no installed git pre-commit hook, so git commit runs nothing of its own (validation/deviations.md D2).
  • Each row passes alone under a --filter selecting only it (AC #29 (closed)), and both --upstream-free-only compositions 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.

Edited by Sylvia Shen

Merge request reports

Loading
Loading