From 3ffd45887f4e4dc6cd1d5bc379d57584e1235902 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Fri, 20 Mar 2026 18:06:34 -0700 Subject: [PATCH 1/4] fix(desktop): harden PR merge detection fallback --- .../routers/changes/git-operations.test.ts | 19 ++- .../trpc/routers/changes/git-operations.ts | 94 ++++++++++- .../src/lib/trpc/routers/changes/git-utils.ts | 4 + .../workspaces/utils/github/github.test.ts | 58 ++++++- .../routers/workspaces/utils/github/github.ts | 149 +++++++++++++++++- .../routers/workspaces/utils/github/types.ts | 6 + 6 files changed, 316 insertions(+), 14 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/changes/git-operations.test.ts b/apps/desktop/src/lib/trpc/routers/changes/git-operations.test.ts index da476c8a504..a848e092c96 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/git-operations.test.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/git-operations.test.ts @@ -1,5 +1,8 @@ import { describe, expect, test } from "bun:test"; -import { isUpstreamMissingError } from "./git-utils"; +import { + isNoPullRequestFoundMessage, + isUpstreamMissingError, +} from "./git-utils"; describe("git-operations error handling", () => { describe("isUpstreamMissingError", () => { @@ -31,6 +34,20 @@ describe("git-operations error handling", () => { }); describe("error message patterns", () => { + test("detects no-pull-request gh messages case-insensitively", () => { + expect( + isNoPullRequestFoundMessage( + 'no pull requests found for branch "feature/my-thing"', + ), + ).toBe(true); + expect( + isNoPullRequestFoundMessage("No pull request found for this branch"), + ).toBe(true); + expect( + isNoPullRequestFoundMessage("failed to push some refs to origin"), + ).toBe(false); + }); + test("commit with no staged changes", () => { const message = "nothing to commit, working tree clean"; expect(message.includes("nothing to commit")).toBe(true); diff --git a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts index 4361960f47a..99cf3856a0a 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts @@ -8,12 +8,16 @@ import { } from "../workspaces/utils/git-client"; import { clearGitHubStatusCacheForWorktree, + getPRForBranch, getPullRequestRepoArgs, getRepoContext, } from "../workspaces/utils/github/github"; import { execWithShellEnv } from "../workspaces/utils/shell-env"; import { resolveTrackingRemoteName } from "../workspaces/utils/upstream-ref"; -import { isUpstreamMissingError } from "./git-utils"; +import { + isNoPullRequestFoundMessage, + isUpstreamMissingError, +} from "./git-utils"; import { assertRegisteredWorktree } from "./security/path-validation"; import { buildPullRequestCompareUrl, @@ -585,18 +589,98 @@ export const createGitOperationsRouter = () => { async ({ input }): Promise<{ success: boolean; mergedAt?: string }> => { assertRegisteredWorktree(input.worktreePath); - const args = ["pr", "merge", `--${input.strategy}`]; - - try { + const legacyMergeArgs = ["pr", "merge", `--${input.strategy}`]; + const runMerge = async ( + args: string[], + ): Promise<{ success: boolean; mergedAt: string }> => { await execWithShellEnv("gh", args, { cwd: input.worktreePath }); clearWorktreeStatusCaches(input.worktreePath); return { success: true, mergedAt: new Date().toISOString() }; + }; + + try { + const repoContext = await getRepoContext(input.worktreePath); + if (!repoContext) { + return await runMerge(legacyMergeArgs); + } + + try { + const [{ stdout: branchOutput }, { stdout: headOutput }] = + await Promise.all([ + execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { + cwd: input.worktreePath, + }), + execGitWithShellPath(["rev-parse", "HEAD"], { + cwd: input.worktreePath, + }), + ]); + const localBranch = branchOutput.trim(); + const headSha = headOutput.trim(); + + const pr = await getPRForBranch( + input.worktreePath, + localBranch, + repoContext, + headSha, + ); + if (!pr) { + return await runMerge(legacyMergeArgs); + } + if (pr.state === "merged") { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "PR is already merged", + }); + } + if (pr.state === "closed") { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "PR is closed and cannot be merged", + }); + } + + const args = [ + "pr", + "merge", + String(pr.number), + `--${input.strategy}`, + ...getPullRequestRepoArgs(repoContext), + ]; + + try { + return await runMerge(args); + } catch (error) { + const message = + error instanceof Error ? error.message : String(error); + if (isNoPullRequestFoundMessage(message)) { + return await runMerge(legacyMergeArgs); + } + throw error; + } + } catch (error) { + if (error instanceof TRPCError) { + throw error; + } + + console.warn( + "[git/mergePR] Explicit PR resolution failed; falling back to branch merge.", + { + worktreePath: input.worktreePath, + error: error instanceof Error ? error.message : String(error), + }, + ); + return await runMerge(legacyMergeArgs); + } } catch (error) { + if (error instanceof TRPCError) { + throw error; + } + const message = error instanceof Error ? error.message : String(error); console.error("[git/mergePR] Failed to merge PR:", message); - if (message.includes("no pull requests found")) { + if (isNoPullRequestFoundMessage(message)) { throw new TRPCError({ code: "NOT_FOUND", message: "No pull request found for this branch", diff --git a/apps/desktop/src/lib/trpc/routers/changes/git-utils.ts b/apps/desktop/src/lib/trpc/routers/changes/git-utils.ts index 663d00897b3..2d40eea1fd7 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/git-utils.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/git-utils.ts @@ -9,3 +9,7 @@ export function isUpstreamMissingError(message: string): boolean { message.includes("cannot be resolved to branch") ); } + +export function isNoPullRequestFoundMessage(message: string): boolean { + return message.toLowerCase().includes("no pull request"); +} diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts index ec59f9cfd10..bf0fd5c28f0 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts @@ -1,5 +1,10 @@ import { describe, expect, test } from "bun:test"; -import { branchMatchesPR, getPullRequestRepoArgs } from "./github"; +import { + branchMatchesPR, + getPRHeadBranchCandidates, + getPullRequestRepoArgs, + prMatchesLocalBranch, +} from "./github"; describe("branchMatchesPR", () => { test("matches same-repo branch exactly", () => { @@ -57,3 +62,54 @@ describe("getPullRequestRepoArgs", () => { ).toEqual([]); }); }); + +describe("getPRHeadBranchCandidates", () => { + test("returns exact branch first", () => { + expect(getPRHeadBranchCandidates("kitenite/feature")).toEqual([ + "kitenite/feature", + "feature", + ]); + }); + + test("de-duplicates single-segment branches", () => { + expect(getPRHeadBranchCandidates("main")).toEqual(["main"]); + }); +}); + +describe("prMatchesLocalBranch", () => { + test("matches exact branch names", () => { + expect( + prMatchesLocalBranch("kitenite/feature", { + headRefName: "kitenite/feature", + headRepositoryOwner: { login: "Kitenite" }, + }), + ).toBe(true); + }); + + test("matches owner-prefixed local branches for fork PRs", () => { + expect( + prMatchesLocalBranch("forkowner/feature/my-thing", { + headRefName: "feature/my-thing", + headRepositoryOwner: { login: "forkowner" }, + }), + ).toBe(true); + }); + + test("rejects suffix-only matches when owner prefix does not match", () => { + expect( + prMatchesLocalBranch("feature/my-thing", { + headRefName: "my-thing", + headRepositoryOwner: { login: "someone-else" }, + }), + ).toBe(false); + }); + + test("rejects owner-prefixed matches without owner metadata", () => { + expect( + prMatchesLocalBranch("forkowner/feature/my-thing", { + headRefName: "feature/my-thing", + headRepositoryOwner: null, + }), + ).toBe(false); + }); +}); diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts index 7649ae7c3f3..394028a10c2 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts @@ -217,9 +217,9 @@ export function getPullRequestRepoArgs( } const PR_JSON_FIELDS = - "number,title,url,state,isDraft,mergedAt,additions,deletions,headRefOid,headRefName,reviewDecision,statusCheckRollup,reviewRequests"; + "number,title,url,state,isDraft,mergedAt,additions,deletions,headRefOid,headRefName,headRepositoryOwner,reviewDecision,statusCheckRollup,reviewRequests"; -async function getPRForBranch( +export async function getPRForBranch( worktreePath: string, localBranch: string, repoContext?: RepoContext, @@ -230,6 +230,16 @@ async function getPRForBranch( return byTracking; } + const byHeadBranch = await findPRByHeadBranch( + worktreePath, + localBranch, + repoContext, + headSha, + ); + if (byHeadBranch) { + return byHeadBranch; + } + return findPRByHeadCommit(worktreePath, repoContext, headSha); } @@ -247,6 +257,83 @@ export function branchMatchesPR( ); } +export function getPRHeadBranchCandidates(localBranch: string): string[] { + const strippedBranch = localBranch.includes("/") + ? localBranch.slice(localBranch.indexOf("/") + 1) + : localBranch; + + return Array.from(new Set([localBranch, strippedBranch].filter(Boolean))); +} + +function getForkOwnerPrefix( + localBranch: string, + prHeadRefName: string, +): string | null { + if (localBranch === prHeadRefName) { + return null; + } + + const suffix = `/${prHeadRefName}`; + if (!localBranch.endsWith(suffix)) { + return null; + } + + const prefix = localBranch.slice(0, -suffix.length).trim(); + return prefix ? prefix.toLowerCase() : null; +} + +export function prMatchesLocalBranch( + localBranch: string, + pr: Pick, +): boolean { + if (!branchMatchesPR(localBranch, pr.headRefName)) { + return false; + } + + const ownerPrefix = getForkOwnerPrefix(localBranch, pr.headRefName); + if (!ownerPrefix) { + return localBranch === pr.headRefName; + } + + return pr.headRepositoryOwner?.login.toLowerCase() === ownerPrefix; +} + +function sortPRCandidates( + candidates: GHPRResponse[], + headSha?: string, +): GHPRResponse[] { + const getStateRank = (candidate: GHPRResponse): number => { + if (candidate.state === "OPEN") return 2; + if (candidate.state === "MERGED") return 1; + return 0; + }; + + return [...candidates].sort((left, right) => { + const leftMatchesHead = Number( + Boolean(headSha && left.headRefOid === headSha), + ); + const rightMatchesHead = Number( + Boolean(headSha && right.headRefOid === headSha), + ); + if (leftMatchesHead !== rightMatchesHead) { + return rightMatchesHead - leftMatchesHead; + } + + const stateDelta = getStateRank(right) - getStateRank(left); + if (stateDelta !== 0) { + return stateDelta; + } + + const leftMergedAt = left.mergedAt ? Date.parse(left.mergedAt) : 0; + const rightMergedAt = right.mergedAt ? Date.parse(right.mergedAt) : 0; + if (leftMergedAt !== rightMergedAt) { + return rightMergedAt - leftMergedAt; + } + + return right.number - left.number; + }); +} + /** * Looks up a PR using `gh pr view` (no args), which matches via the branch's * tracking ref. Essential for fork PRs that track refs/pull/XXX/head. @@ -271,7 +358,7 @@ async function getPRByBranchTracking( // `gh pr view` can match via stale tracking refs (e.g. refs/pull/N/head) // left over from a previous `gh pr checkout`, causing a new workspace // to incorrectly show an old, unrelated PR. - if (!branchMatchesPR(localBranch, data.headRefName)) { + if (!prMatchesLocalBranch(localBranch, data)) { return null; } @@ -287,6 +374,52 @@ async function getPRByBranchTracking( } } +/** + * Looks up PRs by exact head branch name. This avoids relying on `gh pr view` + * branch inference, which can miss fork-tracked branches in some clones. + */ +async function findPRByHeadBranch( + worktreePath: string, + localBranch: string, + repoContext?: RepoContext, + headSha?: string, +): Promise { + try { + const matches = new Map(); + + for (const branchCandidate of getPRHeadBranchCandidates(localBranch)) { + const { stdout } = await execWithShellEnv( + "gh", + [ + "pr", + "list", + ...getPullRequestRepoArgs(repoContext), + "--state", + "all", + "--head", + branchCandidate, + "--limit", + "20", + "--json", + PR_JSON_FIELDS, + ], + { cwd: worktreePath }, + ); + + for (const candidate of parsePRListResponse(stdout)) { + if (prMatchesLocalBranch(localBranch, candidate)) { + matches.set(candidate.number, candidate); + } + } + } + + const bestMatch = sortPRCandidates([...matches.values()], headSha)[0]; + return bestMatch ? formatPRData(bestMatch) : null; + } catch { + return null; + } +} + /** * Looks up PRs that have local HEAD as their head commit. * This avoids matching unrelated PRs that merely contain the same commit. @@ -328,10 +461,12 @@ async function findPRByHeadCommit( ); const candidates = parsePRListResponse(stdout); - for (const candidate of candidates) { - if (candidate.headRefOid === headSha) { - return formatPRData(candidate); - } + const exactHeadMatches = candidates.filter( + (candidate) => candidate.headRefOid === headSha, + ); + const bestMatch = sortPRCandidates(exactHeadMatches, headSha)[0]; + if (bestMatch) { + return formatPRData(bestMatch); } return null; diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/types.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/types.ts index cb26cabfa93..46cf61e1c4c 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/types.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/types.ts @@ -43,6 +43,12 @@ export const GHPRResponseSchema = z.object({ deletions: z.number(), headRefOid: z.string(), headRefName: z.string(), + headRepositoryOwner: z + .object({ + login: z.string(), + }) + .nullable() + .optional(), reviewDecision: z .enum(["APPROVED", "CHANGES_REQUESTED", "REVIEW_REQUIRED", ""]) .nullable(), From 7d1d215844346f8f429c14b6660d225995c8707d Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Fri, 20 Mar 2026 18:20:38 -0700 Subject: [PATCH 2/4] refactor(desktop): split GitHub PR merge helpers --- .../trpc/routers/changes/git-operations.ts | 101 +--- .../changes/utils/merge-pull-request.ts | 99 ++++ .../changes/utils/worktree-status-caches.ts | 7 + .../workspaces/utils/github/github.test.ts | 4 +- .../routers/workspaces/utils/github/github.ts | 545 +----------------- .../routers/workspaces/utils/github/index.ts | 7 + .../workspaces/utils/github/pr-resolution.ts | 428 ++++++++++++++ .../workspaces/utils/github/repo-context.ts | 122 ++++ 8 files changed, 681 insertions(+), 632 deletions(-) create mode 100644 apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts create mode 100644 apps/desktop/src/lib/trpc/routers/changes/utils/worktree-status-caches.ts create mode 100644 apps/desktop/src/lib/trpc/routers/workspaces/utils/github/pr-resolution.ts create mode 100644 apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts diff --git a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts index 99cf3856a0a..278d7d04717 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts @@ -7,11 +7,9 @@ import { getSimpleGitWithShellPath, } from "../workspaces/utils/git-client"; import { - clearGitHubStatusCacheForWorktree, - getPRForBranch, getPullRequestRepoArgs, getRepoContext, -} from "../workspaces/utils/github/github"; +} from "../workspaces/utils/github"; import { execWithShellEnv } from "../workspaces/utils/shell-env"; import { resolveTrackingRemoteName } from "../workspaces/utils/upstream-ref"; import { @@ -19,12 +17,14 @@ import { isUpstreamMissingError, } from "./git-utils"; import { assertRegisteredWorktree } from "./security/path-validation"; +import { mergePullRequest } from "./utils/merge-pull-request"; import { buildPullRequestCompareUrl, normalizeGitHubRepoUrl, parseUpstreamRef, } from "./utils/pull-request-url"; import { clearStatusCacheForWorktree } from "./utils/status-cache"; +import { clearWorktreeStatusCaches } from "./utils/worktree-status-caches"; export { isUpstreamMissingError }; @@ -589,93 +589,9 @@ export const createGitOperationsRouter = () => { async ({ input }): Promise<{ success: boolean; mergedAt?: string }> => { assertRegisteredWorktree(input.worktreePath); - const legacyMergeArgs = ["pr", "merge", `--${input.strategy}`]; - const runMerge = async ( - args: string[], - ): Promise<{ success: boolean; mergedAt: string }> => { - await execWithShellEnv("gh", args, { cwd: input.worktreePath }); - clearWorktreeStatusCaches(input.worktreePath); - return { success: true, mergedAt: new Date().toISOString() }; - }; - try { - const repoContext = await getRepoContext(input.worktreePath); - if (!repoContext) { - return await runMerge(legacyMergeArgs); - } - - try { - const [{ stdout: branchOutput }, { stdout: headOutput }] = - await Promise.all([ - execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { - cwd: input.worktreePath, - }), - execGitWithShellPath(["rev-parse", "HEAD"], { - cwd: input.worktreePath, - }), - ]); - const localBranch = branchOutput.trim(); - const headSha = headOutput.trim(); - - const pr = await getPRForBranch( - input.worktreePath, - localBranch, - repoContext, - headSha, - ); - if (!pr) { - return await runMerge(legacyMergeArgs); - } - if (pr.state === "merged") { - throw new TRPCError({ - code: "BAD_REQUEST", - message: "PR is already merged", - }); - } - if (pr.state === "closed") { - throw new TRPCError({ - code: "BAD_REQUEST", - message: "PR is closed and cannot be merged", - }); - } - - const args = [ - "pr", - "merge", - String(pr.number), - `--${input.strategy}`, - ...getPullRequestRepoArgs(repoContext), - ]; - - try { - return await runMerge(args); - } catch (error) { - const message = - error instanceof Error ? error.message : String(error); - if (isNoPullRequestFoundMessage(message)) { - return await runMerge(legacyMergeArgs); - } - throw error; - } - } catch (error) { - if (error instanceof TRPCError) { - throw error; - } - - console.warn( - "[git/mergePR] Explicit PR resolution failed; falling back to branch merge.", - { - worktreePath: input.worktreePath, - error: error instanceof Error ? error.message : String(error), - }, - ); - return await runMerge(legacyMergeArgs); - } + return await mergePullRequest(input); } catch (error) { - if (error instanceof TRPCError) { - throw error; - } - const message = error instanceof Error ? error.message : String(error); console.error("[git/mergePR] Failed to merge PR:", message); @@ -686,6 +602,15 @@ export const createGitOperationsRouter = () => { message: "No pull request found for this branch", }); } + if ( + message === "PR is already merged" || + message === "PR is closed and cannot be merged" + ) { + throw new TRPCError({ + code: "BAD_REQUEST", + message, + }); + } if ( message.includes("not mergeable") || message.includes("blocked") diff --git a/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts new file mode 100644 index 00000000000..539128d3515 --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts @@ -0,0 +1,99 @@ +import { execGitWithShellPath } from "../../workspaces/utils/git-client"; +import { + getPRForBranch, + getPullRequestRepoArgs, + getRepoContext, +} from "../../workspaces/utils/github"; +import { execWithShellEnv } from "../../workspaces/utils/shell-env"; +import { isNoPullRequestFoundMessage } from "../git-utils"; +import { clearWorktreeStatusCaches } from "./worktree-status-caches"; + +const PR_ALREADY_MERGED_MESSAGE = "PR is already merged"; +const PR_CLOSED_MESSAGE = "PR is closed and cannot be merged"; + +export interface MergePullRequestInput { + worktreePath: string; + strategy: "merge" | "squash" | "rebase"; +} + +export async function mergePullRequest({ + worktreePath, + strategy, +}: MergePullRequestInput): Promise<{ success: boolean; mergedAt: string }> { + const legacyMergeArgs = ["pr", "merge", `--${strategy}`]; + const runMerge = async ( + args: string[], + ): Promise<{ success: boolean; mergedAt: string }> => { + await execWithShellEnv("gh", args, { cwd: worktreePath }); + clearWorktreeStatusCaches(worktreePath); + return { success: true, mergedAt: new Date().toISOString() }; + }; + + const repoContext = await getRepoContext(worktreePath); + if (!repoContext) { + return runMerge(legacyMergeArgs); + } + + try { + const [{ stdout: branchOutput }, { stdout: headOutput }] = + await Promise.all([ + execGitWithShellPath(["rev-parse", "--abbrev-ref", "HEAD"], { + cwd: worktreePath, + }), + execGitWithShellPath(["rev-parse", "HEAD"], { cwd: worktreePath }), + ]); + const localBranch = branchOutput.trim(); + const headSha = headOutput.trim(); + + const pr = await getPRForBranch( + worktreePath, + localBranch, + repoContext, + headSha, + ); + if (!pr) { + return runMerge(legacyMergeArgs); + } + if (pr.state === "merged") { + throw new Error(PR_ALREADY_MERGED_MESSAGE); + } + if (pr.state === "closed") { + throw new Error(PR_CLOSED_MESSAGE); + } + + const args = [ + "pr", + "merge", + String(pr.number), + `--${strategy}`, + ...getPullRequestRepoArgs(repoContext), + ]; + + try { + return await runMerge(args); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if (isNoPullRequestFoundMessage(message)) { + return runMerge(legacyMergeArgs); + } + throw error; + } + } catch (error) { + if ( + error instanceof Error && + (error.message === PR_ALREADY_MERGED_MESSAGE || + error.message === PR_CLOSED_MESSAGE) + ) { + throw error; + } + + console.warn( + "[git/mergePR] Explicit PR resolution failed; falling back to branch merge.", + { + worktreePath, + error: error instanceof Error ? error.message : String(error), + }, + ); + return runMerge(legacyMergeArgs); + } +} diff --git a/apps/desktop/src/lib/trpc/routers/changes/utils/worktree-status-caches.ts b/apps/desktop/src/lib/trpc/routers/changes/utils/worktree-status-caches.ts new file mode 100644 index 00000000000..a91078ad4ec --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/changes/utils/worktree-status-caches.ts @@ -0,0 +1,7 @@ +import { clearGitHubStatusCacheForWorktree } from "../../workspaces/utils/github"; +import { clearStatusCacheForWorktree } from "./status-cache"; + +export function clearWorktreeStatusCaches(worktreePath: string): void { + clearGitHubStatusCacheForWorktree(worktreePath); + clearStatusCacheForWorktree(worktreePath); +} diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts index bf0fd5c28f0..5343a14b42b 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.test.ts @@ -2,9 +2,9 @@ import { describe, expect, test } from "bun:test"; import { branchMatchesPR, getPRHeadBranchCandidates, - getPullRequestRepoArgs, prMatchesLocalBranch, -} from "./github"; +} from "./pr-resolution"; +import { getPullRequestRepoArgs } from "./repo-context"; describe("branchMatchesPR", () => { test("matches same-repo branch exactly", () => { diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts index 394028a10c2..1f0e27cfcec 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts @@ -1,13 +1,12 @@ -import type { CheckItem, GitHubStatus } from "@superset/local-db"; +import type { GitHubStatus } from "@superset/local-db"; import { branchExistsOnRemote, getTrackingRemoteNameForWorktree } from "../git"; import { execGitWithShellPath } from "../git-client"; import { execWithShellEnv } from "../shell-env"; +import { getPRForBranch } from "./pr-resolution"; +import { extractNwoFromUrl, getRepoContext } from "./repo-context"; import { GHDeploymentSchema, GHDeploymentStatusSchema, - type GHPRResponse, - GHPRResponseSchema, - GHRepoResponseSchema, type RepoContext, } from "./types"; @@ -88,100 +87,6 @@ export async function fetchGitHubPRStatus( } } -const repoContextCache = new Map< - string, - { data: RepoContext; timestamp: number } ->(); -const REPO_CONTEXT_CACHE_TTL_MS = 300_000; // 5 minutes - -export async function getRepoContext( - worktreePath: string, -): Promise { - const cached = repoContextCache.get(worktreePath); - if (cached && Date.now() - cached.timestamp < REPO_CONTEXT_CACHE_TTL_MS) { - return cached.data; - } - - try { - const { stdout } = await execWithShellEnv( - "gh", - ["repo", "view", "--json", "url,isFork,parent"], - { cwd: worktreePath }, - ); - const raw = JSON.parse(stdout); - const result = GHRepoResponseSchema.safeParse(raw); - if (!result.success) { - console.error("[GitHub] Repo schema validation failed:", result.error); - console.error("[GitHub] Raw data:", JSON.stringify(raw, null, 2)); - return null; - } - - const data = result.data; - let context: RepoContext; - - if (data.isFork && data.parent) { - context = { - repoUrl: data.url, - upstreamUrl: data.parent.url, - isFork: true, - }; - } else { - const originUrl = await getOriginUrl(worktreePath); - const ghUrl = normalizeGitHubUrl(data.url); - - if (originUrl && ghUrl && originUrl !== ghUrl) { - context = { - repoUrl: originUrl, - upstreamUrl: ghUrl, - isFork: true, - }; - } else { - context = { - repoUrl: data.url, - upstreamUrl: data.url, - isFork: false, - }; - } - } - - repoContextCache.set(worktreePath, { - data: context, - timestamp: Date.now(), - }); - return context; - } catch { - return null; - } -} - -async function getOriginUrl(worktreePath: string): Promise { - try { - const { stdout } = await execGitWithShellPath( - ["remote", "get-url", "origin"], - { cwd: worktreePath }, - ); - return normalizeGitHubUrl(stdout.trim()); - } catch { - return null; - } -} - -function normalizeGitHubUrl(remoteUrl: string): string | null { - const trimmed = remoteUrl.trim(); - const patterns = [ - /^git@github\.com:(?[^/]+\/[^/]+?)(?:\.git)?$/, - /^ssh:\/\/git@github\.com\/(?[^/]+\/[^/]+?)(?:\.git)?$/, - /^https:\/\/github\.com\/(?[^/]+\/[^/]+?)(?:\.git)?\/?$/, - ]; - for (const pattern of patterns) { - const match = pattern.exec(trimmed); - if (match?.groups?.nwo) { - return `https://github.com/${match.groups.nwo}`; - } - } - return null; -} - function isSafeHttpUrl(url: string): boolean { try { const parsed = new URL(url); @@ -191,420 +96,6 @@ function isSafeHttpUrl(url: string): boolean { } } -function extractNwoFromUrl(normalizedUrl: string): string | null { - try { - const path = new URL(normalizedUrl).pathname.slice(1); - return path || null; - } catch { - return null; - } -} - -export function getPullRequestRepoArgs( - repoContext?: Pick | null, -): string[] { - if (!repoContext?.isFork) { - return []; - } - - const normalizedUpstreamUrl = normalizeGitHubUrl(repoContext.upstreamUrl); - if (!normalizedUpstreamUrl) { - return []; - } - - const repoNameWithOwner = extractNwoFromUrl(normalizedUpstreamUrl); - return repoNameWithOwner ? ["--repo", repoNameWithOwner] : []; -} - -const PR_JSON_FIELDS = - "number,title,url,state,isDraft,mergedAt,additions,deletions,headRefOid,headRefName,headRepositoryOwner,reviewDecision,statusCheckRollup,reviewRequests"; - -export async function getPRForBranch( - worktreePath: string, - localBranch: string, - repoContext?: RepoContext, - headSha?: string, -): Promise { - const byTracking = await getPRByBranchTracking(worktreePath, localBranch); - if (byTracking) { - return byTracking; - } - - const byHeadBranch = await findPRByHeadBranch( - worktreePath, - localBranch, - repoContext, - headSha, - ); - if (byHeadBranch) { - return byHeadBranch; - } - - return findPRByHeadCommit(worktreePath, repoContext, headSha); -} - -/** - * Returns true when the local branch name matches the PR's head branch. - * Handles fork PRs where the local branch is prefixed with the fork owner - * (e.g. local "owner/feature" matches PR headRefName "feature"). - */ -export function branchMatchesPR( - localBranch: string, - prHeadRefName: string, -): boolean { - return ( - localBranch === prHeadRefName || localBranch.endsWith(`/${prHeadRefName}`) - ); -} - -export function getPRHeadBranchCandidates(localBranch: string): string[] { - const strippedBranch = localBranch.includes("/") - ? localBranch.slice(localBranch.indexOf("/") + 1) - : localBranch; - - return Array.from(new Set([localBranch, strippedBranch].filter(Boolean))); -} - -function getForkOwnerPrefix( - localBranch: string, - prHeadRefName: string, -): string | null { - if (localBranch === prHeadRefName) { - return null; - } - - const suffix = `/${prHeadRefName}`; - if (!localBranch.endsWith(suffix)) { - return null; - } - - const prefix = localBranch.slice(0, -suffix.length).trim(); - return prefix ? prefix.toLowerCase() : null; -} - -export function prMatchesLocalBranch( - localBranch: string, - pr: Pick, -): boolean { - if (!branchMatchesPR(localBranch, pr.headRefName)) { - return false; - } - - const ownerPrefix = getForkOwnerPrefix(localBranch, pr.headRefName); - if (!ownerPrefix) { - return localBranch === pr.headRefName; - } - - return pr.headRepositoryOwner?.login.toLowerCase() === ownerPrefix; -} - -function sortPRCandidates( - candidates: GHPRResponse[], - headSha?: string, -): GHPRResponse[] { - const getStateRank = (candidate: GHPRResponse): number => { - if (candidate.state === "OPEN") return 2; - if (candidate.state === "MERGED") return 1; - return 0; - }; - - return [...candidates].sort((left, right) => { - const leftMatchesHead = Number( - Boolean(headSha && left.headRefOid === headSha), - ); - const rightMatchesHead = Number( - Boolean(headSha && right.headRefOid === headSha), - ); - if (leftMatchesHead !== rightMatchesHead) { - return rightMatchesHead - leftMatchesHead; - } - - const stateDelta = getStateRank(right) - getStateRank(left); - if (stateDelta !== 0) { - return stateDelta; - } - - const leftMergedAt = left.mergedAt ? Date.parse(left.mergedAt) : 0; - const rightMergedAt = right.mergedAt ? Date.parse(right.mergedAt) : 0; - if (leftMergedAt !== rightMergedAt) { - return rightMergedAt - leftMergedAt; - } - - return right.number - left.number; - }); -} - -/** - * Looks up a PR using `gh pr view` (no args), which matches via the branch's - * tracking ref. Essential for fork PRs that track refs/pull/XXX/head. - */ -async function getPRByBranchTracking( - worktreePath: string, - localBranch: string, -): Promise { - try { - const { stdout } = await execWithShellEnv( - "gh", - ["pr", "view", "--json", PR_JSON_FIELDS], - { cwd: worktreePath }, - ); - - const data = parsePRResponse(stdout); - if (!data) { - return null; - } - - // Verify the PR's head branch matches the local branch. - // `gh pr view` can match via stale tracking refs (e.g. refs/pull/N/head) - // left over from a previous `gh pr checkout`, causing a new workspace - // to incorrectly show an old, unrelated PR. - if (!prMatchesLocalBranch(localBranch, data)) { - return null; - } - - return formatPRData(data); - } catch (error) { - if ( - error instanceof Error && - error.message.toLowerCase().includes("no pull requests found") - ) { - return null; - } - throw error; - } -} - -/** - * Looks up PRs by exact head branch name. This avoids relying on `gh pr view` - * branch inference, which can miss fork-tracked branches in some clones. - */ -async function findPRByHeadBranch( - worktreePath: string, - localBranch: string, - repoContext?: RepoContext, - headSha?: string, -): Promise { - try { - const matches = new Map(); - - for (const branchCandidate of getPRHeadBranchCandidates(localBranch)) { - const { stdout } = await execWithShellEnv( - "gh", - [ - "pr", - "list", - ...getPullRequestRepoArgs(repoContext), - "--state", - "all", - "--head", - branchCandidate, - "--limit", - "20", - "--json", - PR_JSON_FIELDS, - ], - { cwd: worktreePath }, - ); - - for (const candidate of parsePRListResponse(stdout)) { - if (prMatchesLocalBranch(localBranch, candidate)) { - matches.set(candidate.number, candidate); - } - } - } - - const bestMatch = sortPRCandidates([...matches.values()], headSha)[0]; - return bestMatch ? formatPRData(bestMatch) : null; - } catch { - return null; - } -} - -/** - * Looks up PRs that have local HEAD as their head commit. - * This avoids matching unrelated PRs that merely contain the same commit. - */ -async function findPRByHeadCommit( - worktreePath: string, - repoContext?: RepoContext, - providedSha?: string, -): Promise { - try { - let headSha = providedSha; - if (!headSha) { - const { stdout: headOutput } = await execGitWithShellPath( - ["rev-parse", "HEAD"], - { cwd: worktreePath }, - ); - headSha = headOutput.trim(); - } - if (!headSha) { - return null; - } - - const { stdout } = await execWithShellEnv( - "gh", - [ - "pr", - "list", - ...getPullRequestRepoArgs(repoContext), - "--state", - "all", - "--search", - `${headSha} is:pr`, - "--limit", - "20", - "--json", - PR_JSON_FIELDS, - ], - { cwd: worktreePath }, - ); - - const candidates = parsePRListResponse(stdout); - const exactHeadMatches = candidates.filter( - (candidate) => candidate.headRefOid === headSha, - ); - const bestMatch = sortPRCandidates(exactHeadMatches, headSha)[0]; - if (bestMatch) { - return formatPRData(bestMatch); - } - - return null; - } catch { - return null; - } -} - -function parsePRResponse(stdout: string): GHPRResponse | null { - const trimmed = stdout.trim(); - if (!trimmed || trimmed === "null") { - return null; - } - - let raw: unknown; - try { - raw = JSON.parse(trimmed); - } catch (error) { - console.warn( - "[GitHub] Failed to parse PR response JSON:", - error instanceof Error ? error.message : String(error), - ); - return null; - } - const result = GHPRResponseSchema.safeParse(raw); - if (!result.success) { - console.error("[GitHub] PR schema validation failed:", result.error); - console.error("[GitHub] Raw data:", JSON.stringify(raw, null, 2)); - return null; - } - return result.data; -} - -function parsePRListResponse(stdout: string): GHPRResponse[] { - const trimmed = stdout.trim(); - if (!trimmed || trimmed === "null") { - return []; - } - - let raw: unknown; - try { - raw = JSON.parse(trimmed); - } catch (error) { - console.warn( - "[GitHub] Failed to parse PR list response JSON:", - error instanceof Error ? error.message : String(error), - ); - return []; - } - - if (!Array.isArray(raw)) { - return []; - } - - const parsed: GHPRResponse[] = []; - for (const item of raw) { - const result = GHPRResponseSchema.safeParse(item); - if (result.success) { - parsed.push(result.data); - } - } - return parsed; -} - -function formatPRData(data: GHPRResponse): NonNullable { - return { - number: data.number, - title: data.title, - url: data.url, - state: mapPRState(data.state, data.isDraft), - mergedAt: data.mergedAt ? new Date(data.mergedAt).getTime() : undefined, - additions: data.additions, - deletions: data.deletions, - reviewDecision: mapReviewDecision(data.reviewDecision), - checksStatus: computeChecksStatus(data.statusCheckRollup), - checks: parseChecks(data.statusCheckRollup), - requestedReviewers: parseReviewRequests(data.reviewRequests), - }; -} - -function parseReviewRequests( - requests: GHPRResponse["reviewRequests"], -): string[] { - if (!requests || requests.length === 0) return []; - return requests.map((r) => r.login || r.slug || r.name || "").filter(Boolean); -} - -function mapPRState( - state: GHPRResponse["state"], - isDraft: boolean, -): NonNullable["state"] { - if (state === "MERGED") return "merged"; - if (state === "CLOSED") return "closed"; - if (isDraft) return "draft"; - return "open"; -} - -function mapReviewDecision( - decision: GHPRResponse["reviewDecision"], -): NonNullable["reviewDecision"] { - if (decision === "APPROVED") return "approved"; - if (decision === "CHANGES_REQUESTED") return "changes_requested"; - return "pending"; -} - -function parseChecks(rollup: GHPRResponse["statusCheckRollup"]): CheckItem[] { - if (!rollup || rollup.length === 0) { - return []; - } - - // GitHub returns two shapes: CheckRun (name/detailsUrl/conclusion) and - // StatusContext (context/targetUrl/state). Normalize both here. - return rollup.map((ctx) => { - const name = ctx.name || ctx.context || "Unknown check"; - const url = ctx.detailsUrl || ctx.targetUrl; - const rawStatus = ctx.state || ctx.conclusion; - - let status: CheckItem["status"]; - if (rawStatus === "SUCCESS") { - status = "success"; - } else if ( - rawStatus === "FAILURE" || - rawStatus === "ERROR" || - rawStatus === "TIMED_OUT" - ) { - status = "failure"; - } else if (rawStatus === "SKIPPED" || rawStatus === "NEUTRAL") { - status = "skipped"; - } else if (rawStatus === "CANCELLED") { - status = "cancelled"; - } else { - status = "pending"; - } - - return { name, status, url }; - }); -} - /** * Low-level helper: query deployments matching the given params and return * the environment_url of the first successful deployment. Status lookups @@ -698,33 +189,3 @@ async function fetchPreviewDeploymentUrl( return undefined; } } - -function computeChecksStatus( - rollup: GHPRResponse["statusCheckRollup"], -): NonNullable["checksStatus"] { - if (!rollup || rollup.length === 0) { - return "none"; - } - - let hasFailure = false; - let hasPending = false; - - for (const ctx of rollup) { - const status = ctx.state || ctx.conclusion; - - if (status === "FAILURE" || status === "ERROR" || status === "TIMED_OUT") { - hasFailure = true; - } else if ( - status === "PENDING" || - status === "" || - status === null || - status === undefined - ) { - hasPending = true; - } - } - - if (hasFailure) return "failure"; - if (hasPending) return "pending"; - return "success"; -} diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/index.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/index.ts index a3fca6bcd0b..66cedc65192 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/index.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/index.ts @@ -2,3 +2,10 @@ export { clearGitHubStatusCacheForWorktree, fetchGitHubPRStatus, } from "./github"; +export { getPRForBranch } from "./pr-resolution"; +export { + extractNwoFromUrl, + getPullRequestRepoArgs, + getRepoContext, + normalizeGitHubUrl, +} from "./repo-context"; diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/pr-resolution.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/pr-resolution.ts new file mode 100644 index 00000000000..8cd193d44b0 --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/pr-resolution.ts @@ -0,0 +1,428 @@ +import type { CheckItem, GitHubStatus } from "@superset/local-db"; +import { execGitWithShellPath } from "../git-client"; +import { execWithShellEnv } from "../shell-env"; +import { getPullRequestRepoArgs } from "./repo-context"; +import { + type GHPRResponse, + GHPRResponseSchema, + type RepoContext, +} from "./types"; + +const PR_JSON_FIELDS = + "number,title,url,state,isDraft,mergedAt,additions,deletions,headRefOid,headRefName,headRepositoryOwner,reviewDecision,statusCheckRollup,reviewRequests"; + +export async function getPRForBranch( + worktreePath: string, + localBranch: string, + repoContext?: RepoContext, + headSha?: string, +): Promise { + const byTracking = await getPRByBranchTracking(worktreePath, localBranch); + if (byTracking) { + return byTracking; + } + + const byHeadBranch = await findPRByHeadBranch( + worktreePath, + localBranch, + repoContext, + headSha, + ); + if (byHeadBranch) { + return byHeadBranch; + } + + return findPRByHeadCommit(worktreePath, repoContext, headSha); +} + +/** + * Returns true when the local branch name matches the PR's head branch. + * Handles fork PRs where the local branch is prefixed with the fork owner + * (e.g. local "owner/feature" matches PR headRefName "feature"). + */ +export function branchMatchesPR( + localBranch: string, + prHeadRefName: string, +): boolean { + return ( + localBranch === prHeadRefName || localBranch.endsWith(`/${prHeadRefName}`) + ); +} + +export function getPRHeadBranchCandidates(localBranch: string): string[] { + const strippedBranch = localBranch.includes("/") + ? localBranch.slice(localBranch.indexOf("/") + 1) + : localBranch; + + return Array.from(new Set([localBranch, strippedBranch].filter(Boolean))); +} + +function getForkOwnerPrefix( + localBranch: string, + prHeadRefName: string, +): string | null { + if (localBranch === prHeadRefName) { + return null; + } + + const suffix = `/${prHeadRefName}`; + if (!localBranch.endsWith(suffix)) { + return null; + } + + const prefix = localBranch.slice(0, -suffix.length).trim(); + return prefix ? prefix.toLowerCase() : null; +} + +export function prMatchesLocalBranch( + localBranch: string, + pr: Pick, +): boolean { + if (!branchMatchesPR(localBranch, pr.headRefName)) { + return false; + } + + const ownerPrefix = getForkOwnerPrefix(localBranch, pr.headRefName); + if (!ownerPrefix) { + return localBranch === pr.headRefName; + } + + return pr.headRepositoryOwner?.login.toLowerCase() === ownerPrefix; +} + +function sortPRCandidates( + candidates: GHPRResponse[], + headSha?: string, +): GHPRResponse[] { + const getStateRank = (candidate: GHPRResponse): number => { + if (candidate.state === "OPEN") return 2; + if (candidate.state === "MERGED") return 1; + return 0; + }; + + return [...candidates].sort((left, right) => { + const leftMatchesHead = Number( + Boolean(headSha && left.headRefOid === headSha), + ); + const rightMatchesHead = Number( + Boolean(headSha && right.headRefOid === headSha), + ); + if (leftMatchesHead !== rightMatchesHead) { + return rightMatchesHead - leftMatchesHead; + } + + const stateDelta = getStateRank(right) - getStateRank(left); + if (stateDelta !== 0) { + return stateDelta; + } + + const leftMergedAt = left.mergedAt ? Date.parse(left.mergedAt) : 0; + const rightMergedAt = right.mergedAt ? Date.parse(right.mergedAt) : 0; + if (leftMergedAt !== rightMergedAt) { + return rightMergedAt - leftMergedAt; + } + + return right.number - left.number; + }); +} + +/** + * Looks up a PR using `gh pr view` (no args), which matches via the branch's + * tracking ref. Essential for fork PRs that track refs/pull/XXX/head. + */ +async function getPRByBranchTracking( + worktreePath: string, + localBranch: string, +): Promise { + try { + const { stdout } = await execWithShellEnv( + "gh", + ["pr", "view", "--json", PR_JSON_FIELDS], + { cwd: worktreePath }, + ); + + const data = parsePRResponse(stdout); + if (!data) { + return null; + } + + // Verify the PR's head branch matches the local branch. + // `gh pr view` can match via stale tracking refs (e.g. refs/pull/N/head) + // left over from a previous `gh pr checkout`, causing a new workspace + // to incorrectly show an old, unrelated PR. + if (!prMatchesLocalBranch(localBranch, data)) { + return null; + } + + return formatPRData(data); + } catch (error) { + if ( + error instanceof Error && + error.message.toLowerCase().includes("no pull requests found") + ) { + return null; + } + throw error; + } +} + +/** + * Looks up PRs by exact head branch name. This avoids relying on `gh pr view` + * branch inference, which can miss fork-tracked branches in some clones. + */ +async function findPRByHeadBranch( + worktreePath: string, + localBranch: string, + repoContext?: RepoContext, + headSha?: string, +): Promise { + try { + const matches = new Map(); + + for (const branchCandidate of getPRHeadBranchCandidates(localBranch)) { + const { stdout } = await execWithShellEnv( + "gh", + [ + "pr", + "list", + ...getPullRequestRepoArgs(repoContext), + "--state", + "all", + "--head", + branchCandidate, + "--limit", + "20", + "--json", + PR_JSON_FIELDS, + ], + { cwd: worktreePath }, + ); + + for (const candidate of parsePRListResponse(stdout)) { + if (prMatchesLocalBranch(localBranch, candidate)) { + matches.set(candidate.number, candidate); + } + } + } + + const bestMatch = sortPRCandidates([...matches.values()], headSha)[0]; + return bestMatch ? formatPRData(bestMatch) : null; + } catch { + return null; + } +} + +/** + * Looks up PRs that have local HEAD as their head commit. + * This avoids matching unrelated PRs that merely contain the same commit. + */ +async function findPRByHeadCommit( + worktreePath: string, + repoContext?: RepoContext, + providedSha?: string, +): Promise { + try { + let headSha = providedSha; + if (!headSha) { + const { stdout: headOutput } = await execGitWithShellPath( + ["rev-parse", "HEAD"], + { cwd: worktreePath }, + ); + headSha = headOutput.trim(); + } + if (!headSha) { + return null; + } + + const { stdout } = await execWithShellEnv( + "gh", + [ + "pr", + "list", + ...getPullRequestRepoArgs(repoContext), + "--state", + "all", + "--search", + `${headSha} is:pr`, + "--limit", + "20", + "--json", + PR_JSON_FIELDS, + ], + { cwd: worktreePath }, + ); + + const candidates = parsePRListResponse(stdout); + const exactHeadMatches = candidates.filter( + (candidate) => candidate.headRefOid === headSha, + ); + const bestMatch = sortPRCandidates(exactHeadMatches, headSha)[0]; + if (bestMatch) { + return formatPRData(bestMatch); + } + + return null; + } catch { + return null; + } +} + +function parsePRResponse(stdout: string): GHPRResponse | null { + const trimmed = stdout.trim(); + if (!trimmed || trimmed === "null") { + return null; + } + + let raw: unknown; + try { + raw = JSON.parse(trimmed); + } catch (error) { + console.warn( + "[GitHub] Failed to parse PR response JSON:", + error instanceof Error ? error.message : String(error), + ); + return null; + } + const result = GHPRResponseSchema.safeParse(raw); + if (!result.success) { + console.error("[GitHub] PR schema validation failed:", result.error); + console.error("[GitHub] Raw data:", JSON.stringify(raw, null, 2)); + return null; + } + return result.data; +} + +function parsePRListResponse(stdout: string): GHPRResponse[] { + const trimmed = stdout.trim(); + if (!trimmed || trimmed === "null") { + return []; + } + + let raw: unknown; + try { + raw = JSON.parse(trimmed); + } catch (error) { + console.warn( + "[GitHub] Failed to parse PR list response JSON:", + error instanceof Error ? error.message : String(error), + ); + return []; + } + + if (!Array.isArray(raw)) { + return []; + } + + const parsed: GHPRResponse[] = []; + for (const item of raw) { + const result = GHPRResponseSchema.safeParse(item); + if (result.success) { + parsed.push(result.data); + } + } + return parsed; +} + +function formatPRData(data: GHPRResponse): NonNullable { + return { + number: data.number, + title: data.title, + url: data.url, + state: mapPRState(data.state, data.isDraft), + mergedAt: data.mergedAt ? new Date(data.mergedAt).getTime() : undefined, + additions: data.additions, + deletions: data.deletions, + reviewDecision: mapReviewDecision(data.reviewDecision), + checksStatus: computeChecksStatus(data.statusCheckRollup), + checks: parseChecks(data.statusCheckRollup), + requestedReviewers: parseReviewRequests(data.reviewRequests), + }; +} + +function parseReviewRequests( + requests: GHPRResponse["reviewRequests"], +): string[] { + if (!requests || requests.length === 0) return []; + return requests.map((r) => r.login || r.slug || r.name || "").filter(Boolean); +} + +function mapPRState( + state: GHPRResponse["state"], + isDraft: boolean, +): NonNullable["state"] { + if (state === "MERGED") return "merged"; + if (state === "CLOSED") return "closed"; + if (isDraft) return "draft"; + return "open"; +} + +function mapReviewDecision( + decision: GHPRResponse["reviewDecision"], +): NonNullable["reviewDecision"] { + if (decision === "APPROVED") return "approved"; + if (decision === "CHANGES_REQUESTED") return "changes_requested"; + return "pending"; +} + +function parseChecks(rollup: GHPRResponse["statusCheckRollup"]): CheckItem[] { + if (!rollup || rollup.length === 0) { + return []; + } + + // GitHub returns two shapes: CheckRun (name/detailsUrl/conclusion) and + // StatusContext (context/targetUrl/state). Normalize both here. + return rollup.map((ctx) => { + const name = ctx.name || ctx.context || "Unknown check"; + const url = ctx.detailsUrl || ctx.targetUrl; + const rawStatus = ctx.state || ctx.conclusion; + + let status: CheckItem["status"]; + if (rawStatus === "SUCCESS") { + status = "success"; + } else if ( + rawStatus === "FAILURE" || + rawStatus === "ERROR" || + rawStatus === "TIMED_OUT" + ) { + status = "failure"; + } else if (rawStatus === "SKIPPED" || rawStatus === "NEUTRAL") { + status = "skipped"; + } else if (rawStatus === "CANCELLED") { + status = "cancelled"; + } else { + status = "pending"; + } + + return { name, status, url }; + }); +} + +function computeChecksStatus( + rollup: GHPRResponse["statusCheckRollup"], +): NonNullable["checksStatus"] { + if (!rollup || rollup.length === 0) { + return "none"; + } + + let hasFailure = false; + let hasPending = false; + + for (const ctx of rollup) { + const status = ctx.state || ctx.conclusion; + + if (status === "FAILURE" || status === "ERROR" || status === "TIMED_OUT") { + hasFailure = true; + } else if ( + status === "PENDING" || + status === "" || + status === null || + status === undefined + ) { + hasPending = true; + } + } + + if (hasFailure) return "failure"; + if (hasPending) return "pending"; + return "success"; +} diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts new file mode 100644 index 00000000000..20b33c08425 --- /dev/null +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts @@ -0,0 +1,122 @@ +import { execGitWithShellPath } from "../git-client"; +import { execWithShellEnv } from "../shell-env"; +import { GHRepoResponseSchema, type RepoContext } from "./types"; + +const repoContextCache = new Map< + string, + { data: RepoContext; timestamp: number } +>(); +const REPO_CONTEXT_CACHE_TTL_MS = 300_000; // 5 minutes + +export async function getRepoContext( + worktreePath: string, +): Promise { + const cached = repoContextCache.get(worktreePath); + if (cached && Date.now() - cached.timestamp < REPO_CONTEXT_CACHE_TTL_MS) { + return cached.data; + } + + try { + const { stdout } = await execWithShellEnv( + "gh", + ["repo", "view", "--json", "url,isFork,parent"], + { cwd: worktreePath }, + ); + const raw = JSON.parse(stdout); + const result = GHRepoResponseSchema.safeParse(raw); + if (!result.success) { + console.error("[GitHub] Repo schema validation failed:", result.error); + console.error("[GitHub] Raw data:", JSON.stringify(raw, null, 2)); + return null; + } + + const data = result.data; + let context: RepoContext; + + if (data.isFork && data.parent) { + context = { + repoUrl: data.url, + upstreamUrl: data.parent.url, + isFork: true, + }; + } else { + const originUrl = await getOriginUrl(worktreePath); + const ghUrl = normalizeGitHubUrl(data.url); + + if (originUrl && ghUrl && originUrl !== ghUrl) { + context = { + repoUrl: originUrl, + upstreamUrl: ghUrl, + isFork: true, + }; + } else { + context = { + repoUrl: data.url, + upstreamUrl: data.url, + isFork: false, + }; + } + } + + repoContextCache.set(worktreePath, { + data: context, + timestamp: Date.now(), + }); + return context; + } catch { + return null; + } +} + +async function getOriginUrl(worktreePath: string): Promise { + try { + const { stdout } = await execGitWithShellPath( + ["remote", "get-url", "origin"], + { cwd: worktreePath }, + ); + return normalizeGitHubUrl(stdout.trim()); + } catch { + return null; + } +} + +export function normalizeGitHubUrl(remoteUrl: string): string | null { + const trimmed = remoteUrl.trim(); + const patterns = [ + /^git@github\.com:(?[^/]+\/[^/]+?)(?:\.git)?$/, + /^ssh:\/\/git@github\.com\/(?[^/]+\/[^/]+?)(?:\.git)?$/, + /^https:\/\/github\.com\/(?[^/]+\/[^/]+?)(?:\.git)?\/?$/, + ]; + for (const pattern of patterns) { + const match = pattern.exec(trimmed); + if (match?.groups?.nwo) { + return `https://github.com/${match.groups.nwo}`; + } + } + return null; +} + +export function extractNwoFromUrl(normalizedUrl: string): string | null { + try { + const path = new URL(normalizedUrl).pathname.slice(1); + return path || null; + } catch { + return null; + } +} + +export function getPullRequestRepoArgs( + repoContext?: Pick | null, +): string[] { + if (!repoContext?.isFork) { + return []; + } + + const normalizedUpstreamUrl = normalizeGitHubUrl(repoContext.upstreamUrl); + if (!normalizedUpstreamUrl) { + return []; + } + + const repoNameWithOwner = extractNwoFromUrl(normalizedUpstreamUrl); + return repoNameWithOwner ? ["--repo", repoNameWithOwner] : []; +} From a3c4f35f310ae1e42bb73f9130ca48f61517ee80 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Fri, 20 Mar 2026 18:23:46 -0700 Subject: [PATCH 3/4] fix(desktop): remove duplicate rebase merge helper --- apps/desktop/src/lib/trpc/routers/changes/git-operations.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts index 278d7d04717..869e39ef5e8 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/git-operations.ts @@ -77,11 +77,6 @@ async function fetchCurrentBranch(git: SimpleGit): Promise { } } -function clearWorktreeStatusCaches(worktreePath: string): void { - clearGitHubStatusCacheForWorktree(worktreePath); - clearStatusCacheForWorktree(worktreePath); -} - async function pushWithSetUpstream({ git, branch, From 0405d15f89cf105bf52aa00f2e5b904daedb5aa3 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Fri, 20 Mar 2026 19:02:12 -0700 Subject: [PATCH 4/4] fix(desktop): tighten PR merge fallback handling --- .../changes/utils/merge-pull-request.ts | 71 ++++++++----------- .../workspaces/utils/github/repo-context.ts | 4 ++ 2 files changed, 34 insertions(+), 41 deletions(-) diff --git a/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts index 539128d3515..0d563a467ee 100644 --- a/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts +++ b/apps/desktop/src/lib/trpc/routers/changes/utils/merge-pull-request.ts @@ -34,6 +34,7 @@ export async function mergePullRequest({ return runMerge(legacyMergeArgs); } + let pr: Awaited> = null; try { const [{ stdout: branchOutput }, { stdout: headOutput }] = await Promise.all([ @@ -45,48 +46,8 @@ export async function mergePullRequest({ const localBranch = branchOutput.trim(); const headSha = headOutput.trim(); - const pr = await getPRForBranch( - worktreePath, - localBranch, - repoContext, - headSha, - ); - if (!pr) { - return runMerge(legacyMergeArgs); - } - if (pr.state === "merged") { - throw new Error(PR_ALREADY_MERGED_MESSAGE); - } - if (pr.state === "closed") { - throw new Error(PR_CLOSED_MESSAGE); - } - - const args = [ - "pr", - "merge", - String(pr.number), - `--${strategy}`, - ...getPullRequestRepoArgs(repoContext), - ]; - - try { - return await runMerge(args); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - if (isNoPullRequestFoundMessage(message)) { - return runMerge(legacyMergeArgs); - } - throw error; - } + pr = await getPRForBranch(worktreePath, localBranch, repoContext, headSha); } catch (error) { - if ( - error instanceof Error && - (error.message === PR_ALREADY_MERGED_MESSAGE || - error.message === PR_CLOSED_MESSAGE) - ) { - throw error; - } - console.warn( "[git/mergePR] Explicit PR resolution failed; falling back to branch merge.", { @@ -96,4 +57,32 @@ export async function mergePullRequest({ ); return runMerge(legacyMergeArgs); } + + if (!pr) { + return runMerge(legacyMergeArgs); + } + if (pr.state === "merged") { + throw new Error(PR_ALREADY_MERGED_MESSAGE); + } + if (pr.state === "closed") { + throw new Error(PR_CLOSED_MESSAGE); + } + + const args = [ + "pr", + "merge", + String(pr.number), + `--${strategy}`, + ...getPullRequestRepoArgs(repoContext), + ]; + + try { + return await runMerge(args); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + if (isNoPullRequestFoundMessage(message)) { + return runMerge(legacyMergeArgs); + } + throw error; + } } diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts index 20b33c08425..75bef322e2a 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/github/repo-context.ts @@ -43,6 +43,10 @@ export async function getRepoContext( const originUrl = await getOriginUrl(worktreePath); const ghUrl = normalizeGitHubUrl(data.url); + if (data.isFork) { + return null; + } + if (originUrl && ghUrl && originUrl !== ghUrl) { context = { repoUrl: originUrl,