chore(managementapi): repository stats gate (S17 Phase 8 plan: 4/13)

Why

The repository-statistics route answers a placeholder 501 for every input, a repository that does not exist and a virtual or remote one included. The spec makes the scope hosted-only: its Error Cases table gives 404/not_found for "repository missing, or repository kind not hosted".

Landing that resolution once, ahead of the three format-family arms, is what keeps each arm from re-implementing it. Re-implementing it is the risk worth an MR of its own: datastore.RepositoryKindHosted is the int16 zero value, so the obvious check fails open. versions.go's versionFileScope carries the kind and documents exactly that reading, "whose zero value reads as hosted". This scope stores the verdict as a field its constructor sets, so its zero value reads as not hosted.

What is not obvious

  • A kind outside the taxonomy takes the same 404, not a 500. The gate is a positive equality test against hosted, so a value nobody anticipated is refused rather than served. It does log, though, because the answer is a virtual row's 404 and nothing else would show the row is broken — that asymmetry is the one place this gate deliberately differs from hostedVersionFileRepository, its near-duplicate sibling: that helper's own unmapped-kind arm answers 500, which this route's spec forbids, so reuse would need the shared helper's response to vary by caller.
  • The registration row takes the whole deps, and no Deps field is added. registerStatisticsRoutes's own comment says deps carries the seams the rows read as they fill, so passing it now is what keeps that row edited once: the three family arms pull their count readers off the same parameter instead of widening the signature three more times. The nil-checkable seam count pinned in handler_test.go is untouched.
  • Three format cases share one placeholder body. Each later change repoints its own case line; a single collapsed case would make the first arm to land split it, and the other two conflict on the split. The default arm is this route's only format check, so it is live behavior.
  • Two files the plan's Files list for this step omits. handler.go's registration row, which the anchor comment sitting above it assigns to "the resolution step", and one line of handler_test.go's servedStatisticsScopes, the map built so a scope that lands edits one line rather than a sweep's assertions. Both are load-bearing; the plan under-specifies.
  • The contract sweep's pending entry drops in this change set. Once the handler resolves, the sweep's row-less reader turns its probe into a JSON 404, which answersUnserved reads as served, so an entry left standing fails as stale. One consequence worth naming: with the entry gone, the sweep's "must not answer 501" assertion for this operation can no longer fire, because the sweep never seeds a hosted row. Each arm's 501-to-served transition is guarded by its own step's tests instead.
  • That same pending-map line conflicts with feat(api): version statistics contract (S17 Pha... (!1986 - merged) • Hayley Swimelar • 19.4 in either merge order. !1986 (merged) (Step 2) inserts two getVersionStatistics entries immediately after the getRepositoryStatistics line this MR deletes, so the two edits land on one two-sided hunk (confirmed with git merge-tree both ways). Whichever MR merges second must resolve by hand: keep its own change and the other's, never a naive union that reinstates getRepositoryStatistics after this MR has already made the operation servable.
  • A row outside the request namespace is refused, and the refusal is the 500. The proof lives in findRepositoryForRequest, shared by this route and five others, so it runs before any caller's own kind- or format-based dispatch. A reader returning a foreign row has broken an invariant, and a not_found there would invite the caller to cache an absence that was never established.
  • Diff size. 506 reviewable LOC (insertions), past the 500 guardrail. 98 is production (statistics_repository.go, resolve.go, three lines of handler.go); the other 408 is the test matrix (statistics_repository_test.go, the new statistics_repository_integration_test.go, handler_test.go, artifact_internal_test.go, contract_routes_test.go). A split would not help: guardrail 6 requires a positive-hit test per enumerated kind and format, and splitting that matrix from the 98-line change it covers would leave either half unreviewable alone.

Test plan

Spec obligation Pinned by
Missing repository answers 404/not_found TestRepositoryStatistics_KindGate, absent row
Virtual and remote answer that same 404 TestRepositoryStatistics_KindGate, one row per kind
A kind outside the taxonomy answers it too TestRepositoryStatistics_KindGate
The refusal is indistinguishable from a missing row the captured baseline, compared on status, all headers, and raw body bytes
A hosted repository reaches its family arm's 501 TestRepositoryStatistics_HostedReachesEveryFamilyArm, driven off datastore.RepositoryFormats() so a format added to or swapped into the taxonomy is covered without an edit
An unmapped format is a contract violation TestRepositoryStatistics_UnmappedFormat_Returns500
Only a broken kind logs TestRepositoryStatistics_UnmappedKind_IsTheOnlyKindThatLogs
A reader outage is not laundered into the 404 TestRepositoryStatistics_StoreFailure_Returns500
Tenant isolation TestRepositoryStatistics_CrossSlug_Returns404
A row resolved outside the request namespace is a contract violation TestRepositoryStatistics_ForeignRow_IsRefused, hosted, virtual, and remote
The production reader delivers a non-hosted row to the gate TestRepositoryStatisticsIntegration_KindGate, seeded through RepositoryStore.Create
The contract sweep passes with the entry dropped TestContractOperations_AreRegisteredAndServed

Verified on a running instance as well, since a unit suite cannot show the route is reachable. Against the real binary on a real database, hosted maven, npm, and oci repositories each answered 501, remote maven and npm and a missing name each answered a byte-identical 404, and a repository named statistics kept its detail route. The two kinds no write path can produce were forced in by hand: a virtual row (RepositoryStore.Create refuses the kind) answered the 404, and a row with kind = 99 (the check_repositories_kind constraint dropped for the experiment) answered the 404 and left exactly one log line naming the value.

No e2e catalog scenario is added or affected. The 501-to-404 move is client-visible, but docs/testing/ holds only the four protocol-client catalogs (docker, oci, maven, npm) and mentions statistics nowhere. The consumers are the monolith UI slices, whose journeys land with them.

Context for LLM agents

Known merge interactions (measured with git merge-tree)

  • One mechanical conflict, against feat(api): version statistics contract (S17 Pha... (!1986 - merged) • Hayley Swimelar • 19.4. Its pending-map append lands immediately after the entry this MR deletes, so the two edits are a single two-sided hunk. Whoever lands second keeps its two getVersionStatistics entries and drops getRepositoryStatistics. gofmt absorbs the blank line before the closing brace, which is what makes the hunk two-sided rather than a clean append.
  • No conflict with feat(managementapi): destructive repository DEL... (!1885 - merged) • Pawel Rozlach • 19.4. Its four conflicts against this branch (run-artifact-registry/SKILL.md, wire_management.go, handler.go, handler_test.go) are byte-identical to its four against main, so it is behind main independently of this MR and this MR adds nothing to its rebase. The claim-check hook fires on the shared contract_routes_test.go, where the measurement shows no incremental conflict at all.

Known defect this route inherits, not one it introduces

An unstorable byte in {repository_name} (%00, %ff, a lone surrogate) reaches Postgres through findRepositoryForRequest and answers 500 with the caller's bytes in an ERROR log. That is shared by every single-resource route on the surface, and this route is one more path to it rather than its cause: tracked as Reject NUL and control bytes in path values bef... (#484) • Hayley Swimelar, with the fix open at fix(managementapi): screen unstorable bytes out... (!1992 - merged) • Hayley Swimelar • 19.4. Measured here for corroboration only, no pool damage: 31 consecutive %00 requests left OpenConnections at 0 and a normal request afterwards still answered its 404.

Rejected alternatives

  • Answering 500 for a kind outside the taxonomy, matching hostedVersionFileRepository's default arm and artifact.go's unmapped-enum handling. Rejected because the spec's Error Cases row is "repository missing, or repository kind not hosted", which an undefined kind satisfies, and because a positive equality test is what makes the gate fail closed on a value nobody anticipated. The observability the 500 would have bought is bought instead by the log line, which leaves the response contract alone.
  • Passing deps.Repositories rather than deps to the handler, the narrower seam most single-resource handlers take. Rejected because the three family arms each need their own count reader, so the narrow seam makes the registration row an edit per arm, and that row is shared with every other statistics family.
  • Three named thin arm functions at the per-family sub-anchors, mirroring statistics_version.go. Rejected because those anchors reserve the arm functions for the steps that fill them; creating them here would make each anchor read stale and put this MR inside three other steps' regions.

Non-goals

  • Serving any figure. All three family arms answer 501. The 501 is undeclared in the contract, which declares {200, 401, 403, 404, 500} for this operation, and with the sweep's pending entry dropped nothing in the tree fails if the arms slip. The plan accepts that trade-off explicitly; each arm's 501-to-served transition is guarded by its own step's tests.
  • Extending statistics to virtual or remote repositories. The spec ships the scope hosted-only and records the extension as a follow-up.
  • A positive test that a family case is pointed at the right family. All three arms call the same placeholder, so permuting the case labels is invisible to any test until an arm serves real figures. The test comment says so rather than implying coverage it does not have.

Related to #316

Edited by Hayley Swimelar

Merge request reports

Loading
Loading