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
27 changes: 18 additions & 9 deletions src/services/agent-action-executor.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
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, githubErrorStatus } from "../github/app";
import { createInstallationToken, githubErrorStatus, isGitHubRateLimitedError } from "../github/app";
import { fetchLiveCiAggregate, refreshInstallationHealthForInstallation } from "../github/backfill";
import { githubRateLimitAdmissionKeyForToken } from "../github/client";
import { ensurePullRequestLabel, removePullRequestLabel } from "../github/labels";
Expand All @@ -24,6 +24,18 @@ const AGENT_ACTOR = "gittensory";
// (PR_WRITE_ACTION_CLASSES is a superset), so this runtime guard never disagrees with the readiness gate.
export const PR_WRITE_CLASSES = new Set<AgentActionClass>(["request_changes", "approve", "merge", "close", "update_branch"]);

const INSTALLATION_HEALTH_REFRESH_COOLDOWN_MS = 5 * 60 * 1000;
const installationHealthRefreshAttempts = new Map<number, number>();

function shouldRefreshInstallationHealthAfterPrWriteFailure(installationId: number, error: unknown, nowMs = Date.now()): boolean {
if (githubErrorStatus(error) !== 403 || isGitHubRateLimitedError(error)) return false;
if (!/resource not accessible by integration|not have permission/i.test(errorMessage(error))) return false;
const lastAttemptMs = installationHealthRefreshAttempts.get(installationId);
if (lastAttemptMs !== undefined && nowMs - lastAttemptMs < INSTALLATION_HEALTH_REFRESH_COOLDOWN_MS) return false;
installationHealthRefreshAttempts.set(installationId, nowMs);
return true;
}

export type AgentActionExecutionContext = {
installationId: number;
repoFullName: string;
Expand Down Expand Up @@ -181,14 +193,11 @@ 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) {
// #2265: a permission-looking 403 on a PR-write mutation can mean the LOCAL installations.permissions
// snapshot is stale after a maintainer-initiated downgrade (GitHub sends no downgrade webhook). Rate-limit
// 403s and operation-specific forbidden states are not permission evidence, and this refresh scans broad
// installation state, so keep the hot error path narrowly filtered and per-installation cooled down.
if (PR_WRITE_CLASSES.has(action.actionClass) && shouldRefreshInstallationHealthAfterPrWriteFailure(ctx.installationId, error)) {
await refreshInstallationHealthForInstallation(env, ctx.installationId).catch(() => undefined);
}
}
Expand Down
34 changes: 33 additions & 1 deletion test/unit/agent-action-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -519,12 +519,44 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => {
expect(refreshInstallationHealthForInstallation).not.toHaveBeenCalled();
});

it("does not refresh installation health for rate-limit or operation-specific forbidden 403s (#2265)", async () => {
const env = createTestEnv({});
vi.mocked(updatePullRequestBranch)
.mockRejectedValueOnce(Object.assign(new Error("secondary rate limit: please retry later"), { status: 403 }))
.mockRejectedValueOnce(Object.assign(new Error("Update branch is not allowed for this pull request"), { status: 403 }));

await executeAgentMaintenanceActions(env, ctx({ installationId: 124 }), [updateBranch, updateBranch]);

expect(refreshInstallationHealthForInstallation).not.toHaveBeenCalled();
});

it("debounces permission-looking installation health refreshes per installation (#2265)", async () => {
const env = createTestEnv({});
vi.useFakeTimers();
vi.setSystemTime(new Date("2026-07-02T00:00:00Z"));
vi.mocked(closePullRequest).mockRejectedValue(Object.assign(new Error("Resource not accessible by integration"), { status: 403 }));

try {
await executeAgentMaintenanceActions(env, ctx({ installationId: 125 }), [close]);
await executeAgentMaintenanceActions(env, ctx({ installationId: 125 }), [close]);
vi.setSystemTime(new Date("2026-07-02T00:05:01Z"));
await executeAgentMaintenanceActions(env, ctx({ installationId: 125 }), [close]);

expect(refreshInstallationHealthForInstallation).toHaveBeenCalledTimes(2);
expect(refreshInstallationHealthForInstallation).toHaveBeenNthCalledWith(1, env, 125);
expect(refreshInstallationHealthForInstallation).toHaveBeenNthCalledWith(2, env, 125);
} finally {
vi.useRealTimers();
}
});

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]);
const outcomes = await executeAgentMaintenanceActions(env, ctx({ installationId: 126 }), [close]);
expect(outcomes[0]?.outcome).toBe("error");
expect(refreshInstallationHealthForInstallation).toHaveBeenCalledWith(env, 126);
expect((await auditFor(env, "close"))?.outcome).toBe("error");
});
});
Expand Down
Loading