diff --git a/.github/labels.json b/.github/labels.json index 628380ba7..f6bc13f5b 100644 --- a/.github/labels.json +++ b/.github/labels.json @@ -58,5 +58,10 @@ "name": "risk:T3", "color": "D93F0B", "description": "T3 (PASS at the tip, green CI). Advisory; the controller labels it, the merge gate recomputes it" + }, + { + "name": "pass", + "color": "006B75", + "description": "Reviewer PASS verdict at the merged head; set by pass-label, never removed" } ] diff --git a/.github/scripts/pass-label.mjs b/.github/scripts/pass-label.mjs new file mode 100644 index 000000000..3895c5340 --- /dev/null +++ b/.github/scripts/pass-label.mjs @@ -0,0 +1,132 @@ +// The `pass` label: a pull request that carried a PASS verdict while it merged keeps a visible trace +// of it (#739). The verdict itself lives only in a comment thread, and `agent-pr` says nothing about +// whether anything was reviewed. +// +// node .github/scripts/pass-label.mjs --comment # one new comment: label the PR when it is a PASS +// node .github/scripts/pass-label.mjs --backfill # merged pull requests whose head carried a PASS +// +// Both paths read the verdict with `parseReport`, the parser `review-gate.mjs` and `merge-gate.mjs` +// already use, so the fixed format (`Verdict: PASS at ` first line, `Written-by:` second) is read +// the same way everywhere. Three rules keep this a record and not a gate: +// - only a conclusion from someone GitHub ties to this repository counts (`OWNER`, `MEMBER`, +// `COLLABORATOR`), and only when its `Written-by:` names someone in the policy's `writers` (#676); +// - a `Correction from …` comment never labels, because it is text about a conclusion, not one — +// `parseReport` would happily find the `Verdict:` it quotes; +// - the label is only ever added. A later INC or a moved tip does not remove it: it records that a +// PASS was given, and `hold` stays the one manual stop. +import { execFileSync } from 'node:child_process' +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { parseReport, loadPolicy } from './review-gate.mjs' + +export const PASS_LABEL = 'pass' +export const ASSOCIATIONS = new Set(['OWNER', 'MEMBER', 'COLLABORATOR']) +const POLICY = '.github/agent/review-policy.json' +const SHORT = 12 + +const firstLine = (body) => String(body ?? '').split('\n').map((line) => line.trim()).find((line) => line !== '') ?? '' + +// Does this body decide that the tip was PASSed? `Correction from …` never does, whatever it quotes. +export function verdictOf(body, { writers = [], association } = {}) { + if (!ASSOCIATIONS.has(String(association ?? '').toUpperCase())) return null + if (/^Correction\b/i.test(firstLine(body))) return null + const report = parseReport(body) + if (report.verdict !== 'PASS') return null + if (!report.writer || !writers.includes(report.writer)) return null + return report +} + +// A report and the head it names: the tip may be a prefix (a 12-character sha is the usual form). +export function namesHead(tip, head) { + if (!tip || !head) return false + const t = String(tip).toLowerCase() + const h = String(head).toLowerCase() + return h.startsWith(t) || t.startsWith(h.slice(0, t.length)) +} + +// One comment (a comment event's payload) decides for one pull request. +export function planComment({ pr, body, association, writers, labels = [] }) { + const report = verdictOf(body, { writers, association }) + if (!report) return { add: false, reason: 'not a PASS by a writer the policy lists' } + if (labels.map((l) => String(l.name ?? l).toLowerCase()).includes(PASS_LABEL)) return { add: false, reason: `already labelled ${PASS_LABEL}` } + return { add: true, pr, tip: report.tip } +} + +// Merged pull requests whose history carries a PASS at the head that merged, and which do not have the +// label yet. `pages` is the raw API data so the whole decision is testable without the network. +export function planBackfill({ pulls, historyOf, writers }) { + const items = [] + for (const pull of pulls) { + if (!pull.merged_at) continue + const labels = (pull.labels ?? []).map((l) => String(l.name ?? l).toLowerCase()) + if (labels.includes(PASS_LABEL)) continue + const head = pull.head?.sha + if (!head) continue + const history = historyOf(pull.number) ?? [] + const pass = history + .filter((entry) => entry.body && (entry.association ? ASSOCIATIONS.has(String(entry.association).toUpperCase()) : true)) + .map((entry) => ({ at: entry.at ?? '', report: verdictOf(entry.body, { writers, association: entry.association ?? 'MEMBER' }) })) + .filter((entry) => entry.report && namesHead(entry.report.tip, head)) + .sort((a, b) => String(a.at).localeCompare(String(b.at))) + .pop() + if (pass) items.push({ number: pull.number, title: pull.title ?? '', tip: pass.report.tip }) + } + return items.sort((a, b) => a.number - b.number) +} + +const realGh = (...a) => execFileSync('gh', a, { encoding: 'utf8', maxBuffer: 16 << 20 }) +const list = (gh, path) => { + const out = gh('api', '--paginate', '--jq', '.[]', path) + return String(out ?? '').split('\n').filter(Boolean).map((line) => JSON.parse(line)) +} + +// The history of one pull request: its comments and its review bodies, oldest first. A conclusion the +// reviewer wrote as a review counts the same as a comment. +export function historyFrom({ comments = [], reviews = [] }) { + const rows = [ + ...comments.map((c) => ({ body: c.body, association: c.author_association, at: c.created_at })), + ...reviews.filter((r) => r.body).map((r) => ({ body: r.body, association: r.author_association, at: r.submitted_at })), + ] + return rows.sort((a, b) => String(a.at ?? '').localeCompare(String(b.at ?? ''))) +} + +export function main({ gh = realGh, env = process.env } = {}) { + const repo = env.GH_REPO + if (!repo) throw new Error('set GH_REPO') + const writers = loadPolicy(POLICY).writers ?? [] + const add = (pr) => gh('api', '-X', 'POST', `repos/${repo}/issues/${pr}/labels`, '-f', `labels[]=${PASS_LABEL}`) + + if (process.argv.includes('--comment')) { + const pr = Number(env.PR) + if (!pr) throw new Error('set PR') + const plan = planComment({ pr, body: env.COMMENT_BODY ?? '', association: env.ASSOCIATION, writers }) + if (!plan.add) { + console.log(`pass-label: #${pr} not labelled — ${plan.reason}`) + return { ok: true, added: [] } + } + add(pr) + console.log(`pass-label: #${pr} labelled ${PASS_LABEL} (PASS at ${plan.tip})`) + return { ok: true, added: [pr] } + } + + if (process.argv.includes('--backfill')) { + const pulls = list(gh, `repos/${repo}/pulls?state=closed&sort=updated&direction=desc&per_page=100`) + const historyOf = (number) => historyFrom({ + comments: list(gh, `repos/${repo}/issues/${number}/comments?per_page=100`), + reviews: list(gh, `repos/${repo}/pulls/${number}/reviews?per_page=100`), + }) + const items = planBackfill({ pulls, historyOf, writers }) + console.log(`pass-label: backfill: ${items.length} merged pull request(s) carry a PASS at their head and no ${PASS_LABEL}`) + for (const item of items) { + console.log(`#${item.number} ${item.title} (PASS at ${String(item.tip).slice(0, SHORT)})`) + add(item.number) + } + return { ok: true, added: items.map((item) => item.number) } + } + + throw new Error('pass --comment or --backfill') +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + try { main() } catch (error) { console.error(`pass-label: ${error.message}`); process.exit(1) } +} diff --git a/.github/scripts/pass-label.test.mjs b/.github/scripts/pass-label.test.mjs new file mode 100644 index 000000000..b4f7ad917 --- /dev/null +++ b/.github/scripts/pass-label.test.mjs @@ -0,0 +1,91 @@ +// The `pass` label (#739): a verdict that let a pull request in leaves a visible mark, mechanically. +// The parser is the one the gates already use, so these cases pin the three rules that decide it — +// who may write a conclusion, what a correction is, and that the label is only ever added. +import { test } from 'node:test' +import assert from 'node:assert/strict' +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { PASS_LABEL, namesHead, planBackfill, planComment, verdictOf, historyFrom } from './pass-label.mjs' +import { loadPolicy } from './review-gate.mjs' + +const repo = join(import.meta.dirname, '..', '..') +const writers = loadPolicy(join(repo, '.github/agent/review-policy.json')).writers +const head = 'df5d6f7e2b1c4f7a9e0d3c5b8a1f2e4d6c8b0a13' + +const conclusion = (verdict = 'PASS', sha = head, writer = 'Reviewer') => `Verdict: ${verdict} at ${sha} +Written-by: ${writer} + +## What I ran +- \`cd core && npm test\` → 1239/1239 + +## Risk +T1.` + +test('#739: a PASS at the tip labels the pull request', () => { + const plan = planComment({ pr: 7, body: conclusion(), association: 'MEMBER', writers }) + assert.equal(plan.add, true) + assert.equal(plan.tip, head) +}) + +test('#739: an INC, a correction, a non-member and an unknown writer never label', () => { + assert.equal(verdictOf(conclusion('INC'), { writers, association: 'MEMBER' }), null) + const correction = `Correction from Reviewer: the PASS at ${head} listed no command it ran. + +Verdict: PASS at ${head} +Written-by: Reviewer` + assert.equal(verdictOf(correction, { writers, association: 'MEMBER' }), null, 'a correction is text about a conclusion') + assert.equal(verdictOf(conclusion(), { writers, association: 'NONE' }), null, 'a passer-by is not a conclusion') + assert.equal(verdictOf(conclusion('PASS', head, 'Stranger'), { writers, association: 'MEMBER' }), null, 'the name must be in writers') +}) + +test('#739: a replacement PASS labels like a normal PASS, and a second run adds nothing', () => { + const replacement = `Verdict: PASS at ${head} +Written-by: Reviewer +Replaces: the PASS at ${head} merge-gate rejected for having no Ran: line + +## What I ran +- \`node --test test/follow-through.test.js\` → 14/14` + assert.ok(verdictOf(replacement, { writers, association: 'OWNER' })) + const already = planComment({ pr: 7, body: conclusion(), association: 'MEMBER', writers, labels: [{ name: PASS_LABEL }, { name: 'agent-pr' }] }) + assert.equal(already.add, false) + assert.match(already.reason, /already labelled/) +}) + +test('#739: backfill labels a merged pull request whose head carried a PASS, and nothing else', () => { + const pulls = [ + { number: 11, title: 'merged on a PASS', merged_at: '2026-10-22T10:00:00Z', head: { sha: head }, labels: [{ name: 'agent-pr' }] }, + { number: 12, title: 'merged on an INC', merged_at: '2026-10-22T10:05:00Z', head: { sha: head }, labels: [] }, + { number: 13, title: 'already labelled', merged_at: '2026-10-22T10:10:00Z', head: { sha: head }, labels: [{ name: PASS_LABEL }] }, + { number: 14, title: 'still open', merged_at: null, head: { sha: head }, labels: [] }, + // A PASS about some other tip is not a PASS about the head that merged. + { number: 15, title: 'PASS at another tip', merged_at: '2026-10-22T10:20:00Z', head: { sha: head }, labels: [] }, + ] + const history = { + 11: historyFrom({ comments: [{ body: conclusion(), author_association: 'MEMBER', created_at: '2026-10-22T09:00:00Z' }] }), + 12: historyFrom({ comments: [{ body: conclusion('INC'), author_association: 'MEMBER', created_at: '2026-10-22T09:00:00Z' }] }), + 13: historyFrom({ comments: [{ body: conclusion(), author_association: 'MEMBER', created_at: '2026-10-22T09:00:00Z' }] }), + 14: historyFrom({ comments: [{ body: conclusion(), author_association: 'MEMBER', created_at: '2026-10-22T09:00:00Z' }] }), + 15: historyFrom({ comments: [{ body: conclusion('PASS', 'aaaaaaaaaaaa'), author_association: 'MEMBER', created_at: '2026-10-22T09:00:00Z' }] }), + } + const items = planBackfill({ pulls, historyOf: (n) => history[n], writers }) + assert.deepEqual(items.map((item) => item.number), [11]) + // Second run: the label added by the first run is what the API now reports, so nothing is planned. + const after = planBackfill({ pulls: pulls.map((p) => (p.number === 11 ? { ...p, labels: [{ name: PASS_LABEL }] } : p)), historyOf: (n) => history[n], writers }) + assert.deepEqual(after, []) +}) + +test('#739: a review body counts like a comment, and the label is a closed record', () => { + const history = historyFrom({ reviews: [{ body: conclusion(), author_association: 'COLLABORATOR', submitted_at: '2026-10-22T09:30:00Z' }] }) + assert.equal(history.length, 1) + assert.ok(verdictOf(history[0].body, { writers, association: history[0].association })) + // No path removes the label: only `add` lines exist in the script. + const source = readFileSync(join(repo, '.github/scripts/pass-label.mjs'), 'utf8') + assert.doesNotMatch(source, /-X\s+'?\s*DELETE|method:\s*'DELETE'/) +}) + +test('#739: a 12-character tip names the head, a different one does not', () => { + assert.equal(namesHead(head.slice(0, 12), head), true) + assert.equal(namesHead(head, head.slice(0, 12)), true) + assert.equal(namesHead('aaaaaaaaaaaa', head), false) + assert.equal(namesHead(null, head), false) +}) diff --git a/.github/scripts/sync-labels.test.mjs b/.github/scripts/sync-labels.test.mjs index 38c7ab713..0c0ebf9cb 100644 --- a/.github/scripts/sync-labels.test.mjs +++ b/.github/scripts/sync-labels.test.mjs @@ -15,13 +15,14 @@ test('plan: create missing, keep equal, update changed, ignore others', () => { test('labels.json: valid, unique, agent-ready is the approval label', () => { const names = wanted.map((l) => l.name.toLowerCase()) - assert.equal(wanted.length, 12, 'the 12 live labels (#702: the retired approval label is not one of them)') + // #702 retired the approval label; #739 added `pass`, which records that a verdict was a PASS. + assert.equal(wanted.length, 13, 'the 13 live labels') assert.equal(new Set(names).size, names.length) for (const l of wanted) { assert.match(l.color, /^[0-9A-F]{6}$/i) assert.ok(l.description.length > 0 && l.description.length <= 100, l.name) } - for (const n of ['agent-task', 'agent-ready', 'agent-working', 'agent-pr', 'self-test', 'needs-human', 'hold']) assert.ok(names.includes(n), n) + for (const n of ['agent-task', 'agent-ready', 'agent-working', 'agent-pr', 'self-test', 'needs-human', 'hold', 'pass']) assert.ok(names.includes(n), n) }) test('the agent-task form never applies agent-ready', () => { diff --git a/.github/workflows/pass-label.yml b/.github/workflows/pass-label.yml new file mode 100644 index 000000000..cb8b1a788 --- /dev/null +++ b/.github/workflows/pass-label.yml @@ -0,0 +1,62 @@ +name: Pass label + +# A merged pull request kept no trace of the verdict that let it in: `agent-pr` stays, and the PASS +# lives only in a comment thread (#739). This labels a pull request `pass` when a `Verdict: PASS at +# ` conclusion is posted, and backfills the merged ones on demand. +# +# It is a record, not a gate. It only ever adds the label — a later INC, a moved tip or a `hold` never +# removes it — and it reads the conclusion with the same parser the gates use, so a comment from +# outside the repository, a `Written-by:` name the policy does not list and a `Correction from …` +# comment all leave the pull request alone. +on: + issue_comment: + types: [created] + workflow_dispatch: + inputs: + backfill: + description: 'Label every merged pull request whose head carried a PASS' + type: boolean + default: true + +concurrency: + group: pass-label-${{ github.event.issue.number || 'backfill' }} + cancel-in-progress: false + +permissions: + contents: read + issues: write + pull-requests: write + +jobs: + label: + name: pass label + # A comment on a plain issue is not a pull request; a manual run is the backfill. + if: github.event_name == 'workflow_dispatch' || github.event.issue.pull_request != null + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: main + persist-credentials: false + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: 24 + # The comment body is untrusted text: it is only parsed, never run, and the label is added from + # base-branch code. `pull_request` on a comment would run PR code, so this stays an + # `issue_comment` workflow. + - name: Label the pull request a PASS conclusion was posted on + if: github.event_name == 'issue_comment' + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + PR: ${{ github.event.issue.number }} + COMMENT_BODY: ${{ github.event.comment.body }} + ASSOCIATION: ${{ github.event.comment.author_association }} + run: node .github/scripts/pass-label.mjs --comment + - name: Backfill merged pull requests whose head carried a PASS + if: github.event_name == 'workflow_dispatch' + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + run: node .github/scripts/pass-label.mjs --backfill diff --git a/docs/testing-layers.json b/docs/testing-layers.json index 61c206bc8..d717623b8 100644 --- a/docs/testing-layers.json +++ b/docs/testing-layers.json @@ -201,6 +201,15 @@ "layer": "none", "gate": false, "reason": "The mechanical release: preflight, release PR, tag. It runs no tests of its own." + }, + { + "id": "pass-label", + "workflow": ".github/workflows/pass-label.yml", + "kind": "workflow-only", + "layer": "none", + "gate": false, + "runs": "issue_comment (created) and workflow_dispatch", + "reason": "Applies the `pass` label to a pull request whose conclusion was a PASS, and backfills merged ones (#739). A workflow-only layer with no test file of its own, and it gates nothing." } ] } diff --git a/docs/testing-layers.md b/docs/testing-layers.md index 7ea67fe8c..8b6fcd516 100644 --- a/docs/testing-layers.md +++ b/docs/testing-layers.md @@ -100,5 +100,6 @@ The tables below are generated: edit `docs/testing-layers.json`, then run | `label-hygiene.yml` | workflow-only | none | no | daily 03:30 Asia/Shanghai, and workflow_dispatch | Scheduled cleanup of status labels on closed issues and pull requests (#738). A workflow-only layer with no test file of its own, and it gates nothing. | | `nightly.yml` | workflow-only | none | no | daily 00:00 Asia/Shanghai, Sunday 01:00 compat matrix, and workflow_dispatch | The compatibility matrix (other OS, Node and DSH versions) on a schedule. It widens coverage but is not a gate, and its cases are not part of L1 or L2. | | `release.yml` | not-a-layer | none | no | — | The mechanical release: preflight, release PR, tag. It runs no tests of its own. | +| `pass-label.yml` | workflow-only | none | no | issue_comment (created) and workflow_dispatch | Applies the `pass` label to a pull request whose conclusion was a PASS, and backfills merged ones (#739). A workflow-only layer with no test file of its own, and it gates nothing. |