Skip to content

fix(queue): unify the per-PR actuation lock across maintenance and draft-dodge/reopen-reclose - #2447

Merged
JSONbored merged 1 commit into
mainfrom
claude/pr-actuation-lock-unify
Jul 2, 2026
Merged

fix(queue): unify the per-PR actuation lock across maintenance and draft-dodge/reopen-reclose#2447
JSONbored merged 1 commit into
mainfrom
claude/pr-actuation-lock-unify

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Follow-up to fix(queue): add a per-PR actuation mutex for the draft-dodge and reopen-reclose paths #2399 (already merged): a review round on that PR flagged that maybeRunAgentMaintenance claimed a separate claimAgentMaintenanceLock, while the draft-dodge and reopen-reclose paths added in fix(queue): add a per-PR actuation mutex for the draft-dodge and reopen-reclose paths #2399 claimed claimPrActuationLock — two independent lock namespaces for the same PR meant a maintenance/check-suite pass could still race a draft-dodge close or reopen-reclose on the same PR. The fix landed after fix(queue): add a per-PR actuation mutex for the draft-dodge and reopen-reclose paths #2399 had already merged, so this ships the fix as its own PR.
  • maybeRunAgentMaintenance now claims/releases the same claimPrActuationLock/releasePrActuationLock the other two paths use, and the now-redundant claimAgentMaintenanceLock/releaseAgentMaintenanceLock/agentMaintenanceLockKey are removed — one shared lock namespace, not one lock per mutating path.
  • Updated the existing per-PR-lock invariant test to pre-claim the shared pr-actuation-lock: key (previously pre-claimed the now-removed agent-maintenance-lock: key) and removed the now-duplicate claimAgentMaintenanceLock-specific unit tests, since claimPrActuationLock's existing test suite already covers the identical atomic-claim/fail-open guarantees.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • No new issue linked — this is a narrow follow-up fix to an already-merged PR (fix(queue): add a per-PR actuation mutex for the draft-dodge and reopen-reclose paths #2399); the summary above explains why.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally — the only functionally-changed lines (the lock call sites) are covered by the existing test suite; the rest of the diff is comments/deletions.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — no auth/session surface touched.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — no API surface touched.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — backend-only change.)
  • No visible UI change — UI Evidence section not applicable.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A.)

Notes

  • No migrations, OpenAPI, or Cloudflare binding changes — pure lock-namespace consolidation in src/queue/processors.ts.

…-dodge, and reopen-reclose

maybeRunAgentMaintenance claimed a separate claimAgentMaintenanceLock
while the draft-dodge and reopen-reclose paths claimed claimPrActuationLock
— two independent lock namespaces for the same PR meant a maintenance
pass and a draft-dodge/reopen-reclose actuation could still race each
other, the exact gap a review on #2399 flagged after it had already
merged. Route maybeRunAgentMaintenance through claimPrActuationLock/
releasePrActuationLock too and remove the now-redundant
claimAgentMaintenanceLock so every mutating PR path shares one lock.
@loopover-orb

loopover-orb Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-02 01:37:50 UTC

2 files · 1 AI reviewer · no blockers · readiness 93/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The diff coherently removes the separate maintenance-only lock namespace and moves `maybeRunAgentMaintenance` onto the existing per-PR actuation lock, which matches the PR’s stated race fix. The changed call sites use the shared claim/release pair consistently, and the removed helper tests are covered by the remaining `claimPrActuationLock` contract tests plus the updated pre-claimed-lock regression. I do not see a reachable break in the provided diff.

Nits — 5 non-blocking
  • nit: `src/queue/processors.ts:1707` now relies on the broader `PR_ACTUATION_LOCK_TTL_SECONDS` value of 600 seconds for maintenance after deleting the old 60-second maintenance TTL; that is probably intentional, but the PR should explicitly call out the longer crash-stall window because a failed release now suppresses all PR actuation for ten minutes.
  • nit: `test/unit/queue.test.ts:3695` pre-claims the shared key with `set(...)` rather than `claimPrActuationLock(...)`, so the test verifies the literal key string but does not exercise the same helper path a competing actuation pass uses.
  • At `test/unit/queue.test.ts:3695`, consider pre-claiming with `await claimPrActuationLock(env, "owner/agent-repo", 7)` and then asserting the maintenance pass defers; that keeps the regression tied to the public lock helper instead of duplicating the key format.
  • At `src/queue/processors.ts:1707`, add a short comment or test expectation covering the 600-second shared TTL choice for maintenance so the old 60-second maintenance behavior is not accidentally treated as unchanged.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ No-issue rationale PR body explains why no issue is linked.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (size label size:M; no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 65 registered-repo PR(s), 55 merged, 563 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 65 PR(s), 563 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 65 PR(s), 563 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Triage stale or unlinked PRs.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 2, 2026
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.91%. Comparing base (fa6accb) to head (b5b4260).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2447      +/-   ##
==========================================
- Coverage   95.91%   95.91%   -0.01%     
==========================================
  Files         224      224              
  Lines       25240    25235       -5     
  Branches     9177     9177              
==========================================
- Hits        24210    24205       -5     
  Misses        417      417              
  Partials      613      613              
Files with missing lines Coverage Δ
src/queue/processors.ts 91.73% <100.00%> (-0.03%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 38647fc into main Jul 2, 2026
12 checks passed
@JSONbored
JSONbored deleted the claude/pr-actuation-lock-unify branch July 2, 2026 01:45
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant