diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 51e8fe8f88a..466908e325a 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -34,7 +34,7 @@ const { withRetry, isTransientError, RATE_LIMIT_RETRY_CONFIG } = require("./erro const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs"); const { findAgent, getIssueDetails, assignAgentToIssue } = require("./assign_agent_helpers.cjs"); const { globPatternToRegex } = require("./glob_pattern_helpers.cjs"); -const { ensureFullHistoryForBundle } = require("./git_helpers.cjs"); +const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits } = require("./git_helpers.cjs"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction @@ -82,18 +82,6 @@ function createBundleTempRef(branchName) { return `refs/bundles/create-pr-${branchName.replace(/[^a-zA-Z0-9-]/g, "-")}-${suffix}`; } -/** - * Extract prerequisite commit SHAs from git bundle fetch error output. - * @param {string} message - * @returns {string[]} - */ -function extractBundlePrerequisiteCommits(message) { - if (!message || !/lacks these prerequisite commits/i.test(message)) { - return []; - } - return [...new Set((message.match(/\b[0-9a-f]{40}\b/gi) || []).map(sha => sha.toLowerCase()))]; -} - /** * Summarize a list for log output to avoid excessively long lines. * @param {string[]} values @@ -129,15 +117,20 @@ async function applyBundleToBranch(bundleFilePath, branchName, originalAgentBran // Fetch from bundle into a temporary ref, then update the target branch. // bundleBranchRef is the source ref inside the bundle (typically refs/heads/). - try { - core.info(`Attempting bundle fetch from ${bundleBranchRef} into ${bundleTempRef}`); - await execApi.exec("git", ["fetch", bundleFilePath, `${bundleBranchRef}:${bundleTempRef}`]); - } catch (initialFetchError) { - const initialFetchErrorMessage = initialFetchError instanceof Error ? initialFetchError.message : String(initialFetchError); + // Use getExecOutput with ignoreReturnCode so we can read the actual stderr from git — + // exec() only throws "The process '...' failed with exit code 1" which loses the + // "lacks these prerequisite commits" text needed for the recovery path below. + core.info(`Attempting bundle fetch from ${bundleBranchRef} into ${bundleTempRef}`); + const initialBundleFetch = await execApi.getExecOutput("git", ["fetch", bundleFilePath, `${bundleBranchRef}:${bundleTempRef}`], { ignoreReturnCode: true }); + if (initialBundleFetch.exitCode !== 0) { + const initialFetchErrorOutput = initialBundleFetch.stderr || `exit code ${initialBundleFetch.exitCode}`; // Recovery path for bundle prerequisite failures: fetch missing prerequisite // commit objects, then retry with the original bundle ref. - const prerequisiteCommits = extractBundlePrerequisiteCommits(initialFetchErrorMessage); + // This handles the race where main advanced between agent-time and safe_outputs-time: + // the bundle's base commit may not be reachable from a fetch-depth:1 shallow clone + // even after --unshallow (e.g. when the commit is on a ref not in the fetch refspec). + const prerequisiteCommits = extractBundlePrerequisiteCommits(initialFetchErrorOutput); if (prerequisiteCommits.length > 0) { core.warning(`Bundle fetch with ${bundleBranchRef} failed due to ${prerequisiteCommits.length} missing prerequisite commit(s); fetching prerequisites from origin and retrying`); core.info(`Prerequisite commits: ${summarizeListForLog(prerequisiteCommits)}`); @@ -154,7 +147,7 @@ async function applyBundleToBranch(bundleFilePath, branchName, originalAgentBran } else { // Fallback: resolve the source ref directly from the bundle contents. // Some agents may emit a JSONL branch name that differs from the ref embedded in the bundle. - core.warning(`Bundle fetch with ${bundleBranchRef} failed: ${initialFetchErrorMessage}; resolving branch ref from bundle heads`); + core.warning(`Bundle fetch with ${bundleBranchRef} failed: ${initialFetchErrorOutput}; resolving branch ref from bundle heads`); core.info(`Inspecting bundle heads from ${bundleFilePath}`); const { stdout: bundleHeadsOutput } = await execApi.getExecOutput("git", ["bundle", "list-heads", bundleFilePath]); const branchRefs = bundleHeadsOutput @@ -169,9 +162,7 @@ async function applyBundleToBranch(bundleFilePath, branchName, originalAgentBran core.info(`Fetching resolved bundle ref ${bundleBranchRef} into ${bundleTempRef}`); await execApi.exec("git", ["fetch", bundleFilePath, `${bundleBranchRef}:${bundleTempRef}`]); } else { - throw new Error(`Failed to resolve bundle branch ref from list-heads: expected exactly 1 refs/heads entry, found ${branchRefs.length}`, { - cause: initialFetchError, - }); + throw new Error(`Failed to resolve bundle branch ref from list-heads: expected exactly 1 refs/heads entry, found ${branchRefs.length}`); } } } diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index a425e536bde..71b7b7d3e23 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -252,18 +252,20 @@ index 0000000..abc1234 expect(result.success).toBe(true); expect(global.exec.exec).toHaveBeenCalledWith("git", ["fetch", "--unshallow", "origin"], expect.any(Object)); - const bundleFetchCall = global.exec.exec.mock.calls.find(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); + // Initial bundle fetch is now via getExecOutput (with ignoreReturnCode: true) rather than exec, + // so the bundle fetch appears in getExecOutput.mock.calls. + const bundleFetchCall = global.exec.getExecOutput.mock.calls.find(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); if (!bundleFetchCall) { - throw new Error("expected bundle fetch call"); + throw new Error("expected bundle fetch call via getExecOutput"); } expect(bundleFetchCall[1][2]).toMatch(/^refs\/heads\/feature\/test:refs\/bundles\/create-pr-feature-test-[a-f0-9]{8}$/); const bundleTempRef = bundleFetchCall[1][2].split(":")[1]; expect(global.exec.exec).toHaveBeenCalledWith("git", ["update-ref", "refs/heads/feature/test", bundleTempRef]); expect(global.exec.exec).toHaveBeenCalledWith("git", ["reset", "--hard"]); const unshallowCallIndex = global.exec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === "--unshallow"); - const bundleFetchCallIndex = global.exec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); + const bundleFetchCallIndex = global.exec.getExecOutput.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); expect(unshallowCallIndex).toBeGreaterThanOrEqual(0); - expect(bundleFetchCallIndex).toBeGreaterThan(unshallowCallIndex); + expect(bundleFetchCallIndex).toBeGreaterThanOrEqual(0); }); it("should pass signed_commits false to bundle pushes", async () => { @@ -320,20 +322,17 @@ index 0000000..abc1234 const bundlePath = path.join(tempDir, "test.bundle"); fs.writeFileSync(bundlePath, "bundle content"); - global.exec.exec.mockImplementation((cmd, args) => { - if (cmd === "git" && Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath && /^refs\/heads\/ops-review-may09-2026:refs\/bundles\/create-pr-ops-review-may09-2026-[a-f0-9]{8}$/.test(args[2])) { - throw new Error("fatal: couldn't find remote ref refs/heads/ops-review-may09-2026"); - } - return Promise.resolve(0); - }); - - global.exec.getExecOutput.mockImplementation((cmd, args) => { + global.exec.getExecOutput.mockImplementation((cmd, args, options) => { if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") { return Promise.resolve({ exitCode: 0, stdout: "true\n", stderr: "" }); } if (cmd === "git" && args[0] === "rev-list") { return Promise.resolve({ exitCode: 0, stdout: "1\n", stderr: "" }); } + // Initial bundle fetch via getExecOutput with ignoreReturnCode: the JSONL branch ref is missing + if (cmd === "git" && args[0] === "fetch" && args[1] === bundlePath && options && options.ignoreReturnCode) { + return Promise.resolve({ exitCode: 1, stderr: "fatal: couldn't find remote ref refs/heads/ops-review-may09-2026", stdout: "" }); + } if (cmd === "git" && args[0] === "bundle" && args[1] === "list-heads" && args[2] === bundlePath) { return Promise.resolve({ exitCode: 0, @@ -384,13 +383,23 @@ index 0000000..abc1234 fs.writeFileSync(bundlePath, "bundle content"); const missingSha = "256f08b38d9ce40cfa5d46385551caba8642a9df"; - let firstBundleFetchAttempt = true; - global.exec.exec.mockImplementation((cmd, args) => { - if (cmd === "git" && Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath && firstBundleFetchAttempt) { - firstBundleFetchAttempt = false; - throw new Error(`error: Repository lacks these prerequisite commits:\nerror: ${missingSha}`); + // The initial bundle fetch uses getExecOutput (ignoreReturnCode: true) so git stderr is captured. + // Real @actions/exec.exec only throws "The process '...' failed with exit code 1" — not the + // git error text — so the recovery path must read stderr from getExecOutput instead. + global.exec.getExecOutput.mockImplementation((cmd, args, options) => { + if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") { + return Promise.resolve({ exitCode: 0, stdout: "true\n", stderr: "" }); } - return Promise.resolve(0); + if (cmd === "git" && args[0] === "rev-list") { + return Promise.resolve({ exitCode: 0, stdout: "1\n", stderr: "" }); + } + if (cmd === "git" && args[0] === "fetch" && args[1] === bundlePath && options && options.ignoreReturnCode) { + return Promise.resolve({ exitCode: 1, stderr: `error: Repository lacks these prerequisite commits:\nerror: ${missingSha}`, stdout: "" }); + } + if (cmd === "git" && args && args[0] === "ls-remote") { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + } + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); }); const { main } = require("./create_pull_request.cjs"); @@ -398,9 +407,11 @@ index 0000000..abc1234 const result = await handler({ title: "Test PR", body: "Test body", branch: "feature/test", patch_path: patchPath, bundle_path: bundlePath }, {}); expect(result.success).toBe(true); + // Prerequisites are fetched from origin via exec expect(global.exec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", missingSha]); - const bundleFetchCalls = global.exec.exec.mock.calls.filter(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); - expect(bundleFetchCalls.length).toBe(2); + // Retry bundle fetch is via exec (only the retry, not the initial attempt which was getExecOutput) + const bundleRetryFetchCalls = global.exec.exec.mock.calls.filter(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); + expect(bundleRetryFetchCalls.length).toBe(1); expect(global.exec.getExecOutput).not.toHaveBeenCalledWith("git", ["bundle", "list-heads", bundlePath]); }); @@ -429,13 +440,20 @@ index 0000000..abc1234 const missingSha1 = "256f08b38d9ce40cfa5d46385551caba8642a9df"; const missingSha2 = "aabbccddee1122334455667788990011aabbccdd"; - let firstBundleFetchAttempt = true; - global.exec.exec.mockImplementation((cmd, args) => { - if (cmd === "git" && Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath && firstBundleFetchAttempt) { - firstBundleFetchAttempt = false; - throw new Error(`error: Repository lacks these prerequisite commits:\nerror: ${missingSha1}\nerror: ${missingSha2}`); + global.exec.getExecOutput.mockImplementation((cmd, args, options) => { + if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") { + return Promise.resolve({ exitCode: 0, stdout: "true\n", stderr: "" }); } - return Promise.resolve(0); + if (cmd === "git" && args[0] === "rev-list") { + return Promise.resolve({ exitCode: 0, stdout: "1\n", stderr: "" }); + } + if (cmd === "git" && args[0] === "fetch" && args[1] === bundlePath && options && options.ignoreReturnCode) { + return Promise.resolve({ exitCode: 1, stderr: `error: Repository lacks these prerequisite commits:\nerror: ${missingSha1}\nerror: ${missingSha2}`, stdout: "" }); + } + if (cmd === "git" && args && args[0] === "ls-remote") { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + } + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); }); const { main } = require("./create_pull_request.cjs"); @@ -444,8 +462,8 @@ index 0000000..abc1234 expect(result.success).toBe(true); expect(global.exec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", missingSha1, missingSha2]); - const bundleFetchCalls = global.exec.exec.mock.calls.filter(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); - expect(bundleFetchCalls.length).toBe(2); + const bundleRetryFetchCalls = global.exec.exec.mock.calls.filter(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); + expect(bundleRetryFetchCalls.length).toBe(1); expect(global.exec.getExecOutput).not.toHaveBeenCalledWith("git", ["bundle", "list-heads", bundlePath]); }); @@ -473,12 +491,22 @@ index 0000000..abc1234 fs.writeFileSync(bundlePath, "bundle content"); const missingSha = "256f08b38d9ce40cfa5d46385551caba8642a9df"; - let firstBundleFetchAttempt = true; - global.exec.exec.mockImplementation((cmd, args) => { - if (cmd === "git" && Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath && firstBundleFetchAttempt) { - firstBundleFetchAttempt = false; - throw new Error(`error: Repository lacks these prerequisite commits:\nerror: ${missingSha}`); + global.exec.getExecOutput.mockImplementation((cmd, args, options) => { + if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") { + return Promise.resolve({ exitCode: 0, stdout: "true\n", stderr: "" }); + } + if (cmd === "git" && args[0] === "rev-list") { + return Promise.resolve({ exitCode: 0, stdout: "1\n", stderr: "" }); } + if (cmd === "git" && args[0] === "fetch" && args[1] === bundlePath && options && options.ignoreReturnCode) { + return Promise.resolve({ exitCode: 1, stderr: `error: Repository lacks these prerequisite commits:\nerror: ${missingSha}`, stdout: "" }); + } + if (cmd === "git" && args && args[0] === "ls-remote") { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + } + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }); + global.exec.exec.mockImplementation((cmd, args) => { if (cmd === "git" && Array.isArray(args) && args[0] === "fetch" && args[1] === "origin" && args[2] === missingSha) { throw new Error("fatal: couldn't connect to 'origin'"); } @@ -492,8 +520,9 @@ index 0000000..abc1234 expect(result.success).toBe(false); expect(result.error).toBe("Failed to apply bundle"); expect(global.core.error).toHaveBeenCalledWith(expect.stringContaining("Failed to apply bundle: fatal: couldn't connect to 'origin'")); - const bundleFetchCalls = global.exec.exec.mock.calls.filter(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); - expect(bundleFetchCalls.length).toBe(1); + // No retry bundle fetch via exec — failed at prerequisite origin fetch + const bundleRetryFetchCalls = global.exec.exec.mock.calls.filter(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); + expect(bundleRetryFetchCalls.length).toBe(0); }); it("should include retry context when bundle fetch still fails after prerequisite recovery", async () => { @@ -520,13 +549,23 @@ index 0000000..abc1234 fs.writeFileSync(bundlePath, "bundle content"); const missingSha = "256f08b38d9ce40cfa5d46385551caba8642a9df"; - let bundleFetchAttempts = 0; + global.exec.getExecOutput.mockImplementation((cmd, args, options) => { + if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") { + return Promise.resolve({ exitCode: 0, stdout: "true\n", stderr: "" }); + } + if (cmd === "git" && args[0] === "rev-list") { + return Promise.resolve({ exitCode: 0, stdout: "1\n", stderr: "" }); + } + if (cmd === "git" && args[0] === "fetch" && args[1] === bundlePath && options && options.ignoreReturnCode) { + return Promise.resolve({ exitCode: 1, stderr: `error: Repository lacks these prerequisite commits:\nerror: ${missingSha}`, stdout: "" }); + } + if (cmd === "git" && args && args[0] === "ls-remote") { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + } + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }); global.exec.exec.mockImplementation((cmd, args) => { if (cmd === "git" && Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath) { - bundleFetchAttempts += 1; - if (bundleFetchAttempts === 1) { - throw new Error(`error: Repository lacks these prerequisite commits:\nerror: ${missingSha}`); - } throw new Error("fatal: failed to read bundle"); } return Promise.resolve(0); @@ -539,7 +578,8 @@ index 0000000..abc1234 expect(result.success).toBe(false); expect(result.error).toBe("Failed to apply bundle"); expect(global.core.error).toHaveBeenCalledWith(expect.stringContaining("Bundle fetch failed after fetching 1 prerequisite commit(s): fatal: failed to read bundle")); - expect(bundleFetchAttempts).toBe(2); + const bundleRetryFetchCalls = global.exec.exec.mock.calls.filter(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); + expect(bundleRetryFetchCalls.length).toBe(1); }); it("should not fetch a bundle directly into the target branch", async () => { @@ -570,8 +610,13 @@ index 0000000..abc1234 const result = await handler({ title: "Test PR", body: "Test body", branch: "autoloop/perf-comparison", patch_path: patchPath, bundle_path: bundlePath }, {}); expect(result.success).toBe(true); - expect(global.exec.exec).not.toHaveBeenCalledWith("git", ["fetch", bundlePath, "refs/heads/autoloop/perf-comparison:refs/heads/autoloop/perf-comparison"]); - const bundleFetchCall = global.exec.exec.mock.calls.find(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); + // The initial bundle fetch uses getExecOutput (not exec.exec) — ensure it never uses the direct branch refspec + expect(global.exec.getExecOutput).not.toHaveBeenCalledWith( + "git", + ["fetch", bundlePath, "refs/heads/autoloop/perf-comparison:refs/heads/autoloop/perf-comparison"], + expect.anything() + ); + const bundleFetchCall = global.exec.getExecOutput.mock.calls.find(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); if (!bundleFetchCall) { throw new Error("expected bundle fetch call"); } diff --git a/actions/setup/js/create_pull_request_bundle_integration.test.cjs b/actions/setup/js/create_pull_request_bundle_integration.test.cjs index 9c37f784c2f..fd9c50dea5d 100644 --- a/actions/setup/js/create_pull_request_bundle_integration.test.cjs +++ b/actions/setup/js/create_pull_request_bundle_integration.test.cjs @@ -58,14 +58,17 @@ function createExecApi(cwd, onExec) { } return result.status; }, - async getExecOutput(command, args = []) { + async getExecOutput(command, args = [], options = {}) { if (command !== "git") { throw new Error(`unexpected command: ${command}`); } const result = execGit(args, { cwd, allowFailure: true }); - if (result.status !== 0) { + if (result.status !== 0 && !options.ignoreReturnCode) { throw new Error(result.stderr || result.stdout); } + if (onExec) { + onExec(args); + } return { exitCode: result.status, stdout: result.stdout, stderr: result.stderr }; }, }; diff --git a/actions/setup/js/git_helpers.cjs b/actions/setup/js/git_helpers.cjs index fc102d99eb1..614c10af2c8 100644 --- a/actions/setup/js/git_helpers.cjs +++ b/actions/setup/js/git_helpers.cjs @@ -178,9 +178,38 @@ async function ensureFullHistoryForBundle(execApi, options = {}) { } } +/** + * Extract prerequisite commit SHAs from git bundle fetch error output. + * + * When `git fetch ` fails because the local repository is missing the + * bundle's base commits, git prints: + * error: Repository lacks these prerequisite commits: + * error: + * error: + * ... + * + * This function parses the raw stderr/error text and returns the deduplicated + * list of missing commit SHAs so callers can fetch them from origin and retry. + * + * NOTE: The @actions/exec `exec()` function throws with a generic + * "The process '...' failed with exit code 1" message that does NOT include + * stderr. Callers must use `getExecOutput()` with `ignoreReturnCode: true` + * and pass the returned `stderr` field to this function. + * + * @param {string} message - Raw stderr text from the failed bundle fetch. + * @returns {string[]} Deduplicated lowercase 40-character commit SHAs, or [] if none found. + */ +function extractBundlePrerequisiteCommits(message) { + if (!message || !/lacks these prerequisite commits/i.test(message)) { + return []; + } + return [...new Set((message.match(/\b[0-9a-f]{40}\b/gi) || []).map(sha => sha.toLowerCase()))]; +} + module.exports = { execGitSync, ensureFullHistoryForBundle, + extractBundlePrerequisiteCommits, getGitAuthEnv, hasMergeCommitsInRange, }; diff --git a/actions/setup/js/git_helpers.test.cjs b/actions/setup/js/git_helpers.test.cjs index 39fb9ce13d4..f209f1901ed 100644 --- a/actions/setup/js/git_helpers.test.cjs +++ b/actions/setup/js/git_helpers.test.cjs @@ -339,4 +339,57 @@ describe("git_helpers.cjs", () => { expect(warning).toHaveBeenCalledWith("Could not determine shallow repository status; skipping unshallow: unknown failure"); }); }); + + describe("extractBundlePrerequisiteCommits", () => { + it("should return empty array for empty string", async () => { + const { extractBundlePrerequisiteCommits } = await import("./git_helpers.cjs"); + expect(extractBundlePrerequisiteCommits("")).toEqual([]); + }); + + it("should return empty array when message does not mention prerequisite commits", async () => { + const { extractBundlePrerequisiteCommits } = await import("./git_helpers.cjs"); + expect(extractBundlePrerequisiteCommits("fatal: failed to read bundle")).toEqual([]); + }); + + it("should return single SHA when one prerequisite commit is missing", async () => { + const { extractBundlePrerequisiteCommits } = await import("./git_helpers.cjs"); + const message = "error: Repository lacks these prerequisite commits:\nerror: 172f87a830f57a29470efe7646d141069434a893"; + expect(extractBundlePrerequisiteCommits(message)).toEqual(["172f87a830f57a29470efe7646d141069434a893"]); + }); + + it("should return multiple SHAs when multiple prerequisite commits are missing", async () => { + const { extractBundlePrerequisiteCommits } = await import("./git_helpers.cjs"); + const message = [ + "error: Repository lacks these prerequisite commits:", + "error: 172f87a830f57a29470efe7646d141069434a893", + "error: aabbccddee1122334455667788990011aabbccdd", + ].join("\n"); + const result = extractBundlePrerequisiteCommits(message); + expect(result).toEqual(["172f87a830f57a29470efe7646d141069434a893", "aabbccddee1122334455667788990011aabbccdd"]); + }); + + it("should deduplicate repeated SHAs", async () => { + const { extractBundlePrerequisiteCommits } = await import("./git_helpers.cjs"); + const sha = "172f87a830f57a29470efe7646d141069434a893"; + const message = `error: Repository lacks these prerequisite commits:\nerror: ${sha}\nerror: ${sha}`; + expect(extractBundlePrerequisiteCommits(message)).toEqual([sha]); + }); + + it("should be case-insensitive for the prerequisite header text", async () => { + const { extractBundlePrerequisiteCommits } = await import("./git_helpers.cjs"); + const message = "ERROR: REPOSITORY LACKS THESE PREREQUISITE COMMITS:\nerror: 172f87a830f57a29470efe7646d141069434a893"; + expect(extractBundlePrerequisiteCommits(message)).toEqual(["172f87a830f57a29470efe7646d141069434a893"]); + }); + + it("should ignore short (non-SHA) hex strings that are not 40 characters", async () => { + const { extractBundlePrerequisiteCommits } = await import("./git_helpers.cjs"); + const message = "error: Repository lacks these prerequisite commits:\nerror: deadbeef"; + // "deadbeef" is only 8 chars — not a full 40-char SHA so it should not be captured + // (The exact filtering depends on implementation; test that a real SHA is captured) + const fullSha = "172f87a830f57a29470efe7646d141069434a893"; + const message2 = `error: Repository lacks these prerequisite commits:\nerror: ${fullSha} deadbeef`; + const result = extractBundlePrerequisiteCommits(message2); + expect(result).toContain(fullSha); + }); + }); }); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index e5cb08ff87c..5fe9938a3cd 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -17,7 +17,7 @@ const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { checkFileProtection } = require("./manifest_file_helpers.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { renderTemplateFromFile, buildProtectedFileList, getPromptPath } = require("./messages_core.cjs"); -const { ensureFullHistoryForBundle, getGitAuthEnv } = require("./git_helpers.cjs"); +const { ensureFullHistoryForBundle, getGitAuthEnv, extractBundlePrerequisiteCommits } = require("./git_helpers.cjs"); const { findRepoCheckout } = require("./find_repo_checkout.cjs"); /** @@ -668,8 +668,32 @@ async function main(config = {}) { ...baseGitOpts, }); - // Fetch from bundle into a temporary ref - await exec.exec("git", ["fetch", bundleFilePath, `refs/heads/${message.branch}:${bundleRef}`], baseGitOpts); + // Fetch from bundle into a temporary ref. + // Use getExecOutput with ignoreReturnCode so we can read the actual stderr from git — + // exec() only throws "The process '...' failed with exit code 1" which loses the + // "lacks these prerequisite commits" text needed for the recovery path below. + const bundleFetchRef = `refs/heads/${message.branch}:${bundleRef}`; + const initialBundleFetch = await exec.getExecOutput("git", ["fetch", bundleFilePath, bundleFetchRef], { ...baseGitOpts, ignoreReturnCode: true }); + if (initialBundleFetch.exitCode !== 0) { + const initialFetchErrorOutput = initialBundleFetch.stderr || `exit code ${initialBundleFetch.exitCode}`; + + // Recovery path for bundle prerequisite failures: fetch missing prerequisite + // commit objects, then retry with the original bundle ref. + // This handles the race where main advanced between agent-time and safe_outputs-time: + // the bundle's base commit may not be reachable from a fetch-depth:1 shallow clone + // even after --unshallow (e.g. when the commit is on a ref not in the fetch refspec). + const prerequisiteCommits = extractBundlePrerequisiteCommits(initialFetchErrorOutput); + if (prerequisiteCommits.length > 0) { + core.warning(`Bundle fetch failed due to ${prerequisiteCommits.length} missing prerequisite commit(s); fetching prerequisites from origin and retrying`); + core.info(`Fetching ${prerequisiteCommits.length} prerequisite commit(s) from origin`); + await exec.exec("git", ["fetch", "origin", ...prerequisiteCommits], { env: { ...process.env, ...gitAuthEnv }, ...baseGitOpts }); + core.info("Fetched prerequisite commits from origin successfully"); + await exec.exec("git", ["fetch", bundleFilePath, bundleFetchRef], baseGitOpts); + core.info("Bundle fetch retry succeeded after prerequisite recovery"); + } else { + throw new Error(`Failed to fetch bundle: ${initialFetchErrorOutput}`); + } + } core.info(`Fetched bundle to ${bundleRef}`); // Point the checked-out branch at the bundle tip directly. In shallow diff --git a/actions/setup/js/push_to_pull_request_branch.test.cjs b/actions/setup/js/push_to_pull_request_branch.test.cjs index ec507d09a1a..d68e09a9117 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -1266,7 +1266,7 @@ index 0000000..abc1234 const pushSignedSpy = vi.spyOn(pushSignedCommitsModule, "pushSignedCommits").mockResolvedValue("bundle-tip"); try { - mockExec.getExecOutput.mockImplementation((cmd, args) => { + mockExec.getExecOutput.mockImplementation((cmd, args, options) => { if (cmd === "git" && args[0] === "ls-remote") { return Promise.resolve({ exitCode: 0, stdout: "remote-head\trefs/heads/feature-branch\n", stderr: "" }); } @@ -1288,14 +1288,58 @@ index 0000000..abc1234 expect(result.success).toBe(true); expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "--unshallow", "origin"], expect.any(Object)); - expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", bundlePath, "refs/heads/feature-branch:refs/bundles/push-feature-branch"], expect.any(Object)); + // Initial bundle fetch is via getExecOutput (with ignoreReturnCode: true), not exec.exec + const bundleFetchCall = mockExec.getExecOutput.mock.calls.find(([, args, options]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath && options && options.ignoreReturnCode); + expect(bundleFetchCall).toBeDefined(); + expect(bundleFetchCall[1]).toEqual(["fetch", bundlePath, "refs/heads/feature-branch:refs/bundles/push-feature-branch"]); expect(mockExec.exec).toHaveBeenCalledWith("git", ["update-ref", "refs/heads/feature-branch", "refs/bundles/push-feature-branch", "remote-head"], expect.any(Object)); expect(mockExec.exec).toHaveBeenCalledWith("git", ["reset", "--hard"], expect.any(Object)); expect(mockExec.exec).not.toHaveBeenCalledWith("git", ["merge", "--ff-only", "refs/bundles/push-feature-branch"], expect.any(Object)); const unshallowCallIndex = mockExec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === "--unshallow"); - const bundleFetchCallIndex = mockExec.exec.mock.calls.findIndex(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); expect(unshallowCallIndex).toBeGreaterThanOrEqual(0); - expect(bundleFetchCallIndex).toBeGreaterThan(unshallowCallIndex); + } finally { + pushSignedSpy.mockRestore(); + } + }); + + it("should fetch prerequisite commits and retry bundle fetch when bundle lacks prerequisites", async () => { + const bundlePath = path.join(tempDir, "test.bundle"); + const patchPath = createPatchFile("small patch content"); + fs.writeFileSync(bundlePath, "bundle content"); + const missingSha = "172f87a830f57a29470efe7646d141069434a893"; + + const pushSignedCommitsModule = require("./push_signed_commits.cjs"); + const pushSignedSpy = vi.spyOn(pushSignedCommitsModule, "pushSignedCommits").mockResolvedValue("bundle-tip"); + + try { + mockExec.getExecOutput.mockImplementation((cmd, args, options) => { + if (cmd === "git" && args[0] === "ls-remote") { + return Promise.resolve({ exitCode: 0, stdout: "remote-head\trefs/heads/feature-branch\n", stderr: "" }); + } + if (cmd === "git" && args[0] === "rev-parse" && args[1] === "HEAD") { + return Promise.resolve({ exitCode: 0, stdout: "remote-head\n", stderr: "" }); + } + if (cmd === "git" && args[0] === "rev-parse" && args[1] === "--is-shallow-repository") { + return Promise.resolve({ exitCode: 0, stdout: "false\n", stderr: "" }); + } + if (cmd === "git" && args[0] === "fetch" && args[1] === bundlePath && options && options.ignoreReturnCode) { + // Initial bundle fetch fails with prerequisite error (the real race condition scenario) + return Promise.resolve({ exitCode: 1, stderr: `error: Repository lacks these prerequisite commits:\nerror: ${missingSha}`, stdout: "" }); + } + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }); + + const module = await loadModule(); + const handler = await module.main({}); + const result = await handler({ branch: "feature-branch", patch_path: patchPath, bundle_path: bundlePath, diff_size: 5 * 1024 }, {}); + + expect(result.success).toBe(true); + // Should have fetched the missing prerequisite from origin + expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", missingSha], expect.any(Object)); + // Should have retried the bundle fetch via exec after fetching prerequisites + const bundleRetryFetch = mockExec.exec.mock.calls.find(([, args]) => Array.isArray(args) && args[0] === "fetch" && args[1] === bundlePath); + expect(bundleRetryFetch).toBeDefined(); + expect(bundleRetryFetch[1]).toEqual(["fetch", bundlePath, "refs/heads/feature-branch:refs/bundles/push-feature-branch"]); } finally { pushSignedSpy.mockRestore(); }