ci(triage): label fork PRs and stop auto-closing PRs that only lack an issue link#107
Conversation
…abeled Fork PRs run the triage job with a read-only GITHUB_TOKEN under the pull_request event regardless of the workflow's declared write permissions, so addLabels/createComment fail with 403 'Resource not accessible by integration' for every external contributor (e.g. #103). Switch the trigger to pull_request_target, which runs in the base-repo context with the declared write token. The job never checks out or runs PR head code (it only reads context.payload and the pulls/files API and manages labels), so it carries none of the untrusted-code-execution risk that makes pull_request_target dangerous.
needs-issue flags PRs that link no issue, but linking one is a SHOULD in CONTRIBUTING (required only for protocol-level changes), and many legitimate small PRs (docs, obvious fixes) have none. Coupling it to the 14/21-day auto-close was overreach: a substantive, well-described PR could be closed purely for missing an issue reference. Keep needs-issue as an advisory label and guidance nudge; leave only needs-description (an empty/thin body, a far stronger low-effort signal) as the auto-close trigger.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR updates triage and stale automation to focus on missing descriptions, and rewords contributor guidance to match the same closure rule. ChangesWorkflow configuration updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/stale.yml (1)
29-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the stale message to match the narrowed close condition.
Line 29 now targets only
needs-description, but Lines 34-35 still mention a missing linked issue, which can mislead contributors about why the PR may close.Proposed wording fix
stale-pr-message: > - This PR has been inactive for 14 days and is still missing a linked issue - or a description. It will be closed in 7 days if there's no update. Push a + This PR has been inactive for 14 days and is still missing a description. + It will be closed in 7 days if there's no update. Push a change or leave a comment to keep it open — no hard feelings, you can reopen anytime.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/stale.yml around lines 29 - 35, The stale PR message is now inconsistent with the narrowed `only-pr-labels: "needs-description"` condition in the stale workflow. Update the `stale-pr-message` in the stale workflow configuration so it refers only to the missing description state and removes any mention of a missing linked issue, keeping the wording aligned with the `only-pr-labels` and `stale-pr-label` settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/stale.yml:
- Around line 29-35: The stale PR message is now inconsistent with the narrowed
`only-pr-labels: "needs-description"` condition in the stale workflow. Update
the `stale-pr-message` in the stale workflow configuration so it refers only to
the missing description state and removes any mention of a missing linked issue,
keeping the wording aligned with the `only-pr-labels` and `stale-pr-label`
settings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a1e1d2e7-bf01-49de-a6cd-130c57b19ce6
📒 Files selected for processing (2)
.github/workflows/pr-triage.yml.github/workflows/stale.yml
The auto-close trigger narrowed to needs-description, but the stale message still told contributors a PR could close for a missing linked issue. Drop that clause so the warning matches what actually closes a PR.
|
Fixed in 2f850bb. Dropped the "missing a linked issue" clause from the stale message so it matches the narrowed |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@kevincodex1 ready when you have a moment. Fixes the triage bot 403-ing on every fork PR (read-only token under |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Findings
[P2] Align the stale filter with the intended close policy
.github/workflows/stale.yml:29
The pinned actions/stale configuration treats only-pr-labels as an all-labels filter, so the previous needs-issue,needs-description value only processed PRs that had both labels, not PRs that only lacked an issue link. Changing this to only needs-description therefore changes the close criteria to every PR missing a description, including PRs that do link an issue, while CONTRIBUTING.md still tells contributors that stale closure applies to PRs with no linked issue or description. Please either update the docs/workflow comments to make the new missing-description-only close policy explicit, or use the action's one-of label filter if the intended policy is the documented OR behavior.
Auto-close is driven by needs-description only; a missing issue link stays advisory. Align the contributor docs with the workflow so the OR wording no longer implies a missing issue link can close a PR.
|
Good catch. |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
No Findings here
@kevincodex1 LGTM
3 participants
What
The PR triage bot has never labeled a single PR. Both of its target populations fall through:
OWNER/MEMBER/COLLABORATOR) hit the early-return bypass and skip triage entirely.on: pull_requestgives the job a read-onlyGITHUB_TOKENregardless of thepermissions:block, soaddLabels/createCommentfail with403 Resource not accessible by integration.That second case is the visible one: it's the red
Quality-signal triagecheck on every external contributor's PR (e.g. #103). Because the labels never get applied, the downstream stale→close pipeline (stale.ymlkeys offneeds-issue/needs-description) has also been inert for forks. The advertised flag-then-close flow in CONTRIBUTING.md simply never ran for the contributors it was written for.Changes
pr-triage.yml:pull_request→pull_request_target. This runs the job in the base-repo context with the declared write token, even for forks. It's safe here specifically because the job never checks out or executes PR head code: it only readscontext.payload.pull_request, calls the read-only files API, regex-scans patch strings inactions/github-script, and writes labels/comments. None of the untrusted-code-execution risk that makespull_request_targetdangerous applies. The heavy CI inpr-checks.ymlstays onpull_requestand keeps its first-time-contributor approval gate.stale.yml: dropneeds-issuefromonly-pr-labels. Linking an issue is a SHOULD in CONTRIBUTING (required only for protocol-level changes), and plenty of legitimate small PRs have none. Coupling it to the 14/21-day auto-close meant a substantive, well-described PR could be closed purely for missing an issue reference.needs-issuestays as an advisory label and guidance nudge;needs-description(a thin/empty body, a far stronger low-effort signal) remains the sole closer.Notes
pull_request_targetreads the workflow from the base branch, so the fix takes effect for fork PRs only once this merges tomain. It won't retroactively green an existing failed run.Summary by CodeRabbit
needs-descriptionlabel are marked stale/closed (missing issue link remains advisory).