From 48aa40029458d4552d12e6efd24fb511d12c4b12 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 15:56:26 +0000 Subject: [PATCH 1/3] Initial plan From 773f7c1fe0cf971372209101ec7d223ffe2c78d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 16:05:19 +0000 Subject: [PATCH 2/3] Fix create_pull_request patch using stale origin/branchName instead of 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> --- ...atch-fix-stale-origin-branch-merge-base.md | 9 ++ actions/setup/js/generate_git_patch.cjs | 83 +++++++------- actions/setup/js/generate_git_patch.test.cjs | 107 ++++++++++++++++++ 3 files changed, 158 insertions(+), 41 deletions(-) create mode 100644 .changeset/patch-fix-stale-origin-branch-merge-base.md diff --git a/.changeset/patch-fix-stale-origin-branch-merge-base.md b/.changeset/patch-fix-stale-origin-branch-merge-base.md new file mode 100644 index 00000000000..3aadda87605 --- /dev/null +++ b/.changeset/patch-fix-stale-origin-branch-merge-base.md @@ -0,0 +1,9 @@ +--- +"gh-aw": patch +--- + +Fixed `create_pull_request` patch (full mode) using a stale `origin/` as the base ref when that remote-tracking ref existed locally, causing the patch to include commits the agent never made. + +In `generate_git_patch.cjs` full mode, the code previously short-circuited to `origin/` whenever that ref existed locally. But `origin/` is fetched at workflow startup and can be arbitrarily stale — typically pointing at where the remote branch was before the agent run, not the state of the branch *before the agent made changes*. When the local branch is fast-forwarded to the default branch during the agent run (a common autoloop pattern), the resulting patch erroneously included every commit between the old branch tip and the current `main`. + +Full mode now always computes the merge-base with the default branch, matching the behavior of the previous fallback path. Incremental mode (`push_to_pull_request_branch`) is unchanged — its use of `origin/` as a base is intentional and correct for that workflow. diff --git a/actions/setup/js/generate_git_patch.cjs b/actions/setup/js/generate_git_patch.cjs index d7fa08dd2b2..f1d7b591fc2 100644 --- a/actions/setup/js/generate_git_patch.cjs +++ b/actions/setup/js/generate_git_patch.cjs @@ -164,53 +164,54 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { // FULL MODE (for create_pull_request): // Include all commits since merge-base with default branch. // This is appropriate for creating new PRs where we want all changes. - - debugLog(`Strategy 1 (full): Checking if origin/${branchName} exists`); + // + // IMPORTANT: We deliberately do NOT short-circuit to `origin/${branchName}` even + // when that remote-tracking ref exists locally. That ref is fetched at workflow + // startup and represents the *remote* branch state at that moment, not the + // branch state before the agent made changes. If the local branch was + // fast-forwarded to the default branch during the agent run (a common pattern), + // using the stale `origin/${branchName}` would cause the patch to include all + // commits from the default branch since the old branch tip — commits the agent + // never made. Always compute the merge-base with the default branch so the patch + // contains exactly the agent's changes. See issue githubnext/tsessebe autoloop + // failures for context. + debugLog(`Strategy 1 (full): Computing merge-base with ${defaultBranch} (ignoring any stale origin/${branchName})`); + // Check if origin/ already exists locally (e.g., from checkout with fetch-depth: 0) + // This is important for cross-repo checkouts where persist-credentials: false prevents fetching + let hasLocalDefaultBranch = false; try { - // Check if origin/branchName exists - execGitSync(["show-ref", "--verify", "--quiet", `refs/remotes/origin/${branchName}`], { cwd }); - baseRef = `origin/${branchName}`; - debugLog(`Strategy 1 (full): Using existing origin/${branchName} as baseRef`); + execGitSync(["show-ref", "--verify", "--quiet", `refs/remotes/origin/${defaultBranch}`], { cwd }); + hasLocalDefaultBranch = true; + debugLog(`Strategy 1 (full): origin/${defaultBranch} exists locally`); } catch { - // origin/branchName doesn't exist - use merge-base with default branch - debugLog(`Strategy 1 (full): origin/${branchName} not found, trying merge-base with ${defaultBranch}`); - // First check if origin/ already exists locally (e.g., from checkout with fetch-depth: 0) - // This is important for cross-repo checkouts where persist-credentials: false prevents fetching - let hasLocalDefaultBranch = false; + // origin/ doesn't exist locally, try to fetch it + debugLog(`Strategy 1 (full): origin/${defaultBranch} not found locally, attempting fetch`); try { - execGitSync(["show-ref", "--verify", "--quiet", `refs/remotes/origin/${defaultBranch}`], { cwd }); + // Configure git authentication via GIT_CONFIG_* environment variables. + // This ensures the fetch works when .git/config credentials are unavailable + // (e.g. after clean_git_credentials.sh) and on GitHub Enterprise Server (GHES). + // Use options.token when provided (cross-repo PAT), falling back to GITHUB_TOKEN. + // SECURITY: The auth header is passed via env vars so it is never written to + // .git/config on disk, preventing file-monitoring attacks. + const fullFetchEnv = { ...process.env, ...getGitAuthEnv(options.token) }; + // Use "--" to prevent branch names starting with "-" from being interpreted as options + execGitSync(["fetch", "origin", "--", defaultBranch], { cwd, env: fullFetchEnv }); hasLocalDefaultBranch = true; - debugLog(`Strategy 1 (full): origin/${defaultBranch} exists locally`); - } catch { - // origin/ doesn't exist locally, try to fetch it - debugLog(`Strategy 1 (full): origin/${defaultBranch} not found locally, attempting fetch`); - try { - // Configure git authentication via GIT_CONFIG_* environment variables. - // This ensures the fetch works when .git/config credentials are unavailable - // (e.g. after clean_git_credentials.sh) and on GitHub Enterprise Server (GHES). - // Use options.token when provided (cross-repo PAT), falling back to GITHUB_TOKEN. - // SECURITY: The auth header is passed via env vars so it is never written to - // .git/config on disk, preventing file-monitoring attacks. - const fullFetchEnv = { ...process.env, ...getGitAuthEnv(options.token) }; - // Use "--" to prevent branch names starting with "-" from being interpreted as options - execGitSync(["fetch", "origin", "--", defaultBranch], { cwd, env: fullFetchEnv }); - hasLocalDefaultBranch = true; - debugLog(`Strategy 1 (full): Successfully fetched origin/${defaultBranch}`); - } catch (fetchErr) { - // Fetch failed (likely due to persist-credentials: false in cross-repo checkout) - // We'll try other strategies below - debugLog(`Strategy 1 (full): Fetch failed - ${getErrorMessage(fetchErr)} (will try other strategies)`); - } + debugLog(`Strategy 1 (full): Successfully fetched origin/${defaultBranch}`); + } catch (fetchErr) { + // Fetch failed (likely due to persist-credentials: false in cross-repo checkout) + // We'll try other strategies below + debugLog(`Strategy 1 (full): Fetch failed - ${getErrorMessage(fetchErr)} (will try other strategies)`); } + } - if (hasLocalDefaultBranch) { - baseRef = execGitSync(["merge-base", "--", `origin/${defaultBranch}`, branchName], { cwd }).trim(); - debugLog(`Strategy 1 (full): Computed merge-base: ${baseRef}`); - } else { - // No remote refs available - fall through to Strategy 2 - debugLog(`Strategy 1 (full): No remote refs available, falling through to Strategy 2`); - throw new Error(`${ERR_SYSTEM}: No remote refs available for merge-base calculation`); - } + if (hasLocalDefaultBranch) { + baseRef = execGitSync(["merge-base", "--", `origin/${defaultBranch}`, branchName], { cwd }).trim(); + debugLog(`Strategy 1 (full): Computed merge-base: ${baseRef}`); + } else { + // No remote refs available - fall through to Strategy 2 + debugLog(`Strategy 1 (full): No remote refs available, falling through to Strategy 2`); + throw new Error(`${ERR_SYSTEM}: No remote refs available for merge-base calculation`); } } diff --git a/actions/setup/js/generate_git_patch.test.cjs b/actions/setup/js/generate_git_patch.test.cjs index 4212f44ea6c..7af1fee68f9 100644 --- a/actions/setup/js/generate_git_patch.test.cjs +++ b/actions/setup/js/generate_git_patch.test.cjs @@ -485,3 +485,110 @@ describe("generateGitPatch – excludedFiles option", () => { expect(result.success).toBe(false); }); }); + +// ────────────────────────────────────────────────────────────────────────── +// Full mode base ref selection — must use merge-base, never stale origin/ +// Regression test for: create_pull_request patch uses stale origin/branchName +// instead of merge-base with default branch. +// ────────────────────────────────────────────────────────────────────────── + +describe("generateGitPatch – full mode base ref (merge-base, not stale origin)", () => { + let repoDir; + let originalEnv; + + beforeEach(() => { + originalEnv = { GITHUB_WORKSPACE: process.env.GITHUB_WORKSPACE, GITHUB_SHA: process.env.GITHUB_SHA }; + + global.core = { debug: () => {}, info: () => {}, warning: () => {}, error: () => {} }; + + repoDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-patch-fullmode-")); + execSync("git init -b main", { cwd: repoDir }); + execSync('git config user.email "test@example.com"', { cwd: repoDir }); + execSync('git config user.name "Test"', { cwd: repoDir }); + + // Initial commit on main + fs.writeFileSync(path.join(repoDir, "README.md"), "# Repo\n"); + execSync("git add .", { cwd: repoDir }); + execSync('git commit -m "init"', { cwd: repoDir }); + + delete process.env.GITHUB_WORKSPACE; + delete process.env.GITHUB_SHA; + delete require.cache[require.resolve("./generate_git_patch.cjs")]; + }); + + afterEach(() => { + Object.entries(originalEnv).forEach(([k, v]) => { + if (v !== undefined) process.env[k] = v; + else delete process.env[k]; + }); + if (repoDir && fs.existsSync(repoDir)) { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + delete require.cache[require.resolve("./generate_git_patch.cjs")]; + delete global.core; + }); + + it("should NOT include phantom commits when stale origin/ exists (regression test)", async () => { + // Reproduce the autoloop bug scenario: + // 1. A feature branch was pushed in the past at some old commit + // (origin/feature-branch points there — this is the "stale" remote-tracking ref) + // 2. main has since advanced with many "phantom" commits the agent never made + // 3. The agent fast-forwards the local branch to main, then makes one new commit + // 4. Full-mode patch (create_pull_request) must contain ONLY the agent's commit, + // NOT the phantom commits between the old branch tip and main. + + // Set up a "remote" repo to act as origin + const remoteDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-patch-fullmode-remote-")); + try { + 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); + + // Step 4: Generate the full-mode patch + const { generateGitPatch } = require("./generate_git_patch.cjs"); + const result = await generateGitPatch("feature-branch", "main", { cwd: repoDir, mode: "full" }); + + expect(result.success).toBe(true); + const patch = fs.readFileSync(result.patchPath, "utf8"); + + // The patch MUST contain the agent's commit + expect(patch).toContain("agent-change.txt"); + // The patch MUST NOT contain the phantom commits from main + expect(patch).not.toContain("phantom1.md"); + expect(patch).not.toContain("phantom2.md"); + } finally { + if (fs.existsSync(remoteDir)) { + fs.rmSync(remoteDir, { recursive: true, force: true }); + } + } + }); +}); From dc5c61d3429cd1810225e1663b9dd6ecda61f723 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Apr 2026 20:53:36 +0000 Subject: [PATCH 3/3] Remove tsessebe/autoloop references and add explicit fetch in regression 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> --- .changeset/patch-create-pr-max-files-config.md | 2 +- .changeset/patch-fix-stale-origin-branch-merge-base.md | 2 +- .changeset/patch-push-pr-incremental-size-check.md | 2 +- .github/aw/github-agentic-workflows.md | 2 +- actions/setup/js/create_pull_request.test.cjs | 2 +- actions/setup/js/generate_git_patch.cjs | 3 +-- actions/setup/js/generate_git_patch.test.cjs | 7 ++++++- actions/setup/js/safe_outputs_handlers.cjs | 2 +- pkg/workflow/compiler_safe_outputs_config_test.go | 2 +- 9 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.changeset/patch-create-pr-max-files-config.md b/.changeset/patch-create-pr-max-files-config.md index c6f6104f910..ee651a94e04 100644 --- a/.changeset/patch-create-pr-max-files-config.md +++ b/.changeset/patch-create-pr-max-files-config.md @@ -1,4 +1,4 @@ --- "gh-aw": patch --- -Fix `create_pull_request` 100-file limit to count unique files (not raw `diff --git` headers, which inflate the count when a single push contains multiple commits touching the same files), and add a configurable `max-patch-files` top-level safe-outputs option (default 100) so long-running autoloop branches can opt into a higher limit. +Fix `create_pull_request` 100-file limit to count unique files (not raw `diff --git` headers, which inflate the count when a single push contains multiple commits touching the same files), and add a configurable `max-patch-files` top-level safe-outputs option (default 100) so long-running branches can opt into a higher limit. diff --git a/.changeset/patch-fix-stale-origin-branch-merge-base.md b/.changeset/patch-fix-stale-origin-branch-merge-base.md index 3aadda87605..a05a4ae4fe5 100644 --- a/.changeset/patch-fix-stale-origin-branch-merge-base.md +++ b/.changeset/patch-fix-stale-origin-branch-merge-base.md @@ -4,6 +4,6 @@ Fixed `create_pull_request` patch (full mode) using a stale `origin/` as the base ref when that remote-tracking ref existed locally, causing the patch to include commits the agent never made. -In `generate_git_patch.cjs` full mode, the code previously short-circuited to `origin/` whenever that ref existed locally. But `origin/` is fetched at workflow startup and can be arbitrarily stale — typically pointing at where the remote branch was before the agent run, not the state of the branch *before the agent made changes*. When the local branch is fast-forwarded to the default branch during the agent run (a common autoloop pattern), the resulting patch erroneously included every commit between the old branch tip and the current `main`. +In `generate_git_patch.cjs` full mode, the code previously short-circuited to `origin/` whenever that ref existed locally. But `origin/` is fetched at workflow startup and can be arbitrarily stale — typically pointing at where the remote branch was before the agent run, not the state of the branch *before the agent made changes*. When the local branch is fast-forwarded to the default branch during the agent run (a common pattern for long-running iterative workflows), the resulting patch erroneously included every commit between the old branch tip and the current `main`. Full mode now always computes the merge-base with the default branch, matching the behavior of the previous fallback path. Incremental mode (`push_to_pull_request_branch`) is unchanged — its use of `origin/` as a base is intentional and correct for that workflow. diff --git a/.changeset/patch-push-pr-incremental-size-check.md b/.changeset/patch-push-pr-incremental-size-check.md index d0bd590d621..a736a632b06 100644 --- a/.changeset/patch-push-pr-incremental-size-check.md +++ b/.changeset/patch-push-pr-incremental-size-check.md @@ -2,7 +2,7 @@ "gh-aw": patch --- -Fixed `push_to_pull_request_branch` `max_patch_size` check incorrectly measuring the patch from the default branch (checkout base) instead of from the existing PR branch head, causing every push to fail on long-running branches that accumulate iterations (e.g. autoloop's accumulating iteration branches). +Fixed `push_to_pull_request_branch` `max_patch_size` check incorrectly measuring the patch from the default branch (checkout base) instead of from the existing PR branch head, causing every push to fail on long-running branches that accumulate iterations. The `max_patch_size` check now uses the **incremental net diff** between `origin/` and the new working state, not the size of the format-patch transport file or the cumulative diff from the default branch. This means the limit reflects how much the branch will actually change in the push, not the cumulative divergence of the long-running branch from `main`. diff --git a/.github/aw/github-agentic-workflows.md b/.github/aw/github-agentic-workflows.md index b87da0d5754..92e6729a3b1 100644 --- a/.github/aw/github-agentic-workflows.md +++ b/.github/aw/github-agentic-workflows.md @@ -1519,7 +1519,7 @@ The YAML frontmatter supports these fields: - Patches exceeding this size are rejected to prevent accidental large changes - `max-patch-files:` - Maximum allowed number of unique files in a create-pull-request patch (integer, default: 100) - Counts unique file paths deduplicated across multi-commit patches; reflects how many distinct files the agent is pushing per iteration - - Increase this limit for long-running autoloop branches that touch many files + - Increase this limit for long-running branches that touch many files - `group-reports:` - Group workflow failure reports as sub-issues (boolean, default: `false`) - When `true`, creates a parent `[aw] Failed runs` issue that tracks all workflow failures as sub-issues; useful for larger repositories - `report-failure-as-issue:` - Control whether workflow failures are reported as GitHub issues (boolean, default: `true`) diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index 9de957ecde8..b119e7d82a7 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -384,7 +384,7 @@ describe("create_pull_request - max limit enforcement", () => { expect(() => enforcePullRequestLimits(patchContent)).not.toThrow(); }); - it("should count unique files across multi-commit patches (regression: github/gh-aw issue tsessebe autoloop)", () => { + it("should count unique files across multi-commit patches (regression: long-running branch with multi-commit patches)", () => { // Simulate `git format-patch` output where the same files are modified across // multiple commits. Previously the limit check counted every `diff --git` // header (3 commits * 2 files = 6) instead of the 2 unique files actually diff --git a/actions/setup/js/generate_git_patch.cjs b/actions/setup/js/generate_git_patch.cjs index f1d7b591fc2..0c0234f8022 100644 --- a/actions/setup/js/generate_git_patch.cjs +++ b/actions/setup/js/generate_git_patch.cjs @@ -173,8 +173,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { // using the stale `origin/${branchName}` would cause the patch to include all // commits from the default branch since the old branch tip — commits the agent // never made. Always compute the merge-base with the default branch so the patch - // contains exactly the agent's changes. See issue githubnext/tsessebe autoloop - // failures for context. + // contains exactly the agent's changes. debugLog(`Strategy 1 (full): Computing merge-base with ${defaultBranch} (ignoring any stale origin/${branchName})`); // Check if origin/ already exists locally (e.g., from checkout with fetch-depth: 0) // This is important for cross-repo checkouts where persist-credentials: false prevents fetching diff --git a/actions/setup/js/generate_git_patch.test.cjs b/actions/setup/js/generate_git_patch.test.cjs index 7af1fee68f9..0498e3a2315 100644 --- a/actions/setup/js/generate_git_patch.test.cjs +++ b/actions/setup/js/generate_git_patch.test.cjs @@ -529,7 +529,7 @@ describe("generateGitPatch – full mode base ref (merge-base, not stale origin) }); it("should NOT include phantom commits when stale origin/ exists (regression test)", async () => { - // Reproduce the autoloop bug scenario: + // Reproduce the stale-remote-tracking-ref bug scenario: // 1. A feature branch was pushed in the past at some old commit // (origin/feature-branch points there — this is the "stale" remote-tracking ref) // 2. main has since advanced with many "phantom" commits the agent never made @@ -548,6 +548,11 @@ describe("generateGitPatch – full mode base ref (merge-base, not stale origin) // 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 }); + // Explicitly fetch to ensure refs/remotes/origin/feature-branch exists in the + // local repo. `git push` typically updates this automatically, but in repos + // created via `git init` + `git remote add` (no clone) the remote-tracking + // ref population can be inconsistent across git versions, so be explicit. + execSync("git fetch origin feature-branch:refs/remotes/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 diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 897c676d15d..037fb61c202 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -643,7 +643,7 @@ function createHandlers(server, appendSafeOutput, config = {}) { // validate `max_patch_size` against the actual incremental change relative // to the existing PR branch head, not the (potentially much larger) size of // the format-patch transport file. This is critical for the long-running - // branch pattern (e.g. autoloop) where the format-patch can include many + // branch pattern where the format-patch can include many // commits but each iteration only changes a few KB. if (typeof patchResult.diffSize === "number" && patchResult.diffSize >= 0) { entry.diff_size = patchResult.diffSize; diff --git a/pkg/workflow/compiler_safe_outputs_config_test.go b/pkg/workflow/compiler_safe_outputs_config_test.go index d28cc04b109..708d7b86d24 100644 --- a/pkg/workflow/compiler_safe_outputs_config_test.go +++ b/pkg/workflow/compiler_safe_outputs_config_test.go @@ -1420,7 +1420,7 @@ func TestHandlerConfigPatchSize(t *testing.T) { // TestHandlerConfigPatchFiles tests that the max-patch-files configuration is // propagated into the create_pull_request handler config (regression for the -// hardcoded 100-file limit reported in the tsessebe autoloop scenario). +// hardcoded 100-file limit for long-running branches with multi-commit patches). func TestHandlerConfigPatchFiles(t *testing.T) { tests := []struct { name string