Resolve branch and PR identity consistently for stack discovery

packages/review-protocol/src/review-api-client.ts

@@ -22,14 +22,15 @@ export interface ReviewApiSummary {
   createdAt: string;
   /** Timestamp of the earliest retained review version, when known. */
   firstCreatedAt?: string;
-  /** Present on reviews imported from the legacy MDX store. */
+  /** The branch and PR the review is of; see SnapshotOrigin. */
   origin?: {
     branch?: string;
     baseRef?: string;
     baseBranch?: string;
     pullRequestNumber?: number;
     pullRequestUrl?: string;
     pullRequestTitle?: string;
+    pullRequestBaseBranch?: string;
     revision?: string;
   };
   /** The PR's state on GitHub, when the host has read it. */

packages/review/app/src/review-stack-selector.browser.test.tsx

@@ -265,60 +265,80 @@ describe("ReviewStackSelector", () => {
     expect(container.textContent).toBe("");
   });
 
-  it("lists the stack newest first down to its base, the board on screen opening nothing", async () => {
-    const post = vi.fn<ReviewCanvasBridge["post"]>(async () => ({ ok: true }));
-    const session = testReviewSession({}, { post });
+  it.each([
+    { kind: "GitHub", pullRequests: true, label: "from GitHub" },
+    { kind: "local", pullRequests: false, label: "from branch bases" },
+  ])(
+    "lists a $kind stack newest first down to its base, the board on screen opening nothing",
+    async ({ pullRequests, label }) => {
+      const post = vi.fn<ReviewCanvasBridge["post"]>(async () => ({
+        ok: true,
+      }));
 
-    session.review!.headBranch = "b";
-    session.review!.stack = async () => ({
-      base: "main",
-      layers: (["a", "b", "c"] as const).map((branch, index) => ({
-        branch,
-        relation: (["earlier", "current", "later"] as const)[index]!,
-        pullRequestNumber: 10 + index,
-        pullRequestUrl: `https://github.com/o/r/pull/${10 + index}`,
-        base: null,
-        subject: null,
-        reviewUuid: `${index + 1}`.repeat(8) + "-1111-4111-8111-111111111111",
-        reviewTitle: `Review ${branch}`,
-        pullRequestState: index === 0 ? "merged" : "open",
-      })),
-    });
+      const session = testReviewSession({}, { post });
 
-    const container = document.createElement("div");
-    document.body.append(container);
-    root = createRoot(container);
-    await act(async () => {
-      root?.render(
-        <TestCanvasQuery>
-          <ReviewSessionProvider session={session}>
-            <ReviewStackSelector />
-          </ReviewSessionProvider>
-        </TestCanvasQuery>,
+      session.review!.headBranch = "b";
+      session.review!.stack = async () => ({
+        base: "main",
+        layers: (["a", "b", "c"] as const).map((branch, index) => ({
+          branch,
+          relation: (["earlier", "current", "later"] as const)[index]!,
+          pullRequestNumber: pullRequests ? 10 + index : null,
+          pullRequestUrl: pullRequests
+            ? `https://github.com/o/r/pull/${10 + index}`
+            : null,
+          base: null,
+          subject: null,
+          reviewUuid: `${index + 1}`.repeat(8) + "-1111-4111-8111-111111111111",
+          reviewTitle: `Review ${branch}`,
+          pullRequestState: pullRequests
+            ? index === 0
+              ? "merged"
+              : "open"
+            : undefined,
+        })),
+      });
+
+      const container = document.createElement("div");
+      document.body.append(container);
+      root = createRoot(container);
+      await act(async () => {
+        root?.render(
+          <TestCanvasQuery>
+            <ReviewSessionProvider session={session}>
+              <ReviewStackSelector />
+            </ReviewSessionProvider>
+          </TestCanvasQuery>,
+        );
+      });
+
+      await vi.waitFor(() =>
+        expect(
+          container.querySelector("button[aria-haspopup]")?.ariaLabel,
+        ).toBe("Stack, 2 of 3"),
+      );
+      await openMenu(container);
+      expect(container.querySelector("[role=menu]")?.textContent).toContain(
+        label,
       );
-    });
 
-    await vi.waitFor(() =>
-      expect(container.querySelector("button[aria-haspopup]")?.ariaLabel).toBe(
-        "Stack, 2 of 3",
-      ),
-    );
-    await openMenu(container);
+      const rows = [...container.querySelectorAll("[role=menuitem]")];
 
-    const rows = [...container.querySelectorAll("[role=menuitem]")];
-
-    expect(document.activeElement).toBe(rows[1]);
-    expect(rows.map((row) => row.textContent)).toEqual([
-      expect.stringContaining("Review c"),
-      expect.stringContaining("Review b"),
-      expect.stringMatching(/Review a.*#10 merged/),
-    ]);
-    expect(
-      container.querySelector("[role=separator]")?.nextElementSibling
-        ?.textContent,
-    ).toContain("main");
-
-    await act(async () => (rows[1] as HTMLElement).click());
-    expect(post).not.toHaveBeenCalled();
-  });
+      expect(document.activeElement).toBe(rows[1]);
+      expect(rows.map((row) => row.textContent)).toEqual([
+        expect.stringContaining("Review c"),
+        expect.stringContaining("Review b"),
+        expect.stringMatching(
+          pullRequests ? /Review a.*#10 merged/ : /Review a/,
+        ),
+      ]);
+      expect(
+        container.querySelector("[role=separator]")?.nextElementSibling
+          ?.textContent,
+      ).toContain("main");
+
+      await act(async () => (rows[1] as HTMLElement).click());
+      expect(post).not.toHaveBeenCalled();
+    },
+  );
 });

packages/review/app/src/review-stack-selector.tsx

@@ -222,9 +222,9 @@ export function StackPill({
           <div {...stylex.props(menuStyles.label, styles.heading)}>
             <span {...stylex.props(textStyles.sectionTitle)}>Stack</span>
             <span>
-              {layers.some((layer) => layer.base)
-                ? "from branch bases"
-                : "from GitHub"}{" "}
+              {layers.some((layer) => layer.pullRequestUrl)
+                ? "from GitHub"
+                : "from branch bases"}{" "}
               · newest first
             </span>
           </div>

packages/review/src/github-pr-graph.test.ts

@@ -0,0 +1,47 @@
+import { describe, expect, it, vi } from "vitest";
+
+import { type RunGitHubApi, readGitHubPullRequests } from "./github-pr-graph";
+
+const repo = { full_name: "o/r" };
+
+const record = (number: number) => ({
+  number,
+  title: `PR ${number}`,
+  head: { ref: `feature-${number}`, repo },
+  base: { ref: "main", repo },
+});
+
+describe("repository PR facts", () => {
+  it("reads beyond the first page and coalesces concurrent consumers", async () => {
+    const run = vi.fn<RunGitHubApi>(async (_host: string, endpoint: string) =>
+      JSON.stringify(
+        endpoint.endsWith("page=1")
+          ? Array.from({ length: 100 }, (_, index) => record(index + 1))
+          : [record(101)],
+      ),
+    );
+
+    const [first, second] = await Promise.all([
+      readGitHubPullRequests("github.com", "o/r", run),
+      readGitHubPullRequests("github.com", "o/r", run),
+    ]);
+
+    expect(first).toHaveLength(101);
+    expect(second).toEqual(first);
+    expect(run).toHaveBeenCalledTimes(2);
+  });
+
+  it("allows a new read after a transient failure", async () => {
+    const run = vi
+      .fn<RunGitHubApi>()
+      .mockRejectedValueOnce(new Error("offline"))
+      .mockResolvedValueOnce(JSON.stringify([record(1)]));
+
+    await expect(
+      readGitHubPullRequests("github.com", "o/r", run),
+    ).rejects.toThrow("offline");
+    await expect(
+      readGitHubPullRequests("github.com", "o/r", run),
+    ).resolves.toMatchObject([{ number: 1 }]);
+  });
+});

packages/review/src/github-pr-graph.ts

@@ -0,0 +1,97 @@
+import { z } from "zod";
+
+export type RunGitHubApi = (host: string, endpoint: string) => Promise<string>;
+
+const repository = z.object({ full_name: z.string() });
+
+const pullRequest = z.object({
+  number: z.number().int().positive(),
+  title: z.string(),
+  draft: z.boolean().optional(),
+  head: z.object({ ref: z.string(), repo: repository.nullable() }),
+  base: z.object({ ref: z.string(), repo: repository }),
+});
+
+export interface GitHubPullRequest {
+  number: number;
+  url: string;
+  title: string;
+  headBranch: string;
+  baseBranch: string;
+  draft: boolean;
+}
+
+// Repository facts are independent of which PRs have whiteboards. Share a
+// short-lived, coalesced read between source resolution and stack discovery.
+const caches = new WeakMap<
+  RunGitHubApi,
+  Map<
+    string,
+    {
+      expires: number;
+      read: Promise<GitHubPullRequest[]>;
+    }
+  >
+>();
+
+export function readGitHubPullRequests(
+  host: string,
+  slug: string,
+  run: RunGitHubApi,
+): Promise<GitHubPullRequest[]> {
+  let cache = caches.get(run);
+
+  if (!cache) caches.set(run, (cache = new Map()));
+  const key = `${host}/${slug}`.toLowerCase();
+  const previous = cache.get(key);
+
+  if (previous && previous.expires > Date.now()) return previous.read;
+
+  const read = (async () => {
+    const result: GitHubPullRequest[] = [];
+
+    for (let page = 1; page <= 10; page++) {
+      const records = z
+        .array(pullRequest)
+        .parse(
+          JSON.parse(
+            await run(
+              host,
+              `repos/${slug}/pulls?state=open&per_page=100&page=${page}`,
+            ),
+          ),
+        );
+
+      for (const pr of records) {
+        // A fork's identically named branch is not this repository's branch.
+        if (
+          pr.head.repo?.full_name.toLowerCase() !== slug.toLowerCase() ||
+          pr.base.repo.full_name.toLowerCase() !== slug.toLowerCase()
+        )
+          continue;
+        result.push({
+          number: pr.number,
+          url: `https://${host}/${slug}/pull/${pr.number}`,
+          title: pr.title,
+          headBranch: pr.head.ref,
+          baseBranch: pr.base.ref,
+          draft: pr.draft ?? false,
+        });
+      }
+
+      if (records.length < 100) return result;
+    }
+
+    throw new Error(
+      "PR discovery exceeded 1000 open pull requests; refusing a partial stack.",
+    );
+  })();
+
+  cache.set(key, { expires: Date.now() + 60_000, read });
+  // Failures are optional context, but must not poison the next read.
+  void read.catch(() => {
+    if (cache.get(key)?.read === read) cache.delete(key);
+  });
+
+  return read;
+}