fix(miner): report the real post-failure schema version for a partially-applied migration - #6860
Conversation
…ly-applied migration
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6860 +/- ##
=======================================
Coverage 93.64% 93.64%
=======================================
Files 685 685
Lines 68238 68242 +4
Branches 18707 18707
=======================================
+ Hits 63903 63907 +4
Misses 3350 3350
Partials 985 985
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-17 09:11:49 UTC
Review summary Nits — 2 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Closes #6767
Problem
migrateStore(packages/loopover-miner/lib/migrate-cli.js) reportsversionAfter: versionBeforein its catch branch — "nothing changed". Butschema-version.js'sapplySchemaMigrationsapplies and stamps each migration in its own transaction, specifically so a failure part-way through leaves the file at the last fully-applied version. So whenopen(dbPath)throws while applying migration N of several pending ones, the migrations before N have already committed to disk and the real on-disk version has advanced pastversionBefore— but the catch never re-reads it, and thefailedresult misreports what's actually on disk.Fix
Re-read the real on-disk version in the catch branch, guarded by its own
try: a genuinely corrupt/unopenable file (the same case that can leaveversionBeforenull) may fail this read too, in which case the pre-failure reading is all we can honestly report.Tests
test/unit/miner-migrate-cli.test.tsadds a regression test that injects a store whoseopenruns a migrations array where the first migration commits (stampingBASELINE+1) and the second throws — asserting the reportedversionAfterisBASELINE+1(the real partial progress) and that it matches the file's actual on-diskPRAGMA user_version. It fails before the fix (expected 1 to be 2).Both branches of the new guard are covered:
versionBefore.Existing expectations are unchanged (the fake-store case still reports
versionAfter: 0).Validation
npx vitest run test/unit/miner-migrate-cli.test.ts→ 11/11 pass (10 existing + 1 new)npm run typecheck→ clean (0 errors)