test(maven): the Maven strict-equality flip, Step 14 (S08 Maven, batch 4/4)

Summary

Related to #47 (closed). Divergence index: #54.

The Maven band's strict-equality flip, per the S08 Maven goal-run's annex §7 MR clusters ("Maven strict-equality flip"). Lands Step 14, the band's last step, and closes the band.

Stacked on !282 (merged) (sshen/maven-remote-relay-rows, Steps 10-13), which is stacked on !281 (merged) and !280 (merged). Do not merge before any of them: strict equality holds only once every Maven remote row is registered, so this MR fails if it lands ahead of the rows it counts.

Not test-only, despite the plan's Step 14 framing. Production code changed in two rounds: the adversarial pass below added remoteRefusalClause's present-and-empty error-code sentence in remote_write_refused.go, and during review an AppSec finding required routing several remote rows' registry-controlled message values through scrubForMessage instead of redact.ScanContent alone (remote_artifact_relay.go, remote_metadata_relay.go, remote_not_found.go, remote_sidecar_synthesis.go, remote_write_refused.go). Still no new go.mod / go.sum dependency.

The real-target run this MR exists to cite

The plan's §Dependencies makes a passing real-target run the closing criterion for enabling this format's remote CI gate. Here it is.

All eight ## Remote rows pass against a live Artifact Registry in one run, exit 0:

PASS  maven.remote.preflight
PASS  maven.remote.artifact-relay
PASS  maven.remote.metadata-relay
PASS  maven.remote.repeat-read
PASS  maven.remote.write-refused
PASS  maven.remote.sidecar-synthesis
PASS  maven.remote.cold-fill-header-subset
PASS  maven.remote.not-found
Total: 8   Passed: 8   Failed: 0   Skipped: 0

The reference is a standalone Artifact Registry with a hosted Maven repository as the upstream and a remote Maven repository pointed at it. Both axes were verified independently for all six seeding rows, reading the hosted upstream's own client endpoint rather than through the surface under test.

A green from this MR's own tests is evidence about remotefake, not about a provisioned remote repository. The run above is the separate artifact, and the two are not interchangeable, which is why the plan asks for both.

Worth stating for whoever compares this band's evidence to the OCI band's: the Maven verdicts were gathered against the registry exactly as a Maven client reaches it, with a mutating proxy as the only extra element in the path. The OCI band needs a TLS proxy and a path-prefix shim for reasons specific to its token flow and URL model, and attaches a caveat to every verdict accordingly. Maven has no equivalent caveat.

What this step actually guards

Three things land, and the second is the one that stops the first from being decorative.

Strict equality in both directions, so every non-optional maven.remote.* catalog row has a descriptor and every maven.remote.* descriptor has a row. maven.remote.cache-ttl is excluded by its optional Status, which is on main already; no step in this band re-lands it.

AC #35's four NeedsUpstream assertions, over a named list of the two rows that seed nothing. The first three all hold trivially against a bucket containing only those two rows. The fourth, that the module registers at least one maven.remote. descriptor with the field true, is what makes them mean anything.

That is reproduced rather than argued: with the module and catalog both reduced to the two upstream-free rows, the other three subtests pass and only the fourth fails. AC #35 predicts exactly that shape.

The propagation guard, which is the highest-value assertion in the step. A row that builds its TestCase by hand instead of calling NewSeedFailureCase demotes a revoked credential to a plain skip. AC #33 cannot see it, because SetupFailure is unset; AC #36 (closed) cannot see it, because not every seeding row skipped. The run exits 0 having validated nothing about the registry, which is the worst outcome this suite can produce: a green that means nothing.

Every fixtured remote descriptor is therefore driven once with an injected 403 and required to return a non-nil SetupFailure. The six rows are enumerated from the module rather than listed, so a row added later is covered without an edit.

Every guard was made to fail before it was trusted

This step is pure guard, so a guard that cannot fail is the entire risk. Each defect was staged, run and reverted:

Defect staged Caught by
a catalog row with no descriptor Catalog ⊆ Module, plus two AC #35 subtests
a descriptor with no catalog row Module ⊆ Catalog, the partition subtest, the propagation guard
a fixtured row moved into the upstream-free list named_rows_carry_false
an upstream-free row moved out of it every_other_row_carries_true, the partition subtest
a new row in neither set the two AC #35 subtests. Both equality tests pass, which is the gap AC #35 exists to close
module and catalog reduced to the two upstream-free rows only the fourth assertion and the propagation guard
a row hand-building conformance.Skip the propagation guard, for that row alone, so the defect is localised

Nothing was unstageable.

Two files beyond the plan's list, and one spec number that is not this MR's

inventory_test.go is package maven_test and cannot reach newRemoteEnv, the unexported env, or the two slug constants. So the propagation guard is in-package, following Step 8's catalog_categories_test.go precedent, and an export_test.go re-exports the two slugs so the named list is a compiler-enforced reference rather than two more copies of the literals. Going through NewModule().NewEnv instead was barred: it probes mvn and primes a plugin closure against Maven Central, and §Testing Strategy pins no network.

The plan directs this step to write its own MR number into both of S08 §Downstream amendments' Owner cells. One of them would have been false. The docs/catalog/{maven,npm,oci}.md sections row's Maven half was landed by !282 (merged), not here, so it names !282 (merged); the inventory_test.go row names !283 (merged). An Owner cell exists so a reader can check the claimed change happened, and docs/dev/conventions.md §Amendment tables calls a row claiming a change that did not happen the defect to avoid. validation/decisions.md D28.

Both keep Per-format child verbatim beside the number, in the compound form that table already uses twice, so npm's Step 25 and OCI's Step 37 can grep for it and append rather than replace. Neither row reads as discharged until all three formats appear.

Comments this step corrected, including two of its own

Two claims written while authoring this step were checked against a staged defect and turned out false: a misspelled slug prefix was said to fail one guard direction when it fails the other, and the zero-value kind case was said to be uniquely caught by one direction when a leaked resolve fails both. Both are corrected rather than softened.

Six comments in inventory_test.go that Steps 9 to 13 had already falsified were refreshed in the same pass, among them "empty until plan Step 9" and "every ## Remote row is optional today". They were stale before this step, but this file's central claim is the one the step resolves, so leaving them would have left the guard contradicting the code it guards.

The band's adversarial pass landed here too

Two commits beyond Step 14, because the pass ran after the last step and its output had to go somewhere in the stack.

62 mutations against the eight rows and their shared code. 54 killed. Ten findings. The killed half is the useful half of that number: every byte and length comparison, both HEAD parity clauses, every cold-fill clause in both directions including the wildcard and the present-and-empty distinction, the sidecar comparison base and algorithm set, all three not-found clauses, the 405 and Allow equality and no-trace read, the settle-last contract, the 202 carve-out, every Client()/Upstream() swap, and every ctx.Err() re-check.

Two findings are spec contradictions and are filed rather than fixed, in the plan's §Spec-amendment candidates, which goes from one open item to three.

The severe one makes a conformant registry fail. AC #37 requires a resumed settle to confirm byte identity against the run's fixture; S05 permits a registry to regenerate maven-metadata.xml server-side; and maven.remote.metadata-relay settles on that document. Against such a registry a resumed settle compares the suite's bytes with regenerated bytes and reports a collision naming a foreign writer, ending the run at exit 2. The row's own file header states the regeneration premise and then draws the opposite conclusion, which is what makes it a design defect rather than an oversight. Not fixed here because settling on the .jar instead trades a rare false failure for giving up propagation confirmation on the one document S08 names as the slowest.

The second: AC #13 (closed) requires bytes identical and integrity metadata agreeing, and Maven satisfies the halves at two coordinates and the conjunction at neither, because AC #20 (closed) forbids the sidecar row from comparing against what it seeded. That narrowing was living in a file header citing AC #20 (closed) and never mentioning AC #13 (closed).

Seven were fixed, though the one that needed production code no longer shows in this diff: sidecar-synthesis did not pin its primary read at 200, and it is the row where that matters most, because its comparison base is the served bytes, so a 204 with an empty body passed if the sidecars digested the empty string. That pin (relayServedStatusClause) shipped with an earlier MR in the stack and is in the merge base (d87f780b); what this branch adds around it is comments. Three assertions could not fail and read as coverage, among them a three-valued Allow table that asserted only that the message contained the word, so a Header.Get-based row would have passed all four arms, which is what its own comment claimed it could not.

Two more surfaced while fixing those, both outside the pass's ten, and both fixed although neither can produce a wrong verdict: the sidecar reads had the same unpinned status range one layer down, and a present-and-empty error code rendered a blank where the value goes. Of those two, only the error-code sentence is part of this diff; the sidecar reads' pin is in the merge base alongside the primary read's. A wrong stated reason is the defect this suite exists to avoid in its own output, so a message that sends an operator to look at checksum synthesis when the registry answered 204 is a real defect even when the pass or fail bit is right.

After the fixes, all eight rows re-run against the live reference: pass, exit 0. No verdict moved, which is the expected result when the findings were about what the tests pin rather than about what the code does.

Process

This batch comes from a goal run; validation/deviations.md D9 records the batching. Conventional commits, no squash. No commit here uses --no-verify: every commit's tests pass on arrival, so the test-first exception does not apply. The gate was run explicitly with pre-commit run --files, because this checkout's core.hooksPath points at a samples-only directory and git commit runs no gate at all.

Judgement calls are in validation/decisions.md (D17 through D28) and validation/reference-quirks.md.

Test plan

  • go build ./... and go vet ./... clean.
  • go test ./... -count=1 green, no regressions in any sibling package.
  • golangci-lint run clean over the whole touched package.
  • gofmt / goimports clean on every file touched.
  • Production code changed (see Summary), but no new go.mod / go.sum dependency.
  • Every assertion the step adds was made to fail against a staged defect.
  • Real-target run: all eight rows passing against a live Artifact Registry, both axes verified for every seeding row.
Edited by Sylvia Shen

Merge request reports

Loading
Loading