diff --git a/src/queue/processors.ts b/src/queue/processors.ts index ef680f25b4..1891342c12 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7465,28 +7465,8 @@ async function maybePublishPrPublicSurface( reviewEligibility.skipReason, webhook.deliveryId, ); - if (gateEnabled) { - const gateCheckResult = await createOrUpdateSkippedGateCheckRun( - env, - installationId, - repoFullName, - advisory, - "Review skipped: ignored author.", - mode, - ); - /* v8 ignore next -- permission-missing audit behavior mirrors the existing skipped-check path above. */ - if (gateCheckResult?.kind === "permission_missing") { - await auditGateCheckPermissionMissing( - env, - author, - repoFullName, - pr.number, - webhook.deliveryId, - gateCheckResult.warning, - ); - } - } - return undefined; + publicSurfaceSkipped = true; + if (!shouldEvaluateGate) return undefined; } // A missing author already forces publicSurfaceSkipped=true above (decidePublicSurface's own // "missing_author" skip), so the guard just above already returns undefined whenever `!author` combines with @@ -8960,7 +8940,12 @@ async function maybePublishPrPublicSurface( decisionOutcome: gateEvaluation?.conclusion, }, () => - autoReviewSkipReason + // #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 ? createOrUpdateSkippedGateCheckRun( env, installationId, @@ -8995,7 +8980,7 @@ async function maybePublishPrPublicSurface( headSha: advisory.headSha, checkRunId: gateCheckResult.id, /* v8 ignore next -- gate-enabled publication always has a gate evaluation. */ - conclusion: autoReviewSkipReason ? "skipped" : (gateEvaluation?.conclusion ?? null), + conclusion: autoReviewSkipReason && !publicSurfaceSkipped ? "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 f3b00f04cc..89dfe198d7 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -13765,7 +13765,7 @@ describe("queue processors", () => { expect(audit?.detail).toBe("bot_author"); }); - it("publishes a skipped review check and no gate failure for ignored authors", async () => { + it("evaluates the gate while suppressing public review output for ignored authors", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await persistRegistrySnapshot( env, @@ -13785,7 +13785,7 @@ describe("queue processors", () => { gateCheckMode: "enabled", linkedIssueGateMode: "block", }); - const calls = { skippedChecks: 0, comments: 0, minerList: 0 }; + const calls = { gateChecks: 0, comments: 0, minerList: 0 }; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { const url = input.toString(); const method = init?.method ?? "GET"; @@ -13796,21 +13796,25 @@ describe("queue processors", () => { if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); if (url.includes("/commits/ignoredauthor123/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); if (url.includes("/issues/56/comments")) { - calls.comments += 1; + if (method !== "GET") calls.comments += 1; return Response.json([]); } if (url.includes("/check-runs") && method === "POST") { - const body = JSON.parse(String(init?.body ?? "{}")) as { status?: string; conclusion?: string; output?: { title?: string; summary?: string } }; + const body = JSON.parse(String(init?.body ?? "{}")) as { status?: string; conclusion?: string; output?: { title?: string } }; + expect(body).toMatchObject({ status: "in_progress", output: { title: "Gittensory Orb Review Agent is evaluating" } }); + expect(body.conclusion).toBeUndefined(); + calls.gateChecks += 1; + return Response.json({ id: 930 }, { status: 201 }); + } + if (url.includes("/check-runs/930") && method === "PATCH") { + const body = JSON.parse(String(init?.body ?? "{}")) as { status?: string; conclusion?: string; output?: { title?: string } }; expect(body).toMatchObject({ status: "completed", - conclusion: "skipped", - output: { - title: "Gittensory Orb Review Agent skipped", - summary: "Review skipped: ignored author.", - }, + conclusion: "failure", + output: { title: "Gittensory Orb Review Agent: No linked issue detected" }, }); - calls.skippedChecks += 1; - return Response.json({ id: 930 }, { status: 201 }); + calls.gateChecks += 1; + return Response.json({ id: 930 }); } return new Response("not found", { status: 404 }); }); @@ -13831,7 +13835,7 @@ describe("queue processors", () => { }, }); - expect(calls).toEqual({ skippedChecks: 1, comments: 0, minerList: 0 }); + expect(calls).toEqual({ gateChecks: 2, comments: 1, minerList: 0 }); const visibilitySkip = await env.DB.prepare("select detail, metadata_json from audit_events where event_type = ? and target_key = ?") .bind("github_app.pr_visibility_skipped", "JSONbored/gittensory#56") .first<{ detail: string; metadata_json: string }>();