fix(agent-actions): re-verify live CI before a merge or heuristic close

src/services/agent-action-executor.ts

@@ -1,6 +1,9 @@
 import { bumpPullRequestMergeAttempt, createPendingAgentActionIfAbsent, insertNotificationDeliveryIfAbsent, isGlobalAgentFrozen, markPullRequestApproved, markPullRequestMergeBlocked, recordAuditEvent } from "../db/repositories";
 import { classifyMergeFailure, MERGE_RETRY_CAP } from "./merge-failure";
 import { notifyActionToDiscord, notifyActionToSlack, type NotifyOutcome } from "./notify-discord";
+import { createInstallationToken } from "../github/app";
+import { fetchLiveCiAggregate } from "../github/backfill";
+import { githubRateLimitAdmissionKeyForToken } from "../github/client";
 import { ensurePullRequestLabel, removePullRequestLabel } from "../github/labels";
 import { closePullRequest, createIssueComment, createPullRequestReview, dismissLatestBotApproval, mergePullRequest, updatePullRequestBranch } from "../github/pr-actions";
 import { fetchPullRequestFreshness, pullRequestFreshnessDetail } from "../github/pr-freshness";
@@ -114,12 +117,42 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE
       await audit("denied", `${pullRequestFreshnessDetail(freshness)} — action not executed`);
       continue;
     }
-    // 6) Write-permission readiness: a PR-write action needs `pull_requests: write` granted.
+    // 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")) {
+      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);
+      // The planner itself only ever stages a merge when ciState === "passed" exactly (reviewGood in
+      // agent-actions.ts; "pending" short-circuits to no actions at all upstream) -- the live re-check must
+      // require the SAME exact state, not just "not failed". Otherwise a check that regressed to pending or
+      // became unreadable (unverified) between planning and actuation would still merge, on the assumption
+      // that only an explicit failure invalidates the plan.
+      const staleReason =
+        action.actionClass === "merge"
+          ? liveCi.ciState !== "passed"
+            ? `live CI is no longer passing (now: ${liveCi.ciState})`
+            : null
+          : liveCi.ciState !== "failed"
+            ? `CI state changed since planning (now: ${liveCi.ciState})`
+            : null;
+      if (staleReason) {
+        await audit("denied", `${staleReason} — action not executed`);
+        continue;
+      }
+    }
+    // 7) Write-permission readiness: a PR-write action needs `pull_requests: write` granted.
     if (PR_WRITE_CLASSES.has(action.actionClass) && resolveAgentPermissionReadiness({ autonomy: ctx.autonomy, installationPermissions: ctx.installationPermissions }) !== "ready") {
       await audit("denied", "pull_requests: write not granted — maintainer must re-consent");
       continue;
     }
-    // 7) live — perform the real mutation, recording success or the error.
+    // 8) live — perform the real mutation, recording success or the error.
     try {
       await performAction(env, ctx, action);
       await audit("completed", action.reason);
@@ -241,7 +274,9 @@ 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).
+    // 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.
     ...(action.closeKind !== undefined ? { closeKind: action.closeKind } : {}),
   };
 }

src/types.ts

@@ -682,14 +682,17 @@ export type AgentPendingActionParams = {
   reviewBody?: string;
   mergeMethod?: AutoMergeMethod;
   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.
+  closeKind?: "linked-issue-hard-rule" | "blacklist" | "heuristic";
   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
   // field. (#2254)
   dismissStaleApproval?: boolean;
-  // WHICH kind of close this is (see PlannedAgentAction.closeKind) — must round-trip through staging so the
-  // close-precision circuit-breaker can still scope itself correctly when a staged close is later accepted (#2127).
-  closeKind?: "linked-issue-hard-rule" | "blacklist" | "heuristic";
 };
 
 export type AgentPendingActionStatus = "pending" | "accepted" | "rejected";

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

@@ -23,11 +23,23 @@ vi.mock("../../src/github/pr-freshness", async (importOriginal) => {
     })),
   };
 });
+vi.mock("../../src/github/app", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("../../src/github/app")>()),
+  createInstallationToken: vi.fn(async () => "test-installation-token"),
+}));
+// The actuation-time live CI re-check (#2128) defaults to "still passing" so the existing merge tests stay
+// deterministic; individual tests below override this to exercise the staleness-denial path.
+vi.mock("../../src/github/backfill", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("../../src/github/backfill")>()),
+  fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] })),
+}));
 
 import { closePullRequest, createIssueComment, createPullRequestReview, dismissLatestBotApproval, mergePullRequest, updatePullRequestBranch } from "../../src/github/pr-actions";
 import { ensurePullRequestLabel, removePullRequestLabel } from "../../src/github/labels";
 import { fetchPullRequestFreshness } from "../../src/github/pr-freshness";
-import { actionParams, executeAgentMaintenanceActions, pendingClosureLabelApplied, type AgentActionExecutionContext, type AgentActionOutcome } from "../../src/services/agent-action-executor";
+import { createInstallationToken } from "../../src/github/app";
+import { fetchLiveCiAggregate } from "../../src/github/backfill";
+import { actionParams, executeAgentMaintenanceActions, pendingActionToPlanned, pendingClosureLabelApplied, type AgentActionExecutionContext, type AgentActionOutcome } from "../../src/services/agent-action-executor";
 import type { PlannedAgentAction } from "../../src/settings/agent-actions";
 import { AGENT_LABEL_PENDING_CLOSURE } from "../../src/review/linked-issue-hard-rules";
 import { isGlobalAgentFrozen, setGlobalAgentFrozen, upsertPullRequestFromGitHub } from "../../src/db/repositories";
@@ -157,6 +169,84 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
     expect(fetchPullRequestFreshness).toHaveBeenCalledWith(env, expect.objectContaining({ expectedHeadSha: "reviewed-sha" }));
   });
 
+  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" };
+    vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [heuristicClose]);
+    expect(outcomes[0]?.outcome).toBe("denied");
+    expect(outcomes[0]?.detail).toContain("CI state changed since planning (now: passed)");
+    expect(closePullRequest).not.toHaveBeenCalled();
+  });
+
+  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" };
+    vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [heuristicClose]);
+    expect(outcomes[0]?.outcome).toBe("completed");
+    expect(closePullRequest).toHaveBeenCalledWith(env, 123, "owner/repo", 7);
+  });
+
+  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" };
+    // 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.
+    const persisted = actionParams(heuristicClose);
+    const replayed = pendingActionToPlanned({ actionClass: "close", params: persisted, reason: heuristicClose.reason });
+    expect(replayed.closeKind).toBe("heuristic");
+    vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "passed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
+    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-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" };
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [hardRuleClose]);
+    expect(outcomes[0]?.outcome).toBe("completed");
+    expect(fetchLiveCiAggregate).not.toHaveBeenCalled();
+  });
+
+  it("LIVE merge is denied when live CI has since turned failing (#2128)", async () => {
+    const env = createTestEnv({});
+    vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "failed", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]);
+    expect(outcomes[0]?.outcome).toBe("denied");
+    expect(outcomes[0]?.detail).toContain("live CI is no longer passing (now: failed)");
+    expect(mergePullRequest).not.toHaveBeenCalled();
+  });
+
+  it("REGRESSION (#2364): LIVE merge is denied when live CI has since become pending, not just failed", async () => {
+    const env = createTestEnv({});
+    vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "pending", hasPending: true, hasVisiblePending: true, failingDetails: [], nonRequiredFailingDetails: [] });
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]);
+    expect(outcomes[0]?.outcome).toBe("denied");
+    expect(outcomes[0]?.detail).toContain("live CI is no longer passing (now: pending)");
+    expect(mergePullRequest).not.toHaveBeenCalled();
+  });
+
+  it("REGRESSION (#2364): LIVE merge is denied when live CI has since become unverified (unreadable), not just failed", async () => {
+    const env = createTestEnv({});
+    vi.mocked(fetchLiveCiAggregate).mockResolvedValueOnce({ ciState: "unverified", hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] });
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]);
+    expect(outcomes[0]?.outcome).toBe("denied");
+    expect(outcomes[0]?.detail).toContain("live CI is no longer passing (now: unverified)");
+    expect(mergePullRequest).not.toHaveBeenCalled();
+  });
+
+  it("the live CI re-check fails open on a token-mint error — it is defense-in-depth, not the primary gate (#2128)", async () => {
+    const env = createTestEnv({});
+    vi.mocked(createInstallationToken).mockRejectedValueOnce(new Error("mint failed"));
+    const outcomes = await executeAgentMaintenanceActions(env, ctx(), [merge]);
+    expect(outcomes[0]?.outcome).toBe("completed");
+    expect(mergePullRequest).toHaveBeenCalledWith(env, 123, "owner/repo", 7, { mergeMethod: "squash", sha: "sha7" });
+  });
+
   it("LIVE label with labelOp=add + comment: adds the label AND posts the comment", async () => {
     const env = createTestEnv({});
     const flag: PlannedAgentAction = { actionClass: "label", requiresApproval: false, reason: "flag", label: "gittensory:pending-closure", labelOp: "add", comment: "⚠️ flagged" };

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

@@ -24,8 +24,9 @@ vi.mock("../../src/github/app", async (importOriginal) => ({
   ...(await importOriginal<typeof import("../../src/github/app")>()),
   createInstallationToken: vi.fn(async () => "test-installation-token"),
 }));
-// The accept-time live re-check (#2126) defaults to "everything still looks fine" so the existing accept tests
-// stay deterministic; individual tests below override these to exercise the staleness-supersede path.
+// The accept-time live re-check (#2126) AND the actuation-time live CI re-check (#2128) both default to
+// "everything still looks fine" so the existing accept tests stay deterministic; individual tests below
+// override these to exercise the staleness-supersede / staleness-denial paths.
 vi.mock("../../src/github/backfill", async (importOriginal) => ({
   ...(await importOriginal<typeof import("../../src/github/backfill")>()),
   fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [] })),

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

@@ -23,7 +23,9 @@ vi.mock("../../src/github/pr-freshness", async (importOriginal) => {
 // Without this mock, an unconfigured GITHUB_APP_PRIVATE_KEY leaves the accept-time live-recheck token mint
 // undefined; fetchLiveCiAggregate then FULFILLS with ciState "unverified" (not a rejection), which the
 // accept-time staleness check (agent-approval-queue.ts) now treats as non-"passed" — a genuine stale signal,
-// not a fail-open case. Default to "passed" so a staged-merge happy path isn't accidentally exercising that.
+// not a fail-open case. #2364's live CI re-check (in executeAgentMaintenanceActions) runs for every
+// merge/heuristic-close accept too. Default to "passed" so the existing "accept executes the staged merge"
+// happy path executes live instead of being (correctly) denied for CI neither test was simulating.
 vi.mock("../../src/github/backfill", async (importOriginal) => {
   const actual = await importOriginal<typeof import("../../src/github/backfill")>();
   return {