fix(miner): governor reputation-history read-modify-write is unguarded, unlike sibling scalar-state writes - #8988
Conversation
…d, unlike sibling scalar-state writes Fixes JSONbored#8855
|
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 #8988 +/- ##
==========================================
+ Coverage 90.56% 90.68% +0.12%
==========================================
Files 96 98 +2
Lines 22490 22889 +399
Branches 3884 4017 +133
==========================================
+ Hits 20367 20757 +390
- Misses 1945 1950 +5
- Partials 178 182 +4
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-26 15:31:21 UTC
Review summary Nits — 3 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.
|
Summary
packages/loopover-miner/lib/governor-state.ts:382-396'sloadReputationHistory/saveReputationHistorypair has nowithTransactionwrapper, unlike the other three scalar-state methods in the same file (lines 292-296 explain why those three need it: "so two processes on the same file... cannot interleave a stale read with each other's write").packages/loopover-miner/lib/loop-cli.ts:580-587doesconst prior = governorState.loadReputationHistory(...); governorState.saveReputationHistory(..., { decided: prior.decided + 1, unfavorable: prior.unfavorable + (isRejectedPr ? 1 : 0) }, ...)-- a classic read-modify-write with no transaction around the pair, even thoughgovernor-state.sqlite3is shared across fleet containers (DEPLOYMENT.md:60). Two containers finishing an attempt concurrently can both read the same counts and both+1, losing an increment in the Governor's self-reputation safety throttle.Deliverables
governor-state.ts(or itsloop-cli.tscall site) is wrapped in a single atomic transactionsaveReputationHistorycalls racing against oneloadReputationHistoryread) asserting no increment is lostAll of the above Deliverables are required in the same PR unless the deliverable text itself states otherwise.
Test plan
packages/loopover-miner/**-- 99%+ patch coverage including the new concurrency test path.Fixes #8855