schemachange: gate cutover on the ST-5 fidelity checklist and mint CutoverReady

[Kiran01bm]

Adds schemachange.GateCutover: the ST-5 fidelity checklist that stands between a verified shadow and the swap, and the CutoverReady proof the swap will accept nothing but.

Why

The copier fills the shadow and the verifier proves the rows are equal (CO-1), but data equality is not the whole gate. Between the build and the swap the source may have gained a grant, an index, or a new OID; the shadow may carry an invalid index from a cancelled concurrent build; the _old name the swap assigns may still be worn by a leftover from an earlier run; and the swap needs to know which shadow index takes which source index's name — a question LIKE … INCLUDING ALL answers badly, since it names every shadow dependent after the shadow. ST-5 says the swap runs only when all of that holds. Landing the gate on its own keeps the checklist, its refusal causes, and the pairing rule in one review, apart from the rename sequence that consumes them.

What

• pkg/schemachange:
  • GateCutover(ctx, pool, lock, built, verified, opts) refuses before connecting when either proof is empty, the two name different relations or different relation OIDs — VerifiedShadow now carries the source and shadow OIDs the pass compared, so a shadow dropped and rebuilt under the same derived name does not inherit an earlier pass's proof — or the verified watermark is not complete (cutover-unverified, CO-1), then runs the checklist in one bounded read-only REPEATABLE READ transaction under SET LOCAL ROLE owner, with the lock session's Bind context and the in-transaction lock confirmation every shadow operation makes (LK-1). The checklist refuses on a moved source or shadow OID (cutover-relation-replaced, ST-6), an invalid shadow index (cutover-index-invalid), a source or shadow fingerprint that no longer matches the build's (cutover-schema-drift), a source or shadow metadata snapshot that drifted — naming the drifted facts — source identity columns whose sequence options changed, or table grants that differ between the live source and the live shadow, a fact the gated statement cannot change and the one cross-check that holds on a proof re-derived by InspectShadow (cutover-fidelity-drift), and a derived _old name already worn by a relation or statistics object, or a constraint-backed source index name already held by another constraint on the shadow (cutover-name-taken). gateCutoverTx is the checklist proper, so the swap re-runs it inside its own transaction before the first rename.
  • CutoverReady carries the two proofs it held, Indexes() and Statistics() as DependentPairing{Pairs, UnpairedSource, UnpairedShadow} — pairs matched by name-free catalog definition (access method, uniqueness, NULLS NOT DISTINCT, key and included columns by name, opclasses, collations, indoption, predicate, backing constraint; statistics kinds and columns), two source dependents with one definition each taking a distinct partner — and OwnedSequences() for the swap to re-own (D5, D8). An unpaired dependent is reported, not refused: the gated statement may have dropped an index's column or added an index.
  • BuiltShadow.ShadowFidelity() / Proof.ShadowFidelity record the shadow's own metadata snapshot after the gated statement ran, apart from the source's, because the statement may change metadata on purpose; the gate holds the shadow to it. FidelitySnapshot.ColumnStatisticsTargets and ExtendedStatisticsTargets carry each column's and each extended-statistics object's explicit SET STATISTICS target, which LIKE … INCLUDING ALL does not copy, and the builder applies them to the shadow — the extended ones to the LIKE-named copies paired by definition. The gate re-owns only the sequences of columns the shadow kept; a dropped bigserial column's sequence goes with the old table. fidelityDrift derives the fact list from the snapshot's fields by reflection, so a field added later is compared without a hand-kept list changing.
  • Six new RefusalCause values with their Invariant() mapping; requireTableLock takes a schema and table so the gate can use the built shadow's.
• Docs in the same PR: refusal-classes.md class rows for the six causes (pinned by the docs guard), invariants.md ST-5 "enforced" and CO-1 naming GateCutover as the enforcement, copy-and-swap-design.md package map, SAFETY.md row.
• Tests on real PostgreSQL: a built, copied, and verified orders table passes the gate with its primary key, qty index, and extended statistics paired to the LIKE-named shadow objects, the index on the dropped column reported unpaired, and the serial key's sequence listed; a unique constraint the change adds is reported as an unpaired shadow index; zero built proof, zero verified proof, and a verified proof for another table are each cutover-unverified; no lock and a lock for another table are shadow-lock-unproven, a lock whose backend is gone is shadow-lock-unconfirmed; GRANT … TO PUBLIC on the source after the build is cutover-fidelity-drift; CREATE INDEX on the source after the build is cutover-schema-drift; a failed CREATE UNIQUE INDEX CONCURRENTLY on the shadow is cutover-index-invalid; the source renamed away and recreated under its name is cutover-relation-replaced; a table wearing the _old name is cutover-name-taken; a gated ADD CONSTRAINT orders_pkey CHECK is cutover-name-taken while the same borrowed from a plain index passes; SET STATISTICS targets land on the shadow and in both snapshots, column and extended alike, and a type change resets the target of the statistics object it rebuilds exactly as PostgreSQL does on the source; the proof JSON round-trip covers every shadow_fidelity, column_statistics_targets, and extended_statistics_targets key path. The follow-up commit adds: a verified proof held across DropShadow + BuildShadow is cutover-unverified; a REVOKE on the source after the build is cutover-fidelity-drift even through InspectShadow; the shadow replaced, reshaped, or commented after the build and a source identity's SET INCREMENT BY are each refused, while SET (fillfactor = 50) as the gated statement passes; unique indexes differing only in NULLS NOT DISTINCT (PG 15+), indexes differing only in operator class, and statistics objects differing only in kinds each pair with their own copy; a leftover wearing a statistics object's or identity sequence's _old name is cutover-name-taken; a dropped bigserial column's sequence is not listed for re-owning. Unit tests pin pairing-by-definition, partner reuse, derived old names, drift naming over every snapshot field, keptSequences, and the Invariant() of every cause.

Before / after

Before                                        After
┌────────┐ copy+verify ┌────────┐             ┌────────┐ copy+verify ┌────────┐
│ source │────────────▶│ shadow │             │ source │────────────▶│ shadow │
└────────┘             └────────┘             └───┬────┘             └───┬────┘
                                                  │   GateCutover, one read-only tx under the lock:
   VerifiedShadow proves the rows;                │   OIDs unchanged? fingerprints unchanged?
   nothing holds the metadata, the                │   fidelity snapshots unchanged? shadow indexes valid?
   indexes, or the names to the build             │   _old names free? constraint names free?
                                                  ▼
                                            CutoverReady{built, verified,
                                                         index pairs by definition,
                                                         statistics pairs, owned sequences}
                                              ── or a cutover-* RefusalCause

Known gap, carved as the next PR (not a review item here)

• Index pairing across a retyped column (cs9-pair-type-change, lands before the swap consumes the pairing). PostgreSQL rebuilds an index on a column the gated ALTER COLUMN … TYPE changes with the new type's default operator class — int4_ops → int8_ops on int → bigint, while pg_get_constraintdef still reads PRIMARY KEY (id) (checked on PG 16). indexDefinition includes Opclasses, so under this PR's definition-equality rule the shadow's pkey and every secondary index on the widened column are reported unpaired, and a swap built on this pairing alone would leave them with their _pgsprite_<hash>_new_… names. The follow-up adds a second pairing pass for the leftovers: equal on access method, uniqueness, key and included columns, indoption, predicate, and constraint kind, with opclass and collation allowed to differ only on the columns the statement retyped. This PR keeps the strict rule on purpose: it is the exact-match base the relaxed pass refines, and UnpairedSource / UnpairedShadow already make the gap observable.

Follow-ups

• cs9-swap: render the rename sequence from CutoverReady and run it inside one ACCESS EXCLUSIVE transaction that re-runs this checklist first.
• cs9-fidelity-carry: carry attoptions (n_distinct) alongside the statistics target; re-synchronise additive drift (a GRANT after the build) under the lock instead of refusing; an exported drift check an orchestrator can run during the copy.
• cs9-statistics-namespace: an extended-statistics object that lives outside its table's schema — LIKE copies it into the table's schema and takenNames looks for its _old name only there; the gate should refuse or follow stxnamespace.
• cs9-cutover-ready-report: a plain encodable view of CutoverReady (pairs, unpaired dependents with the reason, sequences, accepted proofs) and a typed side/facts on RefusalError so an importer routes drift without reading Detail.

References

• docs/invariants.md ST-5, ST-6, CO-1, LK-1; docs/copy-and-swap-design.md D5, D8.
• Stack: checksum: run the verifier under an explicit divergence policy and mint the proofs (CO-2, CO-3) #136 (checksum.Check and VerifiedShadow) and checksum: chunk verifier comparing source and shadow in one snapshot (CO-1) #134 merged.

🤖 Drafted with Amp (Claude Opus 4.6); reviewed and edited by the author.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[aparajon]

🤖 1/2: adversarial correctness review of b54bd32. I read GateCutover, the checklist, the pairing, and the BuildShadow / InspectShadow changes. I checked them against CO-1, ST-5, ST-6 and LK-1, and against checksum.VerifiedShadow from #136. Probes and mutations ran on real PostgreSQL 16, using the PR's own orders fixture.

1 blocking, 6 non-blocking.

The gate refuses before connecting when there is no lock or a lock for another table. It runs read-only, under the lock's Bind context, and confirms the lock in its transaction, so LK-1 holds. The source half of the checklist is pinned, and the pairing never reuses a partner. The blocking finding is in how the gate ties the data proof to the shadow it is gating. It ties them by name only, so a proof for a shadow that no longer exists passes.

Blocking

1. The gate accepts a VerifiedShadow from an earlier shadow, so a rebuilt shadow that was never copied gets CutoverReady (CO-1). cutover_gate.go:185, proofs.go:13-27

checkCutoverProofs matches the two proofs on schema, table and shadow name. VerifiedShadow carries nothing else. But ShadowName is a pure function of schema and table, so every shadow ever built for orders has the same name. Take a run whose gate refuses, for example on cutover-index-invalid. The orchestrator calls DropShadow, then BuildShadow, and still holds the first pass's VerifiedShadow. The new BuiltShadow and the old proof match on all three names, and the watermark is complete. The gate then mints CutoverReady for an empty table. A swap that consumes it replaces orders with zero rows.

The caller cannot catch this either: the proof carries no OID to compare. The verifier already checks both OIDs on every pass (the replaced-relation test in CO-1). They are just not carried into the proof it mints. The fix is to put shadow.SourceOID() and shadow.ShadowOID() into newVerifiedShadow, and refuse in checkCutoverProofs when they differ from built.SourceOID() / built.ShadowOID(). With that change (13 lines across proofs.go and cutover_gate.go), every TestGateCutover* test and go test ./pkg/checksum/ -run 'TestCheck|TestProve|TestOutcome' still pass.

Test that fails on b54bd32 and passes with the fix

// A verified-shadow proof minted for a shadow that was since dropped does
// not prove the shadow rebuilt under the same derived name, which no copy
// has filled (CO-1).
func TestGateCutoverRefusesAVerifiedProofForAnEarlierShadow(t *testing.T) {
	f := newShadowFixture(t)
	f.createOrders(t)
	s := f.stage(t, "orders", `ALTER TABLE %s.orders DROP COLUMN note`)

	require.NoError(t, schemachange.DropShadow(t.Context(), f.pool, s.lock, s.target, schemachange.Options{}))
	rebuilt, err := schemachange.BuildShadow(t.Context(), f.pool, s.lock, s.target, f.alter(t, `ALTER TABLE %s.orders DROP COLUMN note`), schemachange.Options{})
	require.NoError(t, err)
	require.Equal(t, s.built.ShadowTable(), rebuilt.ShadowTable(), "the rebuild wears the same derived name")
	require.NotEqual(t, s.built.ShadowOID(), rebuilt.ShadowOID(), "the rebuild is a new relation")

	_, err = schemachange.GateCutover(t.Context(), f.pool, s.lock, rebuilt, s.verified, schemachange.Options{})
	assert.Equal(t, schemachange.CauseCutoverUnverified, schemachange.RefusalCauseOf(err), "the rebuilt shadow holds no rows")
}

--- FAIL: TestGateCutoverRefusesAVerifiedProofForAnEarlierShadow (0.62s) on b54bd32 (expected: "cutover-unverified", actual: ""). It passes with the fix.

Non-blocking

1. On the resume path, the gate holds each table only to itself, so a REVOKE on the source after the build reaches the swap (ST-5). inspect.go:76, inspect.go:106, cutover_gate.go:246-271

confirmFidelity compares the live source with built.Fidelity() and the live shadow with built.ShadowFidelity(). It never compares the source with the shadow. For a built proof that is enough, because the build checked that the grants matched (shadow-grants-differ). InspectShadow, though, reads both snapshots from the catalog as they are now, and this PR adds the shadow's. So on resume, each comparison checks a table against itself.

Here is the sequence. GRANT SELECT … TO PUBLIC runs before the build, the shadow copies it, and REVOKE runs after. The inspected proof's fingerprints are equal to the build's, so the check the InspectShadow godoc asks for ("compares the returned fingerprints", inspect.go:23) passes. The gate also passes, while the shadow still grants SELECT to PUBLIC. After the swap, the live table grants a privilege the source had revoked.

This stays non-blocking because nothing calls the resume path yet. A caller can also catch it by comparing the whole Proof with its checkpoint, which is what the design doc describes. Two cheap fixes:

• Have the gate compare the facts an ALTER TABLE cannot change on purpose between the live source and the live shadow. Table grants are the clearest of these. A seven-line jsonEqual(source.Grants, shadow.Grants) check in confirmFidelity makes the test below pass, and every existing gate test still passes.
• Change the InspectShadow godoc to say "the whole Proof" instead of "the fingerprints".

Test that fails on b54bd32 and passes with the grants cross-check

// A privilege revoked on the source after the build is still granted on the
// shadow. Resuming through InspectShadow yields the same fingerprints the
// build recorded, and the gate must still refuse (ST-5).
func TestGateCutoverAfterInspectionRefusesASourceRevoke(t *testing.T) {
	f := newShadowFixture(t)
	f.createOrders(t)
	f.exec(t, `GRANT SELECT ON %s.orders TO PUBLIC`)
	s := f.stage(t, "orders", `ALTER TABLE %s.orders DROP COLUMN note`)
	f.exec(t, `REVOKE SELECT ON %s.orders FROM PUBLIC`)

	inspected, err := schemachange.InspectShadow(t.Context(), f.pool, s.lock, s.target, schemachange.Options{})
	require.NoError(t, err)
	require.Equal(t, s.built.SourceFingerprint(), inspected.SourceFingerprint())
	require.Equal(t, s.built.TargetFingerprint(), inspected.TargetFingerprint())

	_, err = schemachange.GateCutover(t.Context(), f.pool, s.lock, inspected, s.verified, schemachange.Options{})
	assert.Equal(t, schemachange.CauseFidelityDrift, schemachange.RefusalCauseOf(err), "the shadow still grants SELECT to PUBLIC")
}

--- FAIL: TestGateCutoverAfterInspectionRefusesASourceRevoke (0.12s) on b54bd32 (actual: ""). The source ACL reads back without SELECT->0, and the shadow's still has it.

2. Index pairing ignores NULLS NOT DISTINCT, so two unique indexes on the same column can pair crosswise (D8). dependents.go:96-114, dependents.go:128-152

indexDefinition claims "everything that decides which rows and values it covers and how". pg_index.indnullsnotdistinct decides whether a second NULL is a duplicate, and it is not in the key. The per-column pg_get_indexdef does not print it either. Take a source with z_nnd UNIQUE (code) NULLS NOT DISTINCT, created first, and a_plain UNIQUE (code). The two keys are equal. So a_plain takes …_new_code_idx, which is the NULLS NOT DISTINCT copy, and z_nnd takes the plain one. After the swap, the index named a_plain rejects a second NULL, and a DROP INDEX z_nnd drops the wrong index.

The fix is to add COALESCE((to_jsonb(i) ->> 'indnullsnotdistinct')::bool, false) to readIndexes and to the definition. Reading it through to_jsonb should keep the query valid on PG 14, which has no such column (I ran it on 16 only). With that change, the test below and every gate test pass.

Test that fails on b54bd32 and passes with the fix

// Two unique indexes on one column that differ only in NULLS NOT DISTINCT
// are different indexes, and each pairs with the shadow copy that has the
// same setting (D8).
func TestGateCutoverPairsIndexesOnNullsNotDistinct(t *testing.T) {
	f := newShadowFixture(t)
	f.exec(t, `CREATE TABLE %s.items (id bigint PRIMARY KEY, code text, other integer)`)
	f.exec(t, `CREATE UNIQUE INDEX z_nnd ON %s.items (code) NULLS NOT DISTINCT`)
	f.exec(t, `CREATE UNIQUE INDEX a_plain ON %s.items (code)`)
	f.exec(t, `INSERT INTO %s.items SELECT g, 'c' || g, g FROM generate_series(1, 50) g`)
	s := f.stage(t, "items", `ALTER TABLE %s.items DROP COLUMN other`)

	ready, err := f.gate(t, s)
	require.NoError(t, err)
	nullsNotDistinct := func(name string) bool {
		var nnd bool
		require.NoError(t, f.pool.QueryRow(t.Context(), `
			SELECT i.indnullsnotdistinct FROM pg_index i
			JOIN pg_class c ON c.oid = i.indexrelid JOIN pg_namespace n ON n.oid = c.relnamespace
			WHERE n.nspname = $1 AND c.relname = $2`, f.schema, name).Scan(&nnd))
		return nnd
	}
	for _, pair := range ready.Indexes().Pairs {
		assert.Equal(t, nullsNotDistinct(pair.SourceName), nullsNotDistinct(pair.ShadowName),
			"%s pairs with %s", pair.SourceName, pair.ShadowName)
	}
}

--- FAIL: TestGateCutoverPairsIndexesOnNullsNotDistinct (0.08s) on b54bd32 (a_plain pairs with _pgsprite_…_new_code_idx: expected: false, actual: true).

3. Extended statistics lose their target and their schema, and the pairing cannot see either. dependents.go:196-220, dependents.go:245-265

This PR carries column SET STATISTICS targets because LIKE resets them. LIKE … INCLUDING ALL does the same to extended statistics. I measured it on 16: a source object probe137b.st with ALTER STATISTICS … SET STATISTICS 500 comes out as probe137.t_new_a_b_stat with stxstattarget = -1. Note the schema too: a statistics object may live outside its table's schema, and the copy lands in the table's.

The pairing key is kinds plus columns, so these two still pair. After the swap, the live table's statistics object has the default target and has moved schemas. takenNames also looks for the derived _old statistics name only in the table's schema. ALTER STATISTICS … RENAME renames within the object's own schema, so a collision there is not seen until the swap runs under the lock. The fix is to carry stxstattarget the way ColumnStatisticsTargets is carried, and either check names in stxnamespace or refuse a statistics object outside the table's schema.

4. The shadow half of the checklist is unpinned, and so is the identity-options check. Four mutants survive the PR's tests. Skip the shadow OID check (M10). Skip the shadow fingerprint (M13). Skip the shadow fidelity comparison (M15). Skip the identity sequence-options comparison (M17). Each of these is a refusal cause the PR documents. Nothing exercises a statement that changes table metadata on purpose either, which is the reason ShadowFidelity exists: M31 records the source's snapshot as the shadow's, and it survives. The test below passes on b54bd32 and kills M10, M13, M15 and M17. A case that stages ALTER TABLE … SET (fillfactor = 50) and expects the gate to pass would kill M31.

Test that passes on b54bd32 and kills M10, M13, M15, M17

// The checklist holds the shadow to its own build-time record as well as the
// source: a shadow replaced under its name, reshaped, or given metadata after
// the build is refused, and so is a source identity whose sequence options
// changed (ST-5, ST-6).
func TestGateCutoverRefusesShadowSideAndIdentityDrift(t *testing.T) {
	cases := []struct {
		name  string
		after func(f shadowFixture, s staged)
		want  schemachange.RefusalCause
	}{
		{"shadow replaced", func(f shadowFixture, s staged) {
			f.exec(t, "ALTER TABLE "+f.shadowName(s.built)+" RENAME TO shadow_was")
			f.exec(t, "CREATE TABLE "+f.shadowName(s.built)+" (LIKE %s.shadow_was INCLUDING ALL)")
		}, schemachange.CauseRelationReplaced},
		{"shadow reshaped", func(f shadowFixture, s staged) {
			f.exec(t, "ALTER TABLE "+f.shadowName(s.built)+" ADD COLUMN extra integer")
		}, schemachange.CauseSchemaDrift},
		{"shadow metadata", func(f shadowFixture, s staged) {
			f.exec(t, "COMMENT ON TABLE "+f.shadowName(s.built)+" IS 'touched'")
		}, schemachange.CauseFidelityDrift},
		{"source identity options", func(f shadowFixture, s staged) {
			f.exec(t, "ALTER TABLE %s.accounts ALTER COLUMN id SET INCREMENT BY 5")
		}, schemachange.CauseFidelityDrift},
	}
	for _, c := range cases {
		t.Run(c.name, func(t *testing.T) {
			f := newShadowFixture(t)
			f.exec(t, `CREATE TABLE %s.accounts (id bigint GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, qty integer NOT NULL)`)
			f.exec(t, `INSERT INTO %s.accounts (qty) SELECT g FROM generate_series(1, 2500) g`)
			s := f.stage(t, "accounts", `ALTER TABLE %s.accounts ALTER COLUMN qty TYPE bigint`)
			c.after(f, s)
			_, err := f.gate(t, s)
			assert.Equal(t, c.want, schemachange.RefusalCauseOf(err), "%v", err)
		})
	}
}

It passes on b54bd32. Under M10, M13, M15 and M17, the shadow_replaced, shadow_reshaped, shadow_metadata and source_identity_options subtests fail, one each.

5. Smaller unpinned branches. No test pins these. The _old names for identity sequences and for statistics objects (M20, M21). The opclass and statistics-kind fields of the pairing keys (M25, M26). A lock lost while the gate is running, which should be reported as shadow-lock-lost rather than the cancelled statement (M32). The CO-1 mapping of cutover-unverified in Invariant() (M35). Separately, fidelityDrift compares a hand-kept list of twelve facts. A FidelitySnapshot field added later and missing from that list would never drift. A test that fills every field and expects every JSON tag back (or a list built by reflection) would turn that omission into a failing test.

6. CO-1's registry entry still lists cutover as planned. invariants.md:48 says "Planned enforcement: cutover accepts only a VerifiedShadow", and the PR does not touch it. Once blocking 1 is fixed, GateCutover is where that holds, so the Enforced today line can name it. ST-5's rule text (invariants.md:531) says the engine "verifies the shadow carries the source's" metadata. The gate verifies each table against the build's record, which only gives that result when the record came from a build (non-blocking 1).

Verified

• CO-1 extends, but blocking 1 weakens it. The gate refuses an empty or mismatched proof before connecting. The watermark check is equivalent today, because Check mints a VerifiedShadow only on a complete pass (M6).
• ST-5 extends: the gate is new enforcement, and every source-side refusal is pinned (M9, M11, M12, M14, M18, M19, M22, M23). The shadow side is unpinned (non-blocking 4), and the resume path has a gap (non-blocking 1).
• ST-6 upholds for the source (M9). The shadow OID check is correct but untested (M10).
• LK-1 upholds. requireTableLock runs before connecting. Without it, a nil lock panics in Bind instead of being refused (M7). The in-transaction Confirm is pinned (M8).
• The gate never writes, so making the transaction read-write is equivalent (M33). It takes no relation lock and runs at READ COMMITTED, so the standalone CutoverReady is a pre-flight. The swap's own re-run of gateCutoverTx under ACCESS EXCLUSIVE has to be the authority, as the godoc says.
• The pairing never reuses a shadow partner (M24), and owned sequences come from the OWNED BY edge (M28). The column statistics targets land on the shadow and in both snapshots (M29, M34).

Mutant	Caught by
M1 accept an empty built proof	equivalent: the name match refuses it
M2 accept an empty verified proof	equivalent: the name match refuses it
M3, M4, M5 drop one name clause	equivalent: the shadow name hashes schema and table, so each clause covers the others
M6 skip Watermark().Complete()	equivalent today (see CO-1 above)
M7 skip requireTableLock	TestGateCutoverRequiresTheTableLock (nil-pointer panic)
M8 skip the in-transaction Confirm	TestGateCutoverRequiresTheTableLock
M9 skip the source OID check	TestGateCutoverRefusesAReplacedSource
M10 skip the shadow OID check	survives: killed by non-blocking 4's test
M11 ignore invalid shadow indexes	TestGateCutoverRefusesAnInvalidShadowIndex
M12 skip the source fingerprint	TestGateCutoverRefusesASourceIndexAddedAfterTheBuild
M13 skip the shadow fingerprint	survives: killed by non-blocking 4's test
M14 skip the source fidelity check	TestGateCutoverRefusesASourceGrantAddedAfterTheBuild
M15 skip the shadow fidelity check	survives: killed by non-blocking 4's test
M16 skip the shadow identity defaults	survives
M17 skip identity sequence options	survives: killed by non-blocking 4's test
M18 skip the _old name check	TestGateCutoverRefusesWhenTheOldNameIsTaken
M19 skip the shadow constraint-name check	TestGateCutoverRefusesAShadowConstraintNamedAsAConstraintBackedSourceIndex
M20 _old names omit identity sequences	survives
M21 _old names omit statistics	survives
M22 drift ignores column_statistics_targets	TestFidelityDriftNamesExactlyTheFactsThatDiffer
M23 drift ignores grants	TestFidelityDriftNamesExactlyTheFactsThatDiffer, TestGateCutoverRefusesASourceGrantAddedAfterTheBuild
M24 pairing reuses a shadow partner	4 tests, incl. TestPairByDefinitionNeverReusesAShadowPartner
M25 index key drops opclasses	survives
M26 statistics key drops kinds	survives
M27 drop the paired-index exemption in constraintNamesTaken	survives, near-equivalent: a paired shadow constraint never already wears its source name
M28 owned sequences read deptype 'i'	TestGateCutoverPairsDependentsByDefinitionAndListsOwnedSequences
M29 build skips SET STATISTICS on the shadow	TestBuildShadowCarriesColumnStatisticsTargets
M30 inspection records the source's snapshot as the shadow's	survives
M31 build records the source's snapshot as the shadow's	survives (non-blocking 4)
M32 gate returns the raw error, not lockLossCause	survives
M33 gate transaction read-write	equivalent: nothing writes
M34 statistics target reader keeps defaults	TestBuildShadowCarriesColumnStatisticsTargets
M35 Invariant() drops CO-1 for cutover-unverified	survives

go build ./... passes on b54bd32. So does go test ./pkg/schemachange/ (full package, PostgreSQL 16).

This review was generated by Claude Code (claude-opus-5-5).

[aparajon]

🤖 2/2: OSS adoption and integration ease, at b54bd32. These are lenses, not correctness findings. 0 blocking, 4 non-blocking.

The shape is easy to adopt. A proof type with a private constructor, which the swap will accept and nothing else, is the right contract for a third-party orchestrator. The six causes fit the closed RefusalCause set, and the docs guard pins them. DependentPairing is plain data with JSON tags, so an importer can render it as it is. The known gap around retyped columns is stated up front, and the strict rule leaves the gap visible in Unpaired*.

1. Say why a dependent is unpaired. An importer that renders the pairing before cutover cannot tell the two cases apart. One is an index the statement dropped on purpose, like orders_note_idx when note is dropped. The other is an index pairing missed, like every index on a column retyped int → bigint until cs9-pair-type-change lands. The first is expected. The second loses the user's index names at the swap. Two ways to close it: a reason on each unpaired entry (for example, "a key column is gone from the shadow" versus "no definition matched"), or a swap that refuses an unpaired source index whose key columns all still exist on the shadow. Either one lets SchemaBot show "this index goes away" only when it does.

2. Route drift refusals on structure, not on Detail. refusal-classes.md says an importer "routes on the cause rather than on message text". But cutover-schema-drift and cutover-fidelity-drift each cover two cases with different remedies, and only Detail says which. If the source drifted, someone changed the live table and the change has to be re-planned. If the shadow drifted, something touched the engine's own table. The drifted fact names (grants, comment, and so on) also exist only as text. A Side and a Facts []string on RefusalError, or separate source and shadow causes, would let an orchestrator render the refusal without parsing it.

3. Let an orchestrator find fidelity drift while the copy runs, not at cutover. The remedy for cutover-fidelity-drift is a rebuild and a full recopy. So a routine GRANT or COMMENT ON an hour into a ten-hour copy costs the whole run, and nobody learns of it until the gate. That is fine as sequencing, since cs9-fidelity-carry is the real fix. But the gate cannot be polled early, because it needs a VerifiedShadow, which exists only after Check. An exported read-only drift check over just the BuiltShadow, run on the same snapshot comparison, would let SchemaBot surface "the source's grants changed, this run will need a rebuild" while the copy is still running. Until that lands, a line in the operator docs saying that metadata changes during a run force a rebuild would set expectations.

4. Give CutoverReady a display view. SchemaBot renders what the engine reports. The most useful pre-cutover preview would be the pairs, the unpaired dependents, the sequences to re-own, and which proofs were accepted. BuiltShadow has Proof() as a plain, encodable view that never decodes back. CutoverReady has only accessors, so each importer has to assemble its own report. A matching Report() (or MarshalJSON with no decoder) would keep the minting guarantee and give every orchestrator the same rendering.

This review was generated by Claude Code (claude-opus-5-5).

[aparajon]

🤖 Approving b54bd32 with 1 blocking finding: the gate matches the verified-shadow proof to the built shadow by name only. So a proof left over from a dropped shadow lets a rebuilt, never-copied shadow mint CutoverReady (CO-1). The fix is to carry both OIDs in VerifiedShadow and compare them in the gate. The 1/2 comment has the blocking finding with its test and six non-blocking findings. Three of the non-blocking findings come with tests: a source REVOKE survives the resume path, the pairing ignores NULLS NOT DISTINCT, and the shadow half of the checklist is unpinned. The 2/2 comment has four non-blocking integration notes.

This stamp was left by Claude Code (claude-opus-5-5).

[aparajon]

🤖 Delta re-review of 0134431 (since b54bd32): 1 blocking, 10 non-blocking.

The only new commit merges main, which brings in #140. That is a test-only change to the slot cleanup in pkg/preflight/copy_swap_shape_integration_test.go. The merge diff is identical to #140's own diff, and nothing outside that file changed, so the PR's code is the same as at b54bd32. The change does not touch the cutover gate or anything it reads, so there is no semantic conflict.

Blocking

1. The gate accepts a VerifiedShadow from an earlier shadow (CO-1): still open. cutover_gate.go:185, proofs.go:13-27

checkCutoverProofs still matches the proof to the built shadow by schema, table and shadow name only. So after a DropShadow and BuildShadow, the earlier proof still matches the rebuilt, empty shadow, and the gate mints CutoverReady for it. The fix is the same as before: carry SourceOID() and ShadowOID() in VerifiedShadow, and refuse in the gate when they differ from the built shadow's.

I re-ran the test from the 1/2 comment at 0134431 on PostgreSQL 16. It fails the same way:

--- FAIL: TestGateCutoverRefusesAVerifiedProofForAnEarlierShadow (0.12s)
    expected: "cutover-unverified"
    actual  : ""

With the earlier fix applied, it passes, and so does every TestGateCutover* test. Without the fix, all of the PR's own TestGateCutover* tests pass at this head.

Non-blocking

The six non-blocking findings in 1/2 and the four in 2/2 are unchanged. The tests for the source REVOKE on the resume path and for NULLS NOT DISTINCT pairing still fail the same way at 0134431.

This review was generated by Claude Code (claude-opus-5-5).

[aparajon]

🤖 Stamping with comments: 1 blocking (see the delta re-review above).

This stamp was left by Claude Code (claude-opus-5-5).

[Kiran01bm]

🤖 Adversarial review response — created by Kiran's code review agent (Amp, Claude Opus 4.6) — pull/137, follow-up commit

Verdict: the one blocking finding is fixed as proposed (the verified proof now carries both OIDs and the gate compares them); of the six correctness notes, five are taken in full and the sixth (extended statistics) is taken for the target and deferred for the namespace; the four integration-lens notes are tracked as plan rows, since each is an API shape the swap PR should settle. The internal review's REPEATABLE READ suggestion and its owned-sequence and doc findings are taken; its retyped-column pairing finding is already #138. Every surviving mutant the reviewer listed is now killed except M32, explained below.

#	Finding	Status	Explanation
C1-B1	A VerifiedShadow from an earlier shadow passes the gate for a rebuilt, never-copied shadow (CO-1)	✅ Fixed	checksum.VerifiedShadow now stores the source and shadow OIDs from copier.Shadow (SourceOID() / ShadowOID()), and checkCutoverProofs refuses cutover-unverified when either differs from the built proof's. TestGateCutoverRefusesAVerifiedProofForAnEarlierShadow is the reviewer's test: stage, DropShadow, BuildShadow again under the same derived name, gate the rebuild with the first pass's proof → refused. Removing the OID comparison fails it. policy_test.go pins the OIDs the constructor records.
C1-N1	On the resume path the gate holds each table only to itself, so a source REVOKE after the build reaches the swap	✅ Fixed	confirmFidelity now also compares the live source's table grants with the live shadow's (slices.Equal) and refuses cutover-fidelity-drift ("grants differ from source"). Grants are the one fact the gated statement cannot change, so the cross-check holds whether the proof came from the build or from InspectShadow. TestGateCutoverAfterInspectionRefusesASourceRevoke is the reviewer's sequence — GRANT … TO PUBLIC, build, REVOKE, InspectShadow (fingerprints equal to the build's, asserted), gate → refused. The InspectShadow godoc now says the caller compares the whole Proof, not the fingerprints. Column grants, policies, and comments are not cross-checked because ALTER TABLE legitimately changes them (a dropped column's grants vanish); those stay per-table against the record.
C1-N2	Index pairing ignores NULLS NOT DISTINCT	✅ Fixed	indexDefinition.NullsNotDistinct read as COALESCE((to_jsonb(i) ->> 'indnullsnotdistinct')::bool, false), so the query parses on PostgreSQL 14 where the column does not exist (run under PG_VERSION=14). TestGateCutoverPairsIndexesOnNullsNotDistinct creates z_nnd … NULLS NOT DISTINCT and a_plain on the same column — catalog order puts the plain one first, so the old key pairs them crosswise — and asserts each pair agrees on indnullsnotdistinct; skips below PG 15. Reading false in place of the column fails it.
C1-N3	Extended statistics lose their target and their schema	✅ Fixed (target) / ⏳ Deferred (schema)	New extended_statistics.go: FidelitySnapshot.ExtendedStatisticsTargets records each object's explicit stxstattarget (default −1 or NULL excluded), and the builder sets it on the shadow copy paired by definition via ALTER STATISTICS … SET STATISTICS, failing closed if a source object has no counterpart. TestBuildShadowCarriesExtendedStatisticsTargets shows two things: an ADD COLUMN statement carries every target, and ALTER COLUMN qty TYPE bigint leaves the object on (tenant, qty) at the default — because PostgreSQL's own rebuild of a statistics object on a retyped column resets its target (confirmed on postgres:16: stxstattarget 500 → −1 after the ALTER TYPE). The shadow reproduces what a direct ALTER TABLE would do, which is the fidelity the design asks for. The proof round-trip covers the new key paths. The namespace half — an object outside its table's schema, which LIKE relocates and takenNames does not look for — is plan row cs9-statistics-namespace; it needs the swap's rename to follow stxnamespace too, so it belongs with #139.
C1-N4	The shadow half of the checklist is unpinned (M10, M13, M15, M17, M31)	✅ Fixed	TestGateCutoverHoldsTheShadowAndTheIdentitiesToTheBuildRecord: the shadow renamed away and recreated (cutover-relation-replaced), ADD COLUMN on the shadow (cutover-schema-drift), COMMENT ON the shadow (cutover-fidelity-drift), ALTER COLUMN id SET INCREMENT BY 5 on the source (cutover-fidelity-drift); and the pass case the reviewer asked for, SET (fillfactor = 50) as the gated statement, asserting the shadow's record reads fillfactor=50 while the source's is empty and the gate passes. Recording the source's snapshot as the shadow's (M31) fails that subtest.
C1-N5	Smaller unpinned branches (M20, M21, M25, M26, M32, M35) and the hand-kept drift list	✅ Fixed (all but M32)	M20/M21: TestGateCutoverRefusesWhenADependentOrSequenceOldNameIsTaken puts a leftover on the statistics object's and on the identity sequence's _old name. M25: TestGateCutoverPairsIndexesOnOperatorClass (text_pattern_ops vs default, catalog order reversed). M26: TestGateCutoverPairsStatisticsOnKinds (ndistinct vs dependencies on the same columns). M35: TestEveryRefusalCauseNamesItsInvariant now pins every cause to its invariant, not just membership in the set. fidelityDrift derives its facts from FidelitySnapshot's fields by reflection on the JSON tags, and TestFidelityDriftCoversEveryFactOfTheSnapshot fills every field and expects every tag back in order, asserting the count equals the field count — a field added later without a tag or skipped by the loop fails it. M32 stays unpinned: the gate takes no relation lock and runs only catalog reads, so no statement in it can park behind another session's lock the way DROP TABLE does in TestDropShadowAbortsWhenTheLockIsLostMidDrop; the only loss path is the Bind context cancelling a read mid-flight, which has no deterministic hook from a test. lockLossCause is the same wrapper the build and drop use, where it is pinned.
C1-N6	CO-1's registry entry still says cutover is planned; ST-5's text promises the shadow carries the source's metadata	✅ Fixed	CO-1 now says GateCutover accepts only a VerifiedShadow minted for the relations it is gating, by name and OID. ST-5's rule text says each table is held to the build's record, and the two tables' grants to each other; the enforced paragraph adds extended statistics targets and the grants cross-check. refusal-classes.md cutover-unverified and cutover-fidelity-drift rows updated to match; the docs guard passes.
C2-F1	Say why a dependent is unpaired	⏳ Deferred	Agreed, and the right time is when the relaxed pass lands: #138 is the PR that creates a second reason ("paired by the relaxed rule" vs "no counterpart"), and DependentPairing should grow the reason field there rather than here with one value. Tracked on the 9a-ii row.
C2-F2	Route drift refusals on structure (which side, which facts), not on Detail	⏳ Deferred	Agreed. Tracked as cs9-cutover-ready-report alongside F4: a typed Side and Facts on RefusalError is an error-taxonomy change that the SchemaBot adapter will consume, so it should be designed once with the swap's own refusals (cutover-swap-mismatch, cutover-outcome-ambiguous in #139) in view rather than for two causes now.
C2-F3	Let an orchestrator find fidelity drift while the copy runs	⏳ Deferred	Tracked on the existing cs9-fidelity-carry row, which already covers re-synchronising additive drift under the lock; an exported mid-copy drift check is its natural companion. Not taken here because the gate's transaction shape (one read-only tx under the lock session) is what an exported check would reuse, and that shape is still being settled by #139.
C2-F4	Give CutoverReady a display view	⏳ Deferred	Tracked as cs9-cutover-ready-report. BuiltShadow.Proof() is the model; CutoverReady.Report() should carry the pairs, the unpaired dependents with their reason (F1), the sequences, and the accepted proofs' identities — which means it lands after #138 settles the reason field.
I-B2 (internal review)	Indexes on a retyped column never pair, the primary key included	⏳ Already #138	Same finding as the PR body's "known gap"; cs9-pair-type-change is out as #138 stacked on this PR.
I-N1 (internal review)	Owned sequences of a column the statement dropped are still listed for the handoff	✅ Fixed	keptSequences(sequences, targetModel) filters readOwnedSequences's result against the shadow's columns, the way handoffIdentities already does. TestGateCutoverListsOnlyTheOwnedSequencesOfKeptColumns drops a bigserial legacy_id and asserts only orders_id_seq is listed; TestKeptSequencesFollowTheShadowsColumns pins order and the no-drop case. Passing the unfiltered list fails the integration test.
I-N2 (internal review)	FidelitySnapshot doc still promises a shadow-vs-source match	✅ Fixed	Type doc rewritten: one snapshot per table, the gate compares each to its record, and cross-compares only facts the statement cannot change.
I-N3 (internal review)	Extended-statistics targets not carried or checked	✅ Fixed	See C1-N3.
I-S1 (internal review)	Run the gate transaction at REPEATABLE READ	✅ Fixed	gateCutover begins pgx.TxOptions{IsoLevel: pgx.RepeatableRead, AccessMode: pgx.ReadOnly}; the godoc says every catalog read sees one snapshot, so the pairing and names the proof carries describe the instant the fingerprints were checked. No test: a committed index between two reads inside one transaction has no deterministic hook, and the property is the server's.

Tests added or changed in the follow-up commit: TestGateCutoverRefusesAVerifiedProofForAnEarlierShadow, TestGateCutoverAfterInspectionRefusesASourceRevoke, TestGateCutoverHoldsTheShadowAndTheIdentitiesToTheBuildRecord (five subtests), TestGateCutoverPairsIndexesOnNullsNotDistinct, TestGateCutoverPairsIndexesOnOperatorClass, TestGateCutoverPairsStatisticsOnKinds, TestGateCutoverRefusesWhenADependentOrSequenceOldNameIsTaken, TestGateCutoverListsOnlyTheOwnedSequencesOfKeptColumns (new, cutover_gate_integration_test.go); TestBuildShadowCarriesExtendedStatisticsTargets (new, extended_statistics_integration_test.go); TestFidelityDriftCoversEveryFactOfTheSnapshot, TestKeptSequencesFollowTheShadowsColumns (new unit); TestEveryRefusalCauseNamesItsInvariant tightened to a per-cause mapping; TestBuiltShadowJSONRoundTripsAsTheProofInspectionRederives fixture gains a tuned statistics object on untouched columns; policy_test.go pins the OIDs on VerifiedShadow. Mutation-checked by reverting each change in turn: the OID comparison, the grants cross-check, the indnullsnotdistinct read, keptSequences, the carryExtendedStatisticsTargets call, the shadow snapshot recorded as the source's (M31), a truncated reflection loop, and the CO-1 mapping each fail exactly their test. Full pkg/schemachange and pkg/checksum suites pass with -race on PG 16; the gate, build, and proof tests pass on PG 14 and the extended-statistics tests on PG 17 (nullable stxstattarget); golangci-lint clean.

Source: block/pg-sprite#137, review comments 5926992729 and 5926994796 at head b54bd32; internal review scratch/code-reviews/pg-sprite-pr137-review-b54bd325.md items I-B1 to I-S1 (I-B1 = C1-B1) at b54bd32. Branch head before this commit: 0134431 (fast-forward of the main merge that brought #140).