From 34f48cb772e7a904a883471310d8294d260a0d40 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 13:04:58 -0700 Subject: [PATCH 1/2] fix(review): approval-gate manual review labels --- src/settings/agent-actions.ts | 13 ++++++------ test/unit/agent-actions.test.ts | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index c757b4c4e6..f55be7dcce 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -792,7 +792,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne actions.push({ actionClass: "label", autonomyClass: "merge", - requiresApproval: false, + requiresApproval: approval("merge"), reason: `verdict=${conclusion}; ${guardrailReason}`, label: labels.manualReview, labelOp: "add", @@ -811,7 +811,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne actions.push({ actionClass: "label", autonomyClass: "merge", - requiresApproval: false, + requiresApproval: approval("merge"), reason: `verdict=${conclusion}; ${input.migrationCollisionHold.reason}`, label: labels.manualReview, labelOp: "add", @@ -827,7 +827,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne actions.push({ actionClass: "label", autonomyClass: "merge", - requiresApproval: false, + requiresApproval: approval("merge"), reason: `verdict=${conclusion}; ${input.unlinkedIssueMatchHold.reason}`, label: labels.manualReview, labelOp: "add", @@ -843,7 +843,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne actions.push({ actionClass: "label", autonomyClass: "merge", - requiresApproval: false, + requiresApproval: approval("merge"), reason: `verdict=${conclusion}; ${input.unlinkedIssueMatchClose.reason}`, label: labels.manualReview, labelOp: "add", @@ -1124,6 +1124,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne // at a non-acting level". For the owner/admin/automation-bot branch (`!closeEligible`) this is unchanged: those // authors are never close-eligible regardless of the `close` autonomy dial, so the hold must still surface. // (manualHoldReason itself is now computed earlier, above section 1 — see its doc comment there.) + const manualHoldAutonomyClass: AgentActionClass = reviewGood ? "merge" : "close"; if ( manualHoldReason !== null && labels.manualReview !== null && @@ -1132,8 +1133,8 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne ) { actions.push({ actionClass: "label", - autonomyClass: reviewGood ? "merge" : "close", - requiresApproval: false, + autonomyClass: manualHoldAutonomyClass, + requiresApproval: approval(manualHoldAutonomyClass), reason: manualHoldReason, label: labels.manualReview, labelOp: "add", diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index b284595ce3..e20e10d426 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -785,6 +785,41 @@ describe("planAgentMaintenanceActions (#778)", () => { })); expect(plan).toEqual([]); }); + + it("requires approval for merge-governed manual-review labels when merge is auto_with_approval", () => { + const guarded = planAgentMaintenanceActions(input({ + conclusion: "success", + autonomy: { merge: "auto_with_approval" }, + changedPaths: ["src/settings/agent-actions.ts"], + hardGuardrailGlobs: ["src/settings/**"], + pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" }, + })); + expect(guarded).toEqual([ + expect.objectContaining({ actionClass: "label", autonomyClass: "merge", requiresApproval: true, label: AGENT_LABEL_NEEDS_REVIEW }), + ]); + + const collision = planAgentMaintenanceActions(input({ + conclusion: "success", + autonomy: { merge: "auto_with_approval" }, + migrationCollisionHold: { reason: "live migrations/** collision on main", comment: "Please rebase." }, + pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" }, + })); + expect(collision).toEqual([ + expect.objectContaining({ actionClass: "label", autonomyClass: "merge", requiresApproval: true, label: AGENT_LABEL_NEEDS_REVIEW, comment: "Please rebase." }), + ]); + }); + + it("requires approval for close-governed manual-review labels when close is auto_with_approval", () => { + const actionRequired = planAgentMaintenanceActions(input({ conclusion: "action_required", autonomy: { close: "auto_with_approval" }, pr: { labels: [] } })); + expect(actionRequired).toEqual([ + expect.objectContaining({ actionClass: "label", autonomyClass: "close", requiresApproval: true, label: AGENT_LABEL_NEEDS_REVIEW }), + ]); + + const auto = planAgentMaintenanceActions(input({ conclusion: "action_required", autonomy: { close: "auto" }, pr: { labels: [] } })); + expect(auto).toEqual([ + expect.objectContaining({ actionClass: "label", autonomyClass: "close", requiresApproval: false, label: AGENT_LABEL_NEEDS_REVIEW }), + ]); + }); }); describe("AI/review blockers remain blocking even when CI is green", () => { From 8131241ec5a9655c3b1e6bafda16171557d34e7c Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 5 Jul 2026 17:11:52 -0700 Subject: [PATCH 2/2] test(agent-actions): cover unlinked-issue-match fallback approval gate (1d/1e) The rebase onto main surfaced two more manual-review label call sites (unlinkedIssueMatchHold/unlinkedIssueMatchClose fallbacks) added by the credibility-gate-farming guardrail after this PR branched, which had the same hardcoded requiresApproval: false bug this PR already fixes elsewhere. Apply the same approval("merge") derivation and pin it with a test. --- test/unit/agent-actions.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index e20e10d426..32b4ee5773 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -820,6 +820,30 @@ describe("planAgentMaintenanceActions (#778)", () => { expect.objectContaining({ actionClass: "label", autonomyClass: "close", requiresApproval: false, label: AGENT_LABEL_NEEDS_REVIEW }), ]); }); + + it("requires approval for the unlinked-issue-match manual-review fallbacks (1d/1e) when merge is auto_with_approval", () => { + const holdMatched = { unlinkedIssueMatchHold: { reason: "this PR links no issue, but appears to directly solve open issue #42 without linking it", comment: "Please add a linking reference." } }; + const hold = planAgentMaintenanceActions(input({ + conclusion: "success", + autonomy: { merge: "auto_with_approval" }, + ...holdMatched, + pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" }, + })); + expect(hold).toEqual([ + expect.objectContaining({ actionClass: "label", autonomyClass: "merge", requiresApproval: true, label: AGENT_LABEL_NEEDS_REVIEW, comment: holdMatched.unlinkedIssueMatchHold.comment }), + ]); + + const closeRepeated = { unlinkedIssueMatchClose: { reason: "repeat of the same unlinked-issue pattern already flagged", comment: "Closing: please link the issue you're solving going forward." } }; + const closeFallback = planAgentMaintenanceActions(input({ + conclusion: "success", + autonomy: { merge: "auto_with_approval" }, + ...closeRepeated, + pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" }, + })); + expect(closeFallback).toEqual([ + expect.objectContaining({ actionClass: "label", autonomyClass: "merge", requiresApproval: true, label: AGENT_LABEL_NEEDS_REVIEW, comment: closeRepeated.unlinkedIssueMatchClose.comment }), + ]); + }); }); describe("AI/review blockers remain blocking even when CI is green", () => {