fix(miner): fail acquire() on an attempt_id/repo mismatch - #8964
Conversation
|
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 #8964 +/- ##
==========================================
+ Coverage 90.56% 90.61% +0.05%
==========================================
Files 96 97 +1
Lines 22490 22613 +123
Branches 3884 3918 +34
==========================================
+ Hits 20367 20490 +123
Misses 1945 1945
Partials 178 178
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 14:43:29 UTC
Review summary Nits — 5 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.
|
What
worktree-allocator.ts'sacquire(attemptId, repoFullName)early-return(
existing?.status === "active") keyed only onattemptId— it never compared the existingallocation's
repo_full_nameagainst the newly-requestedrepoFullName. So a secondacquirecall for the same
attemptIdbut a different repo silently got back the first repo'sallocation, with no error. Only the same-repo re-acquire case was tested.
Change
On that early-return path, throw a clear
attempt_id_repo_mismatcherror whenexisting.repo_full_name !== normalizedRepo, instead of silently returning the mismatchedallocation. Same-repo re-acquire stays idempotent (returns the same allocation, unchanged).
Validation
test/unit/miner-worktree-allocator.test.ts: re-acquiring the same active attemptfor the same repo returns the same allocation, but for a different repo throws
attempt_id_repo_mismatch.npx vitest run test/unit/miner-worktree-allocator.test.ts→ the new test passes; both sides ofthe new repo-match branch are covered (the same-repo side by existing idempotence tests). The two
pre-existing failures in this file (
resolves DB … env overrides,creates a permissioned SQLite store) are Windows-only baseline (Unix file-mode / path-separator assumptions) — they failidentically on a clean checkout and pass on Linux CI.
Closes #8858