fix(review): claim the AI-review lock before the cache-read, not just the LLM call

[JSONbored]

Summary

Two near-simultaneous GitHub webhook deliveries for the same PR at the same head SHA (JSONbored/awesome-claude PR #4554: two deliveries ~900ms apart) could each independently reach the AI-review cache-read decision in maybePublishPrPublicSurface before either one ever claimed claimAiReviewLock — that claim lived only deep inside runAiReviewForAdvisory. Both passes logged an identical ai_review_cache_miss audit event and proceeded toward a fresh review, duplicating manifest resolution, review-file loading, and fingerprint computation, and, depending on exact timing, a real LLM call.

The caller (maybePublishPrPublicSurface) now claims the same (repo, PR, head SHA, mode) lock immediately before the cache-read decision, wrapping the whole read-decide-run-persist sequence instead of only the LLM call at the end of it. A losing pass defers via the shared inconclusive-hold placeholder (aiReviewLockContendedResult, factored out so both lock-claim sites produce byte-identical output) before ever touching the cache. The winning pass's already-acquired claim is threaded into runAiReviewForAdvisory via a new optional preAcquiredAiReviewLock parameter, so that function trusts it instead of re-claiming — and always losing — against its own caller. Release stays with the claiming caller (not runAiReviewForAdvisory's own finally) so the lock keeps covering the caller's post-return cache write too, not just the read/run. Every existing direct caller of runAiReviewForAdvisory is unaffected — the new parameter is optional and defaults to the prior self-claim/self-release behavior.

Root-caused and reproduced with a regression test that fires two concurrent agent-regate-pr jobs for the same PR/head via Promise.all, with an artificially delayed env.AI.run stub (matching a real LLM call's wall-clock latency) to force a genuine race rather than an incidental one — and asserts github_app.ai_review_cache_miss lands exactly once, not twice. Verified this test fails on the pre-fix code (missAudit?.n is 2) and passes after the fix (missAudit?.n is 1).

Closes #4026

Scope

• The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
• 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.
• I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

• git diff --check
• npm run actionlint
• npm run typecheck
• npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate. — every changed line/branch is covered (100%/100% measured directly from the lcov report, combining test/unit/ai-review-advisory.test.ts and test/unit/queue.test.ts).
• 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

If any required check was skipped, explain why:

• No UI/OpenAPI/MCP-surface change in this PR (backend-only, src/queue/processors.ts), so those checks pass trivially with no relevant diff.

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. — not applicable; this PR does not touch auth/cookie/CORS/session code.
• API/OpenAPI/MCP behavior is updated and tested where needed. — not applicable; no API/OpenAPI/MCP surface changed.
• UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — not applicable; no UI change.
• Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository. — not applicable; no visible UI change.
• Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — no public-facing docs need updating for this internal reliability fix.

UI Evidence

Not applicable — backend-only change with no visible UI/frontend/docs surface.

Notes

• Judgment call on where exactly to move the lock claim: the task allowed either (a) claiming in the caller and wrapping the cache-read decision itself, or (b) moving the cache-check into runAiReviewForAdvisory so its existing lock naturally wraps it. Moving the ~150-line cache-check-and-persist block into runAiReviewForAdvisory (option b, literally) would have meant either relocating a lock with 10+ existing passing tests validating its exact atomicity semantics, or introducing a second lock claim on the same key (which would self-contend and always lose against the caller's own claim). Instead, this PR claims the lock in the caller (option a) and threads the already-acquired claim into runAiReviewForAdvisory via a new optional parameter, so the function's existing, well-tested lock logic is reused rather than duplicated or relocated — the caller does the claiming, runAiReviewForAdvisory trusts a supplied claim instead of re-claiming, and release stays with whichever side did the actual claim. This kept the diff to two files and preserved every existing test's behavior unmodified.

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[loopover-orb]

Warning

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

⏸️ Gittensory review result - manual review recommended

_{Review updated: 2026-07-07 12:20:15 UTC}

2 files · 1 blocker · readiness 100/100 · CI green · clean

⏸️ Suggested Action - Manual Review

• Linked issue overlaps another open PR — Review the related PRs before spending reviewer time on duplicate work.

Nits — 1 non-blocking

• PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.

Concerns raised — review before merging

• Linked issue overlaps another open PR — Review the related PRs before spending reviewer time on duplicate work.

Signal	Result	Evidence
Code review	❌ 1 blocker	No AI review summary
Linked issue	✅ Linked	#4026, #123
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 (2 linked issues).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 51 registered-repo PR(s), 43 merged, 570 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	JSONbored; Gittensor profile; 51 PR(s), 570 issue(s).
Gate result	❌ Blocking	Repo-configured hard blocker found.

Review context

• Author: JSONbored
• Role context: owner (maintainer lane)
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
• Official Gittensor activity: 51 PR(s), 570 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Treat this as maintainer-lane context rather than normal contributor-lane activity.

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.58%. Comparing base (3a74800) to head (76c1619).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4033   +/-   ##
=======================================
  Coverage   93.58%   93.58%           
=======================================
  Files         381      381           
  Lines       35483    35492    +9     
  Branches    13014    13017    +3     
=======================================
+ Hits        33206    33215    +9     
  Misses       1617     1617           
  Partials      660      660           

Files with missing lines	Coverage Δ
src/queue/processors.ts	94.86% <100.00%> (+0.01%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.