diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6659ccac..67db43da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -211,8 +211,8 @@ jobs: run: cargo xtask web-offline-test - name: Affected-package selection self-test run: python3 -I scripts/ci/test_affected.py - - name: Claim bot tests - run: node --test scripts/claim/*.test.js + - name: Claim and triage bot tests + run: node --test scripts/claim/*.test.js scripts/triage/*.test.js clippy: name: Native Linux Clippy diff --git a/.github/workflows/claim.yml b/.github/workflows/claim.yml index 637253e9..f4158721 100644 --- a/.github/workflows/claim.yml +++ b/.github/workflows/claim.yml @@ -1,10 +1,15 @@ # `/claim` and `/unclaim` on issues (CONTRIBUTING.md, "Sending a change"), and a daily sweep that -# frees claims nobody is working on. The logic is in scripts/claim/claim.js (tested in ci.yml). +# frees claims nobody is working on. A pull request that references an issue (` (#N)`, +# `Fixes #N`) claims it too. The logic is in scripts/claim/claim.js (tested in ci.yml). name: Claim on: issue_comment: types: [created] + # pull_request_target so PRs from forks can label and assign. It runs the base branch's + # scripts/claim and only reads the PR's title, body and author; the PR's code is never checked out. + pull_request_target: + types: [opened, reopened] schedule: - cron: '17 5 * * *' workflow_dispatch: @@ -16,7 +21,7 @@ permissions: # One run at a time per issue, so two simultaneous /claim comments can't both win. concurrency: - group: claim-${{ github.event.issue.number || 'sweep' }} + group: claim-${{ github.event.issue.number || github.event.pull_request.number || 'sweep' }} cancel-in-progress: false jobs: @@ -38,6 +43,9 @@ jobs: const claim = require('./scripts/claim/claim.js'); if (context.eventName === 'issue_comment') { await claim.handleComment({ github, context }); + } else if (context.eventName === 'pull_request_target') { + const claimed = await claim.handlePull({ github, context }); + core.info(`Claimed: ${claimed.join(', ') || 'none'}`); } else { const released = await claim.sweep({ github, context }); core.info(`Released: ${released.join(', ') || 'none'}`); diff --git a/.github/workflows/triage.yml b/.github/workflows/triage.yml new file mode 100644 index 00000000..111b3ca1 --- /dev/null +++ b/.github/workflows/triage.yml @@ -0,0 +1,69 @@ +# Labels new issues and PRs from fixed rules, `/triage` for maintainers, and a manual backfill +# over everything still unlabelled (CONTRIBUTING.md, "Labels and triage"). The logic is in +# scripts/triage/triage.js (tested in ci.yml). +name: Triage + +on: + issues: + types: [opened, labeled] + issue_comment: + types: [created] + # pull_request_target so PRs from forks can be labelled. It runs the base branch's + # scripts/triage and only lists the PR's changed paths; the PR's code is never checked out. + pull_request_target: + types: [opened] + workflow_dispatch: + inputs: + dry_run: + description: 'Only report what would be labelled' + type: boolean + default: true + +permissions: + contents: read # to check out scripts/triage + issues: write + pull-requests: write + +concurrency: + group: triage-${{ github.event.issue.number || github.event.pull_request.number || 'backfill' }} + cancel-in-progress: false + +jobs: + triage: + name: Triage bot + runs-on: ubuntu-24.04 + timeout-minutes: 30 + # Filtered here so the common events (every comment, every label) start no runner. + if: >- + (github.event_name == 'issue_comment' && + github.event.comment.user.type != 'Bot' && startsWith(github.event.comment.body, '/triage')) || + (github.event.action == 'labeled' && startsWith(github.event.label.name, 'priority:') && + contains(github.event.issue.labels.*.name, 'needs-triage')) || + (github.event_name != 'issue_comment' && github.event.action != 'labeled') + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + sparse-checkout: scripts/triage + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + DRY_RUN: ${{ inputs.dry_run }} + with: + script: | + const triage = require('./scripts/triage/triage.js'); + const event = context.eventName; + if (event === 'issue_comment') { + core.info(await triage.handleComment({ github, context })); + } else if (event === 'issues' && context.payload.action === 'labeled') { + await triage.handleLabeled({ github, context }); + } else if (event === 'workflow_dispatch') { + const dryRun = process.env.DRY_RUN !== 'false'; + const plan = await triage.backfill({ github, context, dryRun }); + const esc = (t) => String(t).replace(/[&<>"]/g, (c) => ({ '&': '&', '<': '<', '>': '>', '"': '"' })[c]); + const rows = plan.map((p) => [`#${p.number}`, p.pull ? 'PR' : 'issue', esc(p.labels.join(', ') || '—'), esc(p.title)]); + await core.summary + .addHeading(dryRun ? `Backfill dry run: ${plan.length} unlabelled` : `Backfill: ${plan.length} labelled`) + .addTable([[{ data: 'Item', header: true }, { data: 'Kind', header: true }, { data: 'Labels', header: true }, { data: 'Title', header: true }], ...rows]) + .write(); + } else { + core.info(`Labelled: ${(await triage.handleOpened({ github, context })).join(', ') || 'nothing'}`); + } diff --git a/CLAUDE.md b/CLAUDE.md index 6dc6d708..26b27e44 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,7 +129,8 @@ People trust SolidCraft with their designs; a crash loses their work. **This out - Before working on issue #N, check nobody has it: `gh pr list -R bherbruck/solidcraft --state open --search "#N"`. - If none, branch from the latest `origin/main`, `git commit --allow-empty -m "claim: #N"`, push, and open a **draft** PR titled ` (#N)` right away. **The lowest-numbered open PR for an - issue wins**; if yours lost, close it. Don't claim by commenting or by editing files. + issue wins**; if yours lost, close it. Opening the PR is the claim: the bot assigns you and + labels the issue `in progress` (or says on your PR who holds it). Don't claim by editing files. - Push work in progress at least daily. A draft with no pushes for 72 hours may be taken over: comment on it first, then open your own claim (you may start from its branch). - On a shared build machine, check the load before a big build (`cut -d' ' -f1 /proc/loadavg`) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5ab2288..0f6037b4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -258,9 +258,10 @@ and a fresh CI run: open a **draft** PR titled ` (#N)`, e.g. `Fillet: variable radius along a chain (#123)`. - **The lowest-numbered open PR for an issue wins.** If yours lost, close it (or offer to help on the other one). - - Then comment `/claim` on the issue so the bot records the claimant and adds the `in progress` - label. **Do both:** the draft PR establishes ownership and `/claim` marks the issue as taken. - Reference `#N` in the PR so the bot can find it; a comment alone does not replace the draft PR. + - The bot records the claim when the PR is opened: it assigns you, adds `in progress` and + says so on the issue. It reads `#N` in the PR title and `Fixes #N` in the body. If someone + already holds the issue, or an older open PR references it, it says so on your PR instead. + - Want to stake a claim before you have a branch? Comment `/claim` on the issue (below). - Push work in progress at least daily. A draft with no pushes for 72 hours may be taken over: comment on it first, then open your own claim. - Mark the PR ready for review when it is done. @@ -270,10 +271,10 @@ and a fresh CI run: 4. Reference the issue (`Fixes #12`). In the PR, say what you tested and how. For geometry work, include the measured numbers (and the expected ones). -The `/claim` bot assigns you, adds the `in progress` label, and replies with its automatic cleanup +Commenting `/claim` assigns you, adds the `in progress` label, and replies with its automatic cleanup deadline. It frees a claim with no linked PR after 72 hours, or a claim with a PR after 14 days without pushes. These automatic cleanup deadlines are separate from the daily progress and -72-hour takeover policy above; follow the draft PR then `/claim` sequence before starting work. +72-hour takeover policy above. A draft PR that references the issue claims it the same way. `/unclaim` releases the bot claim early (only you or a maintainer can). Issues a maintainer has taken are never freed automatically. @@ -303,6 +304,21 @@ owning one area, with a human setting direction and reporting bugs. Their workin [AGENTS.md](AGENTS.md). Human contributors are just as welcome. Use whatever tools you like, as long as the change meets the rules above. +## Labels and triage + +The triage bot labels every new issue and PR (`.github/workflows/triage.yml`). It only adds labels; +correct them freely and it won't put them back. + +- **PRs** get `area:*` from the paths they change (`crates/kernel` and `crates/geom` → + `area:kernel`, `crates/ui-egui` and the apps → `area:ui`, `.github/`, `scripts/`, `docs/` → + `area:infra`, and so on). +- **Issues** get `bug` (in-app reports and the bug template), `enhancement` (feature requests), + `priority:P1` for crashes, and an `area:*` from the title or the reported command. Until it has + a priority, an issue carries `needs-triage`; setting a priority removes it. +- **Maintainers** comment `/triage area:kernel P1` (any existing label; `P0`–`P3` for a + priority, which replaces the old one; `-label` removes one) to sort an issue or PR. +- `needs-triage` is the queue: [open issues with it](https://github.com/tangency-labs/solidcraft/issues?q=is%3Aopen+label%3Aneeds-triage). + ## Where help is wanted most **Kernel speed (slow fillets, booleans, STEP):** please don't rewrite the current kernel code for speed; it is being replaced by [solidcore](https://github.com/bherbruck/solidcore) ([progress](https://tangency-labs.github.io/solidcore/)). Report slow cases with a file under the `kernel-perf` label, and put kernel effort into solidcore instead. See [#1597](https://github.com/tangency-labs/solidcraft/issues/1597). diff --git a/scripts/claim/claim.js b/scripts/claim/claim.js index 8cfd1955..ef9c366d 100644 --- a/scripts/claim/claim.js +++ b/scripts/claim/claim.js @@ -251,6 +251,79 @@ async function handleComment({ github, context, now = Date.now() }) { return cmd; } +// Issue numbers a PR claims: `#N` in its title (` (#N)`) and closing keywords in its body. +const MAX_REFS = 5; +function referencedIssues(title, body) { + const found = new Set(); + for (const m of String(title || '').matchAll(/(?:^|[\s(])#(\d{1,7})\b/g)) found.add(Number(m[1])); + const closing = /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s+#(\d{1,7})\b/gi; + for (const m of String(body || '').matchAll(closing)) found.add(Number(m[1])); + return [...found].filter((n) => n > 0).slice(0, MAX_REFS); +} + +// A pull request was opened (or its title or body edited): it claims each open issue it +// references that nobody holds yet, so a draft PR alone is enough (CONTRIBUTING.md). +async function handlePull({ github, context }) { + const pull = context.payload.pull_request; + if (!pull || pull.state !== 'open') return []; + const author = pull.user && pull.user.login; + if (!author || pull.user.type === 'Bot' || author.endsWith('[bot]')) return []; + const claimed = []; + for (const n of referencedIssues(pull.title, pull.body)) { + if (n === pull.number) continue; + let issue; + try { + ({ data: issue } = await github.rest.issues.get({ ...repoArgs(context), issue_number: n })); + } catch (e) { + if (e.status === 404 || e.status === 410) continue; + throw e; + } + if (issue.pull_request || issue.state !== 'open' || issue.locked) continue; + const assignees = (issue.assignees || []).map((a) => a.login); + if (assignees.includes(author)) { + if (!hasLabel(issue)) { + await ensureLabel(github, context); + await github.rest.issues.addLabels({ ...repoArgs(context), issue_number: n, labels: [LABEL] }); + } + continue; + } + if (assignees.length || hasLabel(issue)) { + const who = assignees.length ? assignees.map((a) => '@' + a).join(', ') : 'someone else'; + await github.rest.issues.createComment({ + ...repoArgs(context), + issue_number: pull.number, + body: `#${n} is already claimed by ${who}. If this PR duplicates their work, please close it or offer to help there.`, + }); + continue; + } + // An older open PR for the issue wins (lowest number), even if it never recorded its claim. + const timeline = await getTimeline(github, context, n); + const older = linkedPulls(timeline).find((p) => p.number < pull.number && p.user && p.user.login !== author); + if (older) { + await github.rest.issues.createComment({ + ...repoArgs(context), + issue_number: pull.number, + body: `#${older.number} claimed #${n} first (the lowest-numbered open PR wins). Please close this one or offer to help there.`, + }); + continue; + } + await ensureLabel(github, context); + await github.rest.issues.addLabels({ ...repoArgs(context), issue_number: n, labels: [LABEL] }); + try { + await github.rest.issues.addAssignees({ ...repoArgs(context), issue_number: n, assignees: [author] }); + } catch (e) { + // Assignment can be refused; the label and the comment naming them still stand. + } + await github.rest.issues.createComment({ + ...repoArgs(context), + issue_number: n, + body: `Claimed by @${author} through #${pull.number}. The claim lapses after ${PR_IDLE_DAYS} days without pushes.`, + }); + claimed.push(n); + } + return claimed; +} + // The daily sweep over open issues labelled `in progress`. async function sweep({ github, context, now = Date.now() }) { const issues = await github.paginate(github.rest.issues.listForRepo, { @@ -286,9 +359,11 @@ async function sweep({ github, context, now = Date.now() }) { module.exports = { LABEL, parseCommand, + referencedIssues, isMaintainer, linkedPulls, claimStatus, handleComment, + handlePull, sweep, }; diff --git a/scripts/claim/claim.test.js b/scripts/claim/claim.test.js index f2fe47df..424e0178 100644 --- a/scripts/claim/claim.test.js +++ b/scripts/claim/claim.test.js @@ -39,6 +39,11 @@ function fake(world = {}) { removeAssignees: rec('removeAssignees'), createComment: rec('comment'), listForRepo: async () => ({ data: world.issues || [] }), + get: async ({ issue_number }) => { + const found = (world.byNumber || {})[issue_number]; + if (!found) throw Object.assign(new Error('nf'), { status: 404 }); + return { data: found }; + }, listEventsForTimeline: async () => ({ data: world.timeline || [] }), }, }, @@ -231,3 +236,62 @@ test('sweep is idempotent: a released issue is no longer listed', async () => { world.issues = []; assert.deepEqual(await claim.sweep({ github: w.github, context: { repo }, now: NOW }), []); }); + +test('referencedIssues reads the title and closing keywords, capped', () => { + assert.deepEqual(claim.referencedIssues('Fillet: chain (#123)', 'Fixes #9, closes: #10, see #11'), [123, 9, 10]); + assert.deepEqual(claim.referencedIssues('no refs', 'mentions #4 only'), []); + assert.deepEqual(claim.referencedIssues('#1 #2 #3 #4 #5 #6 #7', ''), [1, 2, 3, 4, 5]); + assert.deepEqual(claim.referencedIssues(undefined, null), []); +}); + +const openIssue = (number, assignees = [], labels = []) => ({ + number, + state: 'open', + locked: false, + assignees: assignees.map((login) => ({ login })), + labels: labels.map((name) => ({ name })), +}); +const pullCtx = (number, title, body = '', login = 'alice') => ({ + repo, + payload: { pull_request: { number, state: 'open', title, body, user: { login, type: 'User' } } }, +}); + +test('a PR referencing a free issue claims it for the author', async () => { + const { github, calls } = fake({ byNumber: { 7: openIssue(7) } }); + assert.deepEqual(await claim.handlePull({ github, context: pullCtx(50, 'Thing (#7)') }), [7]); + assert.deepEqual(find(calls, 'addAssignees')[1], { ...repo, issue_number: 7, assignees: ['alice'] }); + assert.deepEqual(find(calls, 'addLabels')[1].labels, ['in progress']); + assert.match(find(calls, 'comment')[1].body, /Claimed by @alice through #50/); +}); + +test('a PR on an issue someone else holds comments on the PR and changes nothing', async () => { + const { github, calls } = fake({ byNumber: { 7: openIssue(7, ['bob'], ['in progress']) } }); + assert.deepEqual(await claim.handlePull({ github, context: pullCtx(50, 'Thing (#7)') }), []); + assert.equal(find(calls, 'addAssignees'), undefined); + const c = find(calls, 'comment')[1]; + assert.equal(c.issue_number, 50); + assert.match(c.body, /@bob/); +}); + +test('the lowest-numbered open PR wins an unrecorded claim', async () => { + const { github, calls } = fake({ byNumber: { 7: openIssue(7) }, timeline: [pr(42, 'bob')] }); + assert.deepEqual(await claim.handlePull({ github, context: pullCtx(50, 'Thing (#7)') }), []); + assert.match(find(calls, 'comment')[1].body, /#42 claimed #7 first/); +}); + +test('the claimant\'s own PR just makes sure the label is there', async () => { + const { github, calls } = fake({ byNumber: { 7: openIssue(7, ['alice']) } }); + assert.deepEqual(await claim.handlePull({ github, context: pullCtx(50, 'Thing (#7)') }), []); + assert.deepEqual(names(calls), ['addLabels']); +}); + +test('PRs from bots, references to PRs, closed and missing issues are ignored', async () => { + const w = fake({ + byNumber: { 7: { ...openIssue(7), pull_request: {} }, 8: { ...openIssue(8), state: 'closed' } }, + }); + assert.deepEqual(await claim.handlePull({ github: w.github, context: pullCtx(50, 'x (#7) (#8) (#9)') }), []); + const bot = pullCtx(51, 'x (#1)'); + bot.payload.pull_request.user = { login: 'dependabot[bot]', type: 'Bot' }; + assert.deepEqual(await claim.handlePull({ github: w.github, context: bot }), []); + assert.equal(w.calls.length, 0); +}); diff --git a/scripts/triage/triage.js b/scripts/triage/triage.js new file mode 100644 index 00000000..f00a13ce --- /dev/null +++ b/scripts/triage/triage.js @@ -0,0 +1,304 @@ +// The triage bot's logic, used by .github/workflows/triage.yml through actions/github-script. +// New issues and PRs get `area:*` and type labels from fixed rules; an issue nobody has given a +// priority yet carries `needs-triage`. Maintainers set labels with `/triage`. The rules only ever +// add labels, so a person's correction is never undone. +// Everything takes the github-script `github` client and `context`, so the tests can pass fakes. +'use strict'; + +const NEEDS_TRIAGE = 'needs-triage'; +const NEEDS_TRIAGE_COLOR = 'ededed'; +const NEEDS_TRIAGE_DESCRIPTION = 'no priority yet; a maintainer sorts it with /triage'; +const MAX_TOKENS = 10; +const MAX_TEXT = 20000; + +// Changed paths of a PR → area. First match wins per file. +const PATH_AREAS = [ + [/^crates\/sketch\//, 'area:sketch'], + [/^crates\/(kernel|geom)\//, 'area:kernel'], + [/^crates\/doc\//, 'area:doc'], + [/^crates\/io\//, 'area:io'], + [/^crates\/(render|gpu)\//, 'area:render'], + [/^(crates\/(ui-egui|space-mouse|threedx-client)|apps\/(solidcraft|solidcraft-web))\//, 'area:ui'], + [/^(\.github\/|xtask\/|scripts\/|tools\/|docs\/|packaging\/|[^/]+\.md$)/, 'area:infra'], +]; + +// Command id prefix (the in-app reports quote it) → area. +const COMMAND_AREAS = { + sketch: 'area:sketch', + solid: 'area:kernel', + surface: 'area:kernel', + sheet: 'area:kernel', + plastic: 'area:kernel', + construct: 'area:kernel', + body: 'area:kernel', + component: 'area:doc', + joint: 'area:doc', + parameters: 'area:doc', + timeline: 'area:doc', + occurrence: 'area:doc', + motion: 'area:doc', + contact: 'area:doc', + explode: 'area:doc', + file: 'area:io', + render: 'area:render', + appearance: 'area:render', + view: 'area:ui', + browser: 'area:ui', + selection: 'area:ui', +}; + +// Words in an issue's title → area. Conservative: a miss leaves the issue for a person. +const WORD_AREAS = [ + [/\b(sketch|constraint|dimension|solver|spline)/i, 'area:sketch'], + [/\b(fillet|chamfer|boolean|shell|loft|sweep|revolve|extrude|thread|hole|b-?rep|combine|offset|mirror|kernel|pattern)(s|ed|ing)?\b/i, 'area:kernel'], + [/\b(step|iges|3mf|stl|gltf|dxf|obj|import|export)\b/i, 'area:io'], + [/\b(render|gpu|wgpu|shader|opengl|vulkan|metal|appearance)\b/i, 'area:render'], + [/\b(joint|assembly|timeline|parameter|component)/i, 'area:doc'], + [/\b(toolbar|dialog|viewport|panel|button|shortcut|menu|window|space ?mouse|3dconnexion|spacenavd)/i, 'area:ui'], +]; + +const CRASH = /^\s*crash\b|\bpanic(ked)?\b|\bcaught panic\b/i; +const FEATURE = /^\s*\[?feature( request)?\]?\b/i; +// The in-app reports: `: operation failed` / `Modeling: unexpected result`. +const IN_APP_FAILURE = /:\s*(operation failed|unexpected result)\b/i; + +function repoArgs(context) { + return { owner: context.repo.owner, repo: context.repo.repo }; +} + +// The text under a `## ` heading, up to the next heading. +function section(body, name) { + const lines = body.split('\n'); + const start = lines.findIndex((l) => new RegExp(`^##\\s*${name}\\s*$`, 'i').test(l.trim())); + if (start < 0) return ''; + const rest = lines.slice(start + 1); + const end = rest.findIndex((l) => /^##\s/.test(l)); + return (end < 0 ? rest : rest.slice(0, end)).join('\n'); +} + +function labelNames(item) { + return (item.labels || []).map((l) => (typeof l === 'string' ? l : l.name)); +} + +function hasPriority(names) { + return names.some((n) => n.startsWith('priority:')); +} + +// Labels for an issue from its title and body. +function classifyIssue(title, body) { + const t = String(title || '').slice(0, 500); + const b = String(body || '').slice(0, MAX_TEXT); + const labels = new Set(); + if (CRASH.test(t)) { + labels.add('bug'); + labels.add('priority:P1'); // a crash loses work (CLAUDE.md, "Never crash") + } else if (IN_APP_FAILURE.test(t) || /^##\s*(behavior|actual behavior)\b/im.test(b)) { + labels.add('bug'); + } else if (FEATURE.test(t)) { + labels.add('enhancement'); + } + // The title names the feature; the command quoted in the report's own section is the fallback + // (a report's history quotes earlier commands too, so the rest of the body is not read). + let area = null; + for (const [re, a] of WORD_AREAS) { + if (re.test(t)) { + area = a; + break; + } + } + if (!area) { + const own = `${t}\n${section(b, 'Settings and selection')}\n${section(b, 'Command')}`; + const m = /["`]([a-z_]+)\.[a-z_][a-z_.]*["`]/.exec(own); + area = (m && COMMAND_AREAS[m[1]]) || null; + } + if (area) labels.add(area); + return [...labels]; +} + +// Labels for a PR from its changed paths. +function classifyPaths(paths) { + const labels = new Set(); + for (const p of paths) { + for (const [re, area] of PATH_AREAS) { + if (re.test(p)) { + labels.add(area); + break; + } + } + } + return [...labels].sort(); +} + +async function ensureNeedsTriage(github, context) { + try { + await github.rest.issues.getLabel({ ...repoArgs(context), name: NEEDS_TRIAGE }); + } catch (e) { + if (e.status !== 404) throw e; + try { + await github.rest.issues.createLabel({ + ...repoArgs(context), + name: NEEDS_TRIAGE, + color: NEEDS_TRIAGE_COLOR, + description: NEEDS_TRIAGE_DESCRIPTION, + }); + } catch (e2) { + if (e2.status !== 422) throw e2; // created meanwhile + } + } +} + +async function pullPaths(github, context, number) { + const files = await github.paginate(github.rest.pulls.listFiles, { + ...repoArgs(context), + pull_number: number, + per_page: 100, + }); + return files.map((f) => f.filename); +} + +// The labels the rules would add to an issue or PR that it doesn't already carry. +async function proposal(github, context, item) { + const current = labelNames(item); + let wanted; + if (item.pull_request) { + wanted = classifyPaths(await pullPaths(github, context, item.number)); + } else { + wanted = classifyIssue(item.title, item.body); + if (!hasPriority(current) && !wanted.some((l) => l.startsWith('priority:'))) wanted.push(NEEDS_TRIAGE); + } + return wanted.filter((l) => !current.includes(l)); +} + +async function apply(github, context, number, labels) { + if (!labels.length) return; + if (labels.includes(NEEDS_TRIAGE)) await ensureNeedsTriage(github, context); + await github.rest.issues.addLabels({ ...repoArgs(context), issue_number: number, labels }); +} + +// issues.opened / pull_request_target.opened. +async function handleOpened({ github, context }) { + const issue = context.payload.issue; + const pull = context.payload.pull_request; + const item = issue || (pull && { ...pull, pull_request: {} }); + if (!item || item.state !== 'open') return []; + const labels = await proposal(github, context, item); + await apply(github, context, item.number, labels); + return labels; +} + +// issues.labeled: a priority settles triage. +async function handleLabeled({ github, context }) { + const issue = context.payload.issue; + const label = context.payload.label; + if (!issue || !label || !label.name.startsWith('priority:')) return false; + if (!labelNames(issue).includes(NEEDS_TRIAGE)) return false; + try { + await github.rest.issues.removeLabel({ ...repoArgs(context), issue_number: issue.number, name: NEEDS_TRIAGE }); + } catch (e) { + if (e.status !== 404) throw e; + } + return true; +} + +// `/triage area:kernel P1 bug -enhancement`: tokens name existing labels; `P1` is short for +// `priority:P1` and replaces any other priority; a leading `-` removes. +function parseTriage(body) { + const line = String(body || '').trimStart().split('\n')[0]; + const m = /^\/triage(\s.*)?$/.exec(line.trimEnd()); + if (!m) return null; + const add = []; + const remove = []; + for (const raw of (m[1] || '').trim().split(/\s+/).filter(Boolean).slice(0, MAX_TOKENS)) { + const minus = raw.startsWith('-'); + let name = minus ? raw.slice(1) : raw; + if (/^p[0-3]$/i.test(name)) name = 'priority:' + name.toUpperCase(); + if (name) (minus ? remove : add).push(name); + } + return { add, remove }; +} + +async function isMaintainer(github, context, login) { + if (login === context.repo.owner) return true; + try { + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ ...repoArgs(context), username: login }); + return ['admin', 'write', 'maintain'].includes(data.permission); + } catch (e) { + return false; + } +} + +async function react(github, context, content) { + await github.rest.reactions.createForIssueComment({ + ...repoArgs(context), + comment_id: context.payload.comment.id, + content, + }); +} + +// issue_comment.created. +async function handleComment({ github, context }) { + const { issue, comment } = context.payload; + if (!issue || !comment || comment.user.type === 'Bot' || comment.user.login.endsWith('[bot]')) return 'skip'; + const cmd = parseTriage(comment.body); + if (!cmd) return 'skip'; + if (!(await isMaintainer(github, context, comment.user.login))) { + await react(github, context, 'eyes'); + return 'denied'; + } + const known = new Set( + (await github.paginate(github.rest.issues.listLabelsForRepo, { ...repoArgs(context), per_page: 100 })).map((l) => l.name), + ); + const unknown = [...cmd.add, ...cmd.remove].filter((n) => !known.has(n)); + if (unknown.length) { + await react(github, context, 'confused'); + await github.rest.issues.createComment({ + ...repoArgs(context), + issue_number: issue.number, + body: `No such label: ${unknown.map((n) => '`' + n + '`').join(', ')}. Nothing was changed.`, + }); + return 'unknown'; + } + const current = labelNames(issue); + const remove = new Set(cmd.remove); + if (cmd.add.some((n) => n.startsWith('priority:'))) { + for (const n of current) if (n.startsWith('priority:') && !cmd.add.includes(n)) remove.add(n); + } + if (cmd.add.length || remove.size) remove.add(NEEDS_TRIAGE); + for (const name of remove) { + if (!current.includes(name)) continue; + await github.rest.issues.removeLabel({ ...repoArgs(context), issue_number: issue.number, name }); + } + const add = cmd.add.filter((n) => !current.includes(n)); + if (add.length) await github.rest.issues.addLabels({ ...repoArgs(context), issue_number: issue.number, labels: add }); + await react(github, context, '+1'); + return 'triage'; +} + +// Workflow dispatch: the rules over every open issue and PR with no labels at all. With `dryRun` +// it changes nothing and returns what it would do. +async function backfill({ github, context, dryRun = true }) { + const items = await github.paginate(github.rest.issues.listForRepo, { + ...repoArgs(context), + state: 'open', + per_page: 100, + }); + const plan = []; + for (const item of items) { + if (labelNames(item).length || item.locked) continue; + const labels = await proposal(github, context, item); + plan.push({ number: item.number, pull: Boolean(item.pull_request), title: item.title, labels }); + if (!dryRun) await apply(github, context, item.number, labels); + } + return plan; +} + +module.exports = { + NEEDS_TRIAGE, + classifyIssue, + classifyPaths, + parseTriage, + handleOpened, + handleLabeled, + handleComment, + backfill, +}; diff --git a/scripts/triage/triage.test.js b/scripts/triage/triage.test.js new file mode 100644 index 00000000..d16a681c --- /dev/null +++ b/scripts/triage/triage.test.js @@ -0,0 +1,189 @@ +'use strict'; +const test = require('node:test'); +const assert = require('node:assert/strict'); +const triage = require('./triage.js'); + +const repo = { owner: 'tangency-labs', repo: 'solidcraft' }; + +// A fake github-script client that records writes. `world` describes the repo. +function fake(world = {}) { + const calls = []; + const rec = (name) => async (args) => { + calls.push([name, args]); + return { data: {} }; + }; + const github = { + paginate: async (fn, args) => (await fn(args)).data, + rest: { + repos: { + getCollaboratorPermissionLevel: async ({ username }) => ({ + data: { permission: (world.perms || {})[username] || 'none' }, + }), + }, + pulls: { listFiles: async () => ({ data: (world.files || []).map((filename) => ({ filename })) }) }, + reactions: { createForIssueComment: rec('react') }, + issues: { + getLabel: async () => { + if (world.labelMissing) throw Object.assign(new Error('nf'), { status: 404 }); + return { data: {} }; + }, + createLabel: rec('createLabel'), + addLabels: rec('addLabels'), + removeLabel: rec('removeLabel'), + createComment: rec('comment'), + listForRepo: async () => ({ data: world.items || [] }), + listLabelsForRepo: async () => ({ + data: (world.labels || ['bug', 'enhancement', 'area:kernel', 'priority:P1', 'priority:P2', 'needs-triage']).map( + (name) => ({ name }), + ), + }), + }, + }, + }; + return { github, calls }; +} + +const find = (calls, n) => calls.find((c) => c[0] === n); +const all = (calls, n) => calls.filter((c) => c[0] === n); +const labels = (names) => names.map((name) => ({ name })); + +test('classifyIssue: in-app failure reports get bug and the area from the title', () => { + assert.deepEqual(triage.classifyIssue('FILLET: operation failed', '## Actual behavior\nx'), ['bug', 'area:kernel']); + assert.deepEqual(triage.classifyIssue('CREATE SKETCH: operation failed', ''), ['bug', 'area:sketch']); +}); + +test('classifyIssue: the report\'s own command is the fallback, not its history', () => { + const body = '## Recent commands\n"sketch.line"\n## Settings and selection\n [["file.export", {}]]\n## Environment\n'; + assert.deepEqual(triage.classifyIssue('Modeling: unexpected result', body), ['bug', 'area:io']); + assert.deepEqual(triage.classifyIssue('Modeling: unexpected result', '## Recent commands\n"sketch.line"\n'), ['bug']); +}); + +test('classifyIssue: crashes are P1 bugs, the command named in the title gives the area', () => { + assert.deepEqual(triage.classifyIssue('Crash: ended without a clean exit while running `solid.loft`', ''), [ + 'bug', + 'priority:P1', + 'area:kernel', + ]); + assert.deepEqual(triage.classifyIssue('Joining 90 segments: caught panic at truck', ''), ['bug', 'priority:P1']); +}); + +test('classifyIssue: feature requests and unmatched text', () => { + assert.deepEqual(triage.classifyIssue('Feature request: macOS please make use of menubar', ''), ['enhancement', 'area:ui']); + assert.deepEqual(triage.classifyIssue('Refactor with types', 'words'), []); + assert.deepEqual(triage.classifyIssue(undefined, undefined), []); +}); + +test('classifyPaths maps changed files to areas', () => { + assert.deepEqual( + triage.classifyPaths(['crates/kernel/src/a.rs', 'crates/geom/b.rs', 'crates/ui-egui/c.rs', 'README.md', 'Cargo.lock']), + ['area:infra', 'area:kernel', 'area:ui'], + ); + assert.deepEqual(triage.classifyPaths(['crates/engine/src/cmd/x.rs']), []); +}); + +test('parseTriage', () => { + assert.deepEqual(triage.parseTriage('/triage area:kernel p1 -enhancement'), { + add: ['area:kernel', 'priority:P1'], + remove: ['enhancement'], + }); + assert.deepEqual(triage.parseTriage('/triage'), { add: [], remove: [] }); + assert.equal(triage.parseTriage('/triaged'), null); + assert.equal(triage.parseTriage('please /triage'), null); + assert.equal(triage.parseTriage('/triage ' + 'bug '.repeat(50)).add.length, 10); +}); + +test('an opened issue gets its labels plus needs-triage, which is created if missing', async () => { + const { github, calls } = fake({ labelMissing: true }); + const issue = { number: 5, state: 'open', title: 'SHELL: operation failed', body: '', labels: [] }; + assert.deepEqual(await triage.handleOpened({ github, context: { repo, payload: { issue } } }), [ + 'bug', + 'area:kernel', + 'needs-triage', + ]); + assert.ok(find(calls, 'createLabel')); + assert.deepEqual(find(calls, 'addLabels')[1].labels, ['bug', 'area:kernel', 'needs-triage']); +}); + +test('an opened issue keeps the labels it was filed with and skips needs-triage with a priority', async () => { + const { github, calls } = fake(); + const issue = { number: 5, state: 'open', title: 'Crash: x', body: '', labels: labels(['bug']) }; + assert.deepEqual(await triage.handleOpened({ github, context: { repo, payload: { issue } } }), ['priority:P1']); + assert.deepEqual(find(calls, 'addLabels')[1].labels, ['priority:P1']); +}); + +test('an opened PR is labelled from its paths and never gets needs-triage', async () => { + const { github, calls } = fake({ files: ['crates/sketch/src/solve.rs'] }); + const pull_request = { number: 9, state: 'open', title: 'Sketch: thing', body: '', labels: [] }; + assert.deepEqual(await triage.handleOpened({ github, context: { repo, payload: { pull_request } } }), ['area:sketch']); + assert.deepEqual(find(calls, 'addLabels')[1], { ...repo, issue_number: 9, labels: ['area:sketch'] }); +}); + +test('a priority label clears needs-triage', async () => { + const { github, calls } = fake(); + const issue = { number: 5, labels: labels(['needs-triage', 'priority:P2']) }; + assert.equal(await triage.handleLabeled({ github, context: { repo, payload: { issue, label: { name: 'priority:P2' } } } }), true); + assert.equal(find(calls, 'removeLabel')[1].name, 'needs-triage'); + const other = fake(); + assert.equal( + await triage.handleLabeled({ github: other.github, context: { repo, payload: { issue, label: { name: 'bug' } } } }), + false, + ); + assert.equal(other.calls.length, 0); +}); + +const comment = (body, login = 'carol', issueLabels = []) => ({ + repo, + payload: { + issue: { number: 5, labels: labels(issueLabels) }, + comment: { id: 1, body, user: { login, type: 'User' } }, + }, +}); + +test('/triage by a maintainer sets labels, replaces the priority and clears needs-triage', async () => { + const { github, calls } = fake({ perms: { carol: 'write' } }); + const ctx = comment('/triage area:kernel P1', 'carol', ['priority:P2', 'needs-triage', 'bug']); + assert.equal(await triage.handleComment({ github, context: ctx }), 'triage'); + assert.deepEqual(all(calls, 'removeLabel').map((c) => c[1].name).sort(), ['needs-triage', 'priority:P2']); + assert.deepEqual(find(calls, 'addLabels')[1].labels, ['area:kernel', 'priority:P1']); + assert.equal(find(calls, 'react')[1].content, '+1'); +}); + +test('/triage from a non-maintainer or with an unknown label changes nothing', async () => { + let w = fake(); + assert.equal(await triage.handleComment({ github: w.github, context: comment('/triage bug', 'mallory') }), 'denied'); + assert.equal(find(w.calls, 'addLabels'), undefined); + w = fake({ perms: { carol: 'admin' } }); + assert.equal(await triage.handleComment({ github: w.github, context: comment('/triage area:nope') }), 'unknown'); + assert.equal(find(w.calls, 'addLabels'), undefined); + assert.match(find(w.calls, 'comment')[1].body, /area:nope/); +}); + +test('bots and other comments are ignored', async () => { + const { github, calls } = fake(); + const bot = comment('/triage bug'); + bot.payload.comment.user = { login: 'x[bot]', type: 'Bot' }; + assert.equal(await triage.handleComment({ github, context: bot }), 'skip'); + assert.equal(await triage.handleComment({ github, context: comment('hello') }), 'skip'); + assert.equal(calls.length, 0); +}); + +test('backfill only touches unlabelled items, and the dry run writes nothing', async () => { + const items = [ + { number: 1, title: 'EXTRUDE: operation failed', body: '', labels: [] }, + { number: 2, title: 'Already sorted', body: '', labels: labels(['bug']) }, + { number: 3, title: 'x', body: '', labels: [], pull_request: {} }, + ]; + let w = fake({ items, files: ['crates/io/src/step.rs'] }); + const plan = await triage.backfill({ github: w.github, context: { repo }, dryRun: true }); + assert.deepEqual( + plan.map((p) => [p.number, p.labels]), + [ + [1, ['bug', 'area:kernel', 'needs-triage']], + [3, ['area:io']], + ], + ); + assert.equal(w.calls.length, 0); + w = fake({ items, files: ['crates/io/src/step.rs'] }); + await triage.backfill({ github: w.github, context: { repo }, dryRun: false }); + assert.deepEqual(all(w.calls, 'addLabels').map((c) => c[1].issue_number), [1, 3]); +});