Skip to content
Closed
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
35 changes: 2 additions & 33 deletions actions/setup/js/generate_git_patch.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ function debugLog(message) {
* In incremental mode, origin/branchName is fetched explicitly and merge-base fallback is disabled.
* @param {string} [options.cwd] - Working directory for git commands. Defaults to GITHUB_WORKSPACE or process.cwd().
* Use this for multi-repo scenarios where repos are checked out to subdirectories.
* @param {string} [options.workspacePath] - Path relative to GITHUB_WORKSPACE used as git working directory.
* When set, this takes precedence over options.cwd and must resolve to a directory under GITHUB_WORKSPACE.
* @param {string} [options.repoSlug] - Repository slug (owner/repo) to include in patch filename for disambiguation.
* Required for multi-repo scenarios to prevent patch file collisions.
* @param {string} [options.token] - GitHub token for git authentication. Falls back to GITHUB_TOKEN env var.
Expand All @@ -53,8 +51,8 @@ function debugLog(message) {
*/
async function generateGitPatch(branchName, baseBranch, options = {}) {
const mode = options.mode || "full";
const workspaceRoot = process.env.GITHUB_WORKSPACE || process.cwd();
let cwd = options.cwd || workspaceRoot;
// Support custom cwd for multi-repo scenarios
const cwd = options.cwd || process.env.GITHUB_WORKSPACE || process.cwd();
// Include repo slug in patch path for multi-repo disambiguation

// Build :(exclude) pathspec arguments from the excludedFiles option.
Expand All @@ -72,35 +70,6 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
}
const patchPath = options.repoSlug ? getPatchPathForRepo(branchName, options.repoSlug) : getPatchPath(branchName);

if (options.workspacePath !== undefined && options.workspacePath !== null && String(options.workspacePath).trim() !== "") {
const root = path.resolve(workspaceRoot);
const candidate = path.resolve(root, String(options.workspacePath));
const relative = path.relative(root, candidate);
if (relative.startsWith("..") || path.isAbsolute(relative)) {
const errorMessage = `Invalid workspacePath '${String(options.workspacePath)}': path must stay under GITHUB_WORKSPACE`;
return {
success: false,
error: errorMessage,
patchPath,
};
}
if (!fs.existsSync(candidate)) {
return {
success: false,
error: `Invalid workspacePath '${String(options.workspacePath)}': directory does not exist`,
patchPath,
};
}
if (!fs.statSync(candidate).isDirectory()) {
return {
success: false,
error: `Invalid workspacePath '${String(options.workspacePath)}': path is not a directory`,
patchPath,
};
}
cwd = candidate;
}

// Validate baseBranch early to avoid confusing git errors (e.g., origin/undefined)
if (typeof baseBranch !== "string" || baseBranch.trim() === "") {
const errorMessage = "baseBranch is required and must be a non-empty string (received: " + String(baseBranch) + ")";
Expand Down
71 changes: 0 additions & 71 deletions actions/setup/js/generate_git_patch.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -282,77 +282,6 @@ describe("generateGitPatch - cross-repo checkout scenarios", () => {
});
});

describe("generateGitPatch - workspacePath option", () => {
let workspaceDir;
let repoDir;
let remoteDir;
let originalEnv;

beforeEach(() => {
originalEnv = { GITHUB_WORKSPACE: process.env.GITHUB_WORKSPACE, GITHUB_SHA: process.env.GITHUB_SHA };
global.core = { debug: () => {}, info: () => {}, warning: () => {}, error: () => {} };
workspaceDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-patch-workspace-"));
repoDir = path.join(workspaceDir, "proxy-frontend");
fs.mkdirSync(repoDir, { recursive: true });

execSync("git init -b main", { cwd: repoDir, stdio: "pipe" });
execSync('git config user.email "test@example.com"', { cwd: repoDir, stdio: "pipe" });
execSync('git config user.name "Test"', { cwd: repoDir, stdio: "pipe" });

fs.writeFileSync(path.join(repoDir, "README.md"), "# Repo\n");
execSync("git add README.md", { cwd: repoDir, stdio: "pipe" });
execSync('git commit -m "init"', { cwd: repoDir, stdio: "pipe" });

execSync("git checkout -b feature/workspace-path", { cwd: repoDir, stdio: "pipe" });
fs.writeFileSync(path.join(repoDir, "feature.txt"), "new change\n");
execSync("git add feature.txt", { cwd: repoDir, stdio: "pipe" });
execSync('git commit -m "feature"', { cwd: repoDir, stdio: "pipe" });

remoteDir = fs.mkdtempSync(path.join(os.tmpdir(), "gh-aw-patch-workspace-remote-"));
execSync("git init --bare -b main", { cwd: remoteDir, stdio: "pipe" });
execSync(`git remote add origin ${remoteDir}`, { cwd: repoDir, stdio: "pipe" });
execSync("git checkout main", { cwd: repoDir, stdio: "pipe" });
execSync("git push origin main", { cwd: repoDir, stdio: "pipe" });
execSync("git checkout feature/workspace-path", { cwd: repoDir, stdio: "pipe" });

process.env.GITHUB_WORKSPACE = workspaceDir;
delete process.env.GITHUB_SHA;
delete require.cache[require.resolve("./generate_git_patch.cjs")];
});

afterEach(() => {
Object.entries(originalEnv).forEach(([key, value]) => {
if (value !== undefined) process.env[key] = value;
else delete process.env[key];
});
if (workspaceDir && fs.existsSync(workspaceDir)) {
fs.rmSync(workspaceDir, { recursive: true, force: true });
}
if (remoteDir && fs.existsSync(remoteDir)) {
fs.rmSync(remoteDir, { recursive: true, force: true });
}
delete require.cache[require.resolve("./generate_git_patch.cjs")];
delete global.core;
});

it("should generate the patch from workspacePath when provided", async () => {
const { generateGitPatch } = require("./generate_git_patch.cjs");
const result = await generateGitPatch("feature/workspace-path", "main", { workspacePath: "proxy-frontend" });

expect(result.success).toBe(true);
const patch = fs.readFileSync(result.patchPath, "utf8");
expect(patch).toContain("feature.txt");
});

it("should reject workspacePath that escapes GITHUB_WORKSPACE", async () => {
const { generateGitPatch } = require("./generate_git_patch.cjs");
const result = await generateGitPatch("feature/workspace-path", "main", { workspacePath: "../outside" });

expect(result.success).toBe(false);
expect(result.error).toContain("Invalid workspacePath");
});
});

describe("sanitizeBranchNameForPatch", () => {
it("should sanitize branch names with path separators", async () => {
const { sanitizeBranchNameForPatch } = await import("./generate_git_patch.cjs");
Expand Down
84 changes: 2 additions & 82 deletions actions/setup/js/safe_outputs_handlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -96,31 +96,6 @@ function isAllowedBranch(branch, allowedPatterns) {
return false;
}

/**
* Resolve and validate a workspace-relative patch path.
* @param {string|undefined} workspacePath
* @returns {{success: true, absolutePath: string} | {success: false, error: string}}
*/
function resolvePatchWorkspacePath(workspacePath) {
const candidatePath = typeof workspacePath === "string" ? workspacePath.trim() : "";
if (!candidatePath) {
return { success: false, error: "patch_workspace_path is empty" };
}
const workspaceRoot = path.resolve(process.env.GITHUB_WORKSPACE || process.cwd());
const resolved = path.resolve(workspaceRoot, candidatePath);
const relative = path.relative(workspaceRoot, resolved);
if (relative.startsWith("..") || path.isAbsolute(relative)) {
return { success: false, error: `Invalid patch_workspace_path '${candidatePath}': path must stay under GITHUB_WORKSPACE` };
}
if (!fs.existsSync(resolved)) {
return { success: false, error: `Invalid patch_workspace_path '${candidatePath}': directory does not exist` };
}
if (!fs.statSync(resolved).isDirectory()) {
return { success: false, error: `Invalid patch_workspace_path '${candidatePath}': path is not a directory` };
}
return { success: true, absolutePath: resolved };
}

/**
* Create handlers for safe output tools
* @param {Object} server - The MCP server instance for logging
Expand Down Expand Up @@ -366,32 +341,8 @@ function createHandlers(server, appendSafeOutput, config = {}) {
// If repo is specified or configured, find where it's checked out
let repoCwd = null;
let repoSlug = null;
const patchWorkspacePath = typeof prConfig.patch_workspace_path === "string" ? prConfig.patch_workspace_path.trim() : "";
const currentCheckoutRepo = typeof prConfig.current_checkout_repo === "string" ? prConfig.current_checkout_repo.trim() : "";
const patchWorkspaceMatchesTargetRepo = patchWorkspacePath && (!currentCheckoutRepo || currentCheckoutRepo === repoResult.repo);

if (patchWorkspaceMatchesTargetRepo) {
const patchWorkspaceResult = resolvePatchWorkspacePath(patchWorkspacePath);
if (!patchWorkspaceResult.success) {
return {
content: [
{
type: "text",
text: JSON.stringify({
result: "error",
error: patchWorkspaceResult.error,
}),
},
],
isError: true,
};
}
repoCwd = patchWorkspaceResult.absolutePath;
repoSlug = repoResult.repo;
server.debug(`Using configured patch_workspace_path for create_pull_request: ${patchWorkspacePath} -> ${repoCwd}`);
}

if (((entry.repo && entry.repo.trim()) || prConfig["target-repo"]) && !repoCwd) {
if ((entry.repo && entry.repo.trim()) || prConfig["target-repo"]) {
// Use the validated/qualified repo slug from repoResult to avoid divergence
// between the raw user input and the normalized/qualified repo name
repoSlug = repoResult.repo;
Expand Down Expand Up @@ -559,9 +510,6 @@ function createHandlers(server, appendSafeOutput, config = {}) {
server.debug(`Generating patch for create_pull_request with branch: ${entry.branch}${repoCwd ? ` in ${repoCwd} baseBranch: ${baseBranch}` : ""}`);
/** @type {Record<string, any>} */
const patchOptions = { ...transportOptions };
if (patchWorkspaceMatchesTargetRepo) {
patchOptions.workspacePath = patchWorkspacePath;
}
// Pass excluded_files so git excludes them via :(exclude) pathspecs at generation time.
if (Array.isArray(prConfig.excluded_files) && prConfig.excluded_files.length > 0) {
patchOptions.excludedFiles = prConfig.excluded_files;
Expand Down Expand Up @@ -720,32 +668,7 @@ function createHandlers(server, appendSafeOutput, config = {}) {
// generation runs from the correct directory when the target repo is checked out in a subdirectory.
let repoCwd = null;
const itemRepo = repoResult.repo;
const pushPatchWorkspacePath = typeof pushConfig.patch_workspace_path === "string" ? pushConfig.patch_workspace_path.trim() : "";
const pushCurrentCheckoutRepo = typeof pushConfig.current_checkout_repo === "string" ? pushConfig.current_checkout_repo.trim() : "";
const pushPatchWorkspaceMatchesTargetRepo = pushPatchWorkspacePath && (!pushCurrentCheckoutRepo || pushCurrentCheckoutRepo === itemRepo);

if (pushPatchWorkspaceMatchesTargetRepo) {
const patchWorkspaceResult = resolvePatchWorkspacePath(pushPatchWorkspacePath);
if (!patchWorkspaceResult.success) {
return {
content: [
{
type: "text",
text: JSON.stringify({
result: "error",
error: patchWorkspaceResult.error,
}),
},
],
isError: true,
};
}
repoCwd = patchWorkspaceResult.absolutePath;
entry.repo_cwd = repoCwd;
server.debug(`Using configured patch_workspace_path for push_to_pull_request_branch: ${pushPatchWorkspacePath} -> ${repoCwd}`);
}

if (((entry.repo && entry.repo.trim()) || pushConfig["target-repo"]) && !repoCwd) {
if ((entry.repo && entry.repo.trim()) || pushConfig["target-repo"]) {
server.debug(`Looking for checkout of target repo: ${itemRepo}`);
const checkoutResult = findRepoCheckout(itemRepo);
if (!checkoutResult.success) {
Expand Down Expand Up @@ -881,9 +804,6 @@ function createHandlers(server, appendSafeOutput, config = {}) {
server.debug(`Generating incremental patch for push_to_pull_request_branch with branch: ${entry.branch}, baseBranch: ${baseBranch}`);
/** @type {Record<string, any>} */
const pushPatchOptions = { ...pushTransportOptions };
if (pushPatchWorkspaceMatchesTargetRepo) {
pushPatchOptions.workspacePath = pushPatchWorkspacePath;
}
// Pass excluded_files so git excludes them via :(exclude) pathspecs at generation time.
if (Array.isArray(pushConfig.excluded_files) && pushConfig.excluded_files.length > 0) {
pushPatchOptions.excludedFiles = pushConfig.excluded_files;
Expand Down
50 changes: 0 additions & 50 deletions actions/setup/js/safe_outputs_handlers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -982,31 +982,6 @@ describe("safe_outputs_handlers", () => {
expect(patchContent).toContain("release tracked commit");
expect(patchContent).not.toContain("MAIN_ONLY.md");
});

it("should use patch_workspace_path when target repo resolves from GH_AW_TARGET_REPO_SLUG", async () => {
createSideRepoOnReleaseBranchWithLocalCommit();
process.env.GH_AW_TARGET_REPO_SLUG = "test-owner/test-repo";
handlers = createHandlers(mockServer, mockAppendSafeOutput, {
create_pull_request: {
patch_workspace_path: "target-repo",
current_checkout_repo: "test-owner/test-repo",
patch_format: "am",
},
});

try {
const result = await handlers.createPullRequestHandler({
branch: "release-1.12.x",
title: "Release fix",
body: "Prepare release branch fix",
});

expect(result.isError).toBeUndefined();
expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining("Using configured patch_workspace_path for create_pull_request"));
} finally {
delete process.env.GH_AW_TARGET_REPO_SLUG;
}
});
});

describe("pushToPullRequestBranchHandler", () => {
Expand Down Expand Up @@ -1134,31 +1109,6 @@ describe("safe_outputs_handlers", () => {
expect(responseData.error).toContain("'path' input");
});

it("should use patch_workspace_path when target repo resolves from GH_AW_TARGET_REPO_SLUG", async () => {
createSideRepoWithTrackedAndLocalCommits();
process.env.GH_AW_TARGET_REPO_SLUG = "test-owner/test-repo";
const handlersWithPatchWorkspace = createHandlers(mockServer, mockAppendSafeOutput, {
push_to_pull_request_branch: {
patch_workspace_path: "target-repo",
current_checkout_repo: "test-owner/test-repo",
patch_format: "am",
},
});

process.env.GITHUB_BASE_REF = "main";
try {
const result = await handlersWithPatchWorkspace.pushToPullRequestBranchHandler({
branch: "feature/test-change",
});

expect(result.isError).toBeFalsy();
expect(mockServer.debug).toHaveBeenCalledWith(expect.stringContaining("Using configured patch_workspace_path for push_to_pull_request_branch"));
} finally {
delete process.env.GH_AW_TARGET_REPO_SLUG;
delete process.env.GITHUB_BASE_REF;
}
});

it("should reject push_to_pull_request_branch when branch still equals base_branch after detection", async () => {
// Simulates the scenario where getBaseBranch() incorrectly resolves to the
// feature branch itself. Detection cannot recover when getCurrentBranch()
Expand Down
Loading