Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions src/queue/review-evasion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
29 changes: 29 additions & 0 deletions test/unit/queue-lifecycle-guards.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down