feat(managementapi): destructive repository DELETE (S20-A plan: 18/22)

What this step delivers

The repository delete route now takes a required destructive query parameter. The route is DELETE /api/v1/:slug/repositories/:repository_name. The handler reads the parameter before it resolves the repository.

• destructive must be present exactly once and spelled exactly true or false. Anything else returns 400 bad_request and changes nothing. The guard runs before any store access, so a refused request tells the caller nothing about the repository.
• An empty repository returns 204 under either value, and its name is free for reuse at once.
• A non-empty repository under destructive=false returns 409 conflict, and the contents stay intact. The message names the remedy that caller has. It tells a hosted npm or hosted container caller to empty the repository first. It tells a Maven caller, and the caller of any remote repository, to resend with destructive=true.
• A non-empty repository under destructive=true returns 202. The new lifecycle.Tombstoner writes soft_deleted_at and enqueues the purge job on one transaction, under a bounded deadline. A rolled-back tombstone leaves no job, and a committed tombstone always leaves one.
• A repository that is absent returns 404 under either value. A repository that an earlier 202 accepted returns 404 too.
• After the 202, every route that names the repository in its own path answers 404. The npm tarball download and dist-tags routes wrote 500 on that race before this change.
• The 202 emits usagedata.EventRepositoryDeleted. The purge that reaps the rows later emits nothing, so a destructive delete counts exactly once.
• A failed tombstone logs repository_id as its own structured field.

Emptiness is never counted in advance. The delete's own outcome classifies the repository, so an artifact that lands mid-request changes which correct answer the caller gets.

The contract surfaces move with the operation. api/openapi/v1.yaml declares the parameter, the 400 and the 202, and the Bruno request sends the parameter. Both state that deletion is irreversible: no restore endpoint, no trash to list, and no window in which an accepted delete can be recalled. That sentence closes the documentation acceptance criterion of issue #464 (closed).

The run recipe in .claude/skills/run-artifact-registry/ carries the new parameter and the 202, and driver.sh smoke ran green against it.

Spec coverage

Numbering follows the Repository entry point group of ## Acceptance Criteria in the spec.

#	Criterion	Tests that pin it
AC-1	DELETE without destructive, or with any value other than true or false, returns 400 and changes nothing	TestDeleteHandler_DestructiveParameter_RejectedValues (13 rows), TestDeleteHandler_RejectedParameter_IsNotAnExistenceOracle
AC-2	destructive=false on an empty repository returns 204, the row is gone, a create at the name returns 201	TestDeleteHandler_EmptyRepository_BothValuesReturn204/destructive=false, TestDeleteHandlerIntegration_DeleteFreesName
AC-3	destructive=false on a non-empty repository returns 409 and leaves the repository and its contents intact	TestDeleteHandler_NonEmptyRepository_ConflictNamesTheRemedy, TestDeleteHandlerIntegration_DestructiveFalse_NonEmptyRepositoryRefuses
AC-4	The 409's message names the remedy the caller has; code is conflict for both arms; asserted for a hosted npm, a Maven and a remote repository	TestDeleteHandler_NonEmptyRepository_ConflictNamesTheRemedy (8 rows: hosted npm, docker, oci, maven, plus all four remote formats), TestDeleteHandler_NonEmptyRepository_TheTwoRemediesAreDistinct, TestDeleteHandler_VirtualKind_SelectorStaysTotalOnAnUnreachableState
AC-5	destructive=true on a non-empty repository returns 202, sets soft_deleted_at, and enqueues the purge job in the tombstone's own transaction; a rolled-back tombstone leaves no job and a committed one always leaves a live one	TestDeleteHandler_DestructiveTrue_NonEmptyRepository_Returns202, TestDeleteHandlerIntegration_DestructiveTrue_TombstonesAndEnqueues, TestDeleteHandlerIntegration_RolledBackTombstone_LeavesNoJobAndNoStamp, TestDeleteHandlerIntegration_SecondTombstoneCollapsesOntoTheLiveJob. Corrected since the table was written: the collapse test is named.
AC-6	destructive=true on an empty repository returns 204 and takes neither the tombstone nor the job	TestDeleteHandler_EmptyRepository_BothValuesReturn204/destructive=true, TestDeleteHandlerIntegration_DestructiveTrue_EmptyRepositoryTakesNeither
AC-7	Immediately after the 202 every client route naming that repository in its own path returns 404, asserted per format	Management API detail, update, delete and the artifact routes: TestDeleteHandlerIntegration_DestructiveTrue_TombstonesAndEnqueues. npm: TestPackument_TombstonedRepository_Returns404, TestDownloadHandler_TombstonedRepository_Returns404, TestDistTagsHandler_TombstonedRepository_404, plus the route-level floor internal/format/npm/tombstoned_repository_integration_test.go. Maven: internal/format/maven/tombstoned_repository_integration_test.go. OCI: internal/format/oci/tombstoned_repository_integration_test.go, from step 3. Corrected since the table was written: the Maven and OCI gap is closed, and no gap remains.
AC-8	A create at a tombstoned repository's name returns 409 while the purge is outstanding, and 201 once the finalizer has run	TestDeleteHandlerIntegration_DestructiveTrue_TombstonesAndEnqueues. The 201 half issues the finalizer's own DELETE FROM repositories directly; that the purge job issues it is Step 14's.
AC-9	The finalizer cannot succeed early, verified through a gofail seam	Step 14's. Not in this MR.
AC-10	A purge reaching the finalizer with the subtree empty removes the row, its format child, and every collection link	Step 14's. Not in this MR.

Three clauses the plan's step adds beyond the spec's list:

#	Criterion	Tests that pin it
AC-P1	npm's three readers of RepositoryStore.FindByID agree on 404 for a repository tombstoned mid-request, pinned per handler	TestPackument_TombstonedRepository_Returns404 (a regression pin — this arm was already 404), TestDownloadHandler_TombstonedRepository_Returns404, TestDistTagsHandler_TombstonedRepository_404
AC-P2	A failed tombstone logs repository_id as its own structured field	TestDeleteHandler_TombstoneFailure_Returns500AndLogsRepositoryID
AC-P3	driver.sh smoke passes against the recipe as updated	Not a Go test. Corrected since the table was written: this has run. driver.sh smoke gives 48 passed, 0 failed against this branch's code, after a forced teardown and rebuild.

Error cases

Condition	Tests that pin it
DELETE omits destructive: 400 bad_request, nothing written	TestDeleteHandler_DestructiveParameter_RejectedValues/absent, TestDeleteHandler_RejectedParameter_IsNotAnExistenceOracle
destructive carries any other value: 400 bad_request	TestDeleteHandler_DestructiveParameter_RejectedValues, twelve further rows
destructive=false on a non-empty repository: 409 conflict, intact, message distinguishes the two cases	TestDeleteHandler_NonEmptyRepository_ConflictNamesTheRemedy, TestDeleteHandler_NonEmptyRepository_TheTwoRemediesAreDistinct, TestDeleteHandlerIntegration_DestructiveFalse_NonEmptyRepositoryRefuses
Artifact published between the DELETE arriving and its DELETE FROM repositories running	Covered by construction: every 409 and 202 arm branches off the guard's own outcome, datastore.ErrRepositoryNotEmpty at the unit layer and the real foreign key at the integration layer. No interleaving test. That was raised and dropped in triage: the arms branch off the guard's own outcome, so an interleaving test would pin the database's behaviour rather than this handler's.
Either value against a missing or already-tombstoned repository: 404 not_found	TestDeleteHandler_MissingRepository_BothValuesReturn404, TestDeleteHandler_TombstoneRaceDeletedRow_Returns404, and the second-delete assertions in TestDeleteHandlerIntegration_DestructiveTrue_TombstonesAndEnqueues
Create at a name held by a tombstoned repository: 409 conflict, freed by the finalizer	TestDeleteHandlerIntegration_DestructiveTrue_TombstonesAndEnqueues
Any client route naming a tombstoned repository: 404 in that route's own vocabulary	The three npm pins and the management-API assertions above, plus the route-level floors for npm, Maven and OCI (step 3). Corrected since the table was written: no gap remains.
Blob mount whose from= names a tombstoned repository: 202	S12's and S17 Phase 4's. Not in this MR.
Purge-job and sweep error rows	Steps 14, 15 and 16. Not in this MR.
Emit API unavailable at a purge or tombstone site	Step 19. Not in this MR.
Parent gate: parent tombstoned between the gate and the child INSERT	Step 4. Not in this MR.

Security considerations

#	Concern	Tests that pin it
S-1	Authorization is unchanged and is not weakened	Nothing here changes it. The route keeps the allow-all stub middleware every management endpoint carries. No assertion added.
S-2	destructive is a confirmation, not an authorization control	A claim about the OpenAPI description, not assertable in Go. Corrected since the table was written: the surface now exists. api/openapi/v1.yaml states that destructive carries the caller's intent and that the guard runs before the repository is resolved, so a rejected value says nothing about whether the name exists.
S-3	Deletion is irreversible in closed beta and the API must say so (#464 (closed)'s documentation criterion)	Same surface, not assertable in Go. Corrected since the table was written: the sentence has landed, in the operation description, the 202 response description, and the Bruno docs block. This closes #464 (closed)'s documentation acceptance criterion.
S-4	destructive is a strict boolean, rejected otherwise	TestDeleteHandler_DestructiveParameter_RejectedValues
S-5	No injection surface is added	The tombstone write is RepositoryStore.SoftDelete, jet-built and parameterized, pinned by Step 4's datastore suite. This step adds no SQL.
S-6	Error messages must not become an existence oracle	TestDeleteHandler_RejectedParameter_IsNotAnExistenceOracle compares the two exchanges body-for-body; TestDeleteHandler_DestructiveParameter_RejectedValues asserts no store call precedes the guard; TestDeleteHandler_MissingRepository_BothValuesReturn404 and the second-delete 404s cover the tombstoned case.
S-7	Purge failures are operationally visible rather than silent	The tombstone-count gauge, Step 17b. Not in this MR.

Coverage added after that table

The table above is the test author's, kept row for row. Four of its cells are marked Corrected since the table was written, because later work on this branch closed what they described as open. This subsection names the tests behind those corrections.

• AC-7 named a gap for Maven and OCI. internal/format/maven/tombstoned_repository_integration_test.go is new here and drives the Maven protocol routes against a tombstoned repository, in TestTombstonedRepositoryIntegration_MavenRoutesAnswerTheUnknownNameMiss and TestTombstonedRepositoryIntegration_MavenRepositoryTierNotNamespaceTier. OCI already has the same file at internal/format/oci/tombstoned_repository_integration_test.go, added by step 3 and unchanged here.
• npm gained the same route-level file, internal/format/npm/tombstoned_repository_integration_test.go, with the two matching test functions. The three per-handler pins the table names are still there.
• The management-API artifact routes are now asserted inside TestDeleteHandlerIntegration_DestructiveTrue_TombstonesAndEnqueues. The package list and package detail routes answer 200 before the tombstone and 404 after it, with the maven_packages row still present.
• S-2, S-3 and AC-P3 read as another phase's surface when the table was written. All three are on this branch now: the OpenAPI and Bruno wording is in the diff, and driver.sh smoke ran green at 48 passed, 0 failed.

e2e catalogs

docs/testing/ is updated in all four catalogs rather than the two the plan names: docker.md, maven.md, npm.md and oci.md. Each one carries an e2e.<catalog>.setup.delete-repository scenario, and this MR rewrites the expected-result cell of that row in each. Every row now states the required parameter and the 400 it enforces. Each row also states the 204 on an empty repository under either value, the 409 with its per-format remedy, and the 202. Each one records that the name stays reserved until the removal finishes. The remedy wording in each row matches the message the handler writes. The usage-data sections need no edit, because they state artifact_registry_repository_deleted on delete without a qualifier, and the 202 emits it.

No new e2e scenario id is added. The plan asks for one, and this MR widens the existing setup.delete-repository row instead. The reason is the automation contract in docs/testing/e2e/README.md: a scenario is implemented when the suite holds a test with that scenario's exact name, and CI diffs the names both ways. A separate id would therefore schedule a single harness test that has to drive the parameter matrix, a 204, a 409, a 202, an asynchronous wait and the post-condition assertions, against a seeded non-empty repository. The catalogs already fold several outcomes into one id in other places. Adding the id belongs with the harness work that can carry it, not with this MR.

Runtime evidence

Every line below was measured against the running service, after a forced teardown and rebuild of the binary, at the branch state that carried the destructive DELETE and its usage-data emission. Later commits on this branch changed the failure arms rather than the success arms this evidence exercises: the bounded-failure classification, the 503 on an unavailable job backend, the 499 on a client disconnect, and comment corrections. Those arms are covered by unit and integration tests rather than by this runtime pass, and the 202, 204, 409 and 400 behaviour it records is unchanged since.

• driver.sh smoke: 48 passed, 0 failed.
• All 16 exercises from the step's hand-back list, driven by hand, plus a post-202 route check per format.
• npm conformance: 17 steps, 51 assertions, 0 failures.
• Maven and OCI conformance: skipped. No production code under either format is in the diff. The one Maven file here is a test, and no container or OCI package changes at all.
• Usage data: observed at a collector, not inferred. Six requests produced exactly three events. The 202 produced one, and the 400, the 409 and the 404 produced none.

Reviewable LOC

Measured with git diff --numstat against this MR's merge base with its target branch, which is this MR's own diff. The figures below are re-measured after the comment-caps pass, because the target branch has been rebased during this MR's life and a pinned commit would not stay reachable. They are current as of merge base cd8daaabc, and they include the review round's own commits.

File group	Added	Deleted	Files
Test Go	2277	120	20
Production Go	432	421	18
Contract (api/openapi/v1.yaml, api/bruno/**)	185	8	2
Run recipe (.claude/skills/run-artifact-registry/SKILL.md)	58	0	1
docs/	5	5	5
Total	2957	554	46

Production Go, by package:

Package	Added	Deleted
internal/lifecycle	192	127
internal/managementapi	169	87
cmd/artifact-registry	33	0
internal/format	23	19
internal/datastore (comments only)	13	187
internal/usagedata (comment only)	2	1

2957 added lines is about six times the 500-line figure in docs/dev/development-model.md. It is also about three and a half times the plan's own forecast for this step, which is ~200 source and ~620 tests. The plan also sets a ~900 added-LOC working ceiling for every step, and this diff is about three and a quarter times that. So the plan's size paragraph alone does not answer the overage, and the split below is the answer.

A split does not help here, for four reasons.

• The plan asked this question for this step and answered it: "Step 18 (~820) carries the OpenAPI, Bruno, and e2e-catalog edits alongside the handler. Rejected on the repository's own guardrails, which require the contract and collection to move in the same MR as the operation."
• 2277 of the 2957 added lines are tests, which is 77% of the diff. Guardrail 6 keeps every test in the MR that adds the code it covers.
• 328 of the 432 production lines are internal/lifecycle/tombstone.go and internal/managementapi/delete.go. The handler calls the tombstoner, so a split leaves one half that no test can exercise.
• The rest of the production Go is small, and each part serves this one route. It is 33 lines of wiring, the lines that reconcile npm's three readers to 404, and 15 comment-only lines. Guardrail 17 requires those comments, because this MR is the caller they said did not exist yet.

Merge order

Closed. This MR targeted prozlach/s20a-lifecycle-closed-beta-step-16 while !1871 (merged), step 16, was open. !1871 (merged) merged on 2026-08-25, this MR was retargeted to main, and it is rebased onto main rather than onto any fixed commit.

The dependency was a compile dependency rather than a preference: internal/lifecycle/register.go and internal/lifecycle/sweep.go did not exist on main and this MR edits register.go, and RegisterPeriodicJobs is what puts the purge worker on the River client, so the job this MR enqueues had a worker only once !1871 (merged) landed. Both files are on main now, so nothing is outstanding.

Notes for the reviewer

destructive is a new required argument on an already-shipped route, which docs/dev/api-style.md lists under ## Breaking changes. The spec takes that break deliberately: the parameter is required so its removal at GA fails loudly for anyone who integrated during the beta, instead of silently converting a permanent delete into a soft one.

A repository of kind virtual answers a logged 500 on this route, under either value. The datastore refuses the delete on its kind gate, ahead of the 409 selector. Neither success arm is reachable for that kind. This predates the parameter and this MR does not change it. The divergence is documented at the branch point in internal/managementapi/delete.go and tracked at work item #819.

Two groups of hunks look unrelated and are not. internal/managementapi/container_list.go and list.go carry a behavior-preserving rename. containerBooleanParams becomes strictBooleanParams and moves beside the other query-parameter vocabularies, so the new guard and the container list read one map. The comment-only edits in internal/datastore, internal/lifecycle and docs/dev/background-jobs.md correct claims this MR falsifies. Most of them said that no production caller writes a tombstone yet, and this MR is that caller.

Changes from review

Two of these move behavior, so they are listed rather than left to the diff.

• A transaction the pool cannot open answers 503. tombstoneTx's BeginTx wrap carried no sentinel, so a pool that cannot produce a connection reached the handler's default arm and answered the logged 500. It now carries ErrTombstoneUnavailable, unconditionally, matching the merged bulk-delete enqueue seam in cmd/artifact-registry for the same call. An exhausted pool reaches the same 503: it blocks on the acquire until the bounded context fires, so the wrap carries the sentinel and the deadline together, and boundedFailure now passes a chain through untouched when it already names a condition the handler has an arm for. The tombstone's own expired budget is unchanged and still answers the logged 500, which work item 252 owns.
• The 202 arm emits the remote repository-write audit event. The 204 hard-delete arm emitted remote.RepositoryWriteDelete for a remote-kind row and the 202 emitted nothing, so a destructive delete of a remote repository left no actor-attributed record. S17-rest-management-api.md:883 and its acceptance criterion at :1346 require one on delete of a remote repository without qualifying the arm, and a remote repository has no empty-first route, so the 202 is its only delete. Work item #697 stays open for the purge's own emission, which carries no actor.

The rest are contract text and comments. The deleteRepository description gains the virtual kind's 500, the publish-after-tombstone window and its work item #901, and the rule that selects which 409 remedy the message names; the Bruno docs block mirrors each. Four Go comment blocks that stated something the code does not do were corrected, and boundedFailure's doc now names the caller it actually has.

One window is accepted rather than closed. A publish that resolved the repository before the tombstone can commit after it and have its rows purged, with no error reaching the writer. It is documented on both contract surfaces, tracked at #901, and recorded in the spec by !1953.

Related to #611

This is a bot message 🤖 — /smurfit