Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion apps/desktop/src/lib/trpc/routers/changes/git-operations.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { describe, expect, test } from "bun:test";
import { isUpstreamMissingError } from "./git-utils";
import {
isNoPullRequestFoundMessage,
isUpstreamMissingError,
} from "./git-utils";
import {
getExistingPRHeadRepoUrl,
resolveRemoteNameForExistingPRHead,
Expand Down Expand Up @@ -35,6 +38,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);
Expand Down
31 changes: 18 additions & 13 deletions apps/desktop/src/lib/trpc/routers/changes/git-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,27 @@ import {
fetchGitHubPRStatus,
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 { isUpstreamMissingError } from "./git-utils";
import {
isNoPullRequestFoundMessage,
isUpstreamMissingError,
} from "./git-utils";
import { assertRegisteredWorktree } from "./security/path-validation";
import {
type GitRemoteInfo,
isOpenPullRequestState,
resolveRemoteNameForExistingPRHead,
} from "./utils/existing-pr-push-target";
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 };

Expand Down Expand Up @@ -88,11 +93,6 @@ async function fetchCurrentBranch(git: SimpleGit): Promise<void> {
}
}

function clearWorktreeStatusCaches(worktreePath: string): void {
clearGitHubStatusCacheForWorktree(worktreePath);
clearStatusCacheForWorktree(worktreePath);
}

async function pushWithSetUpstream({
git,
targetBranch,
Expand Down Expand Up @@ -707,23 +707,28 @@ export const createGitOperationsRouter = () => {
async ({ input }): Promise<{ success: boolean; mergedAt?: string }> => {
assertRegisteredWorktree(input.worktreePath);

const args = ["pr", "merge", `--${input.strategy}`];

try {
await execWithShellEnv("gh", args, { cwd: input.worktreePath });
clearWorktreeStatusCaches(input.worktreePath);
return { success: true, mergedAt: new Date().toISOString() };
return await mergePullRequest(input);
} catch (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",
});
}
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")
Expand Down
4 changes: 4 additions & 0 deletions apps/desktop/src/lib/trpc/routers/changes/git-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
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);
}

let pr: Awaited<ReturnType<typeof getPRForBranch>> = null;
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();

pr = await getPRForBranch(worktreePath, localBranch, repoContext, headSha);
} catch (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);
}

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;
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { describe, expect, test } from "bun:test";
import { resolveRemoteBranchNameForGitHubStatus } from "./github";
import {
branchMatchesPR,
getPullRequestRepoArgs,
resolveRemoteBranchNameForGitHubStatus,
} from "./github";
getPRHeadBranchCandidates,
prMatchesLocalBranch,
} from "./pr-resolution";
import { getPullRequestRepoArgs } from "./repo-context";

describe("branchMatchesPR", () => {
test("matches same-repo branch exactly", () => {
Expand Down Expand Up @@ -62,6 +64,57 @@ describe("getPullRequestRepoArgs", () => {
});
});

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);
});
});

describe("resolveRemoteBranchNameForGitHubStatus", () => {
test("prefers the tracked upstream branch name", () => {
expect(
Expand Down
Loading
Loading