From d8f9542d112b2f37abd0b5f91c95111dc0afdf2d Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 03:13:37 -0700 Subject: [PATCH] fix(review): exempt the owner/admin/automation-bot from the manual-review freeze The maintainer-gated freeze (#3461) reused the last published AI review for any PR carrying the manual-review label, with no author exemption. Confirmed live on PR #3476: pushing genuine follow-up commits to the owner's own held PR kept replaying the ORIGINAL, now-stale AI verdict instead of evaluating the new commits, because github_app.ai_review_frozen_reuse fired on every push. The anti-gaming concern the freeze exists for is specific to a contributor iterating pushes against the bot; it never applies to the repo owner, an ADMIN_GITHUB_LOGINS fleet operator, or a protected automation bot, matching the exemption this codebase already grants those authors everywhere else (auto-close, review-nag, contributor caps). Root-caused via the live audit_events trail on the self-host VPS, which showed ai_review_frozen_reuse firing on every one of #3476's own follow-up commits despite the PR being owner-authored. --- src/queue/processors.ts | 27 +++++++-- test/unit/queue.test.ts | 119 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 6 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 526277e747..aae675b750 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7844,18 +7844,33 @@ async function maybePublishPrPublicSurface( settings.contributorBlacklist, ); // #regate-churn (maintainer-gated freeze): once a PR is held for manual review -- the manual-review label is - // already on it from a PRIOR pass -- a repeat contributor push must not buy a fresh, real AI review. That is + // already on it from a PRIOR pass -- a repeat CONTRIBUTOR push must not buy a fresh, real AI review. That is // exactly the gaming surface this closes: iterating pushes hoping to slip a green verdict past the bot (or // just to see what the AI says next), at real LLM cost, instead of waiting for the human judgment the hold // exists for. Only an explicit maintainer/collaborator retrigger (the PR-panel checkbox, which sets - // `webhook.forceAiReview`) may unfreeze it. CI/mergeable facts and label/assignee reconciliation are - // UNAFFECTED — both are recomputed fresh every pass below regardless of this flag; only the AI's own - // substantive verdict/findings are pinned. The very FIRST pass that establishes the hold is never frozen: the - // label is applied by the disposition executor AFTER this pass publishes, so `pr.labels` (read at the top of - // this sweep, before that write) does not carry it yet. + // `webhook.forceAiReview`) may unfreeze a contributor's held PR. CI/mergeable facts and label/assignee + // reconciliation are UNAFFECTED — both are recomputed fresh every pass below regardless of this flag; only + // the AI's own substantive verdict/findings are pinned. The very FIRST pass that establishes the hold is + // never frozen: the label is applied by the disposition executor AFTER this pass publishes, so `pr.labels` + // (read at the top of this sweep, before that write) does not carry it yet. + // + // #freeze-owner-exemption (incident, confirmed live 2026-07-05 on PR #3476): the freeze must NOT apply to + // the repo owner's own PR, an ADMIN_GITHUB_LOGINS fleet-operator's, or a protected automation bot's -- same + // exemption this codebase already grants these authors everywhere else (auto-close, review-nag, contributor + // caps). The gaming concern this freeze exists to close is specific to a CONTRIBUTOR iterating pushes + // against the bot; it never applies to the maintainer's own PRs. Without this exemption, a maintainer + // pushing a genuine fix to their OWN held PR kept replaying the ORIGINAL (now-stale) AI verdict pass after + // pass, hiding the maintainer's own fix from the review meant to evaluate it -- confirmed live via + // `github_app.ai_review_frozen_reuse` firing on every one of #3476's own follow-up commits. const manualReviewLabel = settings.manualReviewLabel === null ? null : (settings.manualReviewLabel ?? AGENT_LABEL_NEEDS_REVIEW); + const authorIsExemptFromFreeze = + author !== null && + (author.toLowerCase() === repoOwnerLoginFromFullName(repoFullName).toLowerCase() || + parseGitHubLoginList(env.ADMIN_GITHUB_LOGINS).has(author.toLowerCase()) || + isProtectedAutomationAuthor(author)); const isFrozenForManualReview = webhook.forceAiReview !== true && + !authorIsExemptFromFreeze && manualReviewLabel !== null && pr.labels.some((label) => label.toLowerCase() === manualReviewLabel.toLowerCase()); const aiReviewWillRun = diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 4467adb372..3389d8e156 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -4147,6 +4147,125 @@ describe("queue processors", () => { expect(bypassAudit?.outcome).toBe("completed"); // the retrigger genuinely bypassed the freeze, not just a coincidental reuse }); + it("#freeze-owner-exemption (incident, confirmed live on PR #3476): the repo owner's OWN held PR is never frozen -- a new push gets a fresh AI review", async () => { + // The owner pushing a genuine fix to their OWN held PR must not keep replaying the ORIGINAL, now-stale + // verdict pass after pass -- confirmed live via github_app.ai_review_frozen_reuse firing on every one of + // #3476's own follow-up commits, hiding the owner's own fix from the review meant to evaluate it. The + // anti-gaming concern the freeze exists for is specific to a CONTRIBUTOR iterating pushes against the + // bot; it must never apply to the repo owner, matching the same exemption this codebase already grants + // owner/admin/automation-bot authors everywhere else (auto-close, review-nag, contributor caps). + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Fresh owner fix.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env, { publicSurface: "comment_only" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 79, title: "Owner's held PR", state: "open", user: { login: "JSONbored" }, head: { sha: "a79-v1" }, labels: [{ name: "manual-review" }], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 79, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 79, "a79-v1", "block", { notes: "Original stale review.", reviewerCount: 1 }); + await markAiReviewPublished(env, "JSONbored/gittensory", 79, "a79-v1"); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + // The owner pushed a genuine fix: a new head SHA, still carrying the manual-review label. + if (url.includes("/pulls/79/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 2, deletions: 0, changes: 2, patch: "@@\n+export const fixed = true;" }]); + if (url.endsWith("/pulls/79")) return Response.json({ number: 79, title: "Owner's held PR", state: "open", user: { login: "JSONbored" }, head: { sha: "a79-v2" }, labels: [{ name: "manual-review" }], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a79-v2/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a79-v2/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/issues/79/comments")) return method === "POST" || method === "PATCH" ? Response.json({ id: 1 }, { status: 201 }) : Response.json([]); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + await processJob(env, { type: "agent-regate-pr", deliveryId: "owner-held-push", repoFullName: "JSONbored/gittensory", prNumber: 79, installationId: 123 }); + + expect(aiCalls).toBeGreaterThan(0); // NOT frozen -- the owner's own push gets a real, fresh AI review + const freezeAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_frozen_reuse", "JSONbored/gittensory#79") + .first<{ n: number }>(); + expect(freezeAudit?.n).toBe(0); // never took the frozen-reuse path at all + }); + + it("#freeze-owner-exemption: an ADMIN_GITHUB_LOGINS fleet-operator's held PR is never frozen either", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + ADMIN_GITHUB_LOGINS: "fleet-admin", + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Fresh admin fix.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env, { publicSurface: "comment_only" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 80, title: "Admin's held PR", state: "open", user: { login: "fleet-admin" }, head: { sha: "a80-v1" }, labels: [{ name: "manual-review" }], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 80, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 80, "a80-v1", "block", { notes: "Original stale review.", reviewerCount: 1 }); + await markAiReviewPublished(env, "JSONbored/gittensory", 80, "a80-v1"); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + if (url.includes("/pulls/80/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 2, deletions: 0, changes: 2, patch: "@@\n+export const fixed = true;" }]); + if (url.endsWith("/pulls/80")) return Response.json({ number: 80, title: "Admin's held PR", state: "open", user: { login: "fleet-admin" }, head: { sha: "a80-v2" }, labels: [{ name: "manual-review" }], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a80-v2/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a80-v2/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/issues/80/comments")) return method === "POST" || method === "PATCH" ? Response.json({ id: 1 }, { status: 201 }) : Response.json([]); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + await processJob(env, { type: "agent-regate-pr", deliveryId: "admin-held-push", repoFullName: "JSONbored/gittensory", prNumber: 80, installationId: 123 }); + + expect(aiCalls).toBeGreaterThan(0); + const freezeAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_frozen_reuse", "JSONbored/gittensory#80") + .first<{ n: number }>(); + expect(freezeAudit?.n).toBe(0); + }); + + it("#freeze-owner-exemption: a protected automation bot's held PR is never frozen either", async () => { + let aiCalls = 0; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { response: JSON.stringify({ assessment: "Fresh bot fix.", blockers: [], nits: [], suggestions: [] }) }; } } as unknown as Ai, + AI_SUMMARIES_ENABLED: "true", + AI_PUBLIC_COMMENTS_ENABLED: "true", + AI_DAILY_NEURON_BUDGET: "100000", + }); + await seedRegateChurnRepo(env, { publicSurface: "comment_only" }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 81, title: "Bot's held PR", state: "open", user: { login: "dependabot[bot]" }, head: { sha: "a81-v1" }, labels: [{ name: "manual-review" }], body: "Closes #1" }); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 81, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + await putCachedAiReview(env, "JSONbored/gittensory", 81, "a81-v1", "block", { notes: "Original stale review.", reviewerCount: 1 }); + await markAiReviewPublished(env, "JSONbored/gittensory", 81, "a81-v1"); + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); + if (url.includes("/pulls/81/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 2, deletions: 0, changes: 2, patch: "@@\n+export const fixed = true;" }]); + if (url.endsWith("/pulls/81")) return Response.json({ number: 81, title: "Bot's held PR", state: "open", user: { login: "dependabot[bot]" }, head: { sha: "a81-v2" }, labels: [{ name: "manual-review" }], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a81-v2/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a81-v2/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); + if (url.includes("/issues/81/comments")) return method === "POST" || method === "PATCH" ? Response.json({ id: 1 }, { status: 201 }) : Response.json([]); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + return Response.json({}); + }); + + await processJob(env, { type: "agent-regate-pr", deliveryId: "bot-held-push", repoFullName: "JSONbored/gittensory", prNumber: 81, installationId: 123 }); + + expect(aiCalls).toBeGreaterThan(0); + const freezeAudit = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_frozen_reuse", "JSONbored/gittensory#81") + .first<{ n: number }>(); + expect(freezeAudit?.n).toBe(0); + }); + it("maintainer-gated freeze never engages when manualReviewLabel is explicitly disabled (null)", async () => { let aiCalls = 0; const env = createTestEnv({