one_d4: one writer of the schema, and no scrapers left (#1401, #1419, #1426)

[aaylward]

Closes #1401, closes #1419, closes #1426.

The C++ Postgres suites run the real migrations (#1401 Group B)

schema_contract_test regex-parsed CREATE TABLE/ALTER TABLE out of migrations/pg and compared it to DDL hand-copied into five fixtures. Both halves are gone: the fixtures apply the migration files.

• migration_files (testonly) reads manifest.txt, resolves each step the way MigrationFiles.java does, and runs pg/ + shared into a schema of its own.
• pg::Client::ExecScript runs a file whole over the simple query protocol. Exec binds parameters, which puts it on the extended protocol, and that carries one statement per message however few parameters are passed — so a two-statement .sql file is an error there. Running the file whole also means no C++ twin of SqlStatements.
• It checks current_schema() before migrating. A fixture whose search_path went missing would otherwise build its tables in public beside every sibling suite's rows.

Two fixtures were laxer than production, and now fail if they stay that way: game_features.request_id and platform are NOT NULL, and motif_occurrences.id is not generated.

retention_test keeps one schema assertion, asked of information_schema rather than of the DDL text: the sweep binds naive UTC literals, and a column migrated to TIMESTAMPTZ would compare through the session timezone and cut a different set of rows — identically to this one under the suite's UTC stamps, so no result can show it. RunsInItsOwnSchemaRatherThanPublic is deleted; ResetToMigratedSchema's check makes it unfailable, and covers all five suites instead of one.

Boot verifies the schema instead of writing it (#1426)

one_d4_migrate applies the migrations before the service starts, so Migration.run() at boot was a second writer of the same DDL.

Migration.verify() applies the steps to a scratch schema inside a transaction it always rolls back, then reports what the live schema does not have. Nothing is committed and the deployed tables are never touched — the statements only ever see the empty copies. What to expect comes from the files, not from a list anyone maintains.

Existence only: tables, columns, indexes and constraints by name. Types are not compared — both runners read one set of files, and refusing to serve is too blunt an answer to a column somebody widened by hand.

The H2 test path has no deploy step in front of it and still applies at boot. SqlDialect.migratedBeforeBoot() names that, defaulting to true so a dialect added for a deployed engine cannot become a second writer by omission. Migration.atBoot() holds the choice, so a test can watch what it wrote.

Local dev follows: one_d4/README.md used to say starting the service was enough to bring a database up. It runs one_d4_migrate first now.

#1426's other question — whether re-run-everything should grow a tracking table — stays answered "no": neither pressure it named (boot time growing with the step count, a step that cannot be written idempotently) exists. migrations/README.md keeps that trigger without the issue.

#1426's proposed exit criteria were a handful of clean deploys first. Folding it in here on request, so the first deploy after this is where the verifier proves itself; the failure mode is a crash-loop against a schema one_d4_migrate did not finish, which is what its message names.

The Go scraper (#1401 Group A)

TestOneD4SelectorCoversTheServiceTheCppWorkerReportsAs regexed .service_name out of worker_main.cc. TestOneD4QueriesNameRealInstrumentsAndScopeThem already asserts every indexing selector is service_name=~"one_d4(_worker)?"; what the scrape added was that this literal matches how worker_main.cc spells it. Deleted, with the coupling stated in both files — the MAX_RUN disposition. worker_service_name and the Go test's data dep go with it.

What #1419 was still holding open

Its "one copy of the DDL" bullet listed PostgresSqlDialect, the C++ fixture hand-copies, and the H2 translations. The first went with #1419 itself, the second goes here, and the third is option 1 as the issue chose it — h2/*.sql, test-only. Option 2 (moving the Java schema tests onto Postgres) the issue calls its own decision, and it stays one.

one_d4's gate on the migrate one-shot survives with a new reason: not to serialize two writers, but because boot now refuses to serve against a schema that step did not finish. deploy_config_test.go, compose.yaml and both READMEs say so.

Testing

Local Postgres 16 with PG_TEST_DB_URL set, so the gated suites ran rather than skipped: 95/95 C++ under domains/, 66/66 across one_d4, prom_proxy, deploy and bazel/rules.

Mutation checks, each killed:

mutation	caught by
lease_expires_at renamed in V014	pg_queue_test, pg_game_sink_test, retention_test — and not the two reanalysis suites, which do not read it
game_features.indexed_at → TIMESTAMPTZ	TheTimestampsTheSweepComparesAreNaive, alone in its suite
search_path dropped from retention_test's conninfo	every test in the suite, in SetUp
the plain-identifier check removed	ASchemaNameThatIsNotAPlainIdentifierIsRefused
a step removed from manifest.txt	EveryMigrationFileIsReachableFromTheManifest
a step given both a pg/ and a shared file	that, plus EveryManifestStepResolvesForBothEngines
PGRES_EMPTY_QUERY treated as success	RefusesAScriptWithNoStatementsInIt
atBoot() applies instead of verifying	bootDoesNotCreateTheSchemaOnTheDeploymentDialect
verify() commits instead of rolling back	writesNothing
indexes dropped from the catalog query	namesTheIndexAMissingStepWouldHaveBuilt
H2SqlDialect.migratedBeforeBoot() → true	nine module-boot tests

I did not run a review panel myself; this session cannot spawn subagents. cursor[bot] ran a four-lens panel against 9732899; its three findings were all authoring-doc drift and are fixed in bba868a, with the two inline threads answered and resolved. main moved (#1448) and is merged in at 3fedff6.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ST7XLGWstRE4CfUF84ijKD

[cursor]

Review panel on #1449 (four lenses: correctness/control flow; data access/SQL/resource safety; tests/docs/CI; altitude/design). Each agent hunted then tried to refute; survivors re-checked against HEAD 97328994. CI: required checks green except build-and-test still pending on this head.

Headline: right shape for #1401 / #1419 leftover / #1426 — one deployed writer (one_d4_migrate), boot verifies via scratch-schema apply+rollback+catalog names, C++ suites run the real pg/ files through ExecScript, Go scrape deleted with the MAX_RUN disposition. Control flow, SQL/resource safety, and BUILD/test wiring held under refute. What survives is authoring-doc drift: the contract file this PR rewrote still narrates the dual-writer world in two places, and one_d4/README.md (untouched) still says boot applies.

2 findings in the inline comments on migrations/README.md, plus one outside the diff below. Most-severe first.

Also outside the diff (fold-in here or follow-up):

• domains/games/apis/one_d4/README.md:117–123 — “Migrations run at startup” / “Migration applies them at boot” still describes the second writer. Same rewrite as the migrations README / deploy docs.

Noted, not a hold: verify() still pays O(steps) on every boot (apply into scratch, then roll back). #1426’s win is “only migrate writes,” not a cheaper boot. The README’s tracking-table trigger still applies to that path; no shortcut demanded now.

Verified clean (not reported):

• atBoot(): Postgres → verify(), H2 → run() via migratedBeforeBoot(); default true is fail-closed for new dialects
• verify() always rolls back; writesNothing / bootDoesNotCreate… pin non-writer; catalog is existence-by-name only (intentional)
• ResetToMigratedSchema checks current_schema(), refuses non-plain identifiers, applies via ExecScript; empty script → InvalidArgument
• Retention’s remaining information_schema naive-timestamp assert; five PG suites on :migration_files
• Compose/deploy README/deploy_config_test gate wording matches one_d4: demote boot-time Migration.run() to a verifier once one_d4_migrate is trusted #1426; Go scrape retired with selector pin + coupling comments both sides
• migration_files / ExecScript / SqlDialect.migratedBeforeBoot() earn their keep vs shared-binary or file-splitting alternatives

Non-blocking on the schema-writer change itself. Fold the three doc contradictions so the next author does not reintroduce a boot migrator from the README.

[aaylward]

All three doc findings fixed in bba868a; the two inline threads are answered and resolved.

The out-of-diff one was the worst of them, and worse than "still describes the second writer": one_d4/README.md told a local developer that starting the service was enough to bring a database up. That is now a verification failure, not a migration. The recipe runs one_d4_migrate first, and says the step is not optional and needs re-running after a new V<NNN> step.

On the O(steps) boot cost: agreed it is not what #1426 bought, and it is not free. PostgresMigrationVerifyTest runs five full migrate-or-verify cycles against a real Postgres inside a 3.4s target, JVM startup included, so a single verify is well under a second at seventeen steps. The README trigger covers it if that stops being true.

main moved (337542d, #1448); merged in at 3fedff6 and revalidated — //deploy/... and //domains/games/apis/one_d4/... 50/50 with PG_TEST_DB_URL set.

---

Generated by Claude Code

[cursor]

Recheck on 3fedff65 (docs fix bba868ae + merge of main/#1448).

Prior findings: all three closed.

• migrations/README.md intro — migrate applies, Java verifies, C++ suites apply into their own schemas
• Gate paragraph — boot-check vs unfinished one-shot (not “two runners”)
• one_d4/README.md — migrate is required in the local recipe; boot checks and refuses; no “applies at boot”

No leftover dual-writer wording in one_d4 / deploy/consolidated. Docs commit is README-only; merge is Caddy/#1448 only — Migration.atBoot → verify() path untouched.

Surviving findings: none. O(steps) verify cost stays noted, not a hold — author’s timing note on the suite is enough.

CI: required checks green on this head except build-and-test still pending. LGTM once that lands.

[cursor]

build-and-test green on 3fedff65 (20/20). Prior recheck stands — LGTM.