Skip to content

miner(governor-state): the forge-scope table rebuild bypasses the transactional schema-migration convention #10343

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

packages/loopover-miner/lib/schema-version.ts's applySchemaMigrations is this package's shared
convention for evolving a SQLite store's schema safely: each migration runs inside its own
db.exec("BEGIN") / migration(db) / PRAGMA user_version = ... / db.exec("COMMIT") block, with
ROLLBACK on any thrown error — so a crash mid-migration leaves the file at the LAST fully-applied
version and resumes cleanly on the next open, per the function's own comment. Every other repo-
scoped store in this package that needs a post-creation schema change
(claim-ledger.ts's addApiBaseUrlScope, run-state.ts's addApiBaseUrlScope,
event-ledger.ts's addTenantIdColumn, policy-doc-cache.ts, policy-verdict-cache.ts,
governor-ledger.ts, prediction-ledger.ts) routes it through applySchemaMigrations.

packages/loopover-miner/lib/governor-state.ts never imports or calls applySchemaMigrations at
all. Instead, openGovernorState calls ensurePauseColumns(db) and
ensureReputationHistoryForgeScope(db) (around lines 265-274) as plain, un-transacted function
calls outside any migration framework. ensureReputationHistoryForgeScope (lines 217-242) performs
the same "SQLite can't ALTER a PRIMARY KEY, so rebuild the table" pattern as claim-ledger.ts's and
run-state.ts's addApiBaseUrlScope migrations (whose own comments cite the identical reasoning),
but as four separate, un-wrapped, autocommitted statements:

db.exec(`CREATE TABLE governor_reputation_history_v2 (...)`);
db.prepare(`INSERT OR IGNORE INTO governor_reputation_history_v2 (...) SELECT ... FROM governor_reputation_history`).run(...);
db.exec("DROP TABLE governor_reputation_history");
db.exec("ALTER TABLE governor_reputation_history_v2 RENAME TO governor_reputation_history");

If the process is killed between the DROP TABLE and the RENAME statements, the file is left
with NO governor_reputation_history table but a populated governor_reputation_history_v2. On
the next openGovernorState call, the un-conditional CREATE TABLE IF NOT EXISTS governor_reputation_history (lines ~267-273) silently recreates an EMPTY, old-shape table
(masking that the real data now sits, orphaned, under _v2); ensureReputationHistoryForgeScope
then sees the new empty table lacks api_base_url and tries CREATE TABLE governor_reputation_history_v2 again — which throws (the table already exists from the
interrupted run, and this statement has no IF NOT EXISTS). This makes openGovernorState throw
unconditionally on every subsequent call against that file — a permanently broken store requiring
manual database surgery — and the interim reputation-decay history is silently orphaned in the
unreferenced _v2 table.

Requirements

  • Route governor-state.ts's post-baseline schema evolution through applySchemaMigrations, the
    same way every other repo-scoped store in this package does. Register ensurePauseColumns and
    ensureReputationHistoryForgeScope (or their equivalents) as SchemaMigration functions passed
    to applySchemaMigrations(db, [...]), so both get the same BEGIN/COMMIT/ROLLBACK
    atomicity and resumable-versioning guarantee every sibling store already has.
  • If either migration needs adjustment to be idempotent/resumable under the applySchemaMigrations
    contract (e.g. handling a partially-completed _v2 table left over from a PRE-FIX file that
    already hit the crash scenario described above), handle that adjustment as part of this same fix
    — a store that already has an orphaned _v2 table from before this fix must still be recoverable
    by the fixed code, not just newly-safe going forward.
  • Preserve the exact resulting schema (table/column names, data) for a store that migrates cleanly
    — this issue is about transactional safety, not changing the target schema.

Deliverables

  • governor-state.ts routes its schema evolution through applySchemaMigrations, matching
    every sibling store in this package.
  • A new regression test simulating a crash between the DROP TABLE and RENAME steps (e.g. by
    injecting a throwing statement at that point, or by manually constructing a database file
    already in that intermediate state) and asserting that openGovernorState either recovers
    cleanly or fails in a way that does NOT leave the store permanently unopenable on a
    subsequent retry.
  • A new regression test asserting that a full crash-then-retry sequence does not silently lose
    the reputation-history data that was present before the interrupted migration (i.e. the data
    that ended up in the orphaned _v2 table, if the fix's recovery path is triggered, is not
    simply discarded).

All deliverables are required in this one PR.

Test Coverage Requirements

This repo's Codecov patch gate is 99%+ (branch-counted) on packages/loopover-miner/lib/**. Add
the new regression tests to test/unit/miner-governor-state.test.ts (this package's tests live in
the shared root test/ directory, not packages/loopover-miner/test/**). The new
applySchemaMigrations-routed migration path, and its crash-recovery branch, must both be
exercised — a passing normal-migration test alone is not sufficient without the crash-simulation
test.

Expected Outcome

governor-state.ts's schema evolution has the same crash-safety guarantee every other store in
this package already has: a process killed mid-migration leaves the file in a recoverable state on
the next open, rather than a permanently unopenable file with orphaned data.

Links & Resources

  • packages/loopover-miner/lib/schema-version.tsapplySchemaMigrations (~lines 41-70), the
    transactional convention to adopt.
  • packages/loopover-miner/lib/governor-state.tsensurePauseColumns (~line 193),
    ensureReputationHistoryForgeScope (~lines 217-242), openGovernorState (~lines 244-274+).
  • packages/loopover-miner/lib/claim-ledger.ts / run-state.ts — their respective
    addApiBaseUrlScope migrations, the closest existing precedent for the same
    "rebuild the table" pattern already run safely inside applySchemaMigrations.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions