feat(managementapi): serve npm's virtual kind (S17 Phase 6 plan: 37/42)
Why
Phase 1 shipped create with a hosted-only kind gate and no removal contract for the virtual kind, so a virtual create answers 422 for every format and a virtual DELETE answers 500 whatever destructive carries: Repository DELETE answers 500 for the virtual k... (#819) • Unassigned. Phase 6 ends both one format at a time, gated on each format's virtual schema, and npm's is the schema that exists on main. The npm upstream-association surface that follows has no repository to hang off until this lands.
Acceptance is the npm slices of AC #71 (closed) (a virtual create's 201 carries no settings key and an empty upstream list) and AC #109 (closed) (204 under either destructive value, associations removed in the same transaction). Maven keeps the 422 on create, so #819 stays open for it and this MR cites it rather than closing it.
What
The virtual child row is added beside the kind-agnostic format child, never swapped for it. npm_repositories carries every npm repository whatever its kind, and the npm resolver narrows on no kind predicate, so a create that swapped the children would leave every npm protocol route unable to resolve the repository. internal/datastore/npm_virtual_repositories.md carries that and the file's other reasoning.
The delete takes the virtual child row's FOR UPDATE lock before the cascade. One DELETE on the parent still does all the removal, because every foreign key into the virtual child and its junction tables declares ON DELETE CASCADE, but it runs inside a transaction that locks the child row first, the order an upstream association mutation will take. The lock deliberately gates no active parent: Delete filters no soft_deleted_at, so requiring one would strand every tombstoned virtual repository, which nothing on either path could then remove.
repositoryDeleteKindGate is untouched, and Delete branches to the virtual arm ahead of it. The virtual arm resolves its own refusal from the dispatch that resolves the lock, so the kind gate answers for the bare cascade only. Its other caller, RepositoryReaper.finalize, runs a bare DELETE on the walk chunk's handle and holds no child-row lock, so it keeps refusing every virtual kind.
The per-format seams are the container step's, which merged first, which is why they look the way they do. virtualChildInsert and virtualChildDeleteLock read one {insert, lock} map keyed by format, each returning the refusal sentinel for an absent key, and VirtualRepositoryCreateSupported publishes the same verdict to the handler, so the 422 and the store cannot drift. This step adds the npm entry beside the container family's.
One ADR-009 deviation, disclosed rather than closed. ADR-009 requires a 409 under either destructive value for a repository any virtual repository lists as an upstream, stating that destructive intent does not reach it. Instead, the junction key's 23503 becomes ErrRepositoryNotEmpty, and the handler's existing destructive=true arm tombstones a row the reap finalizer then refuses forever on its kind-only gate. The container step opened that path first, for docker and oci, and this one adds npm. After it, the only way in is a direct database write of a row the spec already forbids, because no route writes either junction table. AC #110 (closed) is the acceptance criterion, the plan assigns the guard to Steps 39 to 42, and internal/datastore/repositories.md records the deviation beside the code that has it.
Rollback is a one-way door. Once an npm virtual repository exists, reverting the delete arm leaves it undeletable, which is the defect this fixes. Revert both arms or neither. Mid-rollout, an un-upgraded pod answers 500 for a delete of a repository an upgraded pod created, resolving as the fleet converges.
No contract schema change. Kind's enum already carries virtual and a virtual create declares no settings, so api/openapi/v1.yaml moves prose only. The Bruno collection carries a request per served kind and format, so npm's virtual create joins it.
Size. 1202 reviewable lines, of which 915 are tests and 145 the contract, Bruno, and doc files, leaving 142 of production Go, under the guardrail's ceiling on its own. Splitting create from delete would leave npm virtual repositories answering 500 on DELETE for the window between the two MRs, which is the defect #819 records.
Test plan
| # | Criterion | Tests |
|---|---|---|
| AC #71 (closed) | POST with kind=virtual creates a virtual repository whose response carries no settings key and whose upstream list is empty |
TestCreateHandler_VirtualKind_Npm_Returns201, TestVirtualNpmIntegration_CreateReadDeleteWalk, TestRepositoryStore_Create_VirtualNpm, TestRepositoryStore_Create_VirtualNpmRollsBackWhole |
| AC #109 (closed) | DELETE on a virtual repository returns 204 under either destructive value with its associations removed in the same transaction |
TestVirtualNpmIntegration_CreateReadDeleteWalk (both values), TestRepositoryStore_Delete_VirtualNpmRemovesAssociations, TestRepositoryStore_Create_VirtualNpmThenDelete |
| Error path | Tests |
|---|---|
kind=virtual on a format serving none: 422, no rows written |
TestCreateHandler_NonHostedKind_Returns422, TestValidateCreateKind_PerFormat, TestRepositoryStore_Create_KindGateFailsClosed, TestRepositoryStore_Delete_VirtualKindFailsClosed |
A kind outside the enumeration: 422 naming the accepted kinds |
TestCreateHandler_NonHostedKind_Returns422, TestCreateKindMessageFor_TextsDiffer |
settings on a virtual create: 400 at the handler, typed sentinel at the store |
TestCreateHandler_VirtualKind_Npm_WithSettings_Returns400, TestRepositoryCreateKindGate, TestRepositoryStore_Create_KindDispatchGuards |
| Absent id, or one in another namespace, on the virtual delete path | TestRepositoryStore_Delete_VirtualNpmNotFound |
| A tombstoned virtual parent still deletes (work item 829) | TestRepositoryStore_Delete_VirtualNpmTombstonedParent |
| A virtual parent carrying no virtual child row still deletes | TestRepositoryStore_Delete_VirtualNpmWithoutChildRow |
| A repository that is a live association's upstream cannot delete | TestRepositoryStore_Delete_VirtualNpmUpstreamTargetBlocked |
| Nil context, nil handle, or a zero id on either child-row write | TestNpmVirtualRepositoryWrites_ArgumentGuards, TestNpmVirtualRepositoryWrites_GuardsRunBeforeAnyStatement |
| Insert and lock dispatch drift across every format | TestVirtualChildDispatches_AgreeOnEveryFormat, TestVirtualChildDispatches_ServeNpm |
The lock statement takes FOR UPDATE and filters no soft_deleted_at |
TestLockNpmVirtualRepositoryStmt_TakesForUpdateOnBothKeyColumns |
| The tolerated missing child row leaves a record | TestRepositoryStore_Delete_VirtualNpmWithoutChildRow |
| The audit guards stay closed on the virtual kind | TestCreateHandler_VirtualCreate_EmitsNoAuditEvent, TestDeleteHandler_VirtualRepository_EmitsNoAuditEvent |
| The per-format cap counts virtual rows (AC #72) | TestRepositoryStore_Create_VirtualNpmOccupiesCapRoom |
Gates: golangci-lint in both modes, the integration-tagged run included because CI cannot compile those files, plus the comment-caps gate, both unit suites, and the integration suites against a Postgres with max_locks_per_transaction=4096. Red-then-green was measured on the branch, the test commit failing with both child-row writes panicking and both new query names at zero call sites.
e2e scenarios: docs/testing/e2e/npm.md listed virtual npm repositories as wholly out of scope, which these two routes falsify, so its scope note now carves them in and holds the client journeys and the upstream associations out. No scenario is added: the plan's Testing Strategy records Phase 6's e2e impact as none, because the catalogs are protocol-client driven and the monolith slices that consume these endpoints own those additions.
Conformance: not applicable, no protocol-path behavior changes.
Context for LLM agents
Rationale
- Widen
repositoryDeleteKindGateto dispatch on format, so one gate answers for both removers. Rejected: its other caller,RepositoryReaper.finalize, runs a bareDELETEon the walk chunk's handle and holds no virtual child-row lock, so admitting a virtual kind there would remove one without the ordering the spec requires of the removal. - Inject the npm virtual child writes through a constructor seam, as the three remote creators are. Rejected: that wiring is Step 24's create-enablement gate, and a virtual arm that ships serving needs no gate a wiring slip could trip. Two unexported package-level functions instead, matching
insertFormatChild. - A switch per resolver, refusing in the
defaultarm. Rejected on review: two switches one function apart had to agree on the served set, and a test held them in agreement rather than the code. One{insert, lock}map entry per served format makes that agreement structural. The map is deliberately partial whereformatChildInsertsandremoteChildCreatesare total, so each resolver still answers an absent key with its own sentinel rather than a totality test the virtual arms cannot pass. - Refuse the delete when the lock finds no virtual child row. Rejected: the concurrent-delete case already answers not-found through the parent
DELETE's zero-row count, and refusing would wedge a parent that neitherDeletenor the reap finalizer can remove. - Chosen: two resolvers that return the refusal sentinel as their error, one exported predicate publishing the create verdict, and a virtual delete branch ahead of the untouched kind gate.
Consequences: a format's virtual arm is one map entry plus one file, and the reaper keeps failing closed on every virtual kind until a remover that takes the lock exists.
Non-goals
- The upstream association store, its five routes, and the delete-side
409for a repository some virtual repository lists as an upstream. Steps 39 to 42 own them, and the junction FK'sNO ACTIONanswers today by aborting that delete asErrRepositoryNotEmpty. - The lock-order test against a concurrent associate. It needs the association write path, so it arrives with Step 40.
- A
writeDeleteStoreErrorarm forErrRepositoryDeleteVirtualKind. Within one binary, create refuses the kinds that can still return it, so only a direct database write reaches it, and each format's own step closes its share. The mixed-fleet window is the rollback note above. - npm virtual protocol serving. A created virtual repository resolves its binding and answers
501on the npm routes until S31's serve surface lands, which is that slice's window. - Maven virtual creates.
maven_virtual_repositoriesdoes not exist in the tree. The container arm landed with Step 38, against these same seams.
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.RepositoryStore.Delete.KindProbe |
Limit | repositories_p59_pkey |
1 / 1 | 8.30 | 0.012ms | 3 / 0 | 1/64 |
datastore.RepositoryStore.deleteVirtualRepository |
Delete | repositories_p14_pkey |
0 / 0 | 8.30 | 0.233ms | 68 / 0 | 1/64 |
datastore.insertNpmVirtualRepository |
Insert | n/a |
0 / 0 | 0.01 | 0.169ms | 2 / 2 | 1/64 |
datastore.lockNpmVirtualRepository |
LockRows | npm_virtual_repositories_p06_namespace_id_repository_id_idx |
1 / 1 | 8.31 | 0.022ms | 4 / 0 | 1/64 |
datastore.RepositoryStore.Delete.KindProbe
Summary: The probe reads the two deciding columns by primary key, and the plan matches: an Index Scan on a partition of pk_repositories with both key columns bound, so the namespace literal prunes to one of 64 partitions. Estimate and actual agree at 1 row against 5000 seeded rows in the partition. No anomalies.
Seed shape: namespaces=1, repositories=5000
Rendered SQL:
SELECT repositories.kind AS "repositories.kind",
repositories.format AS "repositories.format"
FROM public.repositories
WHERE (repositories.namespace_id = $1::uuid) AND (repositories.id = $2::uuid)
LIMIT $3;Bound args: [d4ff4103-7e50-700f-9417-32cd325567e3, 35c3b9bb-aeac-794c-85d2-16e8272fc0bc, 1]
Plan (EXPLAIN (ANALYZE, BUFFERS) output):
Limit (cost=0.28..8.30 rows=1 width=4) (actual time=0.011..0.012 rows=1 loops=1)
Buffers: shared hit=3
-> Index Scan using repositories_p59_pkey on repositories_p59 repositories (cost=0.28..8.30 rows=1 width=4) (actual time=0.010..0.011 rows=1 loops=1)
Index Cond: ((id = '35c3b9bb-aeac-794c-85d2-16e8272fc0bc'::uuid) AND (namespace_id = 'd4ff4103-7e50-700f-9417-32cd325567e3'::uuid))
Buffers: shared hit=3
Planning:
Buffers: shared hit=354
Planning Time: 1.202 ms
Execution Time: 0.022 msTimings: planning 1.202ms, execution 0.022ms, total 1.224ms.
datastore.RepositoryStore.deleteVirtualRepository
Summary: The cascading delete reaches its row by primary key on one partition, and execution is dominated by the twelve foreign-key triggers the cascade fires rather than by the scan, which costs 0.010ms of the 13.284ms. Those triggers are the removal contract: npm_virtual_repositories_..._fkey14 clears the virtual child row, and npm_virtual_repository_upstr_upstream_repository_id_name_fkey14 is the NO ACTION check that aborts the delete when the repository is still a listed upstream. No anomalies. Seeded at 50 rows instead of 5000 the same statement falls back to a Seq Scan over the partition, which is the low-cardinality artifact the methodology warns about rather than a plan defect, so the figures above are the 5000-row run.
Seed shape: namespaces=1, repositories=5000, npm_virtual_repositories=1
Rendered SQL:
DELETE FROM public.repositories
WHERE (repositories.namespace_id = $1::uuid) AND (repositories.id = $2::uuid);Bound args: [c0517997-c154-7874-ad5a-cfdf42b2df26, 58ca6bc0-363c-72e3-bd91-6815583f8113]
Plan (EXPLAIN (ANALYZE, BUFFERS) output):
Delete on repositories (cost=0.28..8.30 rows=0 width=0) (actual time=0.233..0.233 rows=0 loops=1)
Delete on repositories_p14 repositories_1
Buffers: shared hit=68
-> Index Scan using repositories_p14_pkey on repositories_p14 repositories_1 (cost=0.28..8.30 rows=1 width=10) (actual time=0.009..0.010 rows=1 loops=1)
Index Cond: ((id = '58ca6bc0-363c-72e3-bd91-6815583f8113'::uuid) AND (namespace_id = 'c0517997-c154-7874-ad5a-cfdf42b2df26'::uuid))
Buffers: shared hit=3
Planning:
Buffers: shared hit=138
Planning Time: 0.636 ms
Trigger for constraint container_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.063 calls=1
Trigger for constraint npm_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.020 calls=1
Trigger for constraint maven_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.003 calls=1
Trigger for constraint repository_collection_reposit_repository_id_namespace_id_fkey14 on repositories_p14: time=0.962 calls=1
Trigger for constraint npm_remote_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.361 calls=1
Trigger for constraint maven_remote_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.309 calls=1
Trigger for constraint container_remote_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=1.338 calls=1
Trigger for constraint npm_virtual_repositories_repository_id_namespace_id_fkey14 on repositories_p14: time=0.893 calls=1
Trigger for constraint npm_virtual_repository_upstr_upstream_repository_id_name_fkey14 on repositories_p14: time=1.498 calls=1
Trigger for constraint container_virtual_repositorie_repository_id_namespace_id_fkey14 on repositories_p14: time=0.976 calls=1
Trigger for constraint container_virtual_repository_upstream_repository_id_name_fkey14 on repositories_p14: time=1.298 calls=1
Trigger for constraint npm_virtual_repository_upstr_npm_virtual_repository_id_n_fkey14 on npm_virtual_repositories_p14: time=0.135 calls=1
Execution Time: 13.284 msTimings: planning 0.636ms, execution 13.284ms, total 13.920ms.
datastore.insertNpmVirtualRepository
Summary: A single-row insert routed by namespace_id to one of 64 partitions, with the two foreign-key triggers that hold the child to its namespace and its parent repository. No scan, so no index applies. No anomalies.
Seed shape: namespaces=1, repositories=1
Rendered SQL:
INSERT INTO public.npm_virtual_repositories (id, namespace_id, repository_id)
VALUES ($1::uuid, $2::uuid, $3::uuid);Bound args: [b7169cdb-d065-74ff-a931-ea6be4597acd, 883bc474-5508-7168-b25d-f357bde0b5ed, 69c6aa81-dd04-723f-a6b4-d84a72ac6131]
Plan (EXPLAIN (ANALYZE, BUFFERS) output):
Insert on npm_virtual_repositories (cost=0.00..0.01 rows=0 width=0) (actual time=0.169..0.169 rows=0 loops=1)
Buffers: shared hit=2 read=2 dirtied=5 written=3
-> Result (cost=0.00..0.01 rows=1 width=48) (actual time=0.001..0.001 rows=1 loops=1)
Planning Time: 0.034 ms
Trigger for constraint fk_nvr_namespace_id_namespaces on npm_virtual_repositories_p35: time=0.153 calls=1
Trigger for constraint fk_nvr_repository_id_repositories on npm_virtual_repositories_p35: time=1.452 calls=1
Execution Time: 1.872 msTimings: planning 0.034ms, execution 1.872ms, total 1.906ms.
datastore.lockNpmVirtualRepository
Summary: The delete-time lock takes the row through a partition of unique_nvr_ns_id_repository_id, the unique index the partition key leads, so the read prunes to one of 64 partitions and returns at most one row by construction. LockRows above the Index Scan is the FOR UPDATE. Estimate and actual agree at 1 row against 5000 seeded child rows in the partition. No anomalies.
Seed shape: namespaces=1, repositories=5000, npm_virtual_repositories=5000
Rendered SQL:
SELECT npm_virtual_repositories.id AS "npm_virtual_repositories.id"
FROM public.npm_virtual_repositories
WHERE (npm_virtual_repositories.namespace_id = $1::uuid) AND (npm_virtual_repositories.repository_id = $2::uuid)
FOR UPDATE;Bound args: [652f0e59-c1c0-71e2-9cd3-b2cd73bc94be, fd1aa7b7-0472-77da-a809-8014fe07b7b6]
Plan (EXPLAIN (ANALYZE, BUFFERS) output):
LockRows (cost=0.28..8.31 rows=1 width=26) (actual time=0.021..0.022 rows=1 loops=1)
Buffers: shared hit=4
-> Index Scan using npm_virtual_repositories_p06_namespace_id_repository_id_idx on npm_virtual_repositories_p06 npm_virtual_repositories (cost=0.28..8.30 rows=1 width=26) (actual time=0.016..0.017 rows=1 loops=1)
Index Cond: ((namespace_id = '652f0e59-c1c0-71e2-9cd3-b2cd73bc94be'::uuid) AND (repository_id = 'fd1aa7b7-0472-77da-a809-8014fe07b7b6'::uuid))
Buffers: shared hit=3
Planning:
Buffers: shared hit=60
Planning Time: 0.697 ms
Execution Time: 0.050 msTimings: planning 0.697ms, execution 0.050ms, total 0.747ms.
Related to #314