grant pull-requests: write to PR quality checks workflow#3719
Merged
thewatermethod merged 4 commits intoJun 23, 2026
Conversation
PR conversation comments via rest.issues.createComment require the
pull-requests permission scope, not issues — despite the API path being
/issues/{number}/comments. The token previously had pull-requests: read,
so both diff_size_check and review_count_check failed with
'Resource not accessible by integration' and the try/catch swallowed
the error (workflow reported success, but no advisory comment appeared).
The earlier #3688 fix added issues: write, which is the wrong scope for
PR comments and therefore had no effect.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a small core.info preamble (event, action, repo, PR#, base ref, cross-repo flag) before any API call, and expands the catch to surface e.status and e.request.url so future failures show *which* endpoint returned what HTTP status — without ever logging the token, secrets, request headers, or response bodies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
✅ Diff size: 20 lines — within the 500-line guideline. |
|
✅ Review count: 2 human approvals — tommaroh, AdamAdHocTeam. |
thewatermethod
changed the title
thewatermethod
requested review from
AdamAdHocTeam,
Andrew565,
Copilot,
hardwarehuman,
kryswisnaskas and
tommaroh
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the PR Quality Checks GitHub Actions workflow to reliably post (or update) advisory PR conversation comments for diff size and required review count, by correcting the workflow token permission scope and improving failure diagnostics.
Changes:
- Switch trigger back to
pull_requestand update jobif:conditions accordingly. - Grant
pull-requests: writepermission (required for posting PR conversation comments). - Add lightweight
core.infocontext logging and enrichcatchlogs with HTTP status + request URL.
Impact assessment (benefit/risk):
- Benefit: High — restores intended automated PR feedback and adds actionable diagnostics for failures.
- Risk: Medium — workflow trigger/permission changes affect CI behavior and can regress review-comment updating if event gating is incorrect.
tommaroh
approved these changes
Jun 23, 2026
AdamAdHocTeam
approved these changes
Jun 23, 2026
thewatermethod
deleted the
mb/permissions-change-pr-quality-checks-workflow
branch
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 free
to 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.
Description of change
Fixes the
pr-quality-checksworkflow so bothdiff_size_checkandreview_count_checkactually post their advisory comments. They had been silently 403'ing withResource not accessible by integrationand the try/catch swallowed the failure, so the workflow reportedsuccesswhile no comment was ever posted.Root cause: PR conversation comments are governed by the
pull-requestspermission scope, notissues— even when created viaPOST /repos/{owner}/{repo}/issues/{number}/comments. The workflow hadpull-requests: read. (#3688 had upgradedissues: write, which is the wrong scope.)Changes:
pull-requests: write(load-bearing fix).core.infopreamble (event, action, repo, PR#, base ref, cross-repo flag) and an enrichedcatchthat logse.statusande.request.url. No tokens, secrets, headers, or response bodies are logged.on: pull_request_target→on: pull_request(reverting [TTAHUB-5460] Update quality_check workflows to use correct permissions #3708). With the real bug now understood as a permission scope issue, thepull_request_targettrade-off has no practical benefit for this repo's all-internal-branch contribution model, andpull_requestlets the workflow be exercised pre-merge on the PR branch.How to test
Already validated on this PR:
github-actions[bot]:<!-- pr-quality-diff-size -->— ✅ within threshold<!-- pr-quality-review-count -->—Jira Issue(s)
Checklists
Every PR
Before merge to main
After merge/deploy