fix(SCHEMA_VERSION 5->6): one-time billable_requests backfill, plus a real destructive-wipe landmine

Closes the boot-time double/under-billing bug in busbarAI core's
governance::state::hydrate_budgets: that function used a value-based
heuristic (billable_requests==0 && requests>0 => "legacy pre-split
row, re-seed it") to detect data needing a one-time backfill - but
that exact counter shape is ALSO what a bucket looks like when every
request in the window was legitimately refunded (refund_bucket
decrements billable_requests but never requests, by design). The
heuristic ran on every boot and could not tell the two cases apart,
silently re-billing correctly-refunded fees on restart.

Fix: do the backfill ONCE, durably, gated on this store's own
schema-version crossing (which only ever happens once, ever, in a
store's lifetime) instead of on every boot. This is safe as an
unconditional blanket UPDATE only because 1.5.0/SCHEMA_VERSION 5 has
never shipped to a real customer - there is no genuine "currently,
legitimately refunded to zero" row in existence yet that this could
incorrectly re-bill. After this lands in busbarAI core, hydrate_budgets
drops the heuristic entirely and trusts billable_requests
unconditionally, forever - v6+ rows are written correctly from birth.

Found and fixed a second, more serious pre-existing bug while
implementing this: migrate()'s legacy-drop detection checked for
'keys'/'store_meta' among its "needs a destructive wipe" signals -
but those are the CURRENT (v5+) table names, not just old ones, so a
real v5 database legitimately has both. Naively bumping SCHEMA_VERSION
to 6 under the existing `version < SCHEMA_VERSION` gate would have
made every real v5 store's OWN live data register as "legacy" on the
v5->v6 crossing and wiped it - the exact opposite of what this backfill
is supposed to do. Fixed by gating the destructive path on the actual
pre-v5 boundary (`version < 5`, the boundary that was ALWAYS destructive
by design) instead of on SCHEMA_VERSION (which moves every bump), and
gating the new additive backfill separately on `version < 6`.

Both fixes proven red-before-green by hand: reverting the has_legacy
gate back to `< SCHEMA_VERSION` reproduces real data loss in
migrate_v5_to_v6_backfills_billable_requests_without_wiping_data;
reverting has_legacy's table list to exclude 'keys'/'store_meta'
breaks the PRE-EXISTING migrate_drops_and_recreates_a_genuinely_older_schema
test (a real, older schema no longer gets detected/dropped).

Full gate: fmt clean, clippy clean, cargo test --workspace (49 lib +
3 e2e) all green.

Author: MattJackson

store-sqlite/src/lib.rs

@@ -39,9 +39,15 @@ impl<T> IntoStoreResult<T> for Result<T, rusqlite::Error> {
 /// retention sweep); `usage_metering` leads with `bucket` for the same write-locality reasoning, and
 /// gains `key_group_at_use`/`pricing_version`/`billable_requests`, and renames
 /// `tokens_cache_creation` -> `tokens_cache_write` to match `TierTokens`'s own naming (a drift fixed
-/// core-side in this same redesign). 1.5.0 is UNRELEASED, so each bump is destructive (drop +
-/// recreate), never a migration: a pre-v5 dev database is recreated empty on open.
-const SCHEMA_VERSION: i64 = 5;
+/// core-side in this same redesign). 1.5.0 is UNRELEASED, so each bump up to and including v5 was
+/// destructive (drop + recreate), never a migration: a pre-v5 dev database was recreated empty on
+/// open.
+///
+/// v6: the FIRST real, additive (non-destructive) migration this store has ever needed — a
+/// one-time backfill of `billable_requests` for any row where a v5-era write left it at 0 despite
+/// a nonzero `requests` (see the `version < 6` block in `migrate`, and
+/// `governance::state::hydrate_budgets` in busbarAI core for the boot-time bug this closes).
+const SCHEMA_VERSION: i64 = 6;
 
 const SCHEMA: &str = "
 CREATE TABLE IF NOT EXISTS store_meta (
@@ -387,11 +393,25 @@ impl SqliteStore {
         let tx = conn
             .transaction_with_behavior(TransactionBehavior::Immediate)
             .store()?;
-        if version < SCHEMA_VERSION {
+        // Gated on the actual pre-v5 boundary (`< 5`), NOT on `SCHEMA_VERSION` (which moves every
+        // bump): every bump up to and including v5 was destructive by design (1.5.0 was
+        // unreleased, so a pre-v5 dev database is simply wiped and recreated). v6+ are ADDITIVE,
+        // non-destructive migrations (see the `version < 6` backfill just below) — they must
+        // never fall into this drop-and-recreate path, even though `keys`/`store_meta` are named
+        // in the drop list below (the list has to cover every table this schema has EVER used,
+        // including its OWN current names, since a version<5 db could theoretically already carry
+        // a same-named-but-incompatibly-shaped table from an even older generation — see
+        // `migrate_drops_and_recreates_a_genuinely_older_schema`). Gating on `< SCHEMA_VERSION`
+        // instead of `< 5` here was tried first and is WRONG: it makes a real v5 database's own
+        // (correctly-shaped, live) `keys`/`store_meta` tables register as "has_legacy" on the
+        // v5->v6 crossing and wipes them — confirmed by hand, reverting this gate to
+        // `< SCHEMA_VERSION` reproduces exactly that data loss in
+        // `migrate_v5_to_v6_backfills_billable_requests_without_wiping_data`.
+        if version < 5 {
             let has_legacy: bool = tx
                 .query_row(
                     "SELECT EXISTS(SELECT 1 FROM sqlite_master WHERE type='table' AND name IN \
-                     ('usage_counters','virtual_keys','keys','store_meta'))",
+                     ('usage_counters','virtual_keys','aws_credentials','usage_ledger','keys','store_meta'))",
                     [],
                     |r| r.get(0),
                 )
@@ -424,6 +444,35 @@ impl SqliteStore {
             [],
         )
         .store()?;
+        // v5 -> v6, ONE-TIME durable backfill (never repeated, gated on the version crossing —
+        // see governance::state::hydrate_budgets in busbarAI core for the bug this closes). A
+        // pre-v6 row can have `billable_requests=0` for either of two reasons that look
+        // IDENTICAL in the data: (a) it was written by v5 code that never split billable_requests
+        // out from requests (a real legacy gap — v5 added the column but not every code path
+        // populated it correctly from day one), or (b) every request in that window was
+        // legitimately refunded (refund_bucket decrements billable_requests but never requests,
+        // by design). Those two cases are NOT distinguishable from the stored values alone, which
+        // is exactly why this must NOT be a per-boot heuristic (hydrate_budgets no longer applies
+        // one after this migration ships) — it is safe to run this AS A BLANKET, UNCONDITIONAL
+        // backfill exactly once, right now, only because 1.5.0 has never shipped to a real
+        // customer: there is no genuine "currently, legitimately refunded to zero" row in
+        // existence yet that this could incorrectly re-bill. Re-running this same UPDATE
+        // unconditionally at ANY later point (once real refund data exists) would reintroduce the
+        // exact bug it closes — that is why it is gated on `version < 6`, never repeated.
+        if version < 6 {
+            tx.execute(
+                "UPDATE usage_windows SET billable_requests = requests \
+                 WHERE model = '' AND billable_requests = 0 AND requests > 0",
+                [],
+            )
+            .store()?;
+            tx.execute(
+                "UPDATE usage_metering SET billable_requests = requests \
+                 WHERE billable_requests = 0 AND requests > 0",
+                [],
+            )
+            .store()?;
+        }
         tx.pragma_update(None, "user_version", SCHEMA_VERSION)
             .store()?;
         tx.commit().store()?;

store-sqlite/src/tests.rs

@@ -929,6 +929,82 @@ fn migrate_drops_and_recreates_a_genuinely_older_schema() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+#[test]
+fn migrate_v5_to_v6_backfills_billable_requests_without_wiping_data() {
+    // The v5->v6 crossing is the FIRST non-destructive migration this store has ever needed — a
+    // real regression risk `migrate_rerun_at_current_schema_version_does_not_wipe_data`'s own
+    // comment already flags but doesn't itself cover: the legacy-drop block's table-name list
+    // ('keys','store_meta', etc) includes the CURRENT schema's own names, so a naive
+    // `version < SCHEMA_VERSION` bump (5 < 6) would find a real v5 database's OWN 'keys' table and
+    // wipe it, unless the has_legacy check is scoped to pre-v5-ONLY names. Hand-build a real v5
+    // database (current table shapes, PRAGMA user_version=5) with a live key AND a usage_windows
+    // row shaped exactly like the boot-time bug this migration exists to close (billable_requests
+    // stuck at 0 with a real nonzero requests count), then open it through the real store
+    // (triggering migrate()) and assert BOTH that the key survived AND the row was backfilled.
+    let dir = tempdir();
+    let file = dir.join("v5.db");
+    {
+        let conn = Connection::open(&file).unwrap();
+        conn.execute_batch(SCHEMA).unwrap();
+        conn.execute(
+            "INSERT INTO keys (id, name, key_group, allowed_pools, labels, enabled, \
+             generation_hash, created_at, updated_at, expires_at, deleted_at, revision) \
+             VALUES ('vk_v5', 'n', NULL, NULL, '{}', 1, 'g1', 0, 0, NULL, NULL, 0)",
+            [],
+        )
+        .unwrap();
+        conn.execute(
+            "INSERT INTO usage_windows (window_start, bucket_id, model, requests, billable_requests) \
+             VALUES (100, 'vk_v5', '', 7, 0)",
+            [],
+        )
+        .unwrap();
+        conn.pragma_update(None, "user_version", 5i64).unwrap();
+    }
+    let s = SqliteStore::open(file.to_str().unwrap(), 5000)
+        .expect("open() must migrate a real v5 database additively, not fail or wipe it");
+    assert!(
+        s.get_key("vk_v5").unwrap().is_some(),
+        "a real v5 key must survive the v5->v6 migration, not be wiped by the legacy-drop path"
+    );
+    let ledger = s.get_usage("vk_v5", 100).unwrap();
+    assert_eq!(ledger.requests, 7, "requests must be untouched");
+    assert_eq!(
+        ledger.billable_requests, 7,
+        "a v5-era row stuck at billable_requests=0 with real requests must be backfilled exactly \
+         once during the v5->v6 crossing"
+    );
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
+#[test]
+fn migrate_v5_to_v6_does_not_touch_an_already_nonzero_billable_requests_row() {
+    // The backfill's WHERE clause (`billable_requests = 0 AND requests > 0`) must not touch a row
+    // that already carries a real, independently-tracked billable_requests value — only the
+    // ambiguous zero-with-nonzero-requests shape is a backfill candidate.
+    let dir = tempdir();
+    let file = dir.join("v5_ok.db");
+    {
+        let conn = Connection::open(&file).unwrap();
+        conn.execute_batch(SCHEMA).unwrap();
+        conn.execute(
+            "INSERT INTO usage_windows (window_start, bucket_id, model, requests, billable_requests) \
+             VALUES (200, 'vk_v5b', '', 10, 3)",
+            [],
+        )
+        .unwrap();
+        conn.pragma_update(None, "user_version", 5i64).unwrap();
+    }
+    let s = SqliteStore::open(file.to_str().unwrap(), 5000).unwrap();
+    let ledger = s.get_usage("vk_v5b", 200).unwrap();
+    assert_eq!(ledger.requests, 10);
+    assert_eq!(
+        ledger.billable_requests, 3,
+        "a row with a real, already-nonzero billable_requests must be left exactly as-is"
+    );
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 #[test]
 fn secret_form_from_str_round_trips_every_named_form() {
     assert_eq!(secret_form_from_str("recoverable"), SecretForm::Recoverable);