test(npm): npm_virtual_upstream_rules rows 3/3 (S31 plan: 3/19)

Stacked MRs

Part	MR	Scope	Reviewable LOC	Target
1/3	!2099 (merged) (merged)	schema, generated companions, and the cascade guard	709	main
2/3	!2100 (merged) (merged)	the catalog and DDL suite, and the sidecar's first two sections	1,021	main
3/3	this MR	the absences and the rows they admit	1,357	main

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. Parts 1/3 and 2/3 have merged, so this MR now targets main directly.

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 40 TestNPMVirtualRules* functions respectively, all passing, against PostgreSQL 16 with the full migration chain applied.

Parts 1/3 and 2/3 carry the figures their own MRs reported; this row is derived at the head commit. Summing the three rows as listed gives 3,087 reviewable LOC. Part 3/3 carries the overshoot and its "Diff size" section says why shrinking it would cost more than the imbalance does.

Merge-order requirement against !2033 (merged)

!2033 (merged) removes the // --- banner carve-out from scripts/ci/check-comment-caps.sh, so a gap-separated divider segment stops keeping its own 2-line row and joins the run below it. This file has four banner-plus-doc runs, and under !2033 (merged)'s version of the script they measure 4, 3, 3 and 3 lines against the test-file cap of 2. Measured by running !2033 (merged)'s script against this branch:

FAIL: ..._rowshapes_integration_test.go:18-19,21-22: test-file comment block is 4 lines (cap 2)
FAIL: ..._rowshapes_integration_test.go:38,40-43:    test-file comment block is 3 lines (cap 2)
FAIL: ..._rowshapes_integration_test.go:331,333-334: test-file comment block is 3 lines (cap 2)
FAIL: ..._rowshapes_integration_test.go:700,702-705: test-file comment block is 3 lines (cap 2)

The current gate passes on this branch. The two MRs share no file, so has_conflicts stays false and neither pipeline reports the interaction.

This MR should merge before !2033 (merged). The banners are deliberately left as they are: the merged sibling npm_virtual_repository_upstreams_rowshapes_integration_test.go carries the same pattern, so the tree-wide adjustment belongs with !2033 (merged) and its ratchet rather than being pre-paid here against a gate that is still in review. If !2033 (merged) lands first, the fix here is to fold each banner into the doc comment under it, which costs one informative line per site.

What this does

The absences, and the row shapes those absences admit.

Three catalog arms pin the absences themselves: the columns this table declines to carry and why, that there is no fourth CHECK, and that there is no trigger, which is the half the CHECK count cannot claim on its own. The rest drive real inserts against the live table.

Enumerated columns get positive hits, not just negative ones

Every value of every enumerated column is covered as an accepted insert rather than merely as a value the CHECK declines to reject: rule_type at both values, target_field at all three, and the upstream repositories row at all three kinds, all four formats and all three visibilitys. That matches the sibling npm_virtual_repository_upstreams suite value for value, which is what AGENTS.md guardrail 6 asks for. The pattern CHECK is covered in both polarities with a multibyte pair that distinguishes char_length from octet_length.

The accepted shapes are the point of the file: a duplicate rule, an allow and a deny naming the same target_field and pattern, the same rule on two associations, more rules than ADR-004's source cap, a rule under an already soft-deleted upstream or virtual repository, a rule under a namespace out of service, any id shape including a v4 and one equal to each of the other two uuid columns, and an id reused across namespaces. Each is a recorded reading rather than a gap. The cross-namespace refusal is attributed to the association key rather than the namespaces key, so the test would notice if the wrong constraint started doing the work.

The sidecar completes here

npm_virtual_upstream_rules_schema_notes.md gains the three sections that name tests in this file: why NOT NULL is the whole defense on four columns, why all three repositories enums get positive coverage, and why the rule count is unbounded. Its intro now names both suites, since both exist as of this MR.

Review findings closed

A branch review raised eight warning-level findings and sixteen observations. All are closed in a2150f0d5, which also touches npm_virtual_upstream_rules_schema_integration_test.go for the last item.

Claims that were wrong:

• The FK-count comment said both foreign-key targets are hash-partitioned and read 2+2*64 unfiltered. namespaces is not partitioned, so it reads 2+64, the 66 this table's own migration measured. The sibling carries the qualifier the adaptation dropped. Note for anyone reading the earlier review comment on this line: the conparentid = 0 filter is load-bearing and must stay. conrelid excludes referencing-side clones, but a partitioned referenced table produces child rows whose conrelid is the parent.
• The rule_type minus_one case, and the same sentence in the sidecar, said a corrupt rule_type turns every read of the repository into a 500. virtual.EligibleUpstreams returns on its anyEligible arm first, so it is a 5xx only when no other position is eligible.
• The no-trigger message quantified over the shapes the Accepts tests admit and omitted the lone wildcard and the 255-character ASCII pattern.
• The no-fourth-CHECK message called a per-target pattern grammar impossible; a predicate on the discriminator is a legal immutable CHECK, so only the rules-per-association cap is unwritable.
• The multibyte pair excludes byte bounds at or below 509, not all of them; the ASCII pair closes 510 and 511.
• The control-character case cited a logging sink for rule patterns that nothing in the tree provides.
• The lower-bound count now says ten of this service's own columns, excluding the vendored river_* tables.
• The target_field "own route" claim did not follow from its premises: 0 and 1 are reached by both routes and only 2 is route-distinct.

Coverage added:

• AcceptsContradictoryAllowAndDeny pins the one row shape the migration catalogues that no test reached.
• AcceptsDuplicateRule gains the positive arbiter arm the sibling has, so the 42P10 is attributable to the columns named rather than to arbiter inference being unavailable.

Assertions that could have stopped testing their premise:

• The three stamping UPDATEs go through a helper requiring one row affected. A zero-row UPDATE is not an error, so the two soft-delete tests passed with their ids swapped.
• AcceptsDuplicateRule counted by association alone, leaving "byte-identical" resting on a literal in another file. It now counts the value columns.

Both guards were checked by breaking the premise and confirming the test goes red, not by inspection.

Also: the two catalog counters name the offending constraint or trigger instead of printing an integer; the loop-driven assertions no longer reuse one message across iterations; three references name the file, tests and subtest they point at; and the three !2101 forward-references that merged with part 2/3 now name the rowshapes file in the present tense, since this change is what falsifies them.

Four of these fixes (the stamping UPDATEs, the two counters' diagnosability and the reused loop messages) diverge from the merged sibling suite, which has the same shapes. That is deliberate: the alternative was leaving known-thin assertions in place to keep the two files byte-comparable for guardrail 6's mirror diff. The mirror still holds subtest for subtest; only these assertion bodies differ, and the sibling is worth a follow-up.

Testing

Verified against PostgreSQL 16 on top of parts 1/3 and 2/3: 40 TestNPMVirtualRules* functions in the package and 60 subtests, all passing, up from 19 at part 2/3, and the whole internal/datastore/migrations package passes under -tags=integration. golangci-lint 2.12.2 with --build-tags=integration and --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false reports 128 findings in the package, identical to the merge base, and 0 in any of the three changed files. scripts/ci/check-comment-caps.sh passes.

Diff size

1,357 reviewable LOC, past the 500 docs/dev/development-model.md asks a split or a justification for, and the largest of the three parts: row-shape suite 1,272, sidecar 77 added and 2 removed, and 3 added and 3 removed in the part 2/3 schema suite for the forward-reference fix.

This part carries the overshoot because the row-shape suite is one file that mirrors its sibling test for test. The obvious way to shrink it would be to move its three Schema_-prefixed catalog arms into part 2/3, but the sibling npm_virtual_repository_upstreams_rowshapes_integration_test.go holds exactly those same three arms, and guardrail 6 asks a reviewer to diff the two suites for dropped subtests. Moving them would break the mirror that check depends on, so the imbalance is the lesser cost.