diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 51479a73dd5..28aa4ac96e7 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -17,6 +17,7 @@ https://linear.app/n8n/issue/ ## Review / Merge checklist +- [ ] I have seen this code, I have run this code, and I take responsibility for this code. - [ ] PR title and summary are descriptive. ([conventions](../blob/master/.github/pull_request_title_conventions.md)) '; + +/** + * Returns true if the PR body contains a checked ownership acknowledgement checkbox. + * + * @param {string | null | undefined} body + * @returns {boolean} + */ +export function isOwnershipCheckboxChecked(body) { + return /\[x\]\s+I have seen this code,\s+I have run this code,\s+and I take responsibility for this code/i.test( + body ?? '', + ); +} + +async function main() { + const event = getEventFromGithubEventPath(); + const pr = event.pull_request; + const { octokit, owner, repo } = initGithub(); + + const { data: comments } = await octokit.rest.issues.listComments({ + owner, + repo, + issue_number: pr.number, + per_page: 100, + }); + const botComment = comments.find((c) => c.body.includes(BOT_MARKER)); + + if (!isOwnershipCheckboxChecked(pr.body)) { + const message = [ + BOT_MARKER, + '## ⚠️ Ownership acknowledgement required', + '', + 'Please add or check the following item in your PR description before this can be merged:', + '', + '```', + '- [x] I have seen this code, I have run this code, and I take responsibility for this code.', + '```', + ].join('\n'); + + if (botComment) { + await octokit.rest.issues.updateComment({ + owner, + repo, + comment_id: botComment.id, + body: message, + }); + } else { + await octokit.rest.issues.createComment({ + owner, + repo, + issue_number: pr.number, + body: message, + }); + } + + console.log( + '::error::Ownership checkbox is not checked. Add it to your PR description and check it.', + ); + process.exit(1); + } else if (botComment) { + await octokit.rest.issues.deleteComment({ + owner, + repo, + comment_id: botComment.id, + }); + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + await main(); +} diff --git a/.github/scripts/quality/check-ownership-checkbox.test.mjs b/.github/scripts/quality/check-ownership-checkbox.test.mjs new file mode 100644 index 00000000000..2336278e04f --- /dev/null +++ b/.github/scripts/quality/check-ownership-checkbox.test.mjs @@ -0,0 +1,85 @@ +import { describe, it, before, mock } from 'node:test'; +import assert from 'node:assert/strict'; + +/** + * Run with: + * node --test --experimental-test-module-mocks .github/scripts/quality/check-ownership-checkbox.test.mjs + */ + +mock.module('../github-helpers.mjs', { + namedExports: { + initGithub: () => {}, + getEventFromGithubEventPath: () => {}, + }, +}); + +let isOwnershipCheckboxChecked; +before(async () => { + ({ isOwnershipCheckboxChecked } = await import('./check-ownership-checkbox.mjs')); +}); + +describe('isOwnershipCheckboxChecked', () => { + it('returns true for a checked checkbox with exact text', () => { + const body = + '- [x] I have seen this code, I have run this code, and I take responsibility for this code.'; + assert.ok(isOwnershipCheckboxChecked(body)); + }); + + it('returns true for uppercase [X]', () => { + const body = + '- [X] I have seen this code, I have run this code, and I take responsibility for this code.'; + assert.ok(isOwnershipCheckboxChecked(body)); + }); + + it('returns false for an unchecked checkbox [ ]', () => { + const body = + '- [ ] I have seen this code, I have run this code, and I take responsibility for this code.'; + assert.equal(isOwnershipCheckboxChecked(body), false); + }); + + it('returns false when the checkbox is absent', () => { + const body = '## Summary\n\nThis PR does some things.\n'; + assert.equal(isOwnershipCheckboxChecked(body), false); + }); + + it('returns false for null body', () => { + assert.equal(isOwnershipCheckboxChecked(null), false); + }); + + it('returns false for undefined body', () => { + assert.equal(isOwnershipCheckboxChecked(undefined), false); + }); + + it('returns false for empty body', () => { + assert.equal(isOwnershipCheckboxChecked(''), false); + }); + + it('returns true when checkbox appears among other content', () => { + const body = [ + '## Summary', + '', + 'Some description here.', + '', + '## Checklist', + '- [x] Tests included', + '- [x] I have seen this code, I have run this code, and I take responsibility for this code.', + '- [ ] Docs updated', + ].join('\n'); + assert.ok(isOwnershipCheckboxChecked(body)); + }); + + it('returns false when only other checkboxes are checked', () => { + const body = [ + '- [x] Tests included', + '- [x] Docs updated', + '- [ ] I have seen this code, I have run this code, and I take responsibility for this code.', + ].join('\n'); + assert.equal(isOwnershipCheckboxChecked(body), false); + }); + + it('is case-insensitive for the checkbox marker', () => { + const lower = + '- [x] i have seen this code, i have run this code, and i take responsibility for this code.'; + assert.ok(isOwnershipCheckboxChecked(lower)); + }); +}); diff --git a/.github/scripts/quality/check-pr-size.mjs b/.github/scripts/quality/check-pr-size.mjs new file mode 100644 index 00000000000..775e233853f --- /dev/null +++ b/.github/scripts/quality/check-pr-size.mjs @@ -0,0 +1,120 @@ +/** + * Checks that the PR does not exceed the line addition limit. + * + * A maintainer (write access or above) can override by commenting `/size-limit-override` + * on the PR. The override takes effect on the next pull_request event (push, reopen, etc.). + * + * Exit codes: + * 0 – PR is within the limit, or a valid override comment exists + * 1 – PR exceeds the limit with no valid override + */ + +import { initGithub, getEventFromGithubEventPath } from '../github-helpers.mjs'; + +export const SIZE_LIMIT = 1000; +export const OVERRIDE_COMMAND = '/size-limit-override'; + +const BOT_MARKER = ''; + +/** + * Returns true if any comment in the list is a valid `/size-limit-override` from a + * user with write access or above. + * + * @param {Array<{ body?: string, user: { login: string } }>} comments + * @param {(username: string) => Promise} getPermission - returns the permission level string + * @returns {Promise} + */ +export async function hasValidOverride(comments, getPermission) { + for (const comment of comments) { + if (!comment.body?.startsWith(OVERRIDE_COMMAND)) { + continue; + } + + const perm = await getPermission(comment.user.login); + if (['admin', 'write', 'maintain'].includes(perm)) { + return true; + } + } + return false; +} + +async function main() { + const event = getEventFromGithubEventPath(); + const pr = event.pull_request; + const { octokit, owner, repo } = initGithub(); + + const additions = pr.additions; + + const { data: comments } = await octokit.rest.issues.listComments({ + owner, + repo, + issue_number: pr.number, + per_page: 100, + sort: 'created', + direction: 'desc', + }); + + const overrideFound = await hasValidOverride(comments, async (username) => { + const { data: perm } = await octokit.rest.repos.getCollaboratorPermissionLevel({ + owner, + repo, + username, + }); + return perm.permission; + }); + + const botComment = comments.find((c) => c.body.includes(BOT_MARKER)); + + if (additions > SIZE_LIMIT && !overrideFound) { + const message = [ + BOT_MARKER, + `## ⚠️ PR exceeds size limit (${additions.toLocaleString()} lines added)`, + '', + `This PR adds **${additions.toLocaleString()} lines**, exceeding the ${SIZE_LIMIT.toLocaleString()}-line limit.`, + '', + 'Large PRs are harder to review and increase the risk of bugs going unnoticed. Please consider:', + '- Breaking this into smaller, logically separate PRs', + '- Moving unrelated changes to a follow-up PR', + '', + `If the size is genuinely justified (e.g. generated code, large migrations, test fixtures), a maintainer can override by commenting \`${OVERRIDE_COMMAND}\` and then pushing a new commit or re-running this check.`, + ].join('\n'); + + if (botComment) { + await octokit.rest.issues.updateComment({ + owner, + repo, + comment_id: botComment.id, + body: message, + }); + } else { + await octokit.rest.issues.createComment({ + owner, + repo, + issue_number: pr.number, + body: message, + }); + } + + console.log( + `::error::PR adds ${additions.toLocaleString()} lines, exceeding the ${SIZE_LIMIT.toLocaleString()}-line limit. Reduce PR size or ask a maintainer to comment \`${OVERRIDE_COMMAND}\`.`, + ); + process.exit(1); + } else { + if (botComment) { + await octokit.rest.issues.deleteComment({ + owner, + repo, + comment_id: botComment.id, + }); + } + if (overrideFound && additions > SIZE_LIMIT) { + console.log( + `PR size limit overridden. ${additions.toLocaleString()} lines added (limit: ${SIZE_LIMIT.toLocaleString()}).`, + ); + } + } +} + +if (import.meta.url === `file://${process.argv[1]}`) { + await main(); +} diff --git a/.github/scripts/quality/check-pr-size.test.mjs b/.github/scripts/quality/check-pr-size.test.mjs new file mode 100644 index 00000000000..f9cab62d30e --- /dev/null +++ b/.github/scripts/quality/check-pr-size.test.mjs @@ -0,0 +1,116 @@ +import { describe, it, before, mock } from 'node:test'; +import assert from 'node:assert/strict'; + +/** + * Run with: + * node --test --experimental-test-module-mocks .github/scripts/quality/check-pr-size.test.mjs + */ + +mock.module('../github-helpers.mjs', { + namedExports: { + initGithub: () => {}, + getEventFromGithubEventPath: () => {}, + }, +}); + +let hasValidOverride, SIZE_LIMIT, OVERRIDE_COMMAND; +before(async () => { + ({ hasValidOverride, SIZE_LIMIT, OVERRIDE_COMMAND } = await import('./check-pr-size.mjs')); +}); + +/** @param {string} permission */ +const permissionGetter = (permission) => async (_username) => permission; + +describe('SIZE_LIMIT', () => { + it('is 1000', () => { + assert.equal(SIZE_LIMIT, 1000); + }); +}); + +describe('hasValidOverride', () => { + it('returns false when there are no comments', async () => { + const result = await hasValidOverride([], permissionGetter('write')); + assert.equal(result, false); + }); + + it('returns false when no comment starts with the override command', async () => { + const comments = [ + { body: 'Looks good to me!', user: { login: 'reviewer' } }, + { body: 'Please split this PR.', user: { login: 'maintainer' } }, + ]; + const result = await hasValidOverride(comments, permissionGetter('write')); + assert.equal(result, false); + }); + + it('returns true when a write-access user has posted the override command', async () => { + const comments = [{ body: OVERRIDE_COMMAND, user: { login: 'maintainer' } }]; + const result = await hasValidOverride(comments, permissionGetter('write')); + assert.ok(result); + }); + + it('returns true for maintain permission', async () => { + const comments = [{ body: OVERRIDE_COMMAND, user: { login: 'lead' } }]; + const result = await hasValidOverride(comments, permissionGetter('maintain')); + assert.ok(result); + }); + + it('returns true for admin permission', async () => { + const comments = [{ body: OVERRIDE_COMMAND, user: { login: 'admin' } }]; + const result = await hasValidOverride(comments, permissionGetter('admin')); + assert.ok(result); + }); + + it('returns false when the override commenter only has read access', async () => { + const comments = [{ body: OVERRIDE_COMMAND, user: { login: 'outsider' } }]; + const result = await hasValidOverride(comments, permissionGetter('read')); + assert.equal(result, false); + }); + + it('returns false when the override commenter only has triage access', async () => { + const comments = [{ body: OVERRIDE_COMMAND, user: { login: 'triager' } }]; + const result = await hasValidOverride(comments, permissionGetter('triage')); + assert.equal(result, false); + }); + + it('returns false when the override command appears mid-comment, not at the start', async () => { + const comments = [ + { + body: `Please note: ${OVERRIDE_COMMAND} should only be used when justified.`, + user: { login: 'maintainer' }, + }, + ]; + const result = await hasValidOverride(comments, permissionGetter('write')); + assert.equal(result, false); + }); + + it('returns true when one of several comments is a valid override', async () => { + const comments = [ + { body: 'Looks good!', user: { login: 'reviewer' } }, + { body: OVERRIDE_COMMAND, user: { login: 'maintainer' } }, + { body: 'Please add tests.', user: { login: 'other' } }, + ]; + const result = await hasValidOverride(comments, permissionGetter('write')); + assert.ok(result); + }); + + it('returns false when override comment exists but all posters lack write access', async () => { + const comments = [ + { body: OVERRIDE_COMMAND, user: { login: 'user1' } }, + { body: OVERRIDE_COMMAND, user: { login: 'user2' } }, + ]; + const result = await hasValidOverride(comments, permissionGetter('read')); + assert.equal(result, false); + }); + + it('checks permissions per commenter independently', async () => { + const permissions = { writer: 'write', reader: 'read' }; + const getPermission = async (username) => permissions[username] ?? 'read'; + + const comments = [ + { body: OVERRIDE_COMMAND, user: { login: 'reader' } }, + { body: OVERRIDE_COMMAND, user: { login: 'writer' } }, + ]; + const result = await hasValidOverride(comments, getPermission); + assert.ok(result); + }); +}); diff --git a/.github/scripts/quality/handle-size-override.mjs b/.github/scripts/quality/handle-size-override.mjs new file mode 100644 index 00000000000..5e5b130cb94 --- /dev/null +++ b/.github/scripts/quality/handle-size-override.mjs @@ -0,0 +1,97 @@ +/** + * Re-triggers the PR Size Limit check when a maintainer comments `/size-limit-override`. + * + * Finds the latest `PR Size Limit` check run on the PR's HEAD commit and re-requests it. + * The re-run scans comments, finds the override, and passes — satisfying branch protection + * without any label manipulation or status API calls. + * + * Exit codes: + * 0 – Check run re-requested successfully + * 1 – Commenter lacks permission, or no check run found to re-request + */ + +import { initGithub, getEventFromGithubEventPath } from '../github-helpers.mjs'; + +const CHECK_NAME = 'PR Size Limit'; + +/** + * @param {{ + * octokit: import('../github-helpers.mjs').GitHubInstance, + * owner: string, + * repo: string, + * prNumber: number, + * commenter: string, + * commentId: number, + * }} params + */ +export async function run({ octokit, owner, repo, prNumber, commenter, commentId }) { + const { data: perm } = await octokit.rest.repos.getCollaboratorPermissionLevel({ + owner, + repo, + username: commenter, + }); + + if (!['admin', 'write', 'maintain'].includes(perm.permission)) { + console.log( + `::error::@${commenter} does not have permission to override the PR size limit (requires write access).`, + ); + process.exit(1); + } + + const { data: pr } = await octokit.rest.pulls.get({ + owner, + repo, + pull_number: prNumber, + }); + const headSha = pr.head.sha; + + const { + data: { check_runs }, + } = await octokit.rest.checks.listForRef({ + owner, + repo, + ref: headSha, + check_name: CHECK_NAME, + per_page: 1, + }); + + if (check_runs.length === 0) { + console.log( + `::error::No '${CHECK_NAME}' check run found for ${headSha}. Push a new commit to trigger it.`, + ); + process.exit(1); + } + + await octokit.rest.checks.rerequestRun({ + owner, + repo, + check_run_id: check_runs[0].id, + }); + + await octokit.rest.reactions.createForIssueComment({ + owner, + repo, + comment_id: commentId, + content: '+1', + }); + + console.log(`Re-requested '${CHECK_NAME}' check run (${check_runs[0].id}) for ${headSha}`); +} + +async function main() { + const event = getEventFromGithubEventPath(); + const { octokit, owner, repo } = initGithub(); + + await run({ + octokit, + owner, + repo, + prNumber: event.issue.number, + commenter: event.sender.login, + commentId: event.comment.id, + }); +} + +if (import.meta.url === `file://${process.argv[1]}`) { + await main(); +} diff --git a/.github/workflows/ci-pr-quality.yml b/.github/workflows/ci-pr-quality.yml new file mode 100644 index 00000000000..4a11a79b4dc --- /dev/null +++ b/.github/workflows/ci-pr-quality.yml @@ -0,0 +1,99 @@ +name: 'CI: PR Quality Checks' + +on: + pull_request: + types: + - opened + - edited + - synchronize + branches: + - master + issue_comment: + types: + - created + +jobs: + handle-size-override: + name: Handle /size-limit-override + # Re-requests the PR Size Limit check run on the PR's HEAD commit, so it re-runs + # in the original PR context and picks up the override comment. + if: | + github.event_name == 'issue_comment' && + github.event.issue.pull_request && + startsWith(github.event.comment.body, '/size-limit-override') + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + checks: write + issues: write + pull-requests: read + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Setup Node.js + uses: ./.github/actions/setup-nodejs + with: + build-command: '' + install-command: pnpm install --frozen-lockfile --dir ./.github/scripts --ignore-workspace + + - name: Re-request PR Size Limit check + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: node .github/scripts/quality/handle-size-override.mjs + + check-ownership-checkbox: + name: Ownership Acknowledgement + # Checks that the author has acknowledged the ownership of their code + # by checking the checkbox in the PR summary. + if: | + github.event_name == 'pull_request' && + !contains(github.event.pull_request.labels.*.name, 'automation:backport') && + !contains(github.event.pull_request.title, '(backport to') + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + issues: write + pull-requests: write + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Setup Node.js + uses: ./.github/actions/setup-nodejs + with: + build-command: '' + install-command: pnpm install --frozen-lockfile --dir ./.github/scripts --ignore-workspace + + - name: Check ownership checkbox + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: node .github/scripts/quality/check-ownership-checkbox.mjs + + check-pr-size: + name: PR Size Limit + # Checks that the PR size doesn't exceed the limit (currently 1000 lines) + # Allows for override via '/size-limit-override' comment + if: | + github.event_name == 'pull_request' && + !contains(github.event.pull_request.labels.*.name, 'automation:backport') && + !contains(github.event.pull_request.title, '(backport to') + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + issues: write + pull-requests: write + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Setup Node.js + uses: ./.github/actions/setup-nodejs + with: + build-command: '' + install-command: pnpm install --frozen-lockfile --dir ./.github/scripts --ignore-workspace + + - name: Check PR size + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: node .github/scripts/quality/check-pr-size.mjs