From eef2e1c016924470ce3ab3a02670b1f7e24903e7 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 30 Jul 2026 17:20:52 -0400 Subject: [PATCH 1/4] feat(prs): add canonical GitHub stack state --- .../automationIngressService.test.ts | 46 ++ .../automations/automationIngressService.ts | 8 + .../main/services/github/adeReleaseFeed.ts | 3 + .../main/services/github/githubApiVersion.ts | 1 + .../github/githubAppUserAuthService.ts | 2 + .../services/github/githubService.test.ts | 8 + .../src/main/services/github/githubService.ts | 8 +- .../src/main/services/prs/githubStackStore.ts | 488 ++++++++++++++++++ .../src/main/services/prs/prService.test.ts | 277 ++++++++++ .../src/main/services/prs/prService.ts | 114 +++- .../src/main/services/state/kvDb.test.ts | 47 ++ apps/desktop/src/main/services/state/kvDb.ts | 48 +- apps/desktop/src/shared/types/prs.ts | 39 ++ apps/webhook-relay/src/relay.ts | 10 +- apps/webhook-relay/test/relay.test.ts | 1 + .../pull-requests/github-stacked-prs.md | 195 +++++++ 16 files changed, 1282 insertions(+), 13 deletions(-) create mode 100644 apps/desktop/src/main/services/github/githubApiVersion.ts create mode 100644 apps/desktop/src/main/services/prs/githubStackStore.ts create mode 100644 docs/features/pull-requests/github-stacked-prs.md diff --git a/apps/desktop/src/main/services/automations/automationIngressService.test.ts b/apps/desktop/src/main/services/automations/automationIngressService.test.ts index fed415091..a47308286 100644 --- a/apps/desktop/src/main/services/automations/automationIngressService.test.ts +++ b/apps/desktop/src/main/services/automations/automationIngressService.test.ts @@ -777,6 +777,52 @@ describe("automationIngressService", () => { ); }); + it("repairs repository stack state after an expired relay cursor is committed", async () => { + const setIngressCursor = vi.fn(); + const reconcileGithubStacks = vi.fn(async () => []); + vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(JSON.stringify({ + events: [], + nextCursor: "seq:9", + cursorExpired: true, + hasMore: false, + }), { headers: { "content-type": "application/json" } })); + + service = createAutomationIngressService({ + logger: makeLogger() as never, + automationService: { + updateIngressStatus: vi.fn(), + dispatchIngressTrigger: vi.fn(), + getIngressCursor: () => "seq:2", + setIngressCursor, + getIngressStatus: () => ({}), + } as never, + prService: { + ingestGithubWebhook: vi.fn(), + reconcileGithubStacks, + } as never, + secretService: { getSecret: () => null } as never, + githubService: { + detectRepo: vi.fn(async () => ({ owner: "arul28", name: "ADE" })), + getAppUserTokenForRelay: vi.fn(async () => "ghu_app_user_token"), + }, + listRules: () => [], + }); + + await service.pollNow(); + + expect(setIngressCursor).toHaveBeenCalledWith({ + source: "github-relay", + cursor: "seq:9", + }); + expect(reconcileGithubStacks).toHaveBeenCalledWith({ + owner: "arul28", + name: "ADE", + }); + expect(setIngressCursor.mock.invocationCallOrder[0]).toBeLessThan( + reconcileGithubStacks.mock.invocationCallOrder[0]!, + ); + }); + it("skips a failing event and still advances the relay cursor (poison-event guard)", async () => { const logger = makeLogger(); const setIngressCursor = vi.fn(); diff --git a/apps/desktop/src/main/services/automations/automationIngressService.ts b/apps/desktop/src/main/services/automations/automationIngressService.ts index a0ec2927c..c48b46473 100644 --- a/apps/desktop/src/main/services/automations/automationIngressService.ts +++ b/apps/desktop/src/main/services/automations/automationIngressService.ts @@ -947,6 +947,14 @@ export function createAutomationIngressService(args: AutomationIngressServiceArg setIngressCursor({ source: "github-relay", cursor: pageLastCursor }); for (const prId of pageIngestedPrIds) committedIngestedPrIds.add(prId); } + if (payload.cursorExpired === true && repo) { + await args.prService?.reconcileGithubStacks(repo).catch((error) => { + args.logger.warn("automations.github_stack_cursor_reconcile_failed", { + repo: `${repo.owner}/${repo.name}`, + error: error instanceof Error ? error.message : String(error), + }); + }); + } lastSeenCursor = pageLastCursor; if (useLegacyProjectRoute || payload.hasMore !== true) break; if (!pageLastCursor || pageLastCursor === pageCursor) { diff --git a/apps/desktop/src/main/services/github/adeReleaseFeed.ts b/apps/desktop/src/main/services/github/adeReleaseFeed.ts index 52a53b030..eb16edd77 100644 --- a/apps/desktop/src/main/services/github/adeReleaseFeed.ts +++ b/apps/desktop/src/main/services/github/adeReleaseFeed.ts @@ -1,3 +1,5 @@ +import { GITHUB_REST_API_VERSION } from "./githubApiVersion"; + export const ADE_RELEASE_REPO = { owner: "arul28", name: "ADE" } as const; export type AdeLatestRelease = { @@ -31,6 +33,7 @@ export async function fetchAdeLatestRelease(options?: { const headers: Record = { accept: "application/vnd.github+json", "user-agent": "ade-desktop", + "x-github-api-version": GITHUB_REST_API_VERSION, }; const token = options?.token?.trim(); if (token) headers.authorization = `Bearer ${token}`; diff --git a/apps/desktop/src/main/services/github/githubApiVersion.ts b/apps/desktop/src/main/services/github/githubApiVersion.ts new file mode 100644 index 000000000..bbdfd99f6 --- /dev/null +++ b/apps/desktop/src/main/services/github/githubApiVersion.ts @@ -0,0 +1 @@ +export const GITHUB_REST_API_VERSION = "2026-03-10"; diff --git a/apps/desktop/src/main/services/github/githubAppUserAuthService.ts b/apps/desktop/src/main/services/github/githubAppUserAuthService.ts index 7aab2317c..e3c42cffa 100644 --- a/apps/desktop/src/main/services/github/githubAppUserAuthService.ts +++ b/apps/desktop/src/main/services/github/githubAppUserAuthService.ts @@ -16,6 +16,7 @@ import { createGitHubRelayAuthAuditLog, type GitHubRelayAuthAuditLog, } from "./githubRelayConfig"; +import { GITHUB_REST_API_VERSION } from "./githubApiVersion"; import { asString } from "../shared/utils"; const GITHUB_APP_USER_TOKEN_KEY = "github.appUserToken.v1"; @@ -142,6 +143,7 @@ export function createGitHubAppUserAuthService(args: { accept: "application/vnd.github+json", authorization: `Bearer ${accessToken}`, "user-agent": args.userAgent, + "x-github-api-version": GITHUB_REST_API_VERSION, }, }); const payload = (await response.json().catch(() => ({}))) as Record; diff --git a/apps/desktop/src/main/services/github/githubService.test.ts b/apps/desktop/src/main/services/github/githubService.test.ts index 1b9098bc2..d922d3397 100644 --- a/apps/desktop/src/main/services/github/githubService.test.ts +++ b/apps/desktop/src/main/services/github/githubService.test.ts @@ -179,6 +179,14 @@ describe("githubService.apiRequest", () => { expect(result.data).toEqual(payload); expect(result.response).toBeDefined(); expect(result.response!.status).toBe(200); + expect(mockFetch).toHaveBeenCalledWith( + "https://api.github.com/repos/owner/repo", + expect.objectContaining({ + headers: expect.objectContaining({ + "x-github-api-version": "2026-03-10", + }), + }), + ); }); it("aborts and rejects when the response body never finishes", async () => { diff --git a/apps/desktop/src/main/services/github/githubService.ts b/apps/desktop/src/main/services/github/githubService.ts index 750a38b4a..028d48f0f 100644 --- a/apps/desktop/src/main/services/github/githubService.ts +++ b/apps/desktop/src/main/services/github/githubService.ts @@ -25,6 +25,7 @@ import type { SyncCredentialStore } from "../../../../../ade-cli/src/services/cr import { mergePathEntries, resolveExecutableFromKnownLocations } from "../ai/cliExecutableResolver"; import { fetchGitHubAppInstallationStatus, type GitHubRelaySecretReader } from "./githubRelayConfig"; import { createGitHubAppUserAuthService } from "./githubAppUserAuthService"; +import { GITHUB_REST_API_VERSION } from "./githubApiVersion"; import { classifyGitHubAuthFailure, GitHubRateLimitError, @@ -801,7 +802,8 @@ export function createGithubService({ headers: { accept: "application/vnd.github+json", authorization: `Bearer ${token}`, - "user-agent": "ade-desktop" + "user-agent": "ade-desktop", + "x-github-api-version": GITHUB_REST_API_VERSION, } }); @@ -850,6 +852,7 @@ export function createGithubService({ accept: "application/vnd.github+json", authorization: `Bearer ${token}`, "user-agent": "ade-desktop", + "x-github-api-version": GITHUB_REST_API_VERSION, }, }, ); @@ -1028,7 +1031,8 @@ export function createGithubService({ accept: args.accept?.trim() || "application/vnd.github+json", authorization: `Bearer ${token}`, "content-type": args.body != null ? "application/json" : "text/plain", - "user-agent": "ade-desktop" + "user-agent": "ade-desktop", + "x-github-api-version": GITHUB_REST_API_VERSION, }; // For GET requests, send If-None-Match with cached ETag if available. diff --git a/apps/desktop/src/main/services/prs/githubStackStore.ts b/apps/desktop/src/main/services/prs/githubStackStore.ts new file mode 100644 index 000000000..fb5a4a20f --- /dev/null +++ b/apps/desktop/src/main/services/prs/githubStackStore.ts @@ -0,0 +1,488 @@ +import type { GithubService } from "../github/githubService"; +import type { AdeDb } from "../state/kvDb"; +import { asNumber, asString, getErrorMessage, isRecord, nowIso } from "../shared/utils"; +import type { GitHubRepoRef } from "../../../shared/types/git"; +import type { + GitHubPrStack, + GitHubPrStackMembership, +} from "../../../shared/types/prs"; + +type GitHubPrStackRow = { + project_id: string; + repo_owner: string; + repo_name: string; + github_stack_number: number; + github_stack_id: string; + github_node_id: string | null; + base_branch: string; + is_open: number; + created_at: string; + synced_at: string; + last_error: string | null; +}; + +type GitHubPrStackEntryRow = { + project_id: string; + repo_owner: string; + repo_name: string; + github_stack_number: number; + github_pr_number: number; + position: number; + state: string; + is_draft: number; + merged_at: string | null; + head_branch: string; + head_sha: string; +}; + +type GithubStackStoreLogger = { + warn: (event: string, meta?: Record) => void; +}; + +type DecodedGithubStack = { + row: GitHubPrStackRow; + entries: GitHubPrStackEntryRow[]; +}; + +function repoKey(repo: GitHubRepoRef): string { + return `${repo.owner.toLowerCase()}/${repo.name.toLowerCase()}`; +} + +function repoPrKey(owner: string, name: string, prNumber: number): string { + return `${owner.toLowerCase()}/${name.toLowerCase()}#${prNumber}`; +} + +function stackKey(owner: string, name: string, stackNumber: number): string { + return `${owner.toLowerCase()}/${name.toLowerCase()}#${stackNumber}`; +} + +function stackFromRows( + row: GitHubPrStackRow, + entries: GitHubPrStackEntryRow[], +): GitHubPrStack { + return { + id: row.github_stack_id, + number: Number(row.github_stack_number), + nodeId: row.github_node_id, + repoOwner: row.repo_owner, + repoName: row.repo_name, + baseBranch: row.base_branch, + open: Number(row.is_open) !== 0, + createdAt: row.created_at, + syncedAt: row.synced_at, + lastError: row.last_error, + entries: [...entries] + .sort((left, right) => Number(left.position) - Number(right.position)) + .map((entry) => ({ + githubPrNumber: Number(entry.github_pr_number), + position: Number(entry.position), + state: entry.state === "closed" ? "closed" : "open", + isDraft: Number(entry.is_draft) !== 0, + mergedAt: entry.merged_at, + headBranch: entry.head_branch, + headSha: entry.head_sha, + })), + }; +} + +export function createGithubStackStore(args: { + db: AdeDb; + projectId: string; + githubService: GithubService; + logger: GithubStackStoreLogger; + onSnapshotChanged: () => void; + onReconciled: () => void; +}) { + const { db, githubService, logger, projectId } = args; + const reconcileInFlight = new Map>(); + const reconcileDirty = new Set(); + const repositoryReconcileInFlight = new Map>(); + const repoMutationTails = new Map>(); + + const withRepoMutationLock = async ( + repo: GitHubRepoRef, + mutation: () => Promise, + ): Promise => { + const key = repoKey(repo); + const previous = repoMutationTails.get(key) ?? Promise.resolve(); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const tail = previous.catch(() => undefined).then(() => gate); + repoMutationTails.set(key, tail); + await previous.catch(() => undefined); + try { + return await mutation(); + } finally { + release(); + if (repoMutationTails.get(key) === tail) repoMutationTails.delete(key); + } + }; + + const parseMembership = (raw: unknown): GitHubPrStackMembership | null => { + if (!isRecord(raw)) return null; + const base = isRecord(raw.base) ? raw.base : {}; + const id = String(raw.id ?? "").trim(); + const number = asNumber(raw.number); + const size = asNumber(raw.size); + const position = asNumber(raw.position); + const baseBranch = asString(base.ref).trim(); + if (!id || number <= 0 || size <= 0 || position <= 0 || !baseBranch) return null; + return { + id, + number, + size, + position, + baseBranch, + }; + }; + + const list = (repo?: GitHubRepoRef | null): GitHubPrStack[] => { + const params: Array = [projectId]; + const repoWhere = repo + ? "and lower(repo_owner) = lower(?) and lower(repo_name) = lower(?)" + : ""; + if (repo) params.push(repo.owner, repo.name); + const rows = db.all( + `select * + from github_pr_stacks + where project_id = ? + ${repoWhere} + order by github_stack_number desc`, + params, + ); + if (rows.length === 0) return []; + const entries = db.all( + `select * + from github_pr_stack_entries + where project_id = ? + ${repoWhere} + order by github_stack_number desc, position asc`, + params, + ); + const entriesByStack = new Map(); + for (const entry of entries) { + const key = stackKey(entry.repo_owner, entry.repo_name, Number(entry.github_stack_number)); + const stackEntries = entriesByStack.get(key) ?? []; + stackEntries.push(entry); + entriesByStack.set(key, stackEntries); + } + return rows.map((row) => stackFromRows( + row, + entriesByStack.get(stackKey(row.repo_owner, row.repo_name, Number(row.github_stack_number))) ?? [], + )); + }; + + const membershipsByPr = (repo?: GitHubRepoRef | null): Map => { + const memberships = new Map(); + for (const stack of list(repo)) { + const size = stack.entries.length; + for (const entry of stack.entries) { + memberships.set( + repoPrKey(stack.repoOwner, stack.repoName, entry.githubPrNumber), + { + id: stack.id, + number: stack.number, + size, + position: entry.position, + baseBranch: stack.baseBranch, + }, + ); + } + } + return memberships; + }; + + const decode = (repo: GitHubRepoRef, rawStack: unknown): DecodedGithubStack => { + if (!isRecord(rawStack)) throw new Error("GitHub returned an invalid stack."); + const stackNumber = asNumber(rawStack.number); + const stackId = String(rawStack.id ?? "").trim(); + const base = isRecord(rawStack.base) ? rawStack.base : {}; + const baseBranch = asString(base.ref).trim(); + const rawEntries = Array.isArray(rawStack.pull_requests) ? rawStack.pull_requests : []; + if (stackNumber <= 0 || !stackId || !baseBranch) { + throw new Error("GitHub returned incomplete stack identity."); + } + const syncedAt = nowIso(); + const createdAt = asString(rawStack.created_at).trim() || syncedAt; + const seenPrNumbers = new Set(); + const entries: GitHubPrStackEntryRow[] = rawEntries.map((rawEntry, index) => { + const entry = isRecord(rawEntry) ? rawEntry : {}; + const head = isRecord(entry.head) ? entry.head : {}; + const githubPrNumber = asNumber(entry.number); + const state = asString(entry.state).trim().toLowerCase(); + const headBranch = asString(head.ref).trim(); + const headSha = asString(head.sha).trim(); + if (githubPrNumber <= 0 || seenPrNumbers.has(githubPrNumber)) { + throw new Error(`GitHub stack #${stackNumber} contains an invalid pull request entry.`); + } + if ((state !== "open" && state !== "closed") || !headBranch || !headSha) { + throw new Error(`GitHub stack #${stackNumber} contains an incomplete pull request entry.`); + } + seenPrNumbers.add(githubPrNumber); + return { + project_id: projectId, + repo_owner: repo.owner, + repo_name: repo.name, + github_stack_number: stackNumber, + github_pr_number: githubPrNumber, + position: index + 1, + state, + is_draft: entry.draft === true ? 1 : 0, + merged_at: asString(entry.merged_at).trim() || null, + head_branch: headBranch, + head_sha: headSha, + }; + }); + return { + row: { + project_id: projectId, + repo_owner: repo.owner, + repo_name: repo.name, + github_stack_number: stackNumber, + github_stack_id: stackId, + github_node_id: asString(rawStack.node_id).trim() || null, + base_branch: baseBranch, + is_open: rawStack.open === false ? 0 : 1, + created_at: createdAt, + synced_at: syncedAt, + last_error: null, + }, + entries, + }; + }; + + const writeDecoded = (stack: DecodedGithubStack): void => { + const { row, entries } = stack; + db.run( + `insert into github_pr_stacks( + project_id, repo_owner, repo_name, github_stack_number, + github_stack_id, github_node_id, base_branch, is_open, + created_at, synced_at, last_error + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, null) + on conflict(project_id, repo_owner, repo_name, github_stack_number) do update set + github_stack_id = excluded.github_stack_id, + github_node_id = excluded.github_node_id, + base_branch = excluded.base_branch, + is_open = excluded.is_open, + created_at = excluded.created_at, + synced_at = excluded.synced_at, + last_error = null`, + [ + row.project_id, + row.repo_owner, + row.repo_name, + row.github_stack_number, + row.github_stack_id, + row.github_node_id, + row.base_branch, + row.is_open, + row.created_at, + row.synced_at, + ], + ); + db.run( + `delete from github_pr_stack_entries + where project_id = ? + and lower(repo_owner) = lower(?) + and lower(repo_name) = lower(?) + and github_stack_number = ?`, + [projectId, row.repo_owner, row.repo_name, row.github_stack_number], + ); + for (const entry of entries) { + db.run( + `delete from github_pr_stack_entries + where project_id = ? + and lower(repo_owner) = lower(?) + and lower(repo_name) = lower(?) + and github_pr_number = ? + and github_stack_number <> ?`, + [ + projectId, + entry.repo_owner, + entry.repo_name, + entry.github_pr_number, + entry.github_stack_number, + ], + ); + db.run( + `insert into github_pr_stack_entries( + project_id, repo_owner, repo_name, github_stack_number, + github_pr_number, position, state, is_draft, merged_at, + head_branch, head_sha + ) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + [ + entry.project_id, + entry.repo_owner, + entry.repo_name, + entry.github_stack_number, + entry.github_pr_number, + entry.position, + entry.state, + entry.is_draft, + entry.merged_at, + entry.head_branch, + entry.head_sha, + ], + ); + } + }; + + const replace = (repo: GitHubRepoRef, rawStack: unknown): GitHubPrStack => { + const decoded = decode(repo, rawStack); + db.run("begin immediate"); + try { + writeDecoded(decoded); + db.run("commit"); + } catch (error) { + try { + db.run("rollback"); + } catch { + // Preserve the original persistence error. + } + throw error; + } + args.onSnapshotChanged(); + return stackFromRows(decoded.row, decoded.entries); + }; + + const reconcile = async ( + repo: GitHubRepoRef, + stackNumber: number, + ): Promise => { + const key = `${repoKey(repo)}#${stackNumber}`; + const existing = reconcileInFlight.get(key); + if (existing) { + reconcileDirty.add(key); + return existing.then(async (stack) => { + if (!reconcileDirty.delete(key)) return stack; + return await reconcile(repo, stackNumber); + }); + } + let request!: Promise; + request = withRepoMutationLock(repo, async () => { + const { data } = await githubService.apiRequest({ + method: "GET", + path: `/repos/${repo.owner}/${repo.name}/stacks/${stackNumber}`, + }); + return replace(repo, data); + }) + .catch((error) => { + db.run( + `update github_pr_stacks + set last_error = ?, synced_at = ? + where project_id = ? + and lower(repo_owner) = lower(?) + and lower(repo_name) = lower(?) + and github_stack_number = ?`, + [getErrorMessage(error), nowIso(), projectId, repo.owner, repo.name, stackNumber], + ); + throw error; + }) + .finally(() => { + if (reconcileInFlight.get(key) === request) reconcileInFlight.delete(key); + }); + reconcileInFlight.set(key, request); + return request; + }; + + const reconcileRepository = async (repo: GitHubRepoRef): Promise => { + const key = repoKey(repo); + const existing = repositoryReconcileInFlight.get(key); + if (existing) return existing; + let request!: Promise; + request = withRepoMutationLock(repo, async () => { + const rawStacks: unknown[] = []; + let page = 1; + while (true) { + const { data, linkHeader } = await githubService.apiRequest({ + method: "GET", + path: `/repos/${repo.owner}/${repo.name}/stacks`, + query: { per_page: 100, page }, + }); + rawStacks.push(...(Array.isArray(data) ? data : [])); + if (!githubService.parseNextLink(linkHeader ?? null)) break; + page += 1; + } + const decodedStacks = rawStacks.map((rawStack) => decode(repo, rawStack)); + const seenPrNumbers = new Set(); + for (const stack of decodedStacks) { + for (const entry of stack.entries) { + if (seenPrNumbers.has(entry.github_pr_number)) { + throw new Error( + `GitHub returned pull request #${entry.github_pr_number} in more than one stack.`, + ); + } + seenPrNumbers.add(entry.github_pr_number); + } + } + db.run("begin immediate"); + try { + db.run( + `delete from github_pr_stacks + where project_id = ? + and lower(repo_owner) = lower(?) + and lower(repo_name) = lower(?)`, + [projectId, repo.owner, repo.name], + ); + for (const stack of decodedStacks) writeDecoded(stack); + db.run("commit"); + } catch (error) { + try { + db.run("rollback"); + } catch { + // Preserve the original persistence error. + } + throw error; + } + args.onSnapshotChanged(); + return decodedStacks.map((stack) => stackFromRows(stack.row, stack.entries)); + }).finally(() => { + if (repositoryReconcileInFlight.get(key) === request) { + repositoryReconcileInFlight.delete(key); + } + }); + repositoryReconcileInFlight.set(key, request); + return request; + }; + + const scheduleReconcile = (repo: GitHubRepoRef, stackNumber: number): void => { + void reconcile(repo, stackNumber).then(() => { + args.onReconciled(); + }).catch((error) => { + logger.warn("prs.github_stack_reconcile_failed", { + repo: `${repo.owner}/${repo.name}`, + stackNumber, + error: getErrorMessage(error), + }); + }); + }; + + const knownStackNumberForPr = ( + repo: GitHubRepoRef, + githubPrNumber: number, + ): number | null => { + const row = db.get<{ github_stack_number: number }>( + `select github_stack_number + from github_pr_stack_entries + where project_id = ? + and lower(repo_owner) = lower(?) + and lower(repo_name) = lower(?) + and github_pr_number = ? + limit 1`, + [projectId, repo.owner, repo.name, githubPrNumber], + ); + return row ? Number(row.github_stack_number) : null; + }; + + return { + knownStackNumberForPr, + list, + membershipsByPr, + parseMembership, + reconcile, + reconcileRepository, + scheduleReconcile, + }; +} diff --git a/apps/desktop/src/main/services/prs/prService.test.ts b/apps/desktop/src/main/services/prs/prService.test.ts index de7bf8149..682d5c8dd 100644 --- a/apps/desktop/src/main/services/prs/prService.test.ts +++ b/apps/desktop/src/main/services/prs/prService.test.ts @@ -217,6 +217,7 @@ function makeGithubService(overrides?: Record) { return { getRepoOrThrow: vi.fn(async () => REPO), apiRequest: vi.fn(), + parseNextLink: vi.fn(() => null), createSecretGist: vi.fn(), getStatus: vi.fn(), setToken: vi.fn(), @@ -1946,6 +1947,199 @@ describe("prService.ingestGithubWebhook", () => { })); }); + it("reconciles and transactionally replaces the whole GitHub stack after a stacked webhook", async () => { + const db = makeMockDb(); + const githubService = makeGithubService({ + apiRequest: vi.fn(async (args: { path: string }) => { + expect(args.path).toBe(`/repos/${REPO.owner}/${REPO.name}/stacks/18`); + return { + data: { + id: 5018, + number: 18, + node_id: "STACK_node18", + base: { ref: "main" }, + open: true, + created_at: "2026-07-30T10:00:00Z", + pull_requests: [ + { + number: 90, + state: "open", + draft: false, + merged_at: null, + head: { ref: "stack/core", sha: "sha-core" }, + }, + { + number: 91, + state: "open", + draft: true, + merged_at: null, + head: { ref: "stack/ui", sha: "sha-ui" }, + }, + ], + }, + }; + }), + }); + const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); + + const result = await service.ingestGithubWebhook({ + eventName: "pull_request", + deliveryId: "delivery-stacked", + payload: { + action: "stacked", + repository: { + full_name: `${REPO.owner}/${REPO.name}`, + owner: { login: REPO.owner }, + name: REPO.name, + }, + stack: { + id: 5018, + number: 18, + size: 2, + position: 1, + base: { ref: "main", sha: "sha-main" }, + }, + pull_request: makeGitHubPull({ + number: 90, + stack: { + id: 5018, + number: 18, + size: 2, + position: 1, + base: { ref: "main", sha: "sha-main" }, + }, + }), + }, + }); + await flushMicrotasks(); + + expect(result).toEqual(expect.objectContaining({ + processed: true, + githubPrNumber: 90, + })); + expect(githubService.apiRequest).toHaveBeenCalledTimes(1); + expect(db.run).toHaveBeenCalledWith("begin immediate"); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("insert into github_pr_stacks"), + expect.arrayContaining(["proj-1", REPO.owner, REPO.name, 18, "5018", "STACK_node18", "main"]), + ); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("delete from github_pr_stack_entries"), + ["proj-1", REPO.owner, REPO.name, 18], + ); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("github_pr_number = ?"), + ["proj-1", REPO.owner, REPO.name, 91, 18], + ); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("insert into github_pr_stack_entries"), + expect.arrayContaining(["proj-1", REPO.owner, REPO.name, 18, 91, 2, "open", 1]), + ); + expect(db.run).toHaveBeenCalledWith("commit"); + }); + + it("reconciles a previously known stack when a later PR webhook omits stack metadata", async () => { + const db = makeMockDb(); + db.get.mockImplementation((sql: string) => { + if (String(sql).includes("from github_pr_stack_entries")) { + return { github_stack_number: 18 }; + } + return null; + }); + const githubService = makeGithubService({ + apiRequest: vi.fn(async () => ({ + data: { + id: 5018, + number: 18, + base: { ref: "main" }, + open: false, + created_at: "2026-07-30T10:00:00Z", + pull_requests: [ + { + number: 90, + state: "closed", + draft: false, + merged_at: "2026-07-30T12:00:00Z", + head: { ref: "stack/core", sha: "sha-core" }, + }, + ], + }, + })), + }); + const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); + + await service.ingestGithubWebhook({ + eventName: "pull_request", + deliveryId: "delivery-merged", + payload: { + action: "closed", + repository: { + full_name: `${REPO.owner}/${REPO.name}`, + owner: { login: REPO.owner }, + name: REPO.name, + }, + pull_request: makeGitHubPull({ + number: 90, + state: "closed", + merged_at: "2026-07-30T12:00:00Z", + }), + }, + }); + await flushMicrotasks(); + + expect(githubService.apiRequest).toHaveBeenCalledWith({ + method: "GET", + path: `/repos/${REPO.owner}/${REPO.name}/stacks/18`, + }); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("insert into github_pr_stacks"), + expect.arrayContaining([18, "5018", "main", 0]), + ); + }); + + it("falls back to the repository list when a known stack has dissolved", async () => { + const db = makeMockDb(); + db.get.mockImplementation((sql: string) => { + if (String(sql).includes("from github_pr_stack_entries")) { + return { github_stack_number: 18 }; + } + return null; + }); + const githubService = makeGithubService({ + apiRequest: vi.fn() + .mockRejectedValueOnce(new Error("Not Found")) + .mockResolvedValueOnce({ data: [], linkHeader: null }), + }); + const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); + + const result = await service.ingestGithubWebhook({ + eventName: "pull_request", + deliveryId: "delivery-unstacked", + payload: { + action: "synchronize", + repository: { + full_name: `${REPO.owner}/${REPO.name}`, + owner: { login: REPO.owner }, + name: REPO.name, + }, + pull_request: makeGitHubPull({ number: 90, stack: null }), + }, + }); + + expect(result.processed).toBe(true); + expect(githubService.apiRequest).toHaveBeenNthCalledWith(2, { + method: "GET", + path: `/repos/${REPO.owner}/${REPO.name}/stacks`, + query: { per_page: 100, page: 1 }, + }); + expect(db.run).toHaveBeenCalledWith("begin immediate"); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("delete from github_pr_stacks"), + ["proj-1", REPO.owner, REPO.name], + ); + expect(db.run).toHaveBeenCalledWith("commit"); + }); + it("emits a PR update when an unmapped pull request changes its projection", async () => { const db = makeMockDb(); const { service } = buildService({ db, laneService: makeLaneService([]) }); @@ -2322,6 +2516,89 @@ describe("prService.getStatus", () => { })); }); + it("uses the ultimate stack base for required approvals", async () => { + const row = makePrRow({ id: "pr-stacked-mergebox", github_pr_number: 96 }); + const db = makeMockDb(); + installPullRequestRowStore(db, [row]); + let graphqlCalls = 0; + const githubService = makeGithubService({ + apiRequest: vi.fn(async (args: { method?: string; path: string; body?: unknown }) => { + if (args.path === "/repos/test-owner/test-repo/pulls/96") { + return { + data: makeGitHubPull({ + number: 96, + html_url: row.github_url, + title: row.title, + mergeable: true, + mergeable_state: "clean", + head: { ref: "stack/ui", sha: "head-96" }, + base: { ref: "stack/core", sha: "base-96" }, + }), + }; + } + if (args.path === "/repos/test-owner/test-repo/commits/head-96/status") { + return { data: { state: "success", statuses: [] } }; + } + if (args.path === "/repos/test-owner/test-repo/commits/head-96/check-runs") { + return { data: { check_runs: [] } }; + } + if (args.path === "/repos/test-owner/test-repo/pulls/96/reviews") { + return { data: [] }; + } + if (args.path === "/repos/test-owner/test-repo/compare/base-96...head-96") { + return { data: { behind_by: 0 } }; + } + if (args.method === "POST" && args.path === "/graphql") { + graphqlCalls += 1; + if (graphqlCalls === 1) { + return { + data: { + data: { + repository: { + viewerPermission: "WRITE", + pullRequest: { + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: null, + headRefOid: "head-96", + baseRefName: "stack/core", + baseRef: { branchProtectionRule: { requiredApprovingReviewCount: 0 } }, + stack: { baseRefName: "main" }, + latestOpinionatedReviews: { nodes: [] }, + }, + }, + }, + }, + }; + } + expect(args.body).toEqual(expect.objectContaining({ + variables: expect.objectContaining({ + qualifiedName: "refs/heads/main", + }), + })); + return { + data: { + data: { + repository: { + ref: { + branchProtectionRule: { requiredApprovingReviewCount: 2 }, + }, + }, + }, + }, + }; + } + throw new Error(`Unexpected GitHub API path: ${args.path}`); + }), + }); + const { service } = buildService({ db, githubService }); + + const status = await service.getStatus("pr-stacked-mergebox"); + + expect(graphqlCalls).toBe(2); + expect(status.requiredApprovals).toBe(2); + }); + it("keeps behindBaseBy unknown when GitHub compare fails", async () => { const row = makePrRow({ id: "pr-status-compare-failed", github_pr_number: 91 }); const db = makeMockDb(); diff --git a/apps/desktop/src/main/services/prs/prService.ts b/apps/desktop/src/main/services/prs/prService.ts index d259d0214..d023bbfc2 100644 --- a/apps/desktop/src/main/services/prs/prService.ts +++ b/apps/desktop/src/main/services/prs/prService.ts @@ -109,6 +109,8 @@ import type { AiReviewSummaryArgs, AiReviewSummary, GitHubPrListItem, + GitHubPrStack, + GitHubPrStackMembership, GitHubPrSnapshot, GitHubWebhookIngestArgs, GitHubWebhookIngestResult, @@ -163,6 +165,7 @@ import { spawn } from "node:child_process"; import { runGit, runGitMergeTree, runGitOrThrow } from "../git/git"; import { shouldAttemptAdminMergeForRestError } from "./resolverUtils"; import { deletePullRequestRowsByIds } from "./pullRequestRowCleanup"; +import { createGithubStackStore } from "./githubStackStore"; import { extractFirstJsonObject } from "../ai/utils"; import { buildIntegrationPreflight } from "./integrationPlanning"; import { createWorkflowGraph, type WorkflowFileSource } from "./workflowGraph"; @@ -4772,8 +4775,9 @@ export function createPrService({ repository(owner:$owner,name:$name){ viewerPermission pullRequest(number:$number){ - mergeable mergeStateStatus reviewDecision headRefOid + mergeable mergeStateStatus reviewDecision headRefOid baseRefName baseRef { branchProtectionRule { requiredApprovingReviewCount } } + stack { baseRefName } latestOpinionatedReviews(first:100){ nodes { state } } } } @@ -4787,7 +4791,9 @@ export function createPrService({ mergeStateStatus?: unknown; reviewDecision?: unknown; headRefOid?: unknown; + baseRefName?: unknown; baseRef?: { branchProtectionRule?: { requiredApprovingReviewCount?: unknown } | null } | null; + stack?: { baseRefName?: unknown } | null; latestOpinionatedReviews?: { nodes?: Array<{ state?: unknown } | null> | null } | null; } | null; } | null; @@ -4803,8 +4809,43 @@ export function createPrService({ const pull = repository?.pullRequest ?? null; if (!pull) return null; - const requiredRaw = pull.baseRef?.branchProtectionRule?.requiredApprovingReviewCount; - const requiredApprovals = Number.isFinite(Number(requiredRaw)) ? Number(requiredRaw) : null; + const directBase = asString(pull.baseRefName).trim(); + const stackBase = asString(pull.stack?.baseRefName).trim(); + let requiredRaw = pull.baseRef?.branchProtectionRule?.requiredApprovingReviewCount; + if (stackBase && stackBase !== directBase) { + try { + const stackBaseData = await graphqlRequest<{ + repository?: { + ref?: { branchProtectionRule?: { requiredApprovingReviewCount?: unknown } | null } | null; + } | null; + }>( + `query($owner:String!,$name:String!,$qualifiedName:String!){ + repository(owner:$owner,name:$name){ + ref(qualifiedName:$qualifiedName){ + branchProtectionRule { requiredApprovingReviewCount } + } + } +}`, + { + owner: repo.owner, + name: repo.name, + qualifiedName: `refs/heads/${stackBase}`, + }, + ); + requiredRaw = stackBaseData.repository?.ref?.branchProtectionRule + ?.requiredApprovingReviewCount; + } catch (error) { + requiredRaw = null; + logger.warn("prs.computeStatus.stack_base_protection_failed", { + repo: `${repo.owner}/${repo.name}`, + prNumber, + error: getErrorMessage(error), + }); + } + } + const requiredApprovals = requiredRaw != null && Number.isFinite(Number(requiredRaw)) + ? Number(requiredRaw) + : null; const reviewNodes = Array.isArray(pull.latestOpinionatedReviews?.nodes) ? pull.latestOpinionatedReviews!.nodes! : []; @@ -8539,7 +8580,10 @@ export function createPrService({ includeStateCounts = false, ): void => { if (snapshot.repo) activeGithubRepo = snapshot.repo; - cachedGithubSnapshot = snapshot; + cachedGithubSnapshot = { + ...snapshot, + stacks: snapshot.repo ? githubStackStore.list(snapshot.repo) : [], + }; cachedGithubSnapshotAt = capturedAt; cachedGithubSnapshotIncludesClosed = includeExternalClosed; cachedGithubSnapshotHistoryPageLimit = includeExternalClosed ? historyPageLimit : 0; @@ -8586,6 +8630,7 @@ export function createPrService({ linkedPrByRepoKey: Map; groupByPrId: Map; workflowByPrId: Map; + stackByPrKey: Map; }; const loadGithubSnapshotMetadata = async (): Promise => { @@ -8617,6 +8662,7 @@ export function createPrService({ linkedPrByRepoKey, groupByPrId, workflowByPrId, + stackByPrKey: githubStackStore.membershipsByPr(activeGithubRepo), }; }; @@ -8632,6 +8678,15 @@ export function createPrService({ return null; }; + const githubStackStore = createGithubStackStore({ + db, + projectId, + githubService, + logger, + onSnapshotChanged: invalidateGithubSnapshotCache, + onReconciled: () => emitPrsUpdated(), + }); + const gitHubItemFromProjection = ( row: GitHubPrProjectionRow, metadata: GithubSnapshotMetadata, @@ -8673,6 +8728,7 @@ export function createPrService({ labels: parseProjectionLabels(row.labels_json), isBot: Number(row.is_bot ?? 0) !== 0, commentCount: Number(row.comment_count ?? 0), + stack: metadata.stackByPrKey.get(repoPrKey(row.repo_owner, row.repo_name, githubPrNumber)) ?? null, }; }; @@ -8725,6 +8781,9 @@ export function createPrService({ : [], isBot: asString(rawPr?.user?.type).toLowerCase() === "bot", commentCount: Number(rawPr?.comments) || 0, + stack: githubStackStore.parseMembership(rawPr?.stack) + ?? metadata?.stackByPrKey.get(repoPrKey(repoOwner, repoName, githubPrNumber)) + ?? null, }; }; @@ -8805,6 +8864,14 @@ export function createPrService({ }; } + if (options.force === true && githubStackStore.list(repo).length > 0) { + await githubStackStore.reconcileRepository(repo).catch((error) => { + logger.warn("prs.github_stack_repository_reconcile_failed", { + repo: `${repo.owner}/${repo.name}`, + error: getErrorMessage(error), + }); + }); + } let metadata = await loadGithubSnapshotMetadata(); const historyPageLimit = normalizeGithubHistoryPageLimit(options); const repoPullRequestMaxPages = options.includeExternalClosed === true @@ -8835,6 +8902,14 @@ export function createPrService({ metadata.pullRequestRows, { skipBranchesWithLocalRows: options.includeExternalClosed !== true }, ); + const observedStackNumbers = new Set(); + for (const rawPull of repoPullRequestsRaw) { + const membership = githubStackStore.parseMembership(rawPull?.stack); + if (membership) observedStackNumbers.add(membership.number); + } + for (const stackNumber of observedStackNumbers) { + githubStackStore.scheduleReconcile(repo, stackNumber); + } upsertGithubPrProjectionsFromRawPulls(repo, repoPullRequestsRaw); // Trigger #2: strict same-repo branch auto-map (emits an Undo-able toast) // runs first so the user-facing path takes precedence. Best-effort — never @@ -8922,7 +8997,7 @@ export function createPrService({ hasExactStateCounts, ); } - return snapshot; + return canPublishSnapshot ? cachedGithubSnapshot ?? snapshot : snapshot; }) .finally(() => { if (githubSnapshotInFlight === inFlight) { @@ -8995,7 +9070,7 @@ export function createPrService({ }); }); } - return projectedSnapshot; + return cachedGithubSnapshot ?? projectedSnapshot; } } const compatibleInFlight = githubSnapshotInFlight; @@ -9199,6 +9274,21 @@ export function createPrService({ pruneGithubPrProjectionsForRepo({ owner: projection.repo_owner, name: projection.repo_name }); linkedPrIds = applyProjectionToLinkedPrRows(projection); const repo = { owner: projection.repo_owner, name: projection.repo_name }; + const webhookMembership = githubStackStore.parseMembership( + isRecord(rawPull?.stack) ? rawPull.stack : payload.stack, + ); + const stackNumber = webhookMembership?.number + ?? githubStackStore.knownStackNumberForPr(repo, Number(projection.github_pr_number)); + if (stackNumber) { + try { + await githubStackStore.reconcile(repo, stackNumber); + } catch { + // A dissolved stack returns 404 from the item endpoint. The + // authoritative repository list distinguishes that from a + // transient failure and removes stale local membership atomically. + await githubStackStore.reconcileRepository(repo); + } + } if (linkedPrIds.length === 0) { const lanes = await laneService.list({ includeArchived: true, includeStatus: false }); await autoMapRawPullsByBranch([rawPull], repo, lanes) @@ -10573,6 +10663,18 @@ export function createPrService({ return await getGithubSnapshot(options); }, + listGithubStacks(repo?: GitHubRepoRef | null): GitHubPrStack[] { + return githubStackStore.list(repo); + }, + + async reconcileGithubStack(repo: GitHubRepoRef, stackNumber: number): Promise { + return await githubStackStore.reconcile(repo, stackNumber); + }, + + async reconcileGithubStacks(repo: GitHubRepoRef): Promise { + return await githubStackStore.reconcileRepository(repo); + }, + async ingestGithubWebhook(args: GitHubWebhookIngestArgs): Promise { return await ingestGithubWebhook(args); }, diff --git a/apps/desktop/src/main/services/state/kvDb.test.ts b/apps/desktop/src/main/services/state/kvDb.test.ts index 63e51daee..d441e8cba 100644 --- a/apps/desktop/src/main/services/state/kvDb.test.ts +++ b/apps/desktop/src/main/services/state/kvDb.test.ts @@ -262,6 +262,53 @@ describe("openKvDb SQL binding", () => { }); }); +describe("GitHub stacked pull request schema", () => { + it("stores repository stacks and their ordered pull requests locally", async () => { + const projectRoot = makeProjectRoot("ade-kvdb-github-stacks-"); + const dbPath = path.join(projectRoot, ".ade", "ade.db"); + const db = await openKvDb(dbPath, createLogger() as any); + activeDisposers.push(async () => db.close()); + + expect( + db.all<{ name: string }>("pragma table_info('github_pr_stacks')").map((row) => row.name), + ).toEqual(expect.arrayContaining([ + "project_id", + "repo_owner", + "repo_name", + "github_stack_number", + "github_stack_id", + "base_branch", + "is_open", + "synced_at", + "last_error", + ])); + expect( + db.all<{ name: string }>("pragma table_info('github_pr_stack_entries')").map((row) => row.name), + ).toEqual(expect.arrayContaining([ + "project_id", + "repo_owner", + "repo_name", + "github_stack_number", + "github_pr_number", + "position", + "state", + "is_draft", + "merged_at", + "head_branch", + "head_sha", + ])); + expect( + db.all<{ name: string; unique: number }>("pragma index_list('github_pr_stack_entries')") + .find((index) => index.name === "idx_github_pr_stack_entries_membership"), + ).toEqual(expect.objectContaining({ unique: 1 })); + expect( + db.all<{ table: string; on_delete: string }>("pragma foreign_key_list('github_pr_stack_entries')") + .some((foreignKey) => + foreignKey.table === "github_pr_stacks" && foreignKey.on_delete.toLowerCase() === "cascade"), + ).toBe(true); + }); +}); + describe("lane_linear_issue_links schema", () => { it("does not keep a non-PK unique index that blocks crsql_as_crr", async () => { const projectRoot = makeProjectRoot("ade-kvdb-linear-issue-links-index-"); diff --git a/apps/desktop/src/main/services/state/kvDb.ts b/apps/desktop/src/main/services/state/kvDb.ts index 366d86ece..f6886d970 100644 --- a/apps/desktop/src/main/services/state/kvDb.ts +++ b/apps/desktop/src/main/services/state/kvDb.ts @@ -769,10 +769,11 @@ const LOCAL_ONLY_CRR_EXCLUDED_TABLES = new Set([ // older client hard-rejects, freezing its sync cursor entirely. LOCAL_CRR_CHANGE_SUPPRESSIONS_TABLE, "github_pr_projections", + "github_pr_stacks", + "github_pr_stack_entries", "github_webhook_deliveries", "lane_detail_snapshots", "lane_list_snapshots", - LOCAL_CRR_CHANGE_SUPPRESSIONS_TABLE, "pr_auto_link_ignores", // Config snapshots rebuilt from ade.yaml are local-derived state. Remote // clients read effective config through RPC, never from a synced replica, @@ -2498,6 +2499,51 @@ function migrate(db: MigrationDb, rawDb: DatabaseSyncType) { db.run("create index if not exists idx_github_pr_projections_project_updated on github_pr_projections(project_id, updated_at desc)"); db.run("create index if not exists idx_github_pr_projections_project_repo on github_pr_projections(project_id, repo_owner, repo_name)"); + db.run(` + create table if not exists github_pr_stacks ( + project_id text not null, + repo_owner text collate nocase not null, + repo_name text collate nocase not null, + github_stack_number integer not null, + github_stack_id text not null, + github_node_id text, + base_branch text not null, + is_open integer not null default 1, + created_at text not null, + synced_at text not null, + last_error text, + primary key(project_id, repo_owner, repo_name, github_stack_number), + foreign key(project_id) references projects(id) + ) + `); + db.run("create index if not exists idx_github_pr_stacks_project_repo on github_pr_stacks(project_id, repo_owner, repo_name)"); + db.run(` + create table if not exists github_pr_stack_entries ( + project_id text not null, + repo_owner text collate nocase not null, + repo_name text collate nocase not null, + github_stack_number integer not null, + github_pr_number integer not null, + position integer not null, + state text not null, + is_draft integer not null default 0, + merged_at text, + head_branch text not null, + head_sha text not null, + primary key(project_id, repo_owner, repo_name, github_stack_number, github_pr_number), + foreign key(project_id) references projects(id), + foreign key(project_id, repo_owner, repo_name, github_stack_number) + references github_pr_stacks(project_id, repo_owner, repo_name, github_stack_number) + on delete cascade + ) + `); + db.run(` + create unique index if not exists idx_github_pr_stack_entries_membership + on github_pr_stack_entries(project_id, repo_owner, repo_name, github_pr_number) + `); + db.run("create index if not exists idx_github_pr_stack_entries_pr on github_pr_stack_entries(project_id, repo_owner, repo_name, github_pr_number)"); + db.run("create index if not exists idx_github_pr_stack_entries_position on github_pr_stack_entries(project_id, repo_owner, repo_name, github_stack_number, position)"); + db.run(` create table if not exists pr_auto_link_ignores ( project_id text not null, diff --git a/apps/desktop/src/shared/types/prs.ts b/apps/desktop/src/shared/types/prs.ts index 7406e31e2..b18a457c3 100644 --- a/apps/desktop/src/shared/types/prs.ts +++ b/apps/desktop/src/shared/types/prs.ts @@ -193,6 +193,41 @@ export type PrReviewThread = { comments: PrReviewThreadComment[]; }; +export type GitHubPrStackMembership = { + /** Global GitHub stack identifier. */ + id: string; + /** Repository-scoped stack number shown by GitHub. */ + number: number; + size: number; + /** One-based position, where 1 is closest to the stack base. */ + position: number; + baseBranch: string; +}; + +export type GitHubPrStackEntry = { + githubPrNumber: number; + position: number; + state: "open" | "closed"; + isDraft: boolean; + mergedAt: string | null; + headBranch: string; + headSha: string; +}; + +export type GitHubPrStack = { + id: string; + number: number; + nodeId: string | null; + repoOwner: string; + repoName: string; + baseBranch: string; + open: boolean; + createdAt: string; + syncedAt: string; + lastError: string | null; + entries: GitHubPrStackEntry[]; +}; + export type GitHubPrListItem = { id: string; scope: "repo" | "external"; @@ -220,6 +255,8 @@ export type GitHubPrListItem = { labels: PrLabel[]; isBot: boolean; commentCount: number; + /** Additive for compatibility with older runtime snapshots. */ + stack?: GitHubPrStackMembership | null; }; export type GitHubPrSnapshot = { @@ -228,6 +265,8 @@ export type GitHubPrSnapshot = { repoPullRequests: GitHubPrListItem[]; externalPullRequests: GitHubPrListItem[]; syncedAt: string; + /** Complete authoritative GitHub stack snapshots for this repository. */ + stacks?: GitHubPrStack[]; history?: { includeExternalClosed: boolean; pageLimit: number; diff --git a/apps/webhook-relay/src/relay.ts b/apps/webhook-relay/src/relay.ts index ca7391238..be3ce2c0f 100644 --- a/apps/webhook-relay/src/relay.ts +++ b/apps/webhook-relay/src/relay.ts @@ -1,5 +1,7 @@ import { createRemoteJWKSet, decodeJwt, jwtVerify, type JWTPayload } from "jose"; +const GITHUB_REST_API_VERSION = "2026-03-10"; + export type RelayEnv = { DB: D1Database; /** One hibernating WebSocket fanout object per lowercased owner/repo. */ @@ -778,7 +780,7 @@ async function fetchGitHubApiJson(apiBaseUrl: string, path: string, token: strin accept: "application/vnd.github+json", authorization: `Bearer ${token}`, "user-agent": "ADE GitHub Webhook Relay", - "x-github-api-version": "2022-11-28", + "x-github-api-version": GITHUB_REST_API_VERSION, }, }); const payload = (await response.json().catch(() => ({}))) as Record; @@ -1080,7 +1082,7 @@ async function fetchGitHubAppApiStatus( accept: "application/vnd.github+json", authorization: `Bearer ${jwt}`, "user-agent": "ADE GitHub Webhook Relay", - "x-github-api-version": "2022-11-28", + "x-github-api-version": GITHUB_REST_API_VERSION, }, }); if (response.status === 404) { @@ -1662,7 +1664,7 @@ async function handleWebhookHeal(request: Request, env: RelayEnv, repo: { owner: authorization: `Bearer ${appAuth.jwt}`, "content-type": "application/json", "user-agent": "ADE GitHub Webhook Relay", - "x-github-api-version": "2022-11-28", + "x-github-api-version": GITHUB_REST_API_VERSION, }, body: JSON.stringify({ secret: webhookSecret }), }); @@ -1703,7 +1705,7 @@ async function handleWebhookDeliveries(request: Request, env: RelayEnv, repo: { accept: "application/vnd.github+json", authorization: `Bearer ${appAuth.jwt}`, "user-agent": "ADE GitHub Webhook Relay", - "x-github-api-version": "2022-11-28", + "x-github-api-version": GITHUB_REST_API_VERSION, }, }); const payload = (await response.json().catch(() => null)) as unknown; diff --git a/apps/webhook-relay/test/relay.test.ts b/apps/webhook-relay/test/relay.test.ts index 7dde9eff9..3507976b3 100644 --- a/apps/webhook-relay/test/relay.test.ts +++ b/apps/webhook-relay/test/relay.test.ts @@ -283,6 +283,7 @@ describe("webhook relay", () => { expect(init?.headers).toEqual(expect.objectContaining({ authorization: `Bearer ${token}`, "user-agent": "ADE GitHub Webhook Relay", + "x-github-api-version": "2026-03-10", })); return new Response(JSON.stringify({ full_name: "owner/repo", permissions }), { status: 200, diff --git a/docs/features/pull-requests/github-stacked-prs.md b/docs/features/pull-requests/github-stacked-prs.md new file mode 100644 index 000000000..045eb0af8 --- /dev/null +++ b/docs/features/pull-requests/github-stacked-prs.md @@ -0,0 +1,195 @@ +# GitHub stacked pull requests + +GitHub is the authority for stacked pull request membership, ordering, review +requirements, rebases performed on GitHub, merge queue state, and merging. ADE +adds local lane and agent context, a fast operational view, native stack +creation and synchronization, and local conflict repair. + +Stacked pull requests are part of GitHub's public preview. ADE uses the +`2026-03-10` REST API contract and treats the repository-scoped GitHub stack +number as the remote identity. + +## Product boundaries + +- Stacks live in the existing PRs tab. There is no separate top-level Stacks + tab. +- GitHub remains the final review and merge surface. ADE opens that surface in + the built-in browser and does not recreate GitHub's merge box. +- ADE may create, extend, unstack, rebase, adopt, and locally repair stacks. +- The old ADE Queue PR landing engine, terminology, state, commands, UI, tests, + skills, and docs are removed rather than deprecated. +- Existing integration PR workflows remain independent from stacked PRs. +- Users do not need to install `gh-stack`; ADE uses GitHub's API directly. + +## Canonical model + +ADE persists complete GitHub stack snapshots rather than inferring remote stack +membership from lane parents or PR base branches. + +```text +GitHubPrStack + repo owner/name + global id + repository-scoped stack number + base branch + open/completed state + ordered entries (bottom → top) + fetched timestamp + last reconciliation error + +GitHubPrStackEntry + PR number + original position + open/closed/merged/draft state + head branch + SHA + merged timestamp +``` + +Lane topology remains ADE's local execution model. When lane topology and the +GitHub stack disagree, GitHub wins for remote membership and ADE presents the +divergence with an explicit adopt or restructure action. + +Whole stack snapshots are replaced transactionally. Webhooks are hints that +schedule an authoritative stack read; they are not independently applied as +membership mutations. + +## GitHub lifecycle + +- A normal `pull_request.opened` delivery may arrive before the PR joins a + stack. +- `pull_request.stacked` identifies the stack when a PR is added. +- Later `pull_request` deliveries include `pull_request.stack` while the PR + remains stacked. +- A webhook referencing a stack schedules one authoritative + `GET /repos/{owner}/{repo}/stacks/{number}`. +- A known member delivery with missing stack metadata schedules reconciliation + of its previously known stack. +- Cursor expiry schedules a bounded repository-wide stack reconciliation. +- Duplicate and out-of-order deliveries cannot overwrite a newer complete + snapshot with partial event data. +- Because GitHub does not document an `unstacked` webhook action, explicit + refresh and background polling reconcile removals and dissolved stacks. + +Partial merges preserve completed entries for history while the remaining open +entries receive their new bases and positions from GitHub. `merge_queued` is not +treated as merged. Fully completed stacks remain visible as history and cannot +be extended. + +## Operations + +ADE exposes the same workflow through desktop, hosted web, CLI actions, and +agent tools: + +```text +list/show status +plan layers +create from ordered lanes or PRs +add an eligible PR or lane to the top +adopt a remote stack into local lanes +sync local and remote state +request a clean GitHub rebase +resolve conflicts locally with ADE +unstack with a consequence preview +open the GitHub review/merge surface +``` + +Structural changes such as reorder, insert, fold, rename, or remove are +destructive remote/local operations. They require a clean worktree, linear +history, no queued member, a preview of the resulting chain, and explicit +confirmation. ADE snapshots the prior branch heads before applying changes. + +## PRs tab + +The normal GitHub PR list renders stack metadata from the cached aggregate +snapshot. Opening the tab never performs per-row stack requests. + +```text +#843 Desktop stack UI Stack 4 of 5 + feat/stack-ui → feat/stack-cli Checks pass · Review pending +``` + +- `Stacked` filters to stack members. +- `Group by stack` shows connected rails; ordinary sorting shows position + badges without implying row adjacency. +- Selecting the badge opens a compact stack map. +- The PR detail pane opens a full stack inspector with GitHub readiness plus + ADE lane, agent, worktree, validation, and conflict context. +- Copy uses concrete state: `Blocked by #842: one required check failed`, not + internal terms such as queue position or landing state. +- Empty, loading, stale, permission, unsupported-host, conflict, queued, + partially merged, and completed states have distinct copy and actions. + +The primary final action is `Review and merge on GitHub`. Mutating ADE actions +open the inspector and require confirmation. + +## Work card + +One stable `pr_stack` card follows an ADE stack plan from proposed layers through +published GitHub PRs and final merge. Its card id is based on the ADE stack plan +id and does not change when GitHub assigns a stack number. + +```text +GitHub stacked PR integration 3 of 5 ready +│ ✓ #840 API and persistence merged +│ ✓ #841 GitHub synchronization approved +│ ! #842 CLI and agent tools checks failed +│ ● #843 PRs UI agent working +│ ○ #844 Work card planned +└ main + +Blocked at #842 · one required check failed +``` + +Desktop renders a graphical rail, iOS reuses the native stack diagram, and the +TUI uses Unicode nodes and lines. The coordinator thread owns the complete card; +member sessions receive a compact layer card. External stacks do not create +chat cards until a user adopts them into an ADE work session. + +Card actions navigate to the stack inspector, GitHub review surface, or owning +agent. Rebase, restructure, and unstack never execute directly from transcript +history. + +## Agent behavior + +Bundled ADE skills teach agents to: + +1. Propose dependency-ordered, independently reviewable layers before coding. +2. Put foundations below their consumers. +3. Create and validate one deliberate branch layer at a time. +4. Delegate only work whose dependency boundary is already stable. +5. Apply feedback to the correct layer, then rebase every layer above it. +6. Review and report progress bottom-up. +7. Keep one stack card current instead of emitting repeated status messages. +8. Send final review and merge decisions to GitHub. + +## Delivery stack + +The implementation is itself delivered as stacked pull requests: + +1. Canonical GitHub stack types, persistence, REST reads, and webhook + reconciliation. +2. Stack mutations, ADE actions, typed CLI commands, and bundled agent skills. +3. Desktop and hosted-web PR list, grouping, routing, and inspector. +4. Work card protocol plus desktop and `ade code` rendering. +5. iOS PR and Work parity. +6. Remove Queue PR code, state, tests, tools, copy, and docs; finish analytics + and documentation. + +Each layer runs focused quality review, contract tests, type checks for touched +packages, and the normal PR ship loop before the stack is merged. + +## Required edge coverage + +- Opened then stacked webhook ordering. +- Duplicate, replayed, and out-of-order deliveries. +- Remote-only, partially adopted, inaccessible, or deleted members. +- Local lane order diverging from GitHub order. +- Partial merges and survivor retarget/rebase. +- Closed or draft members below an otherwise ready member. +- Down-stack checks, reviews, rules, and merge-queue blockers. +- Clean rebase running, diverged branches, merge conflicts, and failed rebase. +- Stale/rate-limited GitHub detail with a retained cached snapshot. +- Inspector open while the remote stack changes. +- Non-default bases, same-repository enforcement, fork rejection, and 100-entry + stacks. +- Completed and remotely unstacked stacks. +- Agent or lane removal while the GitHub member remains active. +- Older desktop, web, TUI, and iOS clients receiving additive card/snapshot + fields. From 588e2ed00cf1948f063f82a9420674c8bcc4f3eb Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:55:09 -0400 Subject: [PATCH 2/4] fix(prs): harden native stack reconciliation --- .../src/main/services/prs/githubStackStore.ts | 25 ++-- .../src/main/services/prs/prService.test.ts | 111 ++++++++++++++++++ .../src/main/services/prs/prService.ts | 74 ++++++++---- 3 files changed, 174 insertions(+), 36 deletions(-) diff --git a/apps/desktop/src/main/services/prs/githubStackStore.ts b/apps/desktop/src/main/services/prs/githubStackStore.ts index fb5a4a20f..4f720fc8d 100644 --- a/apps/desktop/src/main/services/prs/githubStackStore.ts +++ b/apps/desktop/src/main/services/prs/githubStackStore.ts @@ -362,13 +362,13 @@ export function createGithubStackStore(args: { } let request!: Promise; request = withRepoMutationLock(repo, async () => { - const { data } = await githubService.apiRequest({ - method: "GET", - path: `/repos/${repo.owner}/${repo.name}/stacks/${stackNumber}`, - }); - return replace(repo, data); - }) - .catch((error) => { + try { + const { data } = await githubService.apiRequest({ + method: "GET", + path: `/repos/${repo.owner}/${repo.name}/stacks/${stackNumber}`, + }); + return replace(repo, data); + } catch (error) { db.run( `update github_pr_stacks set last_error = ?, synced_at = ? @@ -379,7 +379,8 @@ export function createGithubStackStore(args: { [getErrorMessage(error), nowIso(), projectId, repo.owner, repo.name, stackNumber], ); throw error; - }) + } + }) .finally(() => { if (reconcileInFlight.get(key) === request) reconcileInFlight.delete(key); }); @@ -394,8 +395,8 @@ export function createGithubStackStore(args: { let request!: Promise; request = withRepoMutationLock(repo, async () => { const rawStacks: unknown[] = []; - let page = 1; - while (true) { + const maxPages = 100; + for (let page = 1; page <= maxPages; page += 1) { const { data, linkHeader } = await githubService.apiRequest({ method: "GET", path: `/repos/${repo.owner}/${repo.name}/stacks`, @@ -403,7 +404,9 @@ export function createGithubStackStore(args: { }); rawStacks.push(...(Array.isArray(data) ? data : [])); if (!githubService.parseNextLink(linkHeader ?? null)) break; - page += 1; + if (page === maxPages) { + throw new Error(`GitHub stack list exceeded the ${maxPages}-page safety limit.`); + } } const decodedStacks = rawStacks.map((rawStack) => decode(repo, rawStack)); const seenPrNumbers = new Set(); diff --git a/apps/desktop/src/main/services/prs/prService.test.ts b/apps/desktop/src/main/services/prs/prService.test.ts index 682d5c8dd..2b15db886 100644 --- a/apps/desktop/src/main/services/prs/prService.test.ts +++ b/apps/desktop/src/main/services/prs/prService.test.ts @@ -2140,6 +2140,43 @@ describe("prService.ingestGithubWebhook", () => { expect(db.run).toHaveBeenCalledWith("commit"); }); + it("keeps a webhook processed when both stack reconciliation reads fail", async () => { + const db = makeMockDb(); + db.get.mockImplementation((sql: string) => { + if (String(sql).includes("from github_pr_stack_entries")) { + return { github_stack_number: 18 }; + } + return null; + }); + const githubService = makeGithubService({ + apiRequest: vi.fn() + .mockRejectedValueOnce(new Error("Stack read timed out")) + .mockRejectedValueOnce(new Error("Repository stack list timed out")), + }); + const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); + + const result = await service.ingestGithubWebhook({ + eventName: "pull_request", + deliveryId: "delivery-stack-read-failed", + payload: { + action: "synchronize", + repository: { + full_name: `${REPO.owner}/${REPO.name}`, + owner: { login: REPO.owner }, + name: REPO.name, + }, + pull_request: makeGitHubPull({ number: 90, stack: null }), + }, + }); + + expect(result.processed).toBe(true); + expect(githubService.apiRequest).toHaveBeenCalledTimes(2); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("raw_payload_json = null"), + expect.arrayContaining(["processed"]), + ); + }); + it("emits a PR update when an unmapped pull request changes its projection", async () => { const db = makeMockDb(); const { service } = buildService({ db, laneService: makeLaneService([]) }); @@ -2516,6 +2553,80 @@ describe("prService.getStatus", () => { })); }); + it("retries merge-state GraphQL without stack fields when the schema is unavailable", async () => { + const row = makePrRow({ id: "pr-mergebox-fallback", github_pr_number: 97 }); + const db = makeMockDb(); + installPullRequestRowStore(db, [row]); + const graphqlQueries: string[] = []; + const githubService = makeGithubService({ + apiRequest: vi.fn(async (args: { method?: string; path: string; body?: unknown }) => { + if (args.path === "/repos/test-owner/test-repo/pulls/97") { + return { + data: makeGitHubPull({ + number: 97, + html_url: row.github_url, + title: row.title, + mergeable: false, + mergeable_state: "blocked", + head: { ref: "my-feature", sha: "head-97" }, + base: { ref: "main", sha: "base-97" }, + }), + }; + } + if (args.path === "/repos/test-owner/test-repo/commits/head-97/status") { + return { data: { state: "success", statuses: [] } }; + } + if (args.path === "/repos/test-owner/test-repo/commits/head-97/check-runs") { + return { data: { check_runs: [] } }; + } + if (args.path === "/repos/test-owner/test-repo/pulls/97/reviews") { + return { data: [] }; + } + if (args.path === "/repos/test-owner/test-repo/compare/base-97...head-97") { + return { data: { behind_by: 0 } }; + } + if (args.method === "POST" && args.path === "/graphql") { + const query = String((args.body as { query?: unknown } | undefined)?.query ?? ""); + graphqlQueries.push(query); + if (query.includes("stack { baseRefName }")) { + throw new Error("Field 'stack' doesn't exist on type 'PullRequest'"); + } + return { + data: { + data: { + repository: { + viewerPermission: "WRITE", + pullRequest: { + mergeable: "MERGEABLE", + mergeStateStatus: "CLEAN", + reviewDecision: null, + headRefOid: "head-97", + baseRefName: "main", + baseRef: { branchProtectionRule: null }, + latestOpinionatedReviews: { nodes: [] }, + }, + }, + }, + }, + }; + } + throw new Error(`Unexpected GitHub API path: ${args.path}`); + }), + }); + const { service } = buildService({ db, githubService }); + + const status = await service.getStatus("pr-mergebox-fallback"); + + expect(graphqlQueries).toHaveLength(2); + expect(graphqlQueries[0]).toContain("stack { baseRefName }"); + expect(graphqlQueries[1]).not.toContain("stack { baseRefName }"); + expect(status).toEqual(expect.objectContaining({ + mergeStateStatus: "clean", + isMergeable: true, + headSha: "head-97", + })); + }); + it("uses the ultimate stack base for required approvals", async () => { const row = makePrRow({ id: "pr-stacked-mergebox", github_pr_number: 96 }); const db = makeMockDb(); diff --git a/apps/desktop/src/main/services/prs/prService.ts b/apps/desktop/src/main/services/prs/prService.ts index d023bbfc2..75cd65da8 100644 --- a/apps/desktop/src/main/services/prs/prService.ts +++ b/apps/desktop/src/main/services/prs/prService.ts @@ -4771,39 +4771,52 @@ export function createPrService({ canBypass: boolean; headSha: string | null; } | null> => { - const query = `query($owner:String!,$name:String!,$number:Int!){ + type MergeStateGraphqlData = { + repository?: { + viewerPermission?: unknown; + pullRequest?: { + mergeable?: unknown; + mergeStateStatus?: unknown; + reviewDecision?: unknown; + headRefOid?: unknown; + baseRefName?: unknown; + baseRef?: { branchProtectionRule?: { requiredApprovingReviewCount?: unknown } | null } | null; + stack?: { baseRefName?: unknown } | null; + latestOpinionatedReviews?: { nodes?: Array<{ state?: unknown } | null> | null } | null; + } | null; + } | null; + }; + const query = (includeStack: boolean) => `query($owner:String!,$name:String!,$number:Int!){ repository(owner:$owner,name:$name){ viewerPermission pullRequest(number:$number){ mergeable mergeStateStatus reviewDecision headRefOid baseRefName baseRef { branchProtectionRule { requiredApprovingReviewCount } } - stack { baseRefName } + ${includeStack ? "stack { baseRefName }" : ""} latestOpinionatedReviews(first:100){ nodes { state } } } } }`; try { - const data = await graphqlRequest<{ - repository?: { - viewerPermission?: unknown; - pullRequest?: { - mergeable?: unknown; - mergeStateStatus?: unknown; - reviewDecision?: unknown; - headRefOid?: unknown; - baseRefName?: unknown; - baseRef?: { branchProtectionRule?: { requiredApprovingReviewCount?: unknown } | null } | null; - stack?: { baseRefName?: unknown } | null; - latestOpinionatedReviews?: { nodes?: Array<{ state?: unknown } | null> | null } | null; - } | null; - } | null; - }>( - query, - { owner: repo.owner, name: repo.name, number: prNumber }, - // `mergeStateStatus` is part of GitHub's `merge-info-preview` schema - // preview and errors ("field requires preview header") without this Accept. - { accept: "application/vnd.github.merge-info-preview+json" }, - ); + let data: MergeStateGraphqlData; + try { + data = await graphqlRequest( + query(true), + { owner: repo.owner, name: repo.name, number: prNumber }, + { accept: "application/vnd.github.merge-info-preview+json" }, + ); + } catch (error) { + logger.warn("prs.computeStatus.stack_graphql_fallback", { + repo: `${repo.owner}/${repo.name}`, + prNumber, + error: getErrorMessage(error), + }); + data = await graphqlRequest( + query(false), + { owner: repo.owner, name: repo.name, number: prNumber }, + { accept: "application/vnd.github.merge-info-preview+json" }, + ); + } const repository = data?.repository ?? null; const pull = repository?.pullRequest ?? null; @@ -9282,11 +9295,22 @@ export function createPrService({ if (stackNumber) { try { await githubStackStore.reconcile(repo, stackNumber); - } catch { + } catch (stackError) { // A dissolved stack returns 404 from the item endpoint. The // authoritative repository list distinguishes that from a // transient failure and removes stale local membership atomically. - await githubStackStore.reconcileRepository(repo); + await githubStackStore.reconcileRepository(repo).catch((repositoryError) => { + logger.warn("prs.github_webhook_stack_reconcile_failed", { + eventName, + deliveryId, + repoOwner: repo.owner, + repoName: repo.name, + githubPrNumber: projection.github_pr_number, + stackNumber, + stackError: getErrorMessage(stackError), + repositoryError: getErrorMessage(repositoryError), + }); + }); } } if (linkedPrIds.length === 0) { From 82d91edd6d6152fd074a8a219dfabb9bbaf12cc6 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:10:15 -0400 Subject: [PATCH 3/4] fix(prs): close native stack reconciliation gaps --- .../automationIngressService.test.ts | 53 +++++- .../automations/automationIngressService.ts | 16 +- .../src/main/services/prs/githubStackStore.ts | 7 +- .../src/main/services/prs/prService.test.ts | 168 +++++++++++++++++- .../src/main/services/prs/prService.ts | 55 ++++-- 5 files changed, 269 insertions(+), 30 deletions(-) diff --git a/apps/desktop/src/main/services/automations/automationIngressService.test.ts b/apps/desktop/src/main/services/automations/automationIngressService.test.ts index a47308286..4684d07d3 100644 --- a/apps/desktop/src/main/services/automations/automationIngressService.test.ts +++ b/apps/desktop/src/main/services/automations/automationIngressService.test.ts @@ -818,11 +818,60 @@ describe("automationIngressService", () => { owner: "arul28", name: "ADE", }); - expect(setIngressCursor.mock.invocationCallOrder[0]).toBeLessThan( - reconcileGithubStacks.mock.invocationCallOrder[0]!, + expect(reconcileGithubStacks.mock.invocationCallOrder[0]).toBeLessThan( + setIngressCursor.mock.invocationCallOrder[0]!, ); }); + it("retries cursor-expiry stack repair before committing the replacement cursor", async () => { + const cursors = new Map([["github-relay", "seq:2"]]); + const setIngressCursor = vi.fn(({ source, cursor }: { source: string; cursor: string | null }) => { + cursors.set(source, cursor); + }); + const reconcileGithubStacks = vi.fn() + .mockRejectedValueOnce(new Error("stack list timed out")) + .mockResolvedValueOnce([]); + vi.spyOn(globalThis, "fetch").mockImplementation(async () => new Response(JSON.stringify({ + events: [], + nextCursor: "seq:9", + cursorExpired: true, + hasMore: false, + }), { headers: { "content-type": "application/json" } })); + + service = createAutomationIngressService({ + logger: makeLogger() as never, + automationService: { + updateIngressStatus: vi.fn(), + dispatchIngressTrigger: vi.fn(), + getIngressCursor: (source: string) => cursors.get(source) ?? null, + setIngressCursor, + getIngressStatus: () => ({}), + } as never, + prService: { + ingestGithubWebhook: vi.fn(), + reconcileGithubStacks, + } as never, + secretService: { getSecret: () => null } as never, + githubService: { + detectRepo: vi.fn(async () => ({ owner: "arul28", name: "ADE" })), + getAppUserTokenForRelay: vi.fn(async () => "ghu_app_user_token"), + }, + listRules: () => [], + }); + + await service.pollNow(); + expect(cursors.get("github-relay")).toBe("seq:2"); + expect(setIngressCursor).not.toHaveBeenCalled(); + + await service.pollNow(); + expect(reconcileGithubStacks).toHaveBeenCalledTimes(2); + expect(cursors.get("github-relay")).toBe("seq:9"); + expect(setIngressCursor).toHaveBeenCalledWith({ + source: "github-relay", + cursor: "seq:9", + }); + }); + it("skips a failing event and still advances the relay cursor (poison-event guard)", async () => { const logger = makeLogger(); const setIngressCursor = vi.fn(); diff --git a/apps/desktop/src/main/services/automations/automationIngressService.ts b/apps/desktop/src/main/services/automations/automationIngressService.ts index c48b46473..4830e36cf 100644 --- a/apps/desktop/src/main/services/automations/automationIngressService.ts +++ b/apps/desktop/src/main/services/automations/automationIngressService.ts @@ -941,20 +941,16 @@ export function createAutomationIngressService(args: AutomationIngressServiceArg ? payload.nextCursor : null; if (responseCursor) pageLastCursor = responseCursor; - // Commit only after every event in this page has completed. A failed - // page is replayed from its previous durable cursor on the next drain. + if (payload.cursorExpired === true && repo) { + await args.prService?.reconcileGithubStacks(repo); + } + // Commit only after every event and any cursor-expiry repair in this + // page have completed. A failed page is replayed from its previous + // durable cursor on the next drain. if (pageLastCursor && pageLastCursor !== pageCursor) { setIngressCursor({ source: "github-relay", cursor: pageLastCursor }); for (const prId of pageIngestedPrIds) committedIngestedPrIds.add(prId); } - if (payload.cursorExpired === true && repo) { - await args.prService?.reconcileGithubStacks(repo).catch((error) => { - args.logger.warn("automations.github_stack_cursor_reconcile_failed", { - repo: `${repo.owner}/${repo.name}`, - error: error instanceof Error ? error.message : String(error), - }); - }); - } lastSeenCursor = pageLastCursor; if (useLegacyProjectRoute || payload.hasMore !== true) break; if (!pageLastCursor || pageLastCursor === pageCursor) { diff --git a/apps/desktop/src/main/services/prs/githubStackStore.ts b/apps/desktop/src/main/services/prs/githubStackStore.ts index 4f720fc8d..ba56213f2 100644 --- a/apps/desktop/src/main/services/prs/githubStackStore.ts +++ b/apps/desktop/src/main/services/prs/githubStackStore.ts @@ -388,7 +388,10 @@ export function createGithubStackStore(args: { return request; }; - const reconcileRepository = async (repo: GitHubRepoRef): Promise => { + const reconcileRepository = async ( + repo: GitHubRepoRef, + options: { notifySnapshotChanged?: boolean } = {}, + ): Promise => { const key = repoKey(repo); const existing = repositoryReconcileInFlight.get(key); if (existing) return existing; @@ -439,7 +442,7 @@ export function createGithubStackStore(args: { } throw error; } - args.onSnapshotChanged(); + if (options.notifySnapshotChanged !== false) args.onSnapshotChanged(); return decodedStacks.map((stack) => stackFromRows(stack.row, stack.entries)); }).finally(() => { if (repositoryReconcileInFlight.get(key) === request) { diff --git a/apps/desktop/src/main/services/prs/prService.test.ts b/apps/desktop/src/main/services/prs/prService.test.ts index 2b15db886..392ac5fe6 100644 --- a/apps/desktop/src/main/services/prs/prService.test.ts +++ b/apps/desktop/src/main/services/prs/prService.test.ts @@ -214,9 +214,26 @@ function makeUnmappedBranchPull(overrides?: Partial>) { function makeGithubService(overrides?: Record) { const getTokenOrThrow = (overrides?.getTokenOrThrow as (() => string) | undefined) ?? vi.fn(() => "ghp_mock"); + const apiRequestOverride = overrides?.apiRequest as + | ((args: { path: string; [key: string]: unknown }) => unknown) + | undefined; + const stackApiRequestOverride = overrides?.stackApiRequest as + | ((args: { path: string; [key: string]: unknown }) => unknown) + | undefined; + const { + apiRequest: _apiRequest, + stackApiRequest: _stackApiRequest, + ...remainingOverrides + } = overrides ?? {}; return { getRepoOrThrow: vi.fn(async () => REPO), - apiRequest: vi.fn(), + apiRequest: vi.fn(async (args: { path: string; [key: string]: unknown }) => { + if (args.path === `/repos/${REPO.owner}/${REPO.name}/stacks`) { + if (stackApiRequestOverride) return await stackApiRequestOverride(args); + return { data: [], linkHeader: null }; + } + return await apiRequestOverride?.(args); + }), parseNextLink: vi.fn(() => null), createSecretGist: vi.fn(), getStatus: vi.fn(), @@ -224,7 +241,7 @@ function makeGithubService(overrides?: Record) { clearToken: vi.fn(), getTokenOrThrow, getTokenOrThrowAsync: vi.fn(async () => getTokenOrThrow()), - ...overrides, + ...remainingOverrides, } as any; } @@ -1452,6 +1469,53 @@ describe("prService.getGithubSnapshot", () => { expect(repoCalls).toBe(2); }); + it("bootstraps repository stack state on a forced snapshot when the local store is empty", async () => { + const githubService = makeGithubService({ + getStatus: vi.fn(async () => makeGithubStatus()), + apiRequest: vi.fn(async (args: { path: string }) => { + if (args.path === `/repos/${REPO.owner}/${REPO.name}/pulls`) { + return { data: [] }; + } + throw new Error(`Unexpected GitHub API path: ${args.path}`); + }), + stackApiRequest: vi.fn(async () => ({ + data: [{ + id: 5017, + number: 17, + base: { ref: "main" }, + open: false, + created_at: "2026-07-30T09:00:00Z", + pull_requests: [{ + number: 70, + state: "closed", + draft: false, + merged_at: "2026-07-30T10:00:00Z", + head: { ref: "stack/completed", sha: "sha-completed" }, + }], + }], + linkHeader: null, + })), + }); + const db = makeMockDb(); + const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); + + await service.getGithubSnapshot({ force: true }); + + expect(githubService.apiRequest).toHaveBeenCalledWith({ + method: "GET", + path: `/repos/${REPO.owner}/${REPO.name}/stacks`, + query: { per_page: 100, page: 1 }, + }); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("insert into github_pr_stacks"), + expect.arrayContaining(["proj-1", REPO.owner, REPO.name, 17, "5017", null, "main", 0]), + ); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("insert into github_pr_stack_entries"), + expect.arrayContaining(["proj-1", REPO.owner, REPO.name, 17, 70, 1, "closed"]), + ); + }); + it("preserves repo snapshot cache mode during stale revalidation", async () => { const initialNow = Date.parse("2026-01-01T00:00:00Z"); const nowSpy = vi.spyOn(Date, "now").mockReturnValue(initialNow); @@ -1626,7 +1690,12 @@ describe("prService.getGithubSnapshot", () => { await service.getGithubSnapshot({ force: true }); - expect(githubService.apiRequest).toHaveBeenCalledTimes(1); + expect(githubService.apiRequest).toHaveBeenCalledTimes(2); + expect(githubService.apiRequest).toHaveBeenCalledWith({ + method: "GET", + path: `/repos/${REPO.owner}/${REPO.name}/stacks`, + query: { per_page: 100, page: 1 }, + }); expect(githubService.apiRequest).toHaveBeenCalledWith(expect.objectContaining({ query: expect.objectContaining({ state: "open" }), })); @@ -2038,6 +2107,99 @@ describe("prService.ingestGithubWebhook", () => { expect(db.run).toHaveBeenCalledWith("commit"); }); + it("atomically reconciles the repository when a pull request moves between stacks", async () => { + const db = makeMockDb(); + db.get.mockImplementation((sql: string) => { + if (String(sql).includes("from github_pr_stack_entries")) { + return { github_stack_number: 17 }; + } + return null; + }); + const githubService = makeGithubService({ + apiRequest: vi.fn(async (args: { path: string }) => { + throw new Error(`Unexpected item stack read: ${args.path}`); + }), + stackApiRequest: vi.fn(async () => ({ + data: [ + { + id: 5017, + number: 17, + base: { ref: "main" }, + open: true, + created_at: "2026-07-30T09:00:00Z", + pull_requests: [{ + number: 89, + state: "open", + draft: false, + merged_at: null, + head: { ref: "stack/remaining", sha: "sha-remaining" }, + }], + }, + { + id: 5018, + number: 18, + base: { ref: "main" }, + open: true, + created_at: "2026-07-30T10:00:00Z", + pull_requests: [{ + number: 90, + state: "open", + draft: false, + merged_at: null, + head: { ref: "stack/moved", sha: "sha-moved" }, + }], + }, + ], + linkHeader: null, + })), + }); + const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); + + await service.ingestGithubWebhook({ + eventName: "pull_request", + deliveryId: "delivery-restacked", + payload: { + action: "stacked", + repository: { + full_name: `${REPO.owner}/${REPO.name}`, + owner: { login: REPO.owner }, + name: REPO.name, + }, + pull_request: makeGitHubPull({ + number: 90, + stack: { + id: 5018, + number: 18, + size: 1, + position: 1, + base: { ref: "main", sha: "sha-main" }, + }, + }), + }, + }); + + expect(githubService.apiRequest).not.toHaveBeenCalledWith(expect.objectContaining({ + path: `/repos/${REPO.owner}/${REPO.name}/stacks/18`, + })); + expect(githubService.apiRequest).toHaveBeenCalledWith({ + method: "GET", + path: `/repos/${REPO.owner}/${REPO.name}/stacks`, + query: { per_page: 100, page: 1 }, + }); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("delete from github_pr_stacks"), + ["proj-1", REPO.owner, REPO.name], + ); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("insert into github_pr_stacks"), + expect.arrayContaining([17, "5017", "main", 1]), + ); + expect(db.run).toHaveBeenCalledWith( + expect.stringContaining("insert into github_pr_stacks"), + expect.arrayContaining([18, "5018", "main", 1]), + ); + }); + it("reconciles a previously known stack when a later PR webhook omits stack metadata", async () => { const db = makeMockDb(); db.get.mockImplementation((sql: string) => { diff --git a/apps/desktop/src/main/services/prs/prService.ts b/apps/desktop/src/main/services/prs/prService.ts index 75cd65da8..2008d8149 100644 --- a/apps/desktop/src/main/services/prs/prService.ts +++ b/apps/desktop/src/main/services/prs/prService.ts @@ -8877,14 +8877,6 @@ export function createPrService({ }; } - if (options.force === true && githubStackStore.list(repo).length > 0) { - await githubStackStore.reconcileRepository(repo).catch((error) => { - logger.warn("prs.github_stack_repository_reconcile_failed", { - repo: `${repo.owner}/${repo.name}`, - error: getErrorMessage(error), - }); - }); - } let metadata = await loadGithubSnapshotMetadata(); const historyPageLimit = normalizeGithubHistoryPageLimit(options); const repoPullRequestMaxPages = options.includeExternalClosed === true @@ -8976,12 +8968,25 @@ export function createPrService({ if (cachedGithubSnapshot && !githubSnapshotMatchesStatus(cachedGithubSnapshot, githubStatus)) { invalidateGithubSnapshotCache(); } + const forceRequestEpoch = githubSnapshotCacheEpoch; + if (force && githubStatus.repo) { + await githubStackStore.reconcileRepository( + githubStatus.repo, + { notifySnapshotChanged: false }, + ).catch((error) => { + logger.warn("prs.github_stack_repository_reconcile_failed", { + repo: `${githubStatus.repo!.owner}/${githubStatus.repo!.name}`, + error: getErrorMessage(error), + }); + }); + } const startSnapshotRequest = ( precheckedGithubStatus: GitHubStatus, requestOptions: GithubSnapshotOptions, + requestEpochOverride?: number, ): Promise => { - const requestEpoch = githubSnapshotCacheEpoch; + const requestEpoch = requestEpochOverride ?? githubSnapshotCacheEpoch; const includeExternalClosed = requestOptions.includeExternalClosed === true; const historyPageLimit = normalizeGithubHistoryPageLimit(requestOptions); const includeStateCounts = requestOptions.includeStateCounts === true @@ -9091,7 +9096,11 @@ export function createPrService({ return compatibleInFlight.request; } - return startSnapshotRequest(githubStatus, options); + return startSnapshotRequest( + githubStatus, + options, + force ? forceRequestEpoch : undefined, + ); }; const emitPrsUpdated = (): void => { @@ -9290,9 +9299,29 @@ export function createPrService({ const webhookMembership = githubStackStore.parseMembership( isRecord(rawPull?.stack) ? rawPull.stack : payload.stack, ); - const stackNumber = webhookMembership?.number - ?? githubStackStore.knownStackNumberForPr(repo, Number(projection.github_pr_number)); - if (stackNumber) { + const previousStackNumber = githubStackStore.knownStackNumberForPr( + repo, + Number(projection.github_pr_number), + ); + const stackNumber = webhookMembership?.number ?? previousStackNumber; + if ( + webhookMembership + && previousStackNumber + && previousStackNumber !== webhookMembership.number + ) { + await githubStackStore.reconcileRepository(repo).catch((repositoryError) => { + logger.warn("prs.github_webhook_stack_reconcile_failed", { + eventName, + deliveryId, + repoOwner: repo.owner, + repoName: repo.name, + githubPrNumber: projection.github_pr_number, + previousStackNumber, + stackNumber: webhookMembership.number, + repositoryError: getErrorMessage(repositoryError), + }); + }); + } else if (stackNumber) { try { await githubStackStore.reconcile(repo, stackNumber); } catch (stackError) { From 6ec58ab4e3913ecd6e1a0810958d352074ce6616 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:31:36 -0400 Subject: [PATCH 4/4] test(prs): exercise stack reconciliation fallbacks --- .../src/main/services/prs/prService.test.ts | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/apps/desktop/src/main/services/prs/prService.test.ts b/apps/desktop/src/main/services/prs/prService.test.ts index 392ac5fe6..c8d376af1 100644 --- a/apps/desktop/src/main/services/prs/prService.test.ts +++ b/apps/desktop/src/main/services/prs/prService.test.ts @@ -228,7 +228,10 @@ function makeGithubService(overrides?: Record) { return { getRepoOrThrow: vi.fn(async () => REPO), apiRequest: vi.fn(async (args: { path: string; [key: string]: unknown }) => { - if (args.path === `/repos/${REPO.owner}/${REPO.name}/stacks`) { + if ( + args.method === "GET" + && args.path === `/repos/${REPO.owner}/${REPO.name}/stacks` + ) { if (stackApiRequestOverride) return await stackApiRequestOverride(args); return { data: [], linkHeader: null }; } @@ -2267,11 +2270,9 @@ describe("prService.ingestGithubWebhook", () => { } return null; }); - const githubService = makeGithubService({ - apiRequest: vi.fn() - .mockRejectedValueOnce(new Error("Not Found")) - .mockResolvedValueOnce({ data: [], linkHeader: null }), - }); + const apiRequest = vi.fn().mockRejectedValueOnce(new Error("Not Found")); + const stackApiRequest = vi.fn().mockResolvedValueOnce({ data: [], linkHeader: null }); + const githubService = makeGithubService({ apiRequest, stackApiRequest }); const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); const result = await service.ingestGithubWebhook({ @@ -2294,6 +2295,11 @@ describe("prService.ingestGithubWebhook", () => { path: `/repos/${REPO.owner}/${REPO.name}/stacks`, query: { per_page: 100, page: 1 }, }); + expect(stackApiRequest).toHaveBeenCalledWith({ + method: "GET", + path: `/repos/${REPO.owner}/${REPO.name}/stacks`, + query: { per_page: 100, page: 1 }, + }); expect(db.run).toHaveBeenCalledWith("begin immediate"); expect(db.run).toHaveBeenCalledWith( expect.stringContaining("delete from github_pr_stacks"), @@ -2310,11 +2316,9 @@ describe("prService.ingestGithubWebhook", () => { } return null; }); - const githubService = makeGithubService({ - apiRequest: vi.fn() - .mockRejectedValueOnce(new Error("Stack read timed out")) - .mockRejectedValueOnce(new Error("Repository stack list timed out")), - }); + const apiRequest = vi.fn().mockRejectedValueOnce(new Error("Stack read timed out")); + const stackApiRequest = vi.fn().mockRejectedValueOnce(new Error("Repository stack list timed out")); + const githubService = makeGithubService({ apiRequest, stackApiRequest }); const { service } = buildService({ db, githubService, laneService: makeLaneService([]) }); const result = await service.ingestGithubWebhook({ @@ -2333,6 +2337,8 @@ describe("prService.ingestGithubWebhook", () => { expect(result.processed).toBe(true); expect(githubService.apiRequest).toHaveBeenCalledTimes(2); + expect(apiRequest).toHaveBeenCalledTimes(1); + expect(stackApiRequest).toHaveBeenCalledTimes(1); expect(db.run).toHaveBeenCalledWith( expect.stringContaining("raw_payload_json = null"), expect.arrayContaining(["processed"]),