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
72 changes: 40 additions & 32 deletions actions/setup/js/safe_outputs_handlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -952,20 +952,19 @@ function createHandlers(server, appendSafeOutput, config = {}) {
/**
* Handler for push_to_pull_request_branch tool
* Spec cross-reference: Safe Output Outcome Evaluation §17 (`push_to_pull_request_branch`).
* The agent does NOT supply a branch. The source branch is derived from the
* current working checkout (the agent must already be on the PR head ref to
* have committed onto it). The destination branch is independently derived
* by the apply-time push handler from pulls.get(pull_number).head.ref.
* The agent SHOULD supply a `branch` argument identifying the local branch it
* committed onto. This is required for batch workflows that loop over multiple PRs
* and checkout different branches; without it, the source branch would be inferred
* from the current git HEAD which may not match the PR being processed. When
* `branch` is omitted, it is derived from the current checkout as a fallback.
* The destination branch is independently derived by the apply-time push handler
* from pulls.get(pull_number).head.ref.
*
* Note: Fork PR detection is handled by push_to_pull_request_branch.cjs handler
* which fetches the PR and calls detectForkPR() with full PR data.
*/
const pushToPullRequestBranchHandler = async args => {
// Defensive strip: the input schema no longer declares a `branch` property,
// but an older or non-conforming client could still attempt to pass one.
// Drop it so the agent cannot override the derived source branch.
const { branch: _agentBranch, ...sanitizedArgs } = args || {};
const entry = { ...sanitizedArgs, type: "push_to_pull_request_branch" };
const entry = { ...(args || {}), type: "push_to_pull_request_branch" };
const wildcardTargetValidationError = validateWildcardTargetRequirement(entry);
if (wildcardTargetValidationError) {
return wildcardTargetValidationError;
Expand Down Expand Up @@ -1078,29 +1077,38 @@ function createHandlers(server, appendSafeOutput, config = {}) {
// like issue_comment on PRs targeting non-default branches.
entry.base_branch = baseBranch;

// The agent never supplies a branch; the validator already strips it from
// args. Derive it from the current checkout: the working tree must be on
// the PR head ref because that's what the agent committed onto. The
// apply-time push job independently re-derives the destination from
// pulls.get(pull_number), so this branch name is used only as the source
// ref for the incremental diff against origin/<branch>.
try {
const detectedBranch = getCurrentBranch(repoCwd);
server.debug(`Using current branch for push_to_pull_request_branch: ${detectedBranch}`);
entry.branch = detectedBranch;
} catch (branchErr) {
return {
content: [
{
type: "text",
text: JSON.stringify({
result: "error",
error: `Failed to determine source branch for push_to_pull_request_branch: ${getErrorMessage(branchErr)}. The working tree must be on the pull request's head ref before this tool is called.`,
}),
},
],
isError: true,
};
// Use the agent-supplied branch if provided; fall back to the current checkout.
// The agent-supplied value is authoritative: in multi-PR batch workflows the
// working tree may be checked out to a different PR's branch by the time the
// MCP handler runs, so relying solely on HEAD can produce a wrong source ref
// and cause the apply step to fail with "couldn't find remote ref".
// The apply-time push job re-derives the destination from pulls.get(pull_number)
// independently, so this branch name is used only as the source ref for the
// incremental diff against origin/<branch>.
if (entry.branch && typeof entry.branch === "string" && entry.branch.trim()) {
entry.branch = entry.branch.trim();
server.debug(`Using agent-supplied branch for push_to_pull_request_branch: ${entry.branch}`);
} else {
// Fallback: derive from the current checkout (backward compat for single-PR workflows
// where the working tree is reliably on the PR head ref at call time).
try {
const detectedBranch = getCurrentBranch(repoCwd);
server.debug(`Using current branch for push_to_pull_request_branch: ${detectedBranch}`);
entry.branch = detectedBranch;
} catch (branchErr) {
return {
content: [
{
type: "text",
text: JSON.stringify({
result: "error",
error: `Failed to determine source branch for push_to_pull_request_branch: ${getErrorMessage(branchErr)}. Either supply a 'branch' argument explicitly or ensure the working tree is checked out to the pull request's head ref before calling this tool.`,
}),
},
],
isError: true,
};
}
}

// Reject if the detected branch equals base_branch. This means the workspace
Expand Down
9 changes: 5 additions & 4 deletions actions/setup/js/safe_outputs_mcp_server_defaults.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,11 @@ const canWriteDefault = canWriteToDefaultPath();
(expect(pushTool).toBeDefined(),
expect(pushTool.inputSchema.required).toEqual(["message"]),
expect(pushTool.inputSchema.required).not.toContain("branch"),
// The agent must not be able to specify a branch — it's always
// derived from the pull request's head ref. See issue #37835.
expect(pushTool.inputSchema.properties.branch).toBeUndefined(),
expect(pushTool.description).toMatch(/derived from the pull request/i),
// The agent may supply an explicit branch to avoid HEAD-detection races
// in batch workflows (fix for issue #41643). The field is optional.
expect(pushTool.inputSchema.properties.branch).toBeDefined(),
expect(pushTool.inputSchema.properties.branch.type).toBe("string"),
expect(pushTool.description).toMatch(/supply.*branch|branch.*explicitly|batch workflow/i),
resolve());
}, 500));
});
Expand Down
6 changes: 5 additions & 1 deletion actions/setup/js/safe_outputs_tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@
},
{
"name": "push_to_pull_request_branch",
"description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. The destination branch is always derived from the pull request's head ref \u2014 you do not specify it. IMPORTANT: do NOT use 'git merge' to update the branch against another branch \u2014 merge commits cannot be signed; the action will attempt to squash them into a single linear commit before pushing, but this rewrites history. Use 'git rebase' instead to avoid the rewrite.",
"description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. IMPORTANT: always supply the 'branch' argument with the local branch name you committed to. In batch workflows that process multiple PRs, this is required \u2014 if omitted, the branch is inferred from the current git HEAD, which will produce wrong results if the workspace has been checked out to a different branch between commit and tool-call time. IMPORTANT: do NOT use 'git merge' to update the branch against another branch \u2014 merge commits cannot be signed; the action will attempt to squash them into a single linear commit before pushing, but this rewrites history. Use 'git rebase' instead to avoid the rewrite.",
"inputSchema": {
"type": "object",
"required": ["message"],
Expand All @@ -1082,6 +1082,10 @@
"description": "Commit message describing the changes. Follow repository commit message conventions (e.g., conventional commits). This field is named message, NOT commit_message.",
"x-synonyms": ["commit_message"]
},
"branch": {
"type": "string",
"description": "The local branch name that contains the committed changes to push (e.g., \"feature/my-fix\"). Providing this explicitly prevents race conditions in batch workflows where the working tree may have been checked out to a different PR's branch between commit and tool-call time. When omitted, the branch is inferred from the current git HEAD — only safe for single-PR workflows."
},
"pull_request_number": {
"type": ["number", "string"],
"description": "Pull request number to push changes to. This is the numeric ID from the GitHub URL (e.g., 654 in github.com/owner/repo/pull/654). Required when the workflow target is '*' (any PR).",
Expand Down
6 changes: 5 additions & 1 deletion pkg/workflow/js/safe_outputs_tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -1367,7 +1367,7 @@
},
{
"name": "push_to_pull_request_branch",
"description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. The destination branch is always derived from the pull request's head ref \u2014 you do not specify it. IMPORTANT: do NOT use 'git merge' to update the branch against another branch \u2014 merge commits cannot be signed; the action will attempt to squash them into a single linear commit before pushing, but this rewrites history. Use 'git rebase' instead to avoid the rewrite.",
"description": "Push committed changes to a pull request's branch. APPEND-ONLY: this tool adds new commits on top of the existing PR branch \u2014 force-push is NOT supported and will be rejected. Use this to add follow-up commits to an existing PR, such as addressing review feedback or fixing issues. This is a write-once declaration for a real intended PR branch update, not a sandbox or probe: do not call it with probe branches, placeholder commit messages, or auth experiments. If you are not ready to push the real update, use noop or report_incomplete instead. Changes must be committed locally before calling this tool. IMPORTANT: always supply the 'branch' argument with the local branch name you committed to. In batch workflows that process multiple PRs, this is required \u2014 if omitted, the branch is inferred from the current git HEAD, which will produce wrong results if the workspace has been checked out to a different branch between commit and tool-call time. IMPORTANT: do NOT use 'git merge' to update the branch against another branch \u2014 merge commits cannot be signed; the action will attempt to squash them into a single linear commit before pushing, but this rewrites history. Use 'git rebase' instead to avoid the rewrite.",
"inputSchema": {
"type": "object",
"required": [
Expand All @@ -1381,6 +1381,10 @@
"commit_message"
]
},
"branch": {
"type": "string",
"description": "The local branch name that contains the committed changes to push (e.g., \"feature/my-fix\"). Providing this explicitly prevents race conditions in batch workflows where the working tree may have been checked out to a different PR's branch between commit and tool-call time. When omitted, the branch is inferred from the current git HEAD \u2014 only safe for single-PR workflows."
},
"pull_request_number": {
"type": [
"number",
Expand Down
Loading