chore(managementapi): statistics scaffold (S17 Phase 8 plan: 3/13)

Why

Seven of Phase 8's thirteen steps edit handler.go, wire_management.go, statistics_resources.go, and the contract_test.go block. Without a scaffold owning the anchor layout first, they append to the same positions and conflict on whichever lands second.

Two routing facts also get decided once, at registration, where they are invisible afterwards. A {format} wildcard at the format scope overlaps repositoryPattern on /api/v1/:slug/repositories/statistics with neither pattern more specific, and net/http.ServeMux panics on that pair — so a wildcard there ships as a boot crash, and a repository legitimately named statistics loses its detail route. Four literals cannot overlap it.

Plan: docs/plans/2026-08-12-s17-phase8-statistics.md, step 3.

What (the non-obvious parts)

  • Deps takes its anchor block at the struct tail, not ahead of the trailing shared fields where the Phase 3 and 4 blocks sit. The repositories aggregate lands at the head, and two insertions at one position conflict whichever lands second, so the two root steps are split across the struct deliberately.
  • The version shell dispatches on the format segment before any read. A shell that resolved first would answer the contract sweep's row-less probe a JSON 404, which answersUnserved reads as served, failing a sibling arm's pending entry as stale. A segment outside packageRouteFormats reaches the shell and takes the existence-hiding 404 ahead of any 501.
  • The repository shell answers 501 unconditionally. Its resolve owes the hosted-only 404, and RepositoryKindHosted is the int16 zero value, so the verdict has to be read off the row and stored rather than inferred from an unset field. That is behavior with a security lens, so it lands in its own step and the three family arms inherit it.
  • Comments are terse by rule. check-comment-caps.sh caps an unexported doc at one line, so several rationales ride cap-free trailing comments or // --- banner anchors, and the rest live here and in the plan.

Size: 552 reviewable LOC against the 500 guideline. Only 308 of it is code:

Group code comment blank total
handler.go 28 18 14 60
the five new shell files 36 21 20 77
wire_management.go 0 18 15 33
production 64 57 49 170
handler_test.go 244 39 75 358
contract_test.go 0 16 8 24
tests 244 55 83 382

Splitting does not help. wire_management.go is comment-only, so its 33 lines add no logic and cannot be deferred without leaving later steps regionless. The 64 production code lines are the seven registrations, the pattern constants, and five shells averaging seven lines each. The tests are one sweep per property, none separable from the registrations it pins, and guardrail 6 forbids splitting them off.

Test plan

internal/managementapi/handler_test.go: 501 with the envelope per route and per version-format arm; the same rows against an unseeded reader asserting FindCalls() is empty; the same behind slugMiddleware; GET-only, so a dropped method fails on POST; the version route's out-of-family 404 for docker, oci, and an unknown segment; a repository named statistics keeping both its routes, with the name and the path segment sharing one constant so the collision cannot decay; ADR-009's four withdrawn per-artifact paths taking the mux 404; the four format literals counted against datastore.RepositoryFormats(), since a fifth format otherwise gets no route and nothing fails; and NewHandler not panicking, plus a scratch-mux pin on the stdlib premise the literals rest on.

Verified: go test -race and the -tags=integration suite for internal/managementapi, golangci-lint with and without the integration tag, and check-comment-caps.sh. The no-store sweep was mutation-checked — a shell that reads the store and still answers 501 passes the 501 sweep and is killed only by that test.

Also driven through the real binary (Postgres and MinIO in Docker, the kind rig being unavailable on this host): driver.sh smoke 72/72, then 62 adversarial checks over the seven routes covering the statuses above plus traversal, oversized, unicode, and query-param shapes, and 80 concurrent requests. All pass.

One pre-existing defect surfaced and is not this MR's: a NUL byte in the slug (/api/v1/%00/...) reaches NamespaceStore.FindBySlug as interpolated query text (LabKit pins pgx to simple-protocol mode), truncating the query and failing the whole statement, which resolveSlug maps to a logged 500 on every /api/v1/{slug}/... route, repositories from Phase 1 included, where the existence-hiding 404 is owed. These routes, including this MR's seven statistics routes, inherit it through slugMiddleware. Tracked in #484.

E2e scenario catalogs: unchanged. No route serves anything yet.

Context for LLM agents

Design rationale

Why registerStatisticsRoutes takes Deps and reads nothing from it. The seven shells answer 501 without touching a store. The parameter exists so each handler step reads its seam without editing the signature; two sibling steps adding it would collide on that one line, which is what the anchor scheme exists to prevent. registerArtifactWriteRoutes carries the same justification. unparam is disabled repo-wide and revive runs only a disabled rule, so no linter flags it.

Why the version arms are two named functions. Both answer 501 today, so no test can tell them apart; naming them gives each step a distinct, non-adjacent edit site. The limitation is recorded on statisticsRoutePaths: a dispatch wired to one arm for both formats passes every sweep here, and the sibling arm's landing is what catches it. Same trade the write-route sweeps accepted.

Why the format scope shares one shell across four routes. They differ only in the format their path names. The step that fills them parameterizes the shell, so the scaffold adds no unused parameter now.

Rejected alternatives

  • A {format} wildcard for the format scope. Panics at registration against repositoryPattern. TestServeMux_FormatWildcardOverlapsRepositoryDetail pins the stdlib premise on a scratch mux, so the reason survives even though the handler's own registration can no longer show it.
  • An S01-envelope fallback on the /api/v1/ subtree so an unknown :format answered JSON. Rejected: answersUnserved reads a non-JSON 404 as its unserved marker, so a fallback blinds the contract sweep's staleness detection for every pending operation. The tests here drive the handler directly and so assert the mux's own text/plain 404, which is the right layer for them; server.ResponseInterceptorMiddleware rewrites it one layer out, so the spec sentences describing that response are being corrected separately.
  • Registering all seven inside NewHandler. Each row is edited by its own step, so the rows need their own lines in a function whose body those edits do not share with the read-route block Phases 4 and 6 also touch.
  • Reusing dispatchPackageWrite for the version shell. Its shape is right, but its name and its packageWriteArm type both name the write surface, and generalizing them would touch six write-route families this step does not own. Its requestNamespaceID fail-loud guard is deliberately not carried over: the placeholder would discard the value, and TestHandler_StatisticsRoutes_UnknownSlugIs404 covers the same defect.

Non-goals

  • No DTOs. statistics_resources.go ships conventions plus six anchored regions, so no handler step modifies a file a non-predecessor created.
  • No pending-operations entries — six of seven now declared by !1964 (merged). contract_routes_test.go was untouched by this step, but !1964 (merged) has since merged and declared six of the seven statistics operations (getNamespaceStatistics, the four format operations, getRepositoryStatistics) in api/openapi/v1.yaml and contractPendingOperations. TestContractOperations_AreRegisteredAndServed now drives all six against these shells, and the repository shell's 501 answered before it resolves is what keeps answersUnserved reading each as unserved, exactly as this step intended. The seventh, version statistics, stays contract-undeclared until step 2 (Contract: artifact statistics) merges — this step's Depends on: - does not gate on step 2, so the window closes on step 2's own merge rather than on the two happening to land in sequence.
  • No Deps fields, guards, or assertion lines. A route answering 501 reads no store, so every anchor is prose until its scope's handler lands.
  • No plan-file edit. The Status table has a single writer; this row is recorded in the batch docs(plans) change.
  • No ADR-009 amendment. The merged S17 spec records the four-scope withdrawal and assigns the handbook amendment to its own Follow-ups, and this plan's Dependencies section states the ADR-009 items are handbook follow-ups rather than plan steps. This MR is nonetheless the first change to pin the absence executably, and unlike its neighbouring Follow-ups bullets that one names no work item, so the amendment is currently untracked. Flagged, not fixed here.

What the sweeps cannot see

statisticsRoutePaths returns all eight rows and servedStatisticsScopes is empty, so the two 501 sweeps drive everything today. A handler step drops its row by adding one map entry; the slug and method sweeps stay unconditional because they hold for a served row too. Two guards make the entry cost something: one rejects a key naming no row, the other drives the named row and fails if it still answers as unserved. Both were mutation-checked. Without this, the seven handler steps would each have to edit the sweeps' assertions.

Known merge interactions (measured with git merge-tree)

  • !1936 (merged) merged first, as b74edf4e0 in main. It replaced requireDeps' per-seam chain with a reflective walk and deleted requireArtifactDeps, and the rebase onto it dropped this branch's two guard-chain anchor regions, because the machinery they pointed at is gone. The anchor regions that survive in these two files are the Deps struct tail in handler.go, plus the construction sequence, the Deps literal, and the interface-assertion block in wire_management.go. A seam a later step adds to Deps is covered by the reflective walk with no guard of its own, so the regions that remain are the ones those steps need. The plan's per-step "its two nil guards" wording is now stale and needs one edit from the Status table's writer.
  • !1885 (merged) inserts at the identical position in wire_management.go (right after bulkDeletes := newBulkEnqueuer(...)). The resolution is "take both".

Related to #316

Edited by Hayley Swimelar

Merge request reports

Loading
Loading