diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index 0eba0c53af..17ff5f0258 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -1206,8 +1206,12 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne // Never APPROVE a base-conflicting PR: it is closed below (willClose on isConflict), so a "LoopOver approves — // safe to merge" review on a PR we're about to close is incoherent (and a stale approval strands the PR if it // later goes green). A `behind`/`blocked` PR is fine to approve (it is rebased pre-review or the approval clears - // the block); only a hard `dirty` conflict is excluded here. (#ready-needs-mergeable, the #4220 report) */ - if (reviewGood && !heldForManualReview && !linkedIssueCloseInFlight && !isConflict && acting("approve") && input.pr.reviewDecision !== "APPROVED" && !alreadyApprovedThisHead) { + // the block); only a hard `dirty` conflict is excluded here. (#ready-needs-mergeable, the #4220 report) + // The same coherence rule applies to a confirmed repeat unlinked-issue offender (`unlinkedIssueMatchViolated`), + // whose own CLOSE branch fires below — approving a PR being closed as a repeat violation is equally incoherent. + // The sibling close paths already exclude it (lines 1092, 1128-1134); this guard omitted it, so a green, + // not-yet-approved repeat offender's PR was planned as an incoherent approve+close pair. */ + if (reviewGood && !heldForManualReview && !linkedIssueCloseInFlight && !unlinkedIssueMatchViolated && !isConflict && acting("approve") && input.pr.reviewDecision !== "APPROVED" && !alreadyApprovedThisHead) { actions.push({ actionClass: "approve", requiresApproval: approval("approve"), diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index be5debb3b6..719978869f 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -728,6 +728,23 @@ describe("planAgentMaintenanceActions (#778)", () => { expect(plan.find((a) => a.actionClass === "close")?.expectedHeadSha).toBe("abc123"); }); + it("never ALSO approves the repeat offender's PR it is closing — no incoherent approve+close pair", () => { + // A green, clean, not-yet-approved PR from a confirmed repeat unlinked-issue offender, with BOTH approve + // and close autonomy acting. The dedicated close branch fires on unlinkedIssueMatchViolated; the approve + // guard must suppress here too (like every sibling close path) — approving a PR we're about to close is + // incoherent (function contract: "never both merge and close"; the approve guard's own comment). + const plan = planAgentMaintenanceActions( + input({ + conclusion: "success", + autonomy: { approve: "auto", close: "auto" }, + ...repeated, + pr: { labels: [], mergeableState: "clean" }, // reviewDecision unset: the APPROVED short-circuit is absent + }), + ); + expect(classes(plan)).toContain("close"); + expect(classes(plan)).not.toContain("approve"); + }); + it("cites the repeat-specific reason and the standard close message template, tagged closeKind: heuristic (subject to the precision breaker)", () => { const action = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { close: "auto" }, ...repeated, pr: { labels: [] } })).find((a) => a.actionClass === "close"); expect(action?.reason).toContain("#42");