From 88978f11ca932e5f1981c639781edad7d113a252 Mon Sep 17 00:00:00 2001 From: philluiz2323 Date: Sun, 14 Jun 2026 10:14:51 -0700 Subject: [PATCH] fix(gate): finalize the Gate on a transient 403 at completion PR #655 finalizes the pending in_progress Gate check on any failure that throws. But a 403 on the completion call is classified as permission_missing and returns without throwing, so the catch never runs and the pending check is orphaned in_progress forever. The pending check already posted (pendingGateCheckRunId is set), proving the App had Checks:write, so a 403 at completion is almost always a transient secondary-rate-limit rather than a real revocation. Finalize the pending check to neutral in the permission_missing branch too (mirroring the catch); a genuine revocation makes this PATCH 403 as well and it is swallowed. Closes #730 --- src/queue/processors.ts | 9 +++++++ test/unit/queue.test.ts | 60 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 8250481b91..d254956686 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -1133,6 +1133,15 @@ async function maybePublishPrPublicSurface( if (gateCheckResult?.kind === "published") gateFinalized = true; if (gateCheckResult?.kind === "permission_missing") { await auditGateCheckPermissionMissing(env, author, repoFullName, pr.number, webhook.deliveryId, gateCheckResult.warning); + // A 403 on the COMPLETION call is classified as permission_missing and does NOT throw, so the catch + // below never runs and the pending in_progress check would be orphaned. But the pending check already + // posted (pendingGateCheckRunId is set), proving the App had Checks:write — so a 403 here is almost + // always a transient secondary-rate-limit, not a real revocation. Finalize the pending check to + // neutral (mirrors the catch); if it were a genuine revocation this PATCH also 403s and is swallowed. + if (pendingGateCheckRunId !== undefined && !gateFinalized) { + await createOrUpdateErroredGateCheckRun(env, installationId, repoFullName, advisory, { checkRunId: pendingGateCheckRunId }).catch(() => undefined); + gateFinalized = true; + } } } } catch (error) { diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 7ace91aa63..a0ad446af4 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -1107,6 +1107,66 @@ describe("queue processors", () => { expect(audit?.outcome).toBe("error"); }); + it("finalizes the Gate to neutral when the completion call returns a transient 403 (not left in_progress)", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await persistRegistrySnapshot( + env, + normalizeRegistryPayload( + { "JSONbored/gittensory": { emission_share: 0.01, issue_discovery_share: 0 } }, + { kind: "raw-github", url: "https://example.test" }, + "2026-05-23T00:00:00.000Z", + ), + ); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await upsertRepositorySettings(env, { + repoFullName: "JSONbored/gittensory", + commentMode: "off", + publicSurface: "off", + autoLabelEnabled: false, + checkRunMode: "off", + gateCheckMode: "enabled", + linkedIssueGateMode: "off", + }); + const patchBodies: Array<{ status?: string; conclusion?: string; output?: { title?: string } }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/commits/forbidden403/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); + if (url.includes("/check-runs") && method === "POST") return Response.json({ id: 971 }, { status: 201 }); // pending in_progress + if (url.includes("/check-runs/971") && method === "PATCH") { + const body = JSON.parse(String(init?.body ?? "{}")) as { status?: string; conclusion?: string; output?: { title?: string } }; + patchBodies.push(body); + // First PATCH = the gate completion; a transient 403 (e.g. secondary rate limit) is classified as + // permission_missing and does NOT throw — the fix must still finalize the already-posted pending check. + if (patchBodies.length === 1) return new Response(JSON.stringify({ message: "You have exceeded a secondary rate limit" }), { status: 403 }); + return Response.json({ id: 971 }); + } + return new Response("not found", { status: 404 }); + }); + + await processJob(env, { + type: "github-webhook", + deliveryId: "gate-finalize-on-403", + eventName: "pull_request", + payload: { + action: "opened", + installation: { id: 123, account: { login: "JSONbored", id: 1, type: "User" } }, + repository: { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, + pull_request: { number: 81, title: "Some change", state: "open", user: { login: "contributor" }, head: { sha: "forbidden403" }, labels: [], body: "No issue link." }, + }, + }); + + // The completion PATCH 403'd (permission_missing, no throw), so the fix finalized the SAME check (id 971) + // to a neutral, non-blocking terminal state instead of orphaning it in_progress. + expect(patchBodies.length).toBe(2); + const finalize = patchBodies[1]; + expect(finalize?.status).toBe("completed"); + expect(finalize?.conclusion).toBe("neutral"); + expect(finalize?.output?.title).toBe("Gittensory Gate — could not finish evaluating"); + }); + it("disables the gate from .gittensory.yml (gate.enabled: false) even when repo settings enable it", async () => { const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); await persistRegistrySnapshot(