From ab19cd8c4a618383a2366b24c7508fae73b9b442 Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Mon, 4 May 2026 13:51:02 +0200 Subject: [PATCH 1/7] feat(validate-pr): Rewrite as advisory action (v4) - Drop close-on-failure and label application; post a single warm advisory comment instead. - Add small-PR bypass: skip validation when a PR has fewer than 100 lines changed, excluding common lock files (Cargo.lock, yarn.lock, package-lock.json, etc.). - Remove maintainer-reopen-skip (no longer needed without close logic). - Add idempotence guard via apps.getAuthenticated() to avoid duplicate comments on workflow re-runs. Co-Authored-By: Claude Opus 4.7 (1M context) --- validate-pr/scripts/validate-pr.js | 247 +++++++++++++---------------- 1 file changed, 114 insertions(+), 133 deletions(-) diff --git a/validate-pr/scripts/validate-pr.js b/validate-pr/scripts/validate-pr.js index 113e55b..cfd7879 100644 --- a/validate-pr/scripts/validate-pr.js +++ b/validate-pr/scripts/validate-pr.js @@ -1,10 +1,9 @@ // @ts-check /** - * Validates non-maintainer PRs by checking that they reference a GitHub issue - * with prior discussion between the author and a maintainer. - * - * Closes PRs that don't meet contribution guidelines. + * Advisory validation for community PRs. Posts a single friendly comment + * when the PR doesn't reference an issue with maintainer discussion. + * Never closes PRs, never applies labels. * * @param {object} params * @param {import('@actions/github').getOctokit} params.github @@ -30,11 +29,10 @@ module.exports = async ({ github, context, core }) => { ]; if (ALLOWED_BOTS.includes(prAuthor)) { core.info(`PR author ${prAuthor} is an allowed bot. Skipping.`); - core.setOutput('skipped', 'true'); return; } - // --- Helpers: check user permission on a repo (cached) --- + // --- Helpers: cached collaborator-role lookup --- const roleCache = new Map(); async function getRole(owner, repoName, username) { const key = `${owner}/${repoName}:${username}`; @@ -56,7 +54,6 @@ module.exports = async ({ github, context, core }) => { async function hasWriteAccess(owner, repoName, username) { const role = await getRole(owner, repoName, username); - // role_name values: admin, maintain, push, triage, pull (+ custom roles) return ['admin', 'maintain', 'push', 'write'].includes(role); } @@ -65,36 +62,68 @@ module.exports = async ({ github, context, core }) => { return ['admin', 'maintain'].includes(role); } - // --- Step 1: Skip if a maintainer reopened the PR --- - if (context.payload.action === 'reopened') { - const sender = context.payload.sender.login; - const senderIsMaintainer = await isMaintainer(repo.owner, repo.repo, sender); - if (senderIsMaintainer) { - core.info(`PR reopened by maintainer ${sender}. Skipping all checks.`); - core.setOutput('skipped', 'true'); - return; - } - } - - // --- Step 2: Check if PR author has write access (admin, maintain, or write role) --- - const authorHasWriteAccess = await hasWriteAccess(repo.owner, repo.repo, prAuthor); - if (authorHasWriteAccess) { + // --- Step 1: Skip if PR author has write+ access --- + if (await hasWriteAccess(repo.owner, repo.repo, prAuthor)) { core.info(`PR author ${prAuthor} has write+ access. Skipping.`); - core.setOutput('skipped', 'true'); return; } core.info(`PR author ${prAuthor} does not have write access.`); + // --- Step 2: Small-PR bypass (excluding lock files) --- + const LOCK_FILE_BASENAMES = new Set([ + 'cargo.lock', + 'package-lock.json', + 'yarn.lock', + 'pnpm-lock.yaml', + 'pipfile.lock', + 'poetry.lock', + 'uv.lock', + 'gemfile.lock', + 'composer.lock', + 'go.sum', + 'mix.lock', + 'pubspec.lock', + 'packages.lock.json', + 'podfile.lock', + 'flake.lock', + ]); + const SMALL_PR_THRESHOLD = 100; + + const aggregateLOC = (pullRequest.additions || 0) + (pullRequest.deletions || 0); + if (aggregateLOC < SMALL_PR_THRESHOLD) { + core.info( + `PR has ${aggregateLOC} lines changed (< ${SMALL_PR_THRESHOLD}). Skipping.` + ); + return; + } + + // Stage 2: fetch file list and recompute excluding lock files. + const files = await github.paginate(github.rest.pulls.listFiles, { + owner: repo.owner, + repo: repo.repo, + pull_number: pullRequest.number, + per_page: 100, + }); + function basenameLower(path) { + const idx = path.lastIndexOf('/'); + return (idx >= 0 ? path.slice(idx + 1) : path).toLowerCase(); + } + const nonLockLOC = files + .filter((f) => !LOCK_FILE_BASENAMES.has(basenameLower(f.filename))) + .reduce((sum, f) => sum + (f.additions || 0) + (f.deletions || 0), 0); + if (nonLockLOC < SMALL_PR_THRESHOLD) { + core.info( + `PR has ${nonLockLOC} non-lock-file lines changed (< ${SMALL_PR_THRESHOLD}). Skipping.` + ); + return; + } + // --- Step 3: Parse issue references from PR body --- const body = pullRequest.body || ''; - - // Match all issue reference formats: - // #123, Fixes #123, getsentry/repo#123, Fixes getsentry/repo#123 - // https://github.com/getsentry/repo/issues/123 const issueRefs = []; const seen = new Set(); - // Pattern 1: Full GitHub URLs + // Pattern 1: full GitHub URLs const urlPattern = /https?:\/\/github\.com\/(getsentry)\/([\w.-]+)\/issues\/(\d+)/gi; for (const match of body.matchAll(urlPattern)) { const key = `${match[1]}/${match[2]}#${match[3]}`; @@ -104,7 +133,7 @@ module.exports = async ({ github, context, core }) => { } } - // Pattern 2: Cross-repo references (getsentry/repo#123) + // Pattern 2: cross-repo references (getsentry/repo#123) const crossRepoPattern = /(?:(?:fix|fixes|fixed|close|closes|closed|resolve|resolves|resolved)\s+)?(getsentry)\/([\w.-]+)#(\d+)/gi; for (const match of body.matchAll(crossRepoPattern)) { const key = `${match[1]}/${match[2]}#${match[3]}`; @@ -114,8 +143,7 @@ module.exports = async ({ github, context, core }) => { } } - // Pattern 3: Same-repo references (#123) - // Negative lookbehind to avoid matching cross-repo refs or URLs already captured + // Pattern 3: same-repo references (#123) const sameRepoPattern = /(?:(?:fix|fixes|fixed|close|closes|closed|resolve|resolves|resolved)\s+)?(? { core.info(`Found ${issueRefs.length} issue reference(s): ${[...seen].join(', ')}`); - // --- Helper: close PR with comment and labels --- - async function closePR(message, reasonLabel) { - await github.rest.issues.addLabels({ - ...repo, - issue_number: pullRequest.number, - labels: ['violating-contribution-guidelines', reasonLabel], - }); - - await github.rest.issues.createComment({ - ...repo, - issue_number: pullRequest.number, - body: message, - }); - - await github.rest.pulls.update({ - ...repo, - pull_number: pullRequest.number, - state: 'closed', - }); - - core.setOutput('was-closed', 'true'); - } - - // --- Step 4: No issue references --- - if (issueRefs.length === 0) { - core.info('No issue references found. Closing PR.'); - await closePR([ - 'This PR has been automatically closed. All non-maintainer contributions must reference an existing GitHub issue.', - '', - '**Next steps:**', - '1. Find or open an issue describing the problem or feature', - '2. Discuss the approach with a maintainer in the issue', - '3. Once a maintainer has acknowledged your proposed approach, open a new PR referencing the issue', - '', - `Please review our [contributing guidelines](${contributingUrl}) for more details.`, - ].join('\n'), 'missing-issue-reference'); - return; - } - - // --- Step 5: Validate each referenced issue --- - // A PR is valid if ANY referenced issue passes all checks. - let hasAssigneeConflict = false; - let hasNoDiscussion = false; - + // --- Step 4: Validate referenced issues; succeed if any pass all checks --- for (const ref of issueRefs) { core.info(`Checking issue ${ref.owner}/${ref.repo}#${ref.number}...`); @@ -187,17 +172,16 @@ module.exports = async ({ github, context, core }) => { continue; } - // Check assignee: if assigned to someone other than PR author, flag it + // Assignee check: skip this ref if assigned to someone other than PR author. if (issue.assignees && issue.assignees.length > 0) { - const assignedToAuthor = issue.assignees.some(a => a.login === prAuthor); + const assignedToAuthor = issue.assignees.some((a) => a.login === prAuthor); if (!assignedToAuthor) { core.info(`Issue ${ref.owner}/${ref.repo}#${ref.number} is assigned to someone else.`); - hasAssigneeConflict = true; continue; } } - // Check discussion: both PR author and a maintainer must have commented + // Discussion check: PR author and a maintainer must both have participated. const comments = await github.paginate(github.rest.issues.listComments, { owner: ref.owner, repo: ref.repo, @@ -205,77 +189,74 @@ module.exports = async ({ github, context, core }) => { per_page: 100, }); - // Also consider the issue author as a participant (opening the issue is a form of discussion) - // Guard against null user (deleted/suspended GitHub accounts) const prAuthorParticipated = issue.user?.login === prAuthor || - comments.some(c => c.user?.login === prAuthor); + comments.some((c) => c.user?.login === prAuthor); - let maintainerParticipated = false; - if (prAuthorParticipated) { - // Check each commenter (and issue author) for admin/maintain access on the target repo - const usersToCheck = new Set(); - if (issue.user?.login) usersToCheck.add(issue.user.login); - for (const comment of comments) { - if (comment.user?.login && comment.user.login !== prAuthor) { - usersToCheck.add(comment.user.login); - } + if (!prAuthorParticipated) { + core.info(`Issue ${ref.owner}/${ref.repo}#${ref.number} has no PR author participation.`); + continue; + } + + const usersToCheck = new Set(); + if (issue.user?.login && issue.user.login !== prAuthor) { + usersToCheck.add(issue.user.login); + } + for (const comment of comments) { + if (comment.user?.login && comment.user.login !== prAuthor) { + usersToCheck.add(comment.user.login); } + } - for (const user of usersToCheck) { - if (user === prAuthor) continue; - if (await isMaintainer(repo.owner, repo.repo, user)) { - maintainerParticipated = true; - core.info(`Maintainer ${user} participated in ${ref.owner}/${ref.repo}#${ref.number}.`); - break; - } + let maintainerParticipated = false; + for (const user of usersToCheck) { + if (await isMaintainer(repo.owner, repo.repo, user)) { + maintainerParticipated = true; + core.info(`Maintainer ${user} participated in ${ref.owner}/${ref.repo}#${ref.number}.`); + break; } } - if (prAuthorParticipated && maintainerParticipated) { + if (maintainerParticipated) { core.info(`Issue ${ref.owner}/${ref.repo}#${ref.number} has valid discussion. PR is allowed.`); - return; // PR is valid — at least one issue passes all checks + return; } - - core.info(`Issue ${ref.owner}/${ref.repo}#${ref.number} lacks discussion between author and maintainer.`); - hasNoDiscussion = true; + core.info(`Issue ${ref.owner}/${ref.repo}#${ref.number} lacks maintainer participation.`); } - // --- Step 6: No valid issue found — close with the most relevant reason --- - if (hasAssigneeConflict) { - core.info('Closing PR: referenced issue is assigned to someone else.'); - await closePR([ - 'This PR has been automatically closed. The referenced issue is already assigned to someone else.', - '', - 'If you believe this assignment is outdated, please comment on the issue to discuss before opening a new PR.', - '', - `Please review our [contributing guidelines](${contributingUrl}) for more details.`, - ].join('\n'), 'issue-already-assigned'); - return; + // --- Step 5: Validation failed — post one warm comment (idempotent) --- + let botLogin = null; + try { + const { data: app } = await github.rest.apps.getAuthenticated(); + botLogin = `${app.slug}[bot]`; + } catch (e) { + core.warning(`Could not resolve bot login: ${e.message}`); } - if (hasNoDiscussion) { - core.info('Closing PR: no discussion between PR author and a maintainer in the referenced issue.'); - await closePR([ - 'This PR has been automatically closed. The referenced issue does not show a discussion between you and a maintainer.', - '', - 'To avoid wasted effort on both sides, please discuss your proposed approach in the issue first and wait for a maintainer to respond before opening a PR.', - '', - `Please review our [contributing guidelines](${contributingUrl}) for more details.`, - ].join('\n'), 'missing-maintainer-discussion'); - return; + if (botLogin) { + const existing = await github.paginate(github.rest.issues.listComments, { + ...repo, + issue_number: pullRequest.number, + per_page: 100, + }); + if (existing.some((c) => c.user?.login === botLogin)) { + core.info(`Bot ${botLogin} already commented on this PR. Skipping.`); + return; + } } - // If we get here, all issue refs were unfetchable - core.info('Could not validate any referenced issues. Closing PR.'); - await closePR([ - 'This PR has been automatically closed. The referenced issue(s) could not be found.', + const commentBody = [ + '👋 Thanks for sending this our way! Before a maintainer reviews the code, we ask community contributors to align with us on the approach first — it keeps your time pointed at changes we can land.', '', - '**Next steps:**', - '1. Ensure the issue exists and is in a `getsentry` repository', - '2. Discuss the approach with a maintainer in the issue', - '3. Once a maintainer has acknowledged your proposed approach, open a new PR referencing the issue', + 'The easiest way is to open or find a GitHub issue and discuss the approach with a maintainer there, then link that issue from this PR. If the issue is already assigned to someone else, please check in with them (or with us) before continuing — otherwise two people may end up working on the same task.', '', - `Please review our [contributing guidelines](${contributingUrl}) for more details.`, - ].join('\n'), 'missing-issue-reference'); + `See our [contributing guidelines](${contributingUrl}) for the full picture.`, + ].join('\n'); + + await github.rest.issues.createComment({ + ...repo, + issue_number: pullRequest.number, + body: commentBody, + }); + core.info('Posted advisory comment. PR remains open.'); }; From 05ea073e28fdc6f6739b98187ac76b0a05186ea8 Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Mon, 4 May 2026 13:58:02 +0200 Subject: [PATCH 2/7] fix(validate-pr): Harden listFiles error handling and warning message - Wrap pulls.listFiles in try/catch with fail-through to validation, so a transient GitHub API error does not abort the advisory action. - Tighten the bot-login warning to explain the consequence (the duplicate-comment guard is disabled for that run). Co-Authored-By: Claude Opus 4.7 (1M context) --- validate-pr/scripts/validate-pr.js | 51 +++++++++++++++++++----------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/validate-pr/scripts/validate-pr.js b/validate-pr/scripts/validate-pr.js index cfd7879..d9ddaa5 100644 --- a/validate-pr/scripts/validate-pr.js +++ b/validate-pr/scripts/validate-pr.js @@ -97,25 +97,36 @@ module.exports = async ({ github, context, core }) => { return; } - // Stage 2: fetch file list and recompute excluding lock files. - const files = await github.paginate(github.rest.pulls.listFiles, { - owner: repo.owner, - repo: repo.repo, - pull_number: pullRequest.number, - per_page: 100, - }); - function basenameLower(path) { - const idx = path.lastIndexOf('/'); - return (idx >= 0 ? path.slice(idx + 1) : path).toLowerCase(); - } - const nonLockLOC = files - .filter((f) => !LOCK_FILE_BASENAMES.has(basenameLower(f.filename))) - .reduce((sum, f) => sum + (f.additions || 0) + (f.deletions || 0), 0); - if (nonLockLOC < SMALL_PR_THRESHOLD) { - core.info( - `PR has ${nonLockLOC} non-lock-file lines changed (< ${SMALL_PR_THRESHOLD}). Skipping.` + // Stage 2: fetch file list and recompute excluding lock files. On API + // failure, fall through to validation rather than aborting — a transient + // GitHub error shouldn't break the action. + let files = null; + try { + files = await github.paginate(github.rest.pulls.listFiles, { + owner: repo.owner, + repo: repo.repo, + pull_number: pullRequest.number, + per_page: 100, + }); + } catch (e) { + core.warning( + `Could not fetch PR file list (${e.message}); skipping lock-file exclusion and proceeding to validation.` ); - return; + } + if (files) { + function basenameLower(path) { + const idx = path.lastIndexOf('/'); + return (idx >= 0 ? path.slice(idx + 1) : path).toLowerCase(); + } + const nonLockLOC = files + .filter((f) => !LOCK_FILE_BASENAMES.has(basenameLower(f.filename))) + .reduce((sum, f) => sum + (f.additions || 0) + (f.deletions || 0), 0); + if (nonLockLOC < SMALL_PR_THRESHOLD) { + core.info( + `PR has ${nonLockLOC} non-lock-file lines changed (< ${SMALL_PR_THRESHOLD}). Skipping.` + ); + return; + } } // --- Step 3: Parse issue references from PR body --- @@ -230,7 +241,9 @@ module.exports = async ({ github, context, core }) => { const { data: app } = await github.rest.apps.getAuthenticated(); botLogin = `${app.slug}[bot]`; } catch (e) { - core.warning(`Could not resolve bot login: ${e.message}`); + core.warning( + `Could not resolve bot login (${e.message}); duplicate-comment guard disabled for this run.` + ); } if (botLogin) { From ea6826ec117e8aae2023541950ed954423f27712 Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Mon, 4 May 2026 13:58:54 +0200 Subject: [PATCH 3/7] feat(validate-pr): Drop was-closed output (v4) The advisory action no longer closes PRs, so the was-closed output is meaningless. Dropping it as part of the v4 behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- validate-pr/action.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/validate-pr/action.yml b/validate-pr/action.yml index b305549..f698d5a 100644 --- a/validate-pr/action.yml +++ b/validate-pr/action.yml @@ -10,11 +10,6 @@ inputs: description: 'GitHub App private key for the SDK Maintainer Bot' required: true -outputs: - was-closed: - description: 'Whether the PR was closed by the validation step' - value: ${{ steps.validate.outputs.was-closed }} - runs: using: 'composite' steps: From 2690c5f7e1686ee6620b9f8a21eef43c93334ca9 Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Mon, 4 May 2026 14:02:08 +0200 Subject: [PATCH 4/7] docs(validate-pr): Rewrite README for v4 advisory mode Document advisory behavior, the small-PR + lock-file bypass, and the migration path from v3 (close + labels removed). Co-Authored-By: Claude Opus 4.7 (1M context) --- validate-pr/README.md | 64 ++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/validate-pr/README.md b/validate-pr/README.md index e94a5f3..54272c0 100644 --- a/validate-pr/README.md +++ b/validate-pr/README.md @@ -1,12 +1,12 @@ # Validate PR -Validates non-maintainer pull requests against contribution guidelines. +Advisory validation for non-maintainer pull requests against contribution guidelines. Posts a single friendly comment when a PR doesn't reference an issue with prior maintainer discussion. **PRs are never closed, and no labels are applied.** ## What it does -**Validates issue references** — Non-maintainer PRs must reference a GitHub issue where the PR author and a maintainer have discussed the approach. PRs that don't meet this requirement are automatically closed with a descriptive comment. +For PRs from non-maintainer authors, the action checks that the PR body references a GitHub issue where the PR author and a maintainer have discussed the approach. When that's not the case, the action posts one short advisory comment inviting the contributor to start with an issue. Maintainers (`admin` or `maintain` role) and a hard-coded list of trusted bots are exempt. -Maintainers (users with `admin` or `maintain` role) are exempt from validation. When a maintainer reopens a previously closed PR, all checks are skipped — this allows maintainers to override the action's decision. +Small PRs (< 100 lines changed, excluding lock files) are skipped entirely — typo fixes and tiny bug fixes don't need to go through the issue-discussion loop. ## Usage @@ -17,7 +17,7 @@ name: Validate PR on: pull_request_target: - types: [opened, reopened] + types: [opened] jobs: validate-pr: @@ -25,12 +25,14 @@ jobs: permissions: pull-requests: write steps: - - uses: getsentry/github-workflows/validate-pr@v3 + - uses: getsentry/github-workflows/validate-pr@v4 with: app-id: ${{ vars.SDK_MAINTAINER_BOT_APP_ID }} private-key: ${{ secrets.SDK_MAINTAINER_BOT_PRIVATE_KEY }} ``` +The `pull-requests: write` permission is needed because the action posts comments on the PR. + ## Inputs | Input | Required | Description | @@ -38,33 +40,57 @@ jobs: | `app-id` | Yes | GitHub App ID for the SDK Maintainer Bot | | `private-key` | Yes | GitHub App private key for the SDK Maintainer Bot | -## Outputs +## Validation rules -| Output | Description | -|--------|-------------| -| `was-closed` | `'true'` if the PR was closed by validation, unset otherwise | +### Skipped entirely -## Validation rules +- PR author is in the trusted-bot allowlist (Dependabot, Renovate, Codecov AI, etc.) +- PR author has `admin`, `maintain`, `push`, or `write` access on the repo +- PR has fewer than 100 lines changed (`additions + deletions`), excluding common lock files + +### Lock files excluded from line counts + +Matched by basename, case-insensitive: + +| Ecosystem | File | +|-----------|------| +| Rust | `Cargo.lock` | +| JS | `package-lock.json`, `yarn.lock`, `pnpm-lock.yaml` | +| Python | `Pipfile.lock`, `poetry.lock`, `uv.lock` | +| Ruby | `Gemfile.lock` | +| PHP | `composer.lock` | +| Go | `go.sum` (`go.mod` is hand-edited and stays counted) | +| Elixir | `mix.lock` | +| Dart/Flutter | `pubspec.lock` | +| .NET/NuGet | `packages.lock.json` | +| CocoaPods | `Podfile.lock` | +| Nix | `flake.lock` | ### Issue reference check -The PR body is scanned for issue references in these formats: +For PRs that reach validation, the action scans the PR body for issue references in these formats: - `#123` (same-repo) - `getsentry/repo#123` (cross-repo) - `https://github.com/getsentry/repo/issues/123` (full URL) - With optional keywords: `Fixes #123`, `Closes getsentry/repo#123`, etc. -A PR is valid if **any** referenced issue passes all checks: +The PR is considered compliant if **any** referenced issue passes all of: + - The issue is fetchable and in a `getsentry` repository -- If the issue has assignees, the PR author must be one of them +- If the issue has assignees, the PR author is one of them - Both the PR author and a maintainer have participated in the issue discussion -## Labels +If no referenced issue passes, the action posts one advisory comment. The PR remains open and reviewable; no labels or status checks are applied. The comment is idempotent — workflow re-runs on the same PR will not produce duplicates. + +## Migrating from v3 + +v3 closed non-compliant PRs and applied labels (`violating-contribution-guidelines`, `missing-issue-reference`, `missing-maintainer-discussion`, `issue-already-assigned`). v4 does neither — it only posts a comment. + +To upgrade, update your workflow: -The action creates these labels automatically (they don't need to exist beforehand): +- Change `getsentry/github-workflows/validate-pr@v3` → `@v4` +- Change `types: [opened, reopened]` → `types: [opened]` +- Remove any code that reads the `was-closed` output (it no longer exists) -- `violating-contribution-guidelines` — added to all closed PRs -- `missing-issue-reference` — PR body has no issue references -- `missing-maintainer-discussion` — referenced issue lacks author + maintainer discussion -- `issue-already-assigned` — referenced issue is assigned to someone else +Existing labels on old PRs are not removed automatically. Clean them up with a one-off script if desired. From a576c1077466223865aa556705317b26982d2020 Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Mon, 4 May 2026 14:04:55 +0200 Subject: [PATCH 5/7] docs(validate-pr): Add v4 advisory-mode changelog entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also corrects the original v3-era entry to drop the "enforcing draft status" language — that part of the feature was removed in #159 and never shipped in a tagged release. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b02316c..fc8c280 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,14 @@ ## Unreleased +### Breaking changes + +- Validate PR - Major v4 release: action is now advisory. PRs are no longer closed, and no labels are applied. Instead, a single friendly comment is posted on community PRs that don't reference an issue with maintainer discussion. The `was-closed` output has been removed. Recommended trigger is now `types: [opened]` (drop `reopened`). Pin to `@v3` to keep the previous closing behavior. + ### Features -- Add validate-pr composite action for validating non-maintainer PRs against contribution guidelines and enforcing draft status ([#153](https://github.com/getsentry/github-workflows/pull/153)) +- Validate PR - Skip validation for PRs with fewer than 100 lines changed, excluding common lock files (`Cargo.lock`, `yarn.lock`, `package-lock.json`, `Pipfile.lock`, etc.). Tiny PRs no longer go through the issue-discussion loop. +- Add validate-pr composite action for validating non-maintainer PRs against contribution guidelines ([#153](https://github.com/getsentry/github-workflows/pull/153)) ## 3.3.0 From 271bb7ab5839589473f50def384b15f3a488e38c Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Mon, 4 May 2026 14:19:30 +0200 Subject: [PATCH 6/7] fix(validate-pr): Use app-slug from create-github-app-token for idempotence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous code called github.rest.apps.getAuthenticated() to resolve the bot login, but that endpoint requires JWT auth — our github client is authenticated with an installation access token, so the call always failed. The catch handler then left botLogin null, the idempotence check was skipped, and re-runs of the workflow could produce duplicate comments. Pass the app slug via env var from create-github-app-token's app-slug output (added in v2 of that action) and construct the bot login as "[bot]" directly. No API call needed. Co-Authored-By: Claude Opus 4.7 (1M context) --- validate-pr/action.yml | 2 ++ validate-pr/scripts/validate-pr.js | 36 ++++++++++++++++-------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/validate-pr/action.yml b/validate-pr/action.yml index f698d5a..d5b78b3 100644 --- a/validate-pr/action.yml +++ b/validate-pr/action.yml @@ -23,6 +23,8 @@ runs: - name: Validate PR id: validate uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + env: + APP_SLUG: ${{ steps.app-token.outputs.app-slug }} with: github-token: ${{ steps.app-token.outputs.token }} script: | diff --git a/validate-pr/scripts/validate-pr.js b/validate-pr/scripts/validate-pr.js index d9ddaa5..ac823c7 100644 --- a/validate-pr/scripts/validate-pr.js +++ b/validate-pr/scripts/validate-pr.js @@ -236,26 +236,28 @@ module.exports = async ({ github, context, core }) => { } // --- Step 5: Validation failed — post one warm comment (idempotent) --- - let botLogin = null; - try { - const { data: app } = await github.rest.apps.getAuthenticated(); - botLogin = `${app.slug}[bot]`; - } catch (e) { - core.warning( - `Could not resolve bot login (${e.message}); duplicate-comment guard disabled for this run.` + // The bot's GitHub login is `${app.slug}[bot]`. We get the slug from the + // composite action via the APP_SLUG env var (sourced from + // create-github-app-token's `app-slug` output) — calling + // apps.getAuthenticated() here would fail because it requires JWT auth + // and the github client is authenticated with an installation token. + const appSlug = process.env.APP_SLUG; + if (!appSlug) { + core.setFailed( + 'APP_SLUG env var is not set. The validate-pr composite action must pass app-slug from create-github-app-token to the script.' ); + return; } + const botLogin = `${appSlug}[bot]`; - if (botLogin) { - const existing = await github.paginate(github.rest.issues.listComments, { - ...repo, - issue_number: pullRequest.number, - per_page: 100, - }); - if (existing.some((c) => c.user?.login === botLogin)) { - core.info(`Bot ${botLogin} already commented on this PR. Skipping.`); - return; - } + const existing = await github.paginate(github.rest.issues.listComments, { + ...repo, + issue_number: pullRequest.number, + per_page: 100, + }); + if (existing.some((c) => c.user?.login === botLogin)) { + core.info(`Bot ${botLogin} already commented on this PR. Skipping.`); + return; } const commentBody = [ From 5ff4eb845ca498ce407864eebd44749a9f6206e7 Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Mon, 4 May 2026 14:38:08 +0200 Subject: [PATCH 7/7] docs(validate-pr): Drop version-tag references in favor of SHA pinning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consumers in getsentry/* pin this action by commit SHA, not by @v* tags or major-version moving tags. Update the README example and the "updating earlier revisions" section to reflect that. Drop the "Breaking changes" framing in the changelog — the action has not shipped in a tagged release, so calling it a breaking change overstates the impact. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 5 +---- validate-pr/README.md | 16 ++++++++-------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc8c280..f222658 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,9 @@ ## Unreleased -### Breaking changes - -- Validate PR - Major v4 release: action is now advisory. PRs are no longer closed, and no labels are applied. Instead, a single friendly comment is posted on community PRs that don't reference an issue with maintainer discussion. The `was-closed` output has been removed. Recommended trigger is now `types: [opened]` (drop `reopened`). Pin to `@v3` to keep the previous closing behavior. - ### Features +- Validate PR - Action is advisory: it posts a single friendly comment on community PRs that don't reference an issue with maintainer discussion. PRs are not closed and no labels are applied. Recommended trigger is `types: [opened]`. - Validate PR - Skip validation for PRs with fewer than 100 lines changed, excluding common lock files (`Cargo.lock`, `yarn.lock`, `package-lock.json`, `Pipfile.lock`, etc.). Tiny PRs no longer go through the issue-discussion loop. - Add validate-pr composite action for validating non-maintainer PRs against contribution guidelines ([#153](https://github.com/getsentry/github-workflows/pull/153)) diff --git a/validate-pr/README.md b/validate-pr/README.md index 54272c0..779b179 100644 --- a/validate-pr/README.md +++ b/validate-pr/README.md @@ -25,13 +25,13 @@ jobs: permissions: pull-requests: write steps: - - uses: getsentry/github-workflows/validate-pr@v4 + - uses: getsentry/github-workflows/validate-pr@ with: app-id: ${{ vars.SDK_MAINTAINER_BOT_APP_ID }} private-key: ${{ secrets.SDK_MAINTAINER_BOT_PRIVATE_KEY }} ``` -The `pull-requests: write` permission is needed because the action posts comments on the PR. +Pin to a specific commit SHA (consumers in `getsentry/*` already follow this convention). The `pull-requests: write` permission is needed because the action posts comments on the PR. ## Inputs @@ -83,14 +83,14 @@ The PR is considered compliant if **any** referenced issue passes all of: If no referenced issue passes, the action posts one advisory comment. The PR remains open and reviewable; no labels or status checks are applied. The comment is idempotent — workflow re-runs on the same PR will not produce duplicates. -## Migrating from v3 +## Updating from earlier revisions -v3 closed non-compliant PRs and applied labels (`violating-contribution-guidelines`, `missing-issue-reference`, `missing-maintainer-discussion`, `issue-already-assigned`). v4 does neither — it only posts a comment. +Earlier revisions of this action closed non-compliant PRs and applied labels (`violating-contribution-guidelines`, `missing-issue-reference`, `missing-maintainer-discussion`, `issue-already-assigned`). The current version does neither — it only posts a comment. -To upgrade, update your workflow: +To update an existing consumer: -- Change `getsentry/github-workflows/validate-pr@v3` → `@v4` -- Change `types: [opened, reopened]` → `types: [opened]` -- Remove any code that reads the `was-closed` output (it no longer exists) +- Bump the pinned commit SHA to the latest on `main`. +- Change `types: [opened, reopened]` → `types: [opened]`. +- Remove any code that reads the `was-closed` output (it no longer exists). Existing labels on old PRs are not removed automatically. Clean them up with a one-off script if desired.