Improve base branch resolution for cross-repo PRs

actions/setup/js/create_agent_session.cjs

@@ -65,16 +65,27 @@ async function main() {
     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());
-
     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";
     }
@@ -85,11 +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
-  const baseBranch = process.env.GITHUB_AW_AGENT_SESSION_BASE || (await getBaseBranch());
-
   // Process all agent session items
   const createdTasks = [];
   let summaryContent = "## ✅ Agent Sessions Created\n\n";
@@ -109,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

actions/setup/js/create_pull_request.cjs

@@ -100,12 +100,24 @@ 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
-  // 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);
+
+  // 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}")`);
+    }
+  }
+
   const includeFooter = parseBoolTemplatable(config.footer, true);
   const fallbackAsIssue = config.fallback_as_issue !== false; // Default to true (fallback enabled)
 
@@ -115,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(", ")}`);
@@ -221,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

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,22 +46,31 @@ 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)
  *   - "incremental": Only include commits since origin/branchName (for push_to_pull_request_branch)
  *     In incremental mode, origin/branchName is fetched explicitly and merge-base fallback is disabled.
  * @returns {Promise<Object>} Object with patch info or error
  */
-async function generateGitPatch(branchName, options = {}) {
-  const mode = options.mode || "full";
+async function generateGitPatch(branchName, baseBranch, options = {}) {
   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();
-  // 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());
+  const defaultBranch = baseBranch;
   const githubSha = process.env.GITHUB_SHA;
 
   debugLog(`Starting patch generation: mode=${mode}, branch=${branchName}, defaultBranch=${defaultBranch}`);

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");

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<string>} 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;