Skip to content

revert: undo PR checkout fork support and unique branch naming #937

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ashwin-ant merged 1 commit into from
Feb 12, 2026
Merged

revert: undo PR checkout fork support and unique branch naming #937

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 6 additions & 21 deletions src/github/operations/branch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,7 @@ export async function setupBranch(
// Handle open PR: Checkout the PR branch
console.log("This is an open PR, checking out PR branch...");

const originalBranchName = prData.headRefName;
// Use unique local branch name to avoid conflicts with existing branches
// Format: pr-{number} ensures uniqueness since PR numbers are unique per repo
const localBranchName = `pr-${entityNumber}`;
const branchName = prData.headRefName;

// Determine optimal fetch depth based on PR commit count, with a minimum of 20
const commitCount = prData.commits.totalCount;
Expand All @@ -163,36 +160,24 @@ export async function setupBranch(
console.log(
`PR #${entityNumber}: ${commitCount} commits, using fetch depth ${fetchDepth}`,
);
console.log(
`Fetching PR branch '${originalBranchName}' as local branch '${localBranchName}'`,
);

// Validate branch names before use to prevent command injection
validateBranchName(localBranchName);
validateBranchName(branchName);

// Execute git commands to checkout PR branch (dynamic depth based on PR size)
// Using execFileSync instead of shell template literals for security
// Use GitHub's PR ref which works for both same-repo and fork PRs
// The local branch name uses pr-{number} format to ensure uniqueness
execGit([
"fetch",
"origin",
`--depth=${fetchDepth}`,
`pull/${entityNumber}/head:${localBranchName}`,
]);
execGit(["checkout", localBranchName, "--"]);
execGit(["fetch", "origin", `--depth=${fetchDepth}`, branchName]);
Copy link
Contributor

@claude claude bot Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reverts the pull/{entityNumber}/head:localBranchName refspec back to fetching branchName directly from origin. Since headRefName for a fork PR refers to a branch that only exists on the fork's remote (not on origin), this will fail for fork PRs with:

fatal: couldn't find remote ref <branch-name>

This is the exact bug that PR #851 originally fixed. If this is an intentional decision to drop fork PR support, it would be worth documenting that limitation (e.g., in the FAQ or a code comment here). If fork PRs are expected to work, this line should continue using the pull/{number}/head refspec.

execGit(["checkout", branchName, "--"]);

console.log(
`Successfully checked out PR #${entityNumber} as local branch '${localBranchName}'`,
);
console.log(`Successfully checked out PR branch for PR #${entityNumber}`);

// For open PRs, we need to get the base branch of the PR
const baseBranch = prData.baseRefName;
validateBranchName(baseBranch);

return {
baseBranch,
currentBranch: localBranchName,
currentBranch: branchName,
};
}
}
Expand Down
Loading