feat(maven): client gaps, upstream env and kind-aware catalog, Steps 6-8 (S08 Maven, batch 1/4)

Summary

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

The Maven band's client gaps, upstream env and kind-aware catalog, per the S08 Maven goal-run's annex §7 MR clusters ("Maven client gaps, env, catalog"). Lands Steps 6 through 8.

Step	What it adds
6	HTTPError.Allow and .Code, the terminal request's verb on every ArtifactResponse, Metadata.Raw, and a byte-bounded artifact read for Step 9's settle
7	the env's upstream client, HasUpstream, and the Upstream() / UpstreamURL() accessors
8	the catalog branched on cfg.RepositoryKind, and the inventory guard taught both directions

Step 8 carries one assertion worth a reviewer's eye, because the trap it closes is invisible from the package the guard lives in. TestCatalog flattens the order slice, not the table, so an empty remote bucket and a missing one produce byte-identical output. A return nil, nil remote arm passes every subset guard, passes both idempotency tests, and then silently drops maven.remote.preflight the moment Step 9 registers it. catalogCategories is unexported, so nothing in maven_test can tell the two apart. Hence the new in-package catalog_categories_test.go, which asserts the order slice directly. Step 18's Acceptance states the same requirement for npm, so the plan's own design implies the in-package half.

Targets main directly. Band A's two MRs, !258 (merged) and !259 (merged), both merged on 2026-09-02, so Steps 6 to 8 have no unmerged dependency. No catalog row lands here: none of Steps 6 to 8 registers one, and docs/catalog/maven.md is untouched.

Spec coverage

Spec: docs/specs/S08-remote-contracts.md, with the S04 and S05 rows the same code touches.

Acceptance criteria

#	Criterion	Tests
S08 AC #1 (closed)	Both kinds execute the format's ## Negative auth rows; each kind executes its own section	TestCatalogCategories_OrderPerKind, TestInventory_negativeAuthRegisteredUnderBothKinds, TestModule_NegativeAuthTests_IdenticalAcrossKinds
S08 AC #2 (closed)	A run that names no kind is identical to one before this spec	TestModule_TestCatalog_HostedSetUnchanged, TestCatalogCategories_OrderPerKind/kind_absent_resolves_to_hosted, TestListCommand_MavenOmittedKindPrintsTheHostedSet
S08 AC #11 (closed)	list --format=<f> --repository-kind=<k> prints exactly the names run executes for that format and kind, in execution order	TestListCommand_MavenPerKindMatchesRun, TestListCommand_MavenRemoteRowsAreExactlyTheKindIndependentBucket
S08 AC #22 (closed)	A target-specific assertion is carried by the row, not only cited in its Notes	TestHTTPError_Code_readsTheRefusalErrorCode
S08 AC #24 (closed)	The remote preflight is ordered first among a remote run's descriptors	TestCatalogCategories_OrderPerKind/remote, asserted on the order slice. The flattened slice cannot show it until Step 9 registers the row.
S08 AC #27 (closed)	The per-kind inventory guard fails a descriptor with no non-optional row in its kind's bucket	TestInventory_moduleSlugsSubsetOfCatalog, TestInventory_subsetCheckDetectsUnknownSlug
S08 AC #28 (closed)	A remote run registers only maven.remote.* and maven.negative.*; a hosted run registers no maven.remote.*	TestInventory_registeredSlugsStayInTheirKindsNamespace, TestInventory_keepPrefixesMatchCatalog, TestInventory_catalogSlugsForKindSelectsItsBucket, TestListCommand_MavenRemoteRowsAreExactlyTheKindIndependentBucket
S08 AC #37	A resumed settle's read-back is byte-identical to this run's fixture	Capability only in this batch. Step 6 supplies the verbatim bytes (TestGetMetadata_rawIsTheVerbatimServedDocument) and the read ceiling (TestGetArtifactCapped_boundsTheSuccessRead, TestGetArtifactCapped_andGetArtifactDifferOnAnOverLengthBody). The comparison is Step 9/11's; no row performs it yet.
S04 AC #39 (closed)	TestCatalog is idempotent within and across module instances, for the same Config	TestCatalogCategories_ReturnsFreshValuesPerCall, TestModule_TestCatalog_Idempotent, TestModule_TestCatalog_IdempotentAcrossInstances
S05 AC #30 (closed)(b)	An error body carries a machine-readable code	TestHTTPError_Code_readsTheRefusalErrorCode. The string requirement is this accessor's own and stricter than informativeErrorBody's, which accepts {"code":42}; the Code() doc says so.

Spec sections without an AC number

Section	Requirement	Tests
S08 §Upstream gate	*env satisfies conformance.UpstreamEnv, and the answer keys on the resolved URL rather than on flag set-ness	TestEnv_SatisfiesUpstreamEnv, TestEnv_HasUpstreamKeysOnTheResolvedUpstreamURL
S08 §Fixture seeding model	The env exposes an upstream client and URL bound to --upstream-url	TestNewEnv_WiresUpstreamFromConfig, TestEnv_UpstreamAccessorsReturnTheWiredClientAndURL
S08 §Write refusal per format	A conformant refusal answers 405 with Allow; presence is distinguishable from present-and-empty	TestHTTPError_Allow_distinguishesAbsentFromPresentAndEmpty, TestHTTPError_allowAndCodeAbsentOnTransportFailure
S08 §Preflight	The terminal request's verb is reported, so a redirect-replayed GET cannot read as a stored write	TestArtifactResponse_reportsTheRequestMethod, TestPutArtifactRelease_reportsTheTerminalRequestMethod, TestArtifactResponse_methodFallsBackWhenTheResponseCarriesNoRequest
S08 §S04 amendments	RepositoryKind is the one Config field a descriptor set may depend on; purity holds for the same Config	TestCatalogCategories_ReturnsFreshValuesPerCall, TestCatalogCategories_OrderCoversEveryTableKey

Security considerations

Concern	Tests
S08 §Security Considerations: the credential reaches both base URLs	TestNewEnv_UpstreamClientAddressesUpstreamURLWithTheRunCredential
S08 §Security Considerations: --upstream-url inherits the run's redirect and timeout policy	TestNewEnv_UpstreamClientInheritsTheRunHTTPClientRedirectPolicy
S05 §Security Considerations: a registry-controlled error code is redacted and bounded before it reaches a caller	TestHTTPError_Code_readsTheRefusalErrorCode, rows a keyword-bearing code is redacted, a code carrying URL userinfo is redacted, and an over-cap code member is clipped to the ResponseBody cap. One row per redaction pass and one for the bound, so dropping any of the three fails exactly one row.
S05 §Security Considerations: Metadata.Raw is unscrubbed by design, so the render site owns the scrub	Documented on the field rather than tested: no production path renders it. Carried forward to Steps 9, 11 and 13.

Two rows are honest gaps rather than test names. AC #37 is capability-only, per the note above. Metadata.Raw's scrub obligation cannot be tested here because scrubbing at the boundary would defeat the byte comparison the field exists for, so it is recorded on the field doc for the steps that render it.

Reference validation

Steps 6 to 8 register no catalog test case, so this batch has no row verdicts to report. It is not therefore unvalidated against the reference: Step 6's one open question was closed by measuring the live target rather than by choosing a reading.

S08 §Assertion sources pins method_not_allowed as Maven's refusal code and cites AR S14, which is not in this repository, so nothing here says how the code is framed. Rather than merge on a provisional reading, a PUT was issued against a real Artifact Registry remote Maven repository. It answers 405 with Content-Type: application/problem+json, a top-level string code member holding method_not_allowed, and Allow: GET, HEAD, which is exactly what S08 §Write refusal per format derives for a Maven write route. That document is now one row in the Code() table, cited by evidence path and date.

The assertion did not change because of the measurement, and that distinction matters: every row in that table asserts the same property, that a top-level string code member is surfaced as present, and S08 is what justifies it. The reference supplied a ground-truth input in place of an invented literal. Had it answered a different shape, the response would have been a divergence entry under #54 and a spec-amendment candidate, not a change to the accessor.

The batch was also checked end to end against that reference, which a zero-row batch is not obliged to do but can. The plan predicts a specific intermediate state for the window after Step 8 and before Step 9, and a remote Maven run against a live Artifact Registry produces exactly it: one row enumerated and passed (maven.negative.unauthenticated, the kind-independent bucket), then exit 2 carrying no seeding row completed: this format registers no remote row that seeds. That one run confirms four things no unit test does. The env really satisfies conformance.UpstreamEnv, because the runner's AC #36 (closed) gate consults it and the gate fired. The kind branch really selects the remote catalog against a live run. The gate took the "catalog holds no NeedsUpstream descriptor" branch rather than the "descriptors existed but none established" one, which the exit code alone cannot distinguish and only the wrapped message separates. And the run ID is 16 hex characters, so Step 1's 64-bit widening is in force before any Step 9 fixture derives a coordinate from it.

Three things a reviewer should look at rather than skim

A file outside Step 6's list, fixing a false negative the suite would have shipped. pkg/conformance/maven/metadata_preservation.go compared whole *Metadata values with reflect.DeepEqual. uploaded is a struct literal and got comes back from the GET, so once Metadata.Raw is populated the two can never be equal and maven.metadata.snapshot-version-preservation reports fail against every registry, including a fully conformant one. Fixed by zeroing Raw on a copy, which keeps every other field in the comparison including any field added to the type later.

A follow-up this batch found and deliberately did not fix. The inventory guard compares slug sets, which cannot see two catalog rows whose slug values are swapped, nor a descriptor whose Name and Fn are cross-wired. runOne emits the Fn's name verbatim, so a cross-wired pair mislabels its verdict while the guard stays green. pkg/conformance/oci/inventory_test.go already carries the cheap close, TestInventory_everyDescriptorFnReportsItsOwnSlug; Maven and npm do not. Porting it is out of Step 8's scope and only about 20 of 31 Maven Fns were sampled for the nil-env-safe shape it needs, so it is filed as a follow-up rather than folded in here. This batch does not make the gap worse: no assertion added compares sets where an ordered comparison was available, and the new wantHostedCatalog golden is ordered over slug plus Priority, which is strictly more than the set guard could see.

A plan mechanic that is not reachable, and what replaced it. Step 7's Acceptance requires asserting the upstream client on the *http.Client pointer rather than on its transport, and gives a good reason: with Transport left nil by newDefaultHTTPClient, a transport-identity assertion compares nil against nil and would hold for an upstream client that dropped the redirect policy entirely. But pkg/client/maven.Client.http is unexported with no accessor, so the literal assertion cannot be written from pkg/conformance/maven without reflecting into another package's private field.

The test asserts the discriminator instead: it drives a redirect through the upstream client and requires the run's own CheckRedirect closure to have run. Only a client that received the run's pointer reaches that closure. This is stronger than the shape a literal assertion would have taken, since pkg/conformance/npm/env.go and pkg/conformance/oci/env.go's HTTPClient() form asserts only that the env stored the client, never that the client received it. Two mutants confirm it: client.New(cfg.UpstreamURL, cred, nil) and a fresh &http.Client{} without CheckRedirect are both killed by that one test and by nothing else in the file. No acceptance criterion is narrowed; S08 §Security Considerations' requirement is asserted in full, with the HTTPS-to-HTTP downgrade refusal covered by its own behavioural case.

Falsification

Every facet was checked against the change it is supposed to catch, one mutation at a time. Step 6: eight mutations, eight caught. Step 7: eleven mutations, ten caught and one equivalent mutant correctly not counted (a shallow copy of the run's *http.Client carries CheckRedirect, Timeout and the same Transport pointer, so it is behaviorally indistinguishable). Step 8: seven mutations, seven caught. The per-step commit messages carry the tables.

Two results from that pass are worth a reviewer's attention because they change which line is load-bearing:

• Removing GetArtifactCapped's max(maxBytes, 0) clamp is caught by the -1 row only. io.LimitReader reads nothing for any negative limit, so 0 and a large negative pass unclamped; what the clamp prevents is the collision with the internal unboundedRead = -1 sentinel, which inverts a bound into no bound. That row is named and commented so a later consolidation cannot keep the wrong one.
• Reading Allow with Header.Get instead of RFC 9110 §5.3 field-line combining is caught by the two multi-line rows and nothing else. It matters at Step 13: a registry sending Allow: GET and Allow: PUT on separate lines hides the PUT, and the row whose whole job is to fail on a write verb would pass a non-conformant registry.

Process

This batch comes from a goal run, which is why it batches three plan steps into one MR rather than opening one MR per step; the deviation is recorded in validation/deviations.md D9. Within the batch the per-step commit discipline is kept: a test-author commit then an implementation commit for each step, separately visible in the branch history, conventional commits, no squash. Three commits use --no-verify, one per step, all test-author: the panic-skeleton intentionally fails the go-test hook, which is the test-first authorship contract rather than a defect, and no other commit on the branch uses it.

Judgement calls and deferred findings are logged in validation/decisions.md (D17, D18) rather than fixed silently or dropped.

Test plan

• go build ./... and go vet ./... clean.
• go test ./... -count=1 green, no regressions in any sibling package.
• go test -race green on both touched packages.
• golangci-lint run clean on every touched package, run over the whole package rather than the changed hunks, since funlen, gocognit, gocyclo and dupl are whole-function linters the changed-hunk hook cannot see.
• gofmt / goimports clean on every file touched. module.go carries pre-existing drift on main, so it was checked by comparing the digest of gofmt -d before and after: no new drift, nothing reformatted.
• No new go.mod / go.sum dependency.
• Exactly one --no-verify commit per step, verified in the branch history.