diff --git a/actions/setup/js/assign_to_agent.cjs b/actions/setup/js/assign_to_agent.cjs index 8a308cea14..7c6ad87d59 100644 --- a/actions/setup/js/assign_to_agent.cjs +++ b/actions/setup/js/assign_to_agent.cjs @@ -15,7 +15,7 @@ const { sanitizeContent } = require("./sanitize_content.cjs"); * Module-level state — populated by main(), read by the exported getters below. * Using module-level variables (rather than closure-only state) allows the handler * manager to read final output values after all messages have been processed. - * @type {Array<{issue_number: number|null, pull_number: number|null, agent: string, owner: string|null, repo: string|null, success: boolean, skipped?: boolean, error?: string}>} + * @type {Array<{issue_number: number|null, pull_number: number|null, agent: string, owner: string|null, repo: string|null, pull_request_repo?: string|null, success: boolean, skipped?: boolean, error?: string}>} */ let _allResults = []; @@ -128,6 +128,7 @@ async function main(config = {}) { // Closure-level state let processedCount = 0; const agentCache = {}; + const processedAssignmentTargets = new Set(); // Reset module-level results for this handler invocation _allResults = []; @@ -232,10 +233,16 @@ async function main(config = {}) { const hasExplicitTarget = itemForTarget.issue_number != null || itemForTarget.pull_number != null; const effectiveTarget = hasExplicitTarget ? "*" : targetConfig; + const basePullRequestRepoSlug = pullRequestOwner && pullRequestRepo ? `${pullRequestOwner}/${pullRequestRepo}` : `${effectiveOwner}/${effectiveRepo}`; + // Handle per-item pull_request_repo override let effectivePullRequestRepoId = pullRequestRepoId; - if (message.pull_request_repo) { - const itemPullRequestRepo = String(message.pull_request_repo).trim(); + let effectivePullRequestRepoSlug = basePullRequestRepoSlug; + let hasValidatedPerItemPullRequestRepoOverride = false; + const hasPullRequestRepoOverrideField = message.pull_request_repo != null; + const trimmedPullRequestRepoOverride = typeof message.pull_request_repo === "string" ? message.pull_request_repo.trim() : ""; + if (trimmedPullRequestRepoOverride) { + const itemPullRequestRepo = trimmedPullRequestRepoOverride; const pullRequestRepoParts = itemPullRequestRepo.split("/"); if (pullRequestRepoParts.length === 2) { const defaultPullRequestRepo = pullRequestRepoConfig || defaultTargetRepo; @@ -254,6 +261,8 @@ async function main(config = {}) { `; const itemPullRequestRepoResponse = await githubClient.graphql(itemPullRequestRepoQuery, { owner: pullRequestRepoParts[0], name: pullRequestRepoParts[1] }); effectivePullRequestRepoId = itemPullRequestRepoResponse.repository.id; + effectivePullRequestRepoSlug = itemPullRequestRepo; + hasValidatedPerItemPullRequestRepoOverride = true; core.info(`Using per-item pull request repository: ${itemPullRequestRepo} (ID: ${effectivePullRequestRepoId})`); } catch (error) { const errorMsg = `Failed to fetch pull request repository ID for ${itemPullRequestRepo}: ${getErrorMessage(error)}`; @@ -264,6 +273,10 @@ async function main(config = {}) { } else { core.warning(`Invalid pull_request_repo format: ${itemPullRequestRepo}. Expected owner/repo. Using global pull-request-repo if configured.`); } + } else if (hasPullRequestRepoOverrideField && typeof message.pull_request_repo === "string") { + core.warning("Invalid pull_request_repo value. Expected owner/repo. Using global pull-request-repo if configured."); + } else if (hasPullRequestRepoOverrideField) { + core.warning("Invalid pull_request_repo value. Expected a non-empty owner/repo string. Using global pull-request-repo if configured."); } // Resolve the target issue or pull request number from context @@ -351,14 +364,19 @@ async function main(config = {}) { core.info(`${type} ID: ${assignableId}`); - const hasPerItemPullRequestRepoOverride = !!message.pull_request_repo; + const assignmentContextKey = `${effectiveOwner}/${effectiveRepo}:${type}:${number}:${effectivePullRequestRepoSlug}`; + const seenThisContextBefore = processedAssignmentTargets.has(assignmentContextKey); + // Track assignment context (target + per-item pull_request_repo) to prevent duplicate + // re-assignment calls while still allowing one global issue to fan out to multiple repos. + processedAssignmentTargets.add(assignmentContextKey); + const shouldAllowReassignment = hasValidatedPerItemPullRequestRepoOverride && !seenThisContextBefore; // Skip if agent is already assigned and no explicit per-item pull_request_repo is specified. // When a different pull_request_repo is provided on the message, allow re-assignment // so Copilot can be triggered for a different target repository on the same issue. - if (currentAssignees.some(a => a.id === agentId) && !hasPerItemPullRequestRepoOverride) { + if (currentAssignees.some(a => a.id === agentId) && !shouldAllowReassignment) { core.info(`${agentName} is already assigned to ${type} #${number}`); - _allResults.push({ issue_number: issueNumber, pull_number: pullNumber, agent: agentName, owner: effectiveOwner, repo: effectiveRepo, success: true }); + _allResults.push({ issue_number: issueNumber, pull_number: pullNumber, agent: agentName, owner: effectiveOwner, repo: effectiveRepo, pull_request_repo: effectivePullRequestRepoSlug, success: true }); return { success: true }; } @@ -372,7 +390,7 @@ async function main(config = {}) { if (!success) throw new Error(`Failed to assign ${agentName} via GraphQL`); core.info(`Successfully assigned ${agentName} coding agent to ${type} #${number}`); - _allResults.push({ issue_number: issueNumber, pull_number: pullNumber, agent: agentName, owner: effectiveOwner, repo: effectiveRepo, success: true }); + _allResults.push({ issue_number: issueNumber, pull_number: pullNumber, agent: agentName, owner: effectiveOwner, repo: effectiveRepo, pull_request_repo: effectivePullRequestRepoSlug, success: true }); return { success: true }; } catch (error) { let errorMessage = getErrorMessage(error); @@ -382,7 +400,7 @@ async function main(config = {}) { if (ignoreIfError && isAuthError) { core.warning(`Agent assignment failed for ${agentName} on ${type} #${number} due to authentication/permission error. Skipping due to ignore-if-error=true.`); core.info(`Error details: ${errorMessage}`); - _allResults.push({ issue_number: issueNumber, pull_number: pullNumber, agent: agentName, owner: effectiveOwner, repo: effectiveRepo, success: true, skipped: true }); + _allResults.push({ issue_number: issueNumber, pull_number: pullNumber, agent: agentName, owner: effectiveOwner, repo: effectiveRepo, pull_request_repo: effectivePullRequestRepoSlug, success: true, skipped: true }); return { success: true, skipped: true }; } @@ -410,7 +428,7 @@ async function main(config = {}) { core.warning(`Failed to post failure comment on ${type} #${number}: ${getErrorMessage(commentError)}`); } - _allResults.push({ issue_number: issueNumber, pull_number: pullNumber, agent: agentName, owner: effectiveOwner, repo: effectiveRepo, success: false, error: errorMessage }); + _allResults.push({ issue_number: issueNumber, pull_number: pullNumber, agent: agentName, owner: effectiveOwner, repo: effectiveRepo, pull_request_repo: effectivePullRequestRepoSlug, success: false, error: errorMessage }); return { success: false, error: errorMessage }; } }; @@ -475,7 +493,7 @@ async function writeAssignToAgentSummary() { summaryContent += successResults .map(r => { const itemType = r.issue_number ? `Issue #${r.issue_number}` : `Pull Request #${r.pull_number}`; - return `- ${itemType} → Agent: ${r.agent}`; + return `- ${itemType} → Agent: ${r.agent}${r.pull_request_repo ? ` (PR target: ${r.pull_request_repo})` : ""}`; }) .join("\n"); summaryContent += "\n\n"; @@ -486,7 +504,7 @@ async function writeAssignToAgentSummary() { summaryContent += skippedResults .map(r => { const itemType = r.issue_number ? `Issue #${r.issue_number}` : `Pull Request #${r.pull_number}`; - return `- ${itemType} → Agent: ${r.agent} (assignment failed due to error)`; + return `- ${itemType} → Agent: ${r.agent}${r.pull_request_repo ? ` (PR target: ${r.pull_request_repo})` : ""} (assignment failed due to error)`; }) .join("\n"); summaryContent += "\n\n"; @@ -497,7 +515,7 @@ async function writeAssignToAgentSummary() { summaryContent += failedResults .map(r => { const itemType = r.issue_number ? `Issue #${r.issue_number}` : `Pull Request #${r.pull_number}`; - return `- ${itemType} → Agent: ${r.agent}: ${r.error}`; + return `- ${itemType} → Agent: ${r.agent}${r.pull_request_repo ? ` (PR target: ${r.pull_request_repo})` : ""}: ${r.error}`; }) .join("\n"); diff --git a/actions/setup/js/assign_to_agent.test.cjs b/actions/setup/js/assign_to_agent.test.cjs index 2492d7b5c9..95de8eb397 100644 --- a/actions/setup/js/assign_to_agent.test.cjs +++ b/actions/setup/js/assign_to_agent.test.cjs @@ -38,6 +38,8 @@ global.github = mockGithub; describe("assign_to_agent", () => { let assignToAgentScript; let tempFilePath; + let sleepSpy; + const mockSleep = vi.fn().mockResolvedValue(); // Simulates the safe-output handler manager: builds handler config from env vars, // calls main() as a factory, then processes items from GH_AW_AGENT_OUTPUT. @@ -92,6 +94,7 @@ describe("assign_to_agent", () => { beforeEach(() => { vi.clearAllMocks(); + mockSleep.mockClear(); // Reset mockGithub.graphql to ensure no lingering mock implementations mockGithub.graphql = vi.fn(); @@ -122,6 +125,8 @@ describe("assign_to_agent", () => { // Clear module cache to ensure we get the latest version of assign_agent_helpers const helpersPath = require.resolve("./assign_agent_helpers.cjs"); delete require.cache[helpersPath]; + const errorRecovery = require("./error_recovery.cjs"); + sleepSpy = vi.spyOn(errorRecovery, "sleep").mockImplementation(mockSleep); const scriptPath = path.join(process.cwd(), "assign_to_agent.cjs"); assignToAgentScript = fs.readFileSync(scriptPath, "utf8"); @@ -131,6 +136,7 @@ describe("assign_to_agent", () => { if (tempFilePath && fs.existsSync(tempFilePath)) { fs.unlinkSync(tempFilePath); } + sleepSpy?.mockRestore(); }); it("should handle empty agent output", async () => { @@ -458,6 +464,229 @@ describe("assign_to_agent", () => { expect(lastGraphQLCall[1].targetRepoId).toBe("other-platform-repo-id"); }); + it("should process multiple assignments for the same temporary issue ID across different pull_request_repo targets", async () => { + process.env.GH_AW_AGENT_MAX_COUNT = "5"; + process.env.GH_AW_TEMPORARY_ID_MAP = JSON.stringify({ + aw_multi_repo: { repo: "test-owner/test-repo", number: 6587 }, + }); + process.env.GH_AW_AGENT_ALLOWED_PULL_REQUEST_REPOS = "test-owner/ios-repo,test-owner/android-repo"; + + setAgentOutput({ + items: [ + { + type: "assign_to_agent", + issue_number: "aw_multi_repo", + agent: "copilot", + pull_request_repo: "test-owner/ios-repo", + }, + { + type: "assign_to_agent", + issue_number: "aw_multi_repo", + agent: "copilot", + pull_request_repo: "test-owner/android-repo", + }, + ], + errors: [], + }); + + mockGithub.graphql + // Item 1: get per-item PR repository ID + .mockResolvedValueOnce({ + repository: { + id: "ios-repo-id", + }, + }) + // Item 1: find agent + .mockResolvedValueOnce({ + repository: { + suggestedActors: { + nodes: [{ login: "copilot-swe-agent", id: "agent-id" }], + }, + }, + }) + // Item 1: issue details (not assigned yet) + .mockResolvedValueOnce({ + repository: { + issue: { + id: "issue-id", + assignees: { + nodes: [], + }, + }, + }, + }) + // Item 1: assignment mutation + .mockResolvedValueOnce({ + replaceActorsForAssignable: { + __typename: "ReplaceActorsForAssignablePayload", + }, + }) + // Item 2: get per-item PR repository ID + .mockResolvedValueOnce({ + repository: { + id: "android-repo-id", + }, + }) + // Item 2: issue details (already assigned after item 1) + .mockResolvedValueOnce({ + repository: { + issue: { + id: "issue-id", + assignees: { + nodes: [{ id: "agent-id", login: "copilot-swe-agent" }], + }, + }, + }, + }) + // Item 2: assignment mutation should still run + .mockResolvedValueOnce({ + replaceActorsForAssignable: { + __typename: "ReplaceActorsForAssignablePayload", + }, + }); + + await eval(`(async () => { ${assignToAgentScript}; ${STANDALONE_RUNNER} })()`); + + expect(mockCore.info).not.toHaveBeenCalledWith(expect.stringContaining("copilot is already assigned to issue #6587")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Successfully assigned copilot coding agent to issue #6587")); + + const assignmentCalls = mockGithub.graphql.mock.calls.filter(([query]) => query.includes("replaceActorsForAssignable")); + expect(assignmentCalls).toHaveLength(2); + expect(assignmentCalls[0][1].targetRepoId).toBe("ios-repo-id"); + expect(assignmentCalls[1][1].targetRepoId).toBe("android-repo-id"); + expect(mockSleep).toHaveBeenCalledTimes(1); + expect(mockSleep).toHaveBeenCalledWith(10000); + + const summaryCall = mockCore.summary.addRaw.mock.calls[0][0]; + expect(summaryCall).toContain("PR target: test-owner/ios-repo"); + expect(summaryCall).toContain("PR target: test-owner/android-repo"); + }); + + it("should avoid duplicate re-assignment for the same issue and same pull_request_repo in one run", async () => { + process.env.GH_AW_AGENT_MAX_COUNT = "5"; + process.env.GH_AW_TEMPORARY_ID_MAP = JSON.stringify({ + aw_duplicate: { repo: "test-owner/test-repo", number: 6587 }, + }); + process.env.GH_AW_AGENT_ALLOWED_PULL_REQUEST_REPOS = "test-owner/ios-repo"; + + setAgentOutput({ + items: [ + { + type: "assign_to_agent", + issue_number: "aw_duplicate", + agent: "copilot", + pull_request_repo: "test-owner/ios-repo", + }, + { + type: "assign_to_agent", + issue_number: "aw_duplicate", + agent: "copilot", + pull_request_repo: "test-owner/ios-repo", + }, + ], + errors: [], + }); + + mockGithub.graphql + // Item 1: get per-item PR repository ID + .mockResolvedValueOnce({ + repository: { + id: "ios-repo-id", + }, + }) + // Item 1: find agent + .mockResolvedValueOnce({ + repository: { + suggestedActors: { + nodes: [{ login: "copilot-swe-agent", id: "agent-id" }], + }, + }, + }) + // Item 1: issue details (not assigned yet) + .mockResolvedValueOnce({ + repository: { + issue: { + id: "issue-id", + assignees: { + nodes: [], + }, + }, + }, + }) + // Item 1: assignment mutation + .mockResolvedValueOnce({ + replaceActorsForAssignable: { + __typename: "ReplaceActorsForAssignablePayload", + }, + }) + // Item 2: get per-item PR repository ID + .mockResolvedValueOnce({ + repository: { + id: "ios-repo-id", + }, + }) + // Item 2: issue details (already assigned after item 1) + .mockResolvedValueOnce({ + repository: { + issue: { + id: "issue-id", + assignees: { + nodes: [{ id: "agent-id", login: "copilot-swe-agent" }], + }, + }, + }, + }); + + await eval(`(async () => { ${assignToAgentScript}; ${STANDALONE_RUNNER} })()`); + + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("copilot is already assigned to issue #6587")); + const assignmentCalls = mockGithub.graphql.mock.calls.filter(([query]) => query.includes("replaceActorsForAssignable")); + expect(assignmentCalls).toHaveLength(1); + expect(mockSleep).toHaveBeenCalledTimes(1); + expect(mockSleep).toHaveBeenCalledWith(10000); + }); + + it("should not treat whitespace pull_request_repo as a reassignment override", async () => { + setAgentOutput({ + items: [ + { + type: "assign_to_agent", + issue_number: 42, + agent: "copilot", + pull_request_repo: " ", + }, + ], + errors: [], + }); + + mockGithub.graphql + // Find agent + .mockResolvedValueOnce({ + repository: { + suggestedActors: { + nodes: [{ login: "copilot-swe-agent", id: "agent-id" }], + }, + }, + }) + // Get issue details - already assigned + .mockResolvedValueOnce({ + repository: { + issue: { + id: "issue-id", + assignees: { + nodes: [{ id: "agent-id", login: "copilot-swe-agent" }], + }, + }, + }, + }); + + await eval(`(async () => { ${assignToAgentScript}; ${STANDALONE_RUNNER} })()`); + + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("copilot is already assigned to issue #42")); + const assignmentCalls = mockGithub.graphql.mock.calls.filter(([query]) => query.includes("replaceActorsForAssignable")); + expect(assignmentCalls).toHaveLength(0); + }); + it("should still skip when agent is already assigned with global pull-request-repo but no per-item override", async () => { process.env.GH_AW_AGENT_PULL_REQUEST_REPO = "test-owner/global-pr-repo"; setAgentOutput({ @@ -1356,7 +1585,9 @@ describe("assign_to_agent", () => { // Verify delay message was logged twice (2 delays between 3 items) const delayMessages = mockCore.info.mock.calls.filter(call => call[0].includes("Waiting 10 seconds before processing next agent assignment")); expect(delayMessages).toHaveLength(2); - }, 30000); // Increase timeout to 30 seconds to account for 2x10s delays + expect(mockSleep).toHaveBeenCalledTimes(2); + expect(mockSleep).toHaveBeenCalledWith(10000); + }); describe("Cross-repository allowlist validation", () => { it("should reject target repository not in allowlist", async () => {