diff --git a/src/queue/processors.ts b/src/queue/processors.ts index b44982356c..31eaf15b2d 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -362,6 +362,7 @@ import { composeManifestReviewInstructions, filterReviewFilesForAi, resolvePullRequestAutoReviewSkipReason, + resolveAutoReviewSkipSummary, resolveRepoEnrichmentToggles, resolveReviewAutoReviewConfig, resolveReviewPathInstructions, @@ -6311,14 +6312,22 @@ export async function auditPullRequestAutoReviewSkip( skipReason: string; }, ): Promise { + const summary = resolveAutoReviewSkipSummary(args.skipReason); await recordAuditEvent(env, { eventType: "github_app.ai_review_auto_review_skipped", actor: args.actor, targetKey: `${args.repoFullName}#${args.pullNumber}`, outcome: "completed", detail: args.skipReason, - metadata: { deliveryId: args.deliveryId, repoFullName: args.repoFullName, headSha: args.headSha ?? null }, + metadata: { deliveryId: args.deliveryId, repoFullName: args.repoFullName, headSha: args.headSha ?? null, summary }, }).catch(() => undefined); + await recordGithubProductUsage(env, "ai_review_auto_review_skipped", { + actor: args.actor, + repoFullName: args.repoFullName, + targetKey: `${args.repoFullName}#${args.pullNumber}`, + outcome: "skipped", + metadata: { skipReason: args.skipReason, summary }, + }); } /** Resolve auto-review eligibility for a PR, loading the manifest only when cheaper skip predicates pass. (#1954) */ @@ -8872,21 +8881,31 @@ async function maybePublishPrPublicSurface( decisionOutcome: gateEvaluation?.conclusion, }, () => - createOrUpdateGateCheckRun( - env, - installationId, - repoFullName, - advisory, - gatePolicy, - { - checkRunId: pendingGateCheckRunId, - // #5 (audit): publish the AUTHORITATIVE surface-lane-merged verdict so the check-run conclusion matches - // the disposition; without this the check re-derives the generic verdict and shows green on a surface- - // lane reject/manual PR that is actually auto-closed/held. Undefined (gate off) ⇒ re-derive (identical). - gate: gateEvaluation, - }, - mode, - ), + autoReviewSkipReason + ? createOrUpdateSkippedGateCheckRun( + env, + installationId, + repoFullName, + advisory, + resolveAutoReviewSkipSummary(autoReviewSkipReason), + mode, + { checkRunId: pendingGateCheckRunId }, + ) + : createOrUpdateGateCheckRun( + env, + installationId, + repoFullName, + advisory, + gatePolicy, + { + checkRunId: pendingGateCheckRunId, + // #5 (audit): publish the AUTHORITATIVE surface-lane-merged verdict so the check-run conclusion matches + // the disposition; without this the check re-derives the generic verdict and shows green on a surface- + // lane reject/manual PR that is actually auto-closed/held. Undefined (gate off) ⇒ re-derive (identical). + gate: gateEvaluation, + }, + mode, + ), ); if (gateCheckResult?.kind === "published") { gateFinalized = true; @@ -8897,7 +8916,7 @@ async function maybePublishPrPublicSurface( headSha: advisory.headSha, checkRunId: gateCheckResult.id, /* v8 ignore next -- gate-enabled publication always has a gate evaluation. */ - conclusion: gateEvaluation?.conclusion ?? null, + conclusion: autoReviewSkipReason ? "skipped" : (gateEvaluation?.conclusion ?? null), detailsUrl: gateCheckResult.html_url, deliveryId: webhook.deliveryId, }).catch((error) => { diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 0b4b757832..706973472f 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -2314,6 +2314,36 @@ export function evaluateAutoReviewSkipReason(config: AutoReviewConfig, input: Au return null; } +/** Known auto-review skip reason tokens returned by `evaluateAutoReviewSkipReason`. (#2067) */ +export type AutoReviewSkipReason = + | "review skipped (draft)" + | "review skipped (ignored author)" + | "review skipped (WIP title)" + | "review skipped (label)" + | "review skipped (docs only)" + | "review skipped (too large)" + | "review skipped (base branch out of scope)" + | "review paused (commit threshold)"; + +/** Public-safe one-line summaries for each auto-review skip reason — mirrors settings-preview `SKIP_SUMMARY`. (#2067) */ +export const AUTO_REVIEW_SKIP_SUMMARY: Record = { + "review skipped (draft)": "AI review is skipped for draft pull requests while review.auto_review.skip_drafts is enabled.", + "review skipped (ignored author)": "The author matches review.auto_review.ignore_authors, so AI review is skipped.", + "review skipped (WIP title)": "The title matches review.auto_review.ignore_title_keywords, so AI review is skipped.", + "review skipped (label)": "A configured review.auto_review.skip_labels label is present, so AI review is skipped.", + "review skipped (docs only)": "Every changed file is documentation while review.auto_review.skip_docs_only is enabled, so AI review is skipped.", + "review skipped (too large)": "The pull request exceeds review.auto_review.max_added_lines or max_files, so AI review is skipped.", + "review skipped (base branch out of scope)": "The base branch is outside review.auto_review.base_branches, so AI review is skipped.", + "review paused (commit threshold)": "Published AI review count reached review.auto_review.auto_pause_after_reviewed_commits, so further AI review is paused.", +}; + +export function resolveAutoReviewSkipSummary(skipReason: string): string { + if (Object.prototype.hasOwnProperty.call(AUTO_REVIEW_SKIP_SUMMARY, skipReason)) { + return AUTO_REVIEW_SKIP_SUMMARY[skipReason as AutoReviewSkipReason]; + } + return skipReason; +} + export function resolvePullRequestAutoReviewSkipReason(args: { forceAiReview?: boolean | undefined; manifest: FocusManifest | null; diff --git a/test/unit/auto-review-wiring.test.ts b/test/unit/auto-review-wiring.test.ts index 621e06cde1..70eaedb701 100644 --- a/test/unit/auto-review-wiring.test.ts +++ b/test/unit/auto-review-wiring.test.ts @@ -188,6 +188,9 @@ describe("review.auto_review wiring (#1954)", () => { eventType: "github_app.ai_review_auto_review_skipped", detail: "review skipped (ignored author)", targetKey: "acme/widgets#7", + metadata: expect.objectContaining({ + summary: "The author matches review.auto_review.ignore_authors, so AI review is skipped.", + }), }), ); diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 7aab87f9e4..67997e78ae 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -21,6 +21,8 @@ import { resolveReviewPreMergeChecks, composeRepoReviewContext, evaluateAutoReviewSkipReason, + resolveAutoReviewSkipSummary, + AUTO_REVIEW_SKIP_SUMMARY, resolveAutoReviewConfig, resolveReviewPromptOverrides, composeManifestReviewInstructions, @@ -3167,6 +3169,14 @@ describe("review.auto_review (#1954 / #2038–#2041)", () => { expect(evaluateAutoReviewSkipReason({ ...empty, autoPauseAfterReviewedCommits: null }, { ...input, reviewedCommitCount: 99 })).toBeNull(); }); + it("resolveAutoReviewSkipSummary maps every known skip reason to a public-safe sentence (#2067)", () => { + for (const [reason, summary] of Object.entries(AUTO_REVIEW_SKIP_SUMMARY)) { + expect(resolveAutoReviewSkipSummary(reason)).toBe(summary); + expect(summary.length).toBeGreaterThan(0); + } + expect(resolveAutoReviewSkipSummary("review skipped (unknown)")).toBe("review skipped (unknown)"); + }); + it("parses auto_pause_after_reviewed_commits with bounds validation (#2042)", () => { const ok = parseFocusManifest({ review: { auto_review: { auto_pause_after_reviewed_commits: 3 } } }); expect(ok.review.autoReview.autoPauseAfterReviewedCommits).toBe(3); diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 43dc6979ba..b97e442b51 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3244,6 +3244,65 @@ describe("queue processors", () => { expect(audit?.detail).toBe("review skipped (draft)"); }); + it("publishes a skipped Orb review check with a human summary when auto-review eligibility fails (#2067)", async () => { + let aiCalls = 0; + let gateConclusion: string | null = null; + let gateSummary: string | null = null; + 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 upsertRepoFocusManifest(env, "JSONbored/gittensory", { review: { auto_review: { skip_drafts: true } } }); + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + number: 81, + title: "Draft feature", + state: "open", + draft: true, + user: { login: "contributor" }, + head: { sha: "a81" }, + labels: [], + body: "Closes #1", + } as never); + await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 81, 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: "fake-installation-token" }); + if (url.includes("/pulls/81/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/81")) return Response.json({ number: 81, title: "Draft feature", state: "open", draft: true, user: { login: "contributor" }, head: { sha: "a81" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.includes("/commits/a81/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/commits/a81/status")) return Response.json({ state: "success", statuses: [] }); + if (url.includes("/issues/81/comments")) return method === "POST" ? Response.json({ id: 81 }, { status: 201 }) : Response.json([]); + if (url.includes("/issues/1")) 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("/check-runs") && (method === "POST" || method === "PATCH")) { + const body = JSON.parse(String(init?.body ?? "{}")) as { conclusion?: string; output?: { summary?: string } }; + if (body.conclusion) gateConclusion = body.conclusion; + if (body.output?.summary) gateSummary = body.output.summary; + return Response.json({ id: 981, html_url: "https://github.com/check/981" }, { status: method === "POST" ? 201 : 200 }); + } + return Response.json({}); + }); + + await expect( + processJob(env, { type: "agent-regate-pr", deliveryId: "auto-review-skip-status", repoFullName: "JSONbored/gittensory", prNumber: 81, installationId: 123 }), + ).resolves.toBeUndefined(); + expect(aiCalls).toBe(0); + expect(gateConclusion).toBe("skipped"); + expect(gateSummary).toBe("AI review is skipped for draft pull requests while review.auto_review.skip_drafts is enabled."); + const audit = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = ? and target_key = ?") + .bind("github_app.ai_review_auto_review_skipped", "JSONbored/gittensory#81") + .first<{ detail: string; metadata_json: string }>(); + expect(audit?.detail).toBe("review skipped (draft)"); + expect(JSON.parse(audit?.metadata_json ?? "{}")).toMatchObject({ + summary: "AI review is skipped for draft pull requests while review.auto_review.skip_drafts is enabled.", + }); + }); + it("skips AI review when review.auto_review.skip_labels matches a PR label (#2062)", async () => { let aiCalls = 0; const env = createTestEnv({