ci: auto-close incomplete external PRs#4579
Conversation
Add a pull_request_target workflow that checks PRs from external contributors (org members and bots are exempt via author_association): the PR template must be filled in, and any PR touching functional code (SDK, API, or frontend) must include a demo recording. Non-compliant PRs get a sticky comment, an incomplete-pr label, and are closed, then auto-reopened once fixed. Adds workflow_dispatch dry_run/force_external inputs for testing and registers the new label in .labels.yml.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces a new GitHub Actions workflow that enforces PR contribution requirements for external contributors. It adds an ChangesPR Contribution Requirement Enforcement
Sequence DiagramsequenceDiagram
participant GithubEvent as GitHub Event
participant Workflow as PR Enforcement Workflow
participant PRResolver as PR Resolver
participant TemplateValidator as Template Validator
participant FileAnalyzer as File Analyzer
participant GithubAPI as GitHub API
GithubEvent->>Workflow: pull_request_target or workflow_dispatch
Workflow->>PRResolver: Resolve target PR
PRResolver->>Workflow: PR object (or skip if draft/internal)
Workflow->>TemplateValidator: Parse body for sections
TemplateValidator->>Workflow: Missing sections list
Workflow->>FileAnalyzer: List changed files
FileAnalyzer->>Workflow: Functional changes detected
alt Requirements Pass
Workflow->>GithubAPI: Remove incomplete-pr label
Workflow->>GithubAPI: Reopen if closed
Workflow->>GithubAPI: Upsert success comment
else Requirements Fail
Workflow->>GithubAPI: Add incomplete-pr label
Workflow->>GithubAPI: Upsert failure comment with reasons
Workflow->>GithubAPI: Close PR
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d52cebdd-3b3c-4a66-8454-11db7e933c27
📒 Files selected for processing (2)
.github/.labels.yml.github/workflows/13-check-pr-contribution.yml
| /https?:\/\/[^\s)]*\/user-attachments\//i, // GitHub uploads | ||
| /https?:\/\/[^\s)]*githubusercontent\.com\//i, |
There was a problem hiding this comment.
Tighten the generic githubusercontent media match.
Line 77 accepts any *.githubusercontent.com URL, so a raw link to a text or source file can satisfy the demo gate without an actual screenshot or video. That makes the policy easy to bypass.
Suggested fix
const MEDIA = [
/!\[[^\]]*\]\([^)]+\)/, // markdown image
/<img\s/i,
/<video\s/i,
/https?:\/\/[^\s)]+\.(mp4|mov|webm|gif)/i, // direct video/gif
/https?:\/\/(www\.)?(youtube\.com|youtu\.be)\//i,
/https?:\/\/(www\.)?loom\.com\//i,
/https?:\/\/[^\s)]*\/user-attachments\//i, // GitHub uploads
- /https?:\/\/[^\s)]*githubusercontent\.com\//i,
];| async function upsertComment(text) { | ||
| const comments = await github.paginate(github.rest.issues.listComments, { | ||
| owner, repo, issue_number: number, per_page: 100, | ||
| }); | ||
| const existing = comments.find((c) => c.body && c.body.includes(MARKER)); | ||
| if (existing) { | ||
| await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: text }); | ||
| } else { | ||
| await github.rest.issues.createComment({ owner, repo, issue_number: number, body: text }); |
There was a problem hiding this comment.
Only upsert the bot's own sticky comment.
Line 146 picks the first comment containing MARKER without checking the author. Because the marker is public, a contributor can quote or copy it and this helper will target that human comment on the next run. Best case the update fails and enforcement stops; worst case it overwrites user discussion.
Suggested fix
async function upsertComment(text) {
const comments = await github.paginate(github.rest.issues.listComments, {
owner, repo, issue_number: number, per_page: 100,
});
- const existing = comments.find((c) => c.body && c.body.includes(MARKER));
+ const existing = comments.find(
+ (c) =>
+ c.user?.login === 'github-actions[bot]' &&
+ c.body?.startsWith(MARKER)
+ );
if (existing) {
await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: text });
} else {
await github.rest.issues.createComment({ owner, repo, issue_number: number, body: text });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function upsertComment(text) { | |
| const comments = await github.paginate(github.rest.issues.listComments, { | |
| owner, repo, issue_number: number, per_page: 100, | |
| }); | |
| const existing = comments.find((c) => c.body && c.body.includes(MARKER)); | |
| if (existing) { | |
| await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: text }); | |
| } else { | |
| await github.rest.issues.createComment({ owner, repo, issue_number: number, body: text }); | |
| async function upsertComment(text) { | |
| const comments = await github.paginate(github.rest.issues.listComments, { | |
| owner, repo, issue_number: number, per_page: 100, | |
| }); | |
| const existing = comments.find( | |
| (c) => | |
| c.user?.login === 'github-actions[bot]' && | |
| c.body?.startsWith(MARKER) | |
| ); | |
| if (existing) { | |
| await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: text }); | |
| } else { | |
| await github.rest.issues.createComment({ owner, repo, issue_number: number, body: text }); | |
| } |
| await upsertComment(text); | ||
| try { | ||
| await github.rest.issues.addLabels({ owner, repo, issue_number: number, labels: [LABEL] }); | ||
| } catch (e) { | ||
| core.warning(`Could not add label ${LABEL}: ${e.message}`); | ||
| } | ||
| if (pr.state === 'open') { | ||
| await github.rest.pulls.update({ owner, repo, pull_number: number, state: 'closed' }); | ||
| } | ||
| core.notice(`Closed PR #${number}:\n${list}`); | ||
| } else { | ||
| if (dryRun) { | ||
| core.info(`[dry-run] PR #${number} meets contribution requirements.`); | ||
| return; | ||
| } | ||
| // Compliant. If the bot had previously closed it, reopen and clear the flag. | ||
| const labels = (pr.labels || []).map((l) => l.name); | ||
| if (labels.includes(LABEL)) { | ||
| try { | ||
| await github.rest.issues.removeLabel({ owner, repo, issue_number: number, name: LABEL }); | ||
| } catch (e) { | ||
| core.warning(`Could not remove label ${LABEL}: ${e.message}`); | ||
| } | ||
| if (pr.state === 'closed') { | ||
| await github.rest.pulls.update({ owner, repo, pull_number: number, state: 'open' }); | ||
| } | ||
| await upsertComment(`${MARKER}\n✅ Thanks @${pr.user.login}! This PR now meets the contribution requirements and has been reopened. A maintainer will review it soon.`); |
There was a problem hiding this comment.
Don't close the PR unless the reopen signal was recorded.
This path swallows addLabels failures but still closes the PR, while the compliant path only reopens when incomplete-pr is present. Because label creation is a separate rollout step here, or if addLabels fails transiently, the bot can strand a fixed PR in the closed state with no automatic recovery.
Suggested fix
await upsertComment(text);
+ let labeled = false;
try {
await github.rest.issues.addLabels({ owner, repo, issue_number: number, labels: [LABEL] });
+ labeled = true;
} catch (e) {
core.warning(`Could not add label ${LABEL}: ${e.message}`);
}
+ if (!labeled) {
+ core.setFailed(`Refusing to close PR #${number} because label ${LABEL} could not be applied.`);
+ return;
+ }
if (pr.state === 'open') {
await github.rest.pulls.update({ owner, repo, pull_number: number, state: 'closed' });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await upsertComment(text); | |
| try { | |
| await github.rest.issues.addLabels({ owner, repo, issue_number: number, labels: [LABEL] }); | |
| } catch (e) { | |
| core.warning(`Could not add label ${LABEL}: ${e.message}`); | |
| } | |
| if (pr.state === 'open') { | |
| await github.rest.pulls.update({ owner, repo, pull_number: number, state: 'closed' }); | |
| } | |
| core.notice(`Closed PR #${number}:\n${list}`); | |
| } else { | |
| if (dryRun) { | |
| core.info(`[dry-run] PR #${number} meets contribution requirements.`); | |
| return; | |
| } | |
| // Compliant. If the bot had previously closed it, reopen and clear the flag. | |
| const labels = (pr.labels || []).map((l) => l.name); | |
| if (labels.includes(LABEL)) { | |
| try { | |
| await github.rest.issues.removeLabel({ owner, repo, issue_number: number, name: LABEL }); | |
| } catch (e) { | |
| core.warning(`Could not remove label ${LABEL}: ${e.message}`); | |
| } | |
| if (pr.state === 'closed') { | |
| await github.rest.pulls.update({ owner, repo, pull_number: number, state: 'open' }); | |
| } | |
| await upsertComment(`${MARKER}\n✅ Thanks @${pr.user.login}! This PR now meets the contribution requirements and has been reopened. A maintainer will review it soon.`); | |
| await upsertComment(text); | |
| let labeled = false; | |
| try { | |
| await github.rest.issues.addLabels({ owner, repo, issue_number: number, labels: [LABEL] }); | |
| labeled = true; | |
| } catch (e) { | |
| core.warning(`Could not add label ${LABEL}: ${e.message}`); | |
| } | |
| if (!labeled) { | |
| core.setFailed(`Refusing to close PR #${number} because label ${LABEL} could not be applied.`); | |
| return; | |
| } | |
| if (pr.state === 'open') { | |
| await github.rest.pulls.update({ owner, repo, pull_number: number, state: 'closed' }); | |
| } | |
| core.notice(`Closed PR #${number}:\n${list}`); | |
| } else { | |
| if (dryRun) { | |
| core.info(`[dry-run] PR #${number} meets contribution requirements.`); | |
| return; | |
| } | |
| // Compliant. If the bot had previously closed it, reopen and clear the flag. | |
| const labels = (pr.labels || []).map((l) => l.name); | |
| if (labels.includes(LABEL)) { | |
| try { | |
| await github.rest.issues.removeLabel({ owner, repo, issue_number: number, name: LABEL }); | |
| } catch (e) { | |
| core.warning(`Could not remove label ${LABEL}: ${e.message}`); | |
| } | |
| if (pr.state === 'closed') { | |
| await github.rest.pulls.update({ owner, repo, pull_number: number, state: 'open' }); | |
| } | |
| await upsertComment(`${MARKER}\n✅ Thanks @${pr.user.login}! This PR now meets the contribution requirements and has been reopened. A maintainer will review it soon.`); |
|
💯 |
Summary
External contributors increasingly open low-effort PRs that skip the template and ship no demo, which costs maintainer review time. This adds a bot that enforces our contribution requirements automatically.
A new
pull_request_targetworkflow evaluates every PR from an external author (org members and bots are exempt viaauthor_association, so no hardcoded handles). It fails a PR when:Files outside functional code (tests, docs,
.github, lockfiles, images) are exempt, so a test-only or docs-only PR may mark Demo as N/A.On failure the bot posts one sticky comment listing exactly what is missing, applies an
incomplete-prlabel, and closes the PR. If the contributor later fixes the description or adds a demo, the bot removes the label and reopens the PR automatically, so a genuine contributor never has to open a new one.pull_request_targetis used so the workflow has a write token even for fork PRs. It only reads PR metadata and posts a comment; it never checks out or runs the PR's code, so it is safe.Testing
Verified locally
yaml.safe_load): the workflow and.labels.yml.pull_request_targetandworkflow_dispatchboth read the workflow from the base branch, so live testing only works after this merges tomain.Added or updated tests
N/A. This is a CI workflow with no unit-test harness. It ships
workflow_dispatchinputs (dry_run,force_external) so it can be exercised manually against a throwaway PR after merge.QA follow-up
After merge: run the label sync once so
incomplete-prexists, then dispatch13 - check PR contributionwithdry_run=trueandforce_external=trueagainst a crafted test PR to confirm the decision, before relying on it for real PRs.Demo
N/A. This change only touches
.github/**(CI configuration), which the workflow itself classifies as a non-functional change.Checklist