diff --git a/src/queue/review-evasion.ts b/src/queue/review-evasion.ts index 001d73c161..c4a9bddcf2 100644 --- a/src/queue/review-evasion.ts +++ b/src/queue/review-evasion.ts @@ -311,19 +311,23 @@ async function closeDraftDodgeAttemptIfBlocked( pr.number, `Gate verdict stands for this commit — converting to draft does not reset the review. Re-submit a new PR with the issues addressed${codes ? ` (${codes})` : ""}.`, ).catch(() => undefined); - await closePullRequest( - env, - installationId, - repoFullName, - pr.number, - ).catch(() => undefined); + // #2260/#8801: the audit outcome must reflect whether the close actually happened on GitHub, not just + // whether this handler ran. This guard was the ONE sibling in this file still swallowing the close + // error and unconditionally recording "completed" — a transient 403/5xx left the PR open on GitHub + // while the audit trail claimed a draft-dodge close was enforced. + const closeError = await closePullRequest(env, installationId, repoFullName, pr.number) + .then(() => null) + .catch((error: unknown) => error); await recordAuditEvent(env, { eventType: "github_app.draft_dodge_closed", actor: "loopover", targetKey, - outcome: "completed", - detail: `closed draft-dodge attempt by ${pr.authorLogin ?? "unknown"} — prior gate failure on headSha ${pr.headSha} stands`, - metadata: gateMetadata, + outcome: closeError === null ? "completed" : "error", + detail: + closeError === null + ? `closed draft-dodge attempt by ${pr.authorLogin ?? "unknown"} — prior gate failure on headSha ${pr.headSha} stands` + : `FAILED to close draft-dodge attempt by ${pr.authorLogin ?? "unknown"} — the close API call did not succeed; the PR may still be open`, + metadata: closeError === null ? gateMetadata : { ...gateMetadata, error: errorMessage(closeError) }, }).catch(() => undefined); } } diff --git a/test/unit/queue-lifecycle-guards.test.ts b/test/unit/queue-lifecycle-guards.test.ts index 2d5fba9084..9147cf7478 100644 --- a/test/unit/queue-lifecycle-guards.test.ts +++ b/test/unit/queue-lifecycle-guards.test.ts @@ -1138,6 +1138,35 @@ describe("converted_to_draft gate-close (draft-dodge prevention)", () => { expect(audit?.detail).toContain("contributor"); }); + it("#8801: a FAILED close records outcome 'error' with the failure named — never a false 'completed' (the #2260 contract)", async () => { + const calls: Array<{ url: string; method: string }> = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + calls.push({ url, method }); + if (url.includes("/access_tokens")) return Response.json({ token: "t" }); + if (url.endsWith("/issues/42/comments") && method === "POST") return Response.json({ id: 1 }, { status: 201 }); + // The close PATCH fails transiently — the exact scenario the audit trail used to lie about. + if (url.endsWith("/pulls/42") && method === "PATCH") return new Response("boom", { status: 502 }); + return new Response("not found", { status: 404 }); + }); + + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "loopover-orb" }); + await setupRepo(env); + await recordGateBlockOutcome(env, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] }); + + // No author login on the stored PR — the detail's "unknown" fallback arm is exercised on the failure path. + const payload = draftPayload("contributor"); + payload.pull_request.user = undefined; + await processJob(env, { type: "github-webhook", deliveryId: "draft-dodge-fail", eventName: "pull_request", payload }); + + expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(true); // the close WAS attempted + const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("github_app.draft_dodge_closed").first<{ outcome: string; detail: string }>(); + expect(audit?.outcome).toBe("error"); + expect(audit?.detail).toContain("FAILED to close draft-dodge attempt by unknown"); + expect(audit?.detail).toContain("the PR may still be open"); + }); + it("does NOT draft-dodge close when live PR state has moved since the webhook was received (#2130)", async () => { const calls: Array<{ url: string; method: string }> = []; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {