diff --git a/package-lock.json b/package-lock.json index 837c5ba41e..f11413e6d2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21029,7 +21029,7 @@ }, "packages/loopover-engine": { "name": "@loopover/engine", - "version": "3.15.0", + "version": "3.15.1", "license": "AGPL-3.0-only", "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.3.205", diff --git a/packages/loopover-engine/package.json b/packages/loopover-engine/package.json index 5c3d191887..c32af23df3 100644 --- a/packages/loopover-engine/package.json +++ b/packages/loopover-engine/package.json @@ -1,6 +1,6 @@ { "name": "@loopover/engine", - "version": "3.15.0", + "version": "3.15.1", "license": "AGPL-3.0-only", "type": "module", "description": "Shared deterministic engine logic for the LoopOver review stack and loopover-miner.", diff --git a/packages/loopover-engine/src/advisory/gate-advisory.ts b/packages/loopover-engine/src/advisory/gate-advisory.ts index 3e5fe8a52b..f21d9dd014 100644 --- a/packages/loopover-engine/src/advisory/gate-advisory.ts +++ b/packages/loopover-engine/src/advisory/gate-advisory.ts @@ -520,6 +520,25 @@ function evaluateGateCheckCore(advisoryResult: Advisory, policy: GateCheckPolicy warnings: gateWarnings, }; } + // Incomplete secret scan (#9082): mirrors the ai_review_inconclusive hold immediately above -- checked only + // once NO deterministic blocker fired (blockers.length === 0), so a REAL secret_leak match (or any other + // configured blocker) on a DIFFERENT file in the same PR still hard-blocks; this can never bury a genuine + // leak in a hold. A patch-less file's content couldn't be fetched/verified within the scan cap -- absence + // of evidence, not evidence of a leak -- so it HOLDS (never closes) and re-evaluates automatically once the + // content becomes retrievable. Explicitly appended to `warnings` (not merged into the severity-filtered + // `gateWarnings`) because the finding is `critical` severity by design (still worth a prominent panel + // signal) rather than `warning`, mirroring the size/guardrail hold shape below. + const secretScanIncompleteFindings = advisoryResult.findings.filter((finding) => finding.code === "secret_scan_incomplete"); + if (secretScanIncompleteFindings.length > 0) { + return { + enabled: true, + conclusion: "neutral", + title: `${LOOPOVER_GATE_CHECK_NAME} — held for human review`, + summary: secretScanIncompleteFindings.map((finding) => sanitizeForCheckRun(finding.title)).join("; "), + blockers: [], + warnings: [...gateWarnings, ...secretScanIncompleteFindings], + }; + } // Manual-review HOLD (#gate-size / #gate-guardrail): a PR that would otherwise PASS but is oversized or touches // a guarded path is HELD for a human (neutral → "manual" verdict) rather than auto-approved — never a failure, // so neutral never blocks the merge (dry-run/advisory friendly) and a contributor PR is never auto-closed for size. @@ -608,9 +627,14 @@ function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPoli } if (code === REVIEW_THREAD_BLOCKER_CODE) return true; // A leaked-secret finding (`secret_leak`) ALWAYS hard-blocks: a committed credential must be removed and - // rotated before merge, with no opt-in. This finding is produced ONLY by the flag-gated safety scan - // (LOOPOVER_REVIEW_SAFETY); when the flag is off the finding never exists, so this branch is unreachable and the - // gate verdict is byte-identical to today. + // rotated before merge, with no opt-in. Two independent producers feed this exact code: the flag-gated + // safety scan (LOOPOVER_REVIEW_SAFETY, off by default) over the reviewed diff, and the UNCONDITIONAL + // patch-less scan (maybeAddSecretLeakFinding, #audit-3.4) that runs for every repo regardless of that flag. + // Either way this code means a CONCRETE match was found. `secret_scan_incomplete` (#9082) is the sibling + // finding from that same patch-less scan for the OPPOSITE case -- verification could not complete, not a + // match -- and is deliberately never routed here: it never reaches this function at all (it resolves via + // the default "off" case below), because it's handled earlier, in evaluateGateCheckCore's + // no-deterministic-blocker branch, as a neutral hold rather than a configured gate blocker. if (code === "secret_leak") return true; // A maintainer pre-merge check (#review-pre-merge-checks) marked `enforce: true` produces this DETERMINISTIC // finding when it fails (a required title/description phrase or label is missing). It always blocks: the diff --git a/packages/loopover-miner/expected-engine.version b/packages/loopover-miner/expected-engine.version index f02113fe87..c3df54c9b8 100644 --- a/packages/loopover-miner/expected-engine.version +++ b/packages/loopover-miner/expected-engine.version @@ -1 +1 @@ -3.15.0 +3.15.1 diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 893fa86e24..0825d84b64 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2499,6 +2499,7 @@ function buildAgentMaintenancePlanInput(args: { pr: PullRequestRecord; openDuplicateSiblings: ReturnType; duplicateWinnerEnabled: boolean; + manualReviewLockContentionResolved: AgentActionPlanInput["manualReviewLockContentionResolved"]; }): AgentActionPlanInput { const { gate, @@ -2524,6 +2525,7 @@ function buildAgentMaintenancePlanInput(args: { pr, openDuplicateSiblings, duplicateWinnerEnabled, + manualReviewLockContentionResolved, } = args; return { conclusion: gate.conclusion, @@ -2590,6 +2592,7 @@ function buildAgentMaintenancePlanInput(args: { ...(unlinkedIssueMatchHold !== undefined ? { unlinkedIssueMatchHold } : {}), ...(aiReviewLowConfidenceHold !== undefined ? { aiReviewLowConfidenceHold } : {}), ...(unlinkedIssueMatchClose !== undefined ? { unlinkedIssueMatchClose } : {}), + manualReviewLockContentionResolved, pr: { mergeableState: liveMergeState ?? pr.mergeableState, reviewDecision: liveReviewDecision ?? pr.reviewDecision, @@ -3204,6 +3207,26 @@ async function runAgentMaintenancePlanAndExecute( aiReviewLowConfidenceHold === undefined && settings.aiReviewSalvageabilityMinScore != null ? resolveAiReviewSalvageableHold(gate, settings, await computeSalvageabilityForTarget(env, repoFullName, pr.number, pr.authorLogin, gate)) : undefined; + // #9009 (narrow scope): track whether manual-review is being held specifically for AI-review lock contention + // (aiReviewLockContendedResult's exact finding shape, ai-review-orchestration.ts) so it can be auto-cleared + // once that transient contention resolves -- see planAgentMaintenanceActions' own doc comment on + // manualReviewLockContentionResolved for why this is the ONLY provenance-tracked auto-clear. The marker is + // keyed per (repo, PR) -- not per head SHA -- since the label persists across pushes and a real verdict may + // only land on a LATER head; a generous 24h TTL means a contention that hasn't cleared naturally in a day no + // longer counts as purely transient and reverts to requiring a maintainer, matching every other hold class. + const manualReviewLockContentionMarkerKey = `manual-review-lock-contention:${repoFullName.toLowerCase()}#${pr.number}`; + const hadPriorLockContentionHold = Boolean(await getTransientKey(env, manualReviewLockContentionMarkerKey)); + const aiReviewLockContentionThisPass = gate.warnings.some( + (finding) => finding.code === "ai_review_inconclusive" && finding.title === "AI review already in progress for this PR head", + ); + if (aiReviewLockContentionThisPass) { + await putTransientKey(env, manualReviewLockContentionMarkerKey, "1", 24 * 60 * 60); + } else if (hadPriorLockContentionHold) { + // Consumed regardless of what else fires this pass -- once contention itself has cleared, the marker's + // OWN reason is stale; a different reason now co-occurring (if any) must stand on its own from here on, + // not inherit this transient provenance on some future pass. + await deleteTransientKey(env, manualReviewLockContentionMarkerKey); + } const planned = planAgentMaintenanceActions( buildAgentMaintenancePlanInput({ gate, @@ -3229,6 +3252,7 @@ async function runAgentMaintenancePlanAndExecute( pr, openDuplicateSiblings, duplicateWinnerEnabled, + manualReviewLockContentionResolved: hadPriorLockContentionHold && !aiReviewLockContentionThisPass, }), ); // Accuracy circuit-breakers (#self-improve / GAP-4): two INDEPENDENT, fail-open precision breakers, chained. diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index 768656b863..90cac036e4 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -279,6 +279,16 @@ export type AgentActionPlanInput = { changesRequestedLabel?: string | null | undefined; migrationCollisionLabel?: string | null | undefined; pendingClosureLabel?: string | null | undefined; + // #9009 (narrow scope): true when a transient marker recorded that manual-review was held/applied because a + // PRIOR pass lost the AI-review lock race (a genuinely transient infra artifact -- see + // aiReviewLockContendedResult, ai-review-orchestration.ts), AND this pass's fresh gate evaluation no longer + // shows that specific contention (the caller clears the marker the moment contention resolves, regardless of + // whether this flag ends up removing the label -- see buildAgentMaintenancePlanInput's caller). Deliberately + // the ONLY provenance-tracked auto-clear condition: every OTHER manual-review trigger (guardrail, migration + // collision, breaker downgrades, unstable-mergeable, a human's own hold, ...) still requires a maintainer to + // lift it, exactly as before -- see the stale-disposition-label-cleanup comment below for why a general + // provenance-free auto-clear is unsafe. Absent/false ⇒ byte-identical to pre-#9009 behavior. + manualReviewLockContentionResolved?: boolean | undefined; // True when the PR author is the repo owner (e.g. JSONbored). Standing rule: owner PRs are NEVER // auto-closed. They may still auto-merge when clean + passing. authorIsOwner: boolean; @@ -1540,5 +1550,29 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne }); } + // #9009 (narrow scope): auto-clear manual-review when the caller's transient marker proves this exact hold + // was applied because a prior pass lost the AI-review lock race, and this fresh pass shows the contention + // has cleared. `manualHoldReason === null` is the load-bearing safety check here: it means NOTHING else this + // pass (guardrail, migration collision, unverified CI, action-required conclusion, a not-review-good verdict, + // ...) currently justifies the label, so removing it cannot silently drop a substantive or human-applied + // hold that happens to coexist. If manualHoldReason is non-null, the block above already re-adds/keeps the + // label for that OTHER reason this same pass, so this branch is naturally a no-op there. + if ( + input.manualReviewLockContentionResolved === true && + manualHoldReason === null && + labels.manualReview !== null && + hasLabel(input.pr.labels, labels.manualReview) && + !actions.some((action) => action.actionClass === "label" && action.label?.toLowerCase() === labels.manualReview?.toLowerCase()) + ) { + actions.push({ + actionClass: "label", + autonomyClass: manualHoldAutonomyClass, + requiresApproval: approval(manualHoldAutonomyClass), + reason: "AI-review lock contention has cleared and nothing else currently holds this PR — removing the transient manual-review hold", + label: labels.manualReview, + labelOp: "remove", + }); + } + return actions; } diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 2398820308..ffb3b66d4e 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -139,6 +139,71 @@ describe("planAgentMaintenanceActions (#778)", () => { expect(classes(plan)).toEqual(["label", "approve", "merge"]); }); + describe("manual-review lock-contention auto-clear (#9009, narrow scope)", () => { + // Same base scenario as the "does NOT clear" test immediately above (conclusion success, clean, the + // label already live) -- the ONLY difference in each case is manualReviewLockContentionResolved. This is + // the exact truth table the caller-side marker (processors.ts) is responsible for computing correctly; + // here we pin what the PURE planner does once given each value. + const base = { conclusion: "success" as const, autonomy: { approve: "auto", merge: "auto", review_state_label: "auto" } as const, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" } as const }; + + it("clears manual-review when the marker says lock contention resolved and nothing else holds the PR", () => { + const plan = planAgentMaintenanceActions(input({ ...base, pr: { labels: [AGENT_LABEL_NEEDS_REVIEW], mergeableState: "clean" }, manualReviewLockContentionResolved: true })); + expect(plan).toContainEqual(expect.objectContaining({ actionClass: "label", label: AGENT_LABEL_NEEDS_REVIEW, labelOp: "remove" })); + }); + + it("does NOT clear when manualReviewLockContentionResolved is absent (byte-identical to pre-#9009)", () => { + // Identical to the test above in every other respect -- proves the new behavior is opt-in via the flag, + // not a silent widening of the existing "does NOT clear" test just above. + const plan = planAgentMaintenanceActions(input({ ...base, pr: { labels: [AGENT_LABEL_NEEDS_REVIEW], mergeableState: "clean" } })); + expect(plan.some((a) => a.actionClass === "label" && a.label === AGENT_LABEL_NEEDS_REVIEW && a.labelOp === "remove")).toBe(false); + }); + + it("does NOT clear when a DIFFERENT reason (a guardrail hit) still holds the PR this same pass", () => { + // The load-bearing safety property: manualHoldReason non-null (guardrailHit here) means something ELSE + // currently justifies the label, so the resolved-lock-contention marker must never override it. The + // label is already correctly live (guardrail's own add is a no-op via hasLabelOrPlanned), so the plan + // is empty either way -- what matters is that it is NOT a "remove". + const plan = planAgentMaintenanceActions( + input({ + conclusion: "success", + autonomy: { merge: "auto", review_state_label: "auto" }, + changedPaths: ["src/settings/agent-actions.ts"], + hardGuardrailGlobs: ["src/settings/**"], + pr: { labels: [AGENT_LABEL_NEEDS_REVIEW], mergeableState: "clean" }, + manualReviewLockContentionResolved: true, + }), + ); + expect(plan.some((a) => a.actionClass === "label" && a.label === AGENT_LABEL_NEEDS_REVIEW && a.labelOp === "remove")).toBe(false); + expect(plan).toEqual([]); + + // Sharper version of the same property: guardrailHit, but the label is NOT yet live (a earlier pass's + // marker predates the guardrail path ever adding it). Here the guardrail's own add DOES fire, proving + // the "resolved" marker never suppresses a genuine, currently-justified hold from being (re)applied. + const withoutLabelYet = planAgentMaintenanceActions( + input({ + conclusion: "success", + autonomy: { merge: "auto", review_state_label: "auto" }, + changedPaths: ["src/settings/agent-actions.ts"], + hardGuardrailGlobs: ["src/settings/**"], + pr: { labels: [], mergeableState: "clean" }, + manualReviewLockContentionResolved: true, + }), + ); + expect(withoutLabelYet).toContainEqual(expect.objectContaining({ actionClass: "label", label: AGENT_LABEL_NEEDS_REVIEW, labelOp: "add" })); + expect(withoutLabelYet.some((a) => a.actionClass === "label" && a.labelOp === "remove")).toBe(false); + }); + + it("plans no spurious removal when the label isn't currently on the PR", () => { + const plan = planAgentMaintenanceActions(input({ ...base, pr: { labels: [], mergeableState: "clean" }, manualReviewLockContentionResolved: true })); + expect(plan.some((a) => a.actionClass === "label" && a.label === AGENT_LABEL_NEEDS_REVIEW && a.labelOp === "remove")).toBe(false); + }); + + it("respects a configured manual-review label name, not just the engine default", () => { + const plan = planAgentMaintenanceActions(input({ ...base, manualReviewLabel: "human-review", pr: { labels: ["human-review"], mergeableState: "clean" }, manualReviewLockContentionResolved: true })); + expect(plan).toContainEqual(expect.objectContaining({ actionClass: "label", label: "human-review", labelOp: "remove" })); + }); + }); + it("clears a stale ready-to-merge label when the PR newly becomes guarded", () => { const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", review_state_label: "auto" }, changedPaths: ["src/settings/agent-actions.ts"], hardGuardrailGlobs: ["src/settings/**"], pr: { labels: [AGENT_LABEL_READY], mergeableState: "clean" } })); expect(plan).toContainEqual(expect.objectContaining({ actionClass: "label", label: AGENT_LABEL_NEEDS_REVIEW, labelOp: "add" })); diff --git a/test/unit/queue-2.test.ts b/test/unit/queue-2.test.ts index a08ca69c22..0633211e59 100644 --- a/test/unit/queue-2.test.ts +++ b/test/unit/queue-2.test.ts @@ -380,6 +380,187 @@ describe("queue processors", () => { expect(outcome.verdict).toBe("match"); }); + it("#9009: auto-clears manual-review once AI-review lock contention (the pass that applied it) resolves on a later pass", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { close: "auto", merge: "auto" }, gatePack: "oss-anti-slop" }); + await upsertRepoFocusManifest(env, "owner/agent-repo", { settings: { checkRunMode: "off", commentMode: "off", publicSurface: "off", reviewCheckMode: "required", aiReviewMode: "block", autoMaintain: { requireApprovals: 0, mergeMethod: "squash" } } }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 91, title: "Contention label autoclear", state: "open", user: { login: "contributor" }, head: { sha: "s91" }, labels: [], body: "Closes #1" }); + await upsertPullRequestFile(env, { repoFullName: "owner/agent-repo", pullNumber: 91, path: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, payload: { patch: "@@\n+export const ok = true;" } }); + vi.spyOn(backfillModule, "fetchRequiredStatusContexts").mockResolvedValue(null); + vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue({ + ciState: "passed", + hasPending: false, + hasVisiblePending: false, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + advisoryHoldDetails: [], + ciCompletenessWarning: null, + }); + + // Stateful label list, mutated by the real ensurePullRequestLabel/removePullRequestLabel POST/DELETE calls -- + // mirrors the linked-issue-hard-rule scaffold above -- so pass 2's live re-sync of the PR sees whatever + // pass 1 actually applied, exactly like a real GitHub repo persists label state between two separate passes. + let mergeCalls = 0; + const liveLabels: string[] = []; + 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 (url.includes("/pulls/91/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.includes("/pulls/91/merge")) { + mergeCalls += 1; + return new Response(null, { status: 204 }); + } + if (url.includes("/pulls/91/reviews") && method === "POST") return Response.json({ id: 1 }); + if (url.includes("/pulls/91/reviews")) return Response.json([]); + if (/\/pulls\/91(?:\?|$)/.test(url)) return Response.json({ number: 91, title: "Contention label autoclear", state: "open", user: { login: "contributor" }, head: { sha: "s91" }, labels: liveLabels.map((name) => ({ name })), body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/s91/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/s91/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/commits/s91/check-suites")) return Response.json({ check_suites: [] }); + if (url.includes("/issues/1") && !url.includes("/issues/91")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + if (url.includes(".loopover.yml")) return new Response("Not Found", { status: 404 }); + if (url.endsWith("/check-runs") && method === "POST") return Response.json({ id: 1 }); + if (url.endsWith("/graphql")) return Response.json({ data: {} }); + if (url.includes("/issues/91/labels") && method === "GET") return Response.json(liveLabels.map((name) => ({ name }))); + if (url.includes("/issues/91/labels") && method === "POST") { + const body = init?.body ? (JSON.parse(String(init.body)) as { labels?: string[] }) : {}; + for (const label of body.labels ?? []) if (!liveLabels.includes(label)) liveLabels.push(label); + return Response.json(liveLabels.map((name) => ({ name })), { status: 200 }); + } + if (url.includes("/labels/") && method === "DELETE") { + const removed = decodeURIComponent(url.slice(url.lastIndexOf("/labels/") + "/labels/".length)); + const index = liveLabels.indexOf(removed); + if (index >= 0) liveLabels.splice(index, 1); + return new Response(null, { status: 204 }); + } + return Response.json({}); + }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + const markerKey = "manual-review-lock-contention:owner/agent-repo#91"; + + // Pass 1: a DIFFERENT holder wins the AI-review lock race for this exact (repo, PR, head, mode) first -- + // this pass's own claim inside runAgentMaintenancePlanAndExecute loses, producing the + // ai_review_inconclusive / "AI review already in progress" finding (aiReviewLockContendedResult). + const contender = await claimAiReviewLock(env, "owner/agent-repo", 91, "s91", "block"); + expect(contender.acquired).toBe(true); + + await processJob(env, { type: "agent-regate-pr", deliveryId: "pass-1-contended", repoFullName: "owner/agent-repo", prNumber: 91, installationId: 9001 }); + + expect(mergeCalls).toBe(0); // held (neutral), never merged + expect(liveLabels).toContain("manual-review"); + expect(await env.SELFHOST_TRANSIENT_CACHE?.get?.(markerKey)).toBe("1"); + + // The contending pass finishes and releases its claim -- the lock is free again for pass 2. + await releaseAiReviewLock(env, "owner/agent-repo", 91, "s91", "block", contender.ownerToken); + + // Pass 2: SAME head SHA, no push in between. This pass now wins the lock itself, runs a clean AI review + // (the stubbed AI.run above returns no blockers), and the gate passes outright -- contention has resolved + // AND nothing else this pass justifies the hold, so planAgentMaintenanceActions clears the stale label. + await processJob(env, { type: "agent-regate-pr", deliveryId: "pass-2-resolved", repoFullName: "owner/agent-repo", prNumber: 91, installationId: 9001 }); + + expect(liveLabels).not.toContain("manual-review"); + expect(await env.SELFHOST_TRANSIENT_CACHE?.get?.(markerKey)).toBeNull(); + }); + + it("#9009: does NOT clear manual-review across passes when a DIFFERENT reason (a guardrail hit) still holds the PR", async () => { + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => ({ response: JSON.stringify({ assessment: "Looks fine.", blockers: [], nits: [], suggestions: [] }) }) } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: {}, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); + // A hard-guardrail glob match on the changed path (below) HOLDS the gate independent of AI-review lock + // contention -- the marker's provenance must never suppress THIS reason once contention itself clears. + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { close: "auto", merge: "auto" }, gatePack: "oss-anti-slop" }); + await upsertRepoFocusManifest(env, "owner/agent-repo", { settings: { checkRunMode: "off", commentMode: "off", publicSurface: "off", reviewCheckMode: "required", aiReviewMode: "block", hardGuardrailGlobs: ["src/guarded/**"], autoMaintain: { requireApprovals: 0, mergeMethod: "squash" } } }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 92, title: "Contention + guardrail", state: "open", user: { login: "contributor" }, head: { sha: "s92" }, labels: [], body: "Closes #1" }); + await upsertPullRequestFile(env, { repoFullName: "owner/agent-repo", pullNumber: 92, path: "src/guarded/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, payload: { patch: "@@\n+export const ok = true;" } }); + vi.spyOn(backfillModule, "fetchRequiredStatusContexts").mockResolvedValue(null); + vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue({ + ciState: "passed", + hasPending: false, + hasVisiblePending: false, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + advisoryHoldDetails: [], + ciCompletenessWarning: null, + }); + + let mergeCalls = 0; + const liveLabels: string[] = []; + 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 (url.includes("/pulls/92/files")) return Response.json([{ filename: "src/guarded/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.includes("/pulls/92/merge")) { + mergeCalls += 1; + return new Response(null, { status: 204 }); + } + if (url.includes("/pulls/92/reviews") && method === "POST") return Response.json({ id: 1 }); + if (url.includes("/pulls/92/reviews")) return Response.json([]); + if (/\/pulls\/92(?:\?|$)/.test(url)) return Response.json({ number: 92, title: "Contention + guardrail", state: "open", user: { login: "contributor" }, head: { sha: "s92" }, labels: liveLabels.map((name) => ({ name })), body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/s92/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/s92/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/commits/s92/check-suites")) return Response.json({ check_suites: [] }); + if (url.includes("/issues/1") && !url.includes("/issues/92")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + if (url.includes(".loopover.yml")) return new Response("Not Found", { status: 404 }); + if (url.endsWith("/check-runs") && method === "POST") return Response.json({ id: 1 }); + if (url.endsWith("/graphql")) return Response.json({ data: {} }); + if (url.includes("/issues/92/labels") && method === "GET") return Response.json(liveLabels.map((name) => ({ name }))); + if (url.includes("/issues/92/labels") && method === "POST") { + const body = init?.body ? (JSON.parse(String(init.body)) as { labels?: string[] }) : {}; + for (const label of body.labels ?? []) if (!liveLabels.includes(label)) liveLabels.push(label); + return Response.json(liveLabels.map((name) => ({ name })), { status: 200 }); + } + if (url.includes("/labels/") && method === "DELETE") { + const removed = decodeURIComponent(url.slice(url.lastIndexOf("/labels/") + "/labels/".length)); + const index = liveLabels.indexOf(removed); + if (index >= 0) liveLabels.splice(index, 1); + return new Response(null, { status: 204 }); + } + return Response.json({}); + }); + vi.setSystemTime(new Date("2026-05-28T02:00:00.000Z")); + + const markerKey = "manual-review-lock-contention:owner/agent-repo#92"; + const contender = await claimAiReviewLock(env, "owner/agent-repo", 92, "s92", "block"); + expect(contender.acquired).toBe(true); + + await processJob(env, { type: "agent-regate-pr", deliveryId: "pass-1-contended", repoFullName: "owner/agent-repo", prNumber: 92, installationId: 9001 }); + expect(liveLabels).toContain("manual-review"); + expect(await env.SELFHOST_TRANSIENT_CACHE?.get?.(markerKey)).toBe("1"); + + await releaseAiReviewLock(env, "owner/agent-repo", 92, "s92", "block", contender.ownerToken); + + // Pass 2: the lock is free and the AI review comes back clean -- contention itself has resolved -- but the + // guardrail hit on src/guarded/a.ts is untouched and still independently justifies the hold. The label must + // stay. + await processJob(env, { type: "agent-regate-pr", deliveryId: "pass-2-guardrail-still-hits", repoFullName: "owner/agent-repo", prNumber: 92, installationId: 9001 }); + + expect(mergeCalls).toBe(0); + expect(liveLabels).toContain("manual-review"); // still held, for the OTHER reason + expect(await env.SELFHOST_TRANSIENT_CACHE?.get?.(markerKey)).toBeNull(); // contention's OWN marker still consumed + }); + it("#4603: the SAME sub-floor defect one-shot-closes when aiReviewLowConfidenceDisposition is explicitly one_shot", async () => { let aiCalls = 0; const env = createTestEnv({