Clean up pr naming

This commit is contained in:
Colin McDonnell
2025-12-21 22:42:42 -08:00
parent d5bec7499b
commit 73139a169c
3 changed files with 59 additions and 27 deletions
+23 -12
View File
@@ -130031,17 +130031,18 @@ async function checkoutPrBranch(params) {
const isFork = headRepo.full_name !== pr.data.base.repo.full_name; const isFork = headRepo.full_name !== pr.data.base.repo.full_name;
const baseBranch = pr.data.base.ref; const baseBranch = pr.data.base.ref;
const headBranch = pr.data.head.ref; const headBranch = pr.data.head.ref;
const currentBranch = $("git", ["rev-parse", "--abbrev-ref", "HEAD"], { log: false }).trim(); const localBranch = `pr-${pullNumber}`;
const alreadyOnBranch = currentBranch === headBranch; const currentSha = $("git", ["rev-parse", "HEAD"], { log: false }).trim();
const alreadyOnBranch = currentSha === pr.data.head.sha;
if (alreadyOnBranch) { if (alreadyOnBranch) {
log.debug(`already on PR branch ${headBranch}, skipping checkout`); log.debug(`already on PR branch ${localBranch}, skipping checkout`);
} else { } else {
log.debug(`\u{1F4E5} fetching base branch (${baseBranch})...`); log.debug(`\u{1F4E5} fetching base branch (${baseBranch})...`);
$("git", ["fetch", "--no-tags", "origin", baseBranch]); $("git", ["fetch", "--no-tags", "origin", baseBranch]);
$("git", ["checkout", "-B", baseBranch, `origin/${baseBranch}`]); $("git", ["checkout", "-B", baseBranch, `origin/${baseBranch}`]);
log.debug(`\u{1F33F} fetching PR #${pullNumber} (${headBranch})...`); log.debug(`\u{1F33F} fetching PR #${pullNumber} (${localBranch})...`);
$("git", ["fetch", "--no-tags", "origin", `pull/${pullNumber}/head:${headBranch}`]); $("git", ["fetch", "--no-tags", "origin", `pull/${pullNumber}/head:${localBranch}`]);
$("git", ["checkout", headBranch]); $("git", ["checkout", localBranch]);
log.debug(`\u2713 checked out PR #${pullNumber}`); log.debug(`\u2713 checked out PR #${pullNumber}`);
} }
if (alreadyOnBranch) { if (alreadyOnBranch) {
@@ -130058,15 +130059,17 @@ async function checkoutPrBranch(params) {
$("git", ["remote", "set-url", remoteName, forkUrl]); $("git", ["remote", "set-url", remoteName, forkUrl]);
log.debug(`\u{1F4CC} updated remote '${remoteName}' for fork ${headRepo.full_name}`); log.debug(`\u{1F4CC} updated remote '${remoteName}' for fork ${headRepo.full_name}`);
} }
$("git", ["config", `branch.${headBranch}.pushRemote`, remoteName]); $("git", ["config", `branch.${localBranch}.pushRemote`, remoteName]);
log.debug(`\u{1F4CC} configured branch '${headBranch}' to push to '${remoteName}'`); $("git", ["config", `branch.${localBranch}.merge`, `refs/heads/${headBranch}`]);
log.debug(`\u{1F4CC} configured branch '${localBranch}' to push to '${remoteName}/${headBranch}'`);
if (!pr.data.maintainer_can_modify) { if (!pr.data.maintainer_can_modify) {
log.warning( log.warning(
`\u26A0\uFE0F fork PR has maintainer_can_modify=false - push operations will fail. ask the PR author to enable "Allow edits from maintainers" or the fork may be owned by an organization.` `\u26A0\uFE0F fork PR has maintainer_can_modify=false - push operations will fail. ask the PR author to enable "Allow edits from maintainers" or the fork may be owned by an organization.`
); );
} }
} else { } else {
$("git", ["config", `branch.${headBranch}.pushRemote`, "origin"]); $("git", ["config", `branch.${localBranch}.pushRemote`, "origin"]);
$("git", ["config", `branch.${localBranch}.merge`, `refs/heads/${headBranch}`]);
} }
return { prNumber: pullNumber }; return { prNumber: pullNumber };
} }
@@ -131118,8 +131121,15 @@ function PushBranchTool(_ctx) {
remote = $("git", ["config", `branch.${branch}.pushRemote`], { log: false }).trim(); remote = $("git", ["config", `branch.${branch}.pushRemote`], { log: false }).trim();
} catch { } catch {
} }
const args3 = force ? ["push", "--force", "-u", remote, branch] : ["push", "-u", remote, branch]; let remoteBranch = branch;
log.debug(`pushing branch ${branch} to ${remote}`); try {
const mergeRef = $("git", ["config", `branch.${branch}.merge`], { log: false }).trim();
remoteBranch = mergeRef.replace("refs/heads/", "");
} catch {
}
const refspec = branch === remoteBranch ? branch : `${branch}:${remoteBranch}`;
const args3 = force ? ["push", "--force", "-u", remote, refspec] : ["push", "-u", remote, refspec];
log.debug(`pushing ${branch} to ${remote}/${remoteBranch}`);
if (force) { if (force) {
log.warning(`force pushing - this will overwrite remote history`); log.warning(`force pushing - this will overwrite remote history`);
} }
@@ -131127,9 +131137,10 @@ function PushBranchTool(_ctx) {
return { return {
success: true, success: true,
branch, branch,
remoteBranch,
remote, remote,
force, force,
message: `successfully pushed branch ${branch}` message: `successfully pushed ${branch} to ${remote}/${remoteBranch}`
}; };
}) })
}); });
+18 -10
View File
@@ -61,12 +61,17 @@ export async function checkoutPrBranch(
const baseBranch = pr.data.base.ref; const baseBranch = pr.data.base.ref;
const headBranch = pr.data.head.ref; const headBranch = pr.data.head.ref;
// check if we're already on the correct branch // always use pr-{number} as local branch name for consistency
const currentBranch = $("git", ["rev-parse", "--abbrev-ref", "HEAD"], { log: false }).trim(); // this avoids naming conflicts and makes push config simpler
const alreadyOnBranch = currentBranch === headBranch; const localBranch = `pr-${pullNumber}`;
// check if we're already on the correct commit (not just branch name)
// this handles fork PRs where head branch name might match base branch name
const currentSha = $("git", ["rev-parse", "HEAD"], { log: false }).trim();
const alreadyOnBranch = currentSha === pr.data.head.sha;
if (alreadyOnBranch) { if (alreadyOnBranch) {
log.debug(`already on PR branch ${headBranch}, skipping checkout`); log.debug(`already on PR branch ${localBranch}, skipping checkout`);
} else { } else {
// fetch base branch so origin/<base> exists for diff operations // fetch base branch so origin/<base> exists for diff operations
log.debug(`📥 fetching base branch (${baseBranch})...`); log.debug(`📥 fetching base branch (${baseBranch})...`);
@@ -77,11 +82,11 @@ export async function checkoutPrBranch(
$("git", ["checkout", "-B", baseBranch, `origin/${baseBranch}`]); $("git", ["checkout", "-B", baseBranch, `origin/${baseBranch}`]);
// fetch PR branch using pull/{n}/head refspec (works for both fork and same-repo PRs) // fetch PR branch using pull/{n}/head refspec (works for both fork and same-repo PRs)
log.debug(`🌿 fetching PR #${pullNumber} (${headBranch})...`); log.debug(`🌿 fetching PR #${pullNumber} (${localBranch})...`);
$("git", ["fetch", "--no-tags", "origin", `pull/${pullNumber}/head:${headBranch}`]); $("git", ["fetch", "--no-tags", "origin", `pull/${pullNumber}/head:${localBranch}`]);
// checkout the branch // checkout the branch
$("git", ["checkout", headBranch]); $("git", ["checkout", localBranch]);
log.debug(`✓ checked out PR #${pullNumber}`); log.debug(`✓ checked out PR #${pullNumber}`);
} }
@@ -110,8 +115,10 @@ export async function checkoutPrBranch(
} }
// set branch push config so `git push` knows where to push // set branch push config so `git push` knows where to push
$("git", ["config", `branch.${headBranch}.pushRemote`, remoteName]); $("git", ["config", `branch.${localBranch}.pushRemote`, remoteName]);
log.debug(`📌 configured branch '${headBranch}' to push to '${remoteName}'`); // set merge ref so git knows the remote branch name (may differ from local)
$("git", ["config", `branch.${localBranch}.merge`, `refs/heads/${headBranch}`]);
log.debug(`📌 configured branch '${localBranch}' to push to '${remoteName}/${headBranch}'`);
// warn if maintainer can't modify (push will likely fail) // warn if maintainer can't modify (push will likely fail)
if (!pr.data.maintainer_can_modify) { if (!pr.data.maintainer_can_modify) {
@@ -122,7 +129,8 @@ export async function checkoutPrBranch(
} }
} else { } else {
// for same-repo PRs, push to origin // for same-repo PRs, push to origin
$("git", ["config", `branch.${headBranch}.pushRemote`, "origin"]); $("git", ["config", `branch.${localBranch}.pushRemote`, "origin"]);
$("git", ["config", `branch.${localBranch}.merge`, `refs/heads/${headBranch}`]);
} }
return { prNumber: pullNumber }; return { prNumber: pullNumber };
+18 -5
View File
@@ -158,11 +158,23 @@ export function PushBranchTool(_ctx: ToolContext) {
// no configured pushRemote, default to origin // no configured pushRemote, default to origin
} }
const args = force // check if branch has a configured merge ref (remote branch name may differ from local)
? ["push", "--force", "-u", remote, branch] let remoteBranch = branch;
: ["push", "-u", remote, branch]; try {
const mergeRef = $("git", ["config", `branch.${branch}.merge`], { log: false }).trim();
// merge ref is like "refs/heads/main", extract the branch name
remoteBranch = mergeRef.replace("refs/heads/", "");
} catch {
// no configured merge ref, use local branch name
}
log.debug(`pushing branch ${branch} to ${remote}`); // use refspec when local and remote branch names differ
const refspec = branch === remoteBranch ? branch : `${branch}:${remoteBranch}`;
const args = force
? ["push", "--force", "-u", remote, refspec]
: ["push", "-u", remote, refspec];
log.debug(`pushing ${branch} to ${remote}/${remoteBranch}`);
if (force) { if (force) {
log.warning(`force pushing - this will overwrite remote history`); log.warning(`force pushing - this will overwrite remote history`);
} }
@@ -171,9 +183,10 @@ export function PushBranchTool(_ctx: ToolContext) {
return { return {
success: true, success: true,
branch, branch,
remoteBranch,
remote, remote,
force, force,
message: `successfully pushed branch ${branch}`, message: `successfully pushed ${branch} to ${remote}/${remoteBranch}`,
}; };
}), }),
}); });