Refactor bot assignment logic into shared assign_agent_helpers.cjs module#4796
Refactor bot assignment logic into shared assign_agent_helpers.cjs module#4796
Conversation
…dule Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR successfully extracts duplicated bot assignment logic into a shared assign_agent_helpers.cjs module and updates both assign_to_agent.cjs and assign_issue.cjs to use it. The refactoring consolidates GraphQL-based agent assignment functions including agent lookup, issue details retrieval, and agent assignment mutations.
Key changes:
- Creates reusable helper module with agent detection, GraphQL queries, and assignment logic
- Updates
assign_to_agent.cjsto import helpers instead of duplicating ~300 lines - Enhances
assign_issue.cjsto auto-detect and properly handle agent assignees via GraphQL - Adds comprehensive test coverage for the new shared module
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/js/assign_agent_helpers.cjs |
New shared module with agent assignment functions (GraphQL queries, mutations, error handling) |
pkg/workflow/js/assign_to_agent.cjs |
Refactored to use shared helpers, removed 300+ lines of duplicated code |
pkg/workflow/js/assign_issue.cjs |
Enhanced with agent detection using getAgentName(), routes to GraphQL for agents, gh CLI for users |
pkg/workflow/js/assign_agent_helpers.test.cjs |
Comprehensive tests for all helper functions (456 lines) |
pkg/workflow/js/assign_issue.test.cjs |
Updated test descriptions, added note about agent tests in helpers |
pkg/workflow/js.go |
Registered assign_agent_helpers.cjs for bundler support |
.github/workflows/duplicate-code-detector.lock.yml |
Compiled workflow with unbundled require() statement (critical bug) |
.github/workflows/dev.lock.yml |
Compiled workflow with properly bundled agent helpers |
.github/workflows/ai-triage-campaign.lock.yml |
Compiled workflow with properly bundled agent helpers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| const query = ` | ||
| query { | ||
| repository(owner: "${owner}", name: "${repo}") { | ||
| suggestedActors(first: 100, capabilities: CAN_BE_ASSIGNED) { | ||
| nodes { ... on Bot { login __typename } } | ||
| } | ||
| } | ||
| } | ||
| `; |
There was a problem hiding this comment.
Security: Potential GraphQL injection vulnerability
The GraphQL queries directly interpolate user-controlled values (owner, repo, issueNumber) into query strings without proper escaping or validation. This could allow GraphQL injection attacks.
Vulnerable code patterns:
const query = `
query {
repository(owner: "${owner}", name: "${repo}") {
issue(number: ${issueNumber}) {Recommendation:
Use GraphQL variables instead of string interpolation:
const query = `
query($owner: String!, $repo: String!, $issueNumber: Int!) {
repository(owner: $owner, name: $repo) {
issue(number: $issueNumber) {
...
}
}
}
`;
const variables = { owner, repo, issueNumber };
const response = await github.graphql(query, variables);This applies to all GraphQL queries in this file: getAvailableAgentLogins(), findAgent(), and getIssueDetails().
There was a problem hiding this comment.
Fixed in 8d7fd8a. All GraphQL queries (getAvailableAgentLogins, findAgent, getIssueDetails) now use parameterized variables instead of string interpolation.
| const mutation = ` | ||
| mutation { | ||
| replaceActorsForAssignable(input: { | ||
| assignableId: "${issueId}", | ||
| actorIds: ${JSON.stringify(actorIds)} | ||
| }) { | ||
| __typename | ||
| } | ||
| } | ||
| `; |
There was a problem hiding this comment.
Security: Potential GraphQL injection vulnerability
The mutation directly interpolates issueId and actorIds into the mutation string without proper escaping or parameterization.
Recommendation:
Use GraphQL variables for the mutation:
const mutation = `
mutation($assignableId: ID!, $actorIds: [ID!]!) {
replaceActorsForAssignable(input: {
assignableId: $assignableId,
actorIds: $actorIds
}) {
__typename
}
}
`;
const response = await fetch("https://api.github.com/graphql", {
method: "POST",
headers: {
Authorization: `Bearer ${ghToken}`,
"Content-Type": "application/json",
},
body: JSON.stringify({
query: mutation,
variables: {
assignableId: issueId,
actorIds: actorIds
}
}),
}).then(res => res.json());The same issue exists in the fallback mutation at line 282.
There was a problem hiding this comment.
Fixed in 8d7fd8a. Both the primary mutation (replaceActorsForAssignable) and the fallback mutation (addAssigneesToAssignable) now use GraphQL variables.
| ISSUE_NUMBER: ${{ steps.create_issue.outputs.issue_number }} | ||
| with: | ||
| script: | | ||
| const { getAgentName, getIssueDetails, findAgent, assignAgentToIssue } = require("./assign_agent_helpers.cjs"); |
There was a problem hiding this comment.
Critical Bug: Missing bundled helpers in lock file
This lock file shows unbundled require() statements that will fail at runtime:
const { getAgentName, getIssueDetails, findAgent, assignAgentToIssue } = require("./assign_agent_helpers.cjs");The assign_agent_helpers.cjs file won't exist in the GitHub Actions environment. The bundler should have inlined these functions (as it did in dev.lock.yml lines 3518-3821).
Root cause: The assignIssueScript variable in pkg/workflow/js.go is not being bundled before it's compiled into workflows. It's used directly in copilot_participant_steps.go via FormatJavaScriptForYAML(assignIssueScript) which only formats but doesn't bundle.
Impact: Any workflow that uses this script to assign issues will fail at runtime when it tries to require("./assign_agent_helpers.cjs").
Fix needed: The assignIssueScript needs to be bundled (with BundleJavaScriptFromSources) before being inserted into workflows, or registered in the script registry for automatic bundling.
| const { getAgentName, getIssueDetails, findAgent, assignAgentToIssue } = require("./assign_agent_helpers.cjs"); | |
| // Inlined assign_agent_helpers.cjs | |
| /** | |
| * Returns the agent name if the assignee is a known agent, otherwise null. | |
| * @param {string} assignee | |
| * @returns {string|null} | |
| */ | |
| function getAgentName(assignee) { | |
| if (!assignee) return null; | |
| const name = assignee.replace(/^@/, "").toLowerCase(); | |
| if (name === "copilot" || name === "github-copilot") return "copilot"; | |
| if (name === "claude" || name === "github-claude") return "claude"; | |
| return null; | |
| } | |
| /** | |
| * Finds the agent's user ID in the repository using the GitHub GraphQL API. | |
| * @param {string} owner | |
| * @param {string} repo | |
| * @param {string} agentName | |
| * @returns {Promise<string|null>} | |
| */ | |
| async function findAgent(owner, repo, agentName) { | |
| const query = ` | |
| query($owner: String!, $repo: String!, $login: String!) { | |
| repository(owner: $owner, name: $repo) { | |
| assignableUsers(query: $login, first: 1) { | |
| nodes { | |
| id | |
| login | |
| } | |
| } | |
| } | |
| } | |
| `; | |
| const variables = { owner, repo, login: agentName }; | |
| const result = await github.graphql(query, variables); | |
| const nodes = result?.repository?.assignableUsers?.nodes || []; | |
| if (nodes.length > 0) { | |
| return nodes[0].id; | |
| } | |
| return null; | |
| } | |
| /** | |
| * Gets issue details including current assignees and issue node ID. | |
| * @param {string} owner | |
| * @param {string} repo | |
| * @param {number} issueNumber | |
| * @returns {Promise<{issueId: string, currentAssignees: string[]}|null>} | |
| */ | |
| async function getIssueDetails(owner, repo, issueNumber) { | |
| const query = ` | |
| query($owner: String!, $repo: String!, $number: Int!) { | |
| repository(owner: $owner, name: $repo) { | |
| issue(number: $number) { | |
| id | |
| assignees(first: 10) { | |
| nodes { | |
| id | |
| login | |
| } | |
| } | |
| } | |
| } | |
| } | |
| `; | |
| const variables = { owner, repo, number: issueNumber }; | |
| const result = await github.graphql(query, variables); | |
| const issue = result?.repository?.issue; | |
| if (!issue) return null; | |
| const assignees = issue.assignees?.nodes?.map(a => a.id) || []; | |
| return { issueId: issue.id, currentAssignees: assignees }; | |
| } | |
| /** | |
| * Assigns the agent to the issue using the GitHub GraphQL API. | |
| * @param {string} issueId | |
| * @param {string} agentId | |
| * @param {string[]} currentAssignees | |
| * @param {string} agentName | |
| * @param {string} ghToken | |
| * @returns {Promise<boolean>} | |
| */ | |
| async function assignAgentToIssue(issueId, agentId, currentAssignees, agentName, ghToken) { | |
| const mutation = ` | |
| mutation($issueId: ID!, $assigneeIds: [ID!]!) { | |
| assignable: assignAssigneesToAssignable(input: { assignableId: $issueId, assigneeIds: $assigneeIds }) { | |
| assignable { | |
| id | |
| } | |
| } | |
| } | |
| `; | |
| // Add agentId to currentAssignees if not present | |
| const assigneeIds = currentAssignees.includes(agentId) | |
| ? currentAssignees | |
| : [...currentAssignees, agentId]; | |
| try { | |
| await github.graphql(mutation, { issueId, assigneeIds }); | |
| return true; | |
| } catch (err) { | |
| core.error(`GraphQL assignment failed: ${err instanceof Error ? err.message : String(err)}`); | |
| return false; | |
| } | |
| } | |
| // Main script logic |
There was a problem hiding this comment.
Fixed in 8d7fd8a. The assign_issue script is now registered with DefaultScriptRegistry and uses getAssignIssueScript() getter for proper bundling. The helper functions are now correctly inlined in the lock files.
- Use GraphQL variables instead of string interpolation for all queries/mutations - Register assign_issue script with DefaultScriptRegistry for proper bundling - Update tests to check GraphQL variables instead of query strings Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Applied all review comments in 8d7fd8a:
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:
|
|
✅ Agentic Changeset Generator completed successfully. |
The create-issue safe output feature duplicated bot assignment logic that already existed in assign-to-agent. This PR extracts the shared GraphQL-based agent assignment code into a reusable module.
Changes
assign_agent_helpers.cjs: Extracted agent lookup/assignment functions includingfindAgent(),getIssueDetails(),assignAgentToIssue(), andgetAgentName()assign_to_agent.cjs: Now requires the shared module instead of duplicating logicassign_issue.cjs: Detects when assignee is a known agent (@copilot,copilot) and uses GraphQL API via shared module instead of gh CLIjs.goregistration: Shared module registered for bundler support withDefaultScriptRegistryto ensure proper inlining in lock filesassign_agent_helpers.test.cjs: Comprehensive tests for shared moduleUsage
assign_issue.cjsnow auto-detects agent assignees:Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.