feat(conformance): ask AC #36 whether a row seeded, not what Status it reported

Summary

AC #36 (closed) reports that a run established nothing when its fixture is demonstrably on the upstream, and exits 2, which S04 §Exit codes reserves for a tool error, against a registry the OCI specification calls conformant.

Related to #47 (closed). Found by the S08 remote goal run's reference validation and recorded as the plan's §Spec-amendment candidates Filed 5.

Targets main directly. Deliberately kept out of that run's row MRs (!271 (merged), !272 (merged), !273 (merged)): it moves an acceptance criterion's predicate and adds a constructor to S04's result-type vocabulary, which is a spec decision rather than a row one, and mixing it in would blur what a reviewer is being asked to approve.

What is wrong

AC #36 (closed) named its condition correctly all along, no seeding row completed, but operationalised it as "every selected NeedsUpstream descriptor reported StatusSkip".

A row can seed and then skip. S07 §Pluggable behavior gives ten hosted catalog rows an arm that does exactly that, across six rows of its table, and the S08 remote band inherits two (oci.remote.blob-range, oci.remote.referrers-proxy, both S08 §Per-format rows).

Measured against a live Artifact Registry with the Range header stripped in flight, so the relay behaved as a registry that does not implement it:

Selection Exit
--filter=oci.remote.blob-range 2, "no seeding row completed"
--filter=oci.remote.*blob* 0

The seeding completed in both. The proxy log for the first shows the seeding PUT answering 201 and the read-back 200. The diagnosis sends the operator to --run-id reuse and the credential's write scope on --upstream-url, and neither is implicated.

One passing seeding row satisfied the old check, so a full run hid this and a narrowed run showed it. A narrowed run is the ordinary shape of a CI gate.

oci.remote.referrers-proxy reaches the same arm against GitLab Container Registry, which implements v1.0 only (container-registry#1205), so a named real target takes it.

Why the two obvious fixes do not work

Keying on TestCase.SetupFailure looks right, since only NewSeedFailureCase sets it. It sets it only when SeedError.RunAttributable is true. The row-attributable residue, a refusal that skips this row and lets later rows run, carries a nil SetupFailure exactly like a pluggable skip does. SetupFailure's operative meaning is not "could not seed" but "end the run": runDescriptors short-circuits on it, and that is AC #33.

Keying on the absence of a recorded seeding failure fails the other way, and worse. A row that skips before it writes anything records no failure, so a run of nothing but those rows exits 0 having relayed nothing, which is the state this acceptance criterion exists to make loud. requireCrane's driver gate in pkg/conformance/oci is exactly that shape, and S07 states its pluggable arm as probe-then-skip without ordering the probe after the seed.

A test disproves each wrong fix rather than an argument, which is why the two controls in run_pluggable_skip_test.go are not optional.

What this changes

TestCase carries an unexported affirmative marker, and the new SkipAfterSeeding constructor is the only thing that sets it. The runner asks whether any selected seeding row got the fixture onto the upstream. SetupFailure's semantics are untouched.

The marker is affirmative because the two mistakes cost differently. A row that seeded and failed to say so exits 2: loud, on a named row, fixed once. A row that never seeded and was counted anyway exits 0 on no evidence, and nothing else in the report contradicts it, because a skip moves no exit code. So the unset marker means "established nothing", and a module that forgets the constructor fails toward the verdict rather than away from it. S08 §Preflight records the choice.

The first branch's diagnosis widens with it. It named the refusing status as though every row reaching the branch had one; it now sends the operator to the row reasons, where each row's own cause lives. The verdict also logs its two inputs, seeding_rows_selected and seeding_rows_established, because the marker reaches no renderer and an exit-0 remote run otherwise records nothing about what it established.

Note for library consumers

seeded is the first unexported field on TestCase, so a keyed composite literal becomes the one form that compiles outside pkg/conformance, and cmp.Diff over a TestCase or a Report panics unless given cmpopts.IgnoreUnexported(conformance.TestCase{}) or an Exporter. It does not degrade to a wrong answer: the panic is cannot handle unexported field at {conformance.TestCase}.seeded, so a consumer's passing test becomes a panicking one. reflect.DeepEqual does not panic but changes answer: DeepEqual(Skip(n, r), SkipAfterSeeding(n, r)) is now false.

Nothing in this repo is affected: the full suite passes, and a cmp.Diff over either type would panic rather than fail, so no call site compares one. Every TestCase literal in the tree is keyed.

Nothing in the Artifact Registry is affected either, for a structural reason rather than a lucky one: it consumes this project as a container image, not as a Go module. Its go.mod carries no registry-conformance requirement, and scripts/conformance/maven_setup.sh runs inside the tool image. The constructors S04 already directs authors to are unaffected.

The spec changes with the code

The predicate is normative, so its sites are amended here rather than in a follow-up:

  • S08 §Preflight: the mechanism paragraph, the pinned branch diagnosis, and a new paragraph pinning the constructor rule and why the marker is affirmative.
  • S08 §Acceptance Criteria, AC #36 (closed): what "completed" means, and the upstream qualifier's justification, which reads against the seeding test now.
  • S08 §Error Cases: the AC #36 (closed) row.
  • S08 §S04 amendments: the !231 (merged) row's status-test clause marked superseded by !275, and two new rows for the S04 edits below.
  • S04 §Entry points and §Execution order inside RunModule step 9: the sentinel's doc block and the step-9 predicate.
  • S04 §TestDescriptor, TestCase, Status, Detail: SkipAfterSeeding in the constructor list.

One fixture changed, and it is not a weakened test

run_remote_test.go's skipResult now builds through NewSeedFailureCase. Its own doc already called it "a row-attributable seeding refusal", and it was standing that up with a plain Skip because the old predicate could not tell the two apart. Under the new one a plain Skip means a row that never wrote, so the fixture had to become what it already claimed to be. No assertion was dropped; the test that consumes it is renamed to what it asserts (TestRunModule_NothingEstablishedWhenNoSelectedSeedingRowSeeded).

Spec coverage

Spec: docs/specs/S08-remote-contracts.md

Acceptance criteria

# Criterion Tests
AC #36 (closed) A selected seeding row that seeded and then skipped on a pluggable-behavior arm completed, so the run exits 0 TestRunModule_PluggableSkipIsNotNothingEstablished, TestRunModule_PluggableSkipVerdictDoesNotDependOnWhatElseWasSelected
AC #36 (closed) A selected seeding row that could not seed did not complete, so the run exits 2 TestRunModule_RowAttributableSeedFailureIsStillNothingEstablished, TestRunModule_NothingEstablishedWhenNoSelectedSeedingRowSeeded
AC #36 (closed) A selected seeding row that skipped before it wrote did not complete, so the run exits 2 TestRunModule_SkipBeforeSeedingIsStillNothingEstablished
AC #36 (closed) The verdict does not depend on what else the selection held TestRunModule_PluggableSkipVerdictDoesNotDependOnWhatElseWasSelected
AC #33 SetupFailure's short-circuit still preempts the AC #36 (closed) verdict TestRunModule_SetupFailureShortCircuitPreemptsNothingEstablished (unchanged)

Error cases

# Condition Tests
Runner No selected NeedsUpstream descriptor got the fixture onto the upstream: exit 2, ErrNothingEstablished wrapped in the row-reasons diagnosis TestRunModule_NothingEstablishedWhenNoSelectedSeedingRowSeeded (asserts the widened message verbatim)
Runner m.TestCatalog(cfg) holds no seeding row: unchanged branch and message TestRunModule_NothingEstablishedWhenCatalogHoldsNoSeedingRow (unchanged)

S04 vocabulary

# Addition Tests
§TestDescriptor, TestCase, Status, Detail SkipAfterSeeding returns Skip's shape in every exported field, plus the marker TestSkipAfterSeeding_ShapeMatchesSkipInEveryExportedField

Security considerations

# Concern Tests
S08 §Security Considerations None reached. The marker is a bool, carries no upstream bytes, and reaches no output channel; NewSeedFailureCase's scrub and detailBodyCap bound are untouched n/a

Test plan

  • go test -race ./... green.
  • pre-commit run --all-files clean.
  • Reverting the predicate to the status test kills both pluggable-skip tests and nothing else.
  • Counting every case as seeded kills both controls and the branch-1 table, and nothing else.
  • The row-attributable control passes in both directions, which is what keeps the fix from being "never fire AC #36 (closed) on a skip".

Three mutations, because each isolates one direction of the verdict and the middle one is the property whose absence let the first attempt at this fix through.

Edited by Sylvia Shen

Merge request reports

Loading
Loading