Dead code: migrate_remove_token_columns can never run — legacy DBs abort earlier on a sqlx checksum mismatch

[grunch]

Summary

migrate_remove_token_columns in src/db.rs exists to drop the deprecated disputes.buyer_token / disputes.seller_token columns from legacy databases. That code path can never run: every database old enough to still have those columns aborts earlier, in sqlx's migration checksum validation, and connect() has no recovery for that error class.

Found while reviewing #848, which cleans up the same function. #848 correctly removes the SQLite-version fallback inside it; this issue is about the branch that survives.

Context — why the path is unreachable

1. The columns were introduced by editing an already-shipped migration file in place: 41f182c (additions to have tokens for dispute #346) added buyer_token integer not null and seller_token integer not null to migrations/20230928145530_disputes.sql.
2. ab338c5 (refactor: remove deprecated token columns from disputes table #516) removed them from that same file, again in place, and added migrate_remove_token_columns to drop them from existing databases at startup.
3. sqlx records a checksum of each migration file's bytes in _sqlx_migrations. Migrator::run calls validate_applied_migrations and returns MigrateError::VersionMismatch(version) when the recorded checksum no longer matches the file on disk — see sqlx-core-0.9.0/src/migrate/migrator.rs:255 and :274.
4. In src/db.rs::connect(), migrator.run(&conn) runs before migrate_remove_token_columns(&conn). Its error handling only recovers from duplicate column name failures, via parse_duplicate_column_name → reconcile_existing_add_column_migration. VersionMismatch falls through to the else branch and returns Err.

So a pre-#516 database — the only kind that still has the token columns — fails at step 4 with VersionMismatch(20230928145530) and mostrod refuses to start. migrate_remove_token_columns is never called. The columns are never dropped.

The consequence cuts both ways:

• Nobody can be running a pre-refactor: remove deprecated token columns from disputes table #516 database today, because it would not boot. The drop path has therefore never executed in production.
• If someone does still have such a database on disk, this function is not what unblocks them; the checksum mismatch is.

What to do

Pick one — they are mutually exclusive, and the choice is a maintainer call about whether pre-#516 databases are still in scope:

Option A (recommended) — delete the function

Treat pre-#516 databases as unsupported. That is already the de-facto state, since they cannot start.

• Delete migrate_remove_token_columns from src/db.rs.
• Delete both of its call sites in connect() — the one in the new-database branch (which is a guaranteed no-op: a freshly created database runs the current migration files, which have no token columns) and the one in the existing-database branch.
• Delete the three tests: migrate_remove_token_columns_is_noop_without_token_columns, migrate_remove_token_columns_drops_legacy_columns_and_keeps_rows, migrate_remove_token_columns_handles_single_legacy_column.
• Keep table_column_exists — reconcile_existing_add_column_migration still uses it.
• Check whether insert_dispute / HEX_KEY_A become unused in the test module after the deletions; cargo clippy --all-targets -- -D warnings will say.

Option B — actually support pre-#516 databases

Only worth it if we know such deployments exist and we want them to upgrade in place. This is strictly more work and adds a permanent maintenance burden.

• Extend the error handling in connect() to recognise MigrateError::VersionMismatch(20230928145530) specifically, alongside the existing duplicate column name reconciliation.
• On that specific mismatch, verify the on-disk disputes table is one of the two known legacy shapes (with or without the token columns), then rewrite the stored checksum in _sqlx_migrations to the current file's checksum and re-run the migrator — mirroring what reconcile_existing_add_column_migration does today.
• Only then does migrate_remove_token_columns become reachable and worth keeping.
• Do not blanket-accept every VersionMismatch: that would silently paper over any future in-place edit of a shipped migration, which is exactly the bug that created this situation.

The underlying lesson

Both #346 and #516 edited a migration that had already shipped. That is what broke checksum validation for those databases. Worth stating explicitly in AGENTS.md / CONTRIBUTING: never modify a migration file once it has been released — add a new one. Consider that a sub-task of whichever option is chosen.

How to verify the claim

git show 41f182c -- migrations/20230928145530_disputes.sql   # columns added in place
git show ab338c5 -- migrations/20230928145530_disputes.sql   # columns removed in place
grep -n "validate_applied_migrations\|VersionMismatch" ~/.cargo/registry/src/*/sqlx-core-0.9.0/src/migrate/migrator.rs

Then read src/db.rs::connect() and follow the error arms of migrator.run(&conn).

Acceptance criteria

• A decision is recorded on the issue: pre-refactor: remove deprecated token columns from disputes table #516 databases are supported (Option B) or not (Option A).
• The unreachable code is either removed or made reachable — not left as-is.
• cargo test --all, cargo clippy --all-targets -- -D warnings and cargo fmt --check are clean.
• If Option A: no remaining reference to buyer_token / seller_token outside git history.

Severity: Medium (dead code, no user-visible bug today). Pre-existing — not introduced by #848.