fix(agent-actions): scope live CI close recheck

src/services/agent-action-executor.ts

@@ -129,15 +129,22 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE
       await audit("denied", `${pullRequestFreshnessDetail(freshness)} — action not executed`);
       continue;
     }
-    // 6) Live CI re-verification for a merge or a heuristic close (#2128): the CI aggregate that drove either
-    //    decision was read seconds-to-tens-of-seconds earlier, in the planning pass, and the freshness guard
-    //    above only re-checks head SHA/state, not CI. GitHub's own merge endpoint enforces branch-protection
-    //    REQUIRED checks server-side, but only as a backstop when a repo actually configures them; a heuristic
-    //    close has no server-side check at all. Re-read live CI right before the mutation so a check that
-    //    flipped in this narrow window is never acted on from stale information. Deterministic closes
-    //    (linked-issue hard-rule, blacklist) are exempt — they are zero-hallucination facts that do not depend
-    //    on CI, and the linked-issue rule already has its own flag-then-verify pass.
-    if (action.actionClass === "merge" || (action.actionClass === "close" && action.closeKind === "heuristic")) {
+    // 6) Live CI re-verification for a merge or a CI-driven heuristic close (#2128): the CI aggregate that drove
+    //    either decision was read seconds-to-tens-of-seconds earlier, in the planning pass, and the freshness
+    //    guard above only re-checks head SHA/state, not CI. GitHub's own merge endpoint enforces
+    //    branch-protection REQUIRED checks server-side, but only as a backstop when a repo actually configures
+    //    them; a red-CI close has no server-side check at all. Re-read live CI right before the mutation so a
+    //    check that flipped in this narrow window is never acted on from stale information. Non-CI closes
+    //    (gate verdict, duplicate/slop, conflict, linked-issue hard-rule, blacklist) are exempt — their adverse
+    //    signal does not depend on CI still being red.
+    //    A heuristic close staged BEFORE #2478 has no closeRequiresCiState at all -- that field didn't exist yet
+    //    -- so `undefined` here is genuinely ambiguous (a legacy CI-driven close and a legacy non-CI close are
+    //    byte-identical in storage). The planner now ALWAYS sets the field going forward (never omits it), so
+    //    `undefined` can only mean a legacy row; treat it with the old, broader pre-#2478 guard (require CI still
+    //    failed) rather than skipping the recheck, which would let a stale CI-driven close silently execute
+    //    after CI recovers (flagged by the gate's own review of #2478).
+    const isAmbiguousLegacyHeuristicClose = action.actionClass === "close" && action.closeKind === "heuristic" && action.closeRequiresCiState === undefined;
+    if (action.actionClass === "merge" || (action.actionClass === "close" && action.closeRequiresCiState === "failed") || isAmbiguousLegacyHeuristicClose) {
       const ciToken = await createInstallationToken(env, ctx.installationId).catch(() => undefined);
       const admissionKey = githubRateLimitAdmissionKeyForToken(env, ciToken, ctx.installationId);
       const liveCi = await fetchLiveCiAggregate(env, ctx.repoFullName, expectedHeadSha, ciToken, undefined, admissionKey);
@@ -151,7 +158,9 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE
           ? liveCi.ciState !== "passed"
             ? `live CI is no longer passing (now: ${liveCi.ciState})`
             : null
-          : liveCi.ciState !== "failed"
+          // isAmbiguousLegacyHeuristicClose falls back to "failed" (the old unconditional requirement); an
+          // explicitly-tagged fresh close compares against its own recorded requirement.
+          : liveCi.ciState !== (action.closeRequiresCiState ?? "failed")
             ? `CI state changed since planning (now: ${liveCi.ciState})`
             : null;
       if (staleReason) {
@@ -317,10 +326,11 @@ export function actionParams(action: PlannedAgentAction): AgentPendingActionPara
     ...(action.dismissStaleApproval !== undefined ? { dismissStaleApproval: action.dismissStaleApproval } : {}),
     // Round-trip closeKind so a staged close's kind survives to accept-time — without it, the close-precision
     // breaker's isHeuristicClose check (which matches on closeKind === "heuristic") could never fire for any
-    // staged close, silently defeating the breaker for the entire approval-queue accept path (#2127), and the
-    // actuation-time live-CI re-check above (#2364) — which only applies to a heuristic close — would be
-    // silently skipped for a lost discriminator.
+    // staged close, silently defeating the breaker for the entire approval-queue accept path (#2127).
     ...(action.closeKind !== undefined ? { closeKind: action.closeKind } : {}),
+    // Round-trip the CI dependency separately from closeKind: closeKind is intentionally broad (gate-verdict /
+    // duplicate / slop / CI) for the close-precision breaker, but only red-CI closes need the live-CI guard.
+    ...(action.closeRequiresCiState !== undefined ? { closeRequiresCiState: action.closeRequiresCiState } : {}),
   };
 }
 

src/settings/agent-actions.ts

@@ -62,6 +62,11 @@ export type PlannedAgentAction = {
   // (silently holding a close whose comment already promised closure would be incoherent). Absent on non-close
   // actions; treated as a heuristic close only when explicitly tagged "heuristic".
   closeKind?: "linked-issue-hard-rule" | "blacklist" | "heuristic";
+  // For a CI-driven heuristic close, the CI state that must still hold at actuation time. Other heuristic
+  // closes (gate verdict, duplicate/slop, conflict) do not depend on red CI and must not be blocked by green CI.
+  // ALWAYS set for a heuristic close (never omitted) -- see the field's doc comment on AgentPendingActionParams
+  // in types.ts for why the tri-state (rather than an optional "failed") matters (#2478).
+  closeRequiresCiState?: "failed" | "not_required";
   expectedHeadSha?: string;
   // For an `approve` action: retract the bot's own prior approval instead of posting a new one — a later commit
   // no longer qualifies for approval, but the PR isn't merging or closing this pass, so the stale APPROVE
@@ -559,6 +564,9 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne
       // Pin like merge/approve (#2452): lets the accept-time supersede check detect a force-push after staging;
       // the executor's own step-6 live-CI re-check (#2128) separately covers the CI-driven reason above.
       ...(input.pr.headSha ? { expectedHeadSha: input.pr.headSha } : {}),
+      // Always explicit (never omitted) -- see the field's doc comment (#2478): an omitted value on a REPLAYED
+      // staged action must unambiguously mean "legacy row, predates this field", not "not CI-driven".
+      closeRequiresCiState: ciFailed ? "failed" : "not_required",
     });
   }
   // else: guarded → manual (needs-human/changes label above); not-good OWNER/automation → held

src/types.ts

@@ -693,10 +693,13 @@ export type AgentPendingActionParams = {
   closeComment?: string;
   // Which kind of close this is (see PlannedAgentAction.closeKind), persisted so it round-trips through staging:
   // the close-precision circuit-breaker still scopes itself correctly when a staged close is later accepted
-  // (#2127), and the actuation-time live-CI re-check (#2364) — which only applies to a heuristic close — still
-  // fires correctly once the row is replayed through pendingActionToPlanned, rather than silently skipping for
-  // a lost discriminator.
+  // (#2127).
   closeKind?: "linked-issue-hard-rule" | "blacklist" | "heuristic";
+  // For a CI-driven heuristic close, persist the CI state that must still hold when the staged action replays
+  // (#2364). This is separate from closeKind because heuristic closes also cover non-CI adverse signals.
+  // ALWAYS set (to "failed" or "not_required") for a freshly planned heuristic close (#2478) -- never omitted --
+  // so `undefined` unambiguously means a LEGACY row staged before this field existed, not "not CI-driven".
+  closeRequiresCiState?: "failed" | "not_required";
   expectedHeadSha?: string;
   // For an `approve` action: retract the bot's own stale approval instead of posting a new one (see
   // PlannedAgentAction.dismissStaleApproval). Must round-trip through staging like every other action-specific

test/unit/agent-action-executor.test.ts

@@ -197,7 +197,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
 
   it("LIVE heuristic close is denied when live CI has since turned green (#2128)", async () => {
     const env = createTestEnv({});
-    const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic" };
+    const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic", closeRequiresCiState: "failed" };
     vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
     const outcomes = await executeAgentMaintenanceActions(env, ctx(), [heuristicClose]);
     expect(outcomes[0]?.outcome).toBe("denied");
@@ -207,7 +207,7 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
 
   it("LIVE heuristic close proceeds when live CI is still failing (#2128)", async () => {
     const env = createTestEnv({});
-    const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic" };
+    const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic", closeRequiresCiState: "failed" };
     vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
     const outcomes = await executeAgentMaintenanceActions(env, ctx(), [heuristicClose]);
     expect(outcomes[0]?.outcome).toBe("completed");
@@ -216,20 +216,54 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
 
   it("REGRESSION (#2364): a queued heuristic close still re-checks live CI after the approval-queue replay round trip", async () => {
     const env = createTestEnv({});
-    const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic" };
+    const heuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic", closeRequiresCiState: "failed" };
     // Simulate the persist/replay path: stageForApproval calls actionParams() to persist the row, and accept
-    // rebuilds it via pendingActionToPlanned(). Without persisting closeKind, the rebuilt action would lose the
-    // discriminator the live-CI re-check keys on, silently skipping it for every accepted queued heuristic close.
+    // rebuilds it via pendingActionToPlanned(). Persist both the broad close kind and the narrower CI
+    // dependency so queued red-CI closes still get the live-CI re-check without applying it to every heuristic close.
     const persisted = actionParams(heuristicClose);
     const replayed = pendingActionToPlanned({ actionClass: "close", params: persisted, reason: heuristicClose.reason });
     expect(replayed.closeKind).toBe("heuristic");
+    expect(replayed.closeRequiresCiState).toBe("failed");
     vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
     const outcomes = await executeAgentMaintenanceActions(env, ctx(), [replayed]);
     expect(outcomes[0]?.outcome).toBe("denied");
     expect(outcomes[0]?.detail).toContain("CI state changed since planning (now: passed)");
     expect(closePullRequest).not.toHaveBeenCalled();
   });
 
+  it("LIVE non-CI heuristic close proceeds when live CI is passing because the close reason is independent of CI", async () => {
+    const env = createTestEnv({});
+    // "not_required", not omitted: the planner always tags a fresh heuristic close explicitly (#2478).
+    const gateClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "policy gate blocker", closeComment: "closing", closeKind: "heuristic", closeRequiresCiState: "not_required" };
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [gateClose]);
+    expect(outcomes[0]?.outcome).toBe("completed");
+    expect(closePullRequest).toHaveBeenCalledWith(env, 123, "owner/repo", 7);
+    expect(fetchLiveCiAggregate).not.toHaveBeenCalled();
+  });
+
+  it("REGRESSION (#2478, flagged by the gate's own review of #2478): a LEGACY heuristic close staged before closeRequiresCiState existed (closeKind heuristic, field entirely absent) still re-checks live CI and is DENIED once CI has turned green", async () => {
+    // Simulates a pending_agent_actions row persisted by code that predates #2478 -- closeKind: "heuristic" with
+    // no closeRequiresCiState key at all, since the field didn't exist yet. The fix must NOT silently skip the
+    // live-CI recheck for this row just because the new "not_required" tag is absent, or a stale CI-driven close
+    // could execute after CI recovers.
+    const env = createTestEnv({});
+    const legacyHeuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic" };
+    vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [legacyHeuristicClose]);
+    expect(outcomes[0]?.outcome).toBe("denied");
+    expect(outcomes[0]?.detail).toContain("CI state changed since planning (now: passed)");
+    expect(closePullRequest).not.toHaveBeenCalled();
+  });
+
+  it("a LEGACY heuristic close (closeRequiresCiState absent) still proceeds when live CI is genuinely still failing, matching the old pre-#2478 behavior", async () => {
+    const env = createTestEnv({});
+    const legacyHeuristicClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "CI failed", closeComment: "closing", closeKind: "heuristic" };
+    vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null });
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [legacyHeuristicClose]);
+    expect(outcomes[0]?.outcome).toBe("completed");
+    expect(closePullRequest).toHaveBeenCalledWith(env, 123, "owner/repo", 7);
+  });
+
   it("LIVE non-heuristic close (linked-issue hard-rule) skips the live CI re-check entirely (#2128)", async () => {
     const env = createTestEnv({});
     const hardRuleClose: PlannedAgentAction = { actionClass: "close", requiresApproval: false, reason: "unlinked issue", closeComment: "closing", closeKind: "linked-issue-hard-rule" };

test/unit/agent-actions.test.ts

@@ -357,6 +357,9 @@ describe("planAgentMaintenanceActions (#778)", () => {
       const cls = classes(plan);
       expect(cls).not.toContain("merge");
       expect(cls).toContain("close");
+      // "not_required", not undefined -- the planner always tags a heuristic close explicitly (#2478) so a
+      // REPLAYED staged action can tell "not CI-driven" apart from "legacy row, field didn't exist yet".
+      expect(plan.find((a) => a.actionClass === "close")?.closeRequiresCiState).toBe("not_required");
       expect(plan.find((a) => a.actionClass === "label")?.label).toBe(AGENT_LABEL_CHANGES);
     });
 
@@ -485,6 +488,7 @@ describe("planAgentMaintenanceActions (#778)", () => {
       expect(close).toBeTruthy();
       expect(close?.reason).toContain("CI is failing");
       expect(close?.reason).toContain("codecov/patch");
+      expect(close?.closeRequiresCiState).toBe("failed");
     });
 
     it("NEVER closes the owner's red-CI PR — held via the changes-requested LABEL only (no blocking request_changes), left open", () => {

test/unit/agent-approval-queue.test.ts

@@ -886,7 +886,7 @@ describe("agent approval queue (#779)", () => {
     expect(actionParams({ actionClass: "close", requiresApproval: false, reason: "x", closeComment: "C" })).toEqual({ closeComment: "C" });
     // closeKind must round-trip through staging — without it the close-precision breaker could never match a
     // staged close as heuristic on accept (#2127).
-    expect(actionParams({ actionClass: "close", requiresApproval: false, reason: "x", closeComment: "C", closeKind: "heuristic" })).toEqual({ closeComment: "C", closeKind: "heuristic" });
+    expect(actionParams({ actionClass: "close", requiresApproval: false, reason: "x", closeComment: "C", closeKind: "heuristic", closeRequiresCiState: "failed" })).toEqual({ closeComment: "C", closeKind: "heuristic", closeRequiresCiState: "failed" });
   });
 
   it("lists all pending actions unfiltered and stores a null reason when omitted", async () => {