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
9 changes: 7 additions & 2 deletions src/settings/agent-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,10 @@ export function downgradeMergeToHold(planned: PlannedAgentAction[], holdOnly: bo
if (labels.manualReview !== null && !alreadyNeedsReview) {
next.push({
actionClass: "label",
autonomyClass: "review_state_label",
// Authorized by `merge` (the class actually being downgraded here), NOT `review_state_label` — mirrors
// the guardrail-hold label above (#label-scoping) so this hold label posts whenever merge autonomy is
// acting, independent of whether the repo has separately opted into the advisory review_state_label class.
autonomyClass: "merge",
requiresApproval: stagedMerge?.requiresApproval ?? false,
reason: "accuracy circuit-breaker engaged (merge precision dropped) — would-merge held for human review",
label: labels.manualReview,
Expand Down Expand Up @@ -428,7 +431,9 @@ export function downgradeCloseToHold(planned: PlannedAgentAction[], closeHoldOnl
if (labels.manualReview !== null && !alreadyNeedsReview) {
next.push({
actionClass: "label",
autonomyClass: "review_state_label",
// Authorized by `close` (the class actually being downgraded here), NOT `review_state_label` — same
// reasoning as downgradeMergeToHold's own manual-review label above (#label-scoping).
autonomyClass: "close",
requiresApproval: droppedClose?.requiresApproval ?? false,
reason: "close-precision circuit-breaker engaged — would-close held for human review",
label: labels.manualReview,
Expand Down
23 changes: 23 additions & 0 deletions test/unit/agent-actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,18 @@ describe("downgradeMergeToHold — accuracy circuit-breaker (#self-improve / GAP
const plan = wouldMerge();
expect(downgradeMergeToHold(plan, false)).toBe(plan);
});

it("REGRESSION: the substitute manual-review label is authorized by `merge` autonomy, not `review_state_label` — it must still post when review_state_label is OFF (the default)", () => {
// Deliberately autonomy: { merge: "auto" } ONLY — review_state_label is left unset (default OFF), unlike
// every other test in this block. Before the fix, the pushed label carried autonomyClass: "review_state_label",
// so the executor's autonomy re-check (agent-action-executor.ts) would DENY it here even though merge autonomy
// is fully "auto" — the circuit breaker would correctly drop the merge but silently fail to label the PR.
const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { merge: "auto" }, autoMaintain: { requireApprovals: 0, mergeMethod: "squash" }, pr: { labels: [], mergeableState: "clean" } }));
expect(classes(plan)).toEqual(["merge"]); // sanity: review_state_label OFF means no disposition label is planned
const held = downgradeMergeToHold(plan, true);
const label = held.find((a) => a.actionClass === "label" && a.label === AGENT_LABEL_NEEDS_REVIEW && a.labelOp === "add");
expect(label?.autonomyClass).toBe("merge");
});
});

describe("downgradeCloseToHold — close-precision circuit-breaker (#close-precision-breaker)", () => {
Expand Down Expand Up @@ -1079,6 +1091,17 @@ describe("downgradeCloseToHold — close-precision circuit-breaker (#close-preci
const heldNullish = downgradeCloseToHold([nullishClose], true);
expect(heldNullish.find((a) => a.actionClass === "label" && a.label === AGENT_LABEL_NEEDS_REVIEW)?.requiresApproval).toBe(false);
});

it("REGRESSION: the substitute manual-review label is authorized by `close` autonomy, not `review_state_label` — it must still post when review_state_label is OFF (the default)", () => {
// Deliberately autonomy: { close: "auto" } ONLY — review_state_label is left unset (default OFF), mirroring
// downgradeMergeToHold's own regression test above. Before the fix, the pushed label carried
// autonomyClass: "review_state_label" and the executor would deny it even though `close` autonomy is "auto".
const plan = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { close: "auto" }, ciState: "passed", blockerTitles: ["readiness score too low"], pr: { labels: [] } }));
expect(plan.some((a) => a.actionClass === "close" && a.closeKind === "heuristic")).toBe(true);
const held = downgradeCloseToHold(plan, true);
const label = held.find((a) => a.actionClass === "label" && a.label === AGENT_LABEL_NEEDS_REVIEW && a.labelOp === "add");
expect(label?.autonomyClass).toBe("close");
});
});

describe("closeConcreteEvidence — concrete-evidence exemption from the close-precision breaker (#hard-blockers-not-ai-judgment)", () => {
Expand Down
Loading