diff --git a/src/github/app.ts b/src/github/app.ts index a929132482..2e447eca14 100644 --- a/src/github/app.ts +++ b/src/github/app.ts @@ -143,7 +143,7 @@ export async function createInstallationToken( return mint; } -function githubErrorStatus(error: unknown): number | null { +export function githubErrorStatus(error: unknown): number | null { const err = error as { status?: number; response?: { status?: number } | null; diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index 3d0defde8b..86fd39259a 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -1,8 +1,8 @@ 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 { createInstallationToken, githubErrorStatus } from "../github/app"; +import { fetchLiveCiAggregate, refreshInstallationHealthForInstallation } 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"; @@ -181,6 +181,16 @@ export async function executeAgentMaintenanceActions(env: Env, ctx: AgentActionE if (action.actionClass === "merge" && ctx.headSha) { await handleMergeFailure(env, ctx, error); } + // #2265: a 403 on a PR-write mutation often means the LOCAL installations.permissions snapshot is stale — + // GitHub webhooks a consented permission UPGRADE but sends nothing for a maintainer-initiated downgrade, so + // the write-permission readiness gate (step 6 above) can keep reporting "ready" for up to the 30-minute + // health-refresh cron interval after a live downgrade. Opportunistically refresh now so the DB row (and + // therefore every later sweep/webhook read of it, for this or any other PR on the installation) self-heals + // immediately instead of waiting for the next cron tick. GitHub's own server-side enforcement (this very + // 403) is already the real backstop, so a failed refresh here is safe to swallow. + if (PR_WRITE_CLASSES.has(action.actionClass) && githubErrorStatus(error) === 403) { + await refreshInstallationHealthForInstallation(env, ctx.installationId).catch(() => undefined); + } } } diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index 43c31ec9cb..604aec1192 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -32,13 +32,14 @@ vi.mock("../../src/github/app", async (importOriginal) => ({ vi.mock("../../src/github/backfill", async (importOriginal) => ({ ...(await importOriginal()), fetchLiveCiAggregate: vi.fn(async () => ({ ciState: "passed" as const, hasPending: false, hasVisiblePending: false, failingDetails: [], nonRequiredFailingDetails: [], ciCompletenessWarning: null })), + refreshInstallationHealthForInstallation: vi.fn(async () => null), })); 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 { createInstallationToken } from "../../src/github/app"; -import { fetchLiveCiAggregate } from "../../src/github/backfill"; +import { fetchLiveCiAggregate, refreshInstallationHealthForInstallation } 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"; @@ -494,6 +495,38 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(outcomes[0]?.detail).toMatch(/not mergeable/i); expect((await auditFor(env, "merge"))?.outcome).toBe("error"); }); + + it("opportunistically refreshes installation health when a PR-write mutation fails with a 403 (#2265)", async () => { + const env = createTestEnv({}); + vi.mocked(closePullRequest).mockRejectedValueOnce(Object.assign(new Error("Resource not accessible by integration"), { status: 403 })); + const outcomes = await executeAgentMaintenanceActions(env, ctx(), [close]); + expect(outcomes[0]?.outcome).toBe("error"); + expect(refreshInstallationHealthForInstallation).toHaveBeenCalledTimes(1); + expect(refreshInstallationHealthForInstallation).toHaveBeenCalledWith(env, 123); + }); + + it("does not refresh installation health for a non-403 mutation failure (#2265)", async () => { + const env = createTestEnv({}); + vi.mocked(closePullRequest).mockRejectedValueOnce(new Error("network timeout")); + await executeAgentMaintenanceActions(env, ctx(), [close]); + expect(refreshInstallationHealthForInstallation).not.toHaveBeenCalled(); + }); + + it("does not refresh installation health on a 403 from a non-PR-write action (label uses issues:write, not pull_requests) (#2265)", async () => { + const env = createTestEnv({}); + vi.mocked(ensurePullRequestLabel).mockRejectedValueOnce(Object.assign(new Error("Resource not accessible by integration"), { status: 403 })); + await executeAgentMaintenanceActions(env, ctx(), [label]); + expect(refreshInstallationHealthForInstallation).not.toHaveBeenCalled(); + }); + + it("swallows a failed installation-health refresh — best-effort, does not affect the recorded outcome (#2265)", async () => { + const env = createTestEnv({}); + vi.mocked(closePullRequest).mockRejectedValueOnce(Object.assign(new Error("Resource not accessible by integration"), { status: 403 })); + vi.mocked(refreshInstallationHealthForInstallation).mockRejectedValueOnce(new Error("refresh boom")); + const outcomes = await executeAgentMaintenanceActions(env, ctx(), [close]); + expect(outcomes[0]?.outcome).toBe("error"); + expect((await auditFor(env, "close"))?.outcome).toBe("error"); + }); }); describe("pendingClosureLabelApplied (#1136 Pass-2 trigger)", () => {