test(npm): npm_virtual_repositories absences and the rows they admit (S31 plan: 1/19)

Stacked MRs

Part	MR	Scope	Reviewable LOC	Target
1/3	!1979 (merged) (merged)	the RepositoryStore.Delete foreign-key comments, and the interrupted-down recovery rule	72	main
2/3	!1971 (merged) (merged)	the npm_virtual_repositories migration and the shape it declares	1078	main
3/3	this MR	the absences the table carries, and the rows they admit	757	main

Both earlier parts have merged, so no ordering constraint is outstanding. Part 1 removed the RepositoryStore.Delete foreign-key enumeration that part 2 would otherwise have made wrong, and part 2 created the table and the shared declarations this part's file uses. This MR has been rebased onto main on top of both and targets main directly; the target-branch chain the earlier revision of this table described no longer exists.

Parts 2 and 3 are both row 1 of the S31 plan Status table, so both carry the 1/19 marker. One row shipping as several merge requests is this plan's own recorded precedent, and row 6 carries !1976, !1977 the same way. Part 1 is not a plan step: it is review fallout from part 2's branch that turned out not to depend on the new table.

What this changes

npm_virtual_repositories carries no CHECK, no trigger and no column beyond its three. What it accepts is therefore decided entirely by its two foreign keys, and the migration comment's catalogue of what the table deliberately lacks is the document that says so. This MR pins both halves: that the absences are real, and which parent rows they consequently let in.

It also corrects repositoryReapWalk's comment, which is the production-side consequence of the same fact.

Part 2 asserts the shape the migration positively declares (partitioning, columns, the unique index, identifier lengths, the Down section). The split is along that line: what the table is there, what it therefore admits here.

The absences

Six cases, one per thing the migration comment says the table does not have:

• _NoSoftDeleteColumn, _NoTimestampColumns and _NoParentOwnedColumns assert the columns are absent. Each requires some column to exist first, so the check is not vacuous against a missing table. _NoParentOwnedColumns covers eleven names, including the four cache-validity and health columns npm_remote_repositories carries, which previously had no attributable negative and were pinned only by the schema file's exact column count.
• _NoNonUniqueIndexes holds the "one index is the whole access path and the whole FK cover" claim.
• _NoCheckConstraints holds the absent CHECK, which is what makes the row-shape half below the complete statement of what the table accepts.
• _NoTriggers covers the half _NoCheckConstraints cannot reach: a BEFORE INSERT trigger narrowing any admitted shape would pass every catalog test and every insert test that only ever inserts valid rows.

Both of the last two read the parent and all 64 partitions, through listPartitionsOf, so a CHECK or a trigger created directly on one partition cannot hide behind a parent-only query. Both take the partition count from pg_inherits and assert it is exactly 64 before the zero means anything, rather than counting name matches in the partitions schema: that pattern also matches the 128 per-partition index clones, so the old lower bound of 65 was satisfied by 193 relations and would have passed with half the partitions missing.

The rows those absences admit

Every documented row shape has a positive hit. Five of the six had none.

• _AcceptsParentAtEveryFormat and _AcceptsParentAtEveryKind cover every value check_repositories_format and check_repositories_kind admit, each on its own parent so the unique index cannot mask a result. The container sibling pins its two parent formats the same way, in TestContainerRemoteRepositoriesSchema_AcceptsEitherContainerParentFormat.
• _AcceptsParentAtEveryVisibility covers all three slots.
• _AcceptsSoftDeletedParent, _AcceptsNamespaceOutOfService (a subtest per lifecycle stamp, each on its own namespace) and _AcceptsSiblingPerFormatChild cover the rest. The namespace one is the shape the migration comment was missing: namespaces carries deleted_at, purged_at, blocked_at, disabled_at and suspended_at, and fk_nvr_namespace_id_namespaces checks only that the row exists.

_AcceptsSiblingPerFormatChild has two subtests, and the hosted one is the primary. The migration's documented shape is a parent already carrying a sibling per-format child, "which is every parent the create path builds", and that child is an npm_repositories row: createTx calls insertFormatChild before it branches on kind. An npm_remote_repositories row on a kind=1 parent, which is what this case used to seed on its own, is a pairing createTx can never build, because remoteChildCreates runs only under Kind == RepositoryKindRemote. It stays as a second subtest for the remote-parent reading.

Four foreign-key contracts, one test each

The rejection and cascade cases stay in the schema file where they merged, and the near-duplicates of them are gone from the new file. _ForeignKeyDeleteActions is kept here instead of the schema file's _ForeignKeyNamesAndActions, because it asserts everything that one did plus an exactly-two foreign-key count, so the weaker one is removed. Across the two files the 26 cases now carry 26 distinct names and no duplicated contract.

repositoryReapWalk owned no rows, and now owns some

Its comment said a virtual repository owns no rows of its own. Read as a count that is wrong three ways: every repository owns a per-format child row and a collection link, npm_virtual_repositories is the only virtual table in the schema so three of the four virtual pairs own no such row at all, and the npm pair will own one once a writer for it lands. The walk is empty because all eight inbound foreign keys to repositories cascade, so no row it owns needs a reap level. The code is unchanged; only the recorded reason.

Two further claims in the same block were wrong as written, and are fixed with it:

• "No FK here sets ON DELETE" read unscoped over a file whose own delete path rests on those eight cascading keys, and is now scoped to the artifact level it was always about.
• "One chunk drains the first non-empty level" promised what ReapTotals.Drained means. A chunk works one page of one level, bounded by its budget, which reapRepositoryLevelPage's own doc states.

The same sentence stood in the walk-totality test. Editing it pulled a 17-line doc comment into the diff, which the caps rule then holds to its 2-line cap, so that reasoning moves to internal/datastore/lifecycle_reap_repository.md under "Why a zero walk is a decided answer".

What is deliberately left

Three passages in reconcile_repository.go (lines 270, 387 and 606) say a virtual repository holds no rows of its own. Each is scoped in its own paragraph to counted artifact rows or summed blob bytes, and a virtual repository genuinely owns none of those, so each is true where it stands. Correcting the wording would touch three doc comments of 15 to 25 lines, which the caps rule would then hold to 3, 3 and 1 lines, so the reword costs a rewrite of roughly 50 lines of accurate documentation in a file this step does not otherwise touch. The one occurrence that was cheap to scope, a fixture reason string in reconcile_repository_integration_test.go, now says "no counted rows of its own". The pointer from the corrected site to that map survives at lifecycle_reap_repository.go:217.

Tests

This MR is the tests. Run locally against PostgreSQL 16: the 15 cases in the new file and the 11 in the schema file all pass, 26 in total with no skips and no failures, over 52 test and subtest runs. Also gofmt, go vet -tags=integration ./internal/datastore/..., bash scripts/ci/check-comment-caps.sh --base origin/main (the --base form; a bare run inspects only the worktree diff and prints OK on a clean tree), and

golangci-lint run --build-tags=integration --max-same-issues=0 \
  --max-issues-per-linter=0 --uniq-by-line=false ./internal/datastore/...

which reports nothing in the two npm_virtual_repositories test files or in lifecycle_reap_repository.go, and no unused finding, the risk a split of one file across two carries. It does report five contextcheck findings in reconcile_repository_integration_test.go, which this MR also touches: those sit in fixture helpers at lines 636 to 722, predate this branch, and are not on the one string literal changed here at line 596. Fixing the two errcheck findings the schema file carried took the package from 107 to 105. Everything else the command reports under that tag is pre-existing and is not compiled by CI's lint job, which sets no build tags.

Because both earlier parts merged after this branch was last built, the pipeline on the previous revision never compiled this file alongside the schema file as main now carries it. The pipeline on this revision is the first that does.

Size

757 reviewable lines: 709 test and 48 source and docs, the repositoryReapWalk comment plus its sidecar section.

This is over the 500 LOC ceiling docs/dev/development-model.md sets, and a further split would not help. The absences and the rows they admit are one argument, not two: _NoCheckConstraints and _NoTriggers are what make the row-shape cases a complete statement of what the table accepts rather than a sample, and splitting them apart would land each half asserting something the other half is the evidence for. The 48 source lines have to travel with the row-shape cases for the same reason, since the reap comment's old claim is the production-side reading of exactly the fact those cases pin. Of the 709 test lines, 624 are one new file and the remaining 85 are the consolidation that removes a duplicated test from the schema file.

Plan drift

Step 1 of the plan names one test file and this step shipped two, and its Tests list enumerates nine items, none of them the absence cases or the six Accepts* cases. Step 1 budgets Test: ~620; the two files together are 1,054 lines, about 1.7x. The plan is not edited here, because a step MR does not write to it. The amendment goes on a docs(plans) branch, together with row 1's MR references.

E2E scenarios

No scenario in docs/testing/ is added or affected, and the out-of-scope line for virtual npm repositories in docs/testing/e2e/npm.md, which names S31 as their owner, is still correct. The step this completes adds a table with no reader, no writer, no route and no HTTP surface, so there is no end to end flow to script. The plan gives the S31 catalog section a single owning step, step 19.

Closes #882 (closed)