diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index 888cb6a671..3813378f5f 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -124,7 +124,12 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne const gatePassing = input.conclusion === "success"; // A changed path matching a hard guardrail forces manual review: suppress the irreversible dispositions // (merge / close) AND the auto-approve that could later satisfy a merge. label + request_changes still run. - const guardrailHit = changedPathsHittingGuardrail(input.changedPaths, input.hardGuardrailGlobs).length > 0; + // Fail SAFE on UNKNOWN paths: when guardrails are configured but the changed-file set is empty (cache not + // yet/no-longer populated), we cannot prove the PR doesn't touch a guarded path, so treat it as a hit — + // never auto-merge/close a PR whose diff we don't know. Repos with no guardrails stay permissive. + const guardrailHit = + input.hardGuardrailGlobs.length > 0 && + (input.changedPaths.length === 0 || changedPathsHittingGuardrail(input.changedPaths, input.hardGuardrailGlobs).length > 0); // Auto-merge-ready ONLY when the gate passes AND CI is green AND no guarded path is touched. A red, pending, // or unverified CI is never approved/merged. const readyToMerge = gatePassing && ciPassed && !guardrailHit; diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 7b38770301..b5626b23ea 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -169,6 +169,23 @@ describe("planAgentMaintenanceActions (#778)", () => { // A clean, non-guarded passing PR keeps the `ready-to-merge` label (the auto-merge it promises happens). expect(plan.find((a) => a.actionClass === "label")?.label).toBe(AGENT_LABEL_READY); }); + + it("fails SAFE on UNKNOWN paths: with guardrails configured but no changed-file set, suppress merge/close/approve", () => { + // The changed-file cache can be empty (fresh PR before backfill) or stale; we cannot prove the PR + // doesn't touch a guarded path, so it must fall through to a human (label/request_changes still run). + const merge = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto", label: "auto" }, changedPaths: [], hardGuardrailGlobs: ["src/scoring/**", "scripts/**"], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))); + expect(merge).not.toContain("merge"); + expect(merge).toContain("label"); + const close = classes(planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, blockerTitles: ["x"], changedPaths: [], hardGuardrailGlobs: ["src/scoring/**"], pr: { labels: [], slopRisk: 95 } }))); + expect(close).not.toContain("close"); + const approve = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { approve: "auto" }, changedPaths: [], hardGuardrailGlobs: ["src/scoring/**"], pr: { labels: [] } }))); + expect(approve).not.toContain("approve"); + }); + + it("stays permissive on empty paths when the repo has NO guardrails configured (opted out)", () => { + const plan = classes(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto" }, changedPaths: [], hardGuardrailGlobs: [], pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))); + expect(plan).toContain("merge"); + }); }); describe("owner-PR guard: never auto-close the repo owner's own PRs", () => {