Auto-switch push_to_pull_request_branch to bundle transport when merge commits are detected

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,
 };

actions/setup/js/safe_outputs_handlers.cjs

@@ -12,6 +12,8 @@ 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 { 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");
@@ -551,6 +553,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 +575,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/<branch>..<branch>) 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" };
@@ -615,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: [

actions/setup/js/safe_outputs_handlers.test.cjs

@@ -864,6 +864,129 @@ 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/<branch>..<branch> 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();
+        // 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;
+      }
+    });
+
+    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", () => {