-
Notifications
You must be signed in to change notification settings - Fork 361
[WIP] Add support for multiple assignments to agent per issue #28103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
a49f1ca
a487ae7
0bef8ac
a111860
5b6d985
c99393d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 = []; | ||
|
|
@@ -352,11 +353,18 @@ async function main(config = {}) { | |
| core.info(`${type} ID: ${assignableId}`); | ||
|
|
||
| const hasPerItemPullRequestRepoOverride = !!message.pull_request_repo; | ||
| const normalizedPullRequestRepo = hasPerItemPullRequestRepoOverride ? String(message.pull_request_repo).trim() : "default"; | ||
| const assignmentContextKey = `${effectiveOwner}/${effectiveRepo}:${type}:${number}:${normalizedPullRequestRepo}`; | ||
| 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 = hasPerItemPullRequestRepoOverride && !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 }); | ||
| return { success: true }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -458,6 +458,180 @@ 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"); | ||
| }, 20000); | ||
|
||
|
|
||
| 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); | ||
| }, 20000); | ||
|
|
||
| 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({ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reassignment guard treats any truthy
message.pull_request_repoas a valid per-item override. That means whitespace-only or invalid-format values (which are only warned about earlier) will still causeshouldAllowReassignmentto become true on first encounter and bypass the "already assigned" short-circuit, triggering an unnecessary assignment mutation. Consider basing the override/reassignment decision on whether a per-item repo override was successfully resolved (e.g.,effectivePullRequestRepoIddiffers from the globalpullRequestRepoId), and use that validated/normalized value (or repo ID) inassignmentContextKey.