fix(rereview): add robust team slug fallback (#24)

keithah · web-flow · commit 71cc6f0a2c2a · 2026-02-10T22:32:24.000-08:00
Normalize configured rereview team slugs and fall back between aireview/ai-review on 422 errors. Reuse this logic in both review and mention handlers so rereview triggers stay best-effort but resilient.
diff --git a/src/handlers/mention.test.ts b/src/handlers/mention.test.ts
@@ -1319,6 +1319,7 @@ describe("createMentionHandler rereview command", () => {
               base: { ref: "main" },
             },
           }),
+          listRequestedReviewers: async () => ({ data: { users: [], teams: [] } }),
           requestReviewers: async (params: { team_reviewers: string[] }) => {
             requestedTeams = params.team_reviewers;
             return { data: {} };
diff --git a/src/handlers/mention.ts b/src/handlers/mention.ts
@@ -31,6 +31,7 @@ import { buildMentionContext } from "../execution/mention-context.ts";
 import { buildMentionPrompt } from "../execution/mention-prompt.ts";
 import { classifyError, formatErrorComment, postOrUpdateErrorComment } from "../lib/errors.ts";
 import { wrapInDetails } from "../lib/formatting.ts";
+import { requestRereviewTeamBestEffort } from "./rereview-team.ts";
 
 /**
  * Create the mention handler and register it with the event router.
@@ -580,21 +581,15 @@ export function createMentionHandler(deps: {
           mention.prNumber !== undefined &&
           (normalizedQuestion === "review" || normalizedQuestion === "recheck")
         ) {
-          const rereviewTeam = (config.review.uiRereviewTeam ?? "").trim() || "aireview";
-
-          try {
-            await octokit.rest.pulls.requestReviewers({
-              owner: mention.owner,
-              repo: mention.repo,
-              pull_number: mention.prNumber,
-              team_reviewers: [rereviewTeam],
-            });
-          } catch (err) {
-            logger.warn(
-              { err, prNumber: mention.prNumber, rereviewTeam },
-              "Failed to request ui rereview team for rereview (best-effort)",
-            );
-          }
+          const configuredTeam = (config.review.uiRereviewTeam ?? "").trim() || "aireview";
+          await requestRereviewTeamBestEffort({
+            octokit,
+            owner: mention.owner,
+            repo: mention.repo,
+            prNumber: mention.prNumber,
+            configuredTeam,
+            logger,
+          });
 
           return;
         }
diff --git a/src/handlers/rereview-team.test.ts b/src/handlers/rereview-team.test.ts
@@ -0,0 +1,86 @@
+import { describe, expect, test } from "bun:test";
+import type { Logger } from "pino";
+import {
+  buildRereviewTeamCandidates,
+  requestRereviewTeamBestEffort,
+} from "./rereview-team.ts";
+
+function createNoopLogger(): Logger {
+  return {
+    fatal: () => undefined,
+    error: () => undefined,
+    warn: () => undefined,
+    info: () => undefined,
+    debug: () => undefined,
+    trace: () => undefined,
+    silent: () => undefined,
+  } as unknown as Logger;
+}
+
+describe("rereview-team helpers", () => {
+  test("buildRereviewTeamCandidates normalizes owner/team input and adds alias", () => {
+    expect(buildRereviewTeamCandidates("xbmc/aireview")).toEqual(["aireview", "ai-review"]);
+    expect(buildRereviewTeamCandidates("ai-review")).toEqual(["ai-review", "aireview"]);
+  });
+
+  test("requestRereviewTeamBestEffort skips request when team already present", async () => {
+    let requestCalls = 0;
+    const result = await requestRereviewTeamBestEffort({
+      octokit: {
+        rest: {
+          pulls: {
+            listRequestedReviewers: async () => ({
+              data: { users: [], teams: [{ slug: "aireview", name: "aireview" }] },
+            }),
+            requestReviewers: async () => {
+              requestCalls += 1;
+              return { data: {} };
+            },
+          },
+        },
+      },
+      owner: "xbmc",
+      repo: "kodiai",
+      prNumber: 1,
+      configuredTeam: "aireview",
+      logger: createNoopLogger(),
+    });
+
+    expect(result.alreadyRequested).toBe(true);
+    expect(result.requestedTeam).toBeUndefined();
+    expect(requestCalls).toBe(0);
+  });
+
+  test("requestRereviewTeamBestEffort falls back from aireview to ai-review on 422", async () => {
+    const attempted: string[] = [];
+
+    const result = await requestRereviewTeamBestEffort({
+      octokit: {
+        rest: {
+          pulls: {
+            listRequestedReviewers: async () => ({ data: { users: [], teams: [] } }),
+            requestReviewers: async (params: { team_reviewers: string[] }) => {
+              const slug = params.team_reviewers[0] ?? "";
+              attempted.push(slug);
+              if (slug === "aireview") {
+                const err = new Error("Validation failed") as Error & { status: number };
+                err.status = 422;
+                throw err;
+              }
+              return { data: {} };
+            },
+          },
+        },
+      },
+      owner: "xbmc",
+      repo: "kodiai",
+      prNumber: 2,
+      configuredTeam: "aireview",
+      logger: createNoopLogger(),
+    });
+
+    expect(attempted).toEqual(["aireview", "ai-review"]);
+    expect(result.requestedTeam).toBe("ai-review");
+    expect(result.alreadyRequested).toBe(false);
+  });
+});
diff --git a/src/handlers/rereview-team.ts b/src/handlers/rereview-team.ts
@@ -0,0 +1,104 @@
+import type { Logger } from "pino";
+
+function normalizeConfiguredTeam(team: string): string {
+  const trimmed = team.trim().toLowerCase();
+  if (trimmed.length === 0) return "";
+  const lastSlash = trimmed.lastIndexOf("/");
+  return lastSlash >= 0 ? trimmed.slice(lastSlash + 1) : trimmed;
+}
+
+export function buildRereviewTeamCandidates(team: string): string[] {
+  const primary = normalizeConfiguredTeam(team);
+  if (primary.length === 0) return [];
+
+  const candidates = new Set<string>([primary]);
+  if (primary === "ai-review") candidates.add("aireview");
+  if (primary === "aireview") candidates.add("ai-review");
+  return Array.from(candidates);
+}
+
+function getStatusCode(err: unknown): number | undefined {
+  if (typeof err !== "object" || err === null) return undefined;
+  const value = (err as { status?: unknown }).status;
+  return typeof value === "number" ? value : undefined;
+}
+
+export async function requestRereviewTeamBestEffort(options: {
+  octokit: {
+    rest: {
+      pulls: {
+        listRequestedReviewers: (params: {
+          owner: string;
+          repo: string;
+          pull_number: number;
+        }) => Promise<{ data: { teams?: Array<{ slug?: string | null; name?: string | null }> } }>;
+        requestReviewers: (params: {
+          owner: string;
+          repo: string;
+          pull_number: number;
+          team_reviewers: string[];
+        }) => Promise<unknown>;
+      };
+    };
+  };
+  owner: string;
+  repo: string;
+  prNumber: number;
+  configuredTeam: string;
+  logger: Logger;
+}): Promise<{ requestedTeam?: string; alreadyRequested: boolean }> {
+  const { octokit, owner, repo, prNumber, configuredTeam, logger } = options;
+  const candidates = buildRereviewTeamCandidates(configuredTeam);
+  if (candidates.length === 0) return { alreadyRequested: false };
+
+  try {
+    const requested = await octokit.rest.pulls.listRequestedReviewers({
+      owner,
+      repo,
+      pull_number: prNumber,
+    });
+
+    const already = (requested.data.teams ?? []).some((team) => {
+      const slug = (team.slug ?? "").trim().toLowerCase();
+      const name = (team.name ?? "").trim().toLowerCase();
+      return candidates.includes(slug) || candidates.includes(name);
+    });
+    if (already) return { alreadyRequested: true };
+  } catch (err) {
+    logger.warn(
+      { err, owner, repo, prNumber, configuredTeam },
+      "Failed to list requested reviewers; attempting rereview request anyway",
+    );
+  }
+
+  for (const candidate of candidates) {
+    try {
+      await octokit.rest.pulls.requestReviewers({
+        owner,
+        repo,
+        pull_number: prNumber,
+        team_reviewers: [candidate],
+      });
+      return { requestedTeam: candidate, alreadyRequested: false };
+    } catch (err) {
+      const status = getStatusCode(err);
+      const canFallback = status === 422 && candidate !== candidates[candidates.length - 1];
+      logger.warn(
+        {
+          err,
+          owner,
+          repo,
+          prNumber,
+          configuredTeam,
+          candidate,
+          status,
+          fallbackRemaining: canFallback,
+        },
+        "Failed to request rereview team candidate",
+      );
+      if (!canFallback) break;
+    }
+  }
+
+  return { alreadyRequested: false };
+}
diff --git a/src/handlers/review.ts b/src/handlers/review.ts
@@ -15,6 +15,7 @@ import {
   buildReviewOutputKey,
   ensureReviewOutputNotPublished,
 } from "./review-idempotency.ts";
+import { requestRereviewTeamBestEffort } from "./rereview-team.ts";
 import { $ } from "bun";
 import { fetchAndCheckoutPullRequestHeadRef } from "../jobs/workspace.ts";
 
@@ -61,50 +62,6 @@ export function createReviewHandler(deps: {
 
   const rereviewTeamSlugs = new Set(["ai-review", "aireview"]);
 
-  async function ensureUiRereviewTeamRequested(options: {
-    octokit: Awaited<ReturnType<GitHubApp["getInstallationOctokit"]>>;
-    owner: string;
-    repo: string;
-    prNumber: number;
-    team: string;
-    logger: Logger;
-  }): Promise<void> {
-    const { octokit, owner, repo, prNumber, team } = options;
-    const normalized = team.trim().toLowerCase();
-    if (normalized.length === 0) return;
-
-    try {
-      const requested = await octokit.rest.pulls.listRequestedReviewers({
-        owner,
-        repo,
-        pull_number: prNumber,
-      });
-      const already =
-        (requested.data.teams ?? []).some((t) => (t.slug ?? "").trim().toLowerCase() === normalized) ||
-        (requested.data.teams ?? []).some((t) => (t.name ?? "").trim().toLowerCase() === normalized);
-      if (already) return;
-    } catch (err) {
-      logger.warn(
-        { err, owner, repo, prNumber, team },
-        "Failed to list requested reviewers; attempting to request ui rereview team anyway",
-      );
-    }
-
-    try {
-      await octokit.rest.pulls.requestReviewers({
-        owner,
-        repo,
-        pull_number: prNumber,
-        team_reviewers: [team],
-      });
-    } catch (err) {
-      logger.warn(
-        { err, owner, repo, prNumber, team },
-        "Failed to request ui rereview team (best-effort)",
-      );
-    }
-  }
-
   async function handleReview(event: WebhookEvent): Promise<void> {
     const payload = event.payload as unknown as
       | PullRequestOpenedEvent
@@ -333,12 +290,12 @@ export function createReviewHandler(deps: {
           (action === "opened" || action === "ready_for_review")
         ) {
           const octokit = await githubApp.getInstallationOctokit(event.installationId);
-          await ensureUiRereviewTeamRequested({
+          await requestRereviewTeamBestEffort({
             octokit,
             owner: apiOwner,
             repo: apiRepo,
             prNumber: pr.number,
-            team: config.review.uiRereviewTeam,
+            configuredTeam: config.review.uiRereviewTeam,
             logger,
           });
         }
