From 7c7ac1021a7bc43da38dcfd880ced7233939829b Mon Sep 17 00:00:00 2001 From: Povilas Kirna Date: Wed, 3 Jun 2026 16:58:10 +0200 Subject: [PATCH] ci: enforce issue/PR description completeness for template-bypassing submissions (#1959) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ci: add issue/PR description completeness checks (#1958) Two github-script workflows that validate description structure on issue/PR open/edit/reopen, for submissions that bypass the browser template (API, gh CLI, agent bulk PRs). - PR check: Summary, Linked Issue, Type of Change, duplicate-search box, How to Test. - Issue check: body length + per-label bug/enhancement fields, plus a bug+enhancement conflict guard. - Pass deletes any prior bot comment and applies `ready for review`; fail posts an in-place comment, fails the check, and applies `needs work` (PRs) / `needs more info` (issues). - References existing labels only — never creates or recolours repo labels (checks existence first, warns and skips if absent). - Safe pull_request_target: checkout pinned to the base ref, sparse `.github/scripts` only; PR head never checked out. Closes #1958 Co-authored-by: Povilas Kirna Co-authored-by: Claude Opus 4.8 --- .github/scripts/check-issue-description.js | 165 ++++++++++++++++++ .github/scripts/check-pr-description.js | 121 +++++++++++++ .github/workflows/issue-description-check.yml | 23 +++ .github/workflows/pr-description-check.yml | 28 +++ 4 files changed, 337 insertions(+) create mode 100644 .github/scripts/check-issue-description.js create mode 100644 .github/scripts/check-pr-description.js create mode 100644 .github/workflows/issue-description-check.yml create mode 100644 .github/workflows/pr-description-check.yml diff --git a/.github/scripts/check-issue-description.js b/.github/scripts/check-issue-description.js new file mode 100644 index 0000000..2156082 --- /dev/null +++ b/.github/scripts/check-issue-description.js @@ -0,0 +1,165 @@ +// @ts-check +'use strict'; + +/** @param {{ github: import('@octokit/rest').Octokit, context: import('@actions/github').context, core: import('@actions/core') }} */ +module.exports = async ({ github, context, core }) => { + const issue = context.payload.issue; + const body = (issue.body || '').trim(); + const labels = issue.labels.map(l => l.name); + const owner = context.repo.owner; + const repo = context.repo.repo; + + const isBug = labels.includes('bug'); + const isFeature = labels.includes('enhancement'); + + // Extract a Section's text, stripping HTML comments. Matches any heading + // depth (#, ##, ###, …) so a manually-written body isn't penalised for + // using a different number of hashes than the issue form generates. + function section(heading) { + const re = new RegExp(`#+\\s+${heading}\\s*([\\s\\S]*?)(?=\\n#+\\s+|$)`, 'i'); + const m = body.match(re); + return m ? m[1].replace(//g, '').trim() : ''; + } + + const failures = []; + + // ── Common: body must exist ─────────────────────────────────────────────── + if (body.length < 50) { + failures.push( + '**Description** — body is empty or too short. ' + + 'Please open the issue using one of the provided templates.', + ); + } + + // An issue is one or the other — never both. Resolve to a single type so the + // validation can't run two conflicting blocks at once. + const type = isBug && isFeature ? 'conflict' : isBug ? 'bug' : isFeature ? 'feature' : 'untyped'; + + switch (type) { + case 'conflict': + failures.push('**Labels** — an issue cannot be both `bug` and `enhancement`. Remove one label.'); + break; + + case 'bug': { + if (!section('Install Method')) { + failures.push('**Install Method** — select how you installed Odysseus'); + } + + if (!section('Operating System')) { + failures.push('**Operating System** — select your OS'); + } + + const stepsText = section('Steps to Reproduce'); + if (!stepsText || !/\d+\.|[-*]/.test(stepsText)) { + failures.push('**Steps to Reproduce** — must include at least one numbered or bulleted step'); + } + + if (section('Expected Behaviour').length < 10) { + failures.push('**Expected Behaviour** — section is empty or too short'); + } + + if (section('Actual Behaviour').length < 10) { + failures.push('**Actual Behaviour** — section is empty or too short'); + } + break; + } + + case 'feature': + if (!section('Area')) { + failures.push('**Area** — select which part of the application this affects'); + } + + if (section('Problem or Motivation').length < 20) { + failures.push( + '**Problem or Motivation** — section is empty or too short ' + + '(explain the concrete problem this solves)', + ); + } + + if (section('Proposed Solution').length < 20) { + failures.push( + '**Proposed Solution** — section is empty or too short ' + + '(describe the change you want to see)', + ); + } + + if (!section('Are you willing to implement this\\?')) { + failures.push('**Are you willing to implement this?** — select an option'); + } + break; + + // 'untyped' → only the common body-length check applies. + } + + // ── Labels ──────────────────────────────────────────────────────────────── + // These labels are expected to already exist in the repo — managing the + // repo's label set is the maintainer's job, not this workflow's. We check a + // label exists before applying it (issues.addLabels would otherwise silently + // create a missing label) and fail soft — warn and skip — if it's absent. + async function labelExists(name) { + try { + await github.rest.issues.getLabel({ owner, repo, name }); + return true; + } catch (e) { + if (e.status === 404) return false; + throw e; + } + } + + async function addLabel(name) { + if (await labelExists(name)) { + await github.rest.issues.addLabels({ owner, repo, issue_number: issue.number, labels: [name] }); + } else { + core.warning(`Label "${name}" does not exist in the repo — skipping. Create it once to enable labelling.`); + } + } + + async function dropLabel(name) { + try { + await github.rest.issues.removeLabel({ owner, repo, issue_number: issue.number, name }); + } catch (e) { + if (e.status !== 404 && e.status !== 410) throw e; + } + } + + // ── Find existing bot comment to update in-place ────────────────────────── + const MARKER = ''; + const { data: comments } = await github.rest.issues.listComments({ + owner, repo, issue_number: issue.number, + }); + const existing = comments.find(c => c.user.type === 'Bot' && c.body.includes(MARKER)); + + const LABEL_BAD = 'needs more info'; + const LABEL_GOOD = 'ready for review'; + + if (failures.length === 0) { + if (existing) { + await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id }); + } + + await dropLabel(LABEL_BAD); + await addLabel(LABEL_GOOD); + + } else { + const list = failures.map(f => `- ${f}`).join('\n'); + const commentBody = [ + MARKER, + '⚠️ **Issue description is incomplete.** Please update the following sections:', + '', + list, + '', + '_This comment is deleted automatically once all sections are complete._', + ].join('\n'); + + if (existing) { + await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: commentBody }); + } else { + await github.rest.issues.createComment({ owner, repo, issue_number: issue.number, body: commentBody }); + } + + await dropLabel(LABEL_GOOD); + await addLabel(LABEL_BAD); + + core.setFailed(`Issue description has ${failures.length} issue(s) — see bot comment for details.`); + } +}; diff --git a/.github/scripts/check-pr-description.js b/.github/scripts/check-pr-description.js new file mode 100644 index 0000000..277d789 --- /dev/null +++ b/.github/scripts/check-pr-description.js @@ -0,0 +1,121 @@ +// @ts-check +'use strict'; + +/** @param {{ github: import('@octokit/rest').Octokit, context: import('@actions/github').context, core: import('@actions/core') }} */ +module.exports = async ({ github, context, core }) => { + const body = context.payload.pull_request.body || ''; + const prNum = context.payload.pull_request.number; + const MARKER = ''; + const owner = context.repo.owner; + const repo = context.repo.repo; + + // Strip HTML comments so placeholder text does not count as content. + function strip(text) { + return (text ?? '').replace(//g, '').trim(); + } + + // Extract the text content of a Section. Matches any heading depth (#, ##, + // ###, …) so the check doesn't break if the template's heading level changes. + function section(heading) { + const m = body.match(new RegExp(`#+\\s+${heading}[\\s\\S]*?(?=\\n#+\\s+|$)`, 'i')); + return strip(m?.[0].replace(new RegExp(`#+\\s+${heading}`, 'i'), '') ?? ''); + } + + const problems = []; + + // 1. Summary must be filled in. + if (section('Summary').length < 20) { + problems.push('**Summary** is empty or too short — describe what changed and why.'); + } + + // 2. Linked Issue must reference a real issue. Accept a bare #NNN, a closing + // keyword + #NNN, or a full issue URL (e.g. .../issues/123) — the strict + // keyword-prefixed form previously false-flagged correctly-linked PRs. + const linkedSection = section('Linked Issue'); + const hasIssueRef = /#\d+/.test(linkedSection) || /\/issues\/\d+/.test(linkedSection); + if (!linkedSection || !hasIssueRef) { + problems.push('**Linked Issue** — add a reference like `Fixes #NNN`, a bare `#NNN`, or a link to the issue.'); + } + + // 3. At least one Type of Change box must be checked. + const typeBlock = body.match(/##\s+Type of Change[\s\S]*?(?=\n##\s|$)/i)?.[0] ?? ''; + if (!/- \[x\]/i.test(typeBlock)) { + problems.push('**Type of Change** — check at least one box.'); + } + + // 4. Duplicate-search checklist item must be checked. + if (!/- \[x\] I searched/i.test(body)) { + problems.push('**Checklist** — check the duplicate-search box to confirm you searched existing issues and PRs.'); + } + + // 5. How to Test must have at least one numbered step. + const howTo = section('How to Test'); + if (!howTo || !/\d+\.\s*\S/.test(howTo)) { + problems.push('**How to Test** — add at least one numbered step a reviewer can follow to verify this works.'); + } + + // ── Comment ────────────────────────────────────────────────────────────── + const comments = await github.paginate(github.rest.issues.listComments, { + owner, repo, issue_number: prNum, per_page: 100, + }); + const existing = comments.find(c => (c.body ?? '').includes(MARKER)); + + if (problems.length === 0) { + if (existing) { + await github.rest.issues.deleteComment({ owner, repo, comment_id: existing.id }); + } + } else { + const commentBody = [ + MARKER, + '⚠️ **PR description — action needed**', + '', + 'The following required sections are missing or incomplete. Please update the PR description to address them:', + '', + problems.map(p => `- ${p}`).join('\n'), + '', + '---', + '_This comment is deleted automatically once all sections are complete._', + ].join('\n'); + + if (existing) { + await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: commentBody }); + } else { + await github.rest.issues.createComment({ owner, repo, issue_number: prNum, body: commentBody }); + } + } + + // ── Labels ──────────────────────────────────────────────────────────────── + // These labels are expected to already exist in the repo — managing the + // repo's label set is the maintainer's job, not this workflow's. We check a + // label exists before applying it (issues.addLabels would otherwise silently + // create a missing label) and fail soft — warn and skip — if it's absent. + async function labelExists(name) { + try { + await github.rest.issues.getLabel({ owner, repo, name }); + return true; + } catch (e) { + if (e.status === 404) return false; + throw e; + } + } + + async function swapLabel(num, add, remove) { + if (await labelExists(add)) { + await github.rest.issues.addLabels({ owner, repo, issue_number: num, labels: [add] }); + } else { + core.warning(`Label "${add}" does not exist in the repo — skipping. Create it once to enable labelling.`); + } + try { + await github.rest.issues.removeLabel({ owner, repo, issue_number: num, name: remove }); + } catch (e) { + if (e.status !== 404 && e.status !== 410) throw e; + } + } + + if (problems.length === 0) { + await swapLabel(prNum, 'ready for review', 'needs work'); + } else { + await swapLabel(prNum, 'needs work', 'ready for review'); + core.setFailed(`PR description has ${problems.length} issue(s) — see bot comment for details.`); + } +}; diff --git a/.github/workflows/issue-description-check.yml b/.github/workflows/issue-description-check.yml new file mode 100644 index 0000000..5dc3fdf --- /dev/null +++ b/.github/workflows/issue-description-check.yml @@ -0,0 +1,23 @@ +name: ci / issue description check + +on: + issues: + types: [opened, edited, reopened] + +permissions: + issues: write + +jobs: + check: + name: Check issue description + runs-on: ubuntu-latest + # Skip bots (Dependabot, release-drafter, etc.) + if: ${{ github.event.issue.user.type != 'Bot' }} + steps: + - uses: actions/checkout@v4 + with: + sparse-checkout: .github/scripts + + - uses: actions/github-script@v7 + with: + script: return require('./.github/scripts/check-issue-description.js')({github, context, core}) diff --git a/.github/workflows/pr-description-check.yml b/.github/workflows/pr-description-check.yml new file mode 100644 index 0000000..9ac05b3 --- /dev/null +++ b/.github/workflows/pr-description-check.yml @@ -0,0 +1,28 @@ +name: ci / PR description check + +on: + pull_request_target: + types: [opened, edited, synchronize, reopened] + +# pull_request_target runs in the base-repo context (has secrets). +# The checkout below pins to the base branch so no fork code is executed. +# The script only reads context.payload and calls the GitHub API. +permissions: + issues: write + pull-requests: write + +jobs: + check-description: + name: Check PR description + runs-on: ubuntu-latest + # Skip bots — they open PRs programmatically and have their own process. + if: github.event.pull_request.user.type != 'Bot' + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.base_ref }} + sparse-checkout: .github/scripts + + - uses: actions/github-script@v7 + with: + script: return require('./.github/scripts/check-pr-description.js')({github, context, core})