fix: respect github rate limiting

apps/api/src/services/git-platform/github.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
-import { GitHubPlatform } from "./github.js";
+import { GitHubPlatform, GitHubApiError, classifyGitHubFailure } from "./github.js";
 import type { RepoIdentifier } from "@optio/shared";
 
 const ri: RepoIdentifier = {
@@ -239,5 +239,82 @@ describe("GitHubPlatform", () => {
 
       await expect(platform.getPullRequest(ri, 999)).rejects.toThrow("GitHub API error 404");
     });
+
+    it("throws a GitHubApiError carrying status, retry-after, and secondary-limit flag", async () => {
+      mockFetch.mockResolvedValueOnce({
+        ok: false,
+        status: 403,
+        headers: new Headers({ "retry-after": "120" }),
+        json: () => Promise.resolve({}),
+        text: () => Promise.resolve("You have exceeded a secondary rate limit"),
+      });
+
+      await expect(platform.getPullRequest(ri, 1)).rejects.toMatchObject({
+        name: "GitHubApiError",
+        status: 403,
+        isSecondaryRateLimit: true,
+        retryAfterMs: 120000,
+      });
+    });
+
+    it("flags a primary rate limit from x-ratelimit headers", async () => {
+      mockFetch.mockResolvedValueOnce({
+        ok: false,
+        status: 403,
+        headers: new Headers({ "x-ratelimit-remaining": "0", "x-ratelimit-reset": "1900000000" }),
+        json: () => Promise.resolve({}),
+        text: () => Promise.resolve("API rate limit exceeded"),
+      });
+
+      await expect(platform.getPullRequest(ri, 2)).rejects.toMatchObject({
+        name: "GitHubApiError",
+        status: 403,
+        isPrimaryRateLimit: true,
+        isSecondaryRateLimit: false,
+        rateLimitRemaining: 0,
+        rateLimitResetMs: 1900000000000,
+      });
+    });
+  });
+});
+
+describe("classifyGitHubFailure", () => {
+  it("classifies a secondary rate limit (body marker), carrying Retry-After", () => {
+    const f = classifyGitHubFailure(
+      new GitHubApiError(403, "You have exceeded a secondary rate limit", { retryAfterMs: 120000 }),
+    );
+    expect(f).toMatchObject({ kind: "secondary_rate_limit", retryAfterMs: 120000 });
+  });
+
+  it("classifies a primary rate limit (remaining 0) with the reset time", () => {
+    const f = classifyGitHubFailure(
+      new GitHubApiError(403, "API rate limit exceeded", {
+        rateLimitRemaining: 0,
+        rateLimitResetMs: 1_900_000_000_000,
+      }),
+    );
+    expect(f).toMatchObject({ kind: "primary_rate_limit", resetAtMs: 1_900_000_000_000 });
+  });
+
+  it("treats a bare 429 as a secondary rate limit", () => {
+    expect(classifyGitHubFailure(new GitHubApiError(429, "Too Many Requests"))?.kind).toBe(
+      "secondary_rate_limit",
+    );
+  });
+
+  it("classifies 401 as auth and a non-rate-limit 403 as permission", () => {
+    expect(classifyGitHubFailure(new GitHubApiError(401, "Bad credentials"))?.kind).toBe("auth");
+    expect(
+      classifyGitHubFailure(new GitHubApiError(403, "Resource not accessible by integration"))
+        ?.kind,
+    ).toBe("permission");
+  });
+
+  it("returns null for transient 5xx, 4xx logic errors, and non-GitHub errors", () => {
+    expect(classifyGitHubFailure(new GitHubApiError(500, "Server Error"))).toBeNull();
+    expect(classifyGitHubFailure(new GitHubApiError(404, "Not Found"))).toBeNull();
+    expect(classifyGitHubFailure(new GitHubApiError(422, "Unprocessable"))).toBeNull();
+    expect(classifyGitHubFailure(new Error("db hiccup"))).toBeNull();
+    expect(classifyGitHubFailure(undefined)).toBeNull();
   });
 });

apps/api/src/services/git-platform/github.ts

@@ -11,6 +11,113 @@ import type {
   RepoContent,
 } from "@optio/shared";
 
+/**
+ * Error thrown by GitHubPlatform for any non-2xx GitHub REST response. Carries
+ * the verified response metadata callers need to choose the correct recovery:
+ * HTTP status, `Retry-After`, the `x-ratelimit-*` primary-quota signals, and
+ * whether the body indicates a *secondary* rate limit. The `.message` is kept
+ * identical to the previous `GitHub API error <status>: <body>` string so
+ * existing message-based logging/classification is unaffected.
+ */
+export class GitHubApiError extends Error {
+  readonly status: number;
+  readonly retryAfterMs: number | null;
+  readonly rateLimitRemaining: number | null;
+  readonly rateLimitResetMs: number | null;
+  readonly isSecondaryRateLimit: boolean;
+  readonly isPrimaryRateLimit: boolean;
+
+  constructor(
+    status: number,
+    body: string,
+    meta: {
+      retryAfterMs?: number | null;
+      rateLimitRemaining?: number | null;
+      rateLimitResetMs?: number | null;
+    } = {},
+  ) {
+    super(`GitHub API error ${status}: ${body}`);
+    this.name = "GitHubApiError";
+    this.status = status;
+    this.retryAfterMs = meta.retryAfterMs ?? null;
+    this.rateLimitRemaining = meta.rateLimitRemaining ?? null;
+    this.rateLimitResetMs = meta.rateLimitResetMs ?? null;
+    this.isSecondaryRateLimit =
+      (status === 403 || status === 429) &&
+      /secondary rate limit|exceeded a secondary rate limit|abuse detection/i.test(body);
+    // A primary-quota exhaustion is a 403/429 with the remaining counter at 0
+    // and no secondary marker — it must wait for the reset, not the 60s floor.
+    this.isPrimaryRateLimit =
+      (status === 403 || status === 429) &&
+      this.rateLimitRemaining === 0 &&
+      !this.isSecondaryRateLimit;
+    // Preserve the prototype chain so `instanceof GitHubApiError` holds even
+    // under down-level transpilation targets.
+    Object.setPrototypeOf(this, GitHubApiError.prototype);
+  }
+
+  static fromResponse(res: Response, body: string): GitHubApiError {
+    return new GitHubApiError(res.status, body, {
+      retryAfterMs: parseRetryAfterMs(res.headers),
+      rateLimitRemaining: parseIntHeader(res.headers, "x-ratelimit-remaining"),
+      rateLimitResetMs: parseEpochSecondsToMs(res.headers, "x-ratelimit-reset"),
+    });
+  }
+}
+
+function parseRetryAfterMs(headers: Headers | undefined): number | null {
+  const raw = headers?.get("retry-after");
+  if (!raw) return null;
+  const seconds = Number(raw);
+  return Number.isFinite(seconds) ? Math.max(0, seconds) * 1000 : null;
+}
+
+function parseIntHeader(headers: Headers | undefined, name: string): number | null {
+  const raw = headers?.get(name);
+  if (raw == null || raw === "") return null;
+  const n = Number(raw);
+  return Number.isFinite(n) ? n : null;
+}
+
+function parseEpochSecondsToMs(headers: Headers | undefined, name: string): number | null {
+  const seconds = parseIntHeader(headers, name);
+  return seconds == null ? null : seconds * 1000;
+}
+
+/** Distinct GitHub failure classes, each of which needs a distinct retry policy. */
+export type GitHubFailureKind =
+  | "secondary_rate_limit"
+  | "primary_rate_limit"
+  | "auth"
+  | "permission";
+
+export interface GitHubFailure {
+  kind: GitHubFailureKind;
+  /** Server-directed wait (`Retry-After`), if present. */
+  retryAfterMs: number | null;
+  /** Primary-quota reset time (`x-ratelimit-reset`), if present. */
+  resetAtMs: number | null;
+}
+
+/**
+ * Classify a thrown error into the GitHub failure class that determines retry
+ * policy, or `null` when it is NOT a GitHub block (transient 5xx, 404, 422,
+ * network, or any non-GitHub error) — in which case callers keep their normal
+ * fast-retry behavior. The deadline math (how long to wait) is the caller's
+ * responsibility, since it depends on persisted attempt history.
+ */
+export function classifyGitHubFailure(err: unknown): GitHubFailure | null {
+  if (!(err instanceof GitHubApiError)) return null;
+  const { retryAfterMs, rateLimitResetMs: resetAtMs } = err;
+  if (err.isSecondaryRateLimit) return { kind: "secondary_rate_limit", retryAfterMs, resetAtMs };
+  if (err.isPrimaryRateLimit) return { kind: "primary_rate_limit", retryAfterMs, resetAtMs };
+  // A 429 with no secondary marker / remaining counter is still a rate limit.
+  if (err.status === 429) return { kind: "secondary_rate_limit", retryAfterMs, resetAtMs };
+  if (err.status === 401) return { kind: "auth", retryAfterMs: null, resetAtMs: null };
+  if (err.status === 403) return { kind: "permission", retryAfterMs: null, resetAtMs: null };
+  return null;
+}
+
 export class GitHubPlatform implements GitPlatform {
   readonly type = "github" as const;
   private readonly token: string;
@@ -37,7 +144,7 @@ export class GitHubPlatform implements GitPlatform {
     const res = await fetch(url, init);
     if (!res.ok) {
       const body = await res.text().catch(() => "");
-      throw new Error(`GitHub API error ${res.status}: ${body}`);
+      throw GitHubApiError.fromResponse(res, body);
     }
     return (await res.json()) as T;
   }
@@ -196,7 +303,7 @@ export class GitHubPlatform implements GitPlatform {
     });
     if (!res.ok && res.status !== 422) {
       const body = await res.text().catch(() => "");
-      throw new Error(`GitHub API error ${res.status}: ${body}`);
+      throw GitHubApiError.fromResponse(res, body);
     }
   }
 

apps/api/src/services/reconcile-executor.test.ts

@@ -32,8 +32,21 @@ vi.mock("../db/client.js", () => ({
 }));
 
 vi.mock("../db/schema.js", () => ({
-  tasks: { id: "id", updatedAt: "updated_at" },
+  tasks: {
+    id: "id",
+    updatedAt: "updated_at",
+    reconcileBackoffUntil: "reconcile_backoff_until",
+    reconcileAttempts: "reconcile_attempts",
+    errorMessage: "error_message",
+  },
   workflowRuns: { id: "id", updatedAt: "updated_at", state: "state" },
+  prReviews: {
+    id: "id",
+    updatedAt: "updated_at",
+    reconcileBackoffUntil: "reconcile_backoff_until",
+    reconcileAttempts: "reconcile_attempts",
+    errorMessage: "error_message",
+  },
 }));
 
 vi.mock("./task-service.js", () => ({
@@ -89,6 +102,7 @@ vi.mock("../logger.js", () => ({
 
 // Import AFTER mocks
 import { executeAction } from "./reconcile-executor.js";
+import { GitHubApiError } from "./git-platform/github.js";
 
 // ─── Fixtures ───
 
@@ -537,6 +551,68 @@ describe("reconcile-executor", () => {
       expect(outcome.status).toBe("error");
       expect(mockTransitionTask).not.toHaveBeenCalled();
     });
+
+    it("persists a cooldown and does not transition on a secondary rate limit", async () => {
+      mockGetPlatform.mockResolvedValue({
+        platform: { mergePullRequest: mockMergePR },
+        ri: { owner: "acme", repo: "repo" },
+      });
+      mockMergePR.mockRejectedValue(
+        new GitHubApiError(403, "You have exceeded a secondary rate limit", {
+          retryAfterMs: 120000,
+        }),
+      );
+      const chain = chainable([{ id: "task-1" }]);
+      mockDbUpdate.mockReturnValue(chain);
+
+      const snap = repoSnapshot();
+      if (snap.run.kind !== "repo") throw new Error("fixture mismatch");
+      snap.run.status.prUrl = "https://github.com/acme/repo/pull/42";
+      snap.run.status.state = TaskState.PR_OPENED;
+
+      const outcome = await executeAction(
+        { kind: "autoMergePr", reason: "auto_merge_ready" },
+        snap,
+      );
+
+      // Applied (not error) so the worker won't fast-retry; run NOT advanced.
+      expect(outcome.status).toBe("applied");
+      expect(outcome.reason).toContain("github_cooldown");
+      expect(mockTransitionTask).not.toHaveBeenCalled();
+      expect(chain.set).toHaveBeenCalledWith(
+        expect.objectContaining({
+          reconcileBackoffUntil: expect.anything(),
+          errorMessage: expect.stringContaining("secondary rate limit"),
+        }),
+      );
+      // Single delayed wake honoring Retry-After (>=120s).
+      const enqueueArgs = mockEnqueueReconcile.mock.calls.at(-1)?.[1] as { delayMs: number };
+      expect(enqueueArgs.delayMs).toBeGreaterThanOrEqual(120000);
+    });
+
+    it("persists a long cooldown for an auth (bad credentials) failure", async () => {
+      mockGetPlatform.mockResolvedValue({
+        platform: { mergePullRequest: mockMergePR },
+        ri: { owner: "acme", repo: "repo" },
+      });
+      mockMergePR.mockRejectedValue(new GitHubApiError(401, '{"message":"Bad credentials"}'));
+      mockDbUpdate.mockReturnValue(chainable([{ id: "task-1" }]));
+
+      const snap = repoSnapshot();
+      if (snap.run.kind !== "repo") throw new Error("fixture mismatch");
+      snap.run.status.prUrl = "https://github.com/acme/repo/pull/42";
+      snap.run.status.state = TaskState.PR_OPENED;
+
+      const outcome = await executeAction(
+        { kind: "autoMergePr", reason: "auto_merge_ready" },
+        snap,
+      );
+      expect(outcome.status).toBe("applied");
+      expect(outcome.reason).toContain("github_cooldown:auth");
+      expect(mockTransitionTask).not.toHaveBeenCalled();
+      const enqueueArgs = mockEnqueueReconcile.mock.calls.at(-1)?.[1] as { delayMs: number };
+      expect(enqueueArgs.delayMs).toBeGreaterThanOrEqual(60_000);
+    });
   });
 
   describe("resumeAgent", () => {