Skip to content

fix: always send both GraphQL-Features flags when assigning Copilot to an issue#39719

Merged
pelikhan merged 2 commits into
mainfrom
copilot/review-assign-agent-mutation
Jun 17, 2026
Merged

fix: always send both GraphQL-Features flags when assigning Copilot to an issue#39719
pelikhan merged 2 commits into
mainfrom
copilot/review-assign-agent-mutation

Conversation

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

The GitHub Copilot docs require both issues_copilot_assignment_api_support and coding_agent_model_selection in the GraphQL-Features header for every assignment mutation. The second flag was only sent when an explicit model was provided — without it, the API accepts the call and returns success but never starts a Copilot session.

Changes

  • assign_agent_helpers.cjsgraphqlFeatures is now unconditionally "issues_copilot_assignment_api_support,coding_agent_model_selection" instead of conditional on model
  • assign_agent_helpers.cjs (fallback) — same fix for the addAssigneesToAssignable fallback path, which also only sent the first flag
  • assign_agent_helpers.test.cjs — updated assertion that was explicitly testing for the single-flag behavior when no model is provided
// Before
const graphqlFeatures = model
  ? "issues_copilot_assignment_api_support,coding_agent_model_selection"
  : "issues_copilot_assignment_api_support";

// After
const graphqlFeatures = "issues_copilot_assignment_api_support,coding_agent_model_selection";

Copilot AI and others added 2 commits June 17, 2026 03:07
…header

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…header per GitHub docs

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Fix: always include coding_agent_model_selection in GraphQL-Features header for agent assignment fix: always send both GraphQL-Features flags when assigning Copilot to an issue Jun 17, 2026
Copilot AI requested a review from pelikhan June 17, 2026 03:10
@pelikhan pelikhan marked this pull request as ready for review June 17, 2026 03:11
Copilot AI review requested due to automatic review settings June 17, 2026 03:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Ensures Copilot issue assignment mutations always include the full set of required GraphQL-Features flags so sessions reliably start even when no explicit model is provided.

Changes:

  • Always send issues_copilot_assignment_api_support,coding_agent_model_selection in GraphQL-Features for the primary assignment mutation.
  • Apply the same GraphQL-Features flags to the addAssigneesToAssignable fallback path.
  • Update unit test expectations to match the new always-send-both-flags behavior.
Show a summary per file
File Description
actions/setup/js/assign_agent_helpers.cjs Makes GraphQL-Features include both required flags for the primary assignment call and the fallback mutation path.
actions/setup/js/assign_agent_helpers.test.cjs Updates assertions to expect both feature flags regardless of whether model is provided.
.github/workflows/release.lock.yml Updates safe-outputs validation schema for update_release.body (adds minLength).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 3

Comment on lines 661 to +665
"required": true,
"type": "string",
"sanitize": true,
"maxLength": 65000
"maxLength": 65000,
"minLength": 20
Comment on lines 482 to 487
const fallbackResp = await githubClient.graphql(fallbackMutation, {
assignableId,
assigneeIds: [agentId],
headers: {
"GraphQL-Features": "issues_copilot_assignment_api_support",
"GraphQL-Features": "issues_copilot_assignment_api_support,coding_agent_model_selection",
},
expect(variables.headers["GraphQL-Features"]).toBe("issues_copilot_assignment_api_support");
expect(variables.headers["GraphQL-Features"]).toBe("issues_copilot_assignment_api_support,coding_agent_model_selection");
});

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #39719 does not have the 'implementation' label and has 0 new lines of code in business logic directories (≤100 threshold).

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

@github-actions github-actions Bot mentioned this pull request Jun 17, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skills-Based Review 🧠

Applied /diagnose and /zoom-out — leaving comments, no blocking issues.

📋 Key Themes & Highlights

Key Themes

  • Missing fallback regression test: The addAssigneesToAssignable fallback path is now fixed, but the fallback code branch has no test coverage at all. The PR correctly updated the primary-path test, but the same diligence wasn't applied to the fallback. Given that this was a silent failure (API returns 200, session never starts), a regression test here is especially valuable.
  • Unrelated lock file change: release.lock.yml gains a minLength: 20 constraint that isn't mentioned in the PR description — worth verifying it's intentional.

Positive Highlights

  • Root cause properly fixed: Both code paths updated, not just the one that appeared in the ternary. The fallback path's bug was actually more severe (always broken, regardless of model).
  • Tight, minimal diff: 3 production lines, 3 test lines — exactly the right scope for a correctness fix.
  • Test updated to reflect new invariant: The existing no-model test now correctly guards the both-flags requirement rather than encoding the broken behavior.
  • Clear comment on the constant: // Both feature flags are required per GitHub Copilot documentation gives future readers the why without requiring them to look up the docs.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

Comments that could not be inline-anchored

actions/setup/js/assign_agent_helpers.test.cjs:493

[/diagnose] The fallback path (addAssigneesToAssignable) was fixed to include both flags, but has zero test coverage — there's no test that drives the fallback branch at all, let alone one that guards the header value. Without a regression test, this fix could silently regress in the same way as the original bug.

<details>
<summary>💡 Suggested regression test for the fallback path</summary>

Add a new it block inside the describe(&quot;assignAgentToIssue&quot;) suite, just before the clos…

.github/workflows/release.lock.yml:663

[/zoom-out] This minLength: 20 addition appears unrelated to the GraphQL-Features fix described in the PR. Lock files are generated artifacts — if this change was intentional (e.g. pulled from an upstream source bundle), it should be called out in the PR description so reviewers can verify it wasn't accidentally included.

<details>
<summary>💡 What to check</summary>

  • If unintentional: rebase from main and re-run make recompile to see if the diff disappears.
  • If *intentional

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Request to address outstanding comments before merge

The core fix is correct: always emitting both issues_copilot_assignment_api_support,coding_agent_model_selection flags on both the primary and fallback paths addresses the root cause described in the PR.

However, three concerns from the existing review thread should be resolved first:

Blocking concerns
  1. Fallback fix is untested (line 486) — the addAssigneesToAssignable path was silently broken before this PR and the new behavior has no corresponding test. If it regresses nobody will know.

  2. Duplicate string constant (line 487) — const graphqlFeatures is declared inside the try block; JS const scoping makes it unreachable in the catch block, so the string must be duplicated. Hoisting the constant above the try/catch would eliminate the drift risk.

  3. Unrelated release.lock.yml change (line 665) — adding minLength: 20 to the release-body schema is out of scope for this PR. Confirm intentional or revert.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 85/100 — Excellent

Analyzed 2 modified test(s) in actions/setup/js/assign_agent_helpers.test.cjs: 2 design, 0 implementation, 0 guideline violation(s).

📊 Metrics & Test Classification (2 tests analyzed)
Metric Value
New/modified tests analyzed 2
✅ Design tests (behavioral contracts) 2 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (50%)
Duplicate test clusters 0
Test inflation detected No
🚨 Coding-guideline violations 0
Test File Classification Issues Detected
should successfully assign agent using mutation actions/setup/js/assign_agent_helpers.test.cjs:244 ✅ Design
should include model in agentAssignment when provided actions/setup/js/assign_agent_helpers.test.cjs:316 ✅ Design Comment-only change; assertion was already correct

Go: 0 (*_test.go); JavaScript: 2 (*.test.cjs, *.test.js). No other languages detected.

Scoring breakdown:

  • Behavioral coverage: 2/2 → 40/40 pts
  • Error/edge case coverage: 1/2 → 15/30 pts (Test 1 covers model=null scenario)
  • Duplication penalty: none → 20/20 pts
  • Test inflation: none → 10/10 pts

Notes:

  • Both tests assert on variables.headers["GraphQL-Features"] passed to the external github.graphql client — this is the correct behavioral verification for an API header contract. Mocking mockGithub.graphql is legitimate (external I/O).
  • Test 1 functional change: assertion widened from "issues_copilot_assignment_api_support""issues_copilot_assignment_api_support,coding_agent_model_selection", correctly capturing that both flags must always be sent regardless of whether a model is specified.
  • Test 2 change: comment-only clarification; the assertion was already checking for both flags.

Verdict

Check passed. 0% implementation tests (threshold: 30%). Both modified tests enforce behavioral contracts on outbound API headers, and the key assertion change in Test 1 directly prevents regression of the bug being fixed.

🧪 Test quality analysis by Test Quality Sentinel ·

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Test Quality Sentinel: 85/100. Test quality is excellent — 0% of new/modified tests are implementation tests (threshold: 30%). Both modified tests enforce behavioral contracts on outbound API headers.

@pelikhan pelikhan merged commit 8918ed5 into main Jun 17, 2026
68 checks passed
@pelikhan pelikhan deleted the copilot/review-assign-agent-mutation branch June 17, 2026 03:26
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.

3 participants