chore(datastore): npm_packages and container_images tombstone scans (S20-A plan: 7/21)

What this step delivers

This step adds the two tombstone scans that reach states no version scan and no repository walk can find. The first one finds an npm_packages row that outlived every one of its versions. The second one finds a container_images row that carries a tombstone under a repository that still stands.

Both arrive as read-only methods on the LifecycleScanStore type that step 5 added. They are ScanTombstonedNpmPackages and ScanTombstonedContainerImages. Each one comes with an unexported statement builder and one query-name catalog entry.

No caller of any kind lands with them. The diff adds no handler, route, worker, periodic job, composition-root wiring, migration, configuration key, OpenAPI operation, or Bruno request. The only callers are this step's own tests. The first production caller is the purge worker in step 15. The type of this merge request is chore for that reason, and no Related to line is owed.

Spec coverage

Spec: docs/specs/S20-a-lifecycle-closed-beta.md

Plan step: 7 (npm_packages and container_images tombstone scans)

Acceptance criteria

Purger and discovery:

#	Criterion	Tests
P-1	Tombstone older than the window discovered, newer one not; verified non-zero and at zero	TestLifecycleScanStore_PackageLevelTombstoneScans subtests a tombstone older than a non-zero window is returned and a newer one is not and a zero window makes a just-written tombstone immediately eligible, both run per scan. Repository level is Step 5's TestLifecycleScanStore_ScanTombstonedRepositories
P-2	A soft_deleted_at IS NULL row is never discovered, for every table in the reap inventory	TestLifecycleScanStore_PackageLevelTombstoneScans subtest a row with soft_deleted_at IS NULL is never returned, per scan. Covers npm_packages and container_images; the remaining tables belong to Steps 6, 8-13
P-3	repositories, npm_packages and container_images scans use their discovery index and prune to one partition, asserted by EXPLAIN for each of the three	TestLifecycleScanStore_PackageLevelScans_AreIndexBackedOldestFirst, one case per table, with TestLifecycleScanStore_PackageLevelScansStmt_OrderAndBinding saying the statement rather than the planner is where the ordering comes from. repositories is Step 5's TestLifecycleScanStore_ScanTombstonedRepositories_IsIndexBackedOldestFirst
P-4	The five version-level scans use their indexes and prune, asserted per table	Not this step - Step 6
P-5	Re-running a purge on a reaped subtree is a no-op; a mid-walk abort leaves strictly fewer rows	Not this step - Steps 14 and 15
P-6	The npm repository walk reaps live and already-tombstoned rows in one pass	Not this step - Step 14
P-7	Every purge transaction is bounded	Not this step - Step 15
P-8	A purge job whose namespace stopped serving writes refuses retryably	Not this step - Step 15a

Per-format reap logic:

#	Criterion	Tests
F-1	npm version reaped with its files and their attachments	Not this step - Step 8
F-2	Unpublish-then-republish before the reap leaves no orphan	Not this step - Step 8
F-3	npm package reaped with tags, metadata files and their attachments	Not this step - Step 8
F-4	DeleteNpmMetadataFiles removes each attachment in the same transaction	Not this step - Step 8
F-5	Maven version and package reaped with files and attachments	Not this step - Step 9
F-6	Container repository reaped through the existing per-artifact deleters	Not this step - Step 13
F-7	Index and its children reaped in one purge, parent-first	Not this step - Step 13
F-8	Tombstoned container_images row under a live repository reaped through the walk	Discovery half only: TestLifecycleScanStore_PackageLevelTombstoneScans subtest the row is discovered while its parent repository stands. The reap half is Step 13
F-9	Container repository walk reaps live and already-tombstoned images in one pass	Not this step - Step 13
F-10	Remote-cache subtree reaped for every remote table carrying the column	Not this step - Steps 10, 11, 12
F-11	container_remote_manifests and container_remote_blobs reaped the same way	Not this step - Step 12
F-12	After a reap every referenced blob has zero attachment rows	Not this step - Steps 8-13

Repository entry point:

#	Criterion	Tests
R-1	DELETE without destructive, or with a non-boolean value, returns 400	Not this step - Step 18
R-2	destructive=false on an empty repository returns 204 and frees the name	Not this step - Step 18
R-3	destructive=false on a non-empty repository returns 409	Not this step - Step 18
R-4	The 409 message names the remedy the caller has	Not this step - Step 18
R-5	destructive=true on a non-empty repository returns 202 and enqueues in the tombstone's transaction	Not this step - Step 18
R-6	destructive=true on an empty repository returns 204 with no tombstone and no job	Not this step - Step 18
R-7	Every client route naming the repository answers 404 after the 202	Not this step - Steps 3 and 18
R-8	Create at a tombstoned name returns 409 until the finalizer runs	Not this step - Step 18
R-9	The finalizer cannot succeed early	Not this step - Step 14
R-10	A finalizer reaching an empty subtree removes the row, its child, and its links	Not this step - Step 14

Sweep:

#	Criterion	Tests
S-1	A tombstoned repository with no live purge job is re-queued within one interval	Not this step - Step 16
S-2	The sweep reads each namespace oldest-first, with no Sort node above the scan	Ordering: TestLifecycleScanStore_PackageLevelTombstoneScans subtests rows come back oldest tombstone first and the page is bounded by the limit and keeps the oldest, per scan. Statement: TestLifecycleScanStore_PackageLevelScansStmt_OrderAndBinding. Plan: TestLifecycleScanStore_PackageLevelScans_AreIndexBackedOldestFirst. The sweep's own tick is Step 16
S-3	A tombstoned npm_packages row whose versions are all reaped is enumerated on the added index and re-queued	Enumeration half: TestLifecycleScanStore_ScanTombstonedNpmPackages_ReachesAPackageWhoseVersionsAreAllReaped, seeding the state directly. The re-queue is Step 16 and the npm_metadata_files reap is Step 8
S-4	A repository with a live purge job is not re-queued	Not this step - Step 16
S-5	A repository whose prior job reached a terminal state is re-queued	Not this step - Step 16
S-6	A permanently failing purge stays counted and re-queued at the bounded cadence	Not this step - Steps 16 and 17b
S-7	The sweep registers a worker for its periodic kind	Not this step - Step 16
S-8	A namespace that does not serve writes has nothing enqueued, and is enumerated again once the condition clears	Not this step - Step 16. The scans stay predicate-only: no lifecycle column joins their WHERE clause

Tombstone visibility:

#	Criterion	Tests
V-1	ContainerRepositoryStore.FindByNameInNamespace returns ErrNotFound, for docker and oci	Not this step - Step 3
V-2	RepositoryStore.FindByID returns ErrNotFound	Not this step - Step 3
V-3	RepositoryStore.Update returns ErrNotFound	Not this step - Step 3
V-4	The five unfiltered repository methods still act on a tombstoned row	Not this step - Step 3
V-5	parentRepositoryIsActive refuses a concurrently tombstoned parent	Not this step - Step 4
V-6	The four container_images gates hide a tombstoned image	Not this step - S17 Phase 4
V-7	Every /v2/ route resolving through FindByName answers 404 NAME_UNKNOWN	Not this step - S17 Phase 4
V-8	The blob mount route answers 202, not 404, when every source image is tombstoned	Not this step - S17 Phase 4
V-9	A tombstoned image named in from= still mounts and answers 201	Not this step - S17 Phase 4
V-10	A mount into a tombstoned destination name answers 201	Not this step - S17 Phase 4
V-11	UpsertContainerImage does not resurrect a tombstoned image	Not this step - S17 Phase 4
V-12	ContainerImageStore.Delete still acts on a tombstoned row	Not this step - S17 Phase 4

Accounting call sites:

#	Criterion	Tests
A-1	The destructive DELETE emits no delta at the tombstone write	Not this step - Step 19
A-2	Decrements fire at the purger, with the delta set fixed by the level	Not this step - Step 19
A-3	Every emit site commits its source rows before issuing the delta	Not this step - Step 19
A-4	A purge chunk emits from its committed affected-row aggregate	Not this step - Step 19
A-5	A raw foreign-key cascade emits nothing and is not used	Not this step - Steps 9 and 13

Schema and configuration:

#	Criterion	Tests
C-1	The repositories and npm_packages index migrations apply and roll back cleanly	Not this step - Step 1. This step asserts the npm_packages index is reached, through childIndexNames and the EXPLAIN plan, rather than asserting its DDL
C-2	No migration adds or drops a column, and no CHECK pins soft_deleted_at to NULL	Not this step - Step 1. This step adds no migration
C-3	unique_container_images_ns_id_cr_id_name is partial and no non-partial form survives	Owned by S17 Phase 4's marker migration and its own tests. This step's fixtures neither exercise nor pin the partial form. They seed distinct names, because a live row holds the name and a tombstoned one does not
C-4	Config load accepts retention_window: 0s and rejects the invalid values	Not this step - Step 2

Error cases

#	Condition	Tests
E-1	DELETE omits destructive	Not this step - Step 18
E-2	destructive carries a non-boolean value	Not this step - Step 18
E-3	destructive=false on a non-empty repository	Not this step - Step 18
E-4	An artifact is published between the DELETE and its DELETE FROM repositories	Not this step - Steps 14 and 18
E-5	DELETE on a missing or already-tombstoned repository	Not this step - Steps 3 and 18
E-6	Create at a name held by a tombstoned repository	Not this step - Step 18
E-7	Any client route naming a tombstoned repository	Not this step - Step 3
E-8	A blob mount whose from= names a tombstoned repository	Not this step - S17 Phase 4
E-9	Purge job: 23503 on a parent delete	Not this step - Steps 14 and 15
E-10	Purge job: ContainerManifestDeleter refuses a still-referenced manifest	Not this step - Step 13
E-11	Purge job: the finalizer aborts because an artifact remains	Not this step - Step 14
E-12	Purge job: the repository is already hard-deleted when the job runs	Not this step - Step 15. The scan's own empty answer is TestLifecycleScanStore_PackageLevelTombstoneScans subtest a namespace holding no tombstone yields an empty page and no error
E-13	Purge job: the retention window changed between enqueue and run	Predicate half: TestLifecycleScanStore_PackageLevelTombstoneScans subtest a tombstone older than a non-zero window is returned and a newer one is not shows a lengthened window makes a row ineligible at scan time. The job's reporting is Step 15
E-14	Sweep: the enumeration query fails	TestLifecycleScanStore_PackageLevelTombstoneScans subtest query failure wraps the cause and names no namespace, per scan: the cause stays unwrappable so River can retry. The tick's retry is Step 16
E-15	Sweep: the interval elapses before enumeration completes	Ordering half: the oldest-first and bounded-page subtests above, per scan. The tick is Step 16
E-16	Emit API unavailable at a purge or tombstone site	Not this step - Step 19
E-17	Parent gate: the parent is tombstoned between the gate and the child INSERT	Not this step - Step 4
E-18	A tombstoned coordinate is reoccupied by a republished row before the reap	Not this step - Step 8. The scan side is covered by the IS NULL subtest, which pins that a live row at a scanned table is never returned

Security considerations

#	Concern	Tests
SEC-1	Authorization unchanged; the purger runs with no caller identity	No test here. These scans take a namespace id and a window and make no authorization decision; the surface is Step 18's
SEC-2	destructive is a confirmation, not an authorization control	Not this step - Step 18
SEC-3	Deletion is irreversible in closed beta and the API says so	Not this step - Step 18, plus the OpenAPI text
SEC-4	Input validation; no purger input is user-controlled	TestLifecycleScanStore_PackageLevelTombstoneScans_Guards, per scan: every argument is a UUID, a duration or an int, and each out-of-range value is refused before a statement is built. No identifier, name or digest reaches these queries as text
SEC-5	No injection surface is added	TestLifecycleScanStore_PackageLevelScansStmt_OrderAndBinding, per scan: the namespace and the page size arrive as bound arguments, and a count of one would mean the other was interpolated. The window is the deliberate exception, rendered as an interval literal by tombstoneEligibleExpr. TestLifecycleScanStore_PackageLevelScans_AreIndexBackedOldestFirst executes the store's own statement rather than a copy
SEC-6	Error messages must not become an existence oracle	TestLifecycleScanStore_PackageLevelTombstoneScans subtest query failure wraps the cause and names no namespace. The 404-versus-409 half is Steps 3 and 18
SEC-7	Purge failures stay operationally visible	Not this step - Steps 16, 17a and 17b. The scan's contribution is that a row it cannot drain stays at the head of the index, which the oldest-first subtest pins

e2e scenario catalogs

This run read all five files under docs/testing/e2e/ for their expectations, not for keywords. No scenario needs an addition or an amendment, and this merge request changes none of them. The step adds no caller, so nothing a client can observe changes.

• oci.md:115 and docker.md:117 assert a state "before any reap has run", and this step runs no reap.
• npm.md:127 and npm.md:129 depend on soft-delete semantics and on versions_count. This step hard-deletes nothing and moves no counter.
• maven.md:135 names the purger, but that row is marked blocked and it is Maven-scoped. This step adds no Maven scan.

Diff size, and why a split does not help

The diff is 6 files, 1002 insertions, 0 deletions, measured against origin/main. That is past the 500 reviewable-line guardrail, so the split by file group is below.

Group	Files	Lines
Store implementation	internal/datastore/lifecycle_scan_packages.go	173
Integration tests	internal/datastore/lifecycle_scan_packages_integration_test.go, internal/datastore/lifecycle_explain_integration_test.go	606
Default-build tests	internal/datastore/lifecycle_scan_packages_test.go	219
Query-name catalog and lint directive	internal/datastore/query_names.go, internal/datastore/lifecycle_scan_test.go	4

The store implementation is 173 lines of the 1002. The other 829 lines are the tests plus the two query-name constants.

A split by scan does not help. The two scans share one test table that runs every case once per scan. A split by scan copies that harness into both merge requests, or it drops the positive hit for one of the two database tables. A split that separates the tests from the implementation reverses the test-first order this step was written in.

Three deviations, each deliberate

1. Row 7 of the plan's Status table stays empty

This merge request does not touch the plan file. One separate merge request fills the rows of all six children of this plan, once all six numbers exist. Six merge requests that edit adjacent lines of one table conflict with each other. The empty cell is that decision, not a missed guardrail.

2. Three files sit outside the plan's Files: list for step 7

File	Lines	Why it is here
internal/datastore/query_names.go	+2	TestQueryNames_EachUsedExactlyOnce needs one catalog entry per instrumented statement. Step 5 added its own entry to this same file, also with no plan entry.
internal/datastore/lifecycle_scan_packages_test.go	+219	Two tests reach no database, so they belong in the default build, where the CI lint job compiles them. Step 5's merged counterparts already have that shape.
internal/datastore/lifecycle_scan_test.go	+2	The waiver below.

3. The lifecycle_scan_test.go edit is an authorized waiver

The edit is two additive lines, directly above TestLifecycleScanStore_ScanTombstonedRepositories_Guards. One line is a // separator that gofmt requires above a doc-comment directive. The other line is the directive itself:

//nolint:dupl // mirrors TestLifecycleScanStore_PackageLevelTombstoneScans_Guards' table by design (inlined per the datastore test convention).

The guard table of this step mirrors the merged table in lifecycle_scan_test.go by design. dupl reports the clone as two findings, one at each position, and golangci-lint filters each finding by its own position. A directive on the half that this branch owns therefore suppresses only that half. Without the pair, the untagged whole-package pass moves from 0 issues to 1. The CI golangci_lint job runs a full pass with no --new-from flag, so the pipeline of this merge request fails on that one issue.

A standing instruction of this run keeps a sibling step's files out of this change set. The author waived that instruction for this one comment line, after a decision block that set out seven options with their costs. The repository gives the same answer: nine merged commits reach into a pre-existing sibling file for a counterpart directive and change nothing else there.

The token was measured, not reasoned about. With the directive removed, the untagged whole-package pass reports the clone at lifecycle_scan_test.go:217 and exits 1. With the directive restored, the same pass exits 0, and the file is byte-identical to its state before the measurement.

Verification

GOFLAGS in this worktree carries -tags=integration,development_stubs. A pass with GOFLAGS cleared carries no tags, and that is the pass CI runs. An explicit -tags= overrides GOFLAGS rather than merges with it.

All four lint passes ran at the rebased branch tip, with a private lint cache, --allow-parallel-runners, and golangci-lint 2.12.2. The run read every exit code directly.

Pass	Scope	Findings
No tags at all (the CI-equivalent pass)	whole package	0
No tags at all	new code only	0
integration only	new code only	0
integration only	whole package	3979, all pre-existing, and 0 of them name a file this branch touches

Tests on the rebased tip: the tagged suites report ok 6.9s for the tombstone scans and the orphan case, and ok 18.7s for the EXPLAIN suite. The untagged pair of database-free tests reports ok. Both tagged figures are package wall time rather than suite time. TestMain applies every migration to an isolated database before the first test runs, and the rebase adds migrations to that set. The EXPLAIN suite's own reported duration is 0.4s, unchanged from before the rebase. gofmt -l internal/datastore/ prints nothing.

Paired documentation

The paired-documentation guardrails do not fire. The diff touches no path under proto/artifactregistry/config/, internal/config/, api/openapi/, or api/bruno/, and it does not touch config.example.yaml. The step ships no developer-facing surface, so no docs/dev/ page is owed. docs/dev/database-query-patterns.md already documents the patterns that the two scans follow.

Base and merge order

This merge request targets main, and every figure above is measured against origin/main. The branch started as a stack on step 5's branch. Step 5 merged as !1651 (merged) while this work ran, so the branch moved onto origin/main with git rebase --onto. Nothing of step 5 remains in this diff: the diff after the move was byte-for-byte the diff before it. No finding is deferred to another merge request, and this step widens no predicate ahead of the code that serves it.

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.ScanTombstonedContainerImages	Limit	container_images_p16_namespace_id_soft_deleted_at_idx	25 / 25	2.34	0.021ms	4 / 0	1/64
datastore.ScanTombstonedNpmPackages	Limit	npm_packages_p39_namespace_id_soft_deleted_at_idx	25 / 25	2.39	0.020ms	3 / 0	1/64

datastore.ScanTombstonedContainerImages

Summary: The plan matches the method's intent. The namespace equality prunes to one partition of 64, and the partial index index_container_images_on_ns_id_soft_deleted_at answers both predicates from the index condition. PostgreSQL walks that index backwards, so the oldest tombstone comes first with no sort node and no post-scan filter. Every one of the 5000 seeded rows carries a tombstone, so the partial index holds all of them. The store rejects a page size above MaxLifecycleScanPageSize before the statement runs, so the result set cannot grow with the table. No anomalies.

Seed shape: namespaces=1, repositories=1, container_repositories=1, container_images=5000

Rendered SQL:

SELECT container_images.namespace_id AS "tombstone_row.namespace_id",
     container_images.id AS "tombstone_row.id",
     container_images.soft_deleted_at AS "tombstone_row.soft_deleted_at"
FROM public.container_images
WHERE (container_images.namespace_id = $1::uuid) AND (container_images.soft_deleted_at < (NOW() - INTERVAL '0 MICROSECOND'))
ORDER BY container_images.soft_deleted_at ASC
LIMIT $2;

Bound args: [7526a252-c3f5-43ca-b655-c5ef2bf45e94, 25]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=0.29..2.34 rows=25 width=40) (actual time=0.017..0.021 rows=25 loops=1)
   Buffers: shared hit=4
   ->  Index Scan Backward using container_images_p16_namespace_id_soft_deleted_at_idx on container_images_p16 container_images  (cost=0.29..411.16 rows=5000 width=40) (actual time=0.016..0.019 rows=25 loops=1)
         Index Cond: ((namespace_id = '7526a252-c3f5-43ca-b655-c5ef2bf45e94'::uuid) AND (soft_deleted_at < (now() - '00:00:00'::interval)))
         Buffers: shared hit=4
 Planning:
   Buffers: shared hit=343 read=1
 Planning Time: 1.442 ms
 Execution Time: 0.034 ms

Timings: planning 1.442ms, execution 0.034ms, total 1.476ms.

datastore.ScanTombstonedNpmPackages

Summary: The plan matches the method's intent. The namespace equality prunes to one partition of 64, and the partial index index_npm_packages_on_ns_id_soft_deleted_at answers both predicates from the index condition. PostgreSQL walks that index backwards, so the oldest tombstone comes first with no sort node and no post-scan filter. Every one of the 5000 seeded rows carries a tombstone, so the partial index holds all of them. The store rejects a page size above MaxLifecycleScanPageSize before the statement runs, so the result set cannot grow with the table. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=5000

Rendered SQL:

SELECT npm_packages.namespace_id AS "tombstone_row.namespace_id",
     npm_packages.id AS "tombstone_row.id",
     npm_packages.soft_deleted_at AS "tombstone_row.soft_deleted_at"
FROM public.npm_packages
WHERE (npm_packages.namespace_id = $1::uuid) AND (npm_packages.soft_deleted_at < (NOW() - INTERVAL '0 MICROSECOND'))
ORDER BY npm_packages.soft_deleted_at ASC
LIMIT $2;

Bound args: [c7e9737a-cf08-417a-be78-497a4f71d7fb, 25]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

 Limit  (cost=0.29..2.39 rows=25 width=40) (actual time=0.017..0.020 rows=25 loops=1)
   Buffers: shared hit=3
   ->  Index Scan Backward using npm_packages_p39_namespace_id_soft_deleted_at_idx on npm_packages_p39 npm_packages  (cost=0.29..420.41 rows=5000 width=40) (actual time=0.016..0.018 rows=25 loops=1)
         Index Cond: ((namespace_id = 'c7e9737a-cf08-417a-be78-497a4f71d7fb'::uuid) AND (soft_deleted_at < (now() - '00:00:00'::interval)))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=342 read=2
 Planning Time: 1.774 ms
 Execution Time: 0.033 ms

Timings: planning 1.774ms, execution 0.033ms, total 1.807ms.

The window renders as a literal interval rather than a placeholder, so both plans were taken at window = 0, which makes every tombstoned row eligible. The page size of 25 is the value the step's own EXPLAIN tests bind. enable_seqscan stayed on for both runs, so the index path won on cost against 5000 rows in the scanned partition. No migration ran on this branch, so migration mode collected nothing.

This is a bot message 🤖 — /smurfit