fix(locks): early contributor-cap close joins the per-PR actuation lock namespace#8813
Conversation
…ck namespace (#8805) maybeCloseForContributorCapOnOpen executed its close under only the per-(repo, author) cap lock — outside the "one shared lock namespace covering every mutating PR pass" contract (transient-locks.ts) — so a sweep-fanned agent-regate-pr job holding the PR-actuation lock could plan/execute a different action for the SAME PR concurrently with the cap close. The execution section now also try-claims the per-PR actuation lock, nested author-outer → PR-inner. That is the OPPOSITE nesting of the executor's pre-merge cap re-check (PR-outer → author-inner, #7284) — safe regardless: both locks are non-blocking try-claims, so cross-order contention degrades to both passes deferring cleanly (the end-of-pipeline cap check and the next tick are the backstops), never a blocking-wait deadlock. Documented at the claim site. Contention test mirrors the existing author-lock sibling: the close defers (no PATCH), the normal pipeline falls through.
|
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 #8813 +/- ##
==========================================
- Coverage 93.81% 92.26% -1.55%
==========================================
Files 797 797
Lines 79608 79612 +4
Branches 24127 24128 +1
==========================================
- Hits 74686 73457 -1229
- Misses 3556 5090 +1534
+ Partials 1366 1065 -301
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 07:30:32 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed 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
Round-2 audit fix (#8805): the #7284 early cap-close ran under only the per-author cap lock, outside the per-PR actuation namespace
transient-locks.tsdocuments as covering every mutating PR pass — a concurrent sweep pass for the same PR could act simultaneously.Changes
src/queue/processors.ts— the cap-close'sexecuteAgentMaintenanceActionssection additionally try-claimsclaimPrActuationLock, deferring cleanly on contention (return false; the end-of-pipeline cap check + next tick are the existing backstops). Nesting is author-outer → PR-inner — the opposite of the executor's pre-merge re-check — which is deadlock-free by construction since both namespaces use non-blocking try-claims (cross-order contention = mutual clean defer, documented at the claim site).maybePostVisualFollowupComment, unlocked) is deliberately untouched: it self-heals via canonical-comment dedup on the next call, and lock-skipping there would drop a comment that has no retry backstop — the worse trade. Noted here per the issue's scope line.Test plan
npm run typecheck— cleanpr-actuation-lockdefers the early close (no close PATCH) and the normal pipeline falls throughCloses #8805