Fix create_pull_request patch using stale origin/<branch> instead of merge-base#28960
Fix create_pull_request patch using stale origin/<branch> instead of merge-base#28960
Conversation
…f merge-base Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7eb336e4-2ce6-444e-b0f0-7a00ca8b99a1 Co-authored-by: mrjf <180956+mrjf@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes generate_git_patch.cjs full-mode base ref selection so create_pull_request patches are computed from merge-base origin/<defaultBranch> <branchName> rather than potentially stale origin/<branchName> remote-tracking refs.
Changes:
- Remove the full-mode short-circuit that used
origin/<branchName>when present; always compute merge-base vs the default branch. - Add a regression test that simulates the “stale origin/feature-branch + fast-forward to main + one agent commit” autoloop scenario.
- Add a changeset documenting the patch fix.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/generate_git_patch.cjs | Ensures full-mode patch base is derived from merge-base with the default branch, avoiding stale remote-tracking refs. |
| actions/setup/js/generate_git_patch.test.cjs | Adds end-to-end regression coverage for the stale origin/<branch> phantom-commit patch issue. |
| .changeset/patch-fix-stale-origin-branch-merge-base.md | Records the user-facing patch-level change in release notes. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
| execSync("git init --bare -b main", { cwd: remoteDir }); | ||
| execSync(`git remote add origin ${remoteDir}`, { cwd: repoDir }); | ||
| execSync("git push origin main", { cwd: repoDir }); | ||
|
|
||
| // Step 1: Create the feature branch at the initial commit and push it. | ||
| // This becomes the "old" position of origin/feature-branch. | ||
| execSync("git checkout -b feature-branch", { cwd: repoDir }); | ||
| execSync("git push origin feature-branch", { cwd: repoDir }); | ||
| const oldBranchSha = execSync("git rev-parse HEAD", { cwd: repoDir }).toString().trim(); | ||
|
|
||
| // Step 2: main advances with phantom commits the agent will not make | ||
| execSync("git checkout main", { cwd: repoDir }); | ||
| fs.writeFileSync(path.join(repoDir, "phantom1.md"), "# phantom 1\n"); | ||
| execSync("git add phantom1.md", { cwd: repoDir }); | ||
| execSync('git commit -m "phantom commit 1"', { cwd: repoDir }); | ||
| fs.writeFileSync(path.join(repoDir, "phantom2.md"), "# phantom 2\n"); | ||
| execSync("git add phantom2.md", { cwd: repoDir }); | ||
| execSync('git commit -m "phantom commit 2"', { cwd: repoDir }); | ||
| execSync("git push origin main", { cwd: repoDir }); | ||
|
|
||
| // Step 3: Simulate the agent run — fast-forward the local feature branch to | ||
| // main, then make one new commit. Note that origin/feature-branch is still | ||
| // pointing at oldBranchSha (we deliberately do NOT push the branch update). | ||
| execSync("git checkout feature-branch", { cwd: repoDir }); | ||
| execSync("git reset --hard main", { cwd: repoDir }); | ||
| fs.writeFileSync(path.join(repoDir, "agent-change.txt"), "the only real change\n"); | ||
| execSync("git add agent-change.txt", { cwd: repoDir }); | ||
| execSync('git commit -m "agent change"', { cwd: repoDir }); | ||
|
|
||
| // Sanity check: origin/feature-branch is still stale (points to oldBranchSha) | ||
| const remoteBranchSha = execSync("git rev-parse origin/feature-branch", { cwd: repoDir }).toString().trim(); | ||
| expect(remoteBranchSha).toBe(oldBranchSha); | ||
|
|
| // contains exactly the agent's changes. See issue githubnext/tsessebe autoloop | ||
| // failures for context. |
|
@copilot address comments and fix tests |
|
@copilot remove all references to tsessebe and autoloop in code -- the code should be generically addressed to all gh-aw users, not specific repos that implement it. |
…ion test Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6d0b8600-8bd6-4dda-8fc9-da3ea49ed7cd Co-authored-by: mrjf <180956+mrjf@users.noreply.github.com>
Done in dc5c61d. Removed all |
Already addressed in dc5c61d — all |
|
@copilot make sure all comments on this PR are addresses, especially the one on actions/setup/js/generate_git_patch.test.cjs |
Both prior review comments are already addressed in dc5c61d:
Repo-wide grep for |
In full mode,
generate_git_patch.cjsshort-circuited toorigin/<branchName>whenever that remote-tracking ref existed locally. Since that ref is populated by the startup checkout (fetch: ["*"]) and never refreshed, it points at the pre-run remote tip — not the branch state before the agent's edits. When the agent fast-forwards the local branch tomainand then commits (a common pattern for long-running iterative workflows), the resulting patch includes every commit between the stale tip andmain, producing phantom files the agent never touched.Changes
actions/setup/js/generate_git_patch.cjs— Remove theorigin/<branchName>short-circuit in full mode; always computemerge-base origin/<defaultBranch> <branchName>(this was already the fallback path). Incremental mode is unchanged — its use oforigin/<branchName>is correct forpush_to_pull_request_branch.actions/setup/js/generate_git_patch.test.cjs— Add regression test that builds a real bare-remote + working repo reproducing the stale-remote-tracking-ref scenario (staleorigin/feature-branch+ local fast-forward tomain+ one new commit), and asserts the patch contains only the agent's file. The test explicitly fetchesrefs/remotes/origin/feature-branchafter the initial push so behavior is deterministic across git versions.tsessebe) and workflow-pattern-specific (autoloop) references from code, tests, changesets, and docs in favor of generic "long-running branch" wording so the messaging applies to all gh-aw users.Before / After
The regression test fails on the pre-fix code (patch contains
phantom1.md,phantom2.md,agent-change.txt) and passes after the fix (patch contains onlyagent-change.txt).