diff --git a/src/queue/processors.ts b/src/queue/processors.ts index fe1908f2dd..7e4a7ce5b9 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -101,7 +101,7 @@ import { commandAuthorizationAllowedRoles, commandAuthorizationNeedsMinerDetecti import { isAgentConfigured } from "../settings/autonomy"; import { isGlobalAgentPause, resolveAgentActionMode } from "../settings/agent-execution"; import { selectRegateCandidates } from "../settings/agent-sweep"; -import { planAgentMaintenanceActions } from "../settings/agent-actions"; +import { isProtectedAutomationAuthor, planAgentMaintenanceActions } from "../settings/agent-actions"; import { executeAgentMaintenanceActions } from "../services/agent-action-executor"; import { processSubmitDraft } from "../services/draft"; import { loadIssueQualityReportMap } from "../services/issue-quality"; @@ -493,6 +493,7 @@ async function maybeRunAgentMaintenance( const repoOwner = repoFullName.includes("/") ? repoFullName.slice(0, repoFullName.indexOf("/")) : ""; const authorLogin = pr.authorLogin ?? ""; const authorIsOwner = authorLogin.length > 0 && authorLogin.toLowerCase() === repoOwner.toLowerCase(); + const authorIsAutomationBot = isProtectedAutomationAuthor(pr.authorLogin); const planned = planAgentMaintenanceActions({ conclusion: gate.conclusion, @@ -503,6 +504,7 @@ async function maybeRunAgentMaintenance( changedPaths, hardGuardrailGlobs, authorIsOwner, + authorIsAutomationBot, pr: { mergeableState: pr.mergeableState, reviewDecision: pr.reviewDecision, diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index 232e309c78..e3530ac21b 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -17,6 +17,15 @@ const DEFAULT_SLOP_GATE_MIN_SCORE = 60; export const AGENT_LABEL_READY = "gittensory:ready-to-merge"; export const AGENT_LABEL_CHANGES = "gittensory:changes-requested"; +// Maintainer-managed automation accounts whose PRs are never auto-closed. A recurring accumulator (e.g. +// github-actions[bot] opening automation/readme-refresh) or a dependency PR must not be killed by a duplicate +// or slop heuristic — the maintainer owns its lifecycle. (reviewbot wrongly auto-closed such an accumulator, +// awesome-claude #4192.) Still eligible for auto-merge when clean + passing. +const PROTECTED_AUTOCLOSE_AUTHORS = new Set(["github-actions[bot]", "dependabot[bot]", "renovate[bot]"]); +export function isProtectedAutomationAuthor(login: string | null | undefined): boolean { + return login != null && PROTECTED_AUTOCLOSE_AUTHORS.has(login.toLowerCase()); +} + export type PlannedAgentAction = { actionClass: AgentActionClass; // auto_with_approval → the action is staged for a human approval (the #779 queue) instead of executing now. @@ -44,6 +53,10 @@ export type AgentActionPlanInput = { // True when the PR author is the repo owner (e.g. JSONbored). Standing rule: owner PRs are NEVER // auto-closed. They may still auto-merge when clean + passing. authorIsOwner: boolean; + // True when the PR author is a maintainer-managed automation account (e.g. github-actions[bot] opening an + // accumulator like automation/readme-refresh, or dependabot/renovate). These are NEVER auto-closed — a noise + // heuristic (duplicate/slop) must not kill a recurring maintainer-managed PR. They may still auto-merge. + authorIsAutomationBot: boolean; pr: { mergeableState?: string | null | undefined; reviewDecision?: string | null | undefined; @@ -125,7 +138,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne reason: `gate passed, mergeable, ${autoMaintain.requireApprovals} approval(s) satisfied`, mergeMethod: autoMaintain.mergeMethod, }); - } else if (acting("close") && !passing && !guardrailHit && !input.authorIsOwner) { + } else if (acting("close") && !passing && !guardrailHit && !input.authorIsOwner && !input.authorIsAutomationBot) { const noiseReasons: string[] = []; if (input.pr.slopRisk != null && input.pr.slopRisk >= slopGateMinScore) noiseReasons.push(`slop score ${input.pr.slopRisk} ≥ ${slopGateMinScore}`); if ((input.pr.linkedDuplicateCount ?? 0) > 0) noiseReasons.push("duplicate of another open PR"); diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 4391c37adf..22bb626323 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { AGENT_LABEL_CHANGES, AGENT_LABEL_READY, planAgentMaintenanceActions, type AgentActionPlanInput } from "../../src/settings/agent-actions"; +import { AGENT_LABEL_CHANGES, AGENT_LABEL_READY, isProtectedAutomationAuthor, planAgentMaintenanceActions, type AgentActionPlanInput } from "../../src/settings/agent-actions"; import type { GateCheckConclusion } from "../../src/rules/advisory"; function input(overrides: Partial & { conclusion: GateCheckConclusion }): AgentActionPlanInput { @@ -11,6 +11,7 @@ function input(overrides: Partial & { conclusion: GateChec changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, + authorIsAutomationBot: false, pr: { labels: [] }, ...overrides, }; @@ -76,11 +77,11 @@ describe("planAgentMaintenanceActions (#778)", () => { it("applies conservative defaults when autoMaintain / slopGateMinScore are omitted", () => { // no autoMaintain → requireApprovals defaults to 1 → a clean passing PR without APPROVED does NOT merge - expect(classes(planAgentMaintenanceActions({ conclusion: "success", blockerTitles: [], autonomy: { merge: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, pr: { labels: [], mergeableState: "clean" } }))).not.toContain("merge"); + expect(classes(planAgentMaintenanceActions({ conclusion: "success", blockerTitles: [], autonomy: { merge: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, pr: { labels: [], mergeableState: "clean" } }))).not.toContain("merge"); // no slopGateMinScore → defaults to 60 → slopRisk 70 counts as noise and closes - expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, pr: { labels: [], slopRisk: 70 } }))).toContain("close"); + expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, pr: { labels: [], slopRisk: 70 } }))).toContain("close"); // ...and slopRisk 50 is below the default → no close - expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, pr: { labels: [], slopRisk: 50 } }))).not.toContain("close"); + expect(classes(planAgentMaintenanceActions({ conclusion: "failure", blockerTitles: ["x"], autonomy: { close: "auto" }, changedPaths: [], hardGuardrailGlobs: [], authorIsOwner: false, authorIsAutomationBot: false, pr: { labels: [], slopRisk: 50 } }))).not.toContain("close"); }); it("closes clear noise (high slop or duplicate) on a non-passing verdict, and never closes a passing PR", () => { @@ -153,7 +154,7 @@ describe("planAgentMaintenanceActions (#778)", () => { }); it("DOES auto-close the same noisy PR when the author is not the owner", () => { - const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], authorIsOwner: false, pr: { labels: [], slopRisk: 95 } }))); + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], authorIsOwner: false, authorIsAutomationBot: false, pr: { labels: [], slopRisk: 95 } }))); expect(plan).toContain("close"); }); @@ -162,4 +163,32 @@ describe("planAgentMaintenanceActions (#778)", () => { expect(plan).toContain("merge"); }); }); + + describe("automation-bot guard: never auto-close maintainer-managed accumulator/dependency PRs", () => { + it("does NOT auto-close a noisy failing PR authored by an automation bot (e.g. the readme-refresh accumulator)", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], authorIsAutomationBot: true, pr: { labels: [], slopRisk: 95, linkedDuplicateCount: 3 } }))); + expect(plan).not.toContain("close"); + }); + + it("still auto-merges a clean+approved automation-bot PR (the guard blocks only close, never merge)", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto" }, authorIsAutomationBot: true, pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))); + expect(plan).toContain("merge"); + }); + }); +}); + +describe("isProtectedAutomationAuthor", () => { + it("matches the maintainer-managed automation accounts (case-insensitive)", () => { + expect(isProtectedAutomationAuthor("github-actions[bot]")).toBe(true); + expect(isProtectedAutomationAuthor("GitHub-Actions[bot]")).toBe(true); + expect(isProtectedAutomationAuthor("dependabot[bot]")).toBe(true); + expect(isProtectedAutomationAuthor("renovate[bot]")).toBe(true); + }); + + it("does not match human authors or null", () => { + expect(isProtectedAutomationAuthor("JSONbored")).toBe(false); + expect(isProtectedAutomationAuthor("some-contributor")).toBe(false); + expect(isProtectedAutomationAuthor(null)).toBe(false); + expect(isProtectedAutomationAuthor(undefined)).toBe(false); + }); });