Fix activation permissions for pull request reactions#26720
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3ed08d66-abc5-46fd-8cb2-41a31f827760 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3ed08d66-abc5-46fd-8cb2-41a31f827760 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates activation permission scoping so workflows that add reactions on pull_request events request the required pull-requests: write scope (in addition to existing reaction-related permissions), and aligns compiled smoke workflow output accordingly.
Changes:
- Update activation permission calculation to add
pull-requests: writefor reaction steps onpull_requestevents. - Add a unit test validating
pull_requestreaction permission requirements in activation. - Recompile the smoke workflow lockfile to include
pull-requests: writeon the activation job.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_activation_job.go | Adjusts activation permissions map to include pull-requests: write when reactions are enabled for pull_request-related events. |
| pkg/workflow/activation_permissions_scope_test.go | Adds coverage asserting activation job permissions include both issues: write and pull-requests: write for pull_request reaction workflows. |
| .github/workflows/smoke-copilot.lock.yml | Updates compiled workflow to include pull-requests: write in activation job permissions. |
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: 1
| } | ||
| // Reactions on PR review comments use pull request review comment endpoints. | ||
| if reactionIncludesPullRequests && hasPullRequestReviewCommentEvent { | ||
| // Reactions on pull requests and PR review comments require pull-requests:write. |
There was a problem hiding this comment.
The new comment/logic adds pull-requests: write for pull_request reactions, but it’s not obvious why that scope is required given PR reactions are posted via the /issues/{number}/reactions endpoint. Consider adding a brief rationale (e.g., GitHub permission model requires pull-requests: write for PR reaction writes despite the issues endpoint) to avoid future refactors removing one of the scopes.
| // Reactions on pull requests and PR review comments require pull-requests:write. | |
| // GitHub's permission model still requires pull-requests:write when the reaction target is a | |
| // pull request or PR review comment, even though the reaction is created via issues endpoints. |
🧪 Test Quality Sentinel ReportTest Quality Score: 90/100✅ Excellent test quality
Test Classification Details
Flagged Tests — Requires ReviewNo tests are flagged for quality issues. The test is well-structured and covers the behavioral contract directly. Test Inflation NoteThe test added 31 lines while the production fix touched ~3 logic lines (changing Test Quality Analysis
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: §24532627925
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Test Quality Sentinel: 90/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). The single new test TestActivationPermissionsPullRequestReactionRequiresPullRequestsWrite is a well-designed end-to-end behavioral test that directly validates the bug fix, uses no mock libraries, includes negative assertions to verify permission exclusions, and has proper build tags and assertion messages.
…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>
Summary
pull-requests: writefor reaction steps onpull_requestevents.github/workflows/smoke-copilot.lock.ymlso the workflow now includespull-requests: writein activation permissionsValidation
go test -v -run "TestActivationPermissions(PRReviewReactionOnly|PullRequestReactionRequiresPullRequestsWrite|IssueOnlyReactionAndStatusComment|ReactionPullRequestsDisabled)" ./pkg/workflow/make fmt,make test-unit,make agent-finish)TestWasmGolden_CompileFixtures) unrelated to this change.