Uh oh!
There was an error while loading. Please reload this page.
fix: grant pull-requests: write for issue_comment reactions (slash_command PR comment fix) - #28854
Merged
Merged
Conversation
…mand PR comments)
When a workflow uses `slash_command: events: [pull_request_comment]`, it compiles to
an `issue_comment` GitHub event. GitHub requires `pull-requests: write` to add
reactions to comments associated with pull requests, even though the API path is
`/issues/comments/{id}/reactions`.
The previous fix (#26720) only added `pull-requests: write` for `pull_request` and
`pull_request_review_comment` events, missing the `issue_comment` case.
This fix extends the condition to also include `hasIssueCommentEvent` when
`reactionIncludesPullRequests` is true, so that slash_command workflows with
`events: [pull_request_comment]` (and any workflow using issue_comment) correctly
grant `pull-requests: write` in the activation job.
Fixes #26727"
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/134eab02-7006-4c28-abf0-a5d0ac484eec
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>Agent-Logs-Url: https://github.com/github/gh-aw/sessions/134eab02-7006-4c28-abf0-a5d0ac484eec Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI
changed the title
[WIP] Fix missing pull-requests:write permission for slash_command reactionsfix: grant pull-requests: write for issue_comment reactions (slash_command PR comment fix)Apr 28, 2026
pelikhan
marked this pull request as ready for review
April 28, 2026 05:32
Uh oh!
There was an error while loading. Please reload this page.
Closed
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 83/100✅ Excellent — tests comprehensively cover behavioral contracts for the permission-scoping fix.
Test Classification DetailsView all 14 test classifications
Test Inflation NoteThe ratio of test lines added (61) to production lines added (4) is ≈ 15:1, which exceeds the 2:1 threshold — resulting in a 10-point deduction. However, this is entirely appropriate here: the PR fixes a narrow bug and the tests provide comprehensive regression coverage of all permission-scoping scenarios (9 E2E compile tests + 5 unit tests of the internal helper). The inflation is intentional and adds real safety. Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References:§25035755025
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adjusts activation-job permission derivation so workflows reacting to PR-associated comments (which arrive via the issue_comment GitHub event) receive pull-requests: write, avoiding 403s when adding reactions.
Changes:
- Update activation permission derivation to consider
issue_commentwhen deciding whether to grantpull-requests: writefor reactions. - Add unit + end-to-end tests covering
issue_commentandslash_command: events: [pull_request_comment]. - Update ADR-26535 to correct the documented/normative permission rule.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_activation_job.go | Extends pull-requests: write derivation to include issue_comment when PR reactions are enabled. |
| pkg/workflow/activation_permissions_scope_test.go | Adds regression tests for issue_comment and slash-command PR-comment compilation behavior. |
| docs/adr/26535-event-scoped-activation-permission-derivation.md | Updates the ADR narrative + normative spec to reflect the corrected permission requirements. |
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
| ### Decision | ||
| We will derive activation job permissions by parsing the `on:` section YAML at compile time, filtering out known metadata trigger fields, and granting only the write scopes required by the real GitHub event types that are configured. `issues: write` is granted only when `issues`, `issue_comment`, or `pull_request` events are present (since reactions and status comments on issues/PRs use the Issues REST API). `pull-requests: write` is granted only when `pull_request_review_comment` events are present. `discussions: write` is granted only when `discussion` or `discussion_comment` events are present. A fallback to the previous broad-grant behavior is preserved for synthetic or test `WorkflowData` instances where the `on:` section is empty. | ||
| We will derive activation job permissions by parsing the `on:` section YAML at compile time, filtering out known metadata trigger fields, and granting only the write scopes required by the real GitHub event types that are configured. `issues: write` is granted only when `issues`, `issue_comment`, or `pull_request` events are present (since reactions and status comments on issues/PRs use the Issues REST API). `pull-requests: write` is granted when `pull_request` or `pull_request_review_comment` events are present, or when `issue_comment` is present with PR reactions enabled (because `issue_comment` fires for PR comments and GitHub requires `pull-requests: write` to react to PR comments). `discussions: write` is granted only when `discussion` or `discussion_comment` events are present. A fallback to the previous broad-grant behavior is preserved for synthetic or test `WorkflowData` instances where the `on:` section is empty. |
There was a problem hiding this comment.
In the Decision paragraph, the wording implies pull-requests: write is granted whenever pull_request/pull_request_review_comment events are present. In the implementation, that permission is only added when reactions are enabled for pull requests (and now also for issue_comment in a PR-comment context). Consider tightening this text to explicitly include the reaction target gating so the ADR matches the current behavior.
Suggested change
| We will derive activation job permissions by parsing the `on:` section YAML at compile time, filtering out known metadata trigger fields, and granting only the write scopes required by the real GitHub event types that are configured. `issues: write` is granted only when `issues`, `issue_comment`, or `pull_request` events are present (since reactions and status comments on issues/PRs use the Issues REST API). `pull-requests: write` is granted when `pull_request` or `pull_request_review_comment` events are present, or when`issue_comment`is present with PR reactions enabled (because `issue_comment` fires for PR comments and GitHub requires `pull-requests: write` to react to PR comments). `discussions: write` is granted only when `discussion` or `discussion_comment` events are present. A fallback to the previous broad-grant behavior is preserved for synthetic or test `WorkflowData` instances where the `on:` section is empty. | |
| We will derive activation job permissions by parsing the `on:` section YAML at compile time, filtering out known metadata trigger fields, and granting only the write scopes required by the real GitHub event types that are configured. `issues: write` is granted only when `issues`, `issue_comment`, or `pull_request` events are present (since reactions and status comments on issues/PRs use the Issues REST API). `pull-requests: write` is granted only when reactions are enabled for pull-request targets: for `pull_request` or `pull_request_review_comment` events, or for`issue_comment`when it applies to a PR comment (because `issue_comment` fires for PR comments and GitHub requires `pull-requests: write` to react to PR comments). `discussions: write` is granted only when `discussion` or `discussion_comment` events are present. A fallback to the previous broad-grant behavior is preserved for synthetic or test `WorkflowData` instances where the `on:` section is empty. |
| 1. Implementations **MUST** derive activation job write permissions from the set of real GitHub event types present in the `on:` section, not from the presence of `reaction` or `status-comment` configuration alone. | ||
| 2. Implementations **MUST NOT** grant `pull-requests: write` in the activation job unless `pull_request_review_comment`is among the configured trigger events. | ||
| 2. Implementations **MUST NOT** grant `pull-requests: write` in the activation job unless `pull_request`, `pull_request_review_comment`, or `issue_comment` is among the configured trigger events and the reaction/status-comment configuration includes pull requests. (`issue_comment` events fire for both issue comments and PR comments; since PR comments require `pull-requests: write` for reactions, the presence of `issue_comment` with PR reactions enabled mandates this permission.) |
There was a problem hiding this comment.
Normative spec point (2) says the reaction/status-comment configuration includes pull requests, but the current code only ever grants pull-requests: write in the reaction branch (status comments only add issues: write). To avoid a spec/implementation mismatch, consider rewording this requirement to reference reaction configuration specifically (or update the implementation if status-comment is intended to require pull-requests: write).
Suggested change
| 2. Implementations **MUST NOT** grant `pull-requests: write` in the activation job unless `pull_request`, `pull_request_review_comment`, or `issue_comment` is among the configured trigger events and the reaction/status-comment configuration includes pull requests. (`issue_comment` events fire for both issue comments and PR comments; since PR comments require `pull-requests: write` for reactions, the presence of `issue_comment` with PR reactions enabled mandates this permission.) | |
| 2. Implementations **MUST NOT** grant `pull-requests: write` in the activation job unless `pull_request`, `pull_request_review_comment`, or `issue_comment` is among the configured trigger events and the reaction configuration includes pull requests. (`issue_comment` events fire for both issue comments and PR comments; since PR comments require `pull-requests: write` for reactions, the presence of `issue_comment` with PR reactions enabled mandates this permission.) |
Comment on lines
+169
to
+170
| // compiles to issue_comment), so pull-requests: write is also needed when issue_comment is present. | ||
| if reactionIncludesPullRequests && (hasPullRequestEvent || hasPullRequestReviewCommentEvent || hasIssueCommentEvent) { |
There was a problem hiding this comment.
The new hasIssueCommentEvent branch is gated only by reactionIncludesPullRequests, but the reaction step itself only runs on issue_comment when reactionIncludesIssues is true (see BuildReactionConditionForTargets/buildReactionLikeCondition). With reaction.issues: false and reaction.pull-requests: true, this will grant pull-requests: write even though the activation reaction step can never execute for issue_comment, weakening least-privilege. Consider additionally requiring reactionIncludesIssues for the hasIssueCommentEvent part of this condition (or otherwise aligning permission derivation to the actual reaction condition).
Suggested change
| // compiles to issue_comment), so pull-requests: write is also needed when issue_comment is present. | |
| ifreactionIncludesPullRequests&& (hasPullRequestEvent||hasPullRequestReviewCommentEvent||hasIssueCommentEvent) { | |
| // compiles to issue_comment), but the reaction step only runs for issue_comment when | |
| // issue reactions are enabled, so permission derivation must match that gating. | |
| ifreactionIncludesPullRequests&& (hasPullRequestEvent||hasPullRequestReviewCommentEvent|| (reactionIncludesIssues&&hasIssueCommentEvent)) { |
This was referenced Apr 28, 2026
rolfbjarne added a commit
to dotnet/macios
that referenced
this pull request
May 5, 2026
The activation job in the `macios-reviewer` workflow was missing `pull-requests: write` permission, causing the 'Add eyes reaction for immediate feedback' step to fail with a 403 when triggered by `/review` on a PR comment. This was a known gh-aw bug ([github/gh-aw#28767](github/gh-aw#28767)), fixed in v0.71.2 via [github/gh-aw#28854](github/gh-aw#28854). This PR recompiles the lock file with gh-aw v0.71.2, which adds `pull-requests: write` to the activation job. 🤖 Pull request created by Copilot --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
slash_command: events: [pull_request_comment]compiles toissue_comment(the actual GitHub event). The activation job permission derivation only checked forpull_requestandpull_request_review_commentwhen deciding to grantpull-requests: write— missingissue_comment. GitHub requirespull-requests: writeto react to PR-associated comments even though the API path is/issues/comments/{id}/reactions, causing 403 failures at the reaction step.Changes
pkg/workflow/compiler_activation_job.go: AddhasIssueCommentEventto thepull-requests: writecondition —issue_commentfires for both issue and PR comments, so PR reactions require this permission whenever that event is present:pkg/workflow/activation_permissions_scope_test.go: Two new tests — a unit test againstaddActivationInteractionPermissionsMapdirectly withissue_comment, and an end-to-end compilation test forslash_command: events: [pull_request_comment].docs/adr/26535-event-scoped-activation-permission-derivation.md: Correct the normative spec (point 2 was wrong —issue_commentalso mandatespull-requests: writewhen PR reactions are enabled).