diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index d8b3d278ab7..a5004902b69 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -34,6 +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"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction @@ -1266,6 +1267,8 @@ async function main(config = {}) { core.info(`Applying changes from bundle: ${bundleFilePath}`); const bundleBranchRef = originalAgentBranch || branchName; try { + await ensureFullHistoryForBundle(exec); + // Fetch from bundle: creates a local branch pointing to the bundle's tip commit. // The bundle contains refs/heads/ which was the agent's working branch. await exec.exec("git", ["fetch", bundleFilePath, `refs/heads/${bundleBranchRef}:refs/heads/${branchName}`]); diff --git a/actions/setup/js/create_pull_request.test.cjs b/actions/setup/js/create_pull_request.test.cjs index bead8026a88..5d4db78f067 100644 --- a/actions/setup/js/create_pull_request.test.cjs +++ b/actions/setup/js/create_pull_request.test.cjs @@ -132,6 +132,133 @@ describe("create_pull_request - draft policy enforcement", () => { }); }); +describe("create_pull_request - bundle transport shallow checkout", () => { + let tempDir; + let originalEnv; + let pushSignedSpy; + + 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"; + delete process.env.GITHUB_TOKEN; + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "create-pr-bundle-test-")); + + 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-owner/test-repo/pull/42" } }), + }, + 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: {}, + }; + global.exec = { + exec: vi.fn().mockResolvedValue(0), + getExecOutput: vi.fn().mockImplementation((cmd, args) => { + 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 && args[0] === "ls-remote") { + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + } + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); + }), + }; + + const pushSignedCommitsModule = require("./push_signed_commits.cjs"); + pushSignedSpy = vi.spyOn(pushSignedCommitsModule, "pushSignedCommits").mockResolvedValue("bundle-tip"); + delete require.cache[require.resolve("./create_pull_request.cjs")]; + }); + + afterEach(() => { + if (pushSignedSpy) { + pushSignedSpy.mockRestore(); + } + + 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(); + }); + + it("should unshallow before fetching a bundle", async () => { + const patchPath = path.join(tempDir, "test.patch"); + fs.writeFileSync( + patchPath, + `From abc123 Mon Sep 17 00:00:00 2001 +From: Test Author +Date: Mon, 1 Jan 2024 00:00:00 +0000 +Subject: [PATCH] Test commit + +diff --git a/test.txt b/test.txt +new file mode 100644 +index 0000000..abc1234 +--- /dev/null ++++ b/test.txt +@@ -0,0 +1 @@ ++Hello World +-- +2.34.1 +` + ); + const bundlePath = path.join(tempDir, "test.bundle"); + fs.writeFileSync(bundlePath, "bundle content"); + + const { main } = require("./create_pull_request.cjs"); + const handler = await main({ base_branch: "main", preserve_branch_name: true }); + const result = await handler({ title: "Test PR", body: "Test body", branch: "feature/test", patch_path: patchPath, bundle_path: bundlePath }, {}); + + expect(result.success).toBe(true); + expect(global.exec.exec).toHaveBeenCalledWith("git", ["fetch", "--unshallow", "origin"], expect.any(Object)); + expect(global.exec.exec).toHaveBeenCalledWith("git", ["fetch", bundlePath, "refs/heads/feature/test:refs/heads/feature/test"]); + 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); + expect(unshallowCallIndex).toBeGreaterThanOrEqual(0); + expect(bundleFetchCallIndex).toBeGreaterThan(unshallowCallIndex); + }); +}); + describe("create_pull_request - fallback-as-issue configuration", () => { describe("configuration parsing", () => { it("should default fallback_as_issue to true when not specified", () => { diff --git a/actions/setup/js/git_helpers.cjs b/actions/setup/js/git_helpers.cjs index 6e55cfdc9fe..fc102d99eb1 100644 --- a/actions/setup/js/git_helpers.cjs +++ b/actions/setup/js/git_helpers.cjs @@ -150,8 +150,37 @@ function hasMergeCommitsInRange(baseRef, headRef, options = {}) { } } +/** + * Ensure the current repository has full history before fetching a git bundle. + * + * Bundles generated from a commit range can declare prerequisite commits. A + * depth-1 checkout may not contain those prerequisites, and `git fetch ` + * rejects the bundle before the caller can update refs. Unshallowing first makes + * the prerequisites available while avoiding a no-op network fetch for full + * checkouts. + * + * @param {{ getExecOutput: Function, exec: Function }} execApi - Exec API to run git commands. + * @param {Object} [options] - Options passed through to exec calls. + * @returns {Promise} + */ +async function ensureFullHistoryForBundle(execApi, options = {}) { + let stdout; + try { + ({ stdout } = await execApi.getExecOutput("git", ["rev-parse", "--is-shallow-repository"], options)); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + core.warning(`Could not determine shallow repository status; skipping unshallow: ${message}`); + return; + } + if (stdout.trim() === "true") { + core.info("Repository is shallow; fetching full history before applying bundle"); + await execApi.exec("git", ["fetch", "--unshallow", "origin"], options); + } +} + module.exports = { execGitSync, + ensureFullHistoryForBundle, getGitAuthEnv, hasMergeCommitsInRange, }; diff --git a/actions/setup/js/git_helpers.test.cjs b/actions/setup/js/git_helpers.test.cjs index 63f5d886a06..39fb9ce13d4 100644 --- a/actions/setup/js/git_helpers.test.cjs +++ b/actions/setup/js/git_helpers.test.cjs @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; describe("git_helpers.cjs", () => { let originalCore; @@ -22,6 +22,11 @@ describe("git_helpers.cjs", () => { global.core = originalCore; }); + function mockCoreWarning() { + global.core.warning = vi.fn(); + return global.core.warning; + } + describe("execGitSync", () => { it("should export execGitSync function", async () => { const { execGitSync } = await import("./git_helpers.cjs"); @@ -276,4 +281,62 @@ describe("git_helpers.cjs", () => { expect(env.GIT_CONFIG_KEY_0).toBe("http.https://github.example.com/.extraheader"); }); }); + + describe("ensureFullHistoryForBundle", () => { + it("should fetch full history when the repository is shallow", async () => { + const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs"); + const execApi = { + getExecOutput: vi.fn().mockResolvedValue({ stdout: "true\n" }), + exec: vi.fn().mockResolvedValue(0), + }; + const options = { cwd: "/tmp/repo" }; + + await ensureFullHistoryForBundle(execApi, options); + + expect(execApi.getExecOutput).toHaveBeenCalledWith("git", ["rev-parse", "--is-shallow-repository"], options); + expect(execApi.exec).toHaveBeenCalledWith("git", ["fetch", "--unshallow", "origin"], options); + }); + + it("should not fetch full history when the repository is not shallow", async () => { + const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs"); + const execApi = { + getExecOutput: vi.fn().mockResolvedValue({ stdout: "false\n" }), + exec: vi.fn().mockResolvedValue(0), + }; + + await ensureFullHistoryForBundle(execApi); + + expect(execApi.exec).not.toHaveBeenCalled(); + }); + + it("should skip unshallow when shallow status cannot be determined", async () => { + const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs"); + const warning = mockCoreWarning(); + const execApi = { + getExecOutput: vi.fn().mockRejectedValue(new Error("not a git repository")), + exec: vi.fn().mockResolvedValue(0), + }; + + await ensureFullHistoryForBundle(execApi); + + expect(execApi.exec).not.toHaveBeenCalled(); + expect(warning).toHaveBeenCalledTimes(1); + expect(warning).toHaveBeenCalledWith("Could not determine shallow repository status; skipping unshallow: not a git repository"); + }); + + it("should warn with stringified non-error shallow status failures", async () => { + const { ensureFullHistoryForBundle } = await import("./git_helpers.cjs"); + const warning = mockCoreWarning(); + const execApi = { + getExecOutput: vi.fn().mockRejectedValue("unknown failure"), + exec: vi.fn().mockResolvedValue(0), + }; + + await ensureFullHistoryForBundle(execApi); + + expect(execApi.exec).not.toHaveBeenCalled(); + expect(warning).toHaveBeenCalledTimes(1); + expect(warning).toHaveBeenCalledWith("Could not determine shallow repository status; skipping unshallow: unknown failure"); + }); + }); }); diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 6aecf628af4..b36c81e90af 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 { getGitAuthEnv } = require("./git_helpers.cjs"); +const { ensureFullHistoryForBundle, getGitAuthEnv } = require("./git_helpers.cjs"); const { findRepoCheckout } = require("./find_repo_checkout.cjs"); /** @@ -599,6 +599,11 @@ async function main(config = {}) { core.info(`Applying changes from bundle: ${bundleFilePath}`); const bundleRef = `refs/bundles/push-${branchName.replace(/[^a-zA-Z0-9-]/g, "-")}`; try { + await ensureFullHistoryForBundle(exec, { + env: { ...process.env, ...gitAuthEnv }, + ...baseGitOpts, + }); + // Fetch from bundle into a temporary ref await exec.exec("git", ["fetch", bundleFilePath, `refs/heads/${message.branch}:${bundleRef}`], baseGitOpts); core.info(`Fetched bundle to ${bundleRef}`); 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 075a6f7edc7..e07ef79eca4 100644 --- a/actions/setup/js/push_to_pull_request_branch.test.cjs +++ b/actions/setup/js/push_to_pull_request_branch.test.cjs @@ -1213,20 +1213,36 @@ index 0000000..abc1234 const pushSignedSpy = vi.spyOn(pushSignedCommitsModule, "pushSignedCommits").mockResolvedValue("bundle-tip"); try { - mockExec.getExecOutput - .mockResolvedValueOnce({ exitCode: 0, stdout: "remote-head\trefs/heads/feature-branch\n", stderr: "" }) // preflight ls-remote - .mockResolvedValueOnce({ exitCode: 0, stdout: "remote-head\n", stderr: "" }) // rev-parse HEAD before bundle - .mockResolvedValueOnce({ exitCode: 0, stdout: "2\n", stderr: "" }); // rev-list --count + mockExec.getExecOutput.mockImplementation((cmd, args) => { + 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: "true\n", stderr: "" }); + } + if (cmd === "git" && args[0] === "rev-list") { + return Promise.resolve({ exitCode: 0, stdout: "2\n", stderr: "" }); + } + 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); + 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)); 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(); }