diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 6ddede69153..dc6a64abca3 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -704,11 +704,13 @@ async function main(config = {}) { // Patches are created with git format-patch, so use git am to apply them // Use --3way to handle cross-repo patches where the patch base may differ from target repo // This allows git to resolve create-vs-modify mismatches when a file exists in target but not source + let patchApplied = false; try { - await exec.exec(`git am --3way ${patchFilePath}`); + await exec.exec("git", ["am", "--3way", patchFilePath]); core.info("Patch applied successfully"); + patchApplied = true; } catch (patchError) { - core.error(`Failed to apply patch: ${patchError instanceof Error ? patchError.message : String(patchError)}`); + core.error(`Failed to apply patch with --3way: ${patchError instanceof Error ? patchError.message : String(patchError)}`); // Investigate why the patch failed by logging git status and the failed patch try { @@ -727,7 +729,56 @@ async function main(config = {}) { core.warning(`Failed to investigate patch failure: ${investigateError instanceof Error ? investigateError.message : String(investigateError)}`); } - return { success: false, error: "Failed to apply patch" }; + // Abort the failed git am before attempting any fallback + try { + await exec.exec("git am --abort"); + core.info("Aborted failed git am"); + } catch (abortError) { + core.warning(`Failed to abort git am: ${abortError instanceof Error ? abortError.message : String(abortError)}`); + } + + // Fallback (Option 1): create the PR branch at the original base commit so the PR + // can still be created. GitHub will show the merge conflicts, allowing manual resolution. + // This handles the case where the target branch received intervening commits after + // the patch was generated, making --3way unable to resolve the conflicts automatically. + core.info("Attempting fallback: create PR branch at original base commit..."); + try { + // Use the base commit recorded at patch generation time. + // The From header in format-patch output contains the agent's new commit SHA + // which does not exist in this checkout, so we cannot derive the base from it. + const originalBaseCommit = pullRequestItem.base_commit; + if (!originalBaseCommit) { + core.warning("No base_commit recorded in safe output entry - fallback not possible"); + } else { + core.info(`Original base commit from patch generation: ${originalBaseCommit}`); + + // Verify the base commit is available in this repo (may not exist cross-repo) + await exec.exec("git", ["cat-file", "-e", originalBaseCommit]); + core.info("Original base commit exists locally - proceeding with fallback"); + + // Re-create the PR branch at the original base commit + await exec.exec(`git checkout ${baseBranch}`); + try { + await exec.exec(`git branch -D ${branchName}`); + } catch { + // Branch may not exist yet, ignore + } + await exec.exec(`git checkout -b ${branchName} ${originalBaseCommit}`); + core.info(`Created branch ${branchName} at original base commit ${originalBaseCommit}`); + + // Apply the patch without --3way; we are on the correct base so it should apply cleanly + await exec.exec(`git am ${patchFilePath}`); + core.info("Patch applied successfully at original base commit"); + core.warning(`PR branch ${branchName} is based on an earlier commit than the current ${baseBranch} HEAD. The pull request will show merge conflicts that require manual resolution.`); + patchApplied = true; + } + } catch (fallbackError) { + core.warning(`Fallback to original base commit failed: ${fallbackError instanceof Error ? fallbackError.message : String(fallbackError)}`); + } + + if (!patchApplied) { + return { success: false, error: "Failed to apply patch" }; + } } // Push the applied commits to the branch (with fallback to issue creation on failure) diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index 0028dee4911..e30f5aee7c4 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -1004,3 +1004,218 @@ describe("create_pull_request - wildcard target-repo", () => { expect(result.success).toBe(false); }); }); + +describe("create_pull_request - patch apply fallback to original base commit", () => { + let tempDir; + let originalEnv; + let patchFilePath; + + const MOCK_BASE_COMMIT_SHA = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; + // Minimal valid format-patch output + const PATCH_CONTENT = + `From a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2 Mon Sep 17 00:00:00 2001\n` + + `From: Test Author \n` + + `Date: Wed, 26 Mar 2026 12:00:00 +0000\n` + + `Subject: [PATCH] Test change\n\n` + + `---\n` + + ` file.txt | 1 +\n\n` + + `diff --git a/file.txt b/file.txt\n` + + `index 1234567..abcdefg 100644\n` + + `--- a/file.txt\n` + + `+++ b/file.txt\n` + + `@@ -1 +1,2 @@\n` + + ` existing content\n` + + `+new content\n` + + `--\n` + + `2.39.0\n`; + + beforeEach(() => { + originalEnv = { ...process.env }; + process.env.GH_AW_WORKFLOW_ID = "test-workflow"; + process.env.GITHUB_REPOSITORY = "test-owner/test-repo"; + process.env.GITHUB_BASE_REF = "main"; + + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-fallback-test-")); + patchFilePath = path.join(tempDir, "test.patch"); + fs.writeFileSync(patchFilePath, PATCH_CONTENT, "utf8"); + + global.core = { + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), + startGroup: vi.fn(), + endGroup: vi.fn(), + summary: { + addRaw: vi.fn().mockReturnThis(), + write: vi.fn().mockResolvedValue(undefined), + }, + }; + global.github = { + rest: { + pulls: { + create: vi.fn().mockResolvedValue({ data: { number: 42, html_url: "https://github.com/test/pull/42", node_id: "PR_42" } }), + requestReviewers: vi.fn().mockResolvedValue({}), + }, + repos: { + get: vi.fn().mockResolvedValue({ data: { default_branch: "main" } }), + }, + issues: { + addLabels: vi.fn().mockResolvedValue({}), + }, + }, + graphql: vi.fn(), + }; + global.context = { + eventName: "workflow_dispatch", + repo: { owner: "test-owner", repo: "test-repo" }, + payload: {}, + }; + + delete require.cache[require.resolve("./create_pull_request.cjs")]; + }); + + afterEach(() => { + for (const key of Object.keys(process.env)) { + if (!(key in originalEnv)) { + delete process.env[key]; + } + } + Object.assign(process.env, originalEnv); + + if (tempDir && fs.existsSync(tempDir)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + + delete global.core; + delete global.github; + delete global.context; + delete global.exec; + vi.clearAllMocks(); + }); + + /** + * Helper to detect git am calls in both formats: + * - exec("git", ["am", "--3way", path]) (array form) + * - exec("git am --3way /path") (string form) + */ + function isGitAmCall(cmd, args) { + if (cmd === "git" && Array.isArray(args) && args[0] === "am") return true; + if (typeof cmd === "string" && cmd.startsWith("git am")) return true; + return false; + } + + function isGitAmAbort(cmd, args) { + if (cmd === "git" && Array.isArray(args) && args[0] === "am" && args.includes("--abort")) return true; + if (typeof cmd === "string" && cmd.includes("am --abort")) return true; + return false; + } + + function isGitAm3Way(cmd, args) { + if (cmd === "git" && Array.isArray(args) && args[0] === "am" && args.includes("--3way")) return true; + if (typeof cmd === "string" && cmd.startsWith("git am --3way")) return true; + return false; + } + + it("should fall back to original base commit when git am --3way fails with merge conflicts", async () => { + let primaryAmAttempted = false; + global.exec = { + exec: vi.fn().mockImplementation((cmd, args) => { + // Fail the first "git am --3way" call to simulate a merge conflict + if (isGitAm3Way(cmd, args) && !primaryAmAttempted) { + primaryAmAttempted = true; + throw new Error("CONFLICT (content): Merge conflict in file.txt"); + } + return Promise.resolve(0); + }), + getExecOutput: vi.fn().mockImplementation((cmd, args) => { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }), + }; + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({}); + + const result = await handler({ title: "Test PR", body: "Test body", patch_path: patchFilePath, branch: "test-branch", base_commit: MOCK_BASE_COMMIT_SHA }, {}); + + expect(result.success).toBe(true); + // Should warn that the PR will show merge conflicts + expect(global.core.warning).toHaveBeenCalledWith(expect.stringContaining("merge conflicts")); + }); + + it("should return error when both git am --3way and the fallback git am fail", async () => { + global.exec = { + exec: vi.fn().mockImplementation((cmd, args) => { + // Fail all git am calls except git am --abort + if (isGitAmCall(cmd, args) && !isGitAmAbort(cmd, args)) { + throw new Error("CONFLICT (content): Merge conflict in file.txt"); + } + return Promise.resolve(0); + }), + getExecOutput: vi.fn().mockImplementation((cmd, args) => { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }), + }; + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({}); + + const result = await handler({ title: "Test PR", body: "Test body", patch_path: patchFilePath, branch: "test-branch", base_commit: MOCK_BASE_COMMIT_SHA }, {}); + + expect(result.success).toBe(false); + expect(result.error).toBe("Failed to apply patch"); + }); + + it("should return error when original base commit is not available (cross-repo scenario)", async () => { + global.exec = { + exec: vi.fn().mockImplementation((cmd, args) => { + // Fail git am --3way + if (isGitAm3Way(cmd, args)) { + throw new Error("CONFLICT (content): Merge conflict in file.txt"); + } + // Fail git cat-file to simulate commit not present in local repo + if (cmd === "git" && Array.isArray(args) && args[0] === "cat-file") { + throw new Error("Not a valid object name"); + } + return Promise.resolve(0); + }), + getExecOutput: vi.fn().mockImplementation((cmd, args) => { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }), + }; + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({}); + + const result = await handler({ title: "Test PR", body: "Test body", patch_path: patchFilePath, branch: "test-branch", base_commit: MOCK_BASE_COMMIT_SHA }, {}); + + expect(result.success).toBe(false); + expect(result.error).toBe("Failed to apply patch"); + }); + + it("should return error when no base_commit is provided and git am --3way fails", async () => { + global.exec = { + exec: vi.fn().mockImplementation((cmd, args) => { + if (isGitAm3Way(cmd, args)) { + throw new Error("CONFLICT (content): Merge conflict in file.txt"); + } + return Promise.resolve(0); + }), + getExecOutput: vi.fn().mockImplementation(() => { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }), + }; + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({}); + + // No base_commit provided - fallback should not be possible + const result = await handler({ title: "Test PR", body: "Test body", patch_path: patchFilePath, branch: "test-branch" }, {}); + + expect(result.success).toBe(false); + expect(result.error).toBe("Failed to apply patch"); + expect(global.core.warning).toHaveBeenCalledWith("No base_commit recorded in safe output entry - fallback not possible"); + }); +}); diff --git a/actions/setup/js/generate_git_patch.cjs b/actions/setup/js/generate_git_patch.cjs index c71f00c3a1c..449c6894f56 100644 --- a/actions/setup/js/generate_git_patch.cjs +++ b/actions/setup/js/generate_git_patch.cjs @@ -148,6 +148,10 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { let patchGenerated = false; let errorMessage = null; + // Track the resolved base commit SHA so consumers (e.g. create_pull_request fallback) + // can use it directly. The From header in format-patch output contains the + // *new* commit SHA which won't exist in the target checkout. + let baseCommitSha = null; try { // Strategy 1: If we have a branch name, check if that branch exists and get its diff @@ -263,6 +267,10 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { } } + // Resolve baseRef to a SHA so we can record it for consumers + baseCommitSha = execGitSync(["rev-parse", baseRef], { cwd }).trim(); + debugLog(`Strategy 1: Resolved baseRef ${baseRef} to SHA ${baseCommitSha}`); + // Count commits to be included const commitCount = parseInt(execGitSync(["rev-list", "--count", `${baseRef}..${branchName}`], { cwd }).trim(), 10); debugLog(`Strategy 1: Found ${commitCount} commits between ${baseRef} and ${branchName}`); @@ -332,6 +340,9 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { execGitSync(["merge-base", "--is-ancestor", githubSha, "HEAD"], { cwd }); debugLog(`Strategy 2: GITHUB_SHA is an ancestor of HEAD`); + // Record GITHUB_SHA as the base commit + baseCommitSha = githubSha; + // Count commits between GITHUB_SHA and HEAD const commitCount = parseInt(execGitSync(["rev-list", "--count", `${githubSha}..HEAD`], { cwd }).trim(), 10); debugLog(`Strategy 2: Found ${commitCount} commits between GITHUB_SHA and HEAD`); @@ -396,6 +407,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { } if (baseCommit) { + baseCommitSha = baseCommit; const patchContent = execGitSync(["format-patch", `${baseCommit}..${branchName}`, "--stdout", ...excludeArgs()], { cwd }); if (patchContent && patchContent.trim()) { @@ -438,12 +450,13 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { }; } - debugLog(`Final: SUCCESS - patchSize=${patchSize} bytes, patchLines=${patchLines}`); + debugLog(`Final: SUCCESS - patchSize=${patchSize} bytes, patchLines=${patchLines}, baseCommit=${baseCommitSha || "(unknown)"}`); return { success: true, patchPath: patchPath, patchSize: patchSize, patchLines: patchLines, + baseCommit: baseCommitSha, }; } diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 6632b9626e9..440bb15958a 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -358,6 +358,13 @@ function createHandlers(server, appendSafeOutput, config = {}) { // Store the patch path in the entry so consumers know which file to use entry.patch_path = patchResult.patchPath; + // Store the base commit SHA so the create_pull_request handler can use it + // directly in the fallback path (the From header in format-patch output + // contains the agent's commit SHA which won't exist in the target checkout) + if (patchResult.baseCommit) { + entry.base_commit = patchResult.baseCommit; + } + appendSafeOutput(entry); return { content: [ @@ -472,6 +479,11 @@ function createHandlers(server, appendSafeOutput, config = {}) { // Store the patch path in the entry so consumers know which file to use entry.patch_path = patchResult.patchPath; + // Store the base commit SHA so the push handler can use it directly + if (patchResult.baseCommit) { + entry.base_commit = patchResult.baseCommit; + } + appendSafeOutput(entry); return { content: [ diff --git a/docs/src/content/docs/reference/safe-outputs-pull-requests.md b/docs/src/content/docs/reference/safe-outputs-pull-requests.md index 65041c063c2..4c44a74f4dd 100644 --- a/docs/src/content/docs/reference/safe-outputs-pull-requests.md +++ b/docs/src/content/docs/reference/safe-outputs-pull-requests.md @@ -63,6 +63,25 @@ When `create-pull-request` is configured, git commands (`checkout`, `branch`, `s By default, PRs created with GitHub Agentic Workflows do not trigger CI. See [Triggering CI](/gh-aw/reference/triggering-ci/) for how to configure CI triggers. +### How PR creation works + +When the coding agent finishes its task, it records the requested changes in a structured output file. A separate, permission-controlled job then reads that output and applies the changes: + +1. The agent's commits are exported as a `git format-patch` file covering everything since the original checkout commit. +2. The safe-output job checks out the target repository and fetches the latest state of the base branch. +3. The patch is applied to a new branch using `git am --3way`. The `--3way` flag allows the patch to succeed even when the agent's source repository differs from the target (for example, in cross-repository workflows). +4. The branch is pushed and the GitHub API creates the pull request. + +### If the target branch has changed + +If commits have been pushed to the base branch after the agent started, two outcomes are possible: + +- **No conflicts** — `git am --3way` resolves the patch cleanly against the updated base. The PR is created normally and targets the current head of the base branch. +- **Conflicts** — if `--3way` cannot resolve the conflicts automatically, the safe-output job falls back to applying the patch at the commit the agent originally branched from. The PR is created with the branch based on that earlier commit, and GitHub's pull request UI shows the conflicts for manual resolution. + +> [!NOTE] +> The fallback to the original base commit requires that commit to be present in the target repository. In cross-repository scenarios where the agent repository's history is unrelated, only the `--3way` attempt is made and a hard failure is returned if that also fails. + ## Push to PR Branch (`push-to-pull-request-branch:`) Pushes changes to a PR's branch. Validates via `title-prefix` and `labels` to ensure only approved PRs receive changes. Multiple pushes per run are supported by setting `max` higher than 1.