fix(gh-design): skip self-review requests and tolerate 422 errors

app/tasks/gh-design/filterReviewers.spec.ts

@@ -0,0 +1,51 @@
+import { describe, expect, it } from "bun:test";
+import { filterReviewers } from "./filterReviewers";
+
+describe("filterReviewers", () => {
+  const REVIEWERS = ["PabloWiedemann", "AliceDev"];
+
+  it("excludes the PR author from both lists", () => {
+    const result = filterReviewers(REVIEWERS, "PabloWiedemann");
+    expect(result.requestReviewers).toEqual(["AliceDev"]);
+    expect(result.newReviewers).toEqual(["AliceDev"]);
+  });
+
+  it("returns all reviewers when author is not in the list", () => {
+    const result = filterReviewers(REVIEWERS, "SomeoneElse");
+    expect(result.requestReviewers).toEqual(["PabloWiedemann", "AliceDev"]);
+    expect(result.newReviewers).toEqual(["PabloWiedemann", "AliceDev"]);
+  });
+
+  it("excludes already-requested reviewers from newReviewers only", () => {
+    const result = filterReviewers(REVIEWERS, "SomeoneElse", ["PabloWiedemann"]);
+    expect(result.requestReviewers).toEqual(["PabloWiedemann", "AliceDev"]);
+    expect(result.newReviewers).toEqual(["AliceDev"]);
+  });
+
+  it("returns empty newReviewers when all are already requested", () => {
+    const result = filterReviewers(REVIEWERS, "SomeoneElse", ["PabloWiedemann", "AliceDev"]);
+    expect(result.requestReviewers).toEqual(["PabloWiedemann", "AliceDev"]);
+    expect(result.newReviewers).toEqual([]);
+  });
+
+  it("returns empty lists when author is the only reviewer", () => {
+    const result = filterReviewers(["PabloWiedemann"], "PabloWiedemann");
+    expect(result.requestReviewers).toEqual([]);
+    expect(result.newReviewers).toEqual([]);
+  });
+
+  it("handles undefined alreadyRequested as no-one requested yet", () => {
+    const result = filterReviewers(REVIEWERS, "SomeoneElse", undefined);
+    expect(result.newReviewers).toEqual(["PabloWiedemann", "AliceDev"]);
+  });
+
+  it("compares usernames case-insensitively", () => {
+    const result = filterReviewers(REVIEWERS, "pablowiedemann");
+    expect(result.requestReviewers).toEqual(["AliceDev"]);
+  });
+
+  it("matches already-requested reviewers case-insensitively", () => {
+    const result = filterReviewers(REVIEWERS, "SomeoneElse", ["pablowiedemann"]);
+    expect(result.newReviewers).toEqual(["AliceDev"]);
+  });
+});

app/tasks/gh-design/filterReviewers.ts

@@ -0,0 +1,17 @@
+/**
+ * Compute eligible reviewers for a PR.
+ * Returns `requestReviewers` (all reviewers minus the PR author) and
+ * `newReviewers` (eligible reviewers not yet requested).
+ * GitHub usernames are case-insensitive, so comparisons are normalized.
+ */
+export function filterReviewers(
+  allReviewers: string[],
+  prAuthor: string,
+  alreadyRequested?: string[],
+): { requestReviewers: string[]; newReviewers: string[] } {
+  const normalizedAuthor = prAuthor.toLowerCase();
+  const normalizedRequested = new Set(alreadyRequested?.map((r) => r.toLowerCase()) ?? []);
+  const requestReviewers = allReviewers.filter((e) => e.toLowerCase() !== normalizedAuthor);
+  const newReviewers = requestReviewers.filter((e) => !normalizedRequested.has(e.toLowerCase()));
+  return { requestReviewers, newReviewers };
+}

app/tasks/gh-design/gh-design.ts

@@ -21,6 +21,7 @@ import {
   planDesignCommentNotification,
 } from "./slackNotifications";
 import { slackMessageUrlParse, slackMessageUrlStringify } from "./slackMessageUrlParse";
+import { filterReviewers } from "./filterReviewers";
 const tlog = createTimeLogger();
 
 /**
@@ -254,21 +255,37 @@ export async function runGithubDesignTask() {
           });
 
       if (task.state === "open") {
-        if (
-          task.type === "pull_request" &&
-          REQUEST_REVIEWERS.some((e) => !task.reviewers?.includes(e))
-        ) {
-          const requestReviewers = REQUEST_REVIEWERS;
-          const newReviewers = requestReviewers.filter((e) => !task.reviewers?.includes(e));
-          tlog(`Requesting reviewers: ${newReviewers.join(", ")}`);
-          if (!dryRun) {
-            await gh.pulls.requestReviewers({
-              owner,
-              repo,
-              pull_number: issue_number,
-              reviewers: newReviewers,
-            });
-            task = await saveGithubDesignTask(url, { reviewers: requestReviewers });
+        if (task.type === "pull_request") {
+          const { requestReviewers, newReviewers } = filterReviewers(
+            REQUEST_REVIEWERS,
+            task.user,
+            task.reviewers,
+          );
+          if (newReviewers.length > 0) {
+            tlog(`Requesting reviewers: ${newReviewers.join(", ")}`);
+            if (!dryRun) {
+              let reviewersRequested = false;
+              try {
+                await gh.pulls.requestReviewers({
+                  owner,
+                  repo,
+                  pull_number: issue_number,
+                  reviewers: newReviewers,
+                });
+                reviewersRequested = true;
+              } catch (err: unknown) {
+                // GitHub may return 422 when a requested reviewer cannot be added,
+                // such as when they are not a collaborator or cannot be requested.
+                // We log but don't persist, so the request will be retried on the
+                // next run (the reviewer may become eligible later).
+                const status = (err as { status?: number })?.status;
+                if (status !== 422) throw err;
+                tlog(`Reviewer request rejected (422): ${err}`);
+              }
+              if (reviewersRequested) {
+                task = await saveGithubDesignTask(url, { reviewers: requestReviewers });
+              }
+            }
           }
         }