diff --git a/src/queue/processors.ts b/src/queue/processors.ts index abbd616aa1..64a557eb4d 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -269,6 +269,7 @@ import { MAX_REVIEW_NAG_COOLDOWN_DAYS, isProtectedAutomationAuthor, planAgentMaintenanceActions, + planContributorCapClose, type AgentActionPlanInput, type AgentDispositionLabelSettings, type PlannedAgentAction, @@ -332,8 +333,10 @@ import { estimateReviewEffort } from "../review/review-effort"; import { buildUnifiedCommentBody } from "../review/unified-comment-bridge"; import { isRetryableJobError, RetryableJobError } from "./retryable"; import { + claimContributorCapLock, claimPrActuationLock, PrActuationLockContendedError, + releaseContributorCapLock, releasePrActuationLock, type TransientLockClaim, } from "./transient-locks"; @@ -2514,6 +2517,225 @@ function buildAgentMaintenancePlanInput(args: { }; } +/** + * Resolve JUST the per-repo contributor open-PR cap match (#2270), including the #2479 webhook-delivery-order + * wake side effect — extracted from {@link runAgentMaintenancePlanAndExecute} (#7284-fix) so a cheap, + * CI-independent caller (the PR-open webhook path) can compute the SAME match ahead of the expensive review + * pipeline. Deliberately excludes the install-wide cap below (see {@link resolveContributorCapMatch}): that + * check needs a miner-detection lookup on every non-exempt author regardless of whether this repo even has a + * per-repo cap configured, so an early-path caller that always ran both would double that lookup's cost for + * EVERY PR-open across the fleet, not just the rare over-cap ones — this function alone is cheap enough (one + * DB query + a bounded live-GitHub confirm) to run unconditionally on open. + */ +async function resolvePerRepoContributorCapMatch( + env: Env, + deliveryId: string, + installationId: number, + repoFullName: string, + pr: PullRequestRecord, + settings: RepositorySettings, + token: string | undefined, + admissionKey: string | undefined, + isNewAccount: boolean, +): Promise { + let contributorCapMatch: AgentActionPlanInput["contributorCapMatch"]; + const contributorOpenPrCap = + isNewAccount && typeof settings.contributorOpenPrCap === "number" + ? Math.max(1, Math.ceil(settings.contributorOpenPrCap / 2)) + : settings.contributorOpenPrCap; + if (typeof contributorOpenPrCap === "number" && pr.authorLogin && !isAutoCloseExempt(pr.authorLogin, settings.autoCloseExemptLogins)) { + const otherAuthorOpenPullRequests = await listOtherOpenPullRequestsForAuthor(env, repoFullName, pr.number, pr.authorLogin); + const confirmedOpen = new Set(); + await mapWithConcurrency(otherAuthorOpenPullRequests, CONTRIBUTOR_CAP_LIVE_CHECK_CONCURRENCY, async (other) => { + const liveState = await fetchLivePullRequestState(env, repoFullName, other.number, token, admissionKey).catch(() => undefined); + if (liveState === "open") confirmedOpen.add(other.number); + }); + const authorOpenPrNumbers = otherAuthorOpenPullRequests + .filter((other) => confirmedOpen.has(other.number)) + .map((other) => other.number) + .concat(pr.number) + .sort((a, b) => a - b); + const overCapNumbers = new Set(authorOpenPrNumbers.slice(contributorOpenPrCap)); + if (overCapNumbers.has(pr.number)) { + contributorCapMatch = { matched: true, authorLogin: pr.authorLogin, openCount: authorOpenPrNumbers.length, cap: contributorOpenPrCap, itemKind: "pull requests" }; + } + const otherOverCapSiblingNumbers = otherAuthorOpenPullRequests + .filter((other) => confirmedOpen.has(other.number) && overCapNumbers.has(other.number)) + .map((other) => other.number); + if (otherOverCapSiblingNumbers.length > 0) { + await wakeOverCapSiblingPullRequests(env, deliveryId, installationId, repoFullName, otherOverCapSiblingNumbers); + } + } + return contributorCapMatch; +} + +/** + * Resolve the per-contributor open-item cap match (#2270 per-repo + #2562 install-wide) — the per-repo half + * delegates to {@link resolvePerRepoContributorCapMatch}; the install-wide half (only evaluated when the + * per-repo half didn't already match) is kept HERE, not in the cheap early-path helper, precisely because it + * needs a miner-detection lookup on every non-exempt author and must not run on every single PR-open across + * the fleet — see that function's own doc comment. Identical behavior to the single inline block this used to + * be. + */ +async function resolveContributorCapMatch( + env: Env, + deliveryId: string, + installationId: number, + repoFullName: string, + pr: PullRequestRecord, + settings: RepositorySettings, + token: string | undefined, + admissionKey: string | undefined, + isNewAccount: boolean, +): Promise { + let contributorCapMatch = await resolvePerRepoContributorCapMatch(env, deliveryId, installationId, repoFullName, pr, settings, token, admissionKey, isNewAccount); + + const prGlobalCapForHuman = resolveGlobalContributorOpenItemCap(env); + const prGlobalCapForMiner = resolveGlobalContributorOpenItemCapForMiner(env); + if ( + contributorCapMatch === undefined && + pr.authorLogin && + (prGlobalCapForHuman !== null || prGlobalCapForMiner !== null) && + !isAutoCloseExempt(pr.authorLogin, settings.autoCloseExemptLogins) + ) { + const officialMiner = await getCachedOfficialMinerDetection(env, pr.authorLogin, { + targetKey: `${repoFullName}#${pr.number}`, + deliveryId, + }); + const globalCap = officialMiner.status === "confirmed" ? prGlobalCapForMiner : prGlobalCapForHuman; + if (globalCap !== null) { + const globalOpenCount = await verifiedGlobalOpenItemCount(env, installationId, pr.authorLogin, { + repoFullName, + number: pr.number, + kind: "pull_request", + }, globalCap); + if (globalOpenCount > globalCap) { + contributorCapMatch = { matched: true, authorLogin: pr.authorLogin, openCount: globalOpenCount, cap: globalCap, itemKind: "pull requests and issues", scope: "install" }; + } + } + } + return contributorCapMatch; +} + +/** + * Cheap, CI-independent contributor open-item cap short-circuit (#7284-fix, resource-waste ordering): called + * on `pull_request` `opened` BEFORE prReadyForReview/CI-wait/AI-review ever run, so an over-cap PR is closed + * immediately instead of only after the full review pipeline (miner detection, type-labeling, AI slop + * advisory, linked-issue satisfaction, the actual multi-LLM-call AI code review, CI-completeness + * verification, public surface publish) already ran for it — confirmed live: the end-of-pipeline cap check + * alone let a burst of over-cap PRs pay for that entire pipeline before being caught (2026-07-21 incident, + * PRs #7284-#7289). Returns true when this PR was closed for the cap (caller should stop processing this + * webhook delivery); false when the author is under cap, exempt, owner/admin/bot, or the lock is contended + * (another pass is already deciding for this same author — safe to skip, the existing end-of-pipeline check + * still runs as defense-in-depth, and the #2479 wake mechanism covers a sibling this pass would have caught). + * Wrapped in the per-(repo, author) mutex (#7284-fix Fix B) so this decision and a concurrent sibling's + * decision (or a concurrent merge's pre-merge re-check) can never both act on a stale view of the author's + * open-PR count. + */ +async function maybeCloseForContributorCapOnOpen( + env: Env, + deliveryId: string, + installationId: number, + repoFullName: string, + pr: PullRequestRecord, + settings: RepositorySettings, +): Promise { + if (!pr.authorLogin || isAutoCloseExempt(pr.authorLogin, settings.autoCloseExemptLogins)) return false; + /* v8 ignore next -- defensive: String.prototype.split always returns at least one element, so + * repoFullName.split("/")[0] is never undefined for any non-empty repoFullName (every real caller's). */ + const repoOwner = repoFullName.split("/")[0] ?? ""; + const authorIsOwner = pr.authorLogin.toLowerCase() === repoOwner.toLowerCase(); + const authorIsAdmin = parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(pr.authorLogin.toLowerCase()); + const authorIsAutomationBot = isProtectedAutomationAuthor(pr.authorLogin); + if (authorIsOwner || authorIsAdmin || authorIsAutomationBot) return false; + // #ignore-authors-parity: a manifest ignore_authors match (e.g. "release-please*") means the bot treats + // this author as entirely invisible -- maybePublishPrPublicSurface's own reviewEligibility check (deep + // inside the expensive pipeline this early path exists to skip) already enforces this; this early path must + // enforce it too, or an ignored author would gain a NEW live-GitHub/DB cost here that the rest of the + // pipeline deliberately never pays for them. Cheap: a cached manifest read, no GitHub call. + const reviewManifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null); + const autoReviewConfig = resolveReviewAutoReviewConfig(reviewManifest); + if (!decideReviewEligibility({ authorLogin: pr.authorLogin, ignoreAuthors: autoReviewConfig.ignoreAuthors }).eligible) return false; + // #resource-waste-guard: no per-repo cap configured at all ⇒ resolvePerRepoContributorCapMatch can never + // match regardless of isNewAccount's account-age scaling (that only shrinks an EXISTING numeric cap, never + // conjures one from nothing) — bail before minting an installation token or doing any other work below, so + // a repo with no cap configured pays NOTHING extra for this early path (matches today's behavior exactly). + if (typeof settings.contributorOpenPrCap !== "number") return false; + + const { acquired, ownerToken } = await claimContributorCapLock(env, repoFullName, pr.authorLogin); + if (!acquired) return false; + try { + const ciToken = await createInstallationToken(env, installationId).catch(() => undefined); + /* v8 ignore next -- the GITHUB_PUBLIC_TOKEN fallback mirrors the identical, already-covered pattern in + * runAgentMaintenancePlanAndExecute above; a mint failure here degrades to that same fallback rather than + * a distinct code path. */ + const token = ciToken ?? env.GITHUB_PUBLIC_TOKEN; + const admissionKey = githubAdmissionKeyForToken(env, installationId, token); + const isNewAccount = await isBelowAccountAgeThreshold(env, installationId, pr.authorLogin, settings.accountAgeThresholdDays); + const contributorCapMatch = await resolvePerRepoContributorCapMatch(env, deliveryId, installationId, repoFullName, pr, settings, token, admissionKey, isNewAccount); + if (!contributorCapMatch) return false; + // #account-age-parity: the end-of-pipeline path's account-age throttle (#2561) applies this SAME visibility + // label whenever isNewAccount is true, independent of the cap outcome -- preserve that here too (scoped to + // the PR actually about to close for cap, not every new-account PR unconditionally, so an under-cap + // new-account PR still gets it exactly once, from the normal pipeline this early path lets it reach). + if (isNewAccount && resolveAutonomy(settings.autonomy, "review_state_label") === "auto") { + const newAccountMode = resolveAgentActionMode({ + globalPaused: isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env)), + agentPaused: settings.agentPaused, + agentDryRun: settings.agentDryRun, + }); + /* v8 ignore next -- the settings.newAccountLabel ?? "new-account" fallback mirrors the identical, + * already-covered field the end-of-pipeline account-age throttle above uses ("a configured + * newAccountLabel is used instead of the default"); this call site reuses the same settings field. */ + await ensurePullRequestLabel(env, installationId, repoFullName, pr.number, settings.newAccountLabel ?? "new-account", { + createMissingLabel: settings.createMissingLabel, + mode: newAccountMode, + }).catch( + /* v8 ignore next -- fail-safe: a label-application failure must never block the rest of the handler */ + () => undefined, + ); + } + const planned = planContributorCapClose({ + autonomy: settings.autonomy, + authorIsOwner, + authorIsAdmin, + authorIsAutomationBot, + contributorCapMatch, + contributorCapLabel: settings.contributorCapLabel, + pr: { headSha: pr.headSha }, + }); + if (planned === null || planned.length === 0) return false; + const installation = await getInstallation(env, installationId); + const outcomes = await executeAgentMaintenanceActions( + env, + { + installationId, + repoFullName, + pullNumber: pr.number, + headSha: pr.headSha, + autonomy: settings.autonomy, + agentPaused: settings.agentPaused, + agentDryRun: settings.agentDryRun, + /* v8 ignore next -- defensive: mirrors runAgentMaintenancePlanAndExecute's own identical fallback + * above -- an installed-App PR webhook always carries an installation record; the null is defensive. */ + installationPermissions: installation?.permissions ?? null, + authorLogin: pr.authorLogin, + contributorCapCancelCi: settings.contributorCapCancelCi ?? env.CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT === "true", + moderationSettings: { + moderationGateMode: settings.moderationGateMode, + moderationRules: settings.moderationRules, + moderationWarningLabel: settings.moderationWarningLabel, + moderationBannedLabel: settings.moderationBannedLabel, + }, + }, + planned, + ); + return outcomes.some((outcome) => outcome.actionClass === "close" && outcome.outcome === "completed"); + } finally { + await releaseContributorCapLock(env, repoFullName, pr.authorLogin, ownerToken); + } +} + /** The plan-and-execute critical section of {@link maybeRunAgentMaintenance}, extracted so the caller's * per-PR lock (#2129) wraps it in a try/finally without reindenting this whole block. */ async function runAgentMaintenancePlanAndExecute( @@ -2829,104 +3051,12 @@ async function runAgentMaintenancePlanAndExecute( } } - // Per-contributor open-PR cap (#2270, anti-abuse): count this author's OTHER currently-open PRs on this repo - // (otherOpenPullRequests already excludes the current PR — see reconcileLiveDuplicateSiblings) plus this one, - // ranked by PR NUMBER (GitHub's own creation order, not webhook-arrival order) so a burst of near-simultaneous - // opens still ranks deterministically. Only matches when THIS PR's number is among the ones over the cap — an - // older sibling that was already under the cap when it was opened stays open. The owner/admin/automation-bot - // exemption is applied by the planner itself (defense-in-depth, mirroring how blacklistEntry above is resolved - // unconditionally and exempted only inside planAgentMaintenanceActions). Disabled (null/undefined cap, the - // default) ⇒ this block is a no-op. A below-account-age-threshold author (#2561) gets a TIGHTER effective - // cap (half, rounded up, minimum 1) — visibility/friction, still never a close on account age by itself - // (the close, if any, is still tagged/reasoned as the ordinary contributor-cap close). - let contributorCapMatch: { matched: boolean; authorLogin: string; openCount: number; cap: number; itemKind: "pull requests" | "issues" | "pull requests and issues"; scope?: "repository" | "install" | undefined } | undefined; - const contributorOpenPrCap = - isNewAccount && typeof settings.contributorOpenPrCap === "number" - ? Math.max(1, Math.ceil(settings.contributorOpenPrCap / 2)) - : settings.contributorOpenPrCap; - // #2270/#2463-parity: the per-repo cap now honors the SAME shared `autoCloseExemptLogins` allowlist the - // install-wide cap (below) and review-nag cooldown already do -- previously only the owner/admin/automation-bot - // exemption (applied later, inside planAgentMaintenanceActions) protected an author here, so a maintainer-named - // trusted-but-not-a-recognized-bot login (e.g. a third-party automation App like Sentry's Seer fix bot) had no - // way to opt out of the PER-REPO cap specifically, even though `.loopover.yml`'s own doc comment already - // promised this reuse (auto-close-exempt.ts). - if (typeof contributorOpenPrCap === "number" && pr.authorLogin && !isAutoCloseExempt(pr.authorLogin, settings.autoCloseExemptLogins)) { - // Fixed-budget author-scoped set (the lowest-numbered sibling sample), with every counted sibling - // positively LIVE-confirmed still open before it counts toward an irreversible close decision (#2270 - // busy-repo bypass fix). Runs unconditionally now -- not just for isNewAccount -- since a stale-DB-row - // false positive is exactly as wrong for an established contributor as for a new one; this supersedes the - // narrower new-account-only live-verify this block used to have. Reuses the function-scoped token/admissionKey - // (already resolved above for the live-CI recheck) rather than minting a second one. - const otherAuthorOpenPullRequests = await listOtherOpenPullRequestsForAuthor(env, repoFullName, pr.number, pr.authorLogin); - const confirmedOpen = new Set(); - // Bounded concurrency (security review finding): an unbounded Promise.all here scales with the author's - // OWN open-PR sample, not a fixed small number -- an author with dozens of open PRs would fire that many - // concurrent GitHub calls from a single webhook, and the delivery-order-guard wake below re-triggers this - // same block for every over-cap sibling, compounding into near-quadratic API growth that can exhaust the - // installation's rate-limit budget. Every sampled entry must still be verified (the exact over-cap PR - // numbers below depend on the confirmed-open sample, not just "is the count over cap"), so this bounds - // concurrency in addition to the repository query's total row cap. - await mapWithConcurrency(otherAuthorOpenPullRequests, CONTRIBUTOR_CAP_LIVE_CHECK_CONCURRENCY, async (other) => { - const liveState = await fetchLivePullRequestState(env, repoFullName, other.number, token, admissionKey).catch(() => undefined); - if (liveState === "open") confirmedOpen.add(other.number); - }); - const authorOpenPrNumbers = otherAuthorOpenPullRequests - .filter((other) => confirmedOpen.has(other.number)) - .map((other) => other.number) - .concat(pr.number) - .sort((a, b) => a - b); - const overCapNumbers = new Set(authorOpenPrNumbers.slice(contributorOpenPrCap)); - if (overCapNumbers.has(pr.number)) { - contributorCapMatch = { matched: true, authorLogin: pr.authorLogin, openCount: authorOpenPrNumbers.length, cap: contributorOpenPrCap, itemKind: "pull requests" }; - } - // Webhook-delivery-order guard (#2479 gate finding): delivery order is not guaranteed to match PR creation - // order, so a sibling PR's own webhook can process before THIS PR exists in the DB and wrongly conclude the - // author is within the cap. Use the fixed-budget author-scoped set and only siblings positively confirmed - // open, matching the issue-cap fail-safe close contract. - const otherOverCapSiblingNumbers = otherAuthorOpenPullRequests - .filter((other) => confirmedOpen.has(other.number) && overCapNumbers.has(other.number)) - .map((other) => other.number); - if (otherOverCapSiblingNumbers.length > 0) { - await wakeOverCapSiblingPullRequests(env, deliveryId, installationId, repoFullName, otherOverCapSiblingNumbers); - } - } - - // Install-wide contributor open-item cap (#2562, anti-abuse): IN ADDITION TO the per-repo cap above, not - // instead of it -- only evaluated when the per-repo cap didn't already match (short-circuit: no need for a - // second cross-repo DB read once this PR is already being closed). Defaults to a real cap even when unset - // (#4511) -- a CONFIRMED official Gittensor miner gets its own, higher fleet-appropriate default instead of - // either "no cap" or the plain human default, since a legitimate fleet spread across many repos in one - // install is expected to run more concurrent open items than a single human contributor. Reuses the shared - // autoCloseExemptLogins list (#2463) so a maintainer-named login is exempt here exactly like the per-repo - // caps and review-nag cooldown. - const prGlobalCapForHuman = resolveGlobalContributorOpenItemCap(env); - const prGlobalCapForMiner = resolveGlobalContributorOpenItemCapForMiner(env); - if ( - contributorCapMatch === undefined && - pr.authorLogin && - (prGlobalCapForHuman !== null || prGlobalCapForMiner !== null) && - !isAutoCloseExempt(pr.authorLogin, settings.autoCloseExemptLogins) - ) { - // Deferred until we know at least one of the two resolvers is actually active (#4511): the identity - // lookup below is cached but still a DB/network round trip, and both resolvers above are plain env reads. - const officialMiner = await getCachedOfficialMinerDetection(env, pr.authorLogin, { - targetKey: `${repoFullName}#${pr.number}`, - deliveryId, - }); - const globalCap = officialMiner.status === "confirmed" ? prGlobalCapForMiner : prGlobalCapForHuman; - if (globalCap !== null) { - const globalOpenCount = await verifiedGlobalOpenItemCount(env, installationId, pr.authorLogin, { - repoFullName, - number: pr.number, - kind: "pull_request", - }, globalCap); - if (globalOpenCount > globalCap) { - // verifiedGlobalOpenItemCount sums BOTH open PRs and open issues -- reporting this as "pull requests" - // when the author's over-cap total may include issues would be a factually wrong close message. - contributorCapMatch = { matched: true, authorLogin: pr.authorLogin, openCount: globalOpenCount, cap: globalCap, itemKind: "pull requests and issues", scope: "install" }; - } - } - } + // Per-contributor open-item cap (#2270 per-repo + #2562 install-wide, anti-abuse): a below-account-age- + // threshold author (#2561) gets a TIGHTER effective per-repo cap inside resolveContributorCapMatch (half, + // rounded up, minimum 1) — visibility/friction, still never a close on account age by itself. Extracted + // (#7284-fix) so the SAME logic is also directly callable from a cheap, CI-independent caller (the PR-open + // webhook path) ahead of the expensive review pipeline — see resolveContributorCapMatch's own doc comment. + const contributorCapMatch = await resolveContributorCapMatch(env, deliveryId, installationId, repoFullName, pr, settings, token, admissionKey, isNewAccount); const autoMaintain = settings.autoMaintain ?? DEFAULT_AUTO_MAINTAIN_POLICY; @@ -3178,6 +3308,26 @@ async function runAgentMaintenancePlanAndExecute( // CI-run cancellation on a contributor_cap close (#2462): the repo's own explicit setting always wins; // null/undefined (unset) falls back to the install-wide CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT env var. contributorCapCancelCi: settings.contributorCapCancelCi ?? env.CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT === "true", + // Pre-merge contributor-cap re-check (#7284-fix, TOCTOU race): only constructed when a merge might + // actually execute this pass AND a per-repo cap is configured AND the author isn't exempt from it (the + // SAME conditions resolveContributorCapMatch/planContributorCapClose already gate on above) — absent + // otherwise, so a repo with no cap configured (or an owner/admin/bot merge) pays nothing extra. Closes + // over the current settings/token (already resolved above for THIS pass) and calls the SAME + // resolvePerRepoContributorCapMatch the early-path webhook check and the planning step above both use, + // so "over cap" means the exact same thing everywhere. + contributorCapMergeRecheck: + planHasImminentMerge && + typeof settings.contributorOpenPrCap === "number" && + pr.authorLogin && + !authorIsOwner && + !authorIsAdmin && + !authorIsAutomationBot && + !isAutoCloseExempt(pr.authorLogin, settings.autoCloseExemptLogins) + ? async () => { + const recheckMatch = await resolvePerRepoContributorCapMatch(env, deliveryId, installationId, repoFullName, pr, settings, token, admissionKey, isNewAccount); + return recheckMatch?.matched !== true; + } + : undefined, moderationSettings: { moderationGateMode: settings.moderationGateMode, moderationRules: settings.moderationRules, @@ -5823,6 +5973,23 @@ async function handlePullRequestWebhookEvent( }); return true; } + // #7284-fix (resource-waste ordering): a cheap, CI-independent cap short-circuit on PR-open, BEFORE any of + // the expensive work below (CI-wait, miner detection, AI review, etc.) ever runs for an over-cap PR. See + // maybeCloseForContributorCapOnOpen's own doc comment. Scoped to "opened" only -- the author's open-PR + // count doesn't change on synchronize/reopened/etc, and the existing end-of-pipeline check (inside + // maybeRunAgentMaintenance below) still runs as defense-in-depth for every other action. + if (payload.action === "opened" && installationId && (await maybeCloseForContributorCapOnOpen(env, deliveryId, installationId, repoFullName, pr, settings))) { + await recordWebhookEvent(env, { + deliveryId, + eventName, + action: payload.action, + installationId: payload.installation?.id, + repositoryFullName: payload.repository?.full_name, + payloadHash: "processed", + status: "processed", + }); + return true; + } // #7372: read BEFORE buildPullRequestAdvisory/persistAdvisory below overwrite "most recent" with THIS // pass's own fresh (visual-finding-less) advisory row -- the advisories table has no separate "current" // column, so reading late would always see this pass's own bookkeeping write instead of the real prior diff --git a/src/queue/transient-locks.ts b/src/queue/transient-locks.ts index 47710a262d..a8f012cc6c 100644 --- a/src/queue/transient-locks.ts +++ b/src/queue/transient-locks.ts @@ -130,3 +130,34 @@ export class PrActuationLockContendedError extends RetryableJobError { this.name = "PrActuationLockContendedError"; } } + +// Per-(repo, author) contributor open-item-cap mutex (#7284-fix, TOCTOU race): every existing lock above +// scopes to ONE PR; the cap-membership decision is inherently about the AUTHOR's whole open-PR set on this +// repo, so a burst of sibling PRs from the same author needs ONE shared lock namespace keyed by author, not +// per-PR — two siblings' cap-checks (or a cap-check racing a merge) must never both proceed against a stale +// view of "how many of this author's PRs are currently open" at the same time. Same short-TTL, best-effort, +// per-holder-token shape as claimPrActuationLock above; see this module's own header comment for why a +// missing claim()/releaseIfValue() primitive fails OPEN rather than fake exclusivity. +const CONTRIBUTOR_CAP_LOCK_TTL_SECONDS = 30; +function contributorCapLockKey(repoFullName: string, authorLogin: string): string { + return `contributor-cap-lock:${repoFullName.toLowerCase()}:${authorLogin.toLowerCase()}`; +} +export async function claimContributorCapLock( + env: Env, + repoFullName: string, + authorLogin: string, +): Promise { + return claimTransientLock( + env, + contributorCapLockKey(repoFullName, authorLogin), + CONTRIBUTOR_CAP_LOCK_TTL_SECONDS, + ); +} +export async function releaseContributorCapLock( + env: Env, + repoFullName: string, + authorLogin: string, + ownerToken: string | null, +): Promise { + await releaseTransientLockIfOwner(env, contributorCapLockKey(repoFullName, authorLogin), ownerToken); +} diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index 927a52c2f4..73b65d3bb3 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -41,6 +41,7 @@ import { import { incr } from "../selfhost/metrics"; import { shouldWaitForOlderSiblings } from "../review/merge-train"; import { captureError } from "../selfhost/sentry"; +import { claimContributorCapLock, releaseContributorCapLock } from "../queue/transient-locks"; // The agent actor name on every audit record — the App acts on the maintainer's behalf per their configured // autonomy (the config IS the authorization; there is no human commenter to authorize, unlike #824). @@ -166,6 +167,17 @@ export type AgentActionExecutionContext = { // ?? the CONTRIBUTOR_CAP_CANCEL_CI_DEFAULT env var) before building the context — the executor itself has no // settings access, only whatever ctx carries, mirroring how agentPaused/agentDryRun are already threaded in. contributorCapCancelCi?: boolean | undefined; + // Pre-merge contributor-cap re-check (#7284-fix, TOCTOU race): a caller-supplied closure (closed over the + // repo's settings/token, resolved before this ctx was built — same "the executor has no settings access" + // shape as every other field here) the executor calls, under the per-(repo, author) mutex + // (claimContributorCapLock), immediately before actually executing a merge. Returns true when still safe to + // merge, false when a fresh check confirms the author is NOW over cap (a sibling opened/closed since this + // PR's own cap check earlier in its pipeline pass) — the executor records a "denied" outcome and skips the + // merge (same idiom as every other pre-condition denial in this loop) rather than duplicating close-planning + // logic inline; the next natural re-evaluation (webhook/sweep) plans a close off the current, accurate cap + // state. Absent when the caller has no cap configured for this repo — merge proceeds exactly as before this + // field existed, zero added cost. + contributorCapMergeRecheck?: (() => Promise) | undefined; // Moderation-rules engine (#selfhost-mod-engine): the repo's PER-REPO override fields, resolved by the // CALLER from RepositorySettings before building the context (same "the executor has no settings access" // shape as contributorCapCancelCi above). Absent/undefined ⇒ inherit the global config's own defaults. The @@ -532,6 +544,32 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE }).catch(() => undefined); } } + // 8c) pre-merge contributor-cap re-check (#7284-fix, TOCTOU race): confirmed live -- a PR whose OWN earlier + // cap check passed can still merge after a sibling's later cap-close made the author over cap, because each + // PR's cap check runs independently with no shared lock. Re-verify right before the irreversible write, + // under the SAME per-author mutex a concurrent sibling's cap-close/wake also acquires, so the two can never + // both act on a stale view of the author's open-PR count. `ctx.contributorCapMergeRecheck` is only ever set + // by the caller when a cap is actually configured for this repo (the common case leaves it undefined) — + // zero added cost, byte-identical to before this field existed. + if (action.actionClass === "merge" && ctx.contributorCapMergeRecheck) { + const authorLogin = ctx.authorLogin ?? ""; + const { acquired, ownerToken } = await claimContributorCapLock(env, ctx.repoFullName, authorLogin); + if (!acquired) { + // "denied", not a thrown error: the next natural re-evaluation (webhook/sweep) picks this PR back up + // with fresh state once the concurrent holder finishes deciding for this same author. + await audit("denied", `contributor cap lock contended for ${ctx.repoFullName} author ${authorLogin} — action not executed`); + continue; + } + try { + const stillUnderCap = await ctx.contributorCapMergeRecheck(); + if (!stillUnderCap) { + await audit("denied", `contributor cap re-check confirmed ${authorLogin} is now over cap on ${ctx.repoFullName} — action not executed`); + continue; + } + } finally { + await releaseContributorCapLock(env, ctx.repoFullName, authorLogin, ownerToken); + } + } // 9) live — perform the real mutation, recording success or the error. try { const detailOverride = await performAction(env, ctx, action); diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index 0eba0c53af..c4f7c5a3b1 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -603,6 +603,46 @@ function contributorCapCloseMessage(authorLogin: string, openCount: number, cap: return `LoopOver closed this because @${authorLogin} has ${openCount} open ${itemNoun}, above ${scopeDescription} of ${cap}. Close or merge an existing one to open a new one. This is an automated maintenance action.`; } +/** + * Plan JUST the per-contributor open-item cap short-circuit (#7284-fix, resource-waste ordering) — factored + * out of planAgentMaintenanceActions so a cheap, CI-independent caller (the PR-open webhook path in + * processors.ts) can plan the SAME close+label actions the full disposition plan would eventually produce, + * without needing gate/ciAggregate/blacklistEntry/etc — none of which this short-circuit ever reads. Pure, + * deterministic, zero-hallucination, identical output to what planAgentMaintenanceActions itself produces for + * the same contributorCapMatch input (that function calls this as its own first check, below). + */ +export function planContributorCapClose(input: { + autonomy: AutonomyPolicy | null | undefined; + authorIsOwner: boolean; + authorIsAdmin: boolean; + authorIsAutomationBot: boolean; + contributorCapMatch: AgentActionPlanInput["contributorCapMatch"]; + contributorCapLabel: AgentActionPlanInput["contributorCapLabel"]; + pr: { headSha?: string | null | undefined }; +}): PlannedAgentAction[] | null { + const capContributor = !input.authorIsOwner && !input.authorIsAdmin && !input.authorIsAutomationBot; + if (input.contributorCapMatch?.matched !== true || !capContributor) return null; + const level = (actionClass: AgentActionClass) => resolveAutonomy(input.autonomy, actionClass); + const acting = (actionClass: AgentActionClass) => isActingAutonomyLevel(level(actionClass)); + const approval = (actionClass: AgentActionClass) => autonomyRequiresApproval(level(actionClass)); + const { authorLogin, openCount, cap, itemKind, scope } = input.contributorCapMatch; + const label = resolveNullableLabel(input.contributorCapLabel, DEFAULT_CONTRIBUTOR_CAP_LABEL); + const actions: PlannedAgentAction[] = []; + if (acting("close")) { + actions.push({ + actionClass: "close", + requiresApproval: approval("close"), + reason: "over the per-contributor open-item cap", + closeReasons: ["over the per-contributor open-item cap"], + closeComment: sanitizePublicComment(contributorCapCloseMessage(authorLogin, openCount, cap, itemKind, scope)), + closeKind: "contributor_cap", + ...(input.pr.headSha ? { expectedHeadSha: input.pr.headSha } : {}), + }); + } + if (acting("close") && label !== null) actions.push({ actionClass: "label", autonomyClass: "close", closeKind: "contributor_cap", requiresApproval: approval("close"), reason: "over the per-contributor open-item cap", label, labelOp: "add" }); + return actions; +} + // The close comment for review-nag cooldown (#2463). DOES interpolate authorLogin/pingCount/maxPings — none of // that is private (the author's own login and their own public @loopover ping count are already public/ // derivable from the PR thread itself), mirroring the contributor-cap close message's same reasoning. @@ -735,31 +775,20 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne } // Per-contributor open-item cap (#2270): same zero-hallucination short-circuit shape as the blacklist above — - // fires ahead of ALL merit/CI/AI analysis, for a CONTRIBUTOR only. Re-checks the owner/admin/bot exemption - // independently of the caller (defense-in-depth, matching the blacklist block's own redundant check above). - const capContributor = !input.authorIsOwner && !input.authorIsAdmin && !input.authorIsAutomationBot; - if (input.contributorCapMatch?.matched === true && capContributor) { - const { authorLogin, openCount, cap, itemKind, scope } = input.contributorCapMatch; - // #label-scoping: same close-autonomy-gated, null-clearable shape as the blacklist label above. - const label = resolveNullableLabel(input.contributorCapLabel, DEFAULT_CONTRIBUTOR_CAP_LABEL); - // Close is pushed BEFORE its coupled label (#label-close-split-brain) — see the closeKind doc comment above. - if (acting("close")) { - actions.push({ - actionClass: "close", - requiresApproval: approval("close"), - reason: "over the per-contributor open-item cap", - closeReasons: ["over the per-contributor open-item cap"], - closeComment: sanitizePublicComment(contributorCapCloseMessage(authorLogin, openCount, cap, itemKind, scope)), - closeKind: "contributor_cap", - // Pin like blacklist/review_nag/copycat/screenshot_table above (#2452): without this an auto_with_approval - // stage persists params.expectedHeadSha as undefined, so decidePendingAgentAction's isUnpinnedRatifyingAction - // guard unconditionally rejects the maintainer's accept before the close ever executes. - ...(input.pr.headSha ? { expectedHeadSha: input.pr.headSha } : {}), - }); - } - if (acting("close") && label !== null) actions.push({ actionClass: "label", autonomyClass: "close", closeKind: "contributor_cap", requiresApproval: approval("close"), reason: "over the per-contributor open-item cap", label, labelOp: "add" }); - return actions; - } + // fires ahead of ALL merit/CI/AI analysis, for a CONTRIBUTOR only. Delegates to planContributorCapClose + // (#7284-fix) so the SAME short-circuit is also directly callable from a CI-independent caller (the PR-open + // webhook path in processors.ts) without needing this function's other inputs (gate/ciAggregate/etc, none of + // which this short-circuit reads). + const capClose = planContributorCapClose({ + autonomy: input.autonomy, + authorIsOwner: input.authorIsOwner, + authorIsAdmin: input.authorIsAdmin, + authorIsAutomationBot: input.authorIsAutomationBot, + contributorCapMatch: input.contributorCapMatch, + contributorCapLabel: input.contributorCapLabel, + pr: input.pr, + }); + if (capClose !== null) return capClose; // Review-nag cooldown (#2463): same zero-hallucination short-circuit shape as the blacklist above — fires // ahead of ALL merit/CI/AI analysis, for a CONTRIBUTOR only. The webhook trigger has already decided "this diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index 1f76a6b014..87bf15494c 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -2081,3 +2081,78 @@ describe("executeAgentMaintenanceActions merge-train gate (#selfhost-merge-train expect(mergePullRequest).not.toHaveBeenCalled(); }); }); + +describe("pre-merge contributor-cap re-check (#7284-fix, TOCTOU race)", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(fetchPullRequestFreshness).mockImplementation(async (_env, args) => ({ + status: "current", + liveHeadSha: args.expectedHeadSha ?? null, + liveState: "open", + liveLabels: [], + })); + }); + + it("absent recheck (no cap configured for this repo): merge proceeds exactly as before this field existed", async () => { + const env = createTestEnv({}); + const outcomes = await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99" }), [merge]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(mergePullRequest).toHaveBeenCalled(); + }); + + it("recheck confirms still under cap: merge proceeds", async () => { + const env = createTestEnv({}); + const recheck = vi.fn(async () => true); + const outcomes = await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", contributorCapMergeRecheck: recheck }), [merge]); + expect(recheck).toHaveBeenCalledTimes(1); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(mergePullRequest).toHaveBeenCalled(); + }); + + it("REGRESSION (#7284-fix, reproduces the #7284-#7289 incident): recheck confirms the author is NOW over cap -- the merge is denied, not executed", async () => { + const env = createTestEnv({}); + const recheck = vi.fn(async () => false); + const outcomes = await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", contributorCapMergeRecheck: recheck }), [merge]); + expect(recheck).toHaveBeenCalledTimes(1); + expect(outcomes[0]).toMatchObject({ actionClass: "merge", outcome: "denied" }); + expect(outcomes[0]?.detail).toMatch(/now over cap/); + expect(mergePullRequest).not.toHaveBeenCalled(); + }); + + it("lock contended (another pass already holds the per-author claim): the merge is denied without ever calling the recheck", async () => { + const env = createTestEnv({}); + // Pre-claim the exact same lock key a concurrent sibling's cap-close/wake would hold. + await env.SELFHOST_TRANSIENT_CACHE?.claim?.("contributor-cap-lock:owner/repo:farmer99", "someone-else-token", 30); + const recheck = vi.fn(async () => true); + const outcomes = await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", contributorCapMergeRecheck: recheck }), [merge]); + expect(recheck).not.toHaveBeenCalled(); + expect(outcomes[0]).toMatchObject({ actionClass: "merge", outcome: "denied" }); + expect(outcomes[0]?.detail).toMatch(/lock contended/); + expect(mergePullRequest).not.toHaveBeenCalled(); + }); + + it("releases the lock after a successful recheck, so a later merge for the SAME author can still acquire it", async () => { + const env = createTestEnv({}); + const recheck = vi.fn(async () => true); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", contributorCapMergeRecheck: recheck }), [merge]); + // If the first call's lock leaked (never released), this second claim attempt would fail. + const secondClaim = await env.SELFHOST_TRANSIENT_CACHE?.claim?.("contributor-cap-lock:owner/repo:farmer99", "probe-token", 30); + expect(secondClaim).toBe(true); + }); + + it("releases the lock after a denied (now-over-cap) recheck too, not just the success path", async () => { + const env = createTestEnv({}); + const recheck = vi.fn(async () => false); + await executeAgentMaintenanceActions(env, ctx({ authorLogin: "farmer99", contributorCapMergeRecheck: recheck }), [merge]); + const secondClaim = await env.SELFHOST_TRANSIENT_CACHE?.claim?.("contributor-cap-lock:owner/repo:farmer99", "probe-token", 30); + expect(secondClaim).toBe(true); + }); + + it("no authorLogin on ctx: the lock key degrades to an empty-string author rather than throwing", async () => { + const env = createTestEnv({}); + const recheck = vi.fn(async () => true); + const outcomes = await executeAgentMaintenanceActions(env, ctx({ authorLogin: undefined, contributorCapMergeRecheck: recheck }), [merge]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(mergePullRequest).toHaveBeenCalled(); + }); +}); diff --git a/test/unit/queue-3.test.ts b/test/unit/queue-3.test.ts index 4480a58f56..ed45be1499 100644 --- a/test/unit/queue-3.test.ts +++ b/test/unit/queue-3.test.ts @@ -2741,10 +2741,11 @@ describe("queue processors", () => { }, }); - // Deterministic gate: closed + labeled (with the configured label) regardless of the AI review's own - // (unrelated, advisory-only) verdict -- AI review runs for this author too (#orb-ai-review-always-review: - // it is no longer gated on confirmed-contributor status), but the cap's close decision doesn't wait on it. - expect(aiCalls).toBe(1); + // #7284-fix: the cap short-circuit now runs on PR-open BEFORE the AI review pipeline ever starts (the + // whole point of the fix -- an over-cap PR must never pay for a review it's about to be closed for + // anyway), so the AI review never runs at all for this PR. Deterministic gate: closed + labeled (with the + // configured label) regardless. + expect(aiCalls).toBe(0); expect(seen.closed).toBe(true); expect(seen.labels).toContain("spam-cap"); const closeAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = 'agent.action.close'").first<{ n: number }>(); @@ -2756,6 +2757,49 @@ describe("queue processors", () => { expect(seen.comments.some((c) => c.includes("@farmer99") && c.includes("3 open pull requests") && c.includes("limit of 2"))).toBe(true); }); + it("pre-merge contributor-cap re-check (#7284-fix): a contributor well UNDER a configured cap merges normally through the full pipeline", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertInstallation(env, { + installation: { id: 123, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { contents: "write", pull_requests: "write", issues: "write" }, events: [] }, + }); + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "owner/repo", + autonomy: { merge: "auto", label: "auto" }, + gatePack: "oss-anti-slop", + }); + // A cap of 2 is configured, but this is the author's ONLY open PR -- well under cap, so the merge must + // actually execute (the pre-merge recheck closure gets constructed AND invoked, confirms still under cap). + await upsertRepoFocusManifest(env, "owner/repo", { settings: { checkRunMode: "off", commentMode: "off", publicSurface: "off", aiReviewMode: "off", reviewCheckMode: "required", contributorOpenPrCap: 2, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" } } }); + await upsertPullRequestFromGitHub(env, "owner/repo", { number: 92, title: "Soloist's only PR", state: "open", user: { login: "soloist1" }, head: { sha: "sha92" }, base: { ref: "main" }, labels: [], body: "" }); + const seen = { merged: false }; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = (init?.method ?? "GET").toUpperCase(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (/\/pulls\/\d+(?:\?|$)/.test(url) && method === "GET" && !url.includes("/pulls/92/")) { + return Response.json({ number: 92, state: "open", user: { login: "soloist1" }, head: { sha: "sha92" }, base: { ref: "main", sha: "base" }, mergeable_state: "clean", labels: [] }); + } + if (url.includes("/pulls/92/files")) return Response.json([{ filename: "src/index.ts", status: "modified", additions: 5, deletions: 1, changes: 6, patch: "@@\n+export const x = 1;" }]); + if (url.includes("/commits/sha92/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/sha92/status")) return Response.json({ state: "success", statuses: [{ context: "ci/build", state: "success", description: "ok" }] }); + if (url.includes("/commits/sha92/check-suites")) return Response.json({ check_suites: [] }); + if (url.includes("/branches/")) return Response.json({ contexts: [] }); + if (url === "https://api.github.com/graphql") return Response.json({ data: { repository: { pullRequest: { reviewDecision: "APPROVED" } } } }); + if (url.includes("/pulls/92/merge") && method === "PUT") { seen.merged = true; return Response.json({ merged: true, sha: "merged-sha92" }); } + if (url.includes("/issues/92/labels")) return Response.json([]); + if (url.includes("/issues/92/comments")) return Response.json([]); + return Response.json({}); + }); + + await processJob(env, { type: "agent-regate-pr", deliveryId: "under-cap-merge", repoFullName: "owner/repo", prNumber: 92, installationId: 123 }); + + expect(seen.merged).toBe(true); + const merge = await env.DB.prepare("select outcome from audit_events where event_type = 'agent.action.merge'").first<{ outcome: string }>(); + expect(merge?.outcome).toBe("completed"); + }); + it("contributor open-PR cap (#2270): a maintainer-named autoCloseExemptLogins entry is exempt from the PER-REPO cap too (not just the install-wide cap)", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await upsertInstallation(env, { @@ -2810,6 +2854,119 @@ describe("queue processors", () => { expect(closeAudit?.n).toBe(0); }); + it("early cap short-circuit (#7284-fix): a contended per-author lock skips the early check and falls through to the normal pipeline, rather than blocking or racing", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "n/a", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await upsertInstallation(env, { + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, target_type: "User", repository_selection: "all", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }], + }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 53, title: "Farmer PR one", state: "open", user: { login: "farmer99" }, head: { sha: "f53" }, labels: [], body: "x" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 54, title: "Farmer PR two", state: "open", user: { login: "farmer99" }, head: { sha: "f54" }, labels: [], body: "y" }); + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", autonomy: { close: "auto", label: "auto" } }); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { commentMode: "all_prs", publicSurface: "comment_only", checkRunMode: "off", contributorCapLabel: "spam-cap", reviewCheckMode: "required", aiReviewMode: "advisory", contributorOpenPrCap: 2 } }, "repo_file"); + // Simulate a concurrent sibling pass already deciding for this same author -- the early check must + // gracefully skip (not throw, not block) rather than race it. + await env.SELFHOST_TRANSIENT_CACHE?.claim?.("contributor-cap-lock:jsonbored/gittensory:farmer99", "someone-else-token", 30); + const seen = { closed: false, labels: [] as string[] }; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/56/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+const ok = true;" }]); + if (url.includes("/pulls/56/reviews")) return Response.json([]); + if (url.includes("/pulls/56/commits")) return Response.json([]); + if ((url.endsWith("/pulls/53") || url.endsWith("/pulls/54")) && method === "GET") return Response.json({ state: "open" }); + if (url.endsWith("/pulls/56") && method === "PATCH") { seen.closed = JSON.parse(String(init?.body ?? "{}")).state === "closed"; return Response.json({ number: 56, state: "closed" }); } + if (url.endsWith("/pulls/56")) return Response.json({ number: 56, state: "open", user: { login: "farmer99" }, head: { sha: "f56" }, mergeable_state: "clean" }); + if (url.includes("/commits/f56/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/f56/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/56/labels") && method === "GET") return Response.json([]); + if (url.includes("/issues/56/labels") && method === "POST") { seen.labels.push(...((JSON.parse(String(init?.body ?? "{}")).labels ?? []) as string[])); return Response.json([]); } + if (url.includes("/issues/56/comments")) return Response.json([]); + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "contributor-cap-lock-contended", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 56, title: "Farmer's 3rd PR", state: "open", user: { login: "farmer99" }, head: { sha: "f56" }, labels: [], body: "x", mergeable_state: "clean", reviewDecision: "APPROVED" }, + }, + }); + + // The early path skipped itself (lock contended) -- unlike the uncontended "3rd PR" test above (aiCalls + // stays 0 there), the pipeline actually ran this time, proving the early check gracefully backed off + // instead of throwing or racing the concurrent holder. + expect(aiCalls).toBeGreaterThan(0); + }); + + it("early cap short-circuit (#7284-fix): close autonomy not granted (observe) plans nothing early -- no crash, falls through to the normal pipeline", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "n/a", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await upsertInstallation(env, { + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" }, target_type: "User", repository_selection: "all", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] }, + repositories: [{ name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }], + }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 53, title: "Farmer PR one", state: "open", user: { login: "farmer99" }, head: { sha: "f53" }, labels: [], body: "x" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 54, title: "Farmer PR two", state: "open", user: { login: "farmer99" }, head: { sha: "f54" }, labels: [], body: "y" }); + // close autonomy is "observe" (not "auto") -- over cap, but nothing may act on it. + await upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", autonomy: { close: "observe", label: "observe" } }); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { settings: { commentMode: "all_prs", publicSurface: "comment_only", checkRunMode: "off", contributorCapLabel: "spam-cap", reviewCheckMode: "required", aiReviewMode: "advisory", contributorOpenPrCap: 2 } }, "repo_file"); + const seen = { closed: false }; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/pulls/57/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+const ok = true;" }]); + if (url.includes("/pulls/57/reviews")) return Response.json([]); + if (url.includes("/pulls/57/commits")) return Response.json([]); + if ((url.endsWith("/pulls/53") || url.endsWith("/pulls/54")) && method === "GET") return Response.json({ state: "open" }); + if (url.endsWith("/pulls/57") && method === "PATCH") { seen.closed = JSON.parse(String(init?.body ?? "{}")).state === "closed"; return Response.json({ number: 57, state: "closed" }); } + if (url.endsWith("/pulls/57")) return Response.json({ number: 57, state: "open", user: { login: "farmer99" }, head: { sha: "f57" }, mergeable_state: "clean" }); + if (url.includes("/commits/f57/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/f57/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/57/labels")) return Response.json([]); + if (url.includes("/issues/57/comments")) return Response.json([]); + return Response.json({}); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "contributor-cap-observe-autonomy", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 57, title: "Farmer's 3rd PR (observe)", state: "open", user: { login: "farmer99" }, head: { sha: "f57" }, labels: [], body: "x", mergeable_state: "clean", reviewDecision: "APPROVED" }, + }, + }); + + // planContributorCapClose returns an empty plan (observe autonomy never "acts") -- the early path correctly + // no-ops instead of crashing on an empty planned array, and the pipeline runs normally afterward. + expect(seen.closed).toBe(false); + expect(aiCalls).toBeGreaterThan(0); + }); + function stubContributorCapCiCancelFetch(seen: { closed: boolean; cancelledIds: number[]; listedStatuses: string[] }, runListResponses: { in_progress?: number[]; queued?: number[] } = {}, cancelResponse: () => Response = () => new Response(null, { status: 202 })) { return async (input: RequestInfo | URL, init?: RequestInit) => { const url = input.toString();