From 18ea65827f46df7e3b61c356b4cf71a27324257b Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 21:52:56 -0700 Subject: [PATCH 1/2] fix(review): keep gate evaluation for ignored authors --- src/queue/processors.ts | 25 +++---------------------- test/unit/queue.test.ts | 28 ++++++++++++++++------------ 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index ef680f25b4..d712f8d91d 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 @@ -7602,6 +7582,7 @@ async function maybePublishPrPublicSurface( // promise can gate the public-surface computation too, not just this label decision (#gate-only-type-labels). if ( typeLabelsEnabled && + !publicSurfaceSkipped && !settings.agentPaused && decision.skipReason !== "miner_detection_unavailable" && decision.skipReason !== "not_official_gittensor_miner" 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 }>(); From eb3810b9bb70fd313e62d98a6525d76943143b10 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 6 Jul 2026 02:17:29 -0700 Subject: [PATCH 2/2] fix(review): keep the real gate verdict for ignored authors past #3694 #3694 (merged independently, unaware of this fix) made the gate check-run publish a quiet "skipped (reason)" status instead of the evaluated gate conclusion whenever auto-review is skipped -- including for an ignored author, since evaluateAutoReviewSkipReason checks the same review.auto_review.ignore_authors config this PR's own eligibility check does. That silently reintroduced the exact bypass this PR exists to close: an ignored author's PR would show "skipped" on its gate check-run instead of a real hard-rule failure (e.g. no linked issue), even though the gate evaluation itself was computed correctly. Only take the "skipped status" shortcut when publicSurfaceSkipped is false. publicSurfaceSkipped is already the flag this PR's ignored-author path (and decidePublicSurface's own skip conditions) sets specifically so the deterministic gate keeps reporting truthfully even when public review output is suppressed -- reusing it here keeps the two mechanisms consistent instead of introducing new reason-string matching. A benign content-based skip (draft, WIP title, too-large, docs-only, base-branch, auto-pause) never sets publicSurfaceSkipped, so #3694's quiet status is unaffected for those cases. --- src/queue/processors.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index d712f8d91d..1891342c12 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7582,7 +7582,6 @@ async function maybePublishPrPublicSurface( // promise can gate the public-surface computation too, not just this label decision (#gate-only-type-labels). if ( typeLabelsEnabled && - !publicSurfaceSkipped && !settings.agentPaused && decision.skipReason !== "miner_detection_unavailable" && decision.skipReason !== "not_official_gittensor_miner" @@ -8941,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, @@ -8976,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) => {