Uh oh!
There was an error while loading. Please reload this page.
Restore copilot assignee resolution by preferring issue-scoped assignee checks - #41306
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan
commented
Jun 24, 2026
@copilot Add logging to allow easier debugging |
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in coding-agent assignee validation by preferring the issue-scoped assignee check endpoint when issue/PR context exists, with a repository-scoped fallback for compatibility.
Changes:
- Added a shared assignee-alias validator and threaded optional issue/PR context into
findAgent(...)/getAvailableAgentLogins(...). - Updated multiple call sites to pass issue numbers into agent lookup (issue creation, PR creation fallback, and assign-to-agent).
- Extended tests to cover issue-scoped validation and fallback behavior.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/assign_agent_helpers.cjs | Adds validateAssigneeAlias(...) and updates agent lookup/availability helpers to prefer issue-scoped assignability checks. |
| actions/setup/js/assign_agent_helpers.test.cjs | Adds test coverage for issue-scoped alias validation and fallback behavior. |
| actions/setup/js/assign_to_agent.cjs | Threads issue/PR number into findAgent(...) for assign-to-agent flow. |
| actions/setup/js/assign_copilot_to_created_issues.cjs | Passes issue number into findAgent(...) for created-issues assignment flow. |
| actions/setup/js/assign_copilot_to_created_issues.test.cjs | Updates expectations for findAgent(...) call signature change (now includes issue number). |
| actions/setup/js/create_issue.cjs | Passes issue number into findAgent(...) when auto-assigning Copilot during issue creation. |
| actions/setup/js/create_pull_request.cjs | Passes fallback issue number into findAgent(...) for PR flow fallback issue assignment. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 5
Uh oh!
There was an error while loading. Please reload this page.
| /** | ||
| * Return list of coding agent bot login names that are currently available as assignable actors | ||
| * in this repository, as determined by checkUserCanBeAssigned. | ||
| * @param {string} owner |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added targeted debug logging in |
✅ Test Quality Sentinel completed test quality analysis. |
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #41306 does not have the 'implementation' label (has_implementation_label=false) and has 0 new lines of code in business logic directories (well under the 100-line threshold). The 7 changed files are skill scripts, generated .lock.yml workflows, and a shared markdown doc. |
✅ PR Code Quality Reviewer completed the code quality review. |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnose and /tdd — commenting with observations; no blocking issues found.
📋 Key Themes & Highlights
Key Themes
- Missing regression test: The specific failing scenario (issue-scoped 204 + repo-scoped 404 for
copilot-swe-agent) is not captured as a test case — a future revert to repo-scoped would go undetected. - Test brittleness: The new
getAvailableAgentLoginstests rely on call-order mocks that assume a specific alias ordering fromAGENT_LOGIN_NAMES;mockImplementationwith parameter matching would be safer. - Undocumented fallback intent: The 404 path in
validateAssigneeAliasfalls back to the repo-scoped check despite 404 meaning "not assignable to this issue" in the GitHub API; a comment explaining the deliberate treatment as "inconclusive" would help maintainability. - Parameter insertion risk: Adding
issueNumberbetweenagentNameandgithubClientis a positional breaking change — all call sites are updated here, but an options bag would guard against future silently-broken callers.
Positive Highlights
- ✅ Clean extraction of
validateAssigneeAlias— single responsibility, good debug logging throughout - ✅ Tight error discrimination: only 404/422 trigger fallback; all other statuses re-throw, preserving observability
- ✅
issueNumber || pullNumberinassign_to_agent.cjselegantly handles both issue and PR contexts in one line - ✅ All five call sites correctly updated and the existing test suite adapted to match the new call routing
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 108 AIC · ⌖ 8.35 AIC · ⊞ 6.5K
Comments that could not be inline-anchored
actions/setup/js/assign_agent_helpers.test.cjs:244
[/diagnose] The regression scenario is not directly tested: copilot-swe-agent succeeds the issue-scoped check but fails the repo-scoped check — the root cause described in the PR.
The current test has copilot-swe-agent get a 404 from issue-scoped and fall back, while github-copilot-enterprise succeeds via issue-scoped. That validates the routing logic but misses the specific regression.
<details>
<summary>💡 Suggested regression test</summary>
it("shouldfindcopilot-swe-agen…</details><details><summary>actions/setup/js/assign_agent_helpers.test.cjs:127</summary>**[/tdd]**Themocksequence`mockRejectedValueOnce→mockResolvedValueOnce→mockRejectedValue`isposition-dependent:itassumes`github-copilot-enterprise`isalwaysthe2ndaliasiteratedin`AGENT_LOGIN_NAMES`.Ifthealiasorderchanges,thetestsilentlyteststhewrongalias.
Thesameissueappearsatline146inthe422fallbacktest(`checkUserCanBeAssigned.mockRejectedValueOnce(...).mockResolvedValueOnce(...)`).<details><summary>💡SuggestedfixusingmockImplementation</summary>…
</details><details><summary>actions/setup/js/assign_agent_helpers.cjs:125</summary>
**[/diagnose]** The 404 fallback warrants a comment: per GitHub's API docs, 404 from `GET /repos/{owner}/{repo}/issues/{issue_number}/assignees/{assignee}` means "user cannot be assigned to this issue" — not "endpoint unavailable". Falling back to the repo-scoped check on a definitive 404 could produce a false positive if the repo-scoped check returns 204.
In practice this is benign for bot logins (repo-scoped also returns 404 for them), but the intent is non-obvious and the next maintainer ma…
</details><details><summary>actions/setup/js/assign_agent_helpers.cjs:190</summary>**[/zoom-out]**Inserting`issueNumber`asanewpositionalparameterbetween`agentName`and`githubClient`isabreakingAPIchangemitigatedherebyupdatingallknowncallsites.Anycalleroutsidethisdiffthatpassedacustom`githubClient`asthe4thargwouldsilentlyreceive`issueNumber=<githubClientobject>`(truthy)andattempttouseitasanissuenumber.
Thesameconcernappliesto `getAvailableAgentLogins` atline82.<details><summary>💡Consideranoptionsbagforfuture…</details>pelikhan
commented
Jun 24, 2026
@copilot run or-finisher skill |
There was a problem hiding this comment.
REQUEST_CHANGES — Two high-severity bugs plus two broken tests block this merge.
Blocking issues summary
High
TypeError +
NaNinvalidateAssigneeAlias(line 139,assign_agent_helpers.cjs, existing comment): if the providedgithubClientlacks a.requestmethod, aTypeErroris thrown with nostatusproperty — the conditionstatus !== 404 && status !== 422evaluates totrue, so the error is re-thrown and the intended fallback tocheckUserCanBeAssignedis silently skipped. Additionally,Number(issueNumber)producesNaNfor non-numeric strings and for object values, corrupting the API call without any validation guard.getAvailableAgentLoginscalled with wrong arity inassign_to_agent.cjs(line 408, existing comment at line 335):getAvailableAgentLogins(effectiveOwner, effectiveRepo, githubClient)passes the custom client as theissueNumberparameter. This causesif (issueNumber)to be truthy (an object),Number(githubClient)=NaNis sent asissue_number, andgithubClientdefaults to the globalgithubobject — silently dropping the intended custom client in every error-enrichment path.
Medium
Broken test:
getAvailableAgentLoginswith issue number (line 127,assign_agent_helpers.test.cjs, existing comment):checkUserCanBeAssignedis not mocked to reject in this test. Aftervi.clearAllMocks(), the mock returnsundefined(not a rejected promise), soawait checkUserCanBeAssigned(...)resolves — all four fallback logins pass validation. The test expects["github-copilot-enterprise"]but will receive all five aliases.Broken test:
findAgentwith issue number (line 247,assign_agent_helpers.test.cjs, existing comment): same root cause — the first alias falls back to an unmockedcheckUserCanBeAssignedwhich resolves, sofindAgentreturns on the first alias without ever reaching the second. The test asserts tworequestcalls but only one occurs.404 fallback silently overrides "not assignable" verdict (line 125,
assign_agent_helpers.cjs, new comment):status !== 404 && status !== 422routes both statuses to the repo-scoped fallback. HTTP 404 from this endpoint means the user cannot be assigned; treating it like 422 ("not applicable") allows a rejected login to resurface as available if the repo-scoped check is more permissive, deferring the real failure to assignment time.
🔎 Code quality review by PR Code Quality Reviewer · 116.9 AIC · ⌖ 7.64 AIC · ⊞ 5.2K
Uh oh!
There was an error while loading. Please reload this page.
🧪 Test Quality Sentinel Report
📊 Metrics & Test Classification (13 tests analyzed)
JavaScript: 13 (
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Ran the PR finisher pass and addressed the actionable blockers in commit |
@copilot review all comments and address unresolved review feedback.
|
pelikhan
commented
Jun 24, 2026
@copilot use core.info for logging |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Updated the new assignee-validation diagnostics to use |
pelikhan
commented
Jun 24, 2026
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
A regression switched agent assignee validation to the repo-scoped assignee check, which does not reliably surface coding-agent bots (including
copilot-swe-agent). This change restores effective resolution by validating aliases against the issue-scoped assignee endpoint when issue/PR context is available, with safe fallback behavior.What changed
assign_agent_helpers.cjsthat:GET /repos/{owner}/{repo}/issues/{issue_number}/assignees/{assignee}when an issue/PR number is knownrest.issues.checkUserCanBeAssigned(...)when issue-scoped validation is unavailable or non-applicablefindAgent(...)andgetAvailableAgentLogins(...)to accept optional issue/PR context and route through the new validator.Call-site updates
assign_to_agent.cjsassign_copilot_to_created_issues.cjscreate_issue.cjscreate_pull_request.cjsBehavioral impact
pr-sous-chef: requested branch update for run https://github.com/github/gh-aw/actions/runs/28133822633