diff --git a/.github/workflows/pr-size-check.yml b/.github/workflows/pr-size-check.yml index cb410b8a015f8b6214464277ec1840fa00400f9e..6cbed314e012c66da16fd016dd9b3cdcf9788149 100644 --- a/.github/workflows/pr-size-check.yml +++ b/.github/workflows/pr-size-check.yml @@ -1,12 +1,17 @@ -# PR Size Check +# PR Size Check — Compute # -# Comments on PRs that exceed the 400 LOC soft limit with a friendly reminder -# to consider splitting. Does NOT block the PR — advisory only. -# Also adds size labels (size/S, size/M, size/L, size/XL) for tracking. +# Calculates PR size and saves the result as an artifact. A companion +# workflow (pr-size-label.yml) picks up the artifact via workflow_run +# and applies labels + comments with write permissions. # -# Security note: Uses actions/github-script (JavaScript API) — no shell -# interpolation of untrusted input. PR body is accessed via the JS API, -# not via expression interpolation in run: blocks. +# This two-workflow split is required because fork PRs receive a +# read-only GITHUB_TOKEN. The compute step needs no write access; +# the label/comment step runs via workflow_run on the base repo with +# full write permissions. +# +# Security note: This workflow only reads PR file data via the JS API +# and writes a JSON artifact. No untrusted input is interpolated into +# shell commands. name: PR Size Check @@ -14,20 +19,22 @@ on: pull_request: types: [opened, synchronize] +permissions: + contents: read + pull-requests: read + jobs: - check-size: + compute-size: if: github.repository_owner == 'zed-industries' - permissions: - contents: read - pull-requests: write # PR comments - issues: write # label management (GitHub routes labels through Issues API) runs-on: ubuntu-latest timeout-minutes: 5 steps: - - name: Calculate PR size and label + - name: Calculate PR size uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 with: script: | + const fs = require('fs'); + const { data: files } = await github.rest.pulls.listFiles({ owner: context.repo.owner, repo: context.repo.repo, @@ -53,15 +60,15 @@ jobs: } } - // Assign size label + // Assign size bracket const SIZE_BRACKETS = [ - ['size/S', 0, 100, '0e8a16'], - ['size/M', 100, 400, 'fbca04'], - ['size/L', 400, 800, 'e99695'], - ['size/XL', 800, Infinity, 'b60205'], + ['Size S', 0, 100, '0e8a16'], + ['Size M', 100, 400, 'fbca04'], + ['Size L', 400, 800, 'e99695'], + ['Size XL', 800, Infinity, 'b60205'], ]; - let sizeLabel = 'size/S'; + let sizeLabel = 'Size S'; let labelColor = '0e8a16'; for (const [label, min, max, color] of SIZE_BRACKETS) { if (totalChanges >= min && totalChanges < max) { @@ -71,108 +78,32 @@ jobs: } } - // Update size label only if the classification changed - const existingLabels = (await github.rest.issues.listLabelsOnIssue({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - })).data.map(l => l.name); - - const existingSizeLabels = existingLabels.filter(l => l.startsWith('size/')); - const alreadyCorrect = existingSizeLabels.length === 1 && existingSizeLabels[0] === sizeLabel; - - if (!alreadyCorrect) { - for (const label of existingSizeLabels) { - await github.rest.issues.removeLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - name: label, - }); - } - - // Create the label if it doesn't exist (ignore 422 = already exists) - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name: sizeLabel, - color: labelColor, - }); - } catch (e) { - if (e.status !== 422) throw e; - } - - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - labels: [sizeLabel], - }); - } - - // For large PRs (400+ LOC): auto-apply large-pr label and comment once - if (totalChanges >= 400) { - // Auto-apply the large-pr label - if (!existingLabels.includes('large-pr')) { - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name: 'large-pr', - color: 'e99695', - }); - } catch (e) { - if (e.status !== 422) throw e; - } - - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - labels: ['large-pr'], - }); - } - - // Comment once with guidance - const MARKER = ''; - const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - }); + // Check if the author wrote content in the "How to Review" section. + const rawBody = context.payload.pull_request.body || ''; + const howToReview = rawBody.match(/## How to Review\s*\n([\s\S]*?)(?=\n## |$)/i); + const hasReviewGuidance = howToReview + ? howToReview[1].replace(//g, '').trim().length > 0 + : false; - const alreadyCommented = comments.some(c => c.body.includes(MARKER)); - if (!alreadyCommented) { - // Check if the author wrote content in the "How to Review" section. - // Extract the section, strip HTML comment placeholders from the - // template, and check for any remaining non-whitespace content. - const rawBody = context.payload.pull_request.body || ''; - const howToReview = rawBody.match(/## How to Review\s*\n([\s\S]*?)(?=\n## |$)/i); - const guidedTourPresent = howToReview - ? howToReview[1].replace(//g, '').trim().length > 0 - : false; + const result = { + pr_number: context.issue.number, + total_changes: totalChanges, + size_label: sizeLabel, + label_color: labelColor, + has_review_guidance: hasReviewGuidance, + }; - let body = `${MARKER}\n`; - body += `### :straight_ruler: PR Size: **${totalChanges} lines changed** (${sizeLabel})\n\n`; - body += `Please note: this PR exceeds the 400 LOC soft limit.\n`; - body += `- Consider **splitting** into separate PRs if the changes are separable\n`; - body += `- Ensure the PR description includes a **guided tour** in the "How to Review" section so reviewers know where to start\n`; + console.log(`PR #${result.pr_number}: ${totalChanges} LOC, ${sizeLabel}`); - if (guidedTourPresent) { - body += `\n:white_check_mark: "How to Review" section appears to include guidance — thank you!\n`; - } + fs.mkdirSync('pr-size', { recursive: true }); + fs.writeFileSync('pr-size/result.json', JSON.stringify(result)); - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: body, - }); - } - } - - console.log(`PR #${context.issue.number}: ${totalChanges} LOC changed, labeled ${sizeLabel}`); + - name: Upload size result + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: pr-size-result + path: pr-size/ + retention-days: 1 defaults: run: shell: bash -euxo pipefail {0} diff --git a/.github/workflows/pr-size-label.yml b/.github/workflows/pr-size-label.yml new file mode 100644 index 0000000000000000000000000000000000000000..599daf122aac728c469acd45da865e1079c07fb6 --- /dev/null +++ b/.github/workflows/pr-size-label.yml @@ -0,0 +1,195 @@ +# PR Size Check — Label & Comment +# +# Triggered by workflow_run after pr-size-check.yml completes. +# Downloads the size result artifact and applies labels + comments. +# +# This runs on the base repo with full GITHUB_TOKEN write access, +# so it works for both same-repo and fork PRs. +# +# Security note: The artifact is treated as untrusted data — only +# structured JSON fields (PR number, size label, color, boolean) are +# read. No artifact content is executed or interpolated into shell. + +name: PR Size Label + +on: + workflow_run: + workflows: ["PR Size Check"] + types: [completed] + +jobs: + apply-labels: + if: > + github.repository_owner == 'zed-industries' && + github.event.workflow_run.conclusion == 'success' + permissions: + contents: read + pull-requests: write + issues: write + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Download size result artifact + id: download + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + with: + script: | + const fs = require('fs'); + const path = require('path'); + + const allArtifacts = await github.rest.actions.listWorkflowRunArtifacts({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: context.payload.workflow_run.id, + }); + + const match = allArtifacts.data.artifacts.find(a => a.name === 'pr-size-result'); + if (!match) { + console.log('No pr-size-result artifact found, skipping'); + core.setOutput('found', 'false'); + return; + } + + const download = await github.rest.actions.downloadArtifact({ + owner: context.repo.owner, + repo: context.repo.repo, + artifact_id: match.id, + archive_format: 'zip', + }); + + const temp = path.join(process.env.RUNNER_TEMP, 'pr-size'); + fs.mkdirSync(temp, { recursive: true }); + fs.writeFileSync(path.join(temp, 'result.zip'), Buffer.from(download.data)); + core.setOutput('found', 'true'); + + - name: Unzip artifact + if: steps.download.outputs.found == 'true' + env: + ARTIFACT_DIR: ${{ runner.temp }}/pr-size + run: unzip "$ARTIFACT_DIR/result.zip" -d "$ARTIFACT_DIR" + + - name: Apply labels and comment + if: steps.download.outputs.found == 'true' + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + with: + script: | + const fs = require('fs'); + const path = require('path'); + + const temp = path.join(process.env.RUNNER_TEMP, 'pr-size'); + const resultPath = path.join(temp, 'result.json'); + if (!fs.existsSync(resultPath)) { + console.log('No result.json found, skipping'); + return; + } + + const result = JSON.parse(fs.readFileSync(resultPath, 'utf8')); + + // Validate artifact data (treat as untrusted) + const prNumber = Number(result.pr_number); + const totalChanges = Number(result.total_changes); + const sizeLabel = String(result.size_label); + const labelColor = String(result.label_color); + const hasReviewGuidance = Boolean(result.has_review_guidance); + + if (!prNumber || !sizeLabel.startsWith('Size ')) { + core.setFailed(`Invalid artifact data: pr=${prNumber}, label=${sizeLabel}`); + return; + } + + console.log(`PR #${prNumber}: ${totalChanges} LOC, ${sizeLabel}`); + + // --- Size label (idempotent) --- + const existingLabels = (await github.rest.issues.listLabelsOnIssue({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + })).data.map(l => l.name); + + const existingSizeLabels = existingLabels.filter(l => l.startsWith('Size ')); + const alreadyCorrect = existingSizeLabels.length === 1 && existingSizeLabels[0] === sizeLabel; + + if (!alreadyCorrect) { + for (const label of existingSizeLabels) { + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + name: label, + }); + } + + try { + await github.rest.issues.createLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + name: sizeLabel, + color: labelColor, + }); + } catch (e) { + if (e.status !== 422) throw e; + } + + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + labels: [sizeLabel], + }); + } + + // --- Large PR handling (400+ LOC) --- + if (totalChanges >= 400) { + if (!existingLabels.includes('large-pr')) { + try { + await github.rest.issues.createLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + name: 'large-pr', + color: 'e99695', + }); + } catch (e) { + if (e.status !== 422) throw e; + } + + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + labels: ['large-pr'], + }); + } + + // Comment once with guidance + const MARKER = ''; + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + }); + + const alreadyCommented = comments.some(c => c.body.includes(MARKER)); + if (!alreadyCommented) { + let body = `${MARKER}\n`; + body += `### :straight_ruler: PR Size: **${totalChanges} lines changed** (${sizeLabel})\n\n`; + body += `Please note: this PR exceeds the 400 LOC soft limit.\n`; + body += `- Consider **splitting** into separate PRs if the changes are separable\n`; + body += `- Ensure the PR description includes a **guided tour** in the "How to Review" section so reviewers know where to start\n`; + + if (hasReviewGuidance) { + body += `\n:white_check_mark: "How to Review" section appears to include guidance — thank you!\n`; + } + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: prNumber, + body: body, + }); + } + } + + console.log(`PR #${prNumber}: labeled ${sizeLabel}, done`); +defaults: + run: + shell: bash -euxo pipefail {0}