From 7b203db24f14cf7c350a82cd2a7a389b2cbfe540 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 24 Jun 2026 11:15:36 -0700 Subject: [PATCH] fix(agent): make direct-close paths respect pause / freeze / dry-run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The readiness review for re-enabling autonomous review found two autonomous PR-close paths that call closePullRequest DIRECTLY, bypassing the executor chokepoint — so they honored neither the per-repo pause, the global freeze (#1243), nor agent_dry_run: - maybeRecloseDisallowedReopen (reopen-reclose) consulted none of them. - The draft-dodge auto-close honored agentPaused but not the freeze or dry-run. Both now resolve the agent action mode (the same env-brake || global-freeze || per-repo-pause || dry-run logic the executor uses) and gate the GitHub close on it: paused/frozen → no action, dry-run → audit the would-be close without touching GitHub, live → close as before. This closes the kill-switch coverage gap (frozen=1 is now a COMPLETE stop) and makes agent_dry_run a true no-op for these paths — a prerequisite for safely re-enabling review in dry-run first. --- src/queue/processors.ts | 73 ++++++++++++++++++++++++++++--------- test/unit/queue.test.ts | 79 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 17 deletions(-) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index e159a27f1e..b0a4d8aed9 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -1603,23 +1603,43 @@ async function processGitHubWebhook(env: Env, deliveryId: string, eventName: str const repoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")).toLowerCase() : ""; const authorIsOwner = (pr.authorLogin ?? "").toLowerCase() === repoOwner && repoOwner.length > 0; if (block && block.headSha === pr.headSha && !block.overridden && !authorIsOwner) { - const codes = block.blockerCodes.join(", "); - await createIssueComment( - env, - installationId, - repoFullName, - 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); - await recordAuditEvent(env, { - eventType: "github_app.draft_dodge_closed", - actor: "gittensory", - targetKey: `${repoFullName}#${pr.number}`, - outcome: "completed", - detail: `closed draft-dodge attempt by ${pr.authorLogin ?? "unknown"} — prior gate failure on headSha ${pr.headSha} stands`, - metadata: { deliveryId, repoFullName, headSha: pr.headSha, blockerCodes: block.blockerCodes }, - }).catch(() => undefined); + // Respect the agent action mode (#killswitch-gap): the outer guard already excludes a per-repo pause, + // but this close path must also honor the global freeze and dry-run — so a freeze is a COMPLETE stop + // and a dry-run records the would-be close without touching GitHub. + const draftMode = resolveAgentActionMode({ globalPaused: isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env)), agentPaused: settings.agentPaused, agentDryRun: settings.agentDryRun }); + if (draftMode === "live") { + const codes = block.blockerCodes.join(", "); + await createIssueComment( + env, + installationId, + repoFullName, + 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); + await recordAuditEvent(env, { + eventType: "github_app.draft_dodge_closed", + actor: "gittensory", + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: `closed draft-dodge attempt by ${pr.authorLogin ?? "unknown"} — prior gate failure on headSha ${pr.headSha} stands`, + metadata: { deliveryId, repoFullName, headSha: pr.headSha, blockerCodes: block.blockerCodes }, + }).catch(() => undefined); + } else if (draftMode === "dry_run") { + /* v8 ignore next -- a deleted-account PR yields a null author login; the fallback is defensive */ + const draftAuthor = pr.authorLogin ?? "unknown"; + await recordAuditEvent(env, { + eventType: "github_app.draft_dodge_closed", + actor: "gittensory", + targetKey: `${repoFullName}#${pr.number}`, + outcome: "completed", + detail: `dry-run: would close draft-dodge attempt by ${draftAuthor} — prior gate failure on headSha ${pr.headSha} stands`, + metadata: { deliveryId, repoFullName, headSha: pr.headSha, blockerCodes: block.blockerCodes, mode: "dry_run" }, + }).catch( + /* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */ + () => undefined, + ); + } } } if (installationId && shouldProcessPullRequestPublicSurface(payload.action)) { @@ -3196,6 +3216,25 @@ async function maybeRecloseDisallowedReopen( // self-close sits at the timeline end and is found in-window, so legitimate self-close reopens stay allowed. const windowEvasionSuspected = closer == null && !closerResult.coveredAllPages; if (!closerIsBotOrMaintainer && !windowEvasionSuspected) return false; + // Respect the agent action mode like every other write action (#killswitch-gap): a paused/frozen repo must + // NOT touch GitHub, and dry-run records the would-be re-close without acting — so a dry-run is truly inert and + // the global kill-switch is a COMPLETE stop. This close path previously bypassed pause/freeze/dry-run entirely. + const reopenSettings = await resolveRepositorySettings(env, repoFullName); + const reopenMode = resolveAgentActionMode({ globalPaused: isGlobalAgentPause(env) || (await isGlobalAgentFrozen(env)), agentPaused: reopenSettings.agentPaused, agentDryRun: reopenSettings.agentDryRun }); + if (reopenMode !== "live") { + await recordAuditEvent(env, { + eventType: "github_app.reopen_reclosed", + actor: "gittensory", + targetKey: `${repoFullName}#${pr.number}`, + outcome: reopenMode === "dry_run" ? "completed" : "denied", + detail: `${reopenMode === "dry_run" ? "dry-run: would re-close" : `skipped (agent ${reopenMode}): would re-close`} a disallowed reopen by ${reopener}`, + metadata: { deliveryId, repoFullName, mode: reopenMode }, + }).catch( + /* v8 ignore next -- fail-safe: an audit write failure never blocks the handler */ + () => undefined, + ); + return true; // handled (decision made); never falls through to act on a stood-down repo + } await createIssueComment( env, installationId, diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 7b8a6eb4e2..b91a68cc75 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -6055,6 +6055,48 @@ describe("one-shot reopen prevention", () => { expect(audit?.detail).toContain("originally closed by maintainer"); }); + it("does NOT re-close a disallowed reopen while the global freeze is on — records a skip instead (#killswitch-gap)", 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: "installation-token" }); + if (url.endsWith("/collaborators/contributor/permission")) return Response.json({ permission: "read" }); + if (url.endsWith("/collaborators/maintainer/permission")) return Response.json({ permission: "write" }); + if (url.includes("/issues/42/events")) return Response.json([{ event: "closed", actor: { login: "maintainer" } }]); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" }); + await repositoriesModule.setGlobalAgentFrozen(env, true); // emergency brake on + await processJob(env, { type: "github-webhook", deliveryId: "reopen-frozen", eventName: "pull_request", payload: reopenedPayload("contributor") }); + expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false); // never closed + const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("github_app.reopen_reclosed").first<{ outcome: string; detail: string }>(); + expect(audit?.outcome).toBe("denied"); + expect(audit?.detail).toContain("skipped (agent paused)"); + }); + + it("dry-run: audits a would-be reopen re-close without touching GitHub (#killswitch-gap)", 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: "installation-token" }); + if (url.endsWith("/collaborators/contributor/permission")) return Response.json({ permission: "read" }); + if (url.endsWith("/collaborators/maintainer/permission")) return Response.json({ permission: "write" }); + if (url.includes("/issues/42/events")) return Response.json([{ event: "closed", actor: { login: "maintainer" } }]); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" }); + await repositoriesModule.upsertRepositorySettings(env, { repoFullName: "JSONbored/gittensory", agentDryRun: true }); + await processJob(env, { type: "github-webhook", deliveryId: "reopen-dryrun", eventName: "pull_request", payload: reopenedPayload("contributor") }); + expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false); // never closed + const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("github_app.reopen_reclosed").first<{ outcome: string; detail: string }>(); + expect(audit?.outcome).toBe("completed"); + expect(audit?.detail).toContain("dry-run: would re-close"); + }); + it("allows an admin reopener to reopen without reclosing (fast-path hasMaintainerPermission)", async () => { const calls: Array<{ url: string; method: string }> = []; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { @@ -6259,6 +6301,43 @@ describe("converted_to_draft gate-close (draft-dodge prevention)", () => { expect(audit?.detail).toContain("contributor"); }); + it("does NOT draft-dodge close while the global freeze is on (#killswitch-gap)", 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" }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" }); + await setupRepo(env); + await recordGateBlockOutcome(env, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] }); + await repositoriesModule.setGlobalAgentFrozen(env, true); + await processJob(env, { type: "github-webhook", deliveryId: "draft-frozen", eventName: "pull_request", payload: draftPayload("contributor") }); + expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false); // never closed under freeze + expect(await env.DB.prepare("select count(*) as n from audit_events where event_type = ?").bind("github_app.draft_dodge_closed").first<{ n: number }>()).toMatchObject({ n: 0 }); + }); + + it("dry-run: audits a would-be draft-dodge close without touching GitHub (#killswitch-gap)", 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" }); + return new Response("not found", { status: 404 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" }); + await setupRepo(env, { agentDryRun: true }); + await recordGateBlockOutcome(env, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] }); + await processJob(env, { type: "github-webhook", deliveryId: "draft-dryrun", eventName: "pull_request", payload: draftPayload("contributor") }); + expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false); // never closed in dry-run + 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("completed"); + expect(audit?.detail).toContain("dry-run: would close"); + }); + it("no-ops when no prior gate failure exists for the PR", async () => { const calls: string[] = []; vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {