diff --git a/src/api/types.ts b/src/api/types.ts index c1da6db..8093528 100644 --- a/src/api/types.ts +++ b/src/api/types.ts @@ -4,6 +4,14 @@ import { components } from "@octokit/openapi-types"; // Extended types include body_html from GitHub's HTML media type (application/vnd.github.html+json) export type PullRequest = components["schemas"]["pull-request"] & { body_html?: string; + // Stacked PRs (newer than openapi-types@27; null for non-stacked PRs) + stack?: { + id: number; + number: number; + size: number; + position: number; + base: { ref: string; sha: string }; + } | null; }; export type PullRequestFile = components["schemas"]["diff-entry"]; export type ReviewComment = diff --git a/src/browser/components/pr-overview.tsx b/src/browser/components/pr-overview.tsx index d7c9c6e..df48995 100644 --- a/src/browser/components/pr-overview.tsx +++ b/src/browser/components/pr-overview.tsx @@ -62,6 +62,7 @@ import { parseDiffCached, type ParsedDiff } from "../lib/diff"; import type { ReviewComment } from "@/api/types"; import { useQuery } from "@tanstack/react-query"; import { queries } from "../lib/queries"; +import { useOpenPRReviewTab } from "../contexts/tabs"; import { useGitHub, useGitHubReady, @@ -76,6 +77,7 @@ import { type TimelineEvent, type ReviewThread, type PullRequest, + type PullRequestStackData, type PushVersion, } from "../contexts/github"; import { useCanWrite } from "../contexts/auth"; @@ -1367,6 +1369,15 @@ export const PROverview = memo(function PROverview() { label="Files Changed" count={files.length} /> + {pr.stack && ( + setActiveTab("stack")} + icon={} + label="Stack" + count={pr.stack.size} + /> + )} @@ -2218,6 +2229,8 @@ export const PROverview = memo(function PROverview() { refreshing={refreshingChecks} /> )} + + {activeTab === "stack" && } {/* Right Column - Sidebar */} @@ -4750,6 +4763,119 @@ function CommitsTab({ ); } +// ============================================================================ +// Stack Tab Component +// ============================================================================ + +function StackTab() { + const store = usePRReviewStore(); + const pr = usePRReviewSelector((s) => s.pr); + const owner = usePRReviewSelector((s) => s.owner); + const repo = usePRReviewSelector((s) => s.repo); + const stackData = usePRReviewSelector((s) => s.stackData); + const stackLoading = usePRReviewSelector((s) => s.stackLoading); + const openPRReviewTab = useOpenPRReviewTab(); + + // Lazy-load the stack the first time the tab is opened + useEffect(() => { + store.loadStackData(); + }, [store]); + + if (stackLoading && !stackData) { + return ( +
+ + Loading stack... +
+ ); + } + + if (!stackData) { + return ( +

+ This pull request is not part of a stack. +

+ ); + } + + return ( +
+
+ + + {stackData.size} PRs stacked on{" "} + + {stackData.baseRefName} + + +
+
+ {stackData.entries.map((entry) => { + const { pullRequest } = entry; + const isCurrent = pullRequest.number === pr.number; + return ( + + ); + })} +
+
+ ); +} + // ============================================================================ // Checks Tab Component // ============================================================================ diff --git a/src/browser/contexts/github.tsx b/src/browser/contexts/github.tsx index 7d27b73..7b795d2 100644 --- a/src/browser/contexts/github.tsx +++ b/src/browser/contexts/github.tsx @@ -74,6 +74,19 @@ export type UserProfile = components["schemas"]["public-user"]; // Types // ============================================================================ +/** + * Result of an asynchronous merge (POST /pulls/{pull_number}/merge-async). + * While pending, `details.uuid` can be polled until a terminal status. + */ +export interface AsyncMergeResult { + status: "pending" | "merged" | "enqueued" | "failed"; + details: { + message: string; + uuid?: string; + sha?: string; + }; +} + export interface PRSearchResult { id: number; number: number; @@ -137,6 +150,26 @@ export interface PushVersion { beforeSha?: string; } +// GraphQL-only types for GitHub's stacked PRs feature +export interface PullRequestStackEntry { + position: number; + pullRequest: { + number: number; + title: string; + state: string; + merged: boolean; + isDraft: boolean; + }; +} + +export interface PullRequestStackData { + id: string; + number: number; + size: number; + baseRefName: string; + entries: PullRequestStackEntry[]; +} + export function groupCommitsIntoVersions( commits: PRCommit[], maxGapMinutes = 2 @@ -695,6 +728,17 @@ function createGitHubStore() { return queryClient.fetchQuery(queries.pullRequest(owner, repo, number)); } + function getPRStack( + owner: string, + repo: string, + number: number + ): Promise { + if (!octokit) throw new Error("Not initialized"); + return queryClient.fetchQuery( + queries.pullRequestStack(owner, repo, number) + ); + } + function getPRFiles( owner: string, repo: string, @@ -1120,6 +1164,117 @@ function createGitHubStore() { return data; } + /** + * Merge a PR asynchronously (required for stacked PRs). Submits the merge + * request and polls the returned UUID until the merge reaches a terminal + * state. Throws if the merge fails or times out. + */ + async function mergePRAsync( + owner: string, + repo: string, + number: number, + options?: { + merge_method?: "merge" | "squash" | "rebase"; + sha?: string; + } + ): Promise { + if (!octokit) throw new Error("Not initialized"); + + let result: AsyncMergeResult; + try { + const { data } = await octokit.request({ + method: "PUT", + url: "/repos/{owner}/{repo}/pulls/{pull_number}/merge-async", + owner, + repo, + pull_number: number, + merge_method: options?.merge_method ?? "squash", + merge_action: "direct_merge", + sha: options?.sha, + }); + result = data; + } catch (error) { + // 409: an async merge is already in flight for this PR — the response + // carries that request's UUID, which we can poll instead. + const err = error as { + status?: number; + response?: { data?: AsyncMergeResult }; + }; + if (err.status === 409 && err.response?.data) { + result = err.response.data; + } else { + throw error; + } + } + + const { uuid } = result.details ?? {}; + if (!uuid) return result; // already terminal (merged/enqueued/failed) + + // The merge runs as a background job on GitHub (stacked merges can take + // several minutes), so poll patiently instead of giving up quickly. + const startedAt = Date.now(); + const MAX_POLL_MS = 15 * 60_000; + let intervalMs = 2000; + let attempt = 0; + while ( + result.status === "pending" && + Date.now() - startedAt < MAX_POLL_MS + ) { + await new Promise((resolve) => setTimeout(resolve, intervalMs)); + if (Date.now() - startedAt > 2 * 60_000) { + intervalMs = Math.min(intervalMs + 2000, 10_000); + } + attempt++; + + try { + const { data } = await octokit.request({ + method: "GET", + url: "/repos/{owner}/{repo}/pulls/{pull_number}/merge-async/{uuid}", + owner, + repo, + pull_number: number, + uuid, + }); + result = data; + if (result.status !== "pending") break; + } catch { + // Transient error — keep polling; the PR-state check below still runs. + } + + // Accelerator: if the PR itself has flipped to merged, we're done even + // if the UUID endpoint misbehaves. + if (attempt % 5 === 0) { + try { + const pr = await queryClient.fetchQuery({ + ...queries.pullRequest(owner, repo, number), + staleTime: 0, + }); + if (pr.merged) { + return { + status: "merged", + details: { + message: "Merged", + sha: pr.merge_commit_sha ?? undefined, + }, + }; + } + } catch { + // Ignore — rely on UUID polling + } + } + } + + if (result.status === "pending") { + throw new Error( + "Merge is still running in the background on GitHub. It will complete there and the PR status will update." + ); + } + if (result.status === "failed") { + throw new Error(result.details?.message ?? "Failed to merge"); + } + return result; + } + async function dequeuePullRequest( owner: string, repo: string, @@ -2444,6 +2599,7 @@ function createGitHubStore() { searchRepos, searchUsers, getPR, + getPRStack, getPRFiles, getPRFilesForRange, getCommitFiles, @@ -2462,6 +2618,7 @@ function createGitHubStore() { getWorkflowRuns: getWorkflowRunsForSha, approveWorkflowRun, mergePR, + mergePRAsync, dequeuePullRequest, enqueuePullRequest, getPRCommits, diff --git a/src/browser/contexts/pr-review/index.test.ts b/src/browser/contexts/pr-review/index.test.ts index 6cadd19..0b61266 100644 --- a/src/browser/contexts/pr-review/index.test.ts +++ b/src/browser/contexts/pr-review/index.test.ts @@ -41,7 +41,12 @@ function createMockGitHubStore(): GitHubStore { }), invalidatePR: () => {}, getPR: async () => createMockPR(), + getPRStack: async () => null, mergePR: async () => ({ merged: true }), + mergePRAsync: async () => ({ + status: "merged", + details: { message: "merged" }, + }), closePR: async () => {}, reopenPR: async () => {}, deleteBranch: async () => {}, @@ -762,7 +767,12 @@ function createMockGitHubStoreWithVersions( }), invalidatePR: () => {}, getPR: async () => createMockPR(), + getPRStack: async () => null, mergePR: async () => ({ merged: true }), + mergePRAsync: async () => ({ + status: "merged", + details: { message: "merged" }, + }), closePR: async () => {}, reopenPR: async () => {}, deleteBranch: async () => {}, @@ -987,6 +997,197 @@ test("mergePR sets mergeError and clears merging on failure", async () => { expect(state.pr.merged).toBe(false); }); +// ============================================================================ +// mergePR (stacked PRs - async merge endpoint) +// ============================================================================ + +function createStackedMockPR(): PullRequest { + return createMockPR({ + stack: { + id: 1, + number: 2, + size: 2, + position: 1, + base: { ref: "main", sha: "def456" }, + }, + }); +} + +test("mergePR uses async merge endpoint for stacked PRs and sets merged=true", async () => { + let syncCalled = false; + let asyncCalled = false; + const github = { + ...createMockGitHubStore(), + mergePR: async () => { + syncCalled = true; + return { merged: true }; + }, + mergePRAsync: async () => { + asyncCalled = true; + return { status: "merged", details: { message: "merged" } }; + }, + } as unknown as GitHubStore; + const store = new PRReviewStore(github, { + pr: createStackedMockPR(), + files: [], + comments: [], + owner: "test", + repo: "repo", + viewerPermission: "WRITE", + }); + + const result = await store.mergePR(); + + expect(result).toBe(true); + expect(asyncCalled).toBe(true); + expect(syncCalled).toBe(false); + const state = store.getSnapshot(); + expect(state.pr.merged).toBe(true); + expect(state.pr.state).toBe("closed"); + expect(state.merging).toBe(false); +}); + +test("mergePR sets prInMergeQueue when async merge is enqueued", async () => { + const github = { + ...createMockGitHubStore(), + mergePRAsync: async () => ({ + status: "enqueued", + details: { message: "enqueued" }, + }), + } as unknown as GitHubStore; + const store = new PRReviewStore(github, { + pr: createStackedMockPR(), + files: [], + comments: [], + owner: "test", + repo: "repo", + viewerPermission: "WRITE", + }); + + const result = await store.mergePR(); + + expect(result).toBe(true); + const state = store.getSnapshot(); + expect(state.prInMergeQueue).toBe(true); + expect(state.pr.merged).toBe(false); + expect(state.merging).toBe(false); +}); + +test("mergePR sets mergeError when async merge fails", async () => { + const github = { + ...createMockGitHubStore(), + mergePRAsync: async () => { + throw new Error("Merge failed"); + }, + } as unknown as GitHubStore; + const store = new PRReviewStore(github, { + pr: createStackedMockPR(), + files: [], + comments: [], + owner: "test", + repo: "repo", + viewerPermission: "WRITE", + }); + + const result = await store.mergePR(); + + expect(result).toBe(false); + const state = store.getSnapshot(); + expect(state.merging).toBe(false); + expect(state.mergeError).toBe("Merge failed"); + expect(state.pr.merged).toBe(false); + expect(state.prInMergeQueue).toBe(false); +}); + +// ============================================================================ +// loadStackData +// ============================================================================ + +test("loadStackData fetches and stores the stack for stacked PRs", async () => { + const stackData = { + id: "PRS_1", + number: 3, + size: 2, + baseRefName: "master", + entries: [ + { + position: 1, + pullRequest: { + number: 1, + title: "PR 1", + state: "OPEN", + merged: false, + isDraft: false, + }, + }, + { + position: 2, + pullRequest: { + number: 2, + title: "PR 2", + state: "OPEN", + merged: false, + isDraft: false, + }, + }, + ], + }; + const github = { + ...createMockGitHubStore(), + getPRStack: async () => stackData, + } as unknown as GitHubStore; + const store = new PRReviewStore(github, { + pr: createStackedMockPR(), + files: [], + comments: [], + owner: "test", + repo: "repo", + viewerPermission: "WRITE", + }); + + await store.loadStackData(); + + const state = store.getSnapshot(); + expect(state.stackData).toEqual(stackData); + expect(state.stackLoading).toBe(false); +}); + +test("loadStackData skips fetching when PR is not in a stack", async () => { + let fetchCalled = false; + const github = { + ...createMockGitHubStore(), + getPRStack: async () => { + fetchCalled = true; + return null; + }, + } as unknown as GitHubStore; + const store = new PRReviewStore(github, { + pr: createMockPR(), + files: [], + comments: [], + owner: "test", + repo: "repo", + viewerPermission: "WRITE", + }); + + await store.loadStackData(); + + expect(fetchCalled).toBe(false); + const state = store.getSnapshot(); + expect(state.stackData).toBeNull(); + expect(state.stackLoading).toBe(false); +}); + +test("setOverviewActiveTab switches to the stack tab", () => { + const store = createStore(); + + store.setOverviewActiveTab("stack"); + + const state = store.getSnapshot(); + expect(state.overviewActiveTab).toBe("stack"); + expect(state.showOverview).toBe(true); +}); + // ============================================================================ // Conversation / Timeline events // ============================================================================ diff --git a/src/browser/contexts/pr-review/index.tsx b/src/browser/contexts/pr-review/index.tsx index 7bbeef3..4f942b4 100644 --- a/src/browser/contexts/pr-review/index.tsx +++ b/src/browser/contexts/pr-review/index.tsx @@ -32,6 +32,7 @@ import { type TimelineEvent, type ReviewThread, type PushVersion, + type PullRequestStackData, groupCommitsIntoVersions, } from "@/browser/contexts/github"; import { diffService } from "@/browser/lib/diff"; @@ -157,7 +158,7 @@ export interface WorkflowRunAwaitingApproval { // Merge method type export type MergeMethod = "merge" | "squash" | "rebase"; -export type OverviewTab = "conversation" | "commits" | "checks"; +export type OverviewTab = "conversation" | "commits" | "checks" | "stack"; interface PRReviewState { // Core data @@ -233,6 +234,10 @@ interface PRReviewState { /** Whether deferred version/push data has been loaded */ versionDataLoaded: boolean; + // Stacked PRs (lazily loaded when the Stack tab is first opened) + stackData: PullRequestStackData | null; + stackLoading: boolean; + // Repository merge settings repoAllowMergeCommit: boolean; repoAllowSquashMerge: boolean; @@ -648,6 +653,10 @@ export class PRReviewStore { repoHasMergeQueue: false, prInMergeQueue: false, + // Stacked PRs (lazily loaded) + stackData: null, + stackLoading: false, + // Merge state merging: false, mergeMethod: "squash", @@ -3674,6 +3683,20 @@ export class PRReviewStore { } }; + /** Load the PR stack (lazily, when the Stack tab is first opened). */ + loadStackData = async (): Promise => { + const { owner, repo, pr, stackData, stackLoading } = this.state; + if (stackData || stackLoading || !pr.stack) return; + + this.set({ stackLoading: true }); + try { + const data = await this.github.getPRStack(owner, repo, pr.number); + this.set({ stackData: data, stackLoading: false }); + } catch { + this.set({ stackLoading: false }); + } + }; + /** * Refresh just the checks data */ @@ -3769,6 +3792,25 @@ export class PRReviewStore { this.invalidatePRCaches(owner, repo, pr.number); this.set({ prInMergeQueue: true, merging: false }); + } else if (pr.stack) { + // Stacked PRs must be merged via the asynchronous merge endpoint. + const result = await this.github.mergePRAsync(owner, repo, pr.number, { + merge_method: mergeMethod, + sha: pr.head.sha, + }); + + this.invalidatePRCaches(owner, repo, pr.number); + + if (result.status === "merged") { + this.set({ + pr: { ...this.state.pr, merged: true, state: "closed" as const }, + merging: false, + }); + } else if (result.status === "enqueued") { + this.set({ prInMergeQueue: true, merging: false }); + } else { + throw new Error(result.details?.message ?? "Failed to merge"); + } } else { await this.github.mergePR(owner, repo, pr.number, { merge_method: mergeMethod, diff --git a/src/browser/lib/persistent-cache.test.ts b/src/browser/lib/persistent-cache.test.ts index 80e5591..8a8aaa0 100644 --- a/src/browser/lib/persistent-cache.test.ts +++ b/src/browser/lib/persistent-cache.test.ts @@ -2,10 +2,30 @@ import { test, expect, beforeEach } from "bun:test"; import "fake-indexeddb/auto"; import { get, put, deleteByPRKey, clear } from "./persistent-cache"; +// Simulate the broken pre-existing DB state seen in production: "pulldash" +// exists at version 1 but has no "responses" object store (e.g. a crash +// during the initial upgrade). Module evaluation runs before any openDB +// call, so the first operation below must self-heal this state. +await new Promise((resolve, reject) => { + const req = indexedDB.open("pulldash", 1); + req.onsuccess = () => { + req.result.close(); + resolve(); + }; + req.onerror = () => reject(req.error); +}); + beforeEach(async () => { await clear(); }); +test("openDB self-heals a DB missing the responses store", async () => { + // Must not throw "responses is not a known object store name" + expect(await get("heal-test-key")).toBeNull(); + await put("heal-test-key", "value", "owner/repo/1"); + expect(await get("heal-test-key")).toBe("value"); +}); + test("put then get returns the stored value", async () => { await put("key1", { data: 42 }, "owner/repo/1"); const result = await get<{ data: number }>("key1"); diff --git a/src/browser/lib/persistent-cache.ts b/src/browser/lib/persistent-cache.ts index 78aab7b..59ca11f 100644 --- a/src/browser/lib/persistent-cache.ts +++ b/src/browser/lib/persistent-cache.ts @@ -27,7 +27,39 @@ function openDB(): Promise { } }; - req.onsuccess = () => resolve(req.result); + req.onsuccess = () => { + const db = req.result; + if (db.objectStoreNames.contains(STORE_NAME)) { + resolve(db); + return; + } + // Self-heal: a DB created before the "responses" store existed (e.g. by + // a crash during upgrade) has no usable store, and `transaction(...)` + // would throw. Delete it and reopen so a fresh store is created. + console.warn( + `[persistent-cache] IndexedDB "${DB_NAME}" is missing store "${STORE_NAME}"; recreating it` + ); + db.close(); + const del = indexedDB.deleteDatabase(DB_NAME); + del.onsuccess = () => { + dbPromise = null; + resolve(openDB()); + }; + del.onerror = () => { + dbPromise = null; + reject(del.error); + }; + del.onblocked = () => { + dbPromise = null; + reject( + del.error ?? + new Error( + `IndexedDB: could not delete "${DB_NAME}" (blocked by another connection)` + ) + ); + }; + }; + req.onerror = () => { dbPromise = null; reject(req.error); diff --git a/src/browser/lib/queries.ts b/src/browser/lib/queries.ts index b229ee8..c12b1d2 100644 --- a/src/browser/lib/queries.ts +++ b/src/browser/lib/queries.ts @@ -34,6 +34,7 @@ import type { PRSearchResult, PullRequest, PullRequestFile, + PullRequestStackData, PushVersion, Review, ReviewComment, @@ -350,6 +351,80 @@ export const queries = { staleTime: 30_000, }), + pullRequestStack: (owner: string, repo: string, number: number) => + queryOptions({ + queryKey: ["pull-request", owner, repo, number, "stack"], + queryFn: async ({ signal }) => { + interface StackEntryNode { + position: number; + pullRequest: { + number: number; + title: string; + state: string; + merged: boolean; + isDraft: boolean; + }; + } + const data = await getOctokit().graphql<{ + repository: { + pullRequest: { + stack: { + id: string; + number: number; + size: number; + baseRefName: string; + entries: { edges: Array<{ node: StackEntryNode }> }; + } | null; + }; + }; + }>( + `query GetPullRequestStack($owner: String!, $repo: String!, $number: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + stack { + id + number + size + baseRefName + entries(first: 100) { + edges { + node { + position + pullRequest { + number + title + state + merged + isDraft + } + } + } + } + } + } + } + }`, + { owner, repo, number, request: { signal } } + ); + + const stack = data.repository.pullRequest.stack; + if (!stack) return null; + + const entries = (stack.entries.edges ?? []) + .map(({ node }) => node) + .sort((a, b) => a.position - b.position); + + return { + id: stack.id, + number: stack.number, + size: stack.size, + baseRefName: stack.baseRefName, + entries, + } satisfies PullRequestStackData; + }, + staleTime: 30_000, + }), + pullRequestComments: (owner: string, repo: string, number: number) => queryOptions({ queryKey: ["pull-request", owner, repo, number, "comments"], diff --git a/src/browser/lib/query-client.test.ts b/src/browser/lib/query-client.test.ts new file mode 100644 index 0000000..838dcd4 --- /dev/null +++ b/src/browser/lib/query-client.test.ts @@ -0,0 +1,30 @@ +import { test, expect } from "bun:test"; +import "fake-indexeddb/auto"; + +// Simulate the broken pre-existing DB seen in production: "pulldash-rq" +// exists at version 1 but has no "data" object store (e.g. a crash during +// the initial upgrade). Module evaluation runs before any openDB call, so +// the persister below must self-heal this state. +await new Promise((resolve, reject) => { + const req = indexedDB.open("pulldash-rq", 1); + req.onsuccess = () => { + req.result.close(); + resolve(); + }; + req.onerror = () => reject(req.error); +}); + +import { persister } from "./query-client"; + +test("persister restore self-heals a DB missing the data store", async () => { + // Must not throw "data is not a known object store name" + await persister.restoreClient(); + + const db = await new Promise((resolve, reject) => { + const req = indexedDB.open("pulldash-rq", 1); + req.onsuccess = () => resolve(req.result); + req.onerror = () => reject(req.error); + }); + expect(db.objectStoreNames.contains("data")).toBe(true); + db.close(); +}); diff --git a/src/browser/lib/query-client.ts b/src/browser/lib/query-client.ts index e4acd88..258837a 100644 --- a/src/browser/lib/query-client.ts +++ b/src/browser/lib/query-client.ts @@ -17,7 +17,38 @@ function openDB(): Promise { req.result.createObjectStore(STORE_NAME); } }; - req.onsuccess = () => resolve(req.result); + req.onsuccess = () => { + const db = req.result; + if (db.objectStoreNames.contains(STORE_NAME)) { + resolve(db); + return; + } + // Self-heal: a DB created before the "data" store existed (e.g. by a + // crash during upgrade) has no usable store, and `transaction("data")` + // would throw. Delete it and reopen so a fresh store is created. + console.warn( + `[query-client] IndexedDB "${DB_NAME}" is missing store "${STORE_NAME}"; recreating it` + ); + db.close(); + const del = indexedDB.deleteDatabase(DB_NAME); + del.onsuccess = () => { + dbPromise = null; + resolve(openDB()); + }; + del.onerror = () => { + dbPromise = null; + reject(del.error); + }; + del.onblocked = () => { + dbPromise = null; + reject( + del.error ?? + new Error( + `IndexedDB: could not delete "${DB_NAME}" (blocked by another connection)` + ) + ); + }; + }; req.onerror = () => { dbPromise = null; reject(req.error);