From 7a1cf7e59b5ed11db97e76af75857a1908197ea6 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 27 Feb 2026 01:52:35 +0000 Subject: [PATCH 1/8] improve base branch resolution for cross-repo PRs --- actions/setup/js/create_agent_session.cjs | 13 +++-- actions/setup/js/create_pull_request.cjs | 15 ++++-- actions/setup/js/generate_git_patch.cjs | 12 ++--- actions/setup/js/generate_git_patch.test.cjs | 48 ++++++++----------- actions/setup/js/get_base_branch.cjs | 13 +++-- actions/setup/js/get_base_branch.test.cjs | 19 ++++++++ .../setup/js/git_patch_integration.test.cjs | 6 +-- .../setup/js/push_to_pull_request_branch.cjs | 17 +++++-- actions/setup/js/safe_outputs_handlers.cjs | 23 ++++++--- .../setup/js/safe_outputs_handlers.test.cjs | 11 +++++ 10 files changed, 118 insertions(+), 59 deletions(-) diff --git a/actions/setup/js/create_agent_session.cjs b/actions/setup/js/create_agent_session.cjs index ddec23a8fd..b1c88a8ea2 100644 --- a/actions/setup/js/create_agent_session.cjs +++ b/actions/setup/js/create_agent_session.cjs @@ -2,7 +2,7 @@ /// const { getErrorMessage } = require("./error_helpers.cjs"); -const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo, parseRepoSlug } = require("./repo_helpers.cjs"); const { getBaseBranch } = require("./get_base_branch.cjs"); const fs = require("fs"); @@ -61,12 +61,18 @@ async function main() { allowed_repos: process.env.GH_AW_ALLOWED_REPOS, }); + // Parse default target repo for use in base branch resolution + // This is needed for cross-repo scenarios where the base branch API call + // should target the configured repo, not context.repo + const defaultTargetRepoParts = parseRepoSlug(defaultTargetRepo); + if (isStaged) { let summaryContent = "## 🎭 Staged Mode: Create Agent Sessions Preview\n\n"; summaryContent += "The following agent sessions would be created if staged mode was disabled:\n\n"; // Resolve base branch: use custom config if set, otherwise resolve dynamically - const baseBranch = process.env.GITHUB_AW_AGENT_SESSION_BASE || (await getBaseBranch()); + // Pass target repo for cross-repo scenarios + const baseBranch = process.env.GITHUB_AW_AGENT_SESSION_BASE || (await getBaseBranch(defaultTargetRepoParts)); for (const [index, item] of createAgentSessionItems.entries()) { summaryContent += `### Task ${index + 1}\n\n`; @@ -88,7 +94,8 @@ async function main() { // Resolve base branch: use custom config if set, otherwise resolve dynamically // Dynamic resolution is needed for issue_comment events on PRs where the base branch // is not available in GitHub Actions expressions and requires an API call - const baseBranch = process.env.GITHUB_AW_AGENT_SESSION_BASE || (await getBaseBranch()); + // Pass target repo for cross-repo scenarios + const baseBranch = process.env.GITHUB_AW_AGENT_SESSION_BASE || (await getBaseBranch(defaultTargetRepoParts)); // Process all agent session items const createdTasks = []; diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index da36f45b27..63f2cc3688 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -11,7 +11,7 @@ const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_titl const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { replaceTemporaryIdReferences, isTemporaryId } = require("./temporary_id.cjs"); -const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo, parseRepoSlug } = require("./repo_helpers.cjs"); const { addExpirationToFooter } = require("./ephemerals.cjs"); const { generateWorkflowIdMarker } = require("./generate_footer.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); @@ -100,12 +100,19 @@ async function main(config = {}) { const autoMerge = parseBoolTemplatable(config.auto_merge, false); const expiresHours = config.expires ? parseInt(String(config.expires), 10) : 0; const maxCount = config.max || 1; // PRs are typically limited to 1 + const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024; + const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + + // Parse default target repo for use in base branch resolution + // This is needed for cross-repo scenarios where the base branch API call + // should target the configured repo, not context.repo + const defaultTargetRepoParts = parseRepoSlug(defaultTargetRepo); + // Resolve base branch: use config value if set, otherwise resolve dynamically // Dynamic resolution is needed for issue_comment events on PRs where the base branch // is not available in GitHub Actions expressions and requires an API call - let baseBranch = config.base_branch || (await getBaseBranch()); - const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024; - const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + // Pass target repo for cross-repo scenarios + let baseBranch = config.base_branch || (await getBaseBranch(defaultTargetRepoParts)); const includeFooter = parseBoolTemplatable(config.footer, true); const fallbackAsIssue = config.fallback_as_issue !== false; // Default to true (fallback enabled) diff --git a/actions/setup/js/generate_git_patch.cjs b/actions/setup/js/generate_git_patch.cjs index 384f1213c4..21ae4d728f 100644 --- a/actions/setup/js/generate_git_patch.cjs +++ b/actions/setup/js/generate_git_patch.cjs @@ -4,7 +4,6 @@ const fs = require("fs"); const path = require("path"); -const { getBaseBranch } = require("./get_base_branch.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { execGitSync } = require("./git_helpers.cjs"); @@ -47,6 +46,7 @@ function getPatchPath(branchName) { /** * Generates a git patch file for the current changes * @param {string} branchName - The branch name to generate patch for + * @param {string} baseBranch - The base branch to diff against (e.g., "main", "master") * @param {Object} [options] - Optional parameters * @param {string} [options.mode="full"] - Patch generation mode: * - "full": Include all commits since merge-base with default branch (for create_pull_request) @@ -54,15 +54,13 @@ function getPatchPath(branchName) { * In incremental mode, origin/branchName is fetched explicitly and merge-base fallback is disabled. * @returns {Promise} Object with patch info or error */ -async function generateGitPatch(branchName, options = {}) { +async function generateGitPatch(branchName, baseBranch, options = {}) { const mode = options.mode || "full"; const patchPath = getPatchPath(branchName); const cwd = process.env.GITHUB_WORKSPACE || process.cwd(); - // NOTE: In cross-repo scenarios, DEFAULT_BRANCH comes from the workflow repository - // (via github.event.repository.default_branch), not the checked-out repository. - // If the checked-out repo has a different default branch (e.g., "master" vs "main"), - // Strategy 1's merge-base calculation may fail. Strategy 3 handles this gracefully. - const defaultBranch = process.env.DEFAULT_BRANCH || (await getBaseBranch()); + // baseBranch is now provided by the caller, who has already resolved it + // using getBaseBranch() with the appropriate target repository context. + const defaultBranch = baseBranch; const githubSha = process.env.GITHUB_SHA; debugLog(`Starting patch generation: mode=${mode}, branch=${branchName}, defaultBranch=${defaultBranch}`); diff --git a/actions/setup/js/generate_git_patch.test.cjs b/actions/setup/js/generate_git_patch.test.cjs index 46a564f428..01f9b2af5c 100644 --- a/actions/setup/js/generate_git_patch.test.cjs +++ b/actions/setup/js/generate_git_patch.test.cjs @@ -30,7 +30,7 @@ describe("generateGitPatch", () => { const { generateGitPatch } = await import("./generate_git_patch.cjs"); - const result = await generateGitPatch(null); + const result = await generateGitPatch(null, "main"); expect(result.success).toBe(false); expect(result).toHaveProperty("error"); @@ -43,7 +43,7 @@ describe("generateGitPatch", () => { process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; process.env.GITHUB_SHA = "abc123"; - const result = await generateGitPatch("nonexistent-branch"); + const result = await generateGitPatch("nonexistent-branch", "main"); expect(result.success).toBe(false); expect(result).toHaveProperty("error"); @@ -57,7 +57,7 @@ describe("generateGitPatch", () => { process.env.GITHUB_SHA = "abc123"; // Even if it fails, it should try to create the directory - const result = await generateGitPatch("test-branch"); + const result = await generateGitPatch("test-branch", "main"); expect(result).toHaveProperty("patchPath"); // Patch path includes sanitized branch name @@ -70,7 +70,7 @@ describe("generateGitPatch", () => { process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; process.env.GITHUB_SHA = "abc123"; - const result = await generateGitPatch("test-branch"); + const result = await generateGitPatch("test-branch", "main"); expect(result).toHaveProperty("success"); expect(result).toHaveProperty("patchPath"); @@ -83,7 +83,7 @@ describe("generateGitPatch", () => { process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; process.env.GITHUB_SHA = "abc123"; - const result = await generateGitPatch(null); + const result = await generateGitPatch(null, "main"); expect(result).toHaveProperty("success"); expect(result).toHaveProperty("patchPath"); @@ -95,37 +95,34 @@ describe("generateGitPatch", () => { process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; process.env.GITHUB_SHA = "abc123"; - const result = await generateGitPatch(""); + const result = await generateGitPatch("", "main"); expect(result).toHaveProperty("success"); expect(result).toHaveProperty("patchPath"); }); - it("should use default branch from environment", async () => { + it("should use provided base branch", async () => { const { generateGitPatch } = await import("./generate_git_patch.cjs"); process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; process.env.GITHUB_SHA = "abc123"; - process.env.DEFAULT_BRANCH = "develop"; - const result = await generateGitPatch("feature-branch"); + const result = await generateGitPatch("feature-branch", "develop"); expect(result).toHaveProperty("success"); - // Should attempt to use develop as default branch + // Should use develop as base branch }); - it("should fall back to GH_AW_CUSTOM_BASE_BRANCH if DEFAULT_BRANCH not set", async () => { + it("should use provided master branch as base", async () => { const { generateGitPatch } = await import("./generate_git_patch.cjs"); process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; process.env.GITHUB_SHA = "abc123"; - delete process.env.DEFAULT_BRANCH; - process.env.GH_AW_CUSTOM_BASE_BRANCH = "master"; - const result = await generateGitPatch("feature-branch"); + const result = await generateGitPatch("feature-branch", "master"); expect(result).toHaveProperty("success"); - // Should attempt to use master as base branch + // Should use master as base branch }); it("should safely handle branch names with special characters", async () => { @@ -138,7 +135,7 @@ describe("generateGitPatch", () => { const maliciousBranchNames = ["feature; rm -rf /", "feature && echo hacked", "feature | cat /etc/passwd", "feature$(whoami)", "feature`whoami`", "feature\nrm -rf /"]; for (const branchName of maliciousBranchNames) { - const result = await generateGitPatch(branchName); + const result = await generateGitPatch(branchName, "main"); // Should not throw an error and should handle safely expect(result).toHaveProperty("success"); @@ -155,7 +152,7 @@ describe("generateGitPatch", () => { // Test with malicious SHA that could cause shell injection process.env.GITHUB_SHA = "abc123; echo hacked"; - const result = await generateGitPatch("test-branch"); + const result = await generateGitPatch("test-branch", "main"); // Should not throw an error and should handle safely expect(result).toHaveProperty("success"); @@ -195,7 +192,7 @@ describe("generateGitPatch - cross-repo checkout scenarios", () => { process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; process.env.GITHUB_SHA = "deadbeef123456789"; // SHA that doesn't exist in target repo - const result = await generateGitPatch("feature-branch"); + const result = await generateGitPatch("feature-branch", "main"); // Should fail gracefully, not crash expect(result).toHaveProperty("success"); @@ -209,9 +206,8 @@ describe("generateGitPatch - cross-repo checkout scenarios", () => { // Simulate cross-repo checkout where fetch fails due to persist-credentials: false process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; process.env.GITHUB_SHA = "abc123"; - process.env.DEFAULT_BRANCH = "main"; - const result = await generateGitPatch("feature-branch"); + const result = await generateGitPatch("feature-branch", "main"); // Should try multiple strategies and fail gracefully expect(result).toHaveProperty("success"); @@ -225,9 +221,8 @@ describe("generateGitPatch - cross-repo checkout scenarios", () => { // This tests that Strategy 1 checks for local refs before fetching process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; - process.env.DEFAULT_BRANCH = "main"; - const result = await generateGitPatch("feature-branch"); + const result = await generateGitPatch("feature-branch", "main"); // Should complete without hanging or crashing due to fetch attempts expect(result).toHaveProperty("success"); @@ -239,9 +234,8 @@ describe("generateGitPatch - cross-repo checkout scenarios", () => { process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; process.env.GITHUB_SHA = "sha-from-workflow-repo"; - process.env.DEFAULT_BRANCH = "main"; - const result = await generateGitPatch("agent-created-branch"); + const result = await generateGitPatch("agent-created-branch", "main"); expect(result.success).toBe(false); expect(result).toHaveProperty("error"); @@ -254,10 +248,9 @@ describe("generateGitPatch - cross-repo checkout scenarios", () => { const { generateGitPatch } = await import("./generate_git_patch.cjs"); process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-repo"; - process.env.DEFAULT_BRANCH = "main"; // Incremental mode requires origin/branchName to exist - should fail clearly - const result = await generateGitPatch("feature-branch", { mode: "incremental" }); + const result = await generateGitPatch("feature-branch", "main", { mode: "incremental" }); expect(result.success).toBe(false); expect(result).toHaveProperty("error"); @@ -272,9 +265,8 @@ describe("generateGitPatch - cross-repo checkout scenarios", () => { // GITHUB_SHA would be from side-repo, not target-repo process.env.GITHUB_WORKSPACE = "/tmp/nonexistent-target-repo"; process.env.GITHUB_SHA = "side-repo-sha-not-in-target"; - process.env.DEFAULT_BRANCH = "main"; - const result = await generateGitPatch("agent-changes"); + const result = await generateGitPatch("agent-changes", "main"); // Should not crash, should return failure with helpful error expect(result).toHaveProperty("success"); diff --git a/actions/setup/js/get_base_branch.cjs b/actions/setup/js/get_base_branch.cjs index f6b28645a8..1c5e220d0d 100644 --- a/actions/setup/js/get_base_branch.cjs +++ b/actions/setup/js/get_base_branch.cjs @@ -11,9 +11,13 @@ * 4. API lookup for issue_comment events on PRs (the PR's base ref is not in the payload) * 5. Fallback to DEFAULT_BRANCH env var or "main" * + * @param {{owner: string, repo: string}|null} [targetRepo] - Optional target repository. + * If provided, API calls (step 4) use this instead of context.repo, + * which is needed for cross-repo scenarios where the target repo differs + * from the workflow repository. * @returns {Promise} The base branch name */ -async function getBaseBranch() { +async function getBaseBranch(targetRepo = null) { // 1. Custom base branch from workflow configuration if (process.env.GH_AW_CUSTOM_BASE_BRANCH) { return process.env.GH_AW_CUSTOM_BASE_BRANCH; @@ -30,12 +34,15 @@ async function getBaseBranch() { } // 4. For issue_comment events on PRs - must call API since base ref not in payload + // Use targetRepo if provided (cross-repo scenarios), otherwise fall back to context.repo if (typeof context !== "undefined" && context.eventName === "issue_comment" && context.payload?.issue?.pull_request) { try { if (typeof github !== "undefined") { + const repoOwner = targetRepo?.owner ?? context.repo.owner; + const repoName = targetRepo?.repo ?? context.repo.repo; const { data: pr } = await github.rest.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, + owner: repoOwner, + repo: repoName, pull_number: context.payload.issue.number, }); return pr.base.ref; diff --git a/actions/setup/js/get_base_branch.test.cjs b/actions/setup/js/get_base_branch.test.cjs index 4540363009..e83701c4f9 100644 --- a/actions/setup/js/get_base_branch.test.cjs +++ b/actions/setup/js/get_base_branch.test.cjs @@ -80,4 +80,23 @@ describe("getBaseBranch", () => { process.env.GH_AW_CUSTOM_BASE_BRANCH = "feature/new-feature"; expect(await getBaseBranch()).toBe("feature/new-feature"); }); + + it("should accept optional targetRepo parameter without affecting simple cases", async () => { + // When env vars are set, targetRepo doesn't change the result + process.env.GH_AW_CUSTOM_BASE_BRANCH = "custom-branch"; + + const { getBaseBranch } = await import("./get_base_branch.cjs"); + + // With targetRepo parameter + const result = await getBaseBranch({ owner: "other-owner", repo: "other-repo" }); + expect(result).toBe("custom-branch"); + + // Without targetRepo parameter (null) + const result2 = await getBaseBranch(null); + expect(result2).toBe("custom-branch"); + + // Without targetRepo parameter (undefined) + const result3 = await getBaseBranch(); + expect(result3).toBe("custom-branch"); + }); }); diff --git a/actions/setup/js/git_patch_integration.test.cjs b/actions/setup/js/git_patch_integration.test.cjs index 5606dfe733..d7e0518244 100644 --- a/actions/setup/js/git_patch_integration.test.cjs +++ b/actions/setup/js/git_patch_integration.test.cjs @@ -554,7 +554,7 @@ describe("git patch integration tests", () => { process.env.DEFAULT_BRANCH = "main"; try { - const result = await generateGitPatch("feature-branch", { mode: "incremental" }); + const result = await generateGitPatch("feature-branch", "main", { mode: "incremental" }); expect(result.success).toBe(true); expect(result.patchPath).toBeDefined(); @@ -598,7 +598,7 @@ describe("git patch integration tests", () => { process.env.DEFAULT_BRANCH = "main"; try { - const result = await generateGitPatch("local-only-branch", { mode: "incremental" }); + const result = await generateGitPatch("local-only-branch", "main", { mode: "incremental" }); // Should fail with a clear error message expect(result.success).toBe(false); @@ -638,7 +638,7 @@ describe("git patch integration tests", () => { try { // Full mode (default) - should fall back to merge-base and include all commits - const result = await generateGitPatch("full-mode-branch", { mode: "full" }); + const result = await generateGitPatch("full-mode-branch", "main", { mode: "full" }); // Debug output if test fails if (!result.success) { diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 880f16cd48..8343f1322b 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -9,7 +9,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { normalizeBranchName } = require("./normalize_branch_name.cjs"); const { pushExtraEmptyCommit } = require("./extra_empty_commit.cjs"); const { detectForkPR } = require("./pr_helpers.cjs"); -const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo, parseRepoSlug } = require("./repo_helpers.cjs"); const { getBaseBranch } = require("./get_base_branch.cjs"); /** @@ -32,16 +32,23 @@ async function main(config = {}) { const ifNoChanges = config.if_no_changes || "warn"; const commitTitleSuffix = config.commit_title_suffix || ""; const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024; - // Resolve base branch: use config value if set, otherwise resolve dynamically - // Dynamic resolution is needed for issue_comment events on PRs where the base branch - // is not available in GitHub Actions expressions and requires an API call - const baseBranch = config.base_branch || (await getBaseBranch()); const maxCount = config.max || 0; // 0 means no limit // Cross-repo support: resolve target repository from config // This allows pushing to PRs in a different repository than the workflow const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + // Parse default target repo for use in base branch resolution + // This is needed for cross-repo scenarios where the base branch API call + // should target the configured repo, not context.repo + const defaultTargetRepoParts = parseRepoSlug(defaultTargetRepo); + + // Resolve base branch: use config value if set, otherwise resolve dynamically + // Dynamic resolution is needed for issue_comment events on PRs where the base branch + // is not available in GitHub Actions expressions and requires an API call + // Pass target repo for cross-repo scenarios + const baseBranch = config.base_branch || (await getBaseBranch(defaultTargetRepoParts)); + // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index ad25fa930b..2a5cd18c02 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -13,6 +13,7 @@ const { generateGitPatch } = require("./generate_git_patch.cjs"); const { enforceCommentLimits } = require("./comment_limit_helpers.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { ERR_CONFIG, ERR_SYSTEM, ERR_VALIDATION } = require("./error_codes.cjs"); +const { resolveTargetRepoConfig, parseRepoSlug } = require("./repo_helpers.cjs"); /** * Create handlers for safe output tools @@ -190,7 +191,12 @@ function createHandlers(server, appendSafeOutput, config = {}) { */ const createPullRequestHandler = async args => { const entry = { ...args, type: "create_pull_request" }; - const baseBranch = await getBaseBranch(); + + // Resolve target repo for base branch resolution (cross-repo scenarios) + const prConfig = config.create_pull_request || {}; + const { defaultTargetRepo } = resolveTargetRepoConfig(prConfig); + const targetRepoParts = parseRepoSlug(defaultTargetRepo); + const baseBranch = await getBaseBranch(targetRepoParts); // If branch is not provided, is empty, or equals the base branch, use the current branch from git // This handles cases where the agent incorrectly passes the base branch instead of the working branch @@ -228,8 +234,8 @@ function createHandlers(server, appendSafeOutput, config = {}) { } // Generate git patch - server.debug(`Generating patch for create_pull_request with branch: ${entry.branch}`); - const patchResult = await generateGitPatch(entry.branch); + server.debug(`Generating patch for create_pull_request with branch: ${entry.branch}, baseBranch: ${baseBranch}`); + const patchResult = await generateGitPatch(entry.branch, baseBranch); if (!patchResult.success) { // Patch generation failed or patch is empty @@ -287,7 +293,12 @@ function createHandlers(server, appendSafeOutput, config = {}) { */ const pushToPullRequestBranchHandler = async args => { const entry = { ...args, type: "push_to_pull_request_branch" }; - const baseBranch = await getBaseBranch(); + + // Resolve target repo for base branch resolution (cross-repo scenarios) + const pushConfig = config.push_to_pull_request_branch || {}; + const { defaultTargetRepo } = resolveTargetRepoConfig(pushConfig); + const targetRepoParts = parseRepoSlug(defaultTargetRepo); + const baseBranch = await getBaseBranch(targetRepoParts); // If branch is not provided, is empty, or equals the base branch, use the current branch from git // This handles cases where the agent incorrectly passes the base branch instead of the working branch @@ -306,8 +317,8 @@ function createHandlers(server, appendSafeOutput, config = {}) { // Generate git patch in incremental mode // Incremental mode only includes commits since origin/branchName, // preventing patches that include already-existing commits - server.debug(`Generating incremental patch for push_to_pull_request_branch with branch: ${entry.branch}`); - const patchResult = await generateGitPatch(entry.branch, { mode: "incremental" }); + server.debug(`Generating incremental patch for push_to_pull_request_branch with branch: ${entry.branch}, baseBranch: ${baseBranch}`); + const patchResult = await generateGitPatch(entry.branch, baseBranch, { mode: "incremental" }); if (!patchResult.success) { // Patch generation failed or patch is empty diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index d5c0a3a64e..3534aaebcc 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -14,8 +14,19 @@ const mockCore = { setOutput: vi.fn(), }; +// Mock context object used by repo_helpers.cjs +const mockContext = { + repo: { + owner: "test-owner", + repo: "test-repo", + }, + eventName: "push", + payload: {}, +}; + // Set up global mocks before importing the module global.core = mockCore; +global.context = mockContext; describe("safe_outputs_handlers", () => { let mockServer; From 7467e79dbc5ae8c1a6ae644c5be8c0d82d73672d Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 27 Feb 2026 02:02:54 +0000 Subject: [PATCH 2/8] improve base branch resolution for cross-repo PRs --- .../setup/js/push_to_pull_request_branch.cjs | 20 ++----- actions/setup/js/safe_outputs_handlers.cjs | 60 ++++++++++++++++--- 2 files changed, 57 insertions(+), 23 deletions(-) diff --git a/actions/setup/js/push_to_pull_request_branch.cjs b/actions/setup/js/push_to_pull_request_branch.cjs index 8343f1322b..0510ed1622 100644 --- a/actions/setup/js/push_to_pull_request_branch.cjs +++ b/actions/setup/js/push_to_pull_request_branch.cjs @@ -9,8 +9,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { normalizeBranchName } = require("./normalize_branch_name.cjs"); const { pushExtraEmptyCommit } = require("./extra_empty_commit.cjs"); const { detectForkPR } = require("./pr_helpers.cjs"); -const { resolveTargetRepoConfig, resolveAndValidateRepo, parseRepoSlug } = require("./repo_helpers.cjs"); -const { getBaseBranch } = require("./get_base_branch.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction @@ -38,23 +37,16 @@ async function main(config = {}) { // This allows pushing to PRs in a different repository than the workflow const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); - // Parse default target repo for use in base branch resolution - // This is needed for cross-repo scenarios where the base branch API call - // should target the configured repo, not context.repo - const defaultTargetRepoParts = parseRepoSlug(defaultTargetRepo); - - // Resolve base branch: use config value if set, otherwise resolve dynamically - // Dynamic resolution is needed for issue_comment events on PRs where the base branch - // is not available in GitHub Actions expressions and requires an API call - // Pass target repo for cross-repo scenarios - const baseBranch = config.base_branch || (await getBaseBranch(defaultTargetRepoParts)); + // Base branch from config (if set) - used only for logging at factory level + // Dynamic base branch resolution happens per-message after resolving the actual target repo + const configBaseBranch = config.base_branch || null; // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; core.info(`Target: ${target}`); - if (baseBranch) { - core.info(`Base branch: ${baseBranch}`); + if (configBaseBranch) { + core.info(`Base branch (from config): ${configBaseBranch}`); } if (titlePrefix) { core.info(`Title prefix: ${titlePrefix}`); diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 2a5cd18c02..5527b168c6 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -13,7 +13,7 @@ const { generateGitPatch } = require("./generate_git_patch.cjs"); const { enforceCommentLimits } = require("./comment_limit_helpers.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { ERR_CONFIG, ERR_SYSTEM, ERR_VALIDATION } = require("./error_codes.cjs"); -const { resolveTargetRepoConfig, parseRepoSlug } = require("./repo_helpers.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); /** * Create handlers for safe output tools @@ -192,11 +192,32 @@ function createHandlers(server, appendSafeOutput, config = {}) { const createPullRequestHandler = async args => { const entry = { ...args, type: "create_pull_request" }; - // Resolve target repo for base branch resolution (cross-repo scenarios) + // Resolve target repo configuration and validate the target repo early + // This is needed before getBaseBranch to ensure we resolve the base branch + // for the correct repository (especially in cross-repo scenarios) const prConfig = config.create_pull_request || {}; - const { defaultTargetRepo } = resolveTargetRepoConfig(prConfig); - const targetRepoParts = parseRepoSlug(defaultTargetRepo); - const baseBranch = await getBaseBranch(targetRepoParts); + const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(prConfig); + + // Resolve and validate the target repository from the entry + const repoResult = resolveAndValidateRepo(entry, defaultTargetRepo, allowedRepos, "pull request"); + if (!repoResult.success) { + return { + content: [ + { + type: "text", + text: JSON.stringify({ + result: "error", + error: repoResult.error, + }), + }, + ], + isError: true, + }; + } + const { repoParts } = repoResult; + + // Get base branch for the resolved target repository + const baseBranch = await getBaseBranch(repoParts); // If branch is not provided, is empty, or equals the base branch, use the current branch from git // This handles cases where the agent incorrectly passes the base branch instead of the working branch @@ -294,11 +315,32 @@ function createHandlers(server, appendSafeOutput, config = {}) { const pushToPullRequestBranchHandler = async args => { const entry = { ...args, type: "push_to_pull_request_branch" }; - // Resolve target repo for base branch resolution (cross-repo scenarios) + // Resolve target repo configuration and validate the target repo early + // This is needed before getBaseBranch to ensure we resolve the base branch + // for the correct repository (especially in cross-repo scenarios) const pushConfig = config.push_to_pull_request_branch || {}; - const { defaultTargetRepo } = resolveTargetRepoConfig(pushConfig); - const targetRepoParts = parseRepoSlug(defaultTargetRepo); - const baseBranch = await getBaseBranch(targetRepoParts); + const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(pushConfig); + + // Resolve and validate the target repository from the entry + const repoResult = resolveAndValidateRepo(entry, defaultTargetRepo, allowedRepos, "push to PR branch"); + if (!repoResult.success) { + return { + content: [ + { + type: "text", + text: JSON.stringify({ + result: "error", + error: repoResult.error, + }), + }, + ], + isError: true, + }; + } + const { repoParts } = repoResult; + + // Get base branch for the resolved target repository + const baseBranch = await getBaseBranch(repoParts); // If branch is not provided, is empty, or equals the base branch, use the current branch from git // This handles cases where the agent incorrectly passes the base branch instead of the working branch From 4a76b232f6619f656d6e260f344981d71594d584 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 27 Feb 2026 02:13:19 +0000 Subject: [PATCH 3/8] improve base branch resolution for cross-repo PRs --- actions/setup/js/create_agent_session.cjs | 41 ++++++------ actions/setup/js/generate_git_patch.cjs | 2 - actions/setup/js/repo_helpers.cjs | 79 ++++++++++++++++++++--- 3 files changed, 94 insertions(+), 28 deletions(-) diff --git a/actions/setup/js/create_agent_session.cjs b/actions/setup/js/create_agent_session.cjs index b1c88a8ea2..b7a32ccec0 100644 --- a/actions/setup/js/create_agent_session.cjs +++ b/actions/setup/js/create_agent_session.cjs @@ -2,7 +2,7 @@ /// const { getErrorMessage } = require("./error_helpers.cjs"); -const { resolveTargetRepoConfig, resolveAndValidateRepo, parseRepoSlug } = require("./repo_helpers.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { getBaseBranch } = require("./get_base_branch.cjs"); const fs = require("fs"); @@ -61,26 +61,31 @@ async function main() { allowed_repos: process.env.GH_AW_ALLOWED_REPOS, }); - // Parse default target repo for use in base branch resolution - // This is needed for cross-repo scenarios where the base branch API call - // should target the configured repo, not context.repo - const defaultTargetRepoParts = parseRepoSlug(defaultTargetRepo); - if (isStaged) { let summaryContent = "## 🎭 Staged Mode: Create Agent Sessions Preview\n\n"; summaryContent += "The following agent sessions would be created if staged mode was disabled:\n\n"; - // Resolve base branch: use custom config if set, otherwise resolve dynamically - // Pass target repo for cross-repo scenarios - const baseBranch = process.env.GITHUB_AW_AGENT_SESSION_BASE || (await getBaseBranch(defaultTargetRepoParts)); - for (const [index, item] of createAgentSessionItems.entries()) { + // Resolve and validate target repository for this item using the standardized helper + const repoResult = resolveAndValidateRepo(item, defaultTargetRepo, allowedRepos, "agent session"); + if (!repoResult.success) { + summaryContent += `### Task ${index + 1}\n\n`; + summaryContent += `**Error:** ${repoResult.error}\n\n`; + summaryContent += "---\n\n"; + continue; + } + const { repo: effectiveRepo, repoParts } = repoResult; + + // Resolve base branch: use custom config if set, otherwise resolve dynamically + // Pass target repo for cross-repo scenarios + const baseBranch = process.env.GITHUB_AW_AGENT_SESSION_BASE || (await getBaseBranch(repoParts)); + summaryContent += `### Task ${index + 1}\n\n`; summaryContent += `**Description:**\n${item.body || "No description provided"}\n\n`; summaryContent += `**Base Branch:** ${baseBranch}\n\n`; - summaryContent += `**Target Repository:** ${defaultTargetRepo}\n\n`; + summaryContent += `**Target Repository:** ${effectiveRepo}\n\n`; summaryContent += "---\n\n"; } @@ -91,12 +96,6 @@ async function main() { return; } - // Resolve base branch: use custom config if set, otherwise resolve dynamically - // Dynamic resolution is needed for issue_comment events on PRs where the base branch - // is not available in GitHub Actions expressions and requires an API call - // Pass target repo for cross-repo scenarios - const baseBranch = process.env.GITHUB_AW_AGENT_SESSION_BASE || (await getBaseBranch(defaultTargetRepoParts)); - // Process all agent session items const createdTasks = []; let summaryContent = "## ✅ Agent Sessions Created\n\n"; @@ -116,7 +115,13 @@ async function main() { core.error(`E004: ${repoResult.error}`); continue; } - const effectiveRepo = repoResult.repo; + const { repo: effectiveRepo, repoParts } = repoResult; + + // Resolve base branch: use custom config if set, otherwise resolve dynamically + // Dynamic resolution is needed for issue_comment events on PRs where the base branch + // is not available in GitHub Actions expressions and requires an API call + // Pass target repo for cross-repo scenarios + const baseBranch = process.env.GITHUB_AW_AGENT_SESSION_BASE || (await getBaseBranch(repoParts)); try { // Write task description to a temporary file diff --git a/actions/setup/js/generate_git_patch.cjs b/actions/setup/js/generate_git_patch.cjs index 21ae4d728f..4a4b6ea659 100644 --- a/actions/setup/js/generate_git_patch.cjs +++ b/actions/setup/js/generate_git_patch.cjs @@ -58,8 +58,6 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { const mode = options.mode || "full"; const patchPath = getPatchPath(branchName); const cwd = process.env.GITHUB_WORKSPACE || process.cwd(); - // baseBranch is now provided by the caller, who has already resolved it - // using getBaseBranch() with the appropriate target repository context. const defaultBranch = baseBranch; const githubSha = process.env.GITHUB_SHA; diff --git a/actions/setup/js/repo_helpers.cjs b/actions/setup/js/repo_helpers.cjs index 9b2a94e96b..4f9e2d3c68 100644 --- a/actions/setup/js/repo_helpers.cjs +++ b/actions/setup/js/repo_helpers.cjs @@ -9,6 +9,69 @@ const { globPatternToRegex } = require("./glob_pattern_helpers.cjs"); const { ERR_VALIDATION } = require("./error_codes.cjs"); +// ============================================================================ +// Type Definitions +// ============================================================================ + +/** + * Parsed repository owner and name + * @typedef {Object} RepoParts + * @property {string} owner - Repository owner (organization or user) + * @property {string} repo - Repository name + */ + +/** + * Result of repository validation + * @typedef {Object} RepoValidationResult + * @property {boolean} valid - Whether the repository is allowed + * @property {string|null} error - Error message if validation failed, null otherwise + * @property {string} qualifiedRepo - Fully qualified repository slug (owner/repo) + */ + +/** + * Successful result from resolveAndValidateRepo + * @typedef {Object} RepoResolutionSuccess + * @property {true} success - Always true for success + * @property {string} repo - Fully qualified repository slug (owner/repo) + * @property {RepoParts} repoParts - Parsed owner and repo components + */ + +/** + * Failed result from resolveAndValidateRepo + * @typedef {Object} RepoResolutionError + * @property {false} success - Always false for error + * @property {string} error - Error message describing why resolution failed + */ + +/** + * Union type for resolveAndValidateRepo result + * @typedef {RepoResolutionSuccess | RepoResolutionError} RepoResolutionResult + */ + +/** + * Result of resolveTargetRepoConfig + * @typedef {Object} TargetRepoConfig + * @property {string} defaultTargetRepo - Default target repository slug + * @property {Set} allowedRepos - Set of allowed repository patterns + */ + +/** + * Handler configuration object with repository settings + * @typedef {Object} HandlerRepoConfig + * @property {string} [target-repo] - Configured target repository + * @property {string[]|string} [allowed_repos] - Allowed repositories (array or comma-separated) + */ + +/** + * Message item that may contain a repo field + * @typedef {Object} MessageItemWithRepo + * @property {string} [repo] - Optional repository slug override + */ + +// ============================================================================ +// Functions +// ============================================================================ + /** * Parse the allowed repos from config value (array or comma-separated string) * @param {string[]|string|undefined} allowedReposValue - Allowed repos from config (array or comma-separated string) @@ -33,7 +96,7 @@ function parseAllowedRepos(allowedReposValue) { /** * Get the default target repository - * @param {Object} [config] - Optional config object with target-repo field + * @param {HandlerRepoConfig | import('./types/handler-factory').HandlerConfig} [config] - Optional config object with target-repo field * @returns {string} Repository slug in "owner/repo" format */ function getDefaultTargetRepo(config) { @@ -86,7 +149,7 @@ function isRepoAllowed(qualifiedRepo, allowedRepos) { * @param {string} repo - Repository slug to validate (can be "owner/repo" or just "repo") * @param {string} defaultRepo - Default target repository * @param {Set} allowedRepos - Set of explicitly allowed repo patterns - * @returns {{valid: boolean, error: string|null, qualifiedRepo: string}} + * @returns {RepoValidationResult} */ function validateRepo(repo, defaultRepo, allowedRepos) { // If repo is a bare name (no slash), qualify it with the default repo's org @@ -116,7 +179,7 @@ function validateRepo(repo, defaultRepo, allowedRepos) { /** * Parse owner and repo from a repository slug * @param {string} repoSlug - Repository slug in "owner/repo" format - * @returns {{owner: string, repo: string}|null} + * @returns {RepoParts|null} */ function parseRepoSlug(repoSlug) { const parts = repoSlug.split("/"); @@ -129,8 +192,8 @@ function parseRepoSlug(repoSlug) { /** * Resolve target repository configuration from handler config * Combines parsing of allowed-repos and resolution of default target repo - * @param {Object} config - Handler configuration object - * @returns {{defaultTargetRepo: string, allowedRepos: Set}} + * @param {HandlerRepoConfig | import('./types/handler-factory').HandlerConfig} config - Handler configuration object + * @returns {TargetRepoConfig} */ function resolveTargetRepoConfig(config) { const defaultTargetRepo = getDefaultTargetRepo(config); @@ -144,11 +207,11 @@ function resolveTargetRepoConfig(config) { /** * Resolve and validate target repository from a message item * Combines repo resolution, validation, and parsing into a single function - * @param {Object} item - Message item that may contain a repo field + * @param {MessageItemWithRepo} item - Message item that may contain a repo field * @param {string} defaultTargetRepo - Default target repository slug * @param {Set} allowedRepos - Set of allowed repository slugs * @param {string} operationType - Type of operation (e.g., "comment", "pull request", "issue") for error messages - * @returns {{success: true, repo: string, repoParts: {owner: string, repo: string}}|{success: false, error: string}} + * @returns {RepoResolutionResult} */ function resolveAndValidateRepo(item, defaultTargetRepo, allowedRepos, operationType) { // Determine target repository for this operation @@ -196,7 +259,7 @@ function resolveAndValidateRepo(item, defaultTargetRepo, allowedRepos, operation * @param {string} repo - Repository slug to validate (can be "owner/repo" or just "repo") * @param {string} defaultRepo - Default (always-allowed) target repository * @param {Set} allowedRepos - Set of explicitly allowed repo patterns - * @returns {{valid: boolean, error: string|null, qualifiedRepo: string}} + * @returns {RepoValidationResult} */ function validateTargetRepo(repo, defaultRepo, allowedRepos) { return validateRepo(repo, defaultRepo, allowedRepos); From 6bd7b77147793e68a83093ae94d8c71d067cc159 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 27 Feb 2026 02:19:07 +0000 Subject: [PATCH 4/8] improve base branch resolution for cross-repo PRs --- actions/setup/js/create_pull_request.cjs | 62 +++++++++++++++--------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/actions/setup/js/create_pull_request.cjs b/actions/setup/js/create_pull_request.cjs index 63f2cc3688..96840a47e1 100644 --- a/actions/setup/js/create_pull_request.cjs +++ b/actions/setup/js/create_pull_request.cjs @@ -11,7 +11,7 @@ const { removeDuplicateTitleFromDescription } = require("./remove_duplicate_titl const { sanitizeTitle, applyTitlePrefix } = require("./sanitize_title.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { replaceTemporaryIdReferences, isTemporaryId } = require("./temporary_id.cjs"); -const { resolveTargetRepoConfig, resolveAndValidateRepo, parseRepoSlug } = require("./repo_helpers.cjs"); +const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); const { addExpirationToFooter } = require("./ephemerals.cjs"); const { generateWorkflowIdMarker } = require("./generate_footer.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); @@ -103,16 +103,21 @@ async function main(config = {}) { const maxSizeKb = config.max_patch_size ? parseInt(String(config.max_patch_size), 10) : 1024; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); - // Parse default target repo for use in base branch resolution - // This is needed for cross-repo scenarios where the base branch API call - // should target the configured repo, not context.repo - const defaultTargetRepoParts = parseRepoSlug(defaultTargetRepo); + // Base branch from config (if set) - validated at factory level if explicit + // Dynamic base branch resolution happens per-message after resolving the actual target repo + const configBaseBranch = config.base_branch || null; + + // SECURITY: If base branch is explicitly configured, validate it at factory level + if (configBaseBranch) { + const normalizedConfigBase = normalizeBranchName(configBaseBranch); + if (!normalizedConfigBase) { + throw new Error(`Invalid baseBranch: sanitization resulted in empty string (original: "${configBaseBranch}")`); + } + if (configBaseBranch !== normalizedConfigBase) { + throw new Error(`Invalid baseBranch: contains invalid characters (original: "${configBaseBranch}", normalized: "${normalizedConfigBase}")`); + } + } - // Resolve base branch: use config value if set, otherwise resolve dynamically - // Dynamic resolution is needed for issue_comment events on PRs where the base branch - // is not available in GitHub Actions expressions and requires an API call - // Pass target repo for cross-repo scenarios - let baseBranch = config.base_branch || (await getBaseBranch(defaultTargetRepoParts)); const includeFooter = parseBoolTemplatable(config.footer, true); const fallbackAsIssue = config.fallback_as_issue !== false; // Default to true (fallback enabled) @@ -122,25 +127,13 @@ async function main(config = {}) { throw new Error("GH_AW_WORKFLOW_ID environment variable is required"); } - // SECURITY: Sanitize base branch name to prevent shell injection (defense in depth) - // Even though baseBranch comes from workflow config, normalize it for safety - const originalBaseBranch = baseBranch; - baseBranch = normalizeBranchName(baseBranch); - if (!baseBranch) { - throw new Error(`Invalid baseBranch: sanitization resulted in empty string (original: "${originalBaseBranch}")`); - } - // Fail if base branch name changes during normalization (indicates invalid config) - if (originalBaseBranch !== baseBranch) { - throw new Error(`Invalid baseBranch: contains invalid characters (original: "${originalBaseBranch}", normalized: "${baseBranch}")`); - } - // Extract triggering issue number from context (for auto-linking PRs to issues) const triggeringIssueNumber = context.payload?.issue?.number && !context.payload?.issue?.pull_request ? context.payload.issue.number : undefined; // Check if we're in staged mode const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; - core.info(`Base branch: ${baseBranch}`); + core.info(`Base branch: ${configBaseBranch || "(dynamic - resolved per target repo)"}`); core.info(`Default target repo: ${defaultTargetRepo}`); if (allowedRepos.size > 0) { core.info(`Allowed repos: ${Array.from(allowedRepos).join(", ")}`); @@ -228,6 +221,29 @@ async function main(config = {}) { const { repo: itemRepo, repoParts } = repoResult; core.info(`Target repository: ${itemRepo}`); + // Resolve base branch for this target repository + // Use config value if set, otherwise resolve dynamically for the specific target repo + // Dynamic resolution is needed for issue_comment events on PRs where the base branch + // is not available in GitHub Actions expressions and requires an API call + let baseBranch = configBaseBranch || (await getBaseBranch(repoParts)); + + // SECURITY: Sanitize dynamically resolved base branch to prevent shell injection + const originalBaseBranch = baseBranch; + baseBranch = normalizeBranchName(baseBranch); + if (!baseBranch) { + return { + success: false, + error: `Invalid base branch: sanitization resulted in empty string (original: "${originalBaseBranch}")`, + }; + } + if (originalBaseBranch !== baseBranch) { + return { + success: false, + error: `Invalid base branch: contains invalid characters (original: "${originalBaseBranch}", normalized: "${baseBranch}")`, + }; + } + core.info(`Base branch for ${itemRepo}: ${baseBranch}`); + // Check if patch file exists and has valid content if (!patchFilePath || !fs.existsSync(patchFilePath)) { // If allow-empty is enabled, we can proceed without a patch file From 8d711bb285c0bec36217186d8e1256f532e4bd49 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 27 Feb 2026 02:19:55 +0000 Subject: [PATCH 5/8] Update actions/setup/js/generate_git_patch.cjs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- actions/setup/js/generate_git_patch.cjs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/actions/setup/js/generate_git_patch.cjs b/actions/setup/js/generate_git_patch.cjs index 4a4b6ea659..98cd72ee03 100644 --- a/actions/setup/js/generate_git_patch.cjs +++ b/actions/setup/js/generate_git_patch.cjs @@ -55,8 +55,23 @@ function getPatchPath(branchName) { * @returns {Promise} Object with patch info or error */ async function generateGitPatch(branchName, baseBranch, options = {}) { - const mode = options.mode || "full"; const patchPath = getPatchPath(branchName); + + // 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) + + ")"; + debugLog(`Invalid baseBranch: ${errorMessage}`); + return { + patchPath, + patchGenerated: false, + errorMessage, + }; + } + + const mode = options.mode || "full"; const cwd = process.env.GITHUB_WORKSPACE || process.cwd(); const defaultBranch = baseBranch; const githubSha = process.env.GITHUB_SHA; From 6dff6d56844e711a90c42aa57aba8ca725d7abf1 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 27 Feb 2026 02:20:18 +0000 Subject: [PATCH 6/8] improve base branch resolution for cross-repo PRs --- actions/setup/js/find_repo_checkout.test.cjs | 297 +++++++++++++++++++ 1 file changed, 297 insertions(+) create mode 100644 actions/setup/js/find_repo_checkout.test.cjs diff --git a/actions/setup/js/find_repo_checkout.test.cjs b/actions/setup/js/find_repo_checkout.test.cjs new file mode 100644 index 0000000000..f2dfe0a484 --- /dev/null +++ b/actions/setup/js/find_repo_checkout.test.cjs @@ -0,0 +1,297 @@ +const fs = require("fs"); +const path = require("path"); +const { extractRepoSlugFromUrl, normalizeRepoSlug, findGitDirectories, findRepoCheckout, buildRepoCheckoutMap } = require("./find_repo_checkout.cjs"); +const { getPatchPathForRepo, sanitizeBranchNameForPatch, sanitizeRepoSlugForPatch } = require("./generate_git_patch.cjs"); + +describe("find_repo_checkout", () => { + describe("extractRepoSlugFromUrl", () => { + it("should extract slug from HTTPS URL", () => { + expect(extractRepoSlugFromUrl("https://github.com/owner/repo.git")).toBe("owner/repo"); + expect(extractRepoSlugFromUrl("https://github.com/owner/repo")).toBe("owner/repo"); + }); + + it("should extract slug from SSH URL", () => { + expect(extractRepoSlugFromUrl("git@github.com:owner/repo.git")).toBe("owner/repo"); + expect(extractRepoSlugFromUrl("git@github.com:owner/repo")).toBe("owner/repo"); + }); + + it("should handle GitHub Enterprise URLs", () => { + expect(extractRepoSlugFromUrl("https://github.example.com/org/project.git")).toBe("org/project"); + expect(extractRepoSlugFromUrl("git@github.example.com:org/project.git")).toBe("org/project"); + }); + + it("should normalize to lowercase", () => { + expect(extractRepoSlugFromUrl("https://github.com/Owner/Repo.git")).toBe("owner/repo"); + expect(extractRepoSlugFromUrl("git@github.com:OWNER/REPO")).toBe("owner/repo"); + }); + + it("should return null for invalid URLs", () => { + expect(extractRepoSlugFromUrl("")).toBeNull(); + expect(extractRepoSlugFromUrl("invalid")).toBeNull(); + expect(extractRepoSlugFromUrl(null)).toBeNull(); + expect(extractRepoSlugFromUrl(undefined)).toBeNull(); + }); + + it("should handle URLs with ports", () => { + expect(extractRepoSlugFromUrl("https://github.example.com:8443/org/repo.git")).toBe("org/repo"); + }); + + it("should handle HTTP URLs", () => { + expect(extractRepoSlugFromUrl("http://github.local/owner/repo")).toBe("owner/repo"); + }); + }); + + describe("normalizeRepoSlug", () => { + it("should normalize to lowercase", () => { + expect(normalizeRepoSlug("Owner/Repo")).toBe("owner/repo"); + expect(normalizeRepoSlug("ORG/PROJECT")).toBe("org/project"); + }); + + it("should trim whitespace", () => { + expect(normalizeRepoSlug(" owner/repo ")).toBe("owner/repo"); + }); + + it("should return empty string for invalid input", () => { + expect(normalizeRepoSlug("")).toBe(""); + expect(normalizeRepoSlug(null)).toBe(""); + expect(normalizeRepoSlug(undefined)).toBe(""); + }); + + it("should handle tabs and newlines", () => { + expect(normalizeRepoSlug("\towner/repo\n")).toBe("owner/repo"); + }); + }); + + describe("findGitDirectories", () => { + let testDir; + + beforeEach(() => { + testDir = `/tmp/test-find-git-dirs-${Date.now()}`; + fs.mkdirSync(testDir, { recursive: true }); + }); + + afterEach(() => { + try { + fs.rmSync(testDir, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors + } + }); + + it("should find git directories in workspace", () => { + // Create a mock git repo structure + fs.mkdirSync(path.join(testDir, "repo-a", ".git"), { recursive: true }); + fs.mkdirSync(path.join(testDir, "repo-b", ".git"), { recursive: true }); + + const dirs = findGitDirectories(testDir); + + expect(dirs).toHaveLength(2); + expect(dirs).toContain(path.join(testDir, "repo-a")); + expect(dirs).toContain(path.join(testDir, "repo-b")); + }); + + it("should handle nested repos", () => { + // Create a nested structure + fs.mkdirSync(path.join(testDir, "projects", "frontend", ".git"), { recursive: true }); + fs.mkdirSync(path.join(testDir, "projects", "backend", ".git"), { recursive: true }); + + const dirs = findGitDirectories(testDir); + + expect(dirs).toHaveLength(2); + expect(dirs).toContain(path.join(testDir, "projects", "frontend")); + expect(dirs).toContain(path.join(testDir, "projects", "backend")); + }); + + it("should skip node_modules", () => { + fs.mkdirSync(path.join(testDir, "node_modules", "some-pkg", ".git"), { recursive: true }); + fs.mkdirSync(path.join(testDir, "actual-repo", ".git"), { recursive: true }); + + const dirs = findGitDirectories(testDir); + + expect(dirs).toHaveLength(1); + expect(dirs).toContain(path.join(testDir, "actual-repo")); + }); + + it("should return empty array when no git dirs found", () => { + fs.mkdirSync(path.join(testDir, "empty-folder"), { recursive: true }); + + const dirs = findGitDirectories(testDir); + + expect(dirs).toEqual([]); + }); + + it("should respect maxDepth", () => { + // Create a deeply nested repo + fs.mkdirSync(path.join(testDir, "a", "b", "c", "d", "e", "f", ".git"), { recursive: true }); + + const dirs = findGitDirectories(testDir, 3); + + // Should not find the deeply nested repo + expect(dirs).toEqual([]); + }); + }); + + describe("findRepoCheckout", () => { + it("should return error for invalid repo slug", () => { + const result = findRepoCheckout(""); + expect(result.success).toBe(false); + expect(result.error).toBe("Invalid repo slug provided"); + }); + + it("should return error for null repo slug", () => { + const result = findRepoCheckout(null); + expect(result.success).toBe(false); + expect(result.error).toBe("Invalid repo slug provided"); + }); + + it("should return not found for missing repo", () => { + const testDir = `/tmp/test-find-repo-${Date.now()}`; + fs.mkdirSync(testDir, { recursive: true }); + + try { + const result = findRepoCheckout("owner/missing-repo", testDir); + expect(result.success).toBe(false); + expect(result.error).toContain("not found in workspace"); + } finally { + fs.rmSync(testDir, { recursive: true, force: true }); + } + }); + }); + + describe("buildRepoCheckoutMap", () => { + let testDir; + + beforeEach(() => { + testDir = `/tmp/test-build-map-${Date.now()}`; + fs.mkdirSync(testDir, { recursive: true }); + }); + + afterEach(() => { + try { + fs.rmSync(testDir, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors + } + }); + + it("should return empty map when no repos found", () => { + const map = buildRepoCheckoutMap(testDir); + expect(map.size).toBe(0); + }); + + it("should find repos with valid git remotes", () => { + // Create a mock repo with a config file + const repoPath = path.join(testDir, "my-repo", ".git"); + fs.mkdirSync(repoPath, { recursive: true }); + fs.writeFileSync( + path.join(repoPath, "config"), + `[remote "origin"] + url = https://github.com/owner/my-repo.git + fetch = +refs/heads/*:refs/remotes/origin/* +` + ); + + // Without a real git binary, this won't work, so we expect an empty map + // since execGitSync will fail + const map = buildRepoCheckoutMap(testDir); + + // In a real git repo this would work, but in tests without git setup it's ok to be empty + expect(map).toBeDefined(); + }); + }); +}); + +describe("generate_git_patch multi-repo support", () => { + describe("getPatchPathForRepo", () => { + it("should include repo slug in path", () => { + const filePath = getPatchPathForRepo("feature-branch", "owner/repo"); + expect(filePath).toBe("/tmp/gh-aw/aw-owner-repo-feature-branch.patch"); + }); + + it("should sanitize repo slug", () => { + const filePath = getPatchPathForRepo("main", "org/my-project"); + expect(filePath).toBe("/tmp/gh-aw/aw-org-my-project-main.patch"); + }); + + it("should sanitize branch name", () => { + const filePath = getPatchPathForRepo("feature/add-login", "owner/repo"); + expect(filePath).toBe("/tmp/gh-aw/aw-owner-repo-feature-add-login.patch"); + }); + + it("should handle complex repo names", () => { + const filePath = getPatchPathForRepo("main", "github/gh-aw"); + expect(filePath).toBe("/tmp/gh-aw/aw-github-gh-aw-main.patch"); + }); + + it("should handle uppercase input", () => { + const filePath = getPatchPathForRepo("Feature-Branch", "Owner/Repo"); + expect(filePath).toBe("/tmp/gh-aw/aw-owner-repo-feature-branch.patch"); + }); + }); + + describe("sanitizeRepoSlugForPatch", () => { + it("should replace slash with dash", () => { + expect(sanitizeRepoSlugForPatch("owner/repo")).toBe("owner-repo"); + }); + + it("should handle special characters", () => { + expect(sanitizeRepoSlugForPatch("org:name/proj*test")).toBe("org-name-proj-test"); + }); + + it("should collapse multiple dashes", () => { + expect(sanitizeRepoSlugForPatch("org//repo")).toBe("org-repo"); + }); + + it("should remove leading/trailing dashes", () => { + expect(sanitizeRepoSlugForPatch("/owner/repo/")).toBe("owner-repo"); + }); + + it("should convert to lowercase", () => { + expect(sanitizeRepoSlugForPatch("Owner/Repo")).toBe("owner-repo"); + }); + + it("should return empty string for null/undefined", () => { + expect(sanitizeRepoSlugForPatch(null)).toBe(""); + expect(sanitizeRepoSlugForPatch(undefined)).toBe(""); + expect(sanitizeRepoSlugForPatch("")).toBe(""); + }); + }); + + describe("sanitizeBranchNameForPatch", () => { + it("should replace path separators with dashes", () => { + expect(sanitizeBranchNameForPatch("feature/login")).toBe("feature-login"); + expect(sanitizeBranchNameForPatch("fix\\bug")).toBe("fix-bug"); + }); + + it("should replace special characters", () => { + expect(sanitizeBranchNameForPatch("feature:test")).toBe("feature-test"); + expect(sanitizeBranchNameForPatch("fix*bug")).toBe("fix-bug"); + }); + + it("should collapse multiple dashes", () => { + expect(sanitizeBranchNameForPatch("feature//login")).toBe("feature-login"); + }); + + it("should remove leading/trailing dashes", () => { + expect(sanitizeBranchNameForPatch("-feature-")).toBe("feature"); + }); + + it("should convert to lowercase", () => { + expect(sanitizeBranchNameForPatch("Feature-Branch")).toBe("feature-branch"); + }); + + it("should handle empty/null input", () => { + expect(sanitizeBranchNameForPatch("")).toBe("unknown"); + expect(sanitizeBranchNameForPatch(null)).toBe("unknown"); + expect(sanitizeBranchNameForPatch(undefined)).toBe("unknown"); + }); + + it("should handle question marks and pipes", () => { + expect(sanitizeBranchNameForPatch("branch?name|test")).toBe("branch-name-test"); + }); + + it("should handle angle brackets", () => { + expect(sanitizeBranchNameForPatch("branch<>name")).toBe("branch-name"); + }); + }); +}); From d6148e083b821547cb12f79d2a7130c63f973465 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 27 Feb 2026 02:22:25 +0000 Subject: [PATCH 7/8] improve base branch resolution for cross-repo PRs --- actions/setup/js/generate_git_patch.cjs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/actions/setup/js/generate_git_patch.cjs b/actions/setup/js/generate_git_patch.cjs index 98cd72ee03..d485f8ecc5 100644 --- a/actions/setup/js/generate_git_patch.cjs +++ b/actions/setup/js/generate_git_patch.cjs @@ -59,10 +59,7 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { // 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) + - ")"; + const errorMessage = "baseBranch is required and must be a non-empty string (received: " + String(baseBranch) + ")"; debugLog(`Invalid baseBranch: ${errorMessage}`); return { patchPath, From 454cb4d1b2a995e6198f93b434b6c68526f8f7c1 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Fri, 27 Feb 2026 02:27:38 +0000 Subject: [PATCH 8/8] file --- actions/setup/js/find_repo_checkout.test.cjs | 297 ------------------- 1 file changed, 297 deletions(-) delete mode 100644 actions/setup/js/find_repo_checkout.test.cjs diff --git a/actions/setup/js/find_repo_checkout.test.cjs b/actions/setup/js/find_repo_checkout.test.cjs deleted file mode 100644 index f2dfe0a484..0000000000 --- a/actions/setup/js/find_repo_checkout.test.cjs +++ /dev/null @@ -1,297 +0,0 @@ -const fs = require("fs"); -const path = require("path"); -const { extractRepoSlugFromUrl, normalizeRepoSlug, findGitDirectories, findRepoCheckout, buildRepoCheckoutMap } = require("./find_repo_checkout.cjs"); -const { getPatchPathForRepo, sanitizeBranchNameForPatch, sanitizeRepoSlugForPatch } = require("./generate_git_patch.cjs"); - -describe("find_repo_checkout", () => { - describe("extractRepoSlugFromUrl", () => { - it("should extract slug from HTTPS URL", () => { - expect(extractRepoSlugFromUrl("https://github.com/owner/repo.git")).toBe("owner/repo"); - expect(extractRepoSlugFromUrl("https://github.com/owner/repo")).toBe("owner/repo"); - }); - - it("should extract slug from SSH URL", () => { - expect(extractRepoSlugFromUrl("git@github.com:owner/repo.git")).toBe("owner/repo"); - expect(extractRepoSlugFromUrl("git@github.com:owner/repo")).toBe("owner/repo"); - }); - - it("should handle GitHub Enterprise URLs", () => { - expect(extractRepoSlugFromUrl("https://github.example.com/org/project.git")).toBe("org/project"); - expect(extractRepoSlugFromUrl("git@github.example.com:org/project.git")).toBe("org/project"); - }); - - it("should normalize to lowercase", () => { - expect(extractRepoSlugFromUrl("https://github.com/Owner/Repo.git")).toBe("owner/repo"); - expect(extractRepoSlugFromUrl("git@github.com:OWNER/REPO")).toBe("owner/repo"); - }); - - it("should return null for invalid URLs", () => { - expect(extractRepoSlugFromUrl("")).toBeNull(); - expect(extractRepoSlugFromUrl("invalid")).toBeNull(); - expect(extractRepoSlugFromUrl(null)).toBeNull(); - expect(extractRepoSlugFromUrl(undefined)).toBeNull(); - }); - - it("should handle URLs with ports", () => { - expect(extractRepoSlugFromUrl("https://github.example.com:8443/org/repo.git")).toBe("org/repo"); - }); - - it("should handle HTTP URLs", () => { - expect(extractRepoSlugFromUrl("http://github.local/owner/repo")).toBe("owner/repo"); - }); - }); - - describe("normalizeRepoSlug", () => { - it("should normalize to lowercase", () => { - expect(normalizeRepoSlug("Owner/Repo")).toBe("owner/repo"); - expect(normalizeRepoSlug("ORG/PROJECT")).toBe("org/project"); - }); - - it("should trim whitespace", () => { - expect(normalizeRepoSlug(" owner/repo ")).toBe("owner/repo"); - }); - - it("should return empty string for invalid input", () => { - expect(normalizeRepoSlug("")).toBe(""); - expect(normalizeRepoSlug(null)).toBe(""); - expect(normalizeRepoSlug(undefined)).toBe(""); - }); - - it("should handle tabs and newlines", () => { - expect(normalizeRepoSlug("\towner/repo\n")).toBe("owner/repo"); - }); - }); - - describe("findGitDirectories", () => { - let testDir; - - beforeEach(() => { - testDir = `/tmp/test-find-git-dirs-${Date.now()}`; - fs.mkdirSync(testDir, { recursive: true }); - }); - - afterEach(() => { - try { - fs.rmSync(testDir, { recursive: true, force: true }); - } catch { - // Ignore cleanup errors - } - }); - - it("should find git directories in workspace", () => { - // Create a mock git repo structure - fs.mkdirSync(path.join(testDir, "repo-a", ".git"), { recursive: true }); - fs.mkdirSync(path.join(testDir, "repo-b", ".git"), { recursive: true }); - - const dirs = findGitDirectories(testDir); - - expect(dirs).toHaveLength(2); - expect(dirs).toContain(path.join(testDir, "repo-a")); - expect(dirs).toContain(path.join(testDir, "repo-b")); - }); - - it("should handle nested repos", () => { - // Create a nested structure - fs.mkdirSync(path.join(testDir, "projects", "frontend", ".git"), { recursive: true }); - fs.mkdirSync(path.join(testDir, "projects", "backend", ".git"), { recursive: true }); - - const dirs = findGitDirectories(testDir); - - expect(dirs).toHaveLength(2); - expect(dirs).toContain(path.join(testDir, "projects", "frontend")); - expect(dirs).toContain(path.join(testDir, "projects", "backend")); - }); - - it("should skip node_modules", () => { - fs.mkdirSync(path.join(testDir, "node_modules", "some-pkg", ".git"), { recursive: true }); - fs.mkdirSync(path.join(testDir, "actual-repo", ".git"), { recursive: true }); - - const dirs = findGitDirectories(testDir); - - expect(dirs).toHaveLength(1); - expect(dirs).toContain(path.join(testDir, "actual-repo")); - }); - - it("should return empty array when no git dirs found", () => { - fs.mkdirSync(path.join(testDir, "empty-folder"), { recursive: true }); - - const dirs = findGitDirectories(testDir); - - expect(dirs).toEqual([]); - }); - - it("should respect maxDepth", () => { - // Create a deeply nested repo - fs.mkdirSync(path.join(testDir, "a", "b", "c", "d", "e", "f", ".git"), { recursive: true }); - - const dirs = findGitDirectories(testDir, 3); - - // Should not find the deeply nested repo - expect(dirs).toEqual([]); - }); - }); - - describe("findRepoCheckout", () => { - it("should return error for invalid repo slug", () => { - const result = findRepoCheckout(""); - expect(result.success).toBe(false); - expect(result.error).toBe("Invalid repo slug provided"); - }); - - it("should return error for null repo slug", () => { - const result = findRepoCheckout(null); - expect(result.success).toBe(false); - expect(result.error).toBe("Invalid repo slug provided"); - }); - - it("should return not found for missing repo", () => { - const testDir = `/tmp/test-find-repo-${Date.now()}`; - fs.mkdirSync(testDir, { recursive: true }); - - try { - const result = findRepoCheckout("owner/missing-repo", testDir); - expect(result.success).toBe(false); - expect(result.error).toContain("not found in workspace"); - } finally { - fs.rmSync(testDir, { recursive: true, force: true }); - } - }); - }); - - describe("buildRepoCheckoutMap", () => { - let testDir; - - beforeEach(() => { - testDir = `/tmp/test-build-map-${Date.now()}`; - fs.mkdirSync(testDir, { recursive: true }); - }); - - afterEach(() => { - try { - fs.rmSync(testDir, { recursive: true, force: true }); - } catch { - // Ignore cleanup errors - } - }); - - it("should return empty map when no repos found", () => { - const map = buildRepoCheckoutMap(testDir); - expect(map.size).toBe(0); - }); - - it("should find repos with valid git remotes", () => { - // Create a mock repo with a config file - const repoPath = path.join(testDir, "my-repo", ".git"); - fs.mkdirSync(repoPath, { recursive: true }); - fs.writeFileSync( - path.join(repoPath, "config"), - `[remote "origin"] - url = https://github.com/owner/my-repo.git - fetch = +refs/heads/*:refs/remotes/origin/* -` - ); - - // Without a real git binary, this won't work, so we expect an empty map - // since execGitSync will fail - const map = buildRepoCheckoutMap(testDir); - - // In a real git repo this would work, but in tests without git setup it's ok to be empty - expect(map).toBeDefined(); - }); - }); -}); - -describe("generate_git_patch multi-repo support", () => { - describe("getPatchPathForRepo", () => { - it("should include repo slug in path", () => { - const filePath = getPatchPathForRepo("feature-branch", "owner/repo"); - expect(filePath).toBe("/tmp/gh-aw/aw-owner-repo-feature-branch.patch"); - }); - - it("should sanitize repo slug", () => { - const filePath = getPatchPathForRepo("main", "org/my-project"); - expect(filePath).toBe("/tmp/gh-aw/aw-org-my-project-main.patch"); - }); - - it("should sanitize branch name", () => { - const filePath = getPatchPathForRepo("feature/add-login", "owner/repo"); - expect(filePath).toBe("/tmp/gh-aw/aw-owner-repo-feature-add-login.patch"); - }); - - it("should handle complex repo names", () => { - const filePath = getPatchPathForRepo("main", "github/gh-aw"); - expect(filePath).toBe("/tmp/gh-aw/aw-github-gh-aw-main.patch"); - }); - - it("should handle uppercase input", () => { - const filePath = getPatchPathForRepo("Feature-Branch", "Owner/Repo"); - expect(filePath).toBe("/tmp/gh-aw/aw-owner-repo-feature-branch.patch"); - }); - }); - - describe("sanitizeRepoSlugForPatch", () => { - it("should replace slash with dash", () => { - expect(sanitizeRepoSlugForPatch("owner/repo")).toBe("owner-repo"); - }); - - it("should handle special characters", () => { - expect(sanitizeRepoSlugForPatch("org:name/proj*test")).toBe("org-name-proj-test"); - }); - - it("should collapse multiple dashes", () => { - expect(sanitizeRepoSlugForPatch("org//repo")).toBe("org-repo"); - }); - - it("should remove leading/trailing dashes", () => { - expect(sanitizeRepoSlugForPatch("/owner/repo/")).toBe("owner-repo"); - }); - - it("should convert to lowercase", () => { - expect(sanitizeRepoSlugForPatch("Owner/Repo")).toBe("owner-repo"); - }); - - it("should return empty string for null/undefined", () => { - expect(sanitizeRepoSlugForPatch(null)).toBe(""); - expect(sanitizeRepoSlugForPatch(undefined)).toBe(""); - expect(sanitizeRepoSlugForPatch("")).toBe(""); - }); - }); - - describe("sanitizeBranchNameForPatch", () => { - it("should replace path separators with dashes", () => { - expect(sanitizeBranchNameForPatch("feature/login")).toBe("feature-login"); - expect(sanitizeBranchNameForPatch("fix\\bug")).toBe("fix-bug"); - }); - - it("should replace special characters", () => { - expect(sanitizeBranchNameForPatch("feature:test")).toBe("feature-test"); - expect(sanitizeBranchNameForPatch("fix*bug")).toBe("fix-bug"); - }); - - it("should collapse multiple dashes", () => { - expect(sanitizeBranchNameForPatch("feature//login")).toBe("feature-login"); - }); - - it("should remove leading/trailing dashes", () => { - expect(sanitizeBranchNameForPatch("-feature-")).toBe("feature"); - }); - - it("should convert to lowercase", () => { - expect(sanitizeBranchNameForPatch("Feature-Branch")).toBe("feature-branch"); - }); - - it("should handle empty/null input", () => { - expect(sanitizeBranchNameForPatch("")).toBe("unknown"); - expect(sanitizeBranchNameForPatch(null)).toBe("unknown"); - expect(sanitizeBranchNameForPatch(undefined)).toBe("unknown"); - }); - - it("should handle question marks and pipes", () => { - expect(sanitizeBranchNameForPatch("branch?name|test")).toBe("branch-name-test"); - }); - - it("should handle angle brackets", () => { - expect(sanitizeBranchNameForPatch("branch<>name")).toBe("branch-name"); - }); - }); -});