Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions src/settings/agent-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,10 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
const acting = (actionClass: AgentActionClass) => isActingAutonomyLevel(level(actionClass));
const approval = (actionClass: AgentActionClass) => autonomyRequiresApproval(level(actionClass));

// App/infra-neutral verdicts (not evaluated yet) never drive an action.
if (input.conclusion === "neutral" || input.conclusion === "skipped") return actions;
// Only a SKIPPED gate (genuinely not evaluated) drives no action. A NEUTRAL gate (advisory-only blockers on a
// non-confirmed contributor, or eval-not-ready) is gate-NON-BLOCKING: it flows to the disposition so the PR is
// merged (clean+green) or HELD with a label — never left silently undecided. (#harm-stop neutral-silent-stuck)
if (input.conclusion === "skipped") return actions;

// CI state over ALL of the PR's checks (required OR not — codecov/patch included) — reviewbot's ci_red
// parity. A red CI is NEVER approved/merged and is itself a close-worthy signal (non-owner); while CI is
Expand All @@ -181,6 +183,9 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
// Settle-before-decide: never approve / merge / close on a half-finished CI run.
if (input.ciState === "pending") return actions;

// Only SUCCESS earns the review-good auto-merge. A NEUTRAL gate flows (no longer silently returns []) but is
// NOT auto-merged — it falls through to a HELD + labeled state for review. (Auto-merging neutral / non-confirmed
// contributor PRs is a separate trust/policy decision, deliberately NOT bundled into the harm-stop.) (#harm-stop)
const gatePassing = input.conclusion === "success";
// A changed path matching a hard guardrail forces manual review (suppresses auto-MERGE / auto-approve / auto-close).
// Fail SAFE on UNKNOWN paths (#1062): when guardrails are configured but the changed-file set is empty (cache
Expand Down Expand Up @@ -214,7 +219,11 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
// hallucinated reject on a crucial PR must NOT auto-close a good change (the #1528 near-miss); the owner
// verifies and closes/merges. The BULK (non-guarded) contributor PRs still auto-close one-shot on a bad
// verdict / conflict — only the small crucial set is held. Owner/automation PRs are never closed regardless.
const willClose = !guardrailHit && isContributor && acting("close") && (!reviewGood || isConflict);
// CLOSE a contributor PR ONLY on a REAL adverse signal — a confirmed gate FAILURE, a red required CI, or a base
// CONFLICT. NEVER close merely because CI is UNVERIFIED (a fork whose Actions await approval, or unreadable
// checks) or otherwise not-yet-mergeable — those are HELD for review, not killed (#harm-stop fork-false-close).
// Owner/automation PRs are never closed (isContributor); guarded paths are held (guardrailHit).
const willClose = !guardrailHit && isContributor && acting("close") && (input.conclusion === "failure" || ciFailed || isConflict);
// Linked-issue HARD-RULE close (#linked-issue-hard-rules). A DETERMINISTIC verdict about the LINKED ISSUE
// (owner-assigned / missing point-label / maintainer-only) — NOT an AI verdict, so there is no hallucination
// to guard against: this close fires REGARDLESS of `guardrailHit`. It still only ever closes a CONTRIBUTOR
Expand Down Expand Up @@ -336,7 +345,7 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
// Contributor PR that is NOT review-good (gate blockers / red / unverified CI) OR conflicts with base →
// CLOSE one-shot when no hard guardrail requires manual review. Cite the concrete reasons.
const closeReasons: string[] = [];
if (ciFailed || ciUnverified) closeReasons.push(ciReason);
if (ciFailed) closeReasons.push(ciReason);
if (isConflict) closeReasons.push("conflicts with the base branch — resolve and open a fresh PR");
for (const blockerTitle of input.blockerTitles) closeReasons.push(blockerTitle);
if (input.pr.slopRisk != null && input.pr.slopRisk >= slopGateMinScore) closeReasons.push(`slop score ${input.pr.slopRisk} ≥ ${slopGateMinScore}`);
Expand Down
25 changes: 16 additions & 9 deletions test/unit/agent-actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@ function input(overrides: Partial<AgentActionPlanInput> & { conclusion: GateChec
const classes = (actions: ReturnType<typeof planAgentMaintenanceActions>) => actions.map((a) => a.actionClass);

describe("planAgentMaintenanceActions (#778)", () => {
it("plans nothing for a not-yet-evaluated verdict (neutral / skipped)", () => {
expect(planAgentMaintenanceActions(input({ conclusion: "neutral", autonomy: { merge: "auto", label: "auto", close: "auto" } }))).toEqual([]);
it("plans nothing for SKIPPED; a NEUTRAL verdict FLOWS (advisory non-blocking, never silently undecided)", () => {
// skipped = genuinely not evaluated → no action.
expect(planAgentMaintenanceActions(input({ conclusion: "skipped", autonomy: { approve: "auto" } }))).toEqual([]);
// neutral = advisory-only blockers → NON-blocking: flows to the disposition, earns a label (clean+green here),
// and is NEVER left silently undecided or auto-closed. (#harm-stop neutral-silent-stuck)
const neutral = classes(planAgentMaintenanceActions(input({ conclusion: "neutral", autonomy: { merge: "auto", label: "auto", close: "auto" } })));
expect(neutral).not.toEqual([]);
expect(neutral).not.toContain("close");
});

it("plans nothing when every class is at a non-acting level", () => {
Expand All @@ -50,10 +55,12 @@ describe("planAgentMaintenanceActions (#778)", () => {
expect(noClose).not.toContain("request_changes");
});

it("never emits request_changes even for an action_required verdict (merge-or-close, never block)", () => {
it("an action_required verdict is HELD — never request_changes, never closed (awaiting action ≠ failure)", () => {
const plan = classes(planAgentMaintenanceActions(input({ conclusion: "action_required", autonomy: { request_changes: "auto", close: "auto", label: "auto" }, blockerTitles: [] })));
expect(plan).not.toContain("request_changes");
expect(plan).toContain("close"); // contributor + not review-good → close
// awaiting-action (e.g. a fork's CI awaiting approval) → HELD + labeled, NOT a one-shot close. (#harm-stop)
expect(plan).not.toContain("close");
expect(plan).toContain("label");
});

it("approves a passing verdict and never re-approves; a failing one closes (never approves, never requests changes)", () => {
Expand Down Expand Up @@ -278,13 +285,13 @@ describe("planAgentMaintenanceActions (#778)", () => {
expect(planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", approve: "auto", merge: "auto", close: "auto" }, ciState: "pending", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }))).toEqual([]);
});

it("CLOSES a contributor's gate-passing PR whose CI is UNVERIFIED (fork workflows awaiting approval → green can't be confirmed)", () => {
it("HOLDS a contributor's gate-passing PR whose CI is UNVERIFIED — NEVER closes it (fork workflows awaiting approval) (#harm-stop)", () => {
const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { label: "auto", approve: "auto", merge: "auto", close: "auto" }, ciState: "unverified", pr: { labels: [], mergeableState: "clean", reviewDecision: "APPROVED" } }));
const cls = classes(plan);
expect(cls).not.toContain("merge");
expect(cls).not.toContain("approve");
expect(cls).toContain("close");
expect(plan.find((a) => a.actionClass === "label")?.label).toBe(AGENT_LABEL_CHANGES);
expect(cls).not.toContain("merge"); // can't merge — green not confirmed
expect(cls).not.toContain("approve"); // can't approve — green not confirmed
expect(cls).not.toContain("close"); // NEVER close on unverified CI — held for review, not killed
expect(cls).toContain("label"); // labeled (held), never silently stuck
});

it("NEVER closes the OWNER's unverified-CI PR — held (no blocking request_changes), left open", () => {
Expand Down
7 changes: 5 additions & 2 deletions test/unit/queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1222,8 +1222,11 @@ describe("queue processors", () => {
},
});

const count = await env.DB.prepare("select count(*) as n from audit_events where event_type like 'agent.action.%'").first<{ n: number }>();
expect(count?.n).toBe(0);
// A non-confirmed contributor (neutral/advisory gate) is no longer left SILENT — the bot may surface it with a
// label so it's visible, but it takes NO TERMINAL action (never auto-merge/close/approve a non-confirmed or
// not-review-good PR). (#harm-stop: neutral flows to held+labeled instead of an empty plan.)
const terminal = await env.DB.prepare("select count(*) as n from audit_events where event_type in ('agent.action.merge','agent.action.close','agent.action.approve')").first<{ n: number }>();
expect(terminal?.n).toBe(0);
});

it("auto-maintain (#778): skips a closed PR even on an agent-configured repo", async () => {
Expand Down
Loading