fix(agent-actions): re-check the linked-issue hard rule on merge accept (#2388)

* fix(agent-actions): re-check the linked-issue hard rule on merge accept

decidePendingAgentAction re-validated only the head SHA before
replaying a staged auto_with_approval merge. The linked-issue hard
rule (owner-assigned / missing-point-label / maintainer-only) is
evaluated fresh on every planning pass and takes precedence over
merge, but a staged merge only replayed the plan-time snapshot: a
maintainer relabeling or reassigning the linked issue between staging
and accept (head SHA unchanged) would still merge a now-ineligible PR.

Re-run resolveLinkedIssueHardRule for a staged merge before executing
it, superseding the same way the head-moved check already does. Skips
the check for an owner/automation-authored PR (unless
closeOwnerAuthors is on), mirroring the planner's own closeEligible
exemption so a trusted PR the rule never blocks in the first place
isn't wrongly denied here.

Live CI re-verification for this same accept path is covered
separately by the already-open PR for #2128, which
executeAgentMaintenanceActions applies to every merge regardless of
caller — no changes needed here for that part.

* test(agent-actions): cover the linked-issue recheck's token-mint failure path

The rebase onto main's later head-pinning fixes shifted this diff's
covered range; close the resulting branch-coverage gap on the
createInstallationToken(...).catch(() => undefined) fallback added for
the #2132 linked-issue hard-rule recheck.

* fix(agent-actions): gate the linked-issue recheck on the post-downgrade plan

The linked-issue hard-rule recheck gated on pending.actionClass (the
ORIGINAL staged class) rather than the plan's actual contents after the
#2127 precision-breaker downgrade. A merge already downgraded to a
needs-human-review label by downgradeMergeToHold would still get its
whole row rejected on a stale linked-issue violation, silently swallowing
the hold label the breaker was supposed to guarantee -- since nothing is
about to merge, the linked-issue state is irrelevant to what plan is
actually going to execute.

Also documents why the recheck's best-effort token mint is intentionally
fail-open, consistent with the sibling #2126 CI/mergeable/review
re-check: resolveLinkedIssueHardRule already degrades to
env.GITHUB_PUBLIC_TOKEN before ever returning "not violated," and this is
the same shared resolver + fail-open contract the live planning path
already relies on for the primary hard-rule decision.

Author: JSONbored

src/services/agent-approval-queue.ts

@@ -1,8 +1,9 @@
 import { getInstallation, getPullRequest, getRepositorySettings, getPendingAgentAction, recordAuditEvent, setPendingAgentActionStatus } from "../db/repositories";
+import { createInstallationToken } from "../github/app";
+import { loadLinkedIssueHardRules, resolveLinkedIssueHardRule } from "../review/linked-issue-hard-rules";
 import { executeAgentMaintenanceActions, pendingActionToPlanned } from "./agent-action-executor";
-import { downgradeCloseToHold, downgradeMergeToHold, type PlannedAgentAction } from "../settings/agent-actions";
+import { downgradeCloseToHold, downgradeMergeToHold, isProtectedAutomationAuthor, type PlannedAgentAction } from "../settings/agent-actions";
 import { isCloseHoldOnly, isHoldOnly } from "../review/outcomes-wire";
-import { createInstallationToken } from "../github/app";
 import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, fetchLivePullRequestReviewDecision } from "../github/backfill";
 import { githubRateLimitAdmissionKeyForToken } from "../github/client";
 import type { AgentPendingActionParams, AgentPendingActionRecord } from "../types";
@@ -148,6 +149,58 @@ export async function decidePendingAgentAction(env: Env, input: { id: string; de
   if (holdOnly) plan = downgradeMergeToHold(plan, true);
   if (closeHoldOnly) plan = downgradeCloseToHold(plan, true);
 
+  // Re-validate a staged MERGE against the CURRENT linked-issue hard-rule state (#2132). The hard rule is
+  // evaluated fresh on every planning pass and takes precedence over merge (see planAgentMaintenanceActions),
+  // but a staged merge only replays the PLAN-TIME snapshot — a maintainer relabeling/reassigning the linked
+  // issue between staging and accept (head SHA unchanged, so the check above doesn't catch it) would otherwise
+  // still merge a now-ineligible PR. Mirrors the planner's own owner/automation exemption (closeEligible) so an
+  // owner's staged merge, which the hard rule never blocks in the first place, is not wrongly denied here.
+  // Gated on the POST-downgrade `plan`, not `pending.actionClass`: the precision-breaker downgrade immediately
+  // above can already have replaced a staged merge with a needs-human-review label (downgradeMergeToHold) — that
+  // downgraded plan isn't going to merge anything, so a stale linked-issue violation must not reject the whole
+  // row and suppress the hold label; it only matters while a merge is still the thing about to execute.
+  if (plan.some((action) => action.actionClass === "merge") && pr) {
+    const repoOwner = pending.repoFullName.includes("/") ? pending.repoFullName.slice(0, pending.repoFullName.indexOf("/")) : "";
+    const authorLogin = pr.authorLogin ?? "";
+    const authorIsOwner = authorLogin.length > 0 && authorLogin.toLowerCase() === repoOwner.toLowerCase();
+    const authorIsAutomationBot = isProtectedAutomationAuthor(pr.authorLogin);
+    const closeEligible = (!authorIsOwner && !authorIsAutomationBot) || (authorIsOwner && settings.closeOwnerAuthors === true);
+    if (closeEligible) {
+      const linkedIssueRulesConfig = await loadLinkedIssueHardRules(env, pending.repoFullName);
+      // Best-effort mint, same as the #2126 CI/mergeable/review re-check above: a failed mint here does NOT
+      // silently skip the recheck -- resolveLinkedIssueHardRule falls back to env.GITHUB_PUBLIC_TOKEN when
+      // ciToken is undefined and still attempts the fetch, only returning "not violated" if that ALSO can't
+      // gather issue facts. This is the same shared resolver + same fail-open contract the LIVE planning path
+      // (processors.ts) already relies on for the PRIMARY hard-rule decision; holding this SECONDARY, narrow-
+      // race-window recheck to a stricter fail-closed standard would deny otherwise-legitimate merges on every
+      // transient token-mint hiccup without closing a real gap (the executor mints its OWN token independently
+      // for the actual merge mutation, so a suspended/broken installation still fails there regardless).
+      const ciToken = await createInstallationToken(env, pending.installationId).catch(() => undefined);
+      const linkedIssueHardRule = await resolveLinkedIssueHardRule({
+        env,
+        repoFullName: pending.repoFullName,
+        repoOwner,
+        config: linkedIssueRulesConfig,
+        body: pr.body,
+        linkedIssues: pr.linkedIssues,
+        ciToken,
+        installationId: pending.installationId,
+      });
+      if (linkedIssueHardRule?.violated) {
+        await setPendingAgentActionStatus(env, pending.id, { status: "rejected", decidedBy: input.decidedBy });
+        await recordAuditEvent(env, {
+          eventType: "agent.pending_action.superseded",
+          actor: input.decidedBy,
+          targetKey,
+          outcome: "denied",
+          detail: `superseded merge: linked-issue hard rule now violated — ${linkedIssueHardRule.reason ?? "ineligible linked issue"}`,
+          metadata: { ...baseMetadata, linkedIssueReason: linkedIssueHardRule.reason },
+        });
+        return { status: "rejected", action: { ...pending, status: "rejected", decidedBy: input.decidedBy }, executionOutcome: "linked_issue_hard_rule" };
+      }
+    }
+  }
+
   const outcomes = await executeAgentMaintenanceActions(
     env,
     {

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

@@ -34,11 +34,22 @@ vi.mock("../../src/github/backfill", async (importOriginal) => ({
   fetchLivePullRequestMergeState: vi.fn(async () => "clean"),
   fetchLivePullRequestReviewDecision: vi.fn(async () => undefined),
 }));
+// resolveLinkedIssueHardRule defaults to the REAL implementation, which is a safe no-op here: loadLinkedIssueHardRules
+// (also real, unmocked) always returns the all-off default config, so the real resolver returns undefined (not
+// violated) without any GitHub fetch. Individual tests override it to exercise the accept-time recheck (#2132).
+vi.mock("../../src/review/linked-issue-hard-rules", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../../src/review/linked-issue-hard-rules")>();
+  return {
+    ...actual,
+    resolveLinkedIssueHardRule: vi.fn(actual.resolveLinkedIssueHardRule),
+  };
+});
 
 import { createPullRequestReview, mergePullRequest } from "../../src/github/pr-actions";
 import { ensurePullRequestLabel } from "../../src/github/labels";
 import { createInstallationToken } from "../../src/github/app";
 import { fetchLiveCiAggregate, fetchLivePullRequestMergeState, fetchLivePullRequestReviewDecision } from "../../src/github/backfill";
+import { resolveLinkedIssueHardRule } from "../../src/review/linked-issue-hard-rules";
 import { actionParams, executeAgentMaintenanceActions, pendingActionToPlanned, type AgentActionExecutionContext } from "../../src/services/agent-action-executor";
 import { decidePendingAgentAction } from "../../src/services/agent-approval-queue";
 import {
@@ -357,6 +368,29 @@ describe("agent approval queue (#779)", () => {
     expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 5, "owner/repo", 7, "gittensory:needs-human-review", { createMissingLabel: true });
   });
 
+  it("REGRESSION: a precision-breaker-downgraded merge still executes the hold/label plan even when the linked issue would now violate the hard rule", async () => {
+    // Before the fix, the linked-issue recheck gated on pending.actionClass (the ORIGINAL staged class), not the
+    // post-downgrade plan -- so a merge already downgraded to a needs-human-review label by the #2127 precision
+    // breaker above would still get its whole row rejected on a stale linked-issue violation, silently swallowing
+    // the hold label the breaker was supposed to guarantee.
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" });
+    await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval", label: "auto" } });
+    await seedInstallation(env);
+    await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "Closes #9" });
+    vi.mocked(resolveLinkedIssueHardRule).mockResolvedValueOnce({ violated: true, reason: "Linked issue #9 is labeled `maintainer-only` — it is not open for community PRs." });
+    const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" });
+    // The merge-precision breaker engages fleet-wide AFTER this merge was staged — same as the #2127 test above.
+    await env.DB.prepare("INSERT INTO system_flags (key, value) VALUES (?, ?)").bind("holdonly:owner/repo", "true").run();
+
+    const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" });
+    expect(result.status).toBe("accepted");
+    expect(result.executionOutcome).toBe("completed");
+    expect(mergePullRequest).not.toHaveBeenCalled();
+    expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 5, "owner/repo", 7, "gittensory:needs-human-review", { createMissingLabel: true });
+    // The recheck must not even run once the plan no longer contains a merge -- there's nothing left to validate.
+    expect(resolveLinkedIssueHardRule).not.toHaveBeenCalled();
+  });
+
   it("accept executes a staged merge normally when the precision breaker is off", async () => {
     const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" });
     await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } });
@@ -467,6 +501,108 @@ describe("agent approval queue (#779)", () => {
     expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 5, "owner/repo", 8, "gittensory:needs-human-review", { createMissingLabel: true });
   });
 
+  it("accept supersedes a staged merge when the linked issue trips a hard rule after staging (#2132)", async () => {
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" });
+    await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } });
+    await seedInstallation(env);
+    // Staged against a CONTRIBUTOR PR whose linked issue was eligible at plan time; between staging and accept
+    // another maintainer relabeled the linked issue (head SHA unchanged, so the freshness check above misses it).
+    await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "Closes #9" });
+    vi.mocked(resolveLinkedIssueHardRule).mockResolvedValueOnce({ violated: true, reason: "Linked issue #9 is labeled `maintainer-only` — it is not open for community PRs." });
+    const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" });
+
+    const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" });
+    expect(result.status).toBe("rejected");
+    expect(result.executionOutcome).toBe("linked_issue_hard_rule");
+    expect(mergePullRequest).not.toHaveBeenCalled();
+    expect((await getPendingAgentAction(env, action.id))?.status).toBe("rejected");
+    const audit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ?").bind("agent.pending_action.superseded").first<{ outcome: string; detail: string }>();
+    expect(audit?.outcome).toBe("denied");
+    expect(audit?.detail).toContain("maintainer-only");
+  });
+
+  it("accept executes a staged merge when the linked issue remains eligible", async () => {
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" });
+    await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } });
+    await seedInstallation(env);
+    await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "Closes #9" });
+    vi.mocked(resolveLinkedIssueHardRule).mockResolvedValueOnce({ violated: false, reason: null });
+    const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" });
+
+    const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" });
+    expect(result.status).toBe("accepted");
+    expect(result.executionOutcome).toBe("completed");
+    expect(mergePullRequest).toHaveBeenCalledWith(env, 5, "owner/repo", 7, { mergeMethod: "squash", sha: "h7" });
+  });
+
+  it("accept still executes when the linked-issue recheck's own token mint fails — fails OPEN, ciToken passed as undefined (#2132)", async () => {
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" });
+    await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } });
+    await seedInstallation(env);
+    await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "Closes #9" });
+    vi.mocked(resolveLinkedIssueHardRule).mockResolvedValueOnce({ violated: false, reason: null });
+    // First call is the #2126 merge-live-recheck's own token mint (succeeds); the second is this new linked-issue
+    // recheck's token mint, which fails here. The executor mints its own token for the actual mutation
+    // independently, so this transient failure must fail open on THIS check specifically, not block the accept.
+    vi.mocked(createInstallationToken).mockResolvedValueOnce("test-installation-token").mockRejectedValueOnce(new Error("installation suspended"));
+    const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" });
+
+    const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" });
+    expect(result.status).toBe("accepted");
+    expect(result.executionOutcome).toBe("completed");
+    expect(mergePullRequest).toHaveBeenCalledWith(env, 5, "owner/repo", 7, { mergeMethod: "squash", sha: "h7" });
+    expect(vi.mocked(resolveLinkedIssueHardRule)).toHaveBeenCalledWith(expect.objectContaining({ ciToken: undefined }));
+  });
+
+  it("accept supersedes with a fallback reason when the hard-rule result omits one", async () => {
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" });
+    await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } });
+    await seedInstallation(env);
+    await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "contributor" }, head: { sha: "h7" }, labels: [], body: "Closes #9" });
+    vi.mocked(resolveLinkedIssueHardRule).mockResolvedValueOnce({ violated: true, reason: null });
+    const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" });
+
+    const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" });
+    expect(result.status).toBe("rejected");
+    expect(result.executionOutcome).toBe("linked_issue_hard_rule");
+    const audit = await env.DB.prepare("select detail from audit_events where event_type = ?").bind("agent.pending_action.superseded").first<{ detail: string }>();
+    expect(audit?.detail).toContain("ineligible linked issue");
+  });
+
+  it("accept tolerates a slash-less repoFullName and a missing PR author login (defensive fallbacks)", async () => {
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" });
+    await upsertRepositorySettings(env, { repoFullName: "solorepo", autonomy: { merge: "auto_with_approval" } });
+    await upsertInstallation(env, {
+      installation: { id: 5, account: { login: "owner", id: 1, type: "User" }, repository_selection: "selected", permissions: { metadata: "read", pull_requests: "write", issues: "write" }, events: ["pull_request"] },
+      repositories: [{ name: "solorepo", full_name: "solorepo", private: false, owner: { login: "owner" } }],
+    });
+    // No `user` on the payload → authorLogin stored null; repoFullName has no "/" → repoOwner falls back to "".
+    await upsertPullRequestFromGitHub(env, "solorepo", { number: 7, title: "PR", state: "open", head: { sha: "h7" }, labels: [], body: "x" });
+    const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "solorepo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" });
+
+    const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" });
+    expect(result.status).toBe("accepted");
+    expect(result.executionOutcome).toBe("completed");
+    expect(mergePullRequest).toHaveBeenCalledWith(env, 5, "solorepo", 7, { mergeMethod: "squash", sha: "h7" });
+  });
+
+  it("accept does not consult the linked-issue hard rule for an owner-authored staged merge (mirrors the planner's closeEligible exemption)", async () => {
+    const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: "x" });
+    await upsertRepositorySettings(env, { repoFullName: "owner/repo", autonomy: { merge: "auto_with_approval" } });
+    await seedInstallation(env);
+    // Author IS the repo owner ("owner/repo" → owner login "owner"); closeOwnerAuthors defaults false, so the
+    // hard rule must never even be consulted for this PR, regardless of what it would say.
+    await upsertPullRequestFromGitHub(env, "owner/repo", { number: 7, title: "PR", state: "open", user: { login: "owner" }, head: { sha: "h7" }, labels: [], body: "Closes #9" });
+    vi.mocked(resolveLinkedIssueHardRule).mockResolvedValueOnce({ violated: true, reason: "would have violated, but must not even be checked" });
+    const { action } = await createPendingAgentActionIfAbsent(env, { repoFullName: "owner/repo", pullNumber: 7, installationId: 5, actionClass: "merge", autonomyLevel: "auto_with_approval", params: { mergeMethod: "squash", expectedHeadSha: "h7" }, reason: "clean" });
+
+    const result = await decidePendingAgentAction(env, { id: action.id, decision: "accept", decidedBy: "owner" });
+    expect(result.status).toBe("accepted");
+    expect(result.executionOutcome).toBe("completed");
+    expect(mergePullRequest).toHaveBeenCalledWith(env, 5, "owner/repo", 7, { mergeMethod: "squash", sha: "h7" });
+    expect(resolveLinkedIssueHardRule).not.toHaveBeenCalled();
+  });
+
   it("accept does not supersede when the PR record is absent (no live head to compare) — proceeds to the executor", async () => {
     const env = createTestEnv({});
     // No PR seeded → getPullRequest returns null → pr?.headSha is undefined, so the staleness guard is skipped