From 28c491168880511a5a02be77b44b7c50af48a94d Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:09:26 -0700 Subject: [PATCH] fix(review): preserve gate verdict on auto-review skip --- src/queue/processors.ts | 15 +++++---- test/unit/queue.test.ts | 72 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 138b562334..f4e92752bc 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -9267,12 +9267,10 @@ async function maybePublishPrPublicSurface( decisionOutcome: gateEvaluation?.conclusion, }, () => - // #3698: a benign auto-review skip (draft, WIP title, too-large, docs-only, base-branch, - // auto-pause) shows the quiet "skipped (reason)" status -- but an IGNORED-AUTHOR skip also - // trips publicSurfaceSkipped, and that path exists specifically so the deterministic gate - // (e.g. the linked-issue hard rule) still shows its REAL, truthful conclusion instead of a - // "skipped" veneer that would let an ignored/excluded author's PR silently bypass it. - autoReviewSkipReason && !publicSurfaceSkipped + // #3698/#security: auto_review skip reasons are AI-review eligibility only. They may come + // from PR-controlled metadata, so the quiet skipped status is safe only after the deterministic + // gate has already passed; failures/holds must publish their real blocking conclusion. + autoReviewSkipReason && !publicSurfaceSkipped && gateEvaluation?.conclusion === "success" ? createOrUpdateSkippedGateCheckRun( env, installationId, @@ -9307,7 +9305,10 @@ async function maybePublishPrPublicSurface( headSha: advisory.headSha, checkRunId: gateCheckResult.id, /* v8 ignore next -- gate-enabled publication always has a gate evaluation. */ - conclusion: autoReviewSkipReason && !publicSurfaceSkipped ? "skipped" : (gateEvaluation?.conclusion ?? null), + conclusion: + autoReviewSkipReason && !publicSurfaceSkipped && gateEvaluation?.conclusion === "success" + ? "skipped" + : (gateEvaluation?.conclusion ?? null), detailsUrl: gateCheckResult.html_url, deliveryId: webhook.deliveryId, }).catch((error) => { diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index f61042d03d..533f9f5733 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3713,6 +3713,78 @@ describe("queue processors", () => { }); }); + it("publishes the deterministic gate conclusion when auto-review eligibility is skipped but the gate does not pass", async () => { + let aiCalls = 0; + let gateConclusion: string | null = null; + let gateSummary = ""; + const env = createTestEnv({ + GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem(), + AI: { run: async () => { aiCalls += 1; return { 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 seedRegateChurnRepo(env); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "off", + publicSurface: "off", + autoLabelEnabled: false, + checkRunMode: "enabled", + gateCheckMode: "enabled", + linkedIssueGateMode: "block", + requireLinkedIssue: true, + autonomy: { merge: "observe", request_changes: "observe" }, + agentDryRun: false, + }); + await upsertRepoFocusManifest(env, "JSONbored/gittensory", { + review: { + auto_review: { skip_drafts: true }, + }, + gate: { linkedIssue: "block" }, + }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 82, + title: "Draft feature", + state: "open", + draft: true, + user: { login: "contributor" }, + head: { sha: "a82" }, + labels: [], + body: "No linked issue here.", + } as never); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 82, status: "complete", reviewsSyncedAt: new Date().toISOString() }); + 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: "installation-credential" }); + if (url.includes("/pulls/82/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.endsWith("/pulls/82")) return Response.json({ number: 82, title: "Draft feature", state: "open", draft: true, user: { login: "contributor" }, head: { sha: "a82" }, labels: [], body: "No linked issue here.", mergeable_state: "clean" }); + if (url.includes("/commits/a82/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a82/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/82/comments")) return method === "POST" ? Response.json({ id: 82 }, { status: 201 }) : Response.json([]); + if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + if (url.includes("/check-runs") && (method === "POST" || method === "PATCH")) { + const body = JSON.parse(String(init?.body ?? "{}")) as { conclusion?: string; output?: { title?: string; summary?: string } }; + if (body.conclusion) gateConclusion = body.conclusion; + gateSummary = `${body.output?.title ?? ""} ${body.output?.summary ?? ""}`; + return Response.json({ id: 982, html_url: "https://github.com/check/982" }, { status: method === "POST" ? 201 : 200 }); + } + return Response.json({}); + }); + + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "auto-review-skip-gate-not-pass", repoFullName: "JSONbored/gittensory", prNumber: 82, installationId: 123 }), + ).resolves.toBeUndefined(); + expect(aiCalls).toBe(0); + expect(gateConclusion).toBe("neutral"); + expect(gateSummary).toContain("Gittensory public check output is intentionally minimal"); + const summary = await env.DB.prepare("select conclusion from check_summaries where repo_full_name = ? and pull_number = ?") + .bind("JSONbored/gittensory", 82) + .first<{ conclusion: string }>(); + expect(summary?.conclusion).not.toBe("skipped"); + }); + it("skips AI review when review.auto_review.skip_labels matches a PR label (#2062)", async () => { let aiCalls = 0; const env = createTestEnv({