From 9ea009f4f5081cbbdf8e5d7782c9ba0a2799cc16 Mon Sep 17 00:00:00 2001
From: JSONbored <49853598+JSONbored@users.noreply.github.com>
Date: Tue, 23 Jun 2026 13:55:54 -0700
Subject: [PATCH] fix(agent): close bad/rejected contributor PRs even on
 guarded paths (guard blocks merge, not rejection)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

#1090 gated willClose on !guardrailHit, so a rejected CONTRIBUTOR PR touching a guarded path was HELD instead
of closed (e.g. gittensory #1098: rejected, touches src/review|services|signals → stayed open with a
'closing' message). That contradicts the spec: 'guarded + would-merge → hold; otherwise → closure.'

The hard-guardrail exists to stop auto-MERGING/APPROVING crucial-path changes without owner review (canMerge
+ approve still gate on !guardrailHit). It must NOT keep a rejected PR open — closing rejects bad changes and
merges nothing, so it is always safe. willClose no longer checks guardrailHit; owner/automation PRs are still
never closed (isContributor gates it); GOOD-but-guarded PRs still fall through to the owner (held).

Test flipped: a failing contributor PR on a guarded path now CLOSES.
---
 src/settings/agent-actions.ts   | 13 +++++++------
 test/unit/agent-actions.test.ts |  6 ++++--
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts
index cef3a4c764..919068e204 100644
--- a/src/settings/agent-actions.ts
+++ b/src/settings/agent-actions.ts
@@ -145,12 +145,13 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
   // the merge; it can't complete for this commit. A new commit makes the live head differ from mergeBlockedSha.
   const mergeTerminallyBlocked = input.pr.mergeBlockedSha != null && input.pr.headSha != null && input.pr.mergeBlockedSha === input.pr.headSha;
   const canMerge = reviewGood && !guardrailHit && acting("merge") && mergeableClean && approvalsSatisfied && !mergeTerminallyBlocked;
-  // A good PR on a guarded path → held for the owner's manual safety review (NOT auto-approved, auto-merged, or auto-closed).
-  // A CONTRIBUTOR PR is CLOSED one-shot when it isn't review-good OR it conflicts, unless a hard guardrail
-  // requires manual review. request-changes is then redundant (the close comment carries the reasoning); it only
-  // fires as a FALLBACK when we are NOT closing — a guarded PR, an owner/automation PR (never closed), or a repo
-  // where `close` isn't at an acting autonomy level.
-  const willClose = !guardrailHit && isContributor && acting("close") && (!reviewGood || isConflict);
+  // A GOOD PR on a guarded path → held for the owner's manual safety review (NOT auto-approved or auto-merged).
+  // But a CONTRIBUTOR PR that is NOT review-good (gate blockers / red / unverified CI) OR conflicts is CLOSED
+  // one-shot REGARDLESS of the guardrail. Spec: "guarded + would-merge → hold; otherwise → closure." The
+  // guardrail exists to stop auto-MERGING/APPROVING crucial-path changes without owner review (see canMerge /
+  // approve) — it must NOT keep a rejected PR open: closing rejects bad changes and merges nothing, so it is
+  // always safe. Owner/automation PRs are still never closed (isContributor gates that). (#close-bad-guarded)
+  const willClose = isContributor && acting("close") && (!reviewGood || isConflict);
   const ciReason = ciFailed
     ? `CI is failing${failingCheckNames.length ? ` (${failingCheckNames.join(", ")})` : ""}`
     : ciUnverified
diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts
index 6076e2afff..b3ab4e8678 100644
--- a/test/unit/agent-actions.test.ts
+++ b/test/unit/agent-actions.test.ts
@@ -130,9 +130,11 @@ describe("planAgentMaintenanceActions (#778)", () => {
       expect(plan).not.toContain("merge");
     });
 
-    it("does NOT auto-close a failing PR on a guarded path", () => {
+    it("DOES auto-close a failing contributor PR on a guarded path (the guard blocks auto-merge, NOT rejection)", () => {
+      // Spec: guarded + would-merge → hold; otherwise → closure. Closing a bad PR merges nothing, so the
+      // hard-guardrail (which exists to stop auto-MERGING crucial paths) must not keep a rejected PR open.
       const plan = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], ...guarded, pr: { labels: [], slopRisk: 95 } })));
-      expect(plan).not.toContain("close");
+      expect(plan).toContain("close");
     });
 
     it("does NOT approve or auto-merge a passing PR on a guarded path", () => {