From ca801f912b999690d150312adcbc9154476b901c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 05:57:03 +0000 Subject: [PATCH 1/3] Initial plan From b3000a3daad4b909ead24679ff10a8c45a0988c6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 06:12:14 +0000 Subject: [PATCH 2/3] Auto-switch to bundle transport when push detects merge commits Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e9136023-f723-4e26-a90b-a615b40f6d6f Co-authored-by: mrjf <180956+mrjf@users.noreply.github.com> --- .../patch-auto-bundle-on-merge-commits.md | 5 + actions/setup/js/git_helpers.cjs | 37 ++++++ actions/setup/js/safe_outputs_handlers.cjs | 23 +++- .../setup/js/safe_outputs_handlers.test.cjs | 117 ++++++++++++++++++ 4 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 .changeset/patch-auto-bundle-on-merge-commits.md diff --git a/.changeset/patch-auto-bundle-on-merge-commits.md b/.changeset/patch-auto-bundle-on-merge-commits.md new file mode 100644 index 00000000000..002984a420d --- /dev/null +++ b/.changeset/patch-auto-bundle-on-merge-commits.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +`push_to_pull_request_branch`: when `patch-format` is not explicitly configured and the incremental range (`origin/..`) contains a merge commit, automatically use `bundle` transport instead of the default `am` transport. `git am` cannot apply merge commits, so without this fallback long-running PR branches that periodically merge their base branch locally would fail with add/add conflicts on every push attempt. Set `patch-format: am` explicitly to opt out of the auto-fallback. diff --git a/actions/setup/js/git_helpers.cjs b/actions/setup/js/git_helpers.cjs index 70becf63bb2..6e55cfdc9fe 100644 --- a/actions/setup/js/git_helpers.cjs +++ b/actions/setup/js/git_helpers.cjs @@ -114,7 +114,44 @@ function execGitSync(args, options = {}) { return result.stdout; } +/** + * Check whether a commit range contains any merge commits. + * + * `git am` (the default patch transport) cannot apply merge commits — it only + * handles linear patches produced by `git format-patch`. Callers can use this + * helper to detect when a range requires the `bundle` transport instead, which + * preserves merge commit topology by transferring git objects directly. + * + * Returns `false` (rather than throwing) when the underlying git command fails + * — for example when one of the refs cannot be resolved. Callers should treat + * "unknown" as "no merge commits detected" so that a detection failure never + * blocks the normal patch path. + * + * @param {string} baseRef - The base ref (exclusive). Example: "origin/feature". + * @param {string} headRef - The head ref (inclusive). Example: "feature". + * @param {Object} [options] + * @param {string} [options.cwd] - Working directory for the git command. + * @returns {boolean} True if at least one merge commit exists in baseRef..headRef. + */ +function hasMergeCommitsInRange(baseRef, headRef, options = {}) { + if (!baseRef || !headRef) return false; + try { + const out = execGitSync(["rev-list", "--merges", "--count", `${baseRef}..${headRef}`], { + cwd: options.cwd, + suppressLogs: true, + }); + const count = parseInt(out.trim(), 10); + return Number.isFinite(count) && count > 0; + } catch { + // Detection failure — treat as no merge commits to avoid blocking the + // normal patch path. The caller's downstream patch generation will surface + // any actionable error. + return false; + } +} + module.exports = { execGitSync, getGitAuthEnv, + hasMergeCommitsInRange, }; diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 606024574e9..8c6e680f334 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -12,6 +12,7 @@ const { getCurrentBranch } = require("./get_current_branch.cjs"); const { getBaseBranch } = require("./get_base_branch.cjs"); const { generateGitPatch } = require("./generate_git_patch.cjs"); const { generateGitBundle } = require("./generate_git_bundle.cjs"); +const { hasMergeCommitsInRange } = require("./git_helpers.cjs"); const { enforceCommentLimits } = require("./comment_limit_helpers.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { ERR_CONFIG, ERR_SYSTEM, ERR_VALIDATION } = require("./error_codes.cjs"); @@ -551,6 +552,10 @@ function createHandlers(server, appendSafeOutput, config = {}) { // "am" (default) uses git format-patch / git am (good for linear histories). // Use ?? (nullish coalescing) so an empty-string resolved value is preserved and // rejected below rather than silently falling back to "am". + // Track whether the user explicitly set patch_format so we can auto-fall-back + // to bundle transport when merge commits are detected (since `git am` cannot + // apply merge commits). When the user explicitly chose a format, respect it. + const patchFormatExplicit = pushConfig["patch_format"] !== undefined || config["patch_format"] !== undefined; const pushPatchFormat = pushConfig["patch_format"] ?? config["patch_format"] ?? "am"; const validPushPatchFormats = ["am", "bundle"]; if (!validPushPatchFormats.includes(pushPatchFormat)) { @@ -569,7 +574,23 @@ function createHandlers(server, appendSafeOutput, config = {}) { isError: true, }; } - const useBundle = pushPatchFormat === "bundle"; + let useBundle = pushPatchFormat === "bundle"; + + // Auto-fallback: when patch_format is not explicitly configured and the + // incremental range (origin/..) contains merge commits, + // automatically switch to bundle transport. `git am` (the default) cannot + // apply merge commits, so without this fallback long-running branches that + // periodically merge their base branch locally would fail with add/add + // conflicts on every push attempt. The detection is best-effort and uses + // only local refs (no extra fetch); a detection miss simply preserves the + // existing behavior. + if (!useBundle && !patchFormatExplicit && entry.branch) { + const hasMerges = hasMergeCommitsInRange(`refs/remotes/origin/${entry.branch}`, entry.branch, { cwd: repoCwd || undefined }); + if (hasMerges) { + server.debug(`push_to_pull_request_branch: detected merge commit(s) in incremental range origin/${entry.branch}..${entry.branch}; auto-switching to bundle transport (set patch-format: am to override).`); + useBundle = true; + } + } // Build common options for both patch and bundle generation const pushTransportOptions = { mode: "incremental" }; diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index b51a8672530..2c72c06cbd3 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -864,6 +864,123 @@ describe("safe_outputs_handlers", () => { expect(responseData.error).toContain("Invalid patch_format"); expect(mockAppendSafeOutput).not.toHaveBeenCalled(); }); + + /** + * Reproduces the long-running-branch scenario from the issue: + * the agent merged the default branch into the PR branch (creating a merge + * commit), then committed additional work on top. The incremental range + * origin/.. therefore contains a merge commit, which + * `git am --3way` cannot apply. The handler should auto-switch to bundle + * transport when patch_format is not explicitly set. + */ + function createSideRepoWithMergeCommitInIncrementalRange() { + const targetRepoDir = path.join(testWorkspaceDir, "target-repo-merge"); + fs.mkdirSync(targetRepoDir, { recursive: true }); + + execSync("git init -b main", { cwd: targetRepoDir, stdio: "pipe" }); + execSync("git config user.email 'test@example.com'", { cwd: targetRepoDir, stdio: "pipe" }); + execSync("git config user.name 'Test User'", { cwd: targetRepoDir, stdio: "pipe" }); + + // Initial commit on main + fs.writeFileSync(path.join(targetRepoDir, "README.md"), "base\n"); + execSync("git add README.md", { cwd: targetRepoDir, stdio: "pipe" }); + execSync("git commit -m 'base commit'", { cwd: targetRepoDir, stdio: "pipe" }); + + // Create the feature branch with one commit + execSync("git checkout -b feature/test-change", { cwd: targetRepoDir, stdio: "pipe" }); + fs.writeFileSync(path.join(targetRepoDir, "feature.md"), "feature work\n"); + execSync("git add feature.md", { cwd: targetRepoDir, stdio: "pipe" }); + execSync("git commit -m 'feature commit'", { cwd: targetRepoDir, stdio: "pipe" }); + const featureCommit = execSync("git rev-parse HEAD", { cwd: targetRepoDir, stdio: "pipe" }).toString().trim(); + + // Snapshot the remote tracking ref at this point — this is what the agent's + // workflow checkout would see. Anything ahead of this is "to be pushed". + execSync("git remote add origin https://github.com/test-owner/test-repo.git", { cwd: targetRepoDir, stdio: "pipe" }); + execSync(`git update-ref refs/remotes/origin/feature/test-change ${featureCommit}`, { cwd: targetRepoDir, stdio: "pipe" }); + + // Advance main with new commits (simulating "branch falls behind") + execSync("git checkout main", { cwd: targetRepoDir, stdio: "pipe" }); + fs.writeFileSync(path.join(targetRepoDir, "main-update.md"), "main moved on\n"); + execSync("git add main-update.md", { cwd: targetRepoDir, stdio: "pipe" }); + execSync("git commit -m 'main update'", { cwd: targetRepoDir, stdio: "pipe" }); + + // Agent merges main into feature branch (creates a merge commit) + execSync("git checkout feature/test-change", { cwd: targetRepoDir, stdio: "pipe" }); + execSync("git merge --no-ff main -m 'Merge main into feature'", { cwd: targetRepoDir, stdio: "pipe" }); + + // Agent makes one more commit on top of the merge + fs.writeFileSync(path.join(targetRepoDir, "feature.md"), "feature work updated\n"); + execSync("git add feature.md", { cwd: targetRepoDir, stdio: "pipe" }); + execSync("git commit -m 'follow-up after merge'", { cwd: targetRepoDir, stdio: "pipe" }); + } + + it("auto-switches to bundle transport when incremental range contains a merge commit (default patch_format)", async () => { + createSideRepoWithMergeCommitInIncrementalRange(); + + process.env.GITHUB_BASE_REF = "main"; + try { + const result = await handlers.pushToPullRequestBranchHandler({ + branch: "feature/test-change", + repo: "test-owner/test-repo", + }); + + expect(result.isError).toBeFalsy(); + const responseData = JSON.parse(result.content[0].text); + expect(responseData.result).toBe("success"); + // Must have generated a bundle, not a patch + expect(responseData.bundle).toBeDefined(); + expect(responseData.patch).toBeUndefined(); + expect(responseData.bundle.path).toMatch(/\.bundle$/); + + // The auto-switch debug message must be emitted so operators can trace why + expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining("auto-switching to bundle transport")); + + expect(mockAppendSafeOutput).toHaveBeenCalledWith( + expect.objectContaining({ + type: "push_to_pull_request_branch", + bundle_path: expect.stringMatching(/\.bundle$/), + }) + ); + // Should NOT have written a patch_path + const appended = mockAppendSafeOutput.mock.calls[0][0]; + expect(appended.patch_path).toBeUndefined(); + } finally { + delete process.env.GITHUB_BASE_REF; + } + }); + + it("respects explicit patch_format: am even when incremental range contains a merge commit", async () => { + createSideRepoWithMergeCommitInIncrementalRange(); + + handlers = createHandlers(mockServer, mockAppendSafeOutput, { + push_to_pull_request_branch: { + patch_format: "am", + }, + }); + + process.env.GITHUB_BASE_REF = "main"; + try { + const result = await handlers.pushToPullRequestBranchHandler({ + branch: "feature/test-change", + repo: "test-owner/test-repo", + }); + + // The user explicitly requested "am", so we must respect it and produce a patch + // even though the range contains a merge commit. (The patch may later fail to + // apply, but that is the user's explicit choice.) + expect(result.isError).toBeFalsy(); + const responseData = JSON.parse(result.content[0].text); + expect(responseData.result).toBe("success"); + expect(responseData.patch).toBeDefined(); + expect(responseData.bundle).toBeUndefined(); + + // Auto-switch debug must NOT have fired + const autoSwitchCalls = mockServer.debug.mock.calls.filter(c => typeof c[0] === "string" && c[0].includes("auto-switching to bundle transport")); + expect(autoSwitchCalls).toHaveLength(0); + } finally { + delete process.env.GITHUB_BASE_REF; + } + }); }); describe("handler structure", () => { From 63532bd84fc40df114c49fae9fb65d92852ae8bf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 07:10:30 +0000 Subject: [PATCH 3/3] Record incremental diff_size for bundle transport in push handler Agent-Logs-Url: https://github.com/github/gh-aw/sessions/89949bd9-a843-4d2e-b39f-771b1552fd9c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../patch-auto-bundle-on-merge-commits.md | 2 +- actions/setup/js/safe_outputs_handlers.cjs | 23 +++++++++++++++++++ .../setup/js/safe_outputs_handlers.test.cjs | 6 +++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/.changeset/patch-auto-bundle-on-merge-commits.md b/.changeset/patch-auto-bundle-on-merge-commits.md index 002984a420d..12597d579e9 100644 --- a/.changeset/patch-auto-bundle-on-merge-commits.md +++ b/.changeset/patch-auto-bundle-on-merge-commits.md @@ -2,4 +2,4 @@ "gh-aw": patch --- -`push_to_pull_request_branch`: when `patch-format` is not explicitly configured and the incremental range (`origin/..`) contains a merge commit, automatically use `bundle` transport instead of the default `am` transport. `git am` cannot apply merge commits, so without this fallback long-running PR branches that periodically merge their base branch locally would fail with add/add conflicts on every push attempt. Set `patch-format: am` explicitly to opt out of the auto-fallback. +`push_to_pull_request_branch`: when `patch-format` is not explicitly configured and the incremental range (`origin/..`) contains a merge commit, automatically use `bundle` transport instead of the default `am` transport. `git am` cannot apply merge commits, so without this fallback long-running PR branches that periodically merge their base branch locally would fail with add/add conflicts on every push attempt. Set `patch-format: am` explicitly to opt out of the auto-fallback. The bundle transport now also records the incremental net `diff_size`, so `max-patch-size` is validated against the actual change size rather than the (often much larger) bundle artifact size. diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 8c6e680f334..892f85a652b 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -13,6 +13,7 @@ const { getBaseBranch } = require("./get_base_branch.cjs"); const { generateGitPatch } = require("./generate_git_patch.cjs"); const { generateGitBundle } = require("./generate_git_bundle.cjs"); const { hasMergeCommitsInRange } = require("./git_helpers.cjs"); +const { computeIncrementalDiffSize } = require("./git_patch_utils.cjs"); const { enforceCommentLimits } = require("./comment_limit_helpers.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { ERR_CONFIG, ERR_SYSTEM, ERR_VALIDATION } = require("./error_codes.cjs"); @@ -636,6 +637,28 @@ function createHandlers(server, appendSafeOutput, config = {}) { entry.base_commit = bundleResult.baseCommit; } + // Compute the incremental net diff size so push_to_pull_request_branch can + // validate `max_patch_size` against how much the branch will actually change, + // rather than the full bundle artifact size (which includes packed git + // objects and per-commit metadata, and can be many MB on long-running + // branches even when each iteration changes only a few KB). Without this, + // the push step falls back to the on-disk bundle size and may reject pushes + // that are within the configured net-diff limit. See + // push_to_pull_request_branch.cjs "Size-check source of truth". + if (bundleResult.baseCommit && entry.branch) { + const tmpDiffPath = `${bundleResult.bundlePath}.diff.tmp`; + const diffSize = computeIncrementalDiffSize({ + baseRef: bundleResult.baseCommit, + headRef: entry.branch, + cwd: pushTransportOptions.cwd || process.env.GITHUB_WORKSPACE || process.cwd(), + tmpPath: tmpDiffPath, + }); + if (typeof diffSize === "number" && diffSize >= 0) { + entry.diff_size = diffSize; + server.debug(`Computed incremental diff_size for bundle: ${diffSize} bytes`); + } + } + appendSafeOutput(entry); return { content: [ diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index 2c72c06cbd3..0259963d389 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -944,6 +944,12 @@ describe("safe_outputs_handlers", () => { // Should NOT have written a patch_path const appended = mockAppendSafeOutput.mock.calls[0][0]; expect(appended.patch_path).toBeUndefined(); + // diff_size must be recorded so the downstream push step can validate + // max_patch_size against the net incremental diff (not the bundle size, + // which on long-running branches accumulates packed git objects and can + // exceed the limit even when the actual change is small). + expect(typeof appended.diff_size).toBe("number"); + expect(appended.diff_size).toBeGreaterThanOrEqual(0); } finally { delete process.env.GITHUB_BASE_REF; }