fix push_branch resolving to wrong remote branch (#282)
getPushDestination used git's @{push} which under push.default=simple
resolves using the local branch name as the remote branch name. since
checkout_pr uses pr-N as the local name, this resolved to origin/pr-N
instead of the actual PR branch (e.g. origin/pullfrog/feature-branch).
this caused two failure modes:
- agent passes remote branch name to push_branch → "src refspec does
not match any" because no local branch has that name
- agent calls push_branch with no args → silently pushes to a new
remote branch pr-N instead of updating the PR branch
fix: read branch.X.pushRemote and branch.X.merge from git config
directly (the exact config checkout_pr already writes) instead of
relying on @{push}. also rename head → localBranch + remoteBranch
in CheckoutPrResult to make the distinction explicit.
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Colin McDonnell
committed by
pullfrog[bot]
pullfrog[bot]
parent
30812435f9
commit
6a77ea6612
@@ -141057,14 +141057,15 @@ ${diffPreview}`);
|
||||
number: pr.data.number,
|
||||
title: pr.data.title,
|
||||
base: pr.data.base.ref,
|
||||
head: pr.data.head.ref,
|
||||
localBranch: `pr-${pull_number}`,
|
||||
remoteBranch: `refs/heads/${pr.data.head.ref}`,
|
||||
isFork: headRepo.full_name !== pr.data.base.repo.full_name,
|
||||
maintainerCanModify: pr.data.maintainer_can_modify,
|
||||
url: pr.data.html_url,
|
||||
headRepo: headRepo.full_name,
|
||||
diffPath,
|
||||
toc: formatResult.toc,
|
||||
instructions: `the diff file at diffPath contains a table of contents (TOC) at the top listing every changed file with its line range. use the line ranges to read specific files from the diff instead of reading the entire file. for example, if the TOC says "src/foo.ts \u2192 lines 5-42", read lines 5-42 from diffPath to see that file's changes. review files selectively based on relevance rather than reading everything sequentially.`
|
||||
instructions: `the diff file at diffPath contains a table of contents (TOC) at the top listing every changed file with its line range. use the line ranges to read specific files from the diff instead of reading the entire file. for example, if the TOC says "src/foo.ts \u2192 lines 5-42", read lines 5-42 from diffPath to see that file's changes. review files selectively based on relevance rather than reading everything sequentially. the local branch is 'localBranch' (pr-{number}), not the remote branch name. when pushing, omit branchName to use the current branch. do not use remoteBranch as a local branch name.`
|
||||
};
|
||||
})
|
||||
});
|
||||
@@ -142917,21 +142918,13 @@ function ListDirectoryTool(_ctx) {
|
||||
// mcp/git.ts
|
||||
function getPushDestination(branch) {
|
||||
try {
|
||||
const pushRef = $(
|
||||
"git",
|
||||
["rev-parse", "--abbrev-ref", "--symbolic-full-name", `${branch}@{push}`],
|
||||
{ log: false }
|
||||
).trim();
|
||||
const slashIndex = pushRef.indexOf("/");
|
||||
if (slashIndex === -1) {
|
||||
throw new Error(`unexpected push ref format: ${pushRef}`);
|
||||
}
|
||||
const remoteName = pushRef.slice(0, slashIndex);
|
||||
const remoteBranch = pushRef.slice(slashIndex + 1);
|
||||
const url4 = $("git", ["remote", "get-url", "--push", remoteName], { log: false }).trim();
|
||||
return { remoteName, remoteBranch, url: url4 };
|
||||
const pushRemote = $("git", ["config", `branch.${branch}.pushRemote`], { log: false }).trim();
|
||||
const merge4 = $("git", ["config", `branch.${branch}.merge`], { log: false }).trim();
|
||||
const remoteBranch = merge4.replace(/^refs\/heads\//, "");
|
||||
const url4 = $("git", ["remote", "get-url", "--push", pushRemote], { log: false }).trim();
|
||||
return { remoteName: pushRemote, remoteBranch, url: url4 };
|
||||
} catch {
|
||||
log.debug(`no push tracking for ${branch}, falling back to origin/${branch}`);
|
||||
log.debug(`no push config for ${branch}, falling back to origin/${branch}`);
|
||||
const url4 = $("git", ["remote", "get-url", "--push", "origin"], { log: false }).trim();
|
||||
return { remoteName: "origin", remoteBranch: branch, url: url4 };
|
||||
}
|
||||
@@ -142960,7 +142953,7 @@ function PushBranchTool(ctx) {
|
||||
const pushPermission = ctx.payload.push;
|
||||
return tool({
|
||||
name: "push_branch",
|
||||
description: "Push the current branch (or specified branch) to the remote repository. Git automatically determines the correct remote based on branch config (set by checkout_pr for fork PRs). Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode.",
|
||||
description: "Push the current branch to the remote repository. Omit branchName to push the current branch (recommended). If specifying branchName, use the LOCAL branch name (e.g., 'pr-1'), not the remote branch name. The correct remote and remote branch are determined automatically from branch config set by checkout_pr. Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode.",
|
||||
parameters: PushBranch,
|
||||
execute: execute(async ({ branchName, force }) => {
|
||||
if (pushPermission === "disabled") {
|
||||
|
||||
+7
-3
@@ -132,7 +132,8 @@ export type CheckoutPrResult = {
|
||||
number: number;
|
||||
title: string;
|
||||
base: string;
|
||||
head: string;
|
||||
localBranch: string;
|
||||
remoteBranch: string;
|
||||
isFork: boolean;
|
||||
maintainerCanModify: boolean;
|
||||
url: string;
|
||||
@@ -356,7 +357,8 @@ export function CheckoutPrTool(ctx: ToolContext) {
|
||||
number: pr.data.number,
|
||||
title: pr.data.title,
|
||||
base: pr.data.base.ref,
|
||||
head: pr.data.head.ref,
|
||||
localBranch: `pr-${pull_number}`,
|
||||
remoteBranch: `refs/heads/${pr.data.head.ref}`,
|
||||
isFork: headRepo.full_name !== pr.data.base.repo.full_name,
|
||||
maintainerCanModify: pr.data.maintainer_can_modify,
|
||||
url: pr.data.html_url,
|
||||
@@ -367,7 +369,9 @@ export function CheckoutPrTool(ctx: ToolContext) {
|
||||
`the diff file at diffPath contains a table of contents (TOC) at the top listing every changed file with its line range. ` +
|
||||
`use the line ranges to read specific files from the diff instead of reading the entire file. ` +
|
||||
`for example, if the TOC says "src/foo.ts → lines 5-42", read lines 5-42 from diffPath to see that file's changes. ` +
|
||||
`review files selectively based on relevance rather than reading everything sequentially.`,
|
||||
`review files selectively based on relevance rather than reading everything sequentially. ` +
|
||||
`the local branch is 'localBranch' (pr-{number}), not the remote branch name. ` +
|
||||
`when pushing, omit branchName to use the current branch. do not use remoteBranch as a local branch name.`,
|
||||
} satisfies CheckoutPrResult;
|
||||
}),
|
||||
});
|
||||
|
||||
+17
-25
@@ -14,37 +14,26 @@ type PushDestination = {
|
||||
|
||||
/**
|
||||
* get where git would actually push this branch.
|
||||
* uses git's native @{push} resolution, falls back to origin if unset.
|
||||
* reads branch.X.pushRemote and branch.X.merge set by checkout_pr.
|
||||
*
|
||||
* NOTE: we read git config directly instead of using @{push} because
|
||||
* push.default=simple (git's default) requires local/remote branch names
|
||||
* to match. since checkout_pr uses pr-N as the local name, @{push} resolves
|
||||
* to origin/pr-N instead of the actual remote branch.
|
||||
*
|
||||
* for branches created via checkout_pr: uses configured pushRemote/merge
|
||||
* for new branches (git checkout -b): falls back to origin/<branch>
|
||||
*/
|
||||
function getPushDestination(branch: string): PushDestination {
|
||||
// try git's @{push} resolution first (works for checkout_pr branches)
|
||||
try {
|
||||
const pushRef = $(
|
||||
"git",
|
||||
["rev-parse", "--abbrev-ref", "--symbolic-full-name", `${branch}@{push}`],
|
||||
{ log: false }
|
||||
).trim();
|
||||
|
||||
// pushRef is like "origin/main" or "pr-123/feature/foo"
|
||||
// parse carefully to handle branch names with slashes
|
||||
const slashIndex = pushRef.indexOf("/");
|
||||
if (slashIndex === -1) {
|
||||
throw new Error(`unexpected push ref format: ${pushRef}`);
|
||||
}
|
||||
const remoteName = pushRef.slice(0, slashIndex);
|
||||
const remoteBranch = pushRef.slice(slashIndex + 1);
|
||||
|
||||
// get the actual URL git would push to (handles remote.X.pushurl)
|
||||
const url = $("git", ["remote", "get-url", "--push", remoteName], { log: false }).trim();
|
||||
|
||||
return { remoteName, remoteBranch, url };
|
||||
const pushRemote = $("git", ["config", `branch.${branch}.pushRemote`], { log: false }).trim();
|
||||
const merge = $("git", ["config", `branch.${branch}.merge`], { log: false }).trim();
|
||||
const remoteBranch = merge.replace(/^refs\/heads\//, "");
|
||||
const url = $("git", ["remote", "get-url", "--push", pushRemote], { log: false }).trim();
|
||||
return { remoteName: pushRemote, remoteBranch, url };
|
||||
} catch {
|
||||
// @{push} not configured - branch was created locally without checkout_pr
|
||||
// fall back to origin with the same branch name
|
||||
log.debug(`no push tracking for ${branch}, falling back to origin/${branch}`);
|
||||
// no push config - branch was created locally without checkout_pr
|
||||
log.debug(`no push config for ${branch}, falling back to origin/${branch}`);
|
||||
const url = $("git", ["remote", "get-url", "--push", "origin"], { log: false }).trim();
|
||||
return { remoteName: "origin", remoteBranch: branch, url };
|
||||
}
|
||||
@@ -95,7 +84,10 @@ export function PushBranchTool(ctx: ToolContext) {
|
||||
return tool({
|
||||
name: "push_branch",
|
||||
description:
|
||||
"Push the current branch (or specified branch) to the remote repository. Git automatically determines the correct remote based on branch config (set by checkout_pr for fork PRs). Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode.",
|
||||
"Push the current branch to the remote repository. Omit branchName to push the current branch (recommended). " +
|
||||
"If specifying branchName, use the LOCAL branch name (e.g., 'pr-1'), not the remote branch name. " +
|
||||
"The correct remote and remote branch are determined automatically from branch config set by checkout_pr. " +
|
||||
"Never force push unless explicitly requested. Pushes to the default branch are blocked in restricted mode.",
|
||||
parameters: PushBranch,
|
||||
execute: execute(async ({ branchName, force }) => {
|
||||
// permission check
|
||||
|
||||
Reference in New Issue
Block a user