[WIP] Add support for multiple assignments to agent per issue#28103
[WIP] Add support for multiple assignments to agent per issue#28103
Conversation
…flows Agent-Logs-Url: https://github.com/github/gh-aw/sessions/35aa1b6a-6d55-47e5-b040-3f7167d9bf21 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/35aa1b6a-6d55-47e5-b040-3f7167d9bf21 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
🧪 Test Quality Sentinel ReportTest Quality Score: 80/100✅ Excellent test quality
Test Classification DetailsView Test Details
Test AnalysisBoth new tests directly verify the behavioral contracts introduced by the production change:
Mocking targets are exclusively external runtime dependencies (
|
There was a problem hiding this comment.
Pull request overview
Enables multiple assign_to_agent operations for the same issue within a single run when targeting different pull_request_repo repositories, while still preventing redundant duplicate assignments for the same target.
Changes:
- Track processed assignment contexts to allow re-assignment when
pull_request_repodiffers. - Add JS tests for multi-repo assignment with temporary issue IDs and same-repo duplicate suppression.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/assign_to_agent.cjs | Adds per-run tracking to permit multiple assignments per issue across different pull_request_repo targets. |
| actions/setup/js/assign_to_agent.test.cjs | Adds coverage for multi-repo assignment and same-repo duplicate suppression in a single run. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
actions/setup/js/assign_to_agent.test.cjs:633
- Same as the previous test: running 2 items in one handler invocation will incur the real 10-second inter-assignment delay, slowing the suite and risking timeout flakiness. Please stub/mimic
sleepin tests (or adjust the runner to disable delays under test) so multi-item scenarios don’t add ~10s per test case.
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);
- Files reviewed: 2/2 changed files
- Comments generated: 3
| 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); |
There was a problem hiding this comment.
This test processes 2 assignment items in a single run; the implementation intentionally await sleep(10000) between consecutive assignments, so this adds a real 10s delay to the test suite and may become flaky under CI load with a 20s timeout. Recommend stubbing/mocking sleep (e.g., patching ./error_recovery.cjs export in require.cache before eval) or using fake timers, rather than paying the real delay.
This issue also appears on line 553 of the same file.
| 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 |
There was a problem hiding this comment.
The reassignment guard treats any truthy message.pull_request_repo as a valid per-item override. That means whitespace-only or invalid-format values (which are only warned about earlier) will still cause shouldAllowReassignment to 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., effectivePullRequestRepoId differs from the global pullRequestRepoId), and use that validated/normalized value (or repo ID) in assignmentContextKey.
| 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 | |
| const normalizedPerItemPullRequestRepo = typeof message.pull_request_repo === "string" | |
| ? message.pull_request_repo.trim() | |
| : ""; | |
| const hasPerItemPullRequestRepoOverride = normalizedPerItemPullRequestRepo.length > 0 | |
| && validateRepo(normalizedPerItemPullRequestRepo); | |
| const normalizedPullRequestRepo = hasPerItemPullRequestRepoOverride | |
| ? normalizedPerItemPullRequestRepo | |
| : "default"; | |
| const assignmentContextKey = `${effectiveOwner}/${effectiveRepo}:${type}:${number}:${normalizedPullRequestRepo}`; | |
| const seenThisContextBefore = processedAssignmentTargets.has(assignmentContextKey); | |
| // Track assignment context (target + validated per-item pull_request_repo) to prevent duplicate |
| 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) { |
There was a problem hiding this comment.
With multi-repo assignment now possible, _allResults/step summary/output formatting can’t distinguish which pull_request_repo target was used (the summary will list duplicate “Issue #N → Agent: X” lines, and assigned output repeats issue:N:agent). Consider recording the effective PR target (slug and/or repo ID) in the per-item result and including it in the summary (at minimum) so multi-target assignments are observable and debuggable.
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d2b4077d-7102-41a5-930e-a274fbe9e182 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…erride handling Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d2b4077d-7102-41a5-930e-a274fbe9e182 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d2b4077d-7102-41a5-930e-a274fbe9e182 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reviewed and addressed all actionable review comments. Updates are in c99393d (plus prior related fixes in 5b6d985 and a111860): reassignment now keys off validated per-item pull_request_repo context, whitespace/invalid overrides no longer bypass already-assigned checks, summary now shows PR target per assignment, and tests no longer incur real 10s sleeps. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This PR enables multiple
assign_to_agentoperations for the same issue in a single run when each assignment targets a differentpull_request_repo, while still preventing duplicate assignments to the same target.What changed
pull_request_repohandling so only validated overrides can enable reassignment (whitespace/invalid values no longer bypass the already-assigned short-circuit).sleep, making tests faster and less flaky.Validation
assign_to_agentpass.make agent-finishstill reports pre-existing unrelated failures in this branch baseline (TestCopilotDetectionDefaultModel,TestWasmGolden_CompileFixtures).