diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 7965572..535e0e4 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,4 +1,8 @@ - + ## Family @@ -8,17 +12,24 @@ ## Checklist -- [ ] `uv run bench2 validate ` **passes locally** (CI re-runs it) +- [ ] `uv run bench2 validate ` **passes locally** (CI re-runs it) — + [the contributor loop](https://github.com/BenchCAD-org/benchcad-2/blob/main/CONTRIBUTING.md#the-contributor-loop) · + [red-CI debugging](https://github.com/BenchCAD-org/benchcad-2/blob/main/docs/DEBUGGING.md) - [ ] I ran `uv run bench2 preview `, **looked at every image**, and **committed all three renders** — `preview.png` (easy/medium/hard grid), `preview_views.png` (the four benchmark views), `preview_extremes.png` (min & max draw). The part matches the issue's reference drawing across all tiers, and both extremes — including the hard/largest draw — are sane -- [ ] Multi-body part? `family.json` declares `"solids": N` (single-solid: omit) +- [ ] Multi-body part? `family.json` declares `"solids": N` (single-solid: omit) — + [assembly spec](https://github.com/BenchCAD-org/benchcad-2/blob/main/docs/DESIGN_SPEC.md) - [ ] Every `PARAM_SPEC.source` and every `check()` constraint cites a real rule/table, or honestly says `"proportion"` — **nothing fabricated** -- [ ] PR touches only `designs//` (one family = one PR) -- [ ] Commits are DCO-signed (`git commit -s`) + ([rule 10](https://github.com/BenchCAD-org/benchcad-2/blob/main/CONTRIBUTING.md#ten-rules)) +- [ ] PR touches only `designs//`, and only the package files — reference + drawings/photos live in the family issue + ([hard gates](https://github.com/BenchCAD-org/benchcad-2/blob/main/CONTRIBUTING.md#what-reviewers-and-ci-enforce)) +- [ ] Commits are DCO-signed (`git commit -s`) — + [fine print](https://github.com/BenchCAD-org/benchcad-2/blob/main/CONTRIBUTING.md#fine-print) - [ ] (If AI-assisted) I reviewed every line and stand behind the constraints ## Sources used diff --git a/.github/workflows/family-pr-checks.yml b/.github/workflows/family-pr-checks.yml index 16f799c..f31e41e 100644 --- a/.github/workflows/family-pr-checks.yml +++ b/.github/workflows/family-pr-checks.yml @@ -20,11 +20,13 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + issues: read + pull-requests: read steps: - uses: actions/checkout@v4 with: fetch-depth: 0 # need the base to diff changed files - - name: one-family scope + required files + - name: one-family scope + required files + package whitelist run: | git fetch -q origin "${{ github.base_ref }}" DIFF=$(git diff --name-only "origin/${{ github.base_ref }}...HEAD") @@ -64,4 +66,50 @@ jobs: exit 1 fi - echo "OK — family='${FAM}': one-family scope clean, all six required files present." + # 4) nothing but the package goes into designs// — reference + # PDFs/photos/scans belong in the family ISSUE, not the repo. Checked + # on the files this PR adds/modifies (deletions are fine). + ADDED=$(git diff --name-only --diff-filter=d "origin/${{ github.base_ref }}...HEAD" | grep -E "^designs/${FAM}/" | sed "s|^designs/${FAM}/||" || true) + JUNK=$(echo "$ADDED" | grep -vE '^(part\.py|spec\.py|family\.json|NOTES\.md|preview\.png|preview_views\.png|preview_extremes\.png|preview_parts\.png)$' | grep . || true) + if [ -n "$JUNK" ]; then + echo "::error::designs/${FAM}/ must contain only the family package (part.py, spec.py, family.json, NOTES.md, preview.png, preview_views.png, preview_extremes.png, and preview_parts.png for assemblies). Reference drawings/photos/datasheets go in the family issue instead. Remove from this PR: $(echo $JUNK)" + exit 1 + fi + + echo "OK — family='${FAM}': one-family scope clean, all six required files present, no stray files." + # Two things a human used to catch by hand, now red/green with an exact + # fix in the message: the PR checklist actually completed, and the family + # dir named the same as the family issue it closes. + - name: checklist completed + issue/family name coherence + uses: actions/github-script@v7 + with: + script: | + const files = await github.paginate(github.rest.pulls.listFiles, { + owner: context.repo.owner, repo: context.repo.repo, + pull_number: context.payload.pull_request.number, per_page: 100 }); + const fams = [...new Set(files.filter(f => f.filename.startsWith('designs/')) + .map(f => f.filename.split('/')[1]).filter(Boolean))]; + if (fams.length !== 1) { core.info('not a single-family PR — handled by the scope step'); return; } + const fam = fams[0]; + const body = context.payload.pull_request.body || ''; + const unchecked = (body.match(/^\s*-\s*\[ \]/gm) || []).length; + if (unchecked > 0) { + core.setFailed( + `The PR checklist has ${unchecked} unchecked item(s). Complete every item (honestly — CI re-runs validate) ` + + `and tick it by editing the PR description; no new push needed. Not done yet? Mark the PR as a Draft. ` + + `/ PR 描述里的 checklist 还有 ${unchecked} 项未勾选:如实完成后直接编辑 PR 描述勾上即可,不用重新 push;尚未完成请转为 Draft。`); + } + const m = body.match(/\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s+#(\d+)/i); + if (m) { + try { + const { data: issue } = await github.rest.issues.get({ + owner: context.repo.owner, repo: context.repo.repo, issue_number: Number(m[1]) }); + const t = (issue.title || '').match(/\[(?:family|proposal)\]\s*([a-z0-9_]+)/i); + if (t && t[1] !== fam) { + core.setFailed( + `This PR changes designs/${fam}/ but links issue #${m[1]} ("${t[1]}"). The family dir, ` + + `family.json "family", and the issue name must match exactly — rename one side or fix the Closes line. ` + + `/ 目录名 ${fam} 与所关联 issue 的家族名 ${t[1]} 不一致,请统一命名。`); + } + } catch (e) { core.info('issue lookup skipped: ' + e.message); } + } diff --git a/.github/workflows/pr-welcome.yml b/.github/workflows/pr-welcome.yml new file mode 100644 index 0000000..34180dd --- /dev/null +++ b/.github/workflows/pr-welcome.yml @@ -0,0 +1,45 @@ +name: pr-welcome +# One short orientation comment when a FIRST-TIME contributor opens a family PR +# (vLLM's new_pr_bot pattern: returning contributors get no bot noise). +# Uses `pull_request_target` so the token can comment on fork PRs too. +# SECURITY: this job must never check out or execute PR code — it only reads +# PR metadata and posts a comment. Keep it that way. + +on: + pull_request_target: + types: [opened] + +jobs: + welcome: + runs-on: ubuntu-latest + permissions: + pull-requests: write + steps: + - uses: actions/github-script@v7 + with: + script: | + const pr = context.payload.pull_request; + // returning contributor (has a merged PR here) -> stay quiet + const { data: prior } = await github.rest.search.issuesAndPullRequests({ + q: `repo:${context.repo.owner}/${context.repo.repo} type:pr is:merged author:${pr.user.login}`, + per_page: 1 }); + if (prior.total_count > 0) { core.info('returning contributor — no welcome'); return; } + const files = await github.paginate(github.rest.pulls.listFiles, { + owner: context.repo.owner, repo: context.repo.repo, + pull_number: pr.number, per_page: 100 }); + if (!files.some(f => f.filename.startsWith('designs/'))) { + core.info('not a family PR — no welcome needed'); return; + } + const R = `https://github.com/${context.repo.owner}/${context.repo.repo}/blob/main`; + const body = [ + `Welcome, and thanks for the family PR! Three things that answer most questions:`, + ``, + `- **No checks running?** A maintainer must click *Approve workflows* once for your first PR — pending, not ignored.`, + `- **Red gate?** The message tells you the fix: click **Details**, or read the \`bench2 validate\` report comment posted after each run. Reproduce locally with \`uv sync && uv run bench2 validate \` ([debug help](${R}/docs/DEBUGGING.md)).`, + `- **Scope:** \`designs//\` holds only the package files (reference drawings/photos go in the family issue), and the PR checklist must be fully ticked. Not done yet? Mark the PR as a **Draft**.`, + ``, + `中文:红 ✗ 点 Details 或看 validate 报告评论;本地 \`uv run bench2 validate \` 复现;参考图放 issue 不放 PR;没做完用 Draft。入门 → [GETTING_STARTED.zh](${R}/docs/GETTING_STARTED.zh.md)`, + ].join('\n'); + await github.rest.issues.createComment({ + owner: context.repo.owner, repo: context.repo.repo, + issue_number: pr.number, body }); diff --git a/.github/workflows/validate-report.yml b/.github/workflows/validate-report.yml new file mode 100644 index 0000000..1f3f690 --- /dev/null +++ b/.github/workflows/validate-report.yml @@ -0,0 +1,63 @@ +name: validate-report +# Companion to validate.yml: posts the bench2 validate report back onto the PR +# as a comment. This is a separate `workflow_run` workflow because on FORK PRs +# the `pull_request` event token is read-only — posting the comment from +# validate.yml itself silently failed with "Resource not accessible by +# integration" for every external contributor, i.e. exactly the audience the +# report is for. `workflow_run` executes in base-repo context with a write +# token; it never checks out or runs PR code (comment-only), so it is safe. + +on: + workflow_run: + workflows: [validate-designs] + types: [completed] + +jobs: + comment: + if: github.event.workflow_run.event == 'pull_request' + runs-on: ubuntu-latest + permissions: + actions: read + pull-requests: write + steps: + - name: fetch report artifact from the validate run + id: art + continue-on-error: true # no artifact = no-op validate (docs/CI-only PR) + uses: actions/download-artifact@v4 + with: + name: validate-report + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ github.token }} + - name: upsert the report comment on the PR + if: steps.art.outcome == 'success' + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const pr = Number(fs.readFileSync('pr.txt', 'utf8').trim()); + if (!pr) { core.setFailed('pr.txt missing/empty in validate-report artifact'); return; } + const run = context.payload.workflow_run; + const runUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}/actions/runs/${run.id}`; + let body = '\n'; + if (run.conclusion === 'failure') { + body += '❌ **validate failed** — fix locally, then push again:\n' + + '`uv sync && uv run bench2 validate ` (the pinned env matters — ' + + 'see [docs/DEBUGGING.md](https://github.com/' + context.repo.owner + '/' + context.repo.repo + '/blob/main/docs/DEBUGGING.md)).\n' + + '/ validate 未通过——请在本地用 pinned 环境复现并修复后重新 push。\n\n'; + } + body += fs.readFileSync('report.md', 'utf8'); + body += `\n_Previews: the \`previews\` artifact on [this run](${runUrl})._`; + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, repo: context.repo.repo, + issue_number: pr, per_page: 100 }); + const mine = comments.find(c => + c.body.startsWith('')); + if (mine) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, repo: context.repo.repo, + comment_id: mine.id, body }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, repo: context.repo.repo, + issue_number: pr, body }); + } diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index ec57afe..1667e9b 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -17,7 +17,6 @@ jobs: timeout-minutes: 30 permissions: contents: read - pull-requests: write steps: - uses: actions/checkout@v4 with: @@ -92,30 +91,22 @@ jobs: name: previews path: designs/*/preview.png if-no-files-found: ignore - - name: post validate report as PR comment + # The report COMMENT is posted by validate-report.yml (workflow_run): on + # fork PRs this job's token is read-only, so commenting from here fails + # with "Resource not accessible by integration" — exactly for the external + # contributors the report exists for. We hand the report + PR number over + # via an artifact instead. + - name: package report for validate-report.yml + if: always() && steps.scope.outputs.run == 'true' && github.event_name == 'pull_request' + run: | + mkdir -p /tmp/validate-report + cp report.md /tmp/validate-report/report.md 2>/dev/null || echo 'validate report missing — check the job log.' > /tmp/validate-report/report.md + echo "${{ github.event.pull_request.number }}" > /tmp/validate-report/pr.txt + - uses: actions/upload-artifact@v4 if: always() && steps.scope.outputs.run == 'true' && github.event_name == 'pull_request' - uses: actions/github-script@v7 with: - script: | - const fs = require('fs'); - let body = '\n'; - try { body += fs.readFileSync('report.md', 'utf8'); } - catch { body += 'validate report missing — check the job log.'; } - body += '\n_Previews: see the `previews` artifact on this run._'; - const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, repo: context.repo.repo, - issue_number: context.issue.number, per_page: 100 }); - const mine = comments.find(c => - c.body.startsWith('')); - if (mine) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, repo: context.repo.repo, - comment_id: mine.id, body }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, repo: context.repo.repo, - issue_number: context.issue.number, body }); - } + name: validate-report + path: /tmp/validate-report/ status-board: if: github.event_name == 'push' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 97412aa..9f74cb4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -45,8 +45,8 @@ uv run bench2 preview # inspect every generated view yourself | 1 | Claim | Self-assign and verify the evidence before coding | | 2 | Build | Implement the three family files; add `NOTES.md` for equation-heavy designs | | 3 | Validate | Run `bench2 validate` and inspect `bench2 preview` output | -| 4 | PR | Submit one scoped PR with `Closes #N` | -| 5 | CI | CI reruns validation and posts the report and previews | +| 4 | PR | Submit one scoped PR with `Closes #N` — **after** `bench2 validate` passes locally and you inspected the previews. Want early feedback before that? Open the PR as a GitHub **Draft** | +| 5 | CI | CI reruns validation and posts the report and previews. **Your first PR here:** a maintainer must click *Approve workflows* once before CI runs — no checks yet just means that click is pending | | 6 | Review | A non-author audits the evidence, renders, equations, constraints, and labels | | 7 | Merge | The issue closes and provenance/status automation runs | | 8 | Release | Qualified families enter the next versioned manifest | @@ -86,7 +86,7 @@ structure. A family PR must pass all three: |---|---| | `validate.yml` | `bench2 validate` — samples, constraints, execution, determinism, coverage, and that **every body is non-degenerate** (multi-body: matches `family.json` `"solids"`) | | `require-issue-link.yml` | the PR body links its family issue (`Closes #N`, still open) | -| `family-pr-checks.yml` | **one family per PR** (only `designs//`, plus a `geomlib` helper if you add one) and the family ships all six files: `part.py`, `spec.py`, `family.json`, `preview.png`, `preview_views.png`, `preview_extremes.png` | +| `family-pr-checks.yml` | **one family per PR** (only `designs//`, plus a `geomlib` helper if you add one); the family ships all six files: `part.py`, `spec.py`, `family.json`, `preview.png`, `preview_views.png`, `preview_extremes.png`; **nothing else** goes in the family dir (reference drawings/photos/datasheets belong in the family issue); the PR checklist is fully ticked; and the dir name matches the linked issue's family name | ## Issue taxonomy