From d8829485916226949889246bcb1216a8630b67ba Mon Sep 17 00:00:00 2001
From: jaytbarimbao-collab
 <300663773+jaytbarimbao-collab@users.noreply.github.com>
Date: Thu, 16 Jul 2026 15:09:58 -0400
Subject: [PATCH] fix(agent-actions): never approve a repeat unlinked-issue
 offender's PR it is closing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

planAgentMaintenanceActions guarantees a coherent disposition set ("never
both merge and close"). The approve guard excluded the linked-issue and
conflict close paths (!linkedIssueCloseInFlight, !isConflict) but omitted the
third close path that can coexist with a review-good PR: a confirmed repeat
unlinked-issue offender (unlinkedIssueMatchViolated).

So a green, clean, not-yet-approved PR from a confirmed repeat offender, with
both approve and close autonomy acting, was planned as an incoherent
["approve", "close"] pair — the bot approves the very PR it closes as a
repeat violation, and leaves a stale approval behind. heldForManualReview
does not catch this: its unlinked term is gated on !acting("close"), which is
false precisely when close is acting.

Add !unlinkedIssueMatchViolated to the approve guard, matching the sibling
close paths (lines 1092, 1128-1134) that already carry it. Now the plan is a
single close. Every other approve/close/merge disposition is unchanged.

Adds a regression test asserting the plan contains close and not approve for
this input.
---
 src/settings/agent-actions.ts   |  8 ++++++--
 test/unit/agent-actions.test.ts | 17 +++++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

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");