fix(merge): classify a 429 secondary rate limit as infra-scoped (#9795)

`classifyMergeFailure` reached the self-healing `infra` scope for a rate-limit
window only via `status === 403`, but this repo's own GitHub client documents that
a secondary limit surfaces as 403 OR 429 (client.ts:422). A 429 matched no status
or message branch, so it fell through to the commit-scoped default; once the retry
cap was burned the failure went terminal with no expiry, and the head-scoped block
stranded a green, approved PR until the contributor pushed a commit they had no
reason to push. A fleet-wide 429 window catches every in-flight merge at once.

Add an exported `isRateLimitMessage` predicate and two branches, ahead of the
terminal `403`: a `429` and a rate-limited `403` both classify
`{ terminal: false, scope: "infra" }`, so the two spellings of one condition are
treated identically and the block expires on INFRA_MERGE_BLOCK_TTL_MS and is
re-probed autonomously — exactly what the module already documents. No other
status's `terminal` changes; the commit-scoped fall-through default is untouched.

Closes #9693

Author: shin-core

src/services/merge-failure.ts

@@ -77,6 +77,14 @@ function isConvergenceForbiddenMessage(message: string): boolean {
   return /resource not accessible by integration|secondary rate limit|api rate limit|abuse detection/i.test(message);
 }
 
+/** A GitHub rate-limit signal — the message GitHub attaches to a secondary/abuse or primary rate-limit
+ *  response. A merge failing on this is an infra-scoped, self-healing window, whether GitHub spelled it 403 or
+ *  429 (src/github/client.ts treats both the same). Exported so the 403 and 429 branches share one definition
+ *  (#9693). */
+export function isRateLimitMessage(message: string): boolean {
+  return /secondary rate limit|abuse|api rate limit exceeded/i.test(message);
+}
+
 /** Read the HTTP status off an Octokit RequestError (it sets `.status`); undefined for non-HTTP errors. */
 function httpStatus(error: unknown): number | undefined {
   const status = (error as { status?: unknown } | null | undefined)?.status;
@@ -119,6 +127,13 @@ export function classifyMergeFailure(error: unknown): { terminal: boolean; reaso
   const message = errorMessage(error);
   const status = httpStatus(error);
   if (status === 401) return { terminal: true, scope: "infra", reason: `installation token rejected: App suspended or key rotated (401): ${message}` };
+  // A secondary rate-limit window surfaces as 403 OR 429 (src/github/client.ts:422) and is fleet-wide + self-
+  // healing — infra-scoped so it lapses on the TTL re-probe instead of stranding every in-flight merge on a
+  // head-scoped block until an unrelated commit lands (#9693). The 429 arm sits with the other status branches;
+  // the rate-limited-403 arm must precede the terminal `403` below so it is not swallowed by it.
+  if (status === 429) return { terminal: false, scope: "infra", reason: `merge rate-limited (429 — secondary rate limit, self-healing window): ${message}` };
+  if (status === 403 && isRateLimitMessage(message))
+    return { terminal: false, scope: "infra", reason: `merge rate-limited (403 — secondary rate limit, self-healing window): ${message}` };
   if (status === 403 && isConvergenceForbiddenMessage(message))
     return { terminal: false, scope: "infra", reason: `merge forbidden for now (403 — branch protection or GitHub permission visibility may still be converging): ${message}` };
   if (status === 403) return { terminal: true, scope: "commit", reason: `merge forbidden (403): ${message}` };

test/unit/merge-block-recovery.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from "vitest";
 import { getPullRequest, markPullRequestMergeBlocked, bumpPullRequestMergeAttempt, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub, upsertInstallation } from "../../src/db/repositories";
-import { activeMergeBlockedSha, classifyMergeFailure, INFRA_MERGE_BLOCK_TTL_MS, isMergeBlockInEffect } from "../../src/services/merge-failure";
+import { activeMergeBlockedSha, classifyMergeFailure, INFRA_MERGE_BLOCK_TTL_MS, isMergeBlockInEffect, MERGE_RETRY_CAP } from "../../src/services/merge-failure";
 import { AGENT_LABEL_NEEDS_REVIEW, AGENT_LABEL_READY, planAgentMaintenanceActions } from "../../src/settings/agent-actions";
 import { createTestEnv } from "../helpers/d1";
 
@@ -115,6 +115,47 @@ describe("markPullRequestMergeBlocked persists the scope (#9012)", () => {
   });
 });
 
+// #9693: a 429 secondary-rate-limit window that burns MERGE_RETRY_CAP must persist an INFRA-scoped (expiring)
+// block, not a commit-scoped one that strands the PR until an unrelated commit lands. handleMergeFailure derives
+// the expiry from classifyMergeFailure(...).scope, so we drive that composition exactly as the executor does.
+describe("a rate-limit-exhausted merge persists a self-healing infra block (#9693)", () => {
+  const NOW = Date.parse("2026-07-26T12:00:00.000Z");
+  const blockFor = (error: unknown) => {
+    // Mirror handleMergeFailure's retry-cap path: on exhaustion the classified scope decides the expiry.
+    const { scope, terminal } = classifyMergeFailure(error);
+    // A 429/rate-limited failure is non-terminal, so it reaches the cap path and escalates there.
+    expect(terminal).toBe(false);
+    return scope === "infra" ? new Date(NOW + INFRA_MERGE_BLOCK_TTL_MS).toISOString() : undefined;
+  };
+
+  it("persists a non-null, lapsing mergeBlockedUntil after MERGE_RETRY_CAP 429 failures", async () => {
+    const env = createTestEnv();
+    await seedPr(env, "sha-1");
+    // Exhaust the retry budget on the same head, exactly as repeated 429 attempts would.
+    for (let i = 0; i < MERGE_RETRY_CAP; i += 1) await bumpPullRequestMergeAttempt(env, "alice/repo", 5, "sha-1");
+    const expiresAt = blockFor(httpError(429, "You have exceeded a secondary rate limit"));
+    expect(expiresAt).not.toBeUndefined();
+    await markPullRequestMergeBlocked(env, "alice/repo", 5, "sha-1", "merge could not complete after 5 attempt(s)", expiresAt);
+
+    const stored = await getPullRequest(env, "alice/repo", 5);
+    expect(stored?.mergeBlockedUntil).not.toBeNull();
+    // The block genuinely lapses on the TTL — no new commit required.
+    expect(isMergeBlockInEffect(stored!, "sha-1", NOW + INFRA_MERGE_BLOCK_TTL_MS + 1)).toBe(false);
+  });
+
+  it("keeps a 409 merge-conflict block commit-scoped (mergeBlockedUntil null), unchanged", async () => {
+    const env = createTestEnv();
+    await seedPr(env, "sha-1");
+    // A 409 is terminal on the first failure — commit-scoped, no expiry.
+    const { scope, terminal } = classifyMergeFailure(httpError(409, "Required status check is expected."));
+    expect(terminal).toBe(true);
+    const expiresAt = scope === "infra" ? new Date(NOW + INFRA_MERGE_BLOCK_TTL_MS).toISOString() : undefined;
+    await markPullRequestMergeBlocked(env, "alice/repo", 5, "sha-1", "merge conflict (409)", expiresAt);
+
+    expect((await getPullRequest(env, "alice/repo", 5))?.mergeBlockedUntil ?? null).toBeNull();
+  });
+});
+
 // #9012 compounding bug: mergeAttemptCount's own schema and function docs promised "a new commit's attempts
 // start fresh once the row's head advances", but nothing reset it — bumpPullRequestMergeAttempt only scoped the
 // INCREMENT to the head. So once one head exhausted MERGE_RETRY_CAP, every later head was one-strike-terminal.

test/unit/merge-failure.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { classifyMergeFailure, isMergeConflictMessage, isNoNewBaseCommitsMessage, isWorkflowScopeRefusalMessage, MERGE_RETRY_CAP } from "../../src/services/merge-failure";
+import { classifyMergeFailure, isMergeConflictMessage, isNoNewBaseCommitsMessage, isRateLimitMessage, isWorkflowScopeRefusalMessage, MERGE_RETRY_CAP } from "../../src/services/merge-failure";
 
 /** Build an Octokit-style RequestError: an Error carrying an HTTP `.status`. */
 function httpError(status: number, message: string): Error {
@@ -36,13 +36,33 @@ describe("classifyMergeFailure", () => {
   });
 
   it("retries GitHub's generic 403 merge rejection because branch protection can still converge", () => {
-    for (const message of ["Resource not accessible by integration", "secondary rate limit", "API rate limit exceeded", "abuse detection mechanism triggered"]) {
+    // A non-rate-limit convergence 403 (permission visibility still settling) — the converging arm.
+    const result = classifyMergeFailure(httpError(403, "Resource not accessible by integration"));
+    expect(result.terminal).toBe(false);
+    expect(result.scope).toBe("infra");
+    expect(result.reason).toMatch(/converging/i);
+  });
+
+  it("classifies a 429 and a rate-limited 403 as an infra-scoped, self-healing window (#9693)", () => {
+    // 429: no branch matched this before, so it fell through to a commit-scoped block that stranded the PR.
+    const rateLimited429 = classifyMergeFailure(httpError(429, "You have exceeded a secondary rate limit"));
+    expect(rateLimited429).toMatchObject({ terminal: false, scope: "infra" });
+    expect(rateLimited429.reason).toMatch(/rate-limited/i);
+    // 403 spellings of the same window are treated identically, before the terminal 403 branch.
+    for (const message of ["secondary rate limit", "API rate limit exceeded", "abuse detection mechanism triggered"]) {
       const result = classifyMergeFailure(httpError(403, message));
-      expect(result.terminal).toBe(false);
-      expect(result.reason).toMatch(/converging/i);
+      expect(result).toMatchObject({ terminal: false, scope: "infra" });
+      expect(result.reason).toMatch(/rate-limited/i);
     }
   });
 
+  it("exposes isRateLimitMessage matching only rate-limit text (#9693)", () => {
+    expect(isRateLimitMessage("You have exceeded a secondary rate limit")).toBe(true);
+    expect(isRateLimitMessage("abuse detection mechanism triggered")).toBe(true);
+    expect(isRateLimitMessage("API rate limit exceeded")).toBe(true);
+    expect(isRateLimitMessage("Repository does not allow squash merges")).toBe(false);
+  });
+
   it("treats non-convergence 403s, 409, and real merge-conflict text as terminal", () => {
     expect(classifyMergeFailure(httpError(403, "Repository does not allow squash merges")).terminal).toBe(true);
     expect(classifyMergeFailure(httpError(409, "Required status check is expected.")).terminal).toBe(true);