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)
Depstakes 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, whichanswersUnservedreads as served, failing a sibling arm's pending entry as stale. A segment outsidepackageRouteFormatsreaches the shell and takes the existence-hiding404ahead of any501. - The repository shell answers
501unconditionally. Its resolve owes the hosted-only404, andRepositoryKindHostedis theint16zero 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.shcaps 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 againstrepositoryPattern.TestServeMux_FormatWildcardOverlapsRepositoryDetailpins 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:formatanswered JSON. Rejected:answersUnservedreads a non-JSON404as 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/plain404, which is the right layer for them;server.ResponseInterceptorMiddlewarerewrites 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
dispatchPackageWritefor the version shell. Its shape is right, but its name and itspackageWriteArmtype both name the write surface, and generalizing them would touch six write-route families this step does not own. ItsrequestNamespaceIDfail-loud guard is deliberately not carried over: the placeholder would discard the value, andTestHandler_StatisticsRoutes_UnknownSlugIs404covers the same defect.
Non-goals
- No DTOs.
statistics_resources.goships 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.gowas untouched by this step, but !1964 (merged) has since merged and declared six of the seven statistics operations (getNamespaceStatistics, the four format operations,getRepositoryStatistics) inapi/openapi/v1.yamlandcontractPendingOperations.TestContractOperations_AreRegisteredAndServednow drives all six against these shells, and the repository shell's501answered before it resolves is what keepsanswersUnservedreading each as unserved, exactly as this step intended. The seventh, version statistics, stays contract-undeclared until step 2 (Contract: artifact statistics) merges — this step'sDepends 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
Depsfields, guards, or assertion lines. A route answering501reads 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
b74edf4e0inmain. It replacedrequireDeps' per-seam chain with a reflective walk and deletedrequireArtifactDeps, 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 theDepsstruct tail inhandler.go, plus the construction sequence, theDepsliteral, and the interface-assertion block inwire_management.go. A seam a later step adds toDepsis 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 afterbulkDeletes := newBulkEnqueuer(...)). The resolution is "take both".
Related to #316