chore(datastore): bound last_health_status on npm and Maven remotes

What this does

last_health_status is a small integer column on the three remote-repository tables. It records the last upstream health-check result, and only three values are defined: 0 unknown, 1 healthy, 2 unhealthy.

The container table already had a database check that rejects anything else. The npm and Maven tables did not, so they would store 3 or -1 without complaint and the read path would later meet a value it has no meaning for. This adds the same check to both tables, so all three now agree.

New migration: 20260826072147_add_npm_and_maven_remote_last_health_status_checks.sql. It adds each constraint as NOT VALID and then validates it. Both tables are empty before production, so the validation scans no rows. That emptiness is not what makes the change safe, though: every writer is already bounded, so no row outside the domain can exist. remote.HealthStatus.IsValid rejects an out-of-taxonomy status before either store opens its transaction, the url-change writers set the literal remote.HealthUnknown, and the create statements omit the column so it takes the schema DEFAULT of 0.

Three later commits on this branch address review findings. They change comments only, no DDL, so structure.sql is untouched by them.

Tests

The npm and Maven schema suites each gain a range test mirroring the container one:

• all three defined values inserted and accepted, one row each;
• 3, -1, and 99 rejected, with the assertion naming the new constraint, so a different constraint cannot pass the test by accident.

Both tests were checked against a tree without the migration, and both fail there: on the constraint count and on all three rejections.

The two new tests are deliberate near-copies of each other rather than a shared runner. They differ only in the seed helper, the table name, the URL constant, the namespace slug, and the constraint name, and a shared runner covering all three formats was available (runContainerRemoteRepositoryCheckCases). Keeping them separate keeps each format's suite readable on its own, which is how the sibling constraint tests in both files are already written. Worth naming as a choice rather than leaving it to be rediscovered.

The constraint-count assertion uses pg_get_constraintdef's exact output with no LIKE wildcards, so a widened domain such as ARRAY[0, 1, 2, 3] cannot satisfy it. That is stricter than the container sibling, which matches two wildcard patterns.

Verification, local, PostgreSQL 17

• go build ./... and go vet ./...: clean.
• go test -tags=integration ./internal/datastore/migrations/: pass, 257s.
• go test -tags=integration -run 'Health|RemoteRepositor' ./internal/datastore/: pass.
• golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false: no finding on any line this branch adds, across ./internal/datastore/, ./internal/datastore/migrations/, and ./internal/remote/. dupl does not fire on either new test, so neither carries a dupl suppression; paralleltest does fire on both, and that is the one token in their //nolint.
• squawk: clean. Both squawk-ignore-file lines were re-measured after the comment rewrite by removing them one at a time. Without require-timeout-settings squawk reports 2 issues; without prefer-robust-stmts, 4. Both tokens are earned.
• scripts/ci/check-comment-caps.sh: passes on every block this branch touches.
• goose up, down, up against a real database: 20.86ms, 21.38ms, 27.09ms. The Down section drops both constraints with IF EXISTS, so it replays after an interruption, and the re-up is clean.

Database Review Evidence

Migrations

Note

Timings are from CI (db:migrate matrix, goose verbose) against an empty database, in apply / rollback order per PG version. Production-scale validation via Database Lab is not yet available. See Database review evidence for the matrix rationale and how to read the numbers.

Migration	PG 16	PG 17	PG 18
20260826072147_add_npm_and_maven_remote_last_health_status_checks.sql	OK (35.44ms / 53.32ms)	OK (33.27ms / 54.71ms)	OK (56.52ms / 48.06ms)

Migration notes:

• No anomaly reaches a flag threshold. The slowest apply (PG 18, 56.52ms) is 1.59x the second-slowest (PG 16, 35.44ms), well under the 2x version-regression bar. Every apply is under 60ms, against a 1s slow-migration bar and a 5-minute boot budget, and all three matrix jobs report OK.
• Rollback is slower than apply on PG 16 (1.50x) and PG 17 (1.64x), and faster on PG 18 (0.85x). Both directions are metadata-only, so this is not measuring a more expensive rollback path: the Up adds two NOT VALID CHECKs and validates them over zero rows, and the Down runs two DROP CONSTRAINT IF EXISTS, each recursing the same 64 partitions per table.
• The spread across versions is run-to-run noise rather than a planner difference, and there is direct evidence for that here. The previous pipeline ran the byte-identical DDL (the commit since then changed comments only) and reported applies of 28.62ms, 27.55ms, and 56.18ms against this run's 35.44ms, 33.27ms, and 56.52ms. PG 16 alone moved about 24% between two runs of the same statements, which is the magnitude the cross-version gaps sit in.

Query mode found nothing to collect, and not because the changed Go files are innocent of SQL. Three of them are query files by the skill's content test, since they dispatch statements through the project's instrumentQuery and instrumentExec wrappers rather than calling database/sql directly: internal/datastore/maven_remote_credentials.go, internal/datastore/maven_remote_repositories.go, and internal/datastore/npm_remote_repositories.go. None of the three adds or modifies a query-producing method. Every change in all three is comment-only, and every hunk sits on a package-level var rather than inside a method that builds a statement, so there is no new or changed SQL to EXPLAIN. No container was started and no ### Queries table is emitted.

Overlap with open merge requests

git diff --name-only origin/main...HEAD was compared against the files every open merge request touches. One overlap: !1011 (closed) (Draft, PyPI) also changes internal/datastore/migrations/structure.sql and internal/datastore/migrations/migrations_checksum_test.go. Whichever lands second has to regenerate structure.sql and re-check knownHeadVersion.

!1011 (closed) also adds migrations, and the ordering matters. It adds seven files under internal/datastore/migrations/sql/, timestamped 20260721210301 through 20260721213100, all of them earlier than this branch's 20260826072147. internal/datastore/migrations/migrations.go sets goose.WithAllowOutofOrder(false), so once a database has applied this migration it refuses a lower-versioned one rather than applying it out of order. Reviving !1011 (closed) therefore means renumbering its seven migrations above the head at that time, not just regenerating structure.sql.

Stale comments this change creates

Adding the check makes six Go comments wrong, not the four an earlier draft of this description listed. Five are fixed in the first review-fix commit:

Site	What it said
internal/remote/interfaces.go (remote.HealthStatus.IsValid)	"The column carries no CHECK constraint"
internal/datastore/npm_remote_repositories.go (errNpmRemoteRepositoryInvalidHealthStatus)	"The column carries no CHECK constraint"
internal/datastore/maven_remote_repositories.go (errMavenRemoteRepositoryInvalidHealthStatus)	"The column carries no CHECK constraint"
internal/datastore/maven_remote_credentials.go (mavenRemoteCheckViolation)	the health compare-and-set "writes only the two health columns, which no CHECK covers"
internal/datastore/npm_remote_health_integration_test.go	"The column carries no CHECK constraint, so this guard is the only thing standing between a caller bug and a permanently corrupt row"

interfaces.go is the one worth calling out: it documents the exported seam all three formats call, and it was already half-wrong before this branch, because the container table has carried the check since 20260804120000.

scripts/ci/check-comment-caps.sh measures a whole touched block against its cap, so correcting a one-line claim inside a long legacy block forces that block down to its cap. That is why the two Maven blocks are now short. Nothing load-bearing was lost: the credential-redaction rationale, which is the reason ErrMavenRemoteRepositoryCheckViolation replaces the pgconn error at all, lives on that exported sentinel and is untouched.

The sixth is now fixed as well, in its own commit: the SetHealthStatus doc comment in internal/datastore/maven_remote_repositories.go. Its last clause said the database "would accept it without complaint". Correcting it measures the whole block against its cap of 3, so the block now carries only what is not already written down elsewhere: remote.HealthStatusWriter documents the locked compare-and-set and the previous/applied semantics, the npm twin documents both in prose along with the soft-delete race conclusion, and parent liveness is documented on HealthStatus directly above. An earlier draft of this section declined that trade and offered the choice to review, which asked for the fix.

Follow-ups this branch does not close

• ADR-007 amendment scope. #30 carries the ADR-007 amendment for this column. It was written for the container table, so it covers that block alone and defers these two tables to #486 (closed). It has to cover all three blocks, or a handbook amendment written from it will exclude exactly the two tables this migration bounds. The migration comment now states that obligation instead of claiming it is already met.
• Spec text. Five spec lines need the follow-up spec merge request, not the two named earlier. docs/specs/S16-container-remote.md:95 is the urgent one: it says in the present tense that "The npm and Maven remote-repository tables declare the column with no bound and are not backfilled", which is false once this merges, and it is the first row a reader of that data-model table hits. :1183 and :1225 in the same file scope the amendment to the container table. docs/specs/S14-maven-remote.md:91 and docs/specs/S15-npm-remote.md:148 drift the other way: both give the column as NOT NULL, DEFAULT 0 with the value mapping and no check, so they now understate the schema their own slices ship. docs/specs/S13-virtual-remote-foundation.md:77 has the same omission and is already tracked as an item of #320 (closed). No issue currently covers the S16, S14, and S15 text, so that spec merge request needs one.
• url lower bound. The container migration's comment says #486 (closed) carries a lower length bound on url for all three tables as its second obligation. The issue body's Scope section covers only last_health_status, and this branch follows the issue body. This merge request says Related to rather than Closes, so #486 (closed) stays open and can still carry it, but it should not be closed on the strength of this merge request alone.

Other notes

• scripts/adr-freshness.sh reported the mirror stale with one upstream commit on ADR-007. The upstream file still declares the column on all three tables with no check, so the reading holds against upstream and not only against the mirror. ADR-007 records value mappings rather than range enforcement, and it declares every other enum-style smallint the same way while the shipped schema bounds all of them, so adding this check deviates from nothing the ADR settles.
• No end-to-end scenario in docs/testing/ is affected: this only narrows what the database accepts on a column no scenario writes directly.
• Size: 193 hand-written lines in the first commit, plus three comment-only review-fix commits: 34 insertions and 44 deletions, then 2 and 2, then 3 and 38. The other 130 lines are the regenerated structure.sql, one line per table and per partition.

Related to #486 (closed)