fix: prevent assign_to_agent from posting error comments on PRs#41475
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- In getIssueDetails (assign_agent_helpers.cjs): detect when the GitHub issues API returns a PR (pull_request field present) and throw a typed error with isPullRequest=true - In assign_to_agent.cjs: handle isPullRequest errors by skipping silently without posting any comment on the PR - In issue-monster.md: add explicit guideline never to call assign_to_agent with a PR number - Add test covering the new behavior Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Use Object.assign to attach the isPullRequest flag to the thrown Error, avoiding the TypeScript property-does-not-exist error. Rename prError to pullRequestError per code review feedback. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR prevents assign_to_agent from posting “Assignment failed” comments on pull requests when an issue_number actually resolves to a PR via issues.get, by detecting PR-shaped issue responses early and treating them as a skip rather than a failure/comment action.
Changes:
- Reject PRs in
getIssueDetailsby throwing a typed{ isPullRequest: true }error whenissues.getreturns an object withpull_request. - In
assign_to_agent, detectisPullRequesterrors and skip with a warning (no PR comment, no workflow failure). - Add workflow guidance (Issue Monster) and a unit test ensuring PR targets do not get comments and do not call
setFailed.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/assign_agent_helpers.cjs | Detects PR-shaped “issues” from issues.get and throws a typed error to prevent misclassification. |
| actions/setup/js/assign_to_agent.cjs | Skips PR targets surfaced via issue_number without posting error comments on PRs. |
| actions/setup/js/assign_to_agent.test.cjs | Adds coverage ensuring PR-shaped issues.get responses do not trigger PR comments or failures. |
| .github/workflows/issue-monster.md | Documents that assign_to_agent should only be used for issues and how to detect PRs. |
| .github/workflows/issue-monster.lock.yml | Regenerated lockfile reflecting the workflow markdown update. |
| .github/workflows/smoke-copilot.lock.yml | Lockfile regenerated; also changes compiled strictness (needs confirmation). |
| .github/workflows/smoke-copilot-aoai-entra.lock.yml | Lockfile regenerated; also changes compiled strictness (needs confirmation). |
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
|
|
||
| // When the agent specified an issue_number that turns out to be a PR, skip | ||
| // silently without posting a comment — error comments on PRs are confusing. | ||
| if (/** @type {any} */ error.isPullRequest) { |
| } catch (error) { | ||
| const errorMessage = getErrorMessage(error); | ||
| core.error(`Failed to get issue details: ${errorMessage}`); | ||
| if (!(/** @type {any} */ error.isPullRequest)) { |
| // GitHub's issues API returns pull requests too; reject them here so callers | ||
| // never accidentally treat a PR as an assignable issue. | ||
| if (issue.pull_request) { | ||
| throw Object.assign(new Error(`#${issueNumber} is a pull request, not an issue — assign_to_agent only supports issues`), { isPullRequest: true }); |
| GH_AW_INFO_FIREWALL_TYPE: "squid" | ||
| GH_AW_INFO_FRONTMATTER_EMOJI: "🧪" | ||
| GH_AW_COMPILED_STRICT: "true" | ||
| GH_AW_COMPILED_STRICT: "false" |
| GH_AW_INFO_FIREWALL_TYPE: "squid" | ||
| GH_AW_INFO_FRONTMATTER_EMOJI: "🧪" | ||
| GH_AW_COMPILED_STRICT: "true" | ||
| GH_AW_COMPILED_STRICT: "false" |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Caution agentic threat detected DetailsPotential security threats were detected in the agent output. Review the workflow run logs for details. Great work on this fix, The PR is well-structured:
This looks ready for review. ✅
|
|
@copilot run pr-finisher skill |
- Use /** @type {any} */ (error) with parentheses so ts-check casts the variable before property access, not just the expression result - Update error message to direct callers to use pull_number instead of issue_number when targeting a pull request Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Three issues addressed in commit 8fba8de:
Local validation: |
assign_to_agentwas posting⚠️ Assignment failedcomments on pull requests. GitHub'sissues.getAPI returns PR data without error when given a PR number, so the code would attempt (and fail) the assignment and then post the error comment on the PR itself.Changes
assign_agent_helpers.cjs—getIssueDetailsnow checksissue.pull_requestin the response and throws a typed{ isPullRequest: true }error early:assign_to_agent.cjs— Catch block detectsisPullRequestand skips silently with a warning; no comment is posted on the PR.issue-monster.md— Step 4 and Important Guidelines now explicitly prohibit passing PR numbers toassign_to_agent, with a note on how to detect a PR viaissue_readif uncertain.assign_to_agent.test.cjs— New test: whenissues.getreturns a PR, no comment is created andsetFailedis not called.