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_CloudCDNPresentfails 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 msNot-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
cacheenvelope. 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
Depsseams. - No container routes. Step 10 owns those.
- No
namefield onMavenPackage. The spec makesnamea cross-format sort alias for(group_id, artifact_id), and adding the field would also make the contract'sPackageoneOfambiguous. - No digest or checksum serialization. None of the three resources this step
adds carries one, so Step 3's
formatDigestandformatChecksumare 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
Depsstruct,requireDeps,wireManagementAPI, andwireManagementAPIWithDepsedits 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 itsDepsfieldMavenRepositoriesorNpmRepositoriestoo, the merge produces duplicate struct fields. That is a build error on the integration branch, not a silent one. newTestDeps,newIntegrationHandler,managementWireDeps, theartifactRoutetable, andcontract_test.goare edited by all three handler steps. Each edit here is one contiguous block labelled Step 11.- The shared
seedIntegrationNamespacecleanup could not unwind a namespace once artifacts existed under it: it deletesrepositoriesand relies onON DELETE CASCADEfor 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.goand both sibling test files were a planned add/add conflict withmain(the #372 (closed) relocation declaredMavenRepositoryStoreat this path first). Folded in this rebase:FindHostedByNameInNamespace, its statement test, and the six-shape integration suite now sit insidemain's relocated store and suites with zero deletions ofmain's content, and the twin-doc paragraphs point atFindByNameInNamespacein 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)