feat(managementapi): serve the package and dist-tag read routes

Why

Phase 3 of the S17 management API serves the read-only artifact surface the monolith's repository-detail and artifact-detail views consume. Step 3 registered the package and dist-tag routes as 501 placeholders. This step replaces four of them, so a Maven or npm repository's packages, and an npm package's dist-tags, become readable.

Step 11 of the merged plan (docs/plans/2026-07-22-s17-phase3-format-artifact-reads.md). Originally stacked on hswimelar/s17-phase3-integration while Steps 1 through 9 were open. All roots have merged, so the branch is rebased onto main and retargeted (the retarget delta is the head commit, 4017ef7e).

What is worth a reviewer's attention

The dist-tag routes carry a literal npm segment, not a {format} binding. They resolve through Step 3's resolveNpmArtifactRepository. A family resolver here would read an empty {format} value and answer the logged 500 on every dist-tag request, which is why the happy-path 200 in dist_tags_test.go is the load-bearing assertion for that choice. The package routes resolve through resolvePackageArtifactRepository, which also refuses an out-of-family format segment before any lookup.

The dist-tag list reads its parent package before listing. Step 9's list query scopes to npm_package_id alone by design, and npm_tags carries no soft-delete column, so without the chain read the endpoint would serve a sibling repository's or a soft-deleted package's dist-tags.

MavenRepositoryStore is new. Every artifact query starts from the per-format child row, and Maven was the one member of that trio with no datastore-level resolver. The container and npm ones already exist; Maven's lived only in the protocol store, which also returns the repository Kind the management surface does not want. It mirrors NpmRepositoryStore's signature and predicate legs, which is what lets one consumer interface cover both formats. Unlike it, the constructor panics on a nil client, the package norm (32 of the 34 datastore constructors carry the guard, NewNpmRepositoryStore and NewContainerRepositoryStore are the two that do not).

A cursor key is bounded, not just checked for emptiness. A boundary reaches PostgreSQL as interpolated query text, so a NUL truncates the wire query and invalid UTF-8 is rejected by the server. Both failed the statement rather than the comparison and rendered 500 on malformed client input. The npm package and dist-tag stores carry no guard of their own, so the handler is the only place to catch it.

Two shared test constructors grew, in a Step 11 block. NewHandler and wireManagementAPIWithDeps guard every new seam, so newTestDeps, newIntegrationHandler, and managementWireDeps had to supply them or every test in those packages panics at construction. The 501-placeholder sweep and its per-route flag are deleted: with all 13 routes live, TestContractOperations_AreRegisteredAndServed and the wire-level sweep assert served JSON envelopes instead.

Spec coverage

Acceptance criterion Covered by
AC #19 (closed) packages list and detail, both formats TestPackageListHandler_MavenPage_SerializesCoordinates, TestPackageListHandler_NpmPage_SerializesNameScopeAndCounters, TestPackageDetailHandler_ReturnsTheAddressedPackage
AC #22 (closed) npm dist-tag list and detail TestNpmDistTagListHandler_SerializesTagsWithTheirVersion, TestNpmDistTagDetailHandler_ReturnsTheAddressedTag
AC #23 format segment must match the stored format TestPackageListHandler_FormatSegmentMismatch_Returns404, TestPackageDetailHandler_UnreachableRepositoryOutranksTheIDParse, TestNpmDistTagListHandler_MavenRepository_Returns404
AC #24 keyset pagination, Link header, both directions TestPackageListHandler_MavenKeysetWalk_IsGapFreeInBothDirections, TestPackageListHandler_NpmKeysetWalk_HonorsDescendingOrder, TestNpmDistTagListHandler_KeysetWalk_IsGapFreeAndClampsTheLimit, TestPackageReadHandlersIntegration_*
AC #25 invalid sort, order, limit, cursor return 400; limit clamps TestPackageListHandler_InvalidQueryParameters_Return400, TestNpmDistTagListHandler_InvalidQueryParameters_Return400, TestPackageListHandler_LimitAboveMaximumIsClamped
AC #26 (closed) missing parent 404, existing empty parent 200 [] TestNpmDistTagListHandler_VerifiesTheParentChainBeforeListing, TestPackageListHandler_EmptyParent_Returns200EmptyArray, TestNpmDistTagListHandler_EmptyPackage_Returns200EmptyArray
AC #27 (closed) tenant isolation and existence hiding TestPackageListHandler_ScopesQueryToNamespaceAndChildRow, TestPackageDetailHandler_PackageInAnotherRepository_Returns404, TestPackageDetailHandler_NonCanonicalOrMissingID_Returns404, TestPackageReadHandlersIntegration_NpmWalkAndCrossRepositoryIsolation
Contract conformance for all four operations TestPackageReadHandlers_ResponsesMatchOpenAPIContract, TestPackageReadHandlers_PaginatedResponseMatchesContract

Test plan

go test ./internal/managementapi/ ./internal/datastore/ ./cmd/artifact-registry/
go test -tags=integration -count=1 ./internal/managementapi/
go-lint-ci ./internal/managementapi/... ./cmd/artifact-registry/...
go-lint-ci --build-tags=integration ./internal/managementapi/...

All pass locally. The integration walks seed artifacts through the protocol write paths (Maven find-or-create, the npm package, version, and tag writers) and follow the server's own Link headers against the real keyset SQL.

One pre-existing condition, not introduced here:

  • TestWireStorage_CloudCDNPresent fails locally without Google application default credentials.

No conformance run: Phase 3 is management-API surface, not Maven, npm, or OCI protocol behavior.

No e2e scenario catalog change. The catalogs cover protocol-client journeys, and artifact browsing is a UI journey that lands with the monolith S05 and S06 slices, which own the catalog additions. The plan's Testing Strategy settles this.

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml), with synthesized seed data rolled back per query and the container torn down at the end of the run. Numbers reflect moderate cardinality and do not capture production-scale effects. See Database review evidence for seed sizing, methodology, and the anomalies the skill flags. Expand each row's details for the seed shape, rendered SQL, bound args, and raw plan.

Method Plan node Index Rows (plan / actual) Cost Time Buffers (hit / read) Partitions
datastore.MavenRepositoryStore.FindHostedByNameInNamespace Limit → Nested Loop repositories_p41_namespace_id_format_name_idx, maven_repositories_p41_namespace_id_repository_id_idx 1 / 1 16.62 0.031ms 9 / 0 2 (1 of 64 per table)
datastore.MavenRepositoryStore.FindHostedByNameInNamespace

Summary: Plan matches the method's intent. A Nested Loop drives from an Index Scan on the partial (namespace_id, format, name) index over repositories into a unique-index lookup on maven_repositories, with both tables pruned to one of their 64 hash partitions. The index's WHERE soft_deleted_at IS NULL predicate absorbs the soft-delete leg, and kind falls to a filter applied to the single row the index already isolated. Estimates match reality (1 / 1) at 5000 seeded repositories, with 9 buffer hits and no heap reads. No anomalies.

Seed shape: namespaces=1, repositories=5000, maven_repositories=5000

All 5000 repositories are maven-format, hosted, and active, so the format, kind, and soft_deleted_at legs discriminate nothing and name is the only selective column. That is the worst case for this predicate shape. Every row shares the test namespace, so all 5000 land in a single partition on each table.

Rendered SQL:

SELECT maven_repositories.id AS "maven_repositories.id",
     maven_repositories.repository_id AS "maven_repositories.repository_id"
FROM public.maven_repositories
     INNER JOIN public.repositories ON ((repositories.id = maven_repositories.repository_id) AND (repositories.namespace_id = maven_repositories.namespace_id))
WHERE ((((maven_repositories.namespace_id = $1::uuid) AND (repositories.name = $2::text)) AND (repositories.format = $3)) AND (repositories.kind = $4)) AND (repositories.soft_deleted_at IS NULL)
LIMIT $5;

Bound args: ['f9a0242e-7ed9-44d6-ab67-7ea70bb1ca64', 'review-prep-repo-002500', 1, 0, 1]

$3 and $4 are bound as bigint, matching the wire type the driver sends for jet's pg.Int(int64(...)). The plan shows the widened comparison against the smallint columns still resolving through the index (format = '1'::bigint as an Index Cond), so the width mismatch costs nothing.

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=0.56..16.62 rows=1 width=32) (actual time=0.031..0.031 rows=1 loops=1)
   Buffers: shared hit=9
   ->  Nested Loop  (cost=0.56..16.62 rows=1 width=32) (actual time=0.030..0.030 rows=1 loops=1)
         Buffers: shared hit=9
         ->  Index Scan using repositories_p41_namespace_id_format_name_idx on repositories_p41 repositories  (cost=0.28..8.31 rows=1 width=32) (actual time=0.022..0.022 rows=1 loops=1)
               Index Cond: ((namespace_id = 'f9a0242e-7ed9-44d6-ab67-7ea70bb1ca64'::uuid) AND (format = '1'::bigint) AND (name = 'review-prep-repo-002500'::text))
               Filter: (kind = '0'::bigint)
               Buffers: shared hit=6
         ->  Index Scan using maven_repositories_p41_namespace_id_repository_id_idx on maven_repositories_p41 maven_repositories  (cost=0.28..8.30 rows=1 width=48) (actual time=0.007..0.007 rows=1 loops=1)
               Index Cond: ((namespace_id = 'f9a0242e-7ed9-44d6-ab67-7ea70bb1ca64'::uuid) AND (repository_id = repositories.id))
               Buffers: shared hit=3
 Planning:
   Buffers: shared hit=128 read=1
 Planning Time: 0.691 ms
 Execution Time: 0.048 ms

Not-found path: the same plan shape, with the maven_repositories scan never executed. A name that does not exist costs 5 buffer hits and 0.041ms; a name that exists under another format costs 2 hits and 0.050ms. A miss is no more expensive than a hit, so the existence-hiding 404 leaks no timing signal at this cardinality.

Timings: planning 0.691ms, execution 0.048ms, total 0.739ms.

Context for LLM agents

Design rationale

Why a new resolver method rather than reusing the protocol lookup. The handler needs maven_repositories.id for every Maven artifact query. The protocol resolver (FindByNameInNamespace, relocated into internal/datastore by #372 (closed)) returns the repository Kind so an upload to a remote repository can answer 405, and threads an explicit qrm.DB. The management read surface needs neither, so FindHostedByNameInNamespace lives beside it on the same store, filtered to kind hosted. Its signature mirrors NpmRepositoryStore.FindHostedByNameInNamespace exactly, which collapses two consumer interfaces into one.

Why the repository is resolved twice. resolvePackageArtifactRepository resolves it by name to learn the stored format, and the child-row resolver resolves it again to get the binding surrogate. Collapsing them needs a child lookup keyed by repositories.id, a plausible follow-up that would either add a method to two stores or drop the format-dispatch step the packages routes need. Two indexed point lookups on a management read path was judged the cheaper trade.

Why servePackagePage is generic. The backward-page order flip, the display-order reversal, and the Link presence rules are the part most likely to drift between formats: a fix applied to one copy and not the others pages a whole family wrongly with every other test still green. The remaining per-format duplication carries a dupl suppression because every line of it names a different params struct, order vocabulary, cursor type, store method, and serializer.

Why 400 precedes the parent resolve on a list, but the repository 404 precedes the id parse on a detail. A 400 reveals nothing about whether an artifact exists, so validating parameters first is free and matches Phase 1. A 404, in contrast, has to name the highest break in the chain, or a container repository with a malformed id reports "artifact not found" and describes the wrong level.

Non-goals

  • No cache envelope. It belongs to remote-kind rows and is finalized when S13 lands, so every Phase 3 hosted response omits the key.
  • No versions or files. Step 12 owns those routes and their Deps seams.
  • No container routes. Step 10 owns those.
  • No name field on MavenPackage. The spec makes name a cross-format sort alias for (group_id, artifact_id), and adding the field would also make the contract's Package oneOf ambiguous.
  • No digest or checksum serialization. None of the three resources this step adds carries one, so Step 3's formatDigest and formatChecksum are Step 10's and Step 12's to call.
  • No plan Status table edit. Filled once, later, across the whole batch.

Cross-step hazards for whoever merges the batch

  • The Deps struct, requireDeps, wireManagementAPI, and wireManagementAPIWithDeps edits stay inside Step 3's Step 11 anchors, so they are disjoint from Steps 10 and 12 at the git level. Semantically they are not: all three steps need a per-format child-row resolver seam, which Step 3 did not anticipate, so if another step names its Deps field MavenRepositories or NpmRepositories too, the merge produces duplicate struct fields. That is a build error on the integration branch, not a silent one.
  • newTestDeps, newIntegrationHandler, managementWireDeps, the artifactRoute table, and contract_test.go are edited by all three handler steps. Each edit here is one contiguous block labelled Step 11.
  • The shared seedIntegrationNamespace cleanup could not unwind a namespace once artifacts existed under it: it deletes repositories and relies on ON DELETE CASCADE for the child rows, but the artifact tables reference those child rows with plain foreign keys. Steps 10 and 12 will hit this too. Handled here with a leaf-first cleanup registered from the step's own file rather than by changing the shared helper.
  • internal/datastore/maven_repositories.go and both sibling test files were a planned add/add conflict with main (the #372 (closed) relocation declared MavenRepositoryStore at this path first). Folded in this rebase: FindHostedByNameInNamespace, its statement test, and the six-shape integration suite now sit inside main's relocated store and suites with zero deletions of main's content, and the twin-doc paragraphs point at FindByNameInNamespace in the same file (rewritten in 0c353a46).
  • A prose formatter rewrote a pair of ASCII single quotes inside a Go doc comment into a typographic double quote. Worth knowing if a comment edit ever fails to apply.

Related to #312 (closed)

Edited by Hayley Swimelar

Merge request reports

Loading
Loading