Skip to content

fix(gate): route draft-dodge close through agent executor #1223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JSONbored wants to merge 1 commit into from
Closed

fix(gate): route draft-dodge close through agent executor #1223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
65 changes: 38 additions & 27 deletions src/queue/processors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1584,39 +1584,50 @@ async function processGitHubWebhook(env: Env, deliveryId: string, eventName: str
await persistAdvisory(env, advisory);
// Draft-dodge guard (#converted-to-draft): a contributor converting an OPEN PR to draft cannot use
// draft state to keep a gate-rejected PR alive. When a prior gate failure exists for the PR's current
// headSha (and the block has not been maintainer-overridden), close the PR immediately — the gate
// verdict stands and does not reset on draft conversion. Skipped when the agent is unconfigured or
// paused (the gate doesn't act on paused repos) and for owner / automation PRs.
if (
payload.action === "converted_to_draft" &&
installationId &&
pr.headSha &&
pr.state === "open" &&
isAgentConfigured(settings.autonomy) &&
!settings.agentPaused &&
!isProtectedAutomationAuthor(pr.authorLogin)
) {
// headSha (and the block has not been maintainer-overridden), plan a close through the SAME agent action
// executor used by normal maintenance so close-specific autonomy, approval, dry-run, global pause, and
// permission readiness are all enforced. Owner / automation PRs remain exempt.
if (payload.action === "converted_to_draft" && installationId && pr.headSha && pr.state === "open" && !isProtectedAutomationAuthor(pr.authorLogin)) {
const block = await getGateBlockOutcome(env, repoFullName, pr.number).catch(() => undefined);
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(
const closeReason = `draft-dodge attempt by ${pr.authorLogin ?? "unknown"} — prior gate failure on headSha ${pr.headSha} stands`;
const installation = await getInstallation(env, installationId);
const [outcome] = await executeAgentMaintenanceActions(
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);
{
installationId,
repoFullName,
pullNumber: pr.number,
headSha: pr.headSha,
autonomy: settings.autonomy,
agentPaused: settings.agentPaused,
agentDryRun: settings.agentDryRun,
installationPermissions: installation?.permissions ?? null,
authorLogin: pr.authorLogin,
},
[
{
actionClass: "close",
requiresApproval: autonomyRequiresApproval(resolveAutonomy(settings.autonomy, "close")),
reason: closeReason,
closeKind: "linked-issue-hard-rule",
closeComment: `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})` : ""}.`,
},
],
);
if (outcome?.outcome === "completed") {
await recordAuditEvent(env, {
eventType: "github_app.draft_dodge_closed",
actor: "gittensory",
targetKey: `${repoFullName}#${pr.number}`,
outcome: "completed",
detail: `closed ${closeReason}`,
metadata: { deliveryId, repoFullName, headSha: pr.headSha, blockerCodes: block.blockerCodes },
}).catch(() => undefined);
}
}
}
if (installationId && shouldProcessPullRequestPublicSurface(payload.action)) {
Expand Down
82 changes: 77 additions & 5 deletions test/unit/queue.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6366,7 +6366,7 @@ describe("converted_to_draft gate-close (draft-dodge prevention)", () => {
expect(audit?.detail).toContain("unknown");
});

it("swallows createIssueComment and closePullRequest API errors (fail-safe — both .catch() bodies)", async () => {
it("records executor errors when the draft-dodge close GitHub mutation fails", async () => {
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
if (url.includes("/access_tokens")) return Response.json({ token: "t" });
Expand All @@ -6377,14 +6377,86 @@ describe("converted_to_draft gate-close (draft-dodge prevention)", () => {
await setupRepo(env);
await recordGateBlockOutcome(env, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] });

// Should not throw even though createIssueComment and closePullRequest both throw
// Should not throw even though the close action's comment/close GitHub calls throw; the executor records
// the failed action instead of bypassing the action safety/audit stack.
await expect(
processJob(env, { type: "github-webhook", deliveryId: "api-error-swallow", eventName: "pull_request", payload: draftPayload("contributor") }),
).resolves.toBeUndefined();

// Audit event was still written to DB (recordAuditEvent uses D1, not fetch)
const audit = await env.DB.prepare("select event_type from audit_events where event_type = ?").bind("github_app.draft_dodge_closed").first<{ event_type: string }>();
expect(audit?.event_type).toBe("github_app.draft_dodge_closed");
const closeAudit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("agent.action.close").first<{ outcome: string; detail: string }>();
expect(closeAudit?.outcome).toBe("error");
expect(closeAudit?.detail).toContain("simulated network error");
const draftAudit = await env.DB.prepare("select event_type from audit_events where event_type = ?").bind("github_app.draft_dodge_closed").first<{ event_type: string }>();
expect(draftAudit).toBeUndefined();
});

it("routes draft-dodge close through close autonomy instead of closing when only labels are enabled", async () => {
const calls: Array<{ url: string; method: string }> = [];
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
calls.push({ url, method: init?.method ?? "GET" });
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, { autonomy: { label: "auto", close: "observe" } });
await recordGateBlockOutcome(env, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] });

await processJob(env, { type: "github-webhook", deliveryId: "draft-label-only", eventName: "pull_request", payload: draftPayload("contributor") });

expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false);
const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("agent.action.close").first<{ outcome: string; detail: string }>();
expect(audit?.outcome).toBe("denied");
expect(audit?.detail).toContain("autonomy for close is observe");
});

it("honors dry-run and global pause for draft-dodge close", async () => {
const calls: Array<{ url: string; method: string }> = [];
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
calls.push({ url, method: init?.method ?? "GET" });
if (url.includes("/access_tokens")) return Response.json({ token: "t" });
return new Response("not found", { status: 404 });
});

const dryRunEnv = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory" });
await setupRepo(dryRunEnv, { agentDryRun: true });
await recordGateBlockOutcome(dryRunEnv, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] });
await processJob(dryRunEnv, { type: "github-webhook", deliveryId: "draft-dry-run", eventName: "pull_request", payload: draftPayload("contributor") });
const dryRunAudit = await dryRunEnv.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.action.close").first<{ outcome: string }>();
expect(dryRunAudit?.outcome).toBe("completed");

const pausedEnv = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "gittensory", AGENT_ACTIONS_PAUSED: "true" });
await setupRepo(pausedEnv);
await recordGateBlockOutcome(pausedEnv, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] });
await processJob(pausedEnv, { type: "github-webhook", deliveryId: "draft-global-paused", eventName: "pull_request", payload: draftPayload("contributor") });
const pausedAudit = await pausedEnv.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("agent.action.close").first<{ outcome: string; detail: string }>();
expect(pausedAudit?.outcome).toBe("denied");
expect(pausedAudit?.detail).toContain("agent actions paused");
expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false);
});

it("stages draft-dodge close when close requires approval", async () => {
const calls: Array<{ url: string; method: string }> = [];
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
calls.push({ url, method: init?.method ?? "GET" });
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, { autonomy: { close: "auto_with_approval" } });
await recordGateBlockOutcome(env, { repoFullName: "JSONbored/gittensory", pullNumber: 42, headSha: "abc123", blockerCodes: ["missing_linked_issue"] });

await processJob(env, { type: "github-webhook", deliveryId: "draft-approval", eventName: "pull_request", payload: draftPayload("contributor") });

expect(calls.some((c) => c.method === "PATCH" && c.url.endsWith("/pulls/42"))).toBe(false);
const pending = await env.DB.prepare("select action_class from agent_pending_actions where repo_full_name = ? and pull_number = ?").bind("JSONbored/gittensory", 42).first<{ action_class: string }>();
expect(pending?.action_class).toBe("close");
const audit = await env.DB.prepare("select outcome from audit_events where event_type = ?").bind("agent.action.close").first<{ outcome: string }>();
expect(audit?.outcome).toBe("queued");
});

it("getGateBlockOutcome DB error is caught — handler no-ops gracefully", async () => {
Expand Down
Loading