fix(db): make schema-terminal assumptions version-agnostic - #2177
fix(db): make schema-terminal assumptions version-agnostic#2177ohdearquant wants to merge 4 commits into
Conversation
Several sites assume V21 is the last migration, which breaks the moment a V22 lands: - The blob-GC cutover fence required MAX(version) = 21 in its ledger predicate, so any later migration would silently disarm the completed- cutover fast path. Later migrations record on top of a completed cutover without disturbing the fencing set, so the predicate is now >= 21; the physical facts the reject matrix removes one at a time carry the safety. - attachment_cutover_status treated a complete marker beside a ledger above V21 as an impossible pair; only a ledger BELOW V21 is one. - validate_schema_is_current exempted the cutover-state check on any ledger version other than exactly 21, so a later version would skip physical cutover validation entirely. - Terminal-version test assertions pinned the literal 21 (or 22 for the ahead-of-build fixture). They now derive from the migration chain via a new migrations::latest_schema_version(), so the fixtures stay correct when a migration is added instead of decaying into wrong claims. No behavior changes at the current schema head; every predicate evaluates identically while V21 is the latest migration.
ohdearquant
left a comment
ohdearquant
left a comment
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge by itself.
Verdict on head 3ecd3ed: REQUEST-CHANGES, 1 blocking finding. Finding details are delivered to the review's recipients rather than posted here. Do not merge this head while blocking findings are outstanding; a pipeline comment on a newer head supersedes this one.
…ob GC The destructive blob-GC gate accepted any ledger with MAX(version) >= 21, so an older binary could enter the deletion path against a database migrated by a newer build — a schema epoch whose attachment/liveness semantics the older build never validated. ADR-160 requires that epoch to fail closed. The ledger predicate is now MAX(version) BETWEEN 21 AND latest_schema_version(): migrations this binary applied on top of the completed cutover keep the gate open, while an ahead-of-binary ledger refuses. New regression gate_rejects_ledger_ahead_of_binary_latest ADDS a future ledger row to a passing fixture and requires refusal, with the untouched fixture as positive control; under the old >= 21 predicate the same fixture passes, so the test is pinned to the fix. Also renames rejects_pre_consolidation_ledger to rejects_ledger_ahead_of_binary_latest — the fixture is computed from the live chain and exercises any ahead ledger, not the historical pre-consolidation shape its old name described.
ohdearquant
left a comment
ohdearquant
left a comment
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge by itself.
Verdict on head 94ea178: REQUEST-CHANGES, 1 blocking finding. Finding details are delivered to the review's recipients rather than posted here. Do not merge this head while blocking findings are outstanding; a pipeline comment on a newer head supersedes this one.
ADR-160 scopes Phase-4a sweep admission to an exact completed V21 epoch: any ledger above V21 — ahead of this binary or applied by it — belongs to a schema epoch whose attachment/liveness semantics the gate never validated, and fails closed with the existing typed refusal. A future migration's author extends the gate in the change that proves the new epoch's liveness set, never by default. General schema validation keeps accepting later versions on top of a completed cutover; the exact-epoch rule binds destructive GC admission only. Also folds the two remaining independent terminal-version derivations onto latest_schema_version().
ohdearquant
left a comment
ohdearquant
left a comment
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge by itself.
Verdict on head 389435c: REQUEST-CHANGES, 1 blocking finding. Finding details are delivered to the review's recipients rather than posted here. Do not merge this head while blocking findings are outstanding; a pipeline comment on a newer head supersedes this one.
… gate
The completed-V21 predicate checked only for the named V21 row and
MAX(version) = 21, so a ledger with its below-V21 rows deleted — an
incomplete migration history whose physical schema the gate never
validated — still opened destructive GC. The predicate now also requires
COUNT(*) = 21 and MIN(version) = 1: with version as the table's PRIMARY
KEY those three facts hold iff the ledger is exactly the contiguous set
{1..21}. Name-level canonicality of below-terminal rows remains boot's
job (validate_applied_migration_ledger); the gate enforces the
structural contiguity destructive admission rests on.
Tests: a dedicated regression deletes every row below V21 while
asserting the named V21 row and MAX(version) = 21 survive, so its
rejection can only come from the contiguity clause (removing the clause
turns it and the new acceptance-matrix case red — verified by temporary
mutation, then restored by snapshot). The acceptance matrix gains the
below-V21-deletion case, its stale BETWEEN-based comment now describes
the shipped predicate, the shared V21 fixture asserts the pinned V21
epoch instead of the moving latest version (a future migration fails
loudly at the fixture instead of silently invalidating GC tests), and
the ahead-of-V21 fixture row now matches the shape run_migrations
records.
ohdearquant
left a comment
ohdearquant
left a comment
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge.
Verdict on current head: APPROVE, zero blocking findings. This is a comment, not an approval — a human reviewer decides whether to approve and merge.
1 participant
Problem
Several sites assume V21 (
attachments_first_class) is the last migration in the chain. Each breaks the moment a V22 lands:MAX(version) = 21in its ledger predicate, so any later migration silently disarms the completed-cutover fast path.attachment_cutover_statustreated acompletemarker beside a ledger above V21 as an impossible pair; only a ledger below V21 is one — later migrations record on top of a completed cutover in the normal course.validate_schema_is_currentran the physical cutover-state check only when the ledger was exactly 21, so a later version would skip cutover validation entirely.Change
>= 21; the physical facts the reject matrix removes one at a time continue to carry the safety, and the accept-path tests running at the latest schema are the positive control for the semantics.migrations::latest_schema_version()derives the terminal version from the migration chain; terminal-version assertions and the ahead-of-build fixture use it instead of literals, so they stay correct when a migration is added.No behavior changes at the current schema head: every predicate evaluates identically while V21 is the latest migration.
Testing
cargo test -p khive-db— all suites green (lib 817 passed).cargo test -p kkernel— all suites green (lib 444 passed, integration suites included).cargo fmt --checkandcargo clippy --all-targets --all-features -p khive-db -p kkernelclean.