Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions actions/setup/js/create_pull_request.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/<bundleBranchRef> which was the agent's working branch.
await exec.exec("git", ["fetch", bundleFilePath, `refs/heads/${bundleBranchRef}:refs/heads/${branchName}`]);
Expand Down
127 changes: 127 additions & 0 deletions actions/setup/js/create_pull_request.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <test@example.com>
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", () => {
Expand Down
29 changes: 29 additions & 0 deletions actions/setup/js/git_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <bundle>`
* 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<void>}
*/
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,
};
65 changes: 64 additions & 1 deletion actions/setup/js/git_helpers.test.cjs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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");
});
});
});
7 changes: 6 additions & 1 deletion actions/setup/js/push_to_pull_request_branch.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

/**
Expand Down Expand Up @@ -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}`);
Expand Down
24 changes: 20 additions & 4 deletions actions/setup/js/push_to_pull_request_branch.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down