Skip to content

orb(race): the contributor-cap mutex is released before the merge it guards, so the #7284 TOCTOU is still open — a legitimate PR is closed for a cap already relieved #9159

Description

@JSONbored

Problem

The per-author contributor-cap mutex is released before the merge it exists to protect
(src/services/agent-action-executor.ts:572-593):

try { const stillUnderCap = await ctx.contributorCapMergeRecheck();  }
finally { await releaseContributorCapLock(env, ctx.repoFullName, authorLogin, ownerToken); }
// 9) live — perform the real mutation
try { const detailOverride = await performAction(env, ctx, action); 

The sibling cap-close path holds the same lock across its entire plan-and-execute
(src/queue/processors.ts:2817 claim → :2900 release). The doc comment at :565-571 claims "the two
can never both act on a stale view"
— but the merge itself runs unlocked.

Separately, the approval-queue accept path (src/services/agent-approval-queue.ts:414-442) passes no
contributorCapMergeRecheck at all, so an accepted staged merge skips the re-check entirely.

Trigger

Cap = N. The author has N open PRs, one of which (A) is merging. PR C opens during A's merge PUT: the cap
handler claims the just-released lock, live-verifies A as still open via
resolvePerRepoContributorCapMatch, counts N+1, and one-shot-closes C.

Had A's merge been inside the mutex, C would have deferred (if (!acquired) return false) and survived.

Impact

Wrong-close. A legitimate contributor PR is auto-closed for a cap that was already relieved
milliseconds earlier. One-shot: no recovery except a fresh PR, which the contributor has no way to know
was warranted.

Dedup

Distinct from #8805 (contributor-cap close outside the PR actuation lock namespace) — different
lock, and its fix nested claimPrActuationLock inside. This is the author lock's coverage window on
the executor side. Also distinct from #9024 (the cap-lock TTL being shorter than the work it guards),
which is about duration rather than scope — the two should be fixed together since both leave the same
window open.

Requirements

  • Move releaseContributorCapLock into a finally that wraps step 9's performAction for the merge
    class, so the lock spans the mutation exactly as the close path's does.
  • Thread contributorCapMergeRecheck into the approval-queue accept context.
  • Add a concurrency regression test: a merge in flight must make a concurrent cap evaluation defer, not
    close.

Test Coverage Requirements

99%+ patch coverage, branch-counted; both arms of lock acquisition around the merge.

Links & Resources

maintainer-only — close-critical concurrency.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions