Skip to content

fix: allow multiple agent assignments to same issue when pull_request_repo differs#25916

Merged
pelikhan merged 4 commits intomainfrom
copilot/allow-multiple-assignments
Apr 12, 2026
Merged

fix: allow multiple agent assignments to same issue when pull_request_repo differs#25916
pelikhan merged 4 commits intomainfrom
copilot/allow-multiple-assignments

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 12, 2026

The already-assigned guard in assign_to_agent.cjs unconditionally short-circuits when the agent is already an assignee, preventing a second Copilot session from being triggered against a different pull_request_repo (e.g., iOS repo + Android repo on the same issue).

Change: Only skip when no per-item pull_request_repo override is provided:

// Before: always skips
if (currentAssignees.some(a => a.id === agentId)) {

// After: skips only when no per-item cross-repo target is specified
const hasPerItemPullRequestRepoOverride = !!message.pull_request_repo;
if (currentAssignees.some(a => a.id === agentId) && !hasPerItemPullRequestRepoOverride) {

This keys the dedup guard off message.pull_request_repo (the per-item override) rather than the resolved effectivePullRequestRepoId, which would also be set when a global pull-request-repo is configured. This preserves no-op dedup behavior for plain re-assignments and global config, while allowing Copilot to be triggered for a second platform (different pull_request_repo) on the same issue.

  • assign_to_agent.cjs — Added hasPerItemPullRequestRepoOverride check to the dedup guard so assignments with a per-item target repo proceed
  • assign_to_agent.test.cjs — Added test case verifying re-assignment proceeds when agent is already assigned but a per-item pull_request_repo is specified; added regression test verifying dedup still skips when only a global GH_AW_AGENT_PULL_REQUEST_REPO is set with no per-item override

Copilot AI and others added 2 commits April 12, 2026 15:54
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b4681576-a2d6-406e-982c-95c92bc37bd7

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…pecified

When assign_to_agent is called for an issue where the agent is already
assigned, allow re-assignment if a pull_request_repo is specified. This
enables multi-platform workflows where the same issue triggers Copilot
sessions in different target repositories.

Fixes #6587

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b4681576-a2d6-406e-982c-95c92bc37bd7

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix multiple assignment issue for different repositories fix: allow multiple agent assignments to same issue when pull_request_repo differs Apr 12, 2026
Copilot AI requested a review from pelikhan April 12, 2026 16:03
@pelikhan pelikhan marked this pull request as ready for review April 12, 2026 16:20
Copilot AI review requested due to automatic review settings April 12, 2026 16:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the agent-assignment logic so a second Copilot session can be triggered on the same issue when targeting a different pull_request_repo, and adjusts the maintenance workflow output.

Changes:

  • Adjusted the “already assigned” dedup guard in assign_to_agent.cjs to allow re-assignment when a PR target repo is involved.
  • Added a test for re-assignment behavior when an item specifies a different pull_request_repo.
  • Updated the maintenance workflow (runner label + removed Docker daemon startup step) and modified related generator comments.
Show a summary per file
File Description
actions/setup/js/assign_to_agent.cjs Changes dedup logic for already-assigned agent to permit cross-repo PR targeting.
actions/setup/js/assign_to_agent.test.cjs Adds coverage for re-assignment when an item specifies a different pull_request_repo.
pkg/workflow/maintenance_workflow.go Updates documentation comments for generated workflow conditions (currently introduces incorrect smart-quote literals).
.github/workflows/agentics-maintenance.yml Changes validate_workflows runner label and removes Docker daemon startup step.

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)

pkg/workflow/maintenance_workflow.go:757

  • Same issue here: the comment uses a smart quote in operation != ”, which is misleading and doesn’t match the actual condition built (BuildStringLiteral("")). Update the comment to the correct empty-string literal for clarity.
// buildRunOperationCondition creates the condition for the unified run_operation
// job that handles all dispatch operations except the ones with dedicated jobs.
// Condition: dispatch && operation != ” && operation != each excluded && !fork.
func buildRunOperationCondition(excludedOperations ...string) ConditionNode {
  • Files reviewed: 4/4 changed files
  • Comments generated: 4

Comment on lines +354 to +357
// Skip if agent is already assigned and no explicit pull_request_repo is specified.
// When a different pull_request_repo is provided, allow re-assignment so Copilot
// can be triggered for a different target repository on the same issue.
if (currentAssignees.some(a => a.id === agentId) && !effectivePullRequestRepoId) {
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dedup guard is checking !effectivePullRequestRepoId, but effectivePullRequestRepoId is also set when a global pull-request-repo is configured (see initialization from pullRequestRepoId). That means if an agent is already assigned and a global PR repo is configured, this will no longer skip and will attempt to re-assign every time, which can unintentionally trigger duplicate Copilot sessions. Consider keying the skip condition off whether a per-item message.pull_request_repo override was provided (and/or whether it differs from the global repo), rather than whether the resolved repo ID is non-null.

Suggested change
// Skip if agent is already assigned and no explicit pull_request_repo is specified.
// When a different pull_request_repo is provided, allow re-assignment so Copilot
// can be triggered for a different target repository on the same issue.
if (currentAssignees.some(a => a.id === agentId) && !effectivePullRequestRepoId) {
const hasPerItemPullRequestRepoOverride = !!message.pull_request_repo;
// 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) {

Copilot uses AI. Check for mistakes.
Comment on lines +391 to +399
it("should allow re-assignment when agent is already assigned but pull_request_repo differs", async () => {
process.env.GH_AW_AGENT_PULL_REQUEST_REPO = "test-owner/default-pr-repo";
process.env.GH_AW_AGENT_ALLOWED_PULL_REQUEST_REPOS = "test-owner/other-platform-repo";
setAgentOutput({
items: [
{
type: "assign_to_agent",
issue_number: 42,
agent: "copilot",
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new test covers the per-item pull_request_repo override case, but it doesn’t cover the regression risk introduced by the updated dedup guard when a global pull-request-repo is configured and no per-item override is present. Add a test asserting that when the agent is already assigned and pull_request_repo is not provided, the handler still logs the "already assigned" message and does not call the assignment mutation, even if GH_AW_AGENT_PULL_REQUEST_REPO is set.

Copilot uses AI. Check for mistakes.
Comment on lines 738 to 741
// buildNotForkAndScheduledOrOperation creates a condition for jobs that run on
// schedule (or empty operation) AND when a specific operation is selected.
// Condition: !fork && (not_dispatch || operation == '' || operation == op)
// Condition: !fork && (not_dispatch || operation == || operation == op)
func buildNotForkAndScheduledOrOperation(operation string) ConditionNode {
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition in this comment is now documented as operation == ” (smart quote) which no longer represents an empty-string check and is likely a copy/paste/encoding mistake. Update it to use the correct empty-string literal (consistent with the surrounding code/other comments).

This issue also appears on line 754 of the same file.

Copilot uses AI. Check for mistakes.
Comment on lines 282 to 286
validate_workflows:
if: ${{ github.event_name == 'workflow_dispatch' && github.event.inputs.operation == 'validate' && !github.event.repository.fork }}
runs-on: ubuntu-latest
runs-on: ubuntu-slim
permissions:
contents: read
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR includes workflow changes (runner label change and removal of the Docker-daemon step) that aren’t mentioned in the PR title/description, which currently describes only the assign_to_agent dedup behavior. Either update the PR description to cover these workflow changes, or split them into a separate PR to keep the change set focused and easier to review.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

Excellent test quality

Metric Value
New/modified tests analyzed 1
✅ Design tests (behavioral contracts) 1 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (100%)
Duplicate test clusters 0
Test inflation detected ⚠️ Yes (70 test lines / 4 production lines ≈ 17.5:1)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
should allow re-assignment when agent is already assigned but pull_request_repo differs actions/setup/js/assign_to_agent.test.cjs:388 ✅ Design Minor: test inflation flag (see note)

Test Analysis

should allow re-assignment when agent is already assigned but pull_request_repo differs

Classification: Design test — Behavioral contract

What design invariant does this test enforce?
The core behavioral invariant being fixed: when pull_request_repo differs from the default repo, the assignment logic must not skip due to an existing assignment, and must use the per-item PR repo ID for the GraphQL mutation.

What would break if deleted?
A regression to the old behavior would go undetected — the system would silently skip reassignment when the agent is already assigned, even when a different pull_request_repo is explicitly requested. This is high-value coverage.

Assertions: 5 assertions covering:

  1. Skip message is not emitted (not.toHaveBeenCalledWith on "is already assigned")
  2. Success message is emitted (toHaveBeenCalledWith on "Successfully assigned")
  3. setFailed is not called
  4. The final GraphQL call targets the agentAssignment mutation
  5. The mutation receives the correct targetRepoId ("other-platform-repo-id" — the per-item repo, not the default)

Mocking: External dependencies only (mockGithub.graphql, mockCore) — legitimate for GitHub Actions tests. Business logic is not mocked.


⚠️ Test Inflation Note

The test inflation ratio is 17.5:1 (70 test lines / 4 production lines). This triggers a mechanical penalty in the score, but in this case it is expected and appropriate: the small conditional fix (&& !effectivePullRequestRepoId) requires setting up the full GraphQL mock chain (5 sequential mockResolvedValueOnce calls) to exercise the real code path. This is the standard pattern for JavaScript tests in this codebase. No action required.


Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 0 tests
  • 🟨 JavaScript (*.test.cjs, *.test.js): 1 test (vitest)

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). The single new test directly exercises the behavioral contract introduced by the fix, covering both the negative path (skip not triggered) and the positive path (correct mutation called with correct parameters).


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 476.1K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 80/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). The new test directly verifies the behavioral contract of the fix with 5 assertions covering both the skip-prevention and the correct GraphQL mutation parameters.

@github-actions github-actions bot mentioned this pull request Apr 12, 2026
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review all comments. Revert agentic maintenance validation job

…ce changes

Address PR review feedback:
- Use message.pull_request_repo (per-item override) instead of
  effectivePullRequestRepoId (which includes global config) for the
  already-assigned dedup guard
- Add test for global pull-request-repo with already-assigned agent
  (no per-item override) to verify dedup still works
- Revert unrelated agentics-maintenance.yml and maintenance_workflow.go
  changes

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9202ff8f-8030-4a1e-835d-f6ed3e77b236

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 12, 2026

@copilot review all comments. Revert agentic maintenance validation job

Addressed in 001ba4e:

  • Dedup guard now keys off message.pull_request_repo (per-item override) instead of effectivePullRequestRepoId (which includes global config), so global pull-request-repo alone won't bypass dedup
  • Added test verifying dedup still skips when only global GH_AW_AGENT_PULL_REQUEST_REPO is set with no per-item override
  • Reverted unrelated agentics-maintenance.yml and maintenance_workflow.go changes

@pelikhan pelikhan merged commit c338e73 into main Apr 12, 2026
77 of 78 checks passed
@pelikhan pelikhan deleted the copilot/allow-multiple-assignments branch April 12, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

assign_to_agent: allow multiple assignments to same issue when pull_request_repo differs

3 participants