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
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
branchMatchesPR,
getPRHeadBranchCandidates,
prMatchesLocalBranch,
shouldAcceptPRMatch,
} from "./pr-resolution";
import {
getPullRequestRepoArgs,
Expand Down Expand Up @@ -407,6 +408,53 @@ describe("prMatchesLocalBranch", () => {
});
});

describe("shouldAcceptPRMatch", () => {
test("keeps open PR matches even when local HEAD differs", () => {
expect(
shouldAcceptPRMatch({
localBranch: "feature/my-thing",
headSha: "local-head-sha",
pr: {
headRefName: "feature/my-thing",
headRefOid: "remote-head-sha",
headRepositoryOwner: null,
state: "OPEN",
},
}),
).toBe(true);
});

test("rejects historical PR matches when the head commit differs", () => {
expect(
shouldAcceptPRMatch({
localBranch: "feature/my-thing",
headSha: "new-head-sha",
pr: {
headRefName: "feature/my-thing",
headRefOid: "old-pr-head-sha",
headRepositoryOwner: null,
state: "MERGED",
},
}),
).toBe(false);
});

test("accepts historical PR matches when the head commit still matches", () => {
expect(
shouldAcceptPRMatch({
localBranch: "feature/my-thing",
headSha: "same-head-sha",
pr: {
headRefName: "feature/my-thing",
headRefOid: "same-head-sha",
headRepositoryOwner: null,
state: "MERGED",
},
}),
).toBe(true);
});
});

describe("resolveRemoteBranchNameForGitHubStatus", () => {
test("prefers the tracked upstream branch name", () => {
expect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ export async function getPRForBranch(
repoContext?: RepoContext,
headSha?: string,
): Promise<GitHubStatus["pr"]> {
const byTracking = await getPRByBranchTracking(worktreePath, localBranch);
const byTracking = await getPRByBranchTracking(
worktreePath,
localBranch,
headSha,
);
if (byTracking) {
return byTracking;
}
Expand Down Expand Up @@ -90,6 +94,36 @@ export function prMatchesLocalBranch(
return pr.headRepositoryOwner?.login?.toLowerCase() === ownerPrefix;
}

function isHistoricalPullRequestState(state: GHPRResponse["state"]): boolean {
return state === "CLOSED" || state === "MERGED";
}

export function shouldAcceptPRMatch({
localBranch,
pr,
headSha,
}: {
localBranch: string;
pr: Pick<
GHPRResponse,
"headRefName" | "headRefOid" | "headRepositoryOwner" | "state"
>;
headSha?: string;
}): boolean {
if (!prMatchesLocalBranch(localBranch, pr)) {
return false;
}

// Historical PRs should only attach when this workspace still points at the
// exact PR head commit. Otherwise, reusing a branch name can surface an old,
// unrelated closed or merged PR.
if (headSha && isHistoricalPullRequestState(pr.state)) {
return pr.headRefOid === headSha;
}

return true;
}

function sortPRCandidates(
candidates: GHPRResponse[],
headSha?: string,
Expand Down Expand Up @@ -133,6 +167,7 @@ function sortPRCandidates(
async function getPRByBranchTracking(
worktreePath: string,
localBranch: string,
headSha?: string,
): Promise<GitHubStatus["pr"]> {
try {
const { stdout } = await execWithShellEnv(
Expand All @@ -150,7 +185,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 (!prMatchesLocalBranch(localBranch, data)) {
if (!shouldAcceptPRMatch({ localBranch, pr: data, headSha })) {
return null;
}

Expand Down Expand Up @@ -199,7 +234,7 @@ async function findPRByHeadBranch(
);

for (const candidate of parsePRListResponse(stdout)) {
if (prMatchesLocalBranch(localBranch, candidate)) {
if (shouldAcceptPRMatch({ localBranch, pr: candidate, headSha })) {
matches.set(candidate.number, candidate);
}
}
Expand Down
Loading