Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) => {
Expand Down
72 changes: 72 additions & 0 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down