feat(npm): npm_virtual_upstream_rules schema 1/3 (S31 plan: 3/19)

Stacked MRs

Part	MR	Scope	Reviewable LOC	Target
1/3	this MR	schema, generated companions, and the cascade guard	728	main
2/3	!2100 (merged)	the catalog and DDL suite, and the sidecar's first two sections	949	dmeshcharakou/s31-npm-virtual-step-3a
3/3	!2101 (merged)	the absences and the rows they admit	1,208	dmeshcharakou/s31-npm-virtual-step-3b

Merge 1/3, then 2/3, then 3/3. The order is forced, not a preference. Part 2/3 asserts a table part 1/3 creates, and it also owns the fixtures part 3/3 consumes (npmVirtualRule, seedNPMVirtualRule, seedNPMVirtualRuleTree, npmVirtualRulesInsert and the constraint-name constants), so part 3/3 does not compile without it. Nothing flows the other way.

Part 1/3 additionally cannot be split from its cascade-guard companion: the moment its migration lands, TestRepositoryStore_DeleteFKAssumption_SchemaGuard fails until wantCascadeTargets names the new table, so separating them would leave main red between merges.

Each part was compiled with -tags=integration and run on its own branch, not only at the tip: 0, 19 and 39 TestNPMVirtualRules* functions respectively, all passing, against PostgreSQL 16 with the full migration chain applied.

Total 2,885 reviewable LOC, split as evenly as the atomic units allow (728 / 949 / 1,208). Part 3/3 carries the overshoot and its "Diff size" section says why shrinking it would cost more than the imbalance does.

What this does

Creates npm_virtual_upstream_rules, the allow/deny filter rules of one npm virtual upstream association: six columns, three CHECKs, a composite primary key of (id, namespace_id), and HASH(namespace_id) partitioning into 64 partitions, matching the column and index tables in the S31 spec.

Both foreign keys are composite and carry namespace_id, so a rule cannot be attached to another tenant's association. The upstream key is ON DELETE CASCADE into npm_virtual_repository_upstreams; the namespaces key is NO ACTION.

The Up section runs under -- +goose NO TRANSACTION with IF NOT EXISTS on all 65 CREATEs and the one CREATE INDEX, so an interrupted Up is replayable, and the Down drops the 64 partitions and the parent with no DETACH.

The cascade guard rides with this MR, deliberately

The new ON DELETE CASCADE key widens the repositories delete closure to three levels. TestRepositoryStore_DeleteFKAssumption_SchemaGuard derives that closure from the catalog as a transitive fixpoint over CASCADE edges, so the new table enters it and the guard's ElementsMatch fails from the moment the migration lands until wantCascadeTargets names it. That is why the guard fix is in this MR and not a later one: split apart, they leave main red. Steps 1 and 2 each shipped their guard fix in the commit that added their schema, so the pairing is the established shape rather than a one-off.

wantBlocking needs no change, because the new namespaces key is NO ACTION and namespaces is not in the closure, so mapRepositoryDeleteError's blanket 23503 mapping is unaffected. Step 2 is not a precedent for that half: the commit that added its schema (e739e728b) added a wantBlocking entry, for the association's own NO ACTION key to repositories. This table has no such key, which is why the list is untouched here.

repositories.md gains the reading its two-level list did not state: the set is a transitive closure, so a table enters it without naming repositories in any key.

Generated files

structure.sql and the three go-jet binding files are regenerated, not hand-written. knownHeadVersion moves to this migration's version_id; the embedded-SQL digest is recomputed at runtime rather than pinned, so no checksum constant changes.

Testing

Verified against PostgreSQL 16 with the full migration chain applied.

• The cascade guard fails with extra elements in list B: npm_virtual_upstream_rules without the added entry and passes with it, so the guard is doing its job rather than being satisfied vacuously.
• pg_format 5.9 leaves the migration byte-identical, so lint:sql-format holds. squawk 2.62.0 reports 0 issues.
• Up, Down and re-Up all complete, and an interrupted Up replays.

No conformance suite applies: this MR adds no Maven, npm or OCI protocol behavior.

No e2e scenario is added or affected. The table is unreachable from any request path in this MR. Nothing outside the regenerated go-jet bindings references it, so there is no client-visible behavior for docs/testing/ to describe yet. The read path that makes it reachable is Step 4.

Diff size

728 reviewable LOC, past the 500 docs/dev/development-model.md asks a split or a justification for. This MR is the split: the step was divided into three, mirroring Step 2. By file group here: migration SQL 716, repositories.md 9, repositories_integration_test.go 1, checksum constant 2. Excluded as generated: structure.sql 1,560 and the go-jet bindings 115.

Review round 1

fbb2a444f answers the branch review. One behaviour change and four comment or doc corrections:

• The Down now bounds its lock wait. Its parent DROP takes ACCESS EXCLUSIVE on namespaces and on all 65 relations of the npm_virtual_repository_upstreams tree, and that conflicts with AccessShare, so it blocks reads. The file declined to bound it because "the rollback entrypoint leaves no session to issue a SET in", which is wrong: 20260814150911_add_repositories_remotes_index sets lock_timeout inside its own -- +goose NO TRANSACTION section. The Down now sets lock_timeout = '5s' and resets it. The PGOPTIONS paragraph is dropped rather than kept beside the SET, because the SET runs after the connection is established and so overrides a value supplied that way.
• The Up header carries the readiness consequence both siblings state, the statement_timeout half of the squawk directive, and migrations.upTimeout.
• The ten bare "the spec" citations become docs/specs/S31-npm-virtual.md.
• The admitted-row-shape list gains the allow/deny pair on one (target_field, pattern), which a unique key over the value columns would still admit and where the deny decides.
• 2026989f0 attaches the 63-character measurement to the spelled-out index name in the identifier-naming comment; it was reading as a claim about the 28-character abbreviated form.
• repositories.md said npm_virtual_repository_upstreams "cascades from repositories". It does not; that edge is NO ACTION. The chain runs through npm_virtual_repositories.

Verified for this round: pg_format 5.9 leaves the migration byte-identical and squawk 2.62.0 reports 0 issues. Up, Down and re-Up complete against PostgreSQL 16, 17 and 18 with the full 96-migration chain applied; after the Down no relation named npm_virtual_upstream_rules remains, the re-Up restores 64 partitions and the index, and lock_timeout reads 0 on a fresh session, so the RESET does not leak. With a concurrent reader holding AccessShare on namespaces, the parent DROP now fails at 5s with canceling statement due to lock timeout; unbounded it was still queued at 12s.

The one finding round 1 left open now has its own MR. The shipped index leads on namespace_id, so the batched rules read must carry its own namespace_id equality or it scans all 64 partitions, and docs/specs/S31-npm-virtual.md and the plan both wrote the predicate without it. A step MR does not edit the plan, so the correction went to !2105 (merged), which fixes the spec enumeration, the plan's quote of it, the plan's squawk LIMIT argument and the plan's Step 4 entry. Issue #885 (closed), the artifact Step 4 is actually written from, was corrected the same way. Neither is a dependency of this MR: they can merge in any order.

Review round 2

985606f answers a second branch review. Comment-only, so structure.sql, the go-jet bindings and the checksum constant are untouched; pg_format 5.9 leaves the migration byte-identical and squawk 2.62.0 reports 0 issues.

The ADR-007 divergences are now called out where they appear. docs/specs/S31-npm-virtual.md names four things that are not in ADR-007 and says "Each is called out where it appears." Two of them land in this file, the CHECK constraints and the ON DELETE actions on both keys, and neither was called out. The file's only ADR-007 mention sat in the header and read as a claim of conformance for the CHECKs, so a reader would have taken them for ADR text. The step 2 sibling does this at four sites; this file now does it at two. The amendment itself is handbook !20938, which covers all four S31 divergences.

Four claims were wrong or too broad.

• "the allow adds nothing", for an allow and a deny naming the same (target_field, pattern). Per docs/specs/S13-virtual-remote-foundation.md, an upstream is eligible when the path matches at least one allow rule or the upstream has no allow rules. Adding the allow moves the association out of that second arm, so every path the pattern does not match stops being eligible. The deny decides only for the path it matches.
• rule_type's squawk justification called target_field a closed set of two values, where it has three, and said neither column can grow, which the CHECK comment 25 lines below contradicts by naming a future added value as the ordinary case. Closed, not fixed, is the property the smallint width needs.
• The Down's bound claimed goose runs every statement on one session. That is the provider path (migrations.Down/DownTo), which pins one *sql.Conn. The goose CLI that mise run db:rollback execs runs the statements against the pool, where the SET carries on idle-connection reuse instead. The sibling reasons about that path and reaches the opposite conclusion, so the blanket claim overturned it silently.
• RESET lock_timeout is skipped exactly when the bound fires, since goose abandons a NO TRANSACTION migration at the first error. Recovery from a timed-out Down is a re-run of Down, not of Up.

Known and not fixed here: the two pg_locks measurements are sampled on PostgreSQL 16.15 only, where the step 2 sibling sampled its equivalents on 16.15 and 17.11. The numbers are structural (1 + 65) and CI runs 16/17/18.

Related to #884 (closed)