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
2 changes: 1 addition & 1 deletion src/github/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 12 additions & 2 deletions src/services/agent-action-executor.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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);
}
}
}

Expand Down
35 changes: 34 additions & 1 deletion test/unit/agent-action-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ vi.mock("../../src/github/app", async (importOriginal) => ({
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: [], 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";
Expand Down Expand Up @@ -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)", () => {
Expand Down