SIGN IN SIGN UP

fix(db): an older binary must not reap a newer binary's migration history

Greptile, on PR #631. The finding is real and the mechanism is exactly as
described.

`reap_orphans` deleted every ledger row whose version was absent from the
canonical list:

    DELETE FROM audit.schema_migrations WHERE version <> ALL($1)

A version written by a NEWER binary is absent from an older binary's list in
precisely the way a retired slot is. So an older `agentsfleetd` image rolled
onto an upgraded database would delete the newer deployment's migration
history, then apply its own schema on top of one it does not understand —
leaving the schema split between two versions with no ledger left to say how it
got there.

The safe policy already existed and production did not use it.
`AheadPolicy::Refuse` carries a doc comment naming this exact hazard — "applying
anything to it is how a schema gets torn in half" — but `Migrator::new()`, which
is what `agentsfleetd migrate` calls, selected `Reap`, and `refusing_newer()`
had no caller outside tests.

**The discriminator is the ceiling**, as the review suggested: the highest
canonical version this binary knows.

  - BELOW it and absent — a slot the canonical list retired. The pre-v2.0
    teardown left rows for slots that no longer exist, and a migrate run is
    where they go. Reaping stays correct, and still happens.
  - ABOVE it — a version this binary has never heard of AND higher than
    anything it knows, so something newer wrote it. Refuse, having changed
    nothing.

The refusal is UNCONDITIONAL and runs before the reap. It is not something a
policy opts into, because destroying another deployment's history is not a
tidy-up: `AheadPolicy` now chooses only whether a RETIRED slot is reaped or
refused, and is renamed `ReapRetired` to say so. `reap_orphans` also carries
`AND version <= $2` in the statement itself — the same invariant expressed
where the deletion happens, so a future caller that forgets the check cannot
destroy history with it.

Two tests, and each asserts BOTH halves. A run that refused AFTER reaping would
pass an exit-code check and still have corrupted the ledger, so the assertions
are (1) the run is refused, naming the version, and (2) the newer row is still
there afterwards. `test_a_newer_failure_row_also_refuses_the_run` covers the
second bookkeeping table, because a database whose newer migration FAILED is
the worst moment to overwrite — it is already half-migrated.

Both were confirmed to FAIL against the unfixed code before being kept; the
first reports `reaped: 1`, which is the corruption itself.

Existing behaviour is unchanged: `test_migrate_reaps_orphaned_bookkeeping_rows`
still reaps versions 42 and 43, both far below the canonical floor of 100, and
every `with_migrations` fixture uses versions at or above 9001 — above the
canonical ceiling of 890 — so no test's ceiling moved.

Verified: `make lint-rustd`, `make harness-verify`, and
`make test-coverage-rustd` at 615 tests / 100.00% lines (4378/4378).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
K
Kishore Kumar committed
0dd545fb628bb2a7b83d4faee75bd384fc02d127
Parent: 0cc7e32