docs(datastore): repositories Delete FK comments and down-migration recovery

Stacked MRs

Part MR Scope Reviewable LOC Target
1/3 this MR the RepositoryStore.Delete foreign-key comments, and the interrupted-down recovery rule 211 main
2/3 !1971 (merged) the npm_virtual_repositories migration and the shape it declares 879 dmeshcharakou/s31-repository-delete-comment-fixes
3/3 !1988 (merged) the absences the table carries, and the rows they admit 686 dmeshcharakou/s31-npm-virtual-step-1

Merge order is 1, 2, 3. The two links are not equally strong, so both are stated plainly.

Part 2 before part 3 is a build dependency. Part 3's cases query the table part 2 creates, and its file uses the table name, the identifier-name constants and the seed helpers that part 2's file declares. It neither compiles nor passes without part 2.

Part 1 before part 2 is a correctness one, not a build one. Part 2 compiles and passes against main on its own. What part 1 buys is that part 2 does not leave RepositoryStore.Delete's comment saying seven inbound foreign keys to repositories when there are eight: part 1 deletes that enumeration, and part 2 adds the eighth and updates wantCascadeTargets in the schema guard. Merged the other way round, part 2 ships a comment the same change made wrong.

Nothing in GitLab enforces either link. No merge-request dependency is set; the order is carried by the target-branch chain, and by this table.

This branch was force-pushed on 2026-08-27 to correct the two commits below, so parts 2 and 3 need rebasing onto the new head before their diffs read correctly. Until they are, !1971 (merged) shows this MR's superseded commits inside its own diff. A third commit landed on top of that head the same day, a fast-forward carrying three review corrections, so the rebase target is 038197632 rather than the force-pushed head.

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. 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

Two comment and documentation corrections that a review of the S31 step 1 branch turned up. Neither depends on the npm_virtual_repositories table that step adds, so they come out from under it and land first.

No behavior changes. The diff is comments, one new sidecar document, and one Markdown section.

RepositoryStore.Delete stops enumerating the foreign keys

Delete's doc comment listed every inbound foreign key to repositories and stated a count. TestRepositoryStore_DeleteFKAssumption_SchemaGuard asserts that same set by reading pg_constraint, so the two were copies of one fact, and the prose was the copy that nothing fails on. That is exactly how it went wrong: the branch that adds an eighth child table updated the guard and left the comment saying seven.

The enumeration and the count are gone from both the method and the guard's own doc comment. What is not derivable from the guard stays, as a short doc line plus two body comments next to the statements they describe: the error vocabulary Delete returns, and why the kind read deliberately shares no transaction with the DELETE.

RepositoryReaper's doc comment in internal/datastore/lifecycle_reap_repository.go cited Delete's doc as the place that names the two blocking foreign-key families table by table, which this change makes untrue, so it cites the guard instead. Touching that block puts its whole 37 lines against the 3-line cap, so the reasoning it carried moves to internal/datastore/lifecycle_reap_repository.md beside it, following packument_cache.md in internal/format/npm. That is the first sidecar in internal/datastore; say the word if you would rather it went somewhere else.

Two prose copies of the topology are deliberately left standing, in mapRepositoryDeleteError and mapRepositoryFinalizeError. Neither states a count, and both stay true when part 2 adds the eighth child: fk_nvr_repository_id_repositories is ON DELETE CASCADE and nothing blocking references the new table. They are still copies of the guard's fact, so they are worth a follow-up rather than this MR.

Both Delete blocks were over the caps scripts/ci/check-comment-caps.sh enforces. The rule is that an oversized block stands until an edit touches it and then has to shrink, and any edit here touches them, so they are cut to 3 and 2 lines rather than trimmed by a word. This is a bigger cut than the correction that prompted it. Say the word if you would rather keep more of either block and take the caps waiver instead.

docs/dev/database-migrations.md gains the interrupted-down recovery rule

The create-table migrations that build a partitioned parent plus its 64 partitions run every statement under -- +goose NO TRANSACTION. goose deletes the goose_db_version row only after the whole down section succeeds, and there is nothing to roll back, so a down migration killed partway leaves the migration recorded as applied with only part of its work done.

Running up in that state is the trap whatever the file looks like. goose sees the version row, skips the file, and rebuilds nothing, and nothing reports the gap until a read or write hits the missing table or the dropped partition. On a table nothing writes to yet, that first statement can be a whole sprint after the rollback that caused it.

What clears the version row depends on the shape of the down section, and the family has three shapes rather than one:

Down section Files Re-running down
One DROP TABLE IF EXISTS <parent> 7 works
A DROP TABLE IF EXISTS per partition, then the parent 10 works
An ALTER TABLE IF EXISTS <parent> DETACH PARTITION before each DROP 22 fails

The third shape is the one worth writing down. IF EXISTS guards the parent, not the partition, and DETACH PARTITION takes no guard of its own, so a re-run fails on the first partition the interrupted run already reached. Recovery is to drop the parent by hand, which takes every still-attached partition with it, then drop any partition left detached as an orphan, and only then re-run down so goose reaches its version-row delete.

Verified against PostgreSQL 16: a re-run after an interrupted down fails with relation "partitions.<table>_pNN" does not exist when the partition's DROP completed and relation "<table>_pNN" is not a partition of relation "<table>" when only its DETACH did, and the hand recovery above then lets the whole section replay as clean NOTICE skips. #448 already tracks removing the redundant DETACH statements, and the section links it; three migrations state the cost in their own headers, starting with 20260728120000_create_maven_remote_repositories.sql.

The rule lived only inside individual migration comments, which is not where an operator mid-rollback looks, and only three of the 22 affected files carry the accurate form. In the dev docs it covers the whole partitioned-create family with the distinction intact.

Tests

None added. The diff changes no behavior, and the claim the Delete comment used to make is already asserted by TestRepositoryStore_DeleteFKAssumption_SchemaGuard, which is the reason for deleting the prose copy rather than correcting it. The documented recovery procedure was exercised by hand against PostgreSQL 16 rather than committed as a test, because it is an operator runbook for a path no production caller takes.

Checked locally against PostgreSQL 16: the delete and reap integration tests in internal/datastore pass. Also scripts/ci/check-comment-caps.sh --base origin/main, which the pre-commit hook alone does not prove since it only checks staged blocks; go build, go vet and go vet -tags=integration on ./internal/datastore/...; 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 on lifecycle_reap_repository.go and nothing on either other changed file at the changed lines. markdownlint-cli2 reports nothing on the Markdown and vale no errors.

Size

211 reviewable lines: 52 of documentation in database-migrations.md, 65 in the new lifecycle_reap_repository.md, and 94 of Go comment lines removed or replaced across three files. Under the 500-line ceiling in docs/dev/development-model.md, and all of it comments and prose.

E2E scenarios

No scenario in docs/testing/ is added or affected. Nothing here changes what the service does.

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading