fix(miner): expire claims orphaned by a dead process at claim time - #6441
Conversation
sweepExpiredClaims/findExpiredClaims worked but had zero callers, so no claim ever expired: a claim left behind by a crashed or killed miner kept its issue locked indefinitely, and expireClaim -- which works -- was unreachable in normal operation. That is worse than a claim simply lingering. recordClaim re-activates a row only `WHERE status <> 'active'`, which makes re-claiming an already-active issue a deliberate no-op. So a stale claim doesn't just sit there: it wins. A later attempt calls claimIssue, gets a success back, and silently inherits the dead process's claimedAt and note. claimIssue now sweeps before recording, mirroring claimNextBatch's sweep-then-claim in portfolio-queue-manager.js, where a lease stranded by a dead process would otherwise starve the queue. Same reasoning, same shape: the sweep is the only thing standing between a dead process and a permanent lock. The store also gains reclaimExpiredClaims(maxAgeMs?), the explicit counterpart to reclaimStuckItems for an operator or a scheduled caller. The sweep is global rather than scoped to the issue being claimed, matching sweepStuckItems: a stale claim on issue JSONbored#1 is just as stuck whether or not anyone is claiming JSONbored#1 right now, and only claiming it again would ever notice. expireClaim itself is untouched, per the issue -- this only gives it a caller. The shipped ClaimLedger .d.ts gains reclaimExpiredClaims so the published type matches the store. Regression tests prove the claim actually changes hands: a stale claim's issue re-claimed by a new attempt now carries the new note and timestamp, where before the fix it kept the dead process's. Four of the six fail against the unwired store; the two that pass either way are the guards that claiming does NOT expire a live sibling's claim or disturb the normal release/re-claim lifecycle. Closes JSONbored#6156
|
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 #6441 +/- ##
=======================================
Coverage 95.61% 95.61%
=======================================
Files 598 598
Lines 47308 47312 +4
Branches 15053 15056 +3
=======================================
+ Hits 45233 45237 +4
Misses 1291 1291
Partials 784 784
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-16 08:10:34 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
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
sweepExpiredClaims/findExpiredClaimsworked but had zero callers, so no claim ever expired: a claim left behind by a crashed or killed miner kept its issue locked indefinitely, andexpireClaim— which works — was unreachable in normal operation.That is worse than a claim merely lingering.
recordClaimre-activates a row onlyWHERE status <> 'active', so re-claiming an already-active issue is a deliberate no-op. A stale claim therefore doesn't just sit there — it wins: a later attempt callsclaimIssue, gets a success back, and silently inherits the dead process'sclaimedAtandnote.claimIssuenow sweeps before recording, mirroringclaimNextBatch's sweep-then-claim inportfolio-queue-manager.js(whose own comment describes reclaiming a lease "stranded 'in_progress' by a dead process" so it doesn't "permanently consume a WIP slot and starve the queue" — the same failure, one layer down). The store also gainsreclaimExpiredClaims(maxAgeMs?), the explicit counterpart toreclaimStuckItemsfor an operator or a scheduled caller.Design choices, verified against the sibling pattern
claimIssue, not at one CLI call site.attempt-cli.jsandclaim-ledger-cli.jsboth reach the ledger throughopenClaimLedger(), and the module-levelclaimIssuedelegates to the same store — so this covers every path to a claim, exactly asclaimNextBatchcovers every batch claim.sweepStuckItems. A stale claim on feat(docs): add install site and mcp diagnostics #1 is just as stuck whether or not anyone is claiming feat(docs): add install site and mcp diagnostics #1 right now, and claiming any issue is the natural moment the miner is already touching the ledger.expireClaimis untouched, per the issue — this only gives it a caller.ClaimLedger.d.tsgainsreclaimExpiredClaims, so the published type matches the store.Validation
miner-claim-ledger-sweep-wiring.test.ts— 6/6 pass.main. The diagnostic is the bug itself:expected 'attempt:crashed-process' to be 'attempt:new-process'— the dead process's claim was still winning. The 2 that pass either way are the guards that claiming does not expire a live sibling's claim, nor disturb the normal release → re-claim lifecycle.reclaimExpiredClaims'smaxAgeMsdefault-arg) has both sides covered. Clears the 99%codecov/patchwall onpackages/loopover-miner/lib/**.npm run typecheck— 0 errors (the.d.tsnow declaresreclaimExpiredClaims, which the TS test calls).npm run build:miner+npm run test:miner-pack— pass.npm run docs:drift-check— ok.git diff --check— clean. Rebased on latestmain, no base conflict.Pre-existing failures on clean
main(my commit absent), so not introduced here: two Windows-onlyminer-claim-ledgercases (DB-path resolution, owner-only file permissions) and twominer-attempt-clicases — all pass on CI's Linux.Scope
claim-ledger.js), its published type (claim-ledger.d.ts), and the regression suite. Wanted paths (packages/,test/).site/,CNAME, orlovablechanges.Safety
sweepExpiredClaims, which already echoes each row's ownapiBaseUrl(Scope local ledger keys by forge host, not bare repoFullName #5563), so it can't expire the wrong forge host's claim.Closes #6156