diff --git a/.changeset/fix-pr-url-temp-repos.md b/.changeset/fix-pr-url-temp-repos.md new file mode 100644 index 0000000..d691357 --- /dev/null +++ b/.changeset/fix-pr-url-temp-repos.md @@ -0,0 +1,5 @@ +--- +"@jaydenfyi/diffx": patch +--- + +Resolve GitHub PR URLs in an isolated temporary Git repository so URL diffs work outside an existing Git checkout and support sandbox temp roots via `DIFFX_TMPDIR`. diff --git a/package.json b/package.json index 0c5aa0b..a916ab3 100644 --- a/package.json +++ b/package.json @@ -10,16 +10,16 @@ "patch", "pull-request" ], + "homepage": "https://github.com/jaydenfyi/diffx#readme", + "bugs": { + "url": "https://github.com/jaydenfyi/diffx/issues" + }, "license": "MIT", "author": "Jayden", "repository": { "type": "git", "url": "git+https://github.com/jaydenfyi/diffx.git" }, - "homepage": "https://github.com/jaydenfyi/diffx#readme", - "bugs": { - "url": "https://github.com/jaydenfyi/diffx/issues" - }, "bin": { "diffx": "./dist/bin.mjs" }, diff --git a/src/cli/command-types.ts b/src/cli/command-types.ts index f03b7a7..643469a 100644 --- a/src/cli/command-types.ts +++ b/src/cli/command-types.ts @@ -1,10 +1,12 @@ import type { PatchStyle, FilterOptions } from "../types"; +import type { GitClient } from "../git/git-client"; export interface ResolvedRefs { left: string; right: string; cleanup?: () => Promise; patchStyle?: PatchStyle; + gitClient?: GitClient; } export type FileFilterOptions = FilterOptions; diff --git a/src/cli/command.ts b/src/cli/command.ts index ba4c10d..0bd0ecb 100644 --- a/src/cli/command.ts +++ b/src/cli/command.ts @@ -73,6 +73,9 @@ async function generateDiffOutput( ): Promise { const { left, right, patchStyle } = refs; if (right) { + if (refs.gitClient) { + return generateOutput(mode, left, right, diffOptions, patchStyle, refs.gitClient); + } return generateOutput(mode, left, right, diffOptions, patchStyle); } diff --git a/src/cli/git-pass-through.ts b/src/cli/git-pass-through.ts index 6c74039..839c6f1 100644 --- a/src/cli/git-pass-through.ts +++ b/src/cli/git-pass-through.ts @@ -19,6 +19,7 @@ export async function runGitPassThrough({ noPager: boolean | undefined; }): Promise { let cleanup: (() => Promise) | undefined; + let diffGitClient = gitClient; let left = ""; let right = ""; @@ -46,6 +47,7 @@ export async function runGitPassThrough({ left = resolved.left; right = resolved.right; cleanup = resolved.cleanup; + diffGitClient = resolved.gitClient ?? gitClient; } } else if (useGitCompat) { left = ""; @@ -63,7 +65,7 @@ export async function runGitPassThrough({ const fullGitArgs = [...partitioned.gitArgs, ...gitDiffArgs]; try { - const result = await gitClient.runGitDiffRaw(fullGitArgs); + const result = await diffGitClient.runGitDiffRaw(fullGitArgs); if (result.exitCode !== 0) { const message = result.stderr.trim().length > 0 ? result.stderr : "git diff failed"; throw new DiffxError(message, ExitCode.GIT_ERROR); diff --git a/src/git/git-client.ts b/src/git/git-client.ts index 45665d7..07c2a3c 100644 --- a/src/git/git-client.ts +++ b/src/git/git-client.ts @@ -3,14 +3,18 @@ */ import git from "simple-git"; -import type { StatusResult } from "simple-git"; +import type { SimpleGit, StatusResult } from "simple-git"; import type { GitDiffOptions, GitRemote } from "./types"; /** * Git client wrapper for diffx operations */ export class GitClient { - private readonly git = git(); + private readonly git: SimpleGit; + + constructor(baseDir?: string) { + this.git = baseDir ? git({ baseDir }) : git(); + } private buildColorFlag(color: "always" | "never" | "auto" | undefined): string[] { return color ? [`--color=${color}`] : []; @@ -300,6 +304,13 @@ export class GitClient { await this.git.raw(args); } + /** + * Initialize the client base directory as a bare repository. + */ + async initBare(): Promise { + await this.git.raw(["init", "--bare"]); + } + /** * Fetch a specific PR reference (without depth limit to get merge history) */ diff --git a/src/git/temp-git-repo.test.ts b/src/git/temp-git-repo.test.ts new file mode 100644 index 0000000..277524c --- /dev/null +++ b/src/git/temp-git-repo.test.ts @@ -0,0 +1,91 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mockedFn } from "vitest-mock-extended"; +import { createTemporaryGitClient } from "./temp-git-repo"; +import { mkdtemp, mkdir, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { GitClient } from "./git-client"; + +vi.mock("node:fs/promises", () => ({ + mkdtemp: vi.fn(), + mkdir: vi.fn(), + rm: vi.fn(), +})); + +vi.mock("node:os", () => ({ + tmpdir: vi.fn(), +})); + +const gitClientMocks = vi.hoisted(() => ({ + initBare: vi.fn(), +})); + +vi.mock("./git-client", () => ({ + GitClient: vi.fn(function GitClient() { + return gitClientMocks; + }), +})); + +describe("createTemporaryGitClient", () => { + const originalDiffxTmpdir = process.env.DIFFX_TMPDIR; + const mockMkdtemp = mockedFn(mkdtemp); + const mockMkdir = mockedFn(mkdir); + const mockRm = mockedFn(rm); + const mockTmpdir = mockedFn(tmpdir); + const mockGitClient = mockedFn(GitClient); + const mockInitBare = mockedFn(gitClientMocks.initBare); + + beforeEach(() => { + vi.clearAllMocks(); + delete process.env.DIFFX_TMPDIR; + mockTmpdir.mockReturnValue("/tmp"); + mockMkdir.mockResolvedValue(undefined); + mockMkdtemp.mockResolvedValue("/tmp/diffx-abc"); + mockRm.mockResolvedValue(undefined); + mockInitBare.mockResolvedValue(undefined); + }); + + afterEach(() => { + if (originalDiffxTmpdir === undefined) { + delete process.env.DIFFX_TMPDIR; + } else { + process.env.DIFFX_TMPDIR = originalDiffxTmpdir; + } + }); + + it("creates a bare repository under the OS temp directory by default", async () => { + const result = await createTemporaryGitClient(); + + expect(mockMkdir).toHaveBeenCalledWith("/tmp", { recursive: true }); + expect(mockMkdtemp).toHaveBeenCalledWith("/tmp/diffx-"); + expect(mockGitClient).toHaveBeenCalledWith("/tmp/diffx-abc"); + expect(mockInitBare).toHaveBeenCalled(); + expect(result.gitClient).toBe(gitClientMocks); + }); + + it("uses DIFFX_TMPDIR as the temp root when provided", async () => { + process.env.DIFFX_TMPDIR = "/workspace/.diffx-tmp"; + mockMkdtemp.mockResolvedValue("/workspace/.diffx-tmp/diffx-abc"); + + await createTemporaryGitClient(); + + expect(mockMkdir).toHaveBeenCalledWith("/workspace/.diffx-tmp", { recursive: true }); + expect(mockMkdtemp).toHaveBeenCalledWith("/workspace/.diffx-tmp/diffx-"); + expect(mockGitClient).toHaveBeenCalledWith("/workspace/.diffx-tmp/diffx-abc"); + }); + + it("removes the temporary repo during cleanup", async () => { + const result = await createTemporaryGitClient(); + + await result.cleanup(); + + expect(mockRm).toHaveBeenCalledWith("/tmp/diffx-abc", { recursive: true, force: true }); + }); + + it("removes the temporary repo when bare init fails", async () => { + mockInitBare.mockRejectedValue(new Error("init failed")); + + await expect(createTemporaryGitClient()).rejects.toThrow("init failed"); + + expect(mockRm).toHaveBeenCalledWith("/tmp/diffx-abc", { recursive: true, force: true }); + }); +}); diff --git a/src/git/temp-git-repo.ts b/src/git/temp-git-repo.ts new file mode 100644 index 0000000..29ff5da --- /dev/null +++ b/src/git/temp-git-repo.ts @@ -0,0 +1,37 @@ +import { mkdtemp, mkdir, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { GitClient } from "./git-client"; + +const TMP_PREFIX = "diffx-"; + +export type TemporaryGitClient = { + gitClient: GitClient; + cleanup: () => Promise; +}; + +function getTempRoot(): string { + return process.env.DIFFX_TMPDIR || tmpdir(); +} + +export async function createTemporaryGitClient(): Promise { + const tempRoot = getTempRoot(); + await mkdir(tempRoot, { recursive: true }); + + const repoPath = await mkdtemp(join(tempRoot, TMP_PREFIX)); + const gitClient = new GitClient(repoPath); + + try { + await gitClient.initBare(); + } catch (error) { + await rm(repoPath, { recursive: true, force: true }); + throw error; + } + + return { + gitClient, + cleanup: async () => { + await rm(repoPath, { recursive: true, force: true }); + }, + }; +} diff --git a/src/output/output-factory.ts b/src/output/output-factory.ts index 94f02bc..747b1d3 100644 --- a/src/output/output-factory.ts +++ b/src/output/output-factory.ts @@ -4,6 +4,7 @@ */ import type { OutputMode, PatchStyle } from "../types"; +import type { GitClient } from "../git/git-client"; import { gitClient } from "../git/git-client"; import { generatePatch } from "./patch-generator"; import { GitDiffOptions } from "../git/types"; @@ -13,24 +14,50 @@ type OutputGeneratorFn = ( right: string, options: GitDiffOptions | undefined, patchStyle?: PatchStyle, + client?: GitClient, ) => Promise; const outputGeneratorsByMode = { - diff: (left: string, right: string, options: GitDiffOptions | undefined) => - gitClient.diff(left, right, options), + diff: (left: string, right: string, options: GitDiffOptions | undefined, _patchStyle, client) => + (client ?? gitClient).diff(left, right, options), patch: generatePatch as OutputGeneratorFn, - stat: (left: string, right: string, options: GitDiffOptions | undefined) => - gitClient.diffStat(left, right, options), - numstat: (left: string, right: string, options: GitDiffOptions | undefined) => - gitClient.diffNumStat(left, right, options), - shortstat: (left: string, right: string, options: GitDiffOptions | undefined) => - gitClient.diffShortStat(left, right, options), - "name-only": (left: string, right: string, options: GitDiffOptions | undefined) => - gitClient.diffNameOnly(left, right, options), - "name-status": (left: string, right: string, options: GitDiffOptions | undefined) => - gitClient.diffNameStatus(left, right, options), - summary: (left: string, right: string, options: GitDiffOptions | undefined) => - gitClient.diffSummary(left, right, options), + stat: (left: string, right: string, options: GitDiffOptions | undefined, _patchStyle, client) => + (client ?? gitClient).diffStat(left, right, options), + numstat: ( + left: string, + right: string, + options: GitDiffOptions | undefined, + _patchStyle, + client, + ) => (client ?? gitClient).diffNumStat(left, right, options), + shortstat: ( + left: string, + right: string, + options: GitDiffOptions | undefined, + _patchStyle, + client, + ) => (client ?? gitClient).diffShortStat(left, right, options), + "name-only": ( + left: string, + right: string, + options: GitDiffOptions | undefined, + _patchStyle, + client, + ) => (client ?? gitClient).diffNameOnly(left, right, options), + "name-status": ( + left: string, + right: string, + options: GitDiffOptions | undefined, + _patchStyle, + client, + ) => (client ?? gitClient).diffNameStatus(left, right, options), + summary: ( + left: string, + right: string, + options: GitDiffOptions | undefined, + _patchStyle, + client, + ) => (client ?? gitClient).diffSummary(left, right, options), } as const satisfies Record; type OutputGeneratorAgainstWorktreeFn = ( @@ -66,9 +93,10 @@ export async function generateOutput( right: string, options: GitDiffOptions | undefined, patchStyle: PatchStyle | undefined, + client?: GitClient, ): Promise { const generator = outputGeneratorsByMode[mode]; - return generator(left, right, options, patchStyle); + return generator(left, right, options, patchStyle, client); } /** diff --git a/src/output/patch-generator.ts b/src/output/patch-generator.ts index 2b86e32..232079a 100644 --- a/src/output/patch-generator.ts +++ b/src/output/patch-generator.ts @@ -4,6 +4,7 @@ */ import type { GitDiffOptions } from "../git/types"; +import type { GitClient } from "../git/git-client"; import { gitClient } from "../git/git-client"; import type { PatchStyle } from "../types"; @@ -15,10 +16,11 @@ export async function generatePatch( right: string, options: GitDiffOptions | undefined, patchStyle: PatchStyle = "diff", + client: GitClient = gitClient, ): Promise { if (patchStyle === "diff") { - return gitClient.diff(left, right, options); + return client.diff(left, right, options); } - return gitClient.formatPatch(left, right, options); + return client.formatPatch(left, right, options); } diff --git a/src/resolvers/pr-url-resolver.test.ts b/src/resolvers/pr-url-resolver.test.ts index 946395f..a9f6b71 100644 --- a/src/resolvers/pr-url-resolver.test.ts +++ b/src/resolvers/pr-url-resolver.test.ts @@ -15,6 +15,23 @@ import { gitClient } from "../git/git-client"; import type { RefRange } from "../types"; import { DiffxError, ExitCode } from "../types"; +const tempRepoMocks = vi.hoisted(() => { + const tempGitClient = { + fetchFromUrl: vi.fn(), + deleteRefs: vi.fn(), + mergeBase: vi.fn(), + }; + + return { + tempGitClient, + cleanupTempRepo: vi.fn(), + createTemporaryGitClient: vi.fn(async () => ({ + gitClient: tempGitClient, + cleanup: vi.fn(), + })), + }; +}); + // Mock dependencies vi.mock("../git/git-client", () => ({ gitClient: { @@ -29,20 +46,34 @@ vi.mock("../git/utils", () => ({ createTempRefPrefix: () => "refs/diffx/tmp/pr-test", })); +vi.mock("../git/temp-git-repo", () => ({ + createTemporaryGitClient: tempRepoMocks.createTemporaryGitClient, +})); + const mockFetchFromUrl = mockedFn(gitClient.fetchFromUrl); const mockDeleteRefs = mockedFn(gitClient.deleteRefs); const mockMergeBase = mockedFn(gitClient.mergeBase); +const mockTempFetchFromUrl = mockedFn(tempRepoMocks.tempGitClient.fetchFromUrl); +const mockTempDeleteRefs = mockedFn(tempRepoMocks.tempGitClient.deleteRefs); +const mockCreateTemporaryGitClient = mockedFn(tempRepoMocks.createTemporaryGitClient); describe("resolvePRRefs", () => { beforeEach(() => { vi.clearAllMocks(); mockDeleteRefs.mockResolvedValue(undefined); + mockTempDeleteRefs.mockResolvedValue(undefined); + mockTempFetchFromUrl.mockResolvedValue(undefined); + tempRepoMocks.cleanupTempRepo.mockResolvedValue(undefined); + mockCreateTemporaryGitClient.mockResolvedValue({ + gitClient: tempRepoMocks.tempGitClient, + cleanup: tempRepoMocks.cleanupTempRepo, + }); }); describe("happy path", () => { - it("should resolve PR refs using merge ref logic", async () => { - mockFetchFromUrl.mockResolvedValue(undefined); + it("should resolve PR refs using an isolated git client", async () => { + mockTempFetchFromUrl.mockResolvedValue(undefined); const range: RefRange = { type: "pr-ref", @@ -55,7 +86,8 @@ describe("resolvePRRefs", () => { const result = await resolvePRRefs(range); - expect(mockFetchFromUrl).toHaveBeenCalledWith( + expect(mockFetchFromUrl).not.toHaveBeenCalled(); + expect(mockTempFetchFromUrl).toHaveBeenCalledWith( "https://github.com/owner/repo.git", [ "refs/pull/123/head:refs/diffx/tmp/pr-test/pull/123/head", @@ -67,13 +99,12 @@ describe("resolvePRRefs", () => { expect(result).toEqual({ left: "refs/diffx/tmp/pr-test/pull/123/merge^1", // First parent of merge right: "refs/diffx/tmp/pr-test/pull/123/merge", + gitClient: tempRepoMocks.tempGitClient, cleanup: expect.any(Function), }); }); it("should handle PR from different owner", async () => { - mockFetchFromUrl.mockResolvedValue(undefined); - const range: RefRange = { type: "pr-ref", rangeSyntax: undefined, @@ -85,7 +116,7 @@ describe("resolvePRRefs", () => { const result = await resolvePRRefs(range); - expect(mockFetchFromUrl).toHaveBeenCalledWith( + expect(mockTempFetchFromUrl).toHaveBeenCalledWith( "https://github.com/octocat/Hello-World.git", [ "refs/pull/456/head:refs/diffx/tmp/pr-test/pull/456/head", @@ -97,9 +128,43 @@ describe("resolvePRRefs", () => { expect(result.right).toContain("merge"); }); - it("should call cleanup to delete temp refs", async () => { - mockFetchFromUrl.mockResolvedValue(undefined); + it("should fall back to merged commit metadata when GitHub has no PR merge ref", async () => { + mockTempFetchFromUrl + .mockRejectedValueOnce(new Error("couldn't find remote ref refs/pull/123/merge")) + .mockResolvedValueOnce(undefined); + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + ok: true, + json: async () => ({ + base: { ref: "main" }, + merge_commit_sha: "merge-sha", + }), + })), + ); + + const range: RefRange = { + type: "pr-ref", + rangeSyntax: undefined, + left: "", + right: "", + ownerRepo: "owner/repo", + prNumber: 123, + }; + + const result = await resolvePRRefs(range); + expect(mockTempFetchFromUrl).toHaveBeenNthCalledWith( + 2, + "https://github.com/owner/repo.git", + ["merge-sha:refs/diffx/tmp/pr-test/pull/123/merge"], + 2, + ); + expect(result.left).toBe("refs/diffx/tmp/pr-test/pull/123/merge^1"); + expect(result.right).toBe("refs/diffx/tmp/pr-test/pull/123/merge"); + }); + + it("should call cleanup to remove the temp repo", async () => { const range: RefRange = { type: "pr-ref", rangeSyntax: undefined, @@ -112,10 +177,8 @@ describe("resolvePRRefs", () => { const result = await resolvePRRefs(range); await result.cleanup!(); - expect(mockDeleteRefs).toHaveBeenCalledWith([ - "refs/diffx/tmp/pr-test/pull/123/head", - "refs/diffx/tmp/pr-test/pull/123/merge", - ]); + expect(tempRepoMocks.cleanupTempRepo).toHaveBeenCalled(); + expect(mockDeleteRefs).not.toHaveBeenCalled(); }); }); @@ -162,7 +225,7 @@ describe("resolvePRRefs", () => { }); it("should wrap fetch errors", async () => { - mockFetchFromUrl.mockRejectedValue(new Error("PR not found")); + mockTempFetchFromUrl.mockRejectedValue(new Error("PR not found")); const range: RefRange = { type: "pr-ref", @@ -179,6 +242,7 @@ describe("resolvePRRefs", () => { } catch (error) { expect((error as DiffxError).message).toContain("Failed to fetch PR refs"); expect((error as DiffxError).exitCode).toBe(ExitCode.GIT_ERROR); + expect(tempRepoMocks.cleanupTempRepo).toHaveBeenCalled(); } }); }); diff --git a/src/resolvers/pr-url-resolver.ts b/src/resolvers/pr-url-resolver.ts index a702858..ccf4494 100644 --- a/src/resolvers/pr-url-resolver.ts +++ b/src/resolvers/pr-url-resolver.ts @@ -4,33 +4,118 @@ */ import type { GitHubPRUrl, RefRange } from "../types"; +import type { GitClient } from "../git/git-client"; import { gitClient } from "../git/git-client"; +import { createTemporaryGitClient } from "../git/temp-git-repo"; import { buildGitHubUrl, createTempRefPrefix } from "../git/utils"; import { DiffxError, ExitCode } from "../types"; type PRResolvedRefs = { headRef: string; - mergeRef: string; + leftRef: string; + rightRef: string; cleanupRefs: string[]; }; +type GitHubPRMetadata = { + baseRef: string; + mergeCommitSha?: string; +}; + +async function fetchGitHubPRMetadata( + owner: string, + repo: string, + prNumber: number, +): Promise { + const headers: Record = { + Accept: "application/vnd.github+json", + "User-Agent": "diffx", + }; + const token = process.env.GITHUB_TOKEN; + if (token) { + headers.Authorization = `Bearer ${token}`; + } + + const response = await fetch(`https://api.github.com/repos/${owner}/${repo}/pulls/${prNumber}`, { + headers, + }); + if (!response.ok) { + throw new Error(`GitHub PR metadata request failed: ${response.status}`); + } + + const data = (await response.json()) as { + base?: { ref?: string }; + merge_commit_sha?: string | null; + }; + if (!data.base?.ref) { + throw new Error("GitHub PR metadata response did not include base ref"); + } + + return { + baseRef: data.base.ref, + mergeCommitSha: data.merge_commit_sha ?? undefined, + }; +} + async function fetchPRRefs(pr: GitHubPRUrl, tempPrefix: string): Promise { + return fetchPRRefsWithClient(pr, tempPrefix, gitClient); +} + +async function fetchPRRefsWithClient( + pr: GitHubPRUrl, + tempPrefix: string, + client: GitClient, +): Promise { const { owner, repo, prNumber } = pr; const remoteUrl = buildGitHubUrl(owner, repo); const headRef = `${tempPrefix}/pull/${prNumber}/head`; const mergeRef = `${tempPrefix}/pull/${prNumber}/merge`; + const baseRef = `${tempPrefix}/pull/${prNumber}/base`; + + try { + await client.fetchFromUrl( + remoteUrl, + [`refs/pull/${prNumber}/head:${headRef}`, `refs/pull/${prNumber}/merge:${mergeRef}`], + 2, + ); + + return { + headRef, + leftRef: `${mergeRef}^1`, + rightRef: mergeRef, + cleanupRefs: [headRef, mergeRef], + }; + } catch (error) { + if ( + !(error as Error).message.includes("refs/pull") || + !(error as Error).message.includes("merge") + ) { + throw error; + } + } + + const metadata = await fetchGitHubPRMetadata(owner, repo, prNumber); + + if (metadata.mergeCommitSha) { + await client.fetchFromUrl(remoteUrl, [`${metadata.mergeCommitSha}:${mergeRef}`], 2); + + return { + headRef, + leftRef: `${mergeRef}^1`, + rightRef: mergeRef, + cleanupRefs: [mergeRef], + }; + } - await gitClient.fetchFromUrl( - remoteUrl, - [`refs/pull/${prNumber}/head:${headRef}`, `refs/pull/${prNumber}/merge:${mergeRef}`], - 2, - ); + await client.fetchFromUrl(remoteUrl, [`refs/heads/${metadata.baseRef}:${baseRef}`], 200); + await client.fetchFromUrl(remoteUrl, [`refs/pull/${prNumber}/head:${headRef}`], 200); return { headRef, - mergeRef, - cleanupRefs: [headRef, mergeRef], + leftRef: baseRef, + rightRef: headRef, + cleanupRefs: [headRef, baseRef], }; } @@ -41,30 +126,48 @@ export async function resolvePRRefs(range: RefRange): Promise<{ left: string; right: string; cleanup?: () => Promise; + gitClient?: GitClient; }> { if (!range.ownerRepo || range.prNumber === undefined) { throw new DiffxError("Invalid PR ref", ExitCode.INVALID_INPUT); } + let tempRepo: Awaited> | undefined; + try { const [owner, repo] = range.ownerRepo.split("/"); if (!owner || !repo) { throw new DiffxError(`Invalid owner/repo: ${range.ownerRepo}`, ExitCode.INVALID_INPUT); } + tempRepo = await createTemporaryGitClient(); + const resolvedTempRepo = tempRepo; const tempPrefix = createTempRefPrefix(); - const refs = await fetchPRRefs({ owner, repo, prNumber: range.prNumber }, tempPrefix); + const refs = await fetchPRRefsWithClient( + { owner, repo, prNumber: range.prNumber }, + tempPrefix, + resolvedTempRepo.gitClient, + ); // The merge ref is the PR head merged into the base branch // Diffing merge^1..merge yields the PR changes (mirrors GitHub "Files changed") return { - left: `${refs.mergeRef}^1`, // The first parent of the merge commit (the base branch) - right: refs.mergeRef, + left: refs.leftRef, + right: refs.rightRef, + gitClient: resolvedTempRepo.gitClient, cleanup: async () => { - await gitClient.deleteRefs(refs.cleanupRefs); + await resolvedTempRepo.cleanup(); }, }; } catch (error) { + if (tempRepo) { + try { + await tempRepo.cleanup(); + } catch { + // Preserve the original resolution error. + } + } + throw new DiffxError( `Failed to fetch PR refs: ${(error as Error).message}`, ExitCode.GIT_ERROR, diff --git a/src/resolvers/ref-resolver.ts b/src/resolvers/ref-resolver.ts index 9271952..d91e680 100644 --- a/src/resolvers/ref-resolver.ts +++ b/src/resolvers/ref-resolver.ts @@ -4,6 +4,7 @@ */ import type { RefRange, RefType } from "../types"; +import type { GitClient } from "../git/git-client"; import { resolveLocalRefs } from "./local-ref-resolver"; import { resolveRemoteRefs } from "./remote-ref-resolver"; import { @@ -33,6 +34,7 @@ const resolversByRefRangeType = { left: string; right: string; cleanup?: () => Promise; + gitClient?: GitClient; }> >; /** @@ -42,6 +44,7 @@ export async function resolveRefs(range: RefRange): Promise<{ left: string; right: string; cleanup?: () => Promise; + gitClient?: GitClient; }> { const resolver = resolversByRefRangeType[range.type];