-
Notifications
You must be signed in to change notification settings - Fork 322
create_pull_request: branch name guidance, PR-comment context tool selection, and shallow clone fallback failures #24121
Description
Summary
Three related issues contributed to a create_pull_request failure in fsprojects/FSharp.Formatting run 23860833864. The agent was invoked via a PR comment on PR #1129, made commits on the existing PR branch, then incorrectly used create_pull_request with a non-existent branch name. This caused an over-inclusive patch, a "Patch is empty" error on git am, and a fallback failure due to shallow clone.
Issue 1: Injected prompting for create_pull_request needs much stronger branch name guidance
The current prompting in safe_outputs_create_pull_request.md says:
- If you haven't done so already, create a local branch using an appropriate unique name.
The branch parameter description in the tool schema says:
Source branch name containing the changes. If omitted, uses the current working branch.
In this failure, the agent specified branch: repo-assist/tests-nav-factory-1129 — a branch that did not exist locally. The actual local branch was repo-assist/perf-nav-factory-2026-03-30-4dd5aa8c6fa58713 (checked out via gh pr checkout 1129).
When generate_git_patch.cjs Strategy 1 tried git show-ref --verify refs/heads/repo-assist/tests-nav-factory-1129, it failed because the branch didn't exist. The fallback strategies then generated a patch from the merge-base with origin/main, producing a 5-commit patch that included the original PR commits (already on the target branch) plus the agent's new commits. This caused patch 1/5 to fail with "Patch is empty" because those changes already existed on main.
Suggested fix: The safe_outputs_create_pull_request.md prompting should be much clearer:
- The
branchparameter must be the name of the current local git branch (i.e. whatgit branch --show-currentreturns). - If the agent created a new branch, use that name. If the agent is on an existing branch, use that branch name.
- Never invent a branch name that doesn't correspond to an existing local branch.
Issue 2: Agent should prefer push_to_pull_request_branch in PR comment context
In this run, the agent was invoked via an issue_comment event on PR #1129. The agent was checked out onto the PR branch. The instructions asked it to add tests to the PR. The correct tool was push_to_pull_request_branch — not create_pull_request.
The current pr_context_prompt.md tells the agent it's on the PR branch but gives no guidance about which safe output tool to prefer:
<branch-context trigger="pull-request-comment">
<description>This workflow was triggered by a comment on a pull request...</description>
<current-state>
- The current working directory contains the code from the pull request branch
- Any file operations you perform will be on the PR branch code
...
</current-state>
</branch-context>Suggested fix: When the workflow is triggered by a PR comment, the injected prompting should include guidance like:
When triggered by a pull request comment, if you need to push code changes, prefer using
push_to_pull_request_branchto add commits to the existing PR, rather thancreate_pull_requestwhich creates a new separate PR. Only usecreate_pull_requestif the instructions explicitly ask for a new, separate pull request.
This guidance should only be injected when push_to_pull_request_branch is actually available as a configured safe output.
Issue 3: Shallow clone (fetch-depth: 1) prevents fallback from working
When git am --3way fails, create_pull_request.cjs attempts a fallback: check out the original base commit SHA recorded during patch generation and apply the patch there. This fallback failed because:
Original base commit from patch generation: f40f87a12141fe4e49505bb97f81e66a6a405e64
[command]/usr/bin/git cat-file -e f40f87a12141fe4e49505bb97f81e66a6a405e64
##[warning]Fallback to original base commit failed: exit code 1
The safe_outputs job uses fetch-depth: 1, so only the tip commit of main is available. The base commit f40f87a1 (2 commits behind HEAD) is unreachable.
Suggested fix: When create_pull_request.cjs has a base_commit SHA from patch generation, it should explicitly fetch that commit before attempting git cat-file -e. For example:
// Before verifying, try to fetch the base commit in case of shallow clone
try {
await exec.exec("git", ["fetch", "origin", originalBaseCommit, "--depth=1"]);
} catch { /* ignore - may already be available */ }
await exec.exec("git", ["cat-file", "-e", originalBaseCommit]);This ensures the fallback path actually works regardless of checkout depth. The fallback is the safety net for patch application failures, so it should be robust by default.
Reproduction
- Repository:
fsprojects/FSharp.Formatting - Failed run: https://github.com/fsprojects/FSharp.Formatting/actions/runs/23860833864
- PR: [Repo Assist] perf: pre-compute navigation menu structure once per build (O(n²) → O(n)) fsprojects/FSharp.Formatting#1129
- Trigger:
issue_commenton the PR - gh-aw version: v0.65.4
Error sequence
- Agent checked out PR [tidy] Fix Go formatting in Copilot engine files #1129 branch via
gh pr checkout 1129 - Agent made 5 commits (tests + CI triggers + changelog fix)
- Agent called
create_pull_requestwithbranch: repo-assist/tests-nav-factory-1129(non-existent branch) generate_git_patch.cjsfell through Strategy 1, produced over-inclusive patch from merge-basesafe_outputsapplied patch viagit am --3way→ Patch 1/5 "Patch is empty" (exit 128)- Fallback to base commit
f40f87a1→git cat-file -efailed (shallow clone) - PR creation failed entirely