diff --git a/.githooks/pre-push b/.githooks/pre-push index 126341db8e..1225377135 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -1,24 +1,39 @@ #!/bin/sh # Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. # -# The collection half of the os-regen deferred merge (#8047). `pre-commit` may -# accept a MERGE commit whose regeneration is deferred to the next commit — but -# at the moment it decides, that next commit does not exist yet, so it can only -# RECORD the deferral. Something has to be the event that notices the deferral -# was never discharged. Only two events can follow a merge commit: another -# commit (which `pre-commit` already refuses while the artifacts are stale) and -# the push. This is the push. +# TWO refusals, one event. Both are about something that is cheap to repair now +# and expensive — or impossible — the moment the push lands. +# +# 1. The collection half of the os-regen deferred merge (#8047). `pre-commit` +# may accept a MERGE commit whose regeneration is deferred to the next commit — +# but at the moment it decides, that next commit does not exist yet, so it can +# only RECORD the deferral. Something has to be the event that notices the +# deferral was never discharged. Only two events can follow a merge commit: +# another commit (which `pre-commit` already refuses while the artifacts are +# stale) and the push. This is the push. +# +# 2. Card relations in commit messages. The relation is declared ONCE, in the PR +# body; a commit carrying it is refused here rather than on the pull request, +# because on a PUSHED branch nothing an author may legally do removes the +# message — the check that used to report it read the PR's commit list, so a new +# commit on top joins that list, and two rounds each paid a full redo. Before +# the push, the repair is an ordinary reword. The script's header is the +# authority; this file is the invocation. # # Registered by the same `core.hooksPath=.githooks` that registers `pre-commit`, # so it needs no change to `scripts/setup-git-hooks.mjs` and no separate opt-in. # -# Cheap by construction, exactly like `pre-commit`: with no pending marker the -# script exits before doing any work, which is every push in this repo that did -# not just defer a merge. The ref list git writes on stdin is drained and -# ignored on purpose — the marker is per-worktree state, not per-ref state, so -# which refs are being pushed cannot change the answer. +# The ref list git writes on stdin is read ONCE, here, and handed to the check +# that needs it. The regen marker is per-worktree state rather than per-ref +# state, so which refs are being pushed cannot change that answer; the card +# check is the opposite — the refs are its whole input, since they say which +# commits this push would publish. + +REFS=$(cat) +ROOT=$(git rev-parse --show-toplevel) if [ -z "$OS_SKIP_REGEN_CHECK" ]; then - cat >/dev/null - node "$(git rev-parse --show-toplevel)/scripts/check-regen-pending.mjs" --pre-push || exit 1 + node "$ROOT/scripts/check-regen-pending.mjs" --pre-push || exit 1 fi + +printf '%s\n' "$REFS" | node "$ROOT/scripts/check-commit-card-trailers.mjs" --pre-push || exit 1 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9d6415cc08..5dc469c435 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -982,6 +982,21 @@ jobs: - name: Part-of closing-keyword guard self-test run: pnpm check:partof-closing-keyword + # Commit card-trailer refusal self-test. The check itself is a PRE-PUSH + # hook — it judges the commits a push would publish, which no CI job has + # and no CI job should try to reconstruct — so what runs HERE is its + # self-test, the half whose verdict depends on nothing but the script. + # Unconditional for the same reason as the step above. + # + # It is the only thing standing behind that hook: a hook is registered per + # clone, runs on a developer's machine, and reports to nobody, so a break + # in its rule is invisible everywhere else. The self-test also pins the + # WIRING (the hook still calls the script, and still runs the os-regen + # deferral check it carried before), so unwiring it reddens here rather + # than going quiet. Pure functions plus two file reads; ~0.1s. + - name: Commit card-trailer pre-push refusal self-test + run: pnpm check:commit-card-trailers + # Publish-smoke tarball pin-set self-test. The assertion it pins lives on # the RELEASE path (scripts/publish-smoke-pack.mjs runs only inside the # packed-tarball smoke), so without this step a regression in it would be diff --git a/.github/workflows/partof-closing-keyword-guard.yml b/.github/workflows/partof-closing-keyword-guard.yml index 532b65fbd3..f9a414f13d 100644 --- a/.github/workflows/partof-closing-keyword-guard.yml +++ b/.github/workflows/partof-closing-keyword-guard.yml @@ -33,14 +33,14 @@ on: pull_request: types: [opened, edited, reopened, synchronize] -# `contents: read` checks the repo out to get at the script. `pull-requests: -# read` is what the commit-list gather below needs, and naming a `permissions:` -# block at all sets every scope NOT listed to `none`, so both must be spelled. -# Read-only is the whole grant: this gate reports, and never closes a PR, -# comments, or edits a body. +# `contents: read` checks the repo out to get at the script, and naming a +# `permissions:` block at all sets every scope NOT listed to `none`, so that one +# line is the whole grant. It used to also grant `pull-requests: read`, for a +# step that gathered the PR's commit messages; that rule is a pre-push hook now +# (the script header says why), so the scope went with it. This gate reports, +# and never closes a PR, comments, or edits a body. permissions: contents: read - pull-requests: read concurrency: group: partof-closing-keyword-${{ github.event.pull_request.number }} @@ -93,47 +93,8 @@ jobs: # No install step: the script imports one sibling module and reads no # workspace package, so `node` on the pinned runtime is the whole # toolchain it needs. - # RULE 2's input. The script judges it but never fetches it: the judging - # path stays HTTP-free, and the gather is a step of its own so that a - # network failure reads as a failed gather rather than as a verdict about - # somebody's PR. - # - # The endpoint is chosen over `git log base..head` deliberately. It - # returns exactly the set GitHub will squash. The git walk needs the merge - # base present to exclude what is already on the default branch, and the - # checkout above is depth 1 — so on a branch that has merged `main` back - # in, the walk cannot exclude those commits and would report another - # author's landed trailers as this PR's. Deepening until the merge base - # appears is unbounded, and `fetch-depth: 0` clones the whole repository - # to read a handful of messages. - # - # `--paginate` is load-bearing: without it a PR over one page silently - # loses its later commits, and a rule that read half the commits would - # report the unread half as clean. `--jq` emits one JSON object per line, - # and JSON escapes the newlines inside a commit message, so one row really - # is one line. The messages go to a FILE rather than into the environment: - # they are multi-line attacker-controlled text, and a path is inert where - # a body of prose is not. - # - # No pipeline here, on purpose. A `run:` block executes as `bash -e` - # WITHOUT pipefail, so `gh ... | jq ...` would take jq's exit code and a - # failed gather would reach the script as an empty file. It is a single - # redirect, so a failing `gh` fails the step; and if it ever did produce an - # empty file, the script reads zero rows as a failed gather, not as a PR - # with no commits. - - name: Gather the PR's commit messages - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.pull_request.number }} - REPO: ${{ github.repository }} - run: > - gh api --paginate "/repos/$REPO/pulls/$PR_NUMBER/commits" - --jq '.[] | {sha: .sha, message: .commit.message}' - > "$RUNNER_TEMP/pr-commits.jsonl" - - - name: A PR body may not close the card it is only part of, and no commit may carry a card trailer + - name: A PR body may not close the card it is only part of env: PR_BODY: ${{ github.event.pull_request.body }} PR_NUMBER: ${{ github.event.pull_request.number }} - PR_COMMITS_FILE: ${{ runner.temp }}/pr-commits.jsonl run: node scripts/check-partof-closing-keyword.mjs diff --git a/package.json b/package.json index 4bc1006c72..d7e5d4f224 100644 --- a/package.json +++ b/package.json @@ -87,6 +87,7 @@ "check:pm-governed-prose": "node scripts/pm/check-governed-prose.mjs --self-test && node scripts/pm/check-governed-prose.mjs", "check:publish-smoke-pin": "node scripts/publish-smoke-pack.mjs --self-test", "check:partof-closing-keyword": "node scripts/check-partof-closing-keyword.mjs --self-test", + "check:commit-card-trailers": "node scripts/check-commit-card-trailers.mjs --self-test", "check:single-claim-paths": "node scripts/check-single-claim-paths.mjs --self-test", "check:pnpm-filter-targets": "node scripts/pnpm-filter-targets.mjs --self-test && node scripts/check-pnpm-filter-targets.mjs --self-test && node scripts/check-pnpm-filter-targets.mjs", "check:turbo-task-graph": "node scripts/check-turbo-task-graph.mjs --self-test && node scripts/check-turbo-task-graph.mjs", diff --git a/scripts/check-commit-card-trailers.mjs b/scripts/check-commit-card-trailers.mjs new file mode 100644 index 0000000000..dcb20ace88 --- /dev/null +++ b/scripts/check-commit-card-trailers.mjs @@ -0,0 +1,834 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * check:commit-card-trailers — the PRE-PUSH refusal over card relations in + * commit messages. No commit may carry a card relation in its message: the pull + * request body is the only carrier of the relation. + * + * node scripts/check-commit-card-trailers.mjs --pre-push # git hands the refs on stdin + * node scripts/check-commit-card-trailers.mjs --range origin/main..HEAD + * node scripts/check-commit-card-trailers.mjs --self-test + * + * ⚠️ Repo paths are named UNQUOTED in this header, for the reason the sibling + * gate's header states at length: the dispatch-gates derivation turns a quoted + * path literal in a gate's source into a watch hint, header prose included, so + * a backticked path here would fabricate MATCHED leads for cards that touch + * none of this. The only quoted paths in this file are its real inputs. + * + * ## Why this is a pre-push hook and not a check on the pull request + * + * The rule itself is older than this file and was enforced at PR time, by the + * commit-list half of check-partof-closing-keyword. That placement was measured + * to be a post-push detector for a pre-push mistake, and the measurement is the + * argument for this file: + * + * - Two rounds, hours apart, pushed a branch whose commit message carried the + * relation, read the red correctly, and ⛔ declined to amend, rebase or + * force-push, which is the right call and the only one available. Each paid + * a FULL EXTRA ROUND: a new branch, the diff re-applied, a new pull request, + * the old one closed as superseded. + * - By the time that gate spoke, the cheap repair no longer existed. It read + * the pull request's COMMIT LIST, so a new commit on top JOINS that list and + * leaves the offending message in it; nothing an author may legally do + * removes it. + * + * A push is the moment the repair stops being free, so it is the moment to + * refuse. Everything this file judges is, BY CONSTRUCTION, unpublished: the + * range excludes every commit reachable from a remote-tracking ref and, on a + * re-push, everything the remote already has. So the remedy it prints — + * reword the message — rewrites nothing anybody else has, and this gate can + * prescribe it without contradicting the prohibition on rewriting pushed + * history. That is the whole trade the move buys. + * + * ## What "a card relation" means here, and where the grammar comes from + * + * ⛔ NOT A NEW PARSER. The relation extractors are the half-state sweep's, + * imported: this repository spells GitHub's closing-keyword grammar in three + * places and holds them behaviourally equal with a parity gate, and a FOURTH + * spelling would be a fourth thing to keep in step — the one nobody remembers + * when the grammar next moves. The parity gate's own sweep would find it. + * + * They are read at `markdown: false`, which is a measured contract rather than + * a default: a commit message is not markdown, nothing renders it, so backticks + * and fences are ordinary characters there and a relation inside them binds + * exactly like one in plain prose. An author who "quotes" a trailer to defuse + * it has defused nothing. + * + * Three relations, because all three are the body's to declare: a closing + * keyword acts on merge, while Part-of and Refs are read by this repo's own + * board tooling, so a commit carrying either tells the board something its + * author only meant to tell the pull request. + * + * ## The fourth shape: a bare reference in TRAILER POSITION + * + * The three relations above are all spelled KEYWORD plus reference. A trailer + * block can carry the same declaration with no keyword at all — a token, a + * colon, a bare reference — and the board tooling reads a trailer. So the last + * paragraph of the message is judged as a block when it IS one: every line a + * git trailer (a token, a colon, a value) or a continuation of one. Inside it, + * any reference is a finding. + * + * Deliberately NOT judged: a reference in ordinary body PROSE. A commit message + * that explains which card's discussion a decision came out of is not declaring + * a relation, and refusing it would tax the one kind of commit prose worth + * writing. The trailer block is where a machine reads a declaration, and that + * is the line this draws. + * + * ## The three false-positive shapes this must NOT refuse + * + * Measured by the spec seat, three times in one day, against the hand grep this + * file replaces — which counted BARE STEMS and therefore flagged each of them: + * + * a closed ten-member enum the stem inside ordinary prose + * findClosestMatches the stem inside an identifier + * fixture the stem inside an ordinary word + * + * None is a card relation and all three cost a real rewrite before a commit. + * The imported extractors bind the word AND the reference that follows it, so + * all three are silent here — and they are pinned below as cases that must + * stay green, because a future widening that is "obviously harmless" is exactly + * how a gate starts taxing correct commits. + * + * The harness trailer pair — the session URL and the co-author line — carries + * no reference at all and is pinned green for the same reason. + * + * ## The range, and why the zero sha is not the interesting half + * + * git hands a pre-push hook one line per ref on stdin: + * + * + * + * A deleted ref arrives with a zero LOCAL sha and is skipped: there is no + * commit to judge. A NEW branch arrives with a zero REMOTE sha, which is git's + * sample hook's cue to exclude what is already published rather than to judge + * the whole history. + * + * This goes one step further than the sample and excludes BOTH: the remote sha + * when the remote has one, and every remote-tracking ref in the clone. The + * second exclusion is what keeps a branch that has merged the default branch + * back in from reporting ANOTHER author's landed trailers as this push's — + * the same failure mode the PR-time gate avoided by reading an endpoint instead + * of walking a shallow clone. Since the repository's squash message is now the + * pull request BODY, landed commits carry the relations their bodies declared, + * so that exclusion is load-bearing rather than theoretical. + * + * A remote sha the clone does not have (someone else pushed since the last + * fetch) is dropped from the exclusion rather than failing the walk, and the + * remote-tracking exclusion still bounds it. + * + * ## Exit codes — and why "could not judge" refuses the push + * + * 0 judged, clean. + * 1 judged, finding. The push is refused and each offending commit is named + * with its line. + * 2 NOT JUDGED — no mode, or the walk could not run. A usage or wiring + * failure, never a statement about anybody's commits. + * + * Exit 2 refuses the push too, and that is the deliberate half. A verifier that + * silently degrades reports success, which is the failure this repository keeps + * paying for; the refusal names the deliberate override instead — + * `OS_ALLOW_CARD_TRAILER_PUSH=1`, spelled the way this repo spells an escape + * hatch, so that bypassing it is a visible act and not a flag anyone reaches + * for by habit. + */ + +import { execFileSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import process from 'node:process'; + +// The relation extractors are the sweep's, imported and not re-spelled — see +// the header. `markdown: false` is the commit surface, also the sweep's call. +import { closingKeywordTargets, partOfTargets, refsTargets } from './pm/check-half-states.mjs'; +import { isEntrypoint } from './invoked-as.mjs'; + +const ROOT = new URL('..', import.meta.url).pathname; + +export const EXIT_CLEAN = 0; +export const EXIT_FINDING = 1; +export const EXIT_NOT_JUDGED = 2; + +/** The deliberate override, spelled as this repo spells a dangerous one. */ +export const OVERRIDE_ENV = 'OS_ALLOW_CARD_TRAILER_PUSH'; + +/** The hook that calls this. Quoted: editing it really does implicate this gate. */ +const HOOK_SOURCE = '.githooks/pre-push'; + +/** + * The contract the finding CITES. Quoted, not restated: the sentence is written + * down elsewhere in this repository, that copy is the authority, and a gate + * that paraphrased it would become a second source for one rule. + * + * ⛔ Nothing inside the corner brackets may be written HERE — a sentence added + * here rather than there attributes to the ruling a claim it does not make, and + * prints that attribution to the very population that reads the rules file. The + * brackets are held to their source MECHANICALLY: the self-test reads the rules + * file and requires every sentence between them to appear in it verbatim. + * + * The path is named in prose here rather than as a literal because it is + * spelled once, as a literal, at AGENT_RULES_SOURCE. + */ +const RELATION_CONTRACT = + 'The contract is written down in the agent rules at .claude/agents/os-dev.md — ' + + '「卡片关系只在 PR 正文声明一次:commit ⛔ 不带卡片 trailer,其 trailer pair 一律 model-free。」 ' + + '(The card relation is declared ONCE, in the PR body; a commit carries no card trailer, ' + + 'and its trailer pair is model-free.)'; + +/** + * The file RELATION_CONTRACT quotes — a REAL input, read by the self-test. + * + * Quoted as a path literal on purpose, unlike the paths in the header: editing + * the sentence at the other end of it breaks the citation pin, so a card + * touching that file really does want this gate run. + */ +const AGENT_RULES_SOURCE = '.claude/agents/os-dev.md'; + +/** Every sentence inside the corner brackets of a citation, in order. */ +export function citedSentences(text) { + const quoted = /「([^」]*)」/.exec(String(text ?? '')); + if (quoted === null) return []; + return quoted[1] + .split(/(?<=。)/) + .map((sentence) => sentence.trim()) + .filter((sentence) => sentence !== ''); +} + +// --------------------------------------------------------------------------- +// The rule +// --------------------------------------------------------------------------- + +/** + * Every card-relation trailer one commit message carries, in a fixed order. + * + * ⛔ No grammar of its own: three imported extractors, read at the commit + * surface. A fourth spelling belongs nowhere, least of all here. + */ +export function commitRelations(message) { + const found = []; + for (const card of partOfTargets(message, { markdown: false })) found.push({ keyword: 'Part of', card }); + for (const card of refsTargets(message, { markdown: false }).keys()) found.push({ keyword: 'Refs', card }); + for (const [card, keyword] of closingKeywordTargets(message, { markdown: false })) found.push({ keyword, card }); + return found; +} + +/** A git trailer line: a token, a colon, a value. */ +const TRAILER_LINE = /^[A-Za-z][A-Za-z0-9-]*:[ \t]/; + +/** A continuation of the trailer above it. */ +const TRAILER_CONTINUATION = /^[ \t]+\S/; + +/** + * The message's trailer block — `{ from, lines }` — or null when it has none. + * + * The LAST paragraph, and only when every line in it is a trailer or a + * continuation. Stricter than git's own "at least a quarter of the lines" rule, + * deliberately: this decides whether a bare reference is a DECLARATION, and the + * cost of reading an ordinary closing paragraph as a trailer block is a refused + * push over prose. A message with no blank line at all has a subject and no + * trailer block — a subject is not a trailer, whatever its shape. + */ +export function trailerBlock(message) { + const lines = String(message ?? '').replace(/[ \t\r\n]+$/, '').split('\n'); + let from = lines.length; + while (from > 0 && lines[from - 1].trim() !== '') from--; + if (from === 0) return null; + const block = lines.slice(from); + if (block.length === 0) return null; + if (!TRAILER_LINE.test(block[0])) return null; + if (!block.every((line) => TRAILER_LINE.test(line) || TRAILER_CONTINUATION.test(line))) return null; + return { from, lines: block }; +} + +/** + * Bare references sitting in trailer position — `{ line, text, card }` each. + * + * A reference in a trailer is a declaration whether or not a keyword is beside + * it, which is why this is judged at all; a reference in body prose is not, and + * is deliberately untouched. + */ +export function trailerCardRefs(message) { + const block = trailerBlock(message); + if (block === null) return []; + const found = []; + block.lines.forEach((text, i) => { + for (const m of text.matchAll(/#(\d+)\b/g)) found.push({ line: block.from + i + 1, text, card: m[1] }); + }); + return found; +} + +/** The commit's subject, trimmed to a length that keeps a log line readable. */ +export function commitSubject(message) { + const first = String(message ?? '').split('\n', 1)[0].trim(); + return first.length > 72 ? `${first.slice(0, 69)}…` : first; +} + +/** The 1-based line that first names the card, or 1 when nothing does. */ +function lineNaming(message, card) { + const lines = String(message ?? '').split('\n'); + const at = lines.findIndex((line) => new RegExp(`#${card}\\b`).test(line)); + return at < 0 ? 1 : at + 1; +} + +/** + * One commit's findings — `{ line, text, why }` each, empty when clean. + * + * Deduplicated per line and card: a trailer line carrying a relation is one + * mistake and one line to edit, not two findings because two rules saw it. + */ +export function commitFindings(message) { + const lines = String(message ?? '').split('\n'); + const found = new Map(); + for (const { keyword, card } of commitRelations(message)) { + const line = lineNaming(message, card); + found.set(`${line}:${card}`, { + line, + text: (lines[line - 1] ?? '').trim(), + why: `\`${keyword} #${card}\` — a card relation.`, + }); + } + for (const { line, text, card } of trailerCardRefs(message)) { + const key = `${line}:${card}`; + if (found.has(key)) continue; + found.set(key, { + line, + text: text.trim(), + why: `\`#${card}\` in a TRAILER line — a bare card reference is a declaration too.`, + }); + } + return [...found.values()].sort((a, b) => a.line - b.line || a.why.localeCompare(b.why)); +} + +/** Every offending commit, as `{ sha, subject, findings }`. */ +export function offendingCommits(commits) { + const out = []; + for (const commit of commits) { + const findings = commitFindings(commit.message); + if (findings.length === 0) continue; + out.push({ + sha: String(commit.sha ?? '').slice(0, 9) || '(unknown sha)', + subject: commitSubject(commit.message), + findings, + }); + } + return out; +} + +// --------------------------------------------------------------------------- +// The push +// --------------------------------------------------------------------------- + +const ZERO_SHA = /^0{40,}$/; + +/** + * git's pre-push stdin, parsed — one row per ref, malformed lines dropped. + * + * `{ localRef, localSha, remoteRef, remoteSha }`. A row whose LOCAL sha is zero + * is a deletion and carries no commit to judge; it is dropped here rather than + * downstream so that a push of only deletions reaches the verdict as a push + * with no commits, which is clean rather than unjudged. + */ +export function parsePrePushRefs(stdin) { + const rows = []; + for (const line of String(stdin ?? '').split('\n')) { + const parts = line.trim().split(/\s+/); + if (parts.length !== 4) continue; + const [localRef, localSha, remoteRef, remoteSha] = parts; + if (ZERO_SHA.test(localSha)) continue; + rows.push({ localRef, localSha, remoteRef, remoteSha }); + } + return rows; +} + +/** + * The rev-list arguments for one pushed ref. + * + * The local sha, then the exclusions: the remote sha when the remote has one + * and this clone holds it, and every remote-tracking ref — see the header on + * why both are present and why a missing remote sha is dropped rather than + * fatal. + */ +export function revListArgs(row, { hasObject = () => true } = {}) { + const args = [row.localSha, '--not']; + if (!ZERO_SHA.test(row.remoteSha) && hasObject(row.remoteSha)) args.push(row.remoteSha); + args.push('--remotes'); + return args; +} + +function objectExists(sha, cwd) { + try { + execFileSync('git', ['cat-file', '-e', `${sha}^{commit}`], { cwd, stdio: 'ignore' }); + return true; + } catch { + return false; + } +} + +/** + * The record separator `git log -z` writes between commits. + * + * Built rather than typed: this repository refuses a raw control byte in any + * tracked file (`pnpm check:nul-bytes`), and an editor asked to write the + * escape can materialise it as the byte itself — which is how this line got + * the gate's attention once already. `String.fromCharCode` cannot be + * materialised into anything. + */ +const NUL = String.fromCharCode(0); + +/** + * The commits a rev-list range names, as `{ sha, message }` rows. + * + * NUL-terminated records: a commit message is multi-line by nature, so no + * line-oriented separator can be trusted to bound one. + */ +export function readCommits(args, { cwd = ROOT, run = execFileSync } = {}) { + const out = run('git', ['log', '-z', '--format=%H%n%B', ...args], { + cwd, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + }); + const rows = []; + for (const record of String(out).split(NUL)) { + if (record.trim() === '') continue; + const nl = record.indexOf('\n'); + if (nl < 0) { + rows.push({ sha: record.trim(), message: '' }); + continue; + } + rows.push({ sha: record.slice(0, nl).trim(), message: record.slice(nl + 1) }); + } + return rows; +} + +/** + * Every commit this push would publish, or `{ problem }`. + * + * Deduplicated by sha: pushing two refs that share history must not report one + * commit twice. + */ +export function pushedCommits(rows, { cwd = ROOT, run = execFileSync } = {}) { + const seen = new Map(); + for (const row of rows) { + const args = revListArgs(row, { hasObject: (sha) => objectExists(sha, cwd) }); + let commits; + try { + commits = readCommits(args, { cwd, run }); + } catch (err) { + return { problem: `the commit walk for ${row.localRef} failed — ${err?.message ?? err}` }; + } + for (const commit of commits) if (!seen.has(commit.sha)) seen.set(commit.sha, commit); + } + return { commits: [...seen.values()] }; +} + +// --------------------------------------------------------------------------- +// The verdict +// --------------------------------------------------------------------------- + +/** `{ exit, lines }`, pure, so the self-test drives it directly. */ +export function judge({ commits, problem, where = 'this push' }) { + if (problem) { + return { + exit: EXIT_NOT_JUDGED, + lines: [ + `✗ check:commit-card-trailers: NOT JUDGED — ${problem}`, + '', + " This is a wiring or usage failure, NOT a verdict about anybody's commits: nothing was read,", + ' so nothing can be said to be clean. The push is refused rather than waved through, because a', + ' check that cannot read its input and exits 0 reports success it never measured.', + '', + ` Deliberate override, if you know why this cannot run: ${OVERRIDE_ENV}=1 git push …`, + ], + }; + } + + const offenders = offendingCommits(commits); + if (offenders.length === 0) { + return { + exit: EXIT_CLEAN, + lines: [ + `✓ check:commit-card-trailers: ${commits.length} commit message(s) on ${where} carry no card relation.`, + ], + }; + } + + const lines = [ + `✗ check:commit-card-trailers: ${offenders.length} commit message(s) on ${where} carry a card relation.`, + ' The PR body is the only carrier of the relation.', + '', + ]; + for (const commit of offenders) { + lines.push(` commit ${commit.sha} ("${commit.subject}")`); + for (const finding of commit.findings) { + lines.push(` line ${finding.line}: ${finding.text}`); + lines.push(` ${finding.why}`); + } + lines.push(''); + } + lines.push( + ` ${RELATION_CONTRACT}`, + '', + ' REMEDY — and it is the cheap one, which is the whole reason this refusal happens HERE. Every', + ' commit above is UNPUBLISHED: this check judges only what the remote does not already have, so', + ' rewording these messages rewrites nothing anybody else has and needs no force-push.', + '', + ' the tip commit only git commit --amend (then push)', + ' an older one git reset --soft , then commit again with the', + ' relation removed from the message', + '', + ' State the relation ONCE, in the pull request body, where the contract puts it — and where, since', + ' the squash message is taken from the body, it is also what lands on the default branch.', + '', + ' ⛔ Do NOT reach for the override to get past this. Once these commits are pushed the repair above', + ' is gone: the only thing that would remove the message from a published branch is the history', + ' rewrite this repository forbids, and two rounds have already paid a full redo for exactly that.', + ` The override exists for a push whose history is not yours to reword: ${OVERRIDE_ENV}=1 git push …`, + ); + return { exit: EXIT_FINDING, lines }; +} + +// --------------------------------------------------------------------------- +// Self-test — the rule, the range arithmetic, the verdict layer and the wiring. +// --------------------------------------------------------------------------- + +const SELF_TEST_BATTERIES = Object.freeze({ + 'Every card-relation spelling in a commit message is a finding, including': 12, + 'The shapes that must stay green, so the gate does not tax ordinary commit': 8, + 'A bare reference in TRAILER position is a declaration; one in prose is not.': 7, + 'The finding names the commit AND the line, which is what a pusher acts on.': 6, + 'Delegation, not a second copy of the rule: the commit surface is the': 4, + 'The push arithmetic: which refs are judged, and what the range excludes.': 8, + 'The verdict layer: clean, finding, and a walk that could not run.': 6, + 'The wiring: the hook still calls this, and the cited contract is still': 5, +}); + +// DELETING an entry silences that battery's floor exactly as effectively as +// zeroing it, so the roster's own size is pinned too. +const SELF_TEST_BATTERY_FLOOR = 8; + +const UNATTRIBUTED_BATTERY = '(no battery open)'; + +/** Cases registered per battery: `battery()` opens one, `registerCase()` files into it. */ +const batteryCases = new Map(); +let openBattery = null; + +function battery(name) { + openBattery = name; +} + +function registerCase() { + const name = openBattery ?? UNATTRIBUTED_BATTERY; + batteryCases.set(name, (batteryCases.get(name) ?? 0) + 1); +} + +/** The floor: every declared battery RAN, and ran its cases. */ +function batteryFloorFailures() { + const declared = Object.keys(SELF_TEST_BATTERIES); + const problems = []; + if (declared.length < SELF_TEST_BATTERY_FLOOR) { + problems.push( + `SELF_TEST_BATTERIES declares ${declared.length} batteries, below the pinned ` + + `${SELF_TEST_BATTERY_FLOOR} — a battery deleted from the roster takes its own floor with it.`, + ); + } + for (const [name, count] of batteryCases) { + if (declared.includes(name)) continue; + problems.push( + `self-test battery "${name}" registered ${count} case(s) but is not declared in ` + + 'SELF_TEST_BATTERIES — an assertion attributed to no declared battery is one nothing floors.', + ); + } + for (const name of declared) { + const count = batteryCases.get(name) ?? 0; + if (count >= SELF_TEST_BATTERIES[name]) continue; + problems.push( + count === 0 + ? `self-test battery "${name}" DID NOT RUN — 0 cases registered, ${SELF_TEST_BATTERIES[name]} pinned. ` + + 'The verdict below would have claimed those cases hold.' + : `self-test battery "${name}" registered ${count} case(s), below its pinned floor of ` + + `${SELF_TEST_BATTERIES[name]} — cases that used to run no longer do.`, + ); + } + if (problems.length) { + problems.push( + 'A battery at or below its floor means cases STOPPED RUNNING — the battery is the bug, not the ' + + 'number. Find what stopped registering (an early return, a deleted block, a guard that now ' + + 'skips) and restore it.', + ); + } + return problems; +} + +const SELF_TEST_VERDICT = 'check-commit-card-trailers self-test reached its verdict'; + +function selfTest() { + const cases = []; + const t = (name, actual, expected) => { + registerCase(); + return cases.push([name, actual, expected]); + }; + const verdict = (...messages) => + judge({ commits: messages.map((message, i) => ({ sha: `${i}0deadbeef1234567`, message })), problem: null }); + + battery('Every card-relation spelling in a commit message is a finding, including'); + for (const spelling of ['Fixes', 'Closes', 'Resolves', 'Part of', 'Refs']) { + t( + `a commit message carrying "${spelling}" bound to a card is a finding`, + verdict(`fix(x): a subject\n\n${spelling} #4242\n`).exit, + EXIT_FINDING, + ); + } + // The measured gap: every spelling the dispatch orders forbid. The hyphen and + // the colon forms were invisible to this grammar until the extractors were + // widened, so a dev who greppped for them got 0 from a net with no thread in + // that square. + for (const spelling of ['Part-of #4242', 'Part of: #4242', 'Part-of: #4242', 'Refs: #4242', 'Fixes: #4242']) { + t(`"${spelling}" is a finding too`, verdict(`fix(x): a subject\n\n${spelling}\n`).exit, EXIT_FINDING); + } + t( + 'a lower-case spelling binds as well — the extractors are case-insensitive', + verdict('fix(x): a subject\n\npart-of: #4242\n').exit, + EXIT_FINDING, + ); + t( + 'a relation in the SUBJECT is a finding, not only one in the trailer block', + verdict('fix(x): a subject that resolves #4242 on its own\n').exit, + EXIT_FINDING, + ); + + battery('The shapes that must stay green, so the gate does not tax ordinary commit'); + t( + 'an ordinary commit message with no card relation is clean', + verdict('fix(cli): stop counting the walk instead of the tree\n\nBody prose about the change.\n').exit, + EXIT_CLEAN, + ); + // The three substring shapes measured against the hand grep this replaces. + t( + 'the stem inside ordinary prose is not a relation (a ten-member enum, closed)', + verdict('refactor(spec): read the enum as a closed ten-member set\n').exit, + EXIT_CLEAN, + ); + t( + 'the stem inside an identifier is not a relation (findClosestMatches)', + verdict('perf(cli): hoist findClosestMatches out of the loop\n').exit, + EXIT_CLEAN, + ); + t( + 'the stem inside an ordinary word is not a relation (fixture)', + verdict('test(rest): add a fixture for the empty page\n').exit, + EXIT_CLEAN, + ); + t( + 'the harness trailer pair carries no reference and stays clean', + verdict( + 'fix(x): a subject\n\nClaude-Session: https://claude.ai/code/session_0\n' + + 'Co-authored-by: Claude \n', + ).exit, + EXIT_CLEAN, + ); + t( + 'the squash subject marker is not a card relation (the paren stands between)', + verdict('fix(cli): report key counts off the emitted bytes (#16247)\n').exit, + EXIT_CLEAN, + ); + t( + 'a keyword with no reference beside it binds nothing', + verdict('docs: explain why closing the card by hand would drop the severe half\n').exit, + EXIT_CLEAN, + ); + t('an empty push is clean rather than unjudged', judge({ commits: [], problem: null }).exit, EXIT_CLEAN); + + battery('A bare reference in TRAILER position is a declaration; one in prose is not.'); + t('a bare reference on a trailer line is a finding', verdict('fix(x): a subject\n\nIssue: #4242\n').exit, EXIT_FINDING); + t( + 'the same reference in body prose is NOT a finding', + verdict('fix(x): a subject\n\nThe decision this came out of is #4242, which stays open.\n').exit, + EXIT_CLEAN, + ); + t( + 'a last paragraph that is prose is not a trailer block, however it ends', + trailerBlock('fix(x): a subject\n\nSo the remedy landed: see #4242 for the rest.\n'), + null, + ); + t( + 'a trailer block is only one when EVERY line in it is a trailer', + trailerBlock('fix(x): a subject\n\nCo-authored-by: A \nand a trailing prose line\n'), + null, + ); + t('a subject-only message has no trailer block at all', trailerBlock('fix(x): a subject'), null); + t('a continuation line stays inside the block', trailerBlock('s\n\nToken: value\n continued\n')?.lines.length, 2); + t('the trailer reference is reported at its own line number', trailerCardRefs('s\n\nbody\n\nIssue: #4242\n')[0]?.line, 5); + + battery('The finding names the commit AND the line, which is what a pusher acts on.'); + const named = verdict('fix(x): the subject that must be quoted back\n\nsome body\n\nPart-of: #4242\n').lines.join('\n'); + t('the finding names the offending commit by short sha', named.includes('00deadbee'), true); + t('the finding quotes the commit subject back', named.includes('the subject that must be quoted back'), true); + t('the finding names the line number', named.includes('line 5'), true); + t('the finding quotes the offending line', named.includes('Part-of: #4242'), true); + t('the finding CITES the contract rather than restating it', named.includes(RELATION_CONTRACT), true); + t( + 'the remedy is the cheap one and says why it is available here', + named.includes('UNPUBLISHED') && named.includes('git commit --amend') && named.includes('needs no force-push'), + true, + ); + + battery('Delegation, not a second copy of the rule: the commit surface is the'); + t( + 'a relation inside backticks is still a finding (a commit is not markdown)', + verdict('fix(x): a subject\n\n`Fixes #4242`\n').exit, + EXIT_FINDING, + ); + t( + 'a relation inside a fenced block is still a finding', + verdict('fix(x): a subject\n\n```\nFixes #4242\n```\n').exit, + EXIT_FINDING, + ); + t( + "the relations found are exactly the sweep extractors' union over the message", + commitRelations('Part of #1 and Refs #2 and Fixes #3').map((r) => `${r.keyword} #${r.card}`), + ['Part of #1', 'Refs #2', 'Fixes #3'], + ); + t( + 'the verdict is exactly the extractors over every fixture (no forked rule)', + ['Fixes #1', 'nothing here', 'Part-of: #2', 'a fixture and a closed enum'].every( + (message) => (verdict(message).exit === EXIT_FINDING) === (commitRelations(message).length > 0), + ), + true, + ); + + battery('The push arithmetic: which refs are judged, and what the range excludes.'); + const STDIN = [ + 'refs/heads/a 1111111111111111111111111111111111111111 refs/heads/a 2222222222222222222222222222222222222222', + 'refs/heads/new 3333333333333333333333333333333333333333 refs/heads/new 0000000000000000000000000000000000000000', + 'refs/heads/gone 0000000000000000000000000000000000000000 refs/heads/gone 4444444444444444444444444444444444444444', + '', + ].join('\n'); + const refs = parsePrePushRefs(STDIN); + t('a deleted ref carries no commits to judge and is dropped', refs.length, 2); + t('the surviving rows keep their local shas in order', refs.map((r) => r.localSha[0]).join(''), '13'); + t('a malformed line is dropped rather than judged', parsePrePushRefs('not four fields\n').length, 0); + t('empty stdin yields no rows', parsePrePushRefs('').length, 0); + t( + 'an existing remote tip is excluded by sha AND by the remote-tracking refs', + revListArgs(refs[0]).join(' '), + '1111111111111111111111111111111111111111 --not 2222222222222222222222222222222222222222 --remotes', + ); + t( + 'a new branch excludes the remote-tracking refs only (the zero sha is not a commit)', + revListArgs(refs[1]).join(' '), + '3333333333333333333333333333333333333333 --not --remotes', + ); + t( + 'a remote sha this clone does not have is dropped, never fatal', + revListArgs(refs[0], { hasObject: () => false }).join(' '), + '1111111111111111111111111111111111111111 --not --remotes', + ); + t( + 'the walk reads NUL-terminated records, so a multi-line message stays one commit', + readCommits([], { run: () => ['abc\nfix(x): s\n\nbody\n', 'def\nfix(y): t\n'].join(NUL) }).map((c) => c.sha).join(','), + 'abc,def', + ); + + battery('The verdict layer: clean, finding, and a walk that could not run.'); + const clean = judge({ commits: [{ sha: 'a1b2c3d4e5', message: 'chore: a clean subject\n' }], problem: null }); + t('a clean push says how many messages it read', clean.lines.join('\n').includes('1 commit message(s)'), true); + t('a clean push exits clean', clean.exit, EXIT_CLEAN); + const broken = judge({ commits: null, problem: 'the commit walk for refs/heads/x failed — boom' }); + t('a walk that could not run is NOT JUDGED', broken.exit, EXIT_NOT_JUDGED); + t('…and does not read as a clean push', broken.lines.join('\n').includes('✓'), false); + t("…and says it is not a verdict about anybody's commits", broken.lines.join('\n').includes('NOT a verdict'), true); + t('…and names the deliberate override', broken.lines.join('\n').includes(OVERRIDE_ENV), true); + + battery('The wiring: the hook still calls this, and the cited contract is still'); + const hook = readFileSync(join(ROOT, HOOK_SOURCE), 'utf8'); + t('the pre-push hook calls this script', hook.includes('check-commit-card-trailers.mjs'), true); + t('the hook hands it the ref lines git wrote on stdin', hook.includes('--pre-push'), true); + t( + 'the hook still runs the os-regen deferral check it carried before', + hook.includes('check-regen-pending.mjs'), + true, + ); + const agentRules = readFileSync(join(ROOT, AGENT_RULES_SOURCE), 'utf8'); + const cited = citedSentences(RELATION_CONTRACT); + t('the citation carries quoted sentences at all (never a vacuous zero)', cited.length > 0, true); + t( + 'every sentence inside the corner brackets is verbatim in the cited rules file', + cited.filter((sentence) => !agentRules.includes(sentence)), + [], + ); + + // The floor runs BEFORE the verdict below, so a success line can only be + // printed by a run in which every declared battery registered its cases. + for (const message of batteryFloorFailures()) cases.push([message, false, true]); + + let failedCount = 0; + for (const [name, actual, expected] of cases) { + const ok = JSON.stringify(actual) === JSON.stringify(expected); + if (!ok) failedCount++; + console.log(` ${ok ? '✓' : '✗'} ${name}${ok ? '' : ` (got ${JSON.stringify(actual)}, want ${JSON.stringify(expected)})`}`); + } + if (failedCount) { + console.error(`✗ check-commit-card-trailers self-test: ${failedCount} of ${cases.length} case(s) failed.`); + process.exit(1); + } + console.log(`✓ check-commit-card-trailers self-test: ${cases.length} cases pass.`); + + return SELF_TEST_VERDICT; +} + +// --------------------------------------------------------------------------- + +/** The mode dispatch, pure in its inputs so the entry point stays thin. */ +export function run(argv, env, stdin) { + if (env[OVERRIDE_ENV]) { + return { + exit: EXIT_CLEAN, + lines: [`⚠️ check:commit-card-trailers: SKIPPED — ${OVERRIDE_ENV} is set. Nothing was judged.`], + }; + } + const rangeAt = argv.indexOf('--range'); + if (rangeAt >= 0) { + const range = argv[rangeAt + 1]; + if (!range) return judge({ commits: null, problem: '`--range` was given no revision range.' }); + let commits; + try { + commits = readCommits([range]); + } catch (err) { + return judge({ commits: null, problem: `the commit walk for ${range} failed — ${err?.message ?? err}` }); + } + return judge({ commits, problem: null, where: range }); + } + if (!argv.includes('--pre-push')) { + return judge({ commits: null, problem: 'no mode given — expected --pre-push, --range or --self-test.' }); + } + const rows = parsePrePushRefs(stdin); + if (rows.length === 0) return judge({ commits: [], problem: null }); + const walked = pushedCommits(rows); + return judge({ commits: walked.commits ?? null, problem: walked.problem ?? null }); +} + +if (isEntrypoint(import.meta.url)) { + if (process.argv.includes('--self-test')) { + if (selfTest() !== SELF_TEST_VERDICT) { + console.error( + '\n✗ check-commit-card-trailers self-test: selfTest() returned without reaching its verdict,\n' + + 'so no success line was printed. Exiting 0 here would report a self-test that never\n' + + 'finished as a self-test that passed.\n', + ); + process.exit(1); + } + } else { + let stdin = ''; + if (process.argv.includes('--pre-push')) { + try { + stdin = readFileSync(0, 'utf8'); + } catch { + stdin = ''; + } + } + const result = run(process.argv.slice(2), process.env, stdin); + const emit = result.exit === EXIT_CLEAN ? console.log : console.error; + for (const line of result.lines) emit(line); + process.exit(result.exit); + } +} diff --git a/scripts/check-partof-closing-keyword.mjs b/scripts/check-partof-closing-keyword.mjs index 950cef304b..f58046e49e 100644 --- a/scripts/check-partof-closing-keyword.mjs +++ b/scripts/check-partof-closing-keyword.mjs @@ -2,16 +2,18 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * check:partof-closing-keyword — the PR-scoped BLOCKING guard over BOTH the - * surfaces GitHub's reference parser reads when a pull request merges. + * check:partof-closing-keyword — the PR-scoped BLOCKING guard over the pull + * request BODY, which is both what GitHub's reference parser reads when the + * request merges and — since the squash message is taken from it — the text + * that lands on the default branch. * * RULE 1 — THE BODY. A pull request may not declare itself only `Part of #N` * while also telling GitHub to close that same `#N`. This is the half-state * sweep's H7, called. * - * RULE 2 — THE COMMIT MESSAGES. No commit on the pull request may carry a - * card-relation trailer at all: no closing keyword, no Part-of and no Refs - * bound to any card number. The body is the only carrier of the relation. + * RULE 2 — THE COMMIT MESSAGES — is no longer judged here. It moved to a + * pre-push hook when the squash message stopped being assembled from the + * commits; the section below is the authority on where it went and why. * * RULE 3 — THE BODY, NEGATED. A pull request may not bind a closing keyword * to a `#N` inside a sentence that reads as NOT closing it. This is the @@ -78,114 +80,51 @@ * whose body has since been fixed replays the stale body and stays red. The * remedy is to edit the body (which fires a new run), not to re-run. * - * ## RULE 2 — why a commit trailer is a defect even when it is TRUE - * - * This repository squash-merges, so the commit that lands on the default branch - * is ASSEMBLED AT MERGE TIME by concatenating the branch's commit messages. - * Each of those was written at a different moment about a different slice of - * the work, and each can be perfectly honest on its own; the concatenation is - * written by nobody and reviewed by nobody. - * - * Measured on the squash of PR 16247, commit fc3fb7c4619, an ancestor of the - * default branch: three bullets assembled from three commit messages, a closing - * trailer for a card sitting inside the first bullet's body, and a third bullet - * that retracts a claim the first bullet still makes. The landed message - * therefore asserts and withdraws the same thing in one text, and it told - * GitHub to close the card from inside a bullet nobody read as a declaration. - * - * Note what RULE 1 cannot see there: the contradictory text existed in no body, - * so the body was clean — correctly so. The sweep's H23 reads this surface, but - * AFTER the fact, on the default branch, and it binds narrower: it reports only - * the Part-of-plus-closing-keyword CONTRADICTION, and the specimen above - * carries no Part-of at all, so H23 is silent on exactly this shape. - * - * RULE 2 is the pre-merge, blocking, strictly WIDER statement, and the width is - * what makes it enforceable. "Trailers that would contradict each other once - * concatenated" cannot be judged commit by commit, because the contradiction is - * a property of the assembly, which does not exist until the merge button. "No - * relation trailer in any commit" is a property of ONE commit, so one commit is - * enough to judge — and an assembly cannot manufacture what none of its inputs - * contain. Squash is not the only cost either: the trailers are also live on - * their own, so a branch pushed to the default branch by any other route closes - * the cards its intermediate commits name. - * - * The contract is the one this repository already carries, cited rather than - * restated — the card relation is declared ONCE, in the PR body, and no commit - * carries a card trailer. - * - * ## RULE 2 — what the output may ask for, and when it may ask for nothing - * - * ⛔ Nothing in this gate's output asks anyone to rewrite history: amend, rebase - * and force-push are forbidden here. That prohibition is absolute, which makes - * the TIMING of a RULE 2 finding the whole story: - * - * - BEFORE the branch is pushed the remedy is ordinary work — reword the - * commit messages, state the relation only in the body, push once. Nothing - * published is rewritten and the check goes green. - * - ONCE THE BRANCH IS PUSHED no author action clears the red. This gate reads - * the PR's COMMIT LIST, so a new commit on top JOINS that list and leaves - * the offending message in it; the only thing that would remove it is the - * rewrite forbidden above. The red is PERMANENT for that branch, and saying - * so plainly is the point of this section. An earlier revision instead told - * the author to push reworded commits — which on a pushed branch IS the - * forbidden rewrite — and a seat that read it instructed one; the dev's - * refusal is what stopped it, not the wording. - * - * ⚠️ That earlier revision also had the merger "take the squash message from - * that body". Measured, that is false: on 0a61db1f5, the squash of PR #16646, - * the landed message is the branch commit's verbatim and carries its - * `Refs #16624`, while the body's `Fixes #16624` appears nowhere in it. The - * squash message is assembled from the COMMITS. This does not weaken RULE 2 — - * it is RULE 2's premise: a trailer left on a pushed commit really does reach - * the default branch's permanent history. - * - * What DISCHARGES a pushed-branch red is therefore the merge, and only a merge - * whose squash message is the PR BODY — then the trailer never reaches the - * default branch. Whether that happens by itself is the repository setting - * `squash_merge_commit_message`: at `COMMIT_MESSAGES` (the value behind the - * measurement above) it takes the lander replacing the assembled commit list - * with the PR body by hand at the merge button, which a queue merge never does; - * at `PR_BODY` every squash does it, queue included. The output states that - * CONDITION rather than the value the setting holds today, so it stays true - * whichever way the repository is configured when it is read — and discharging - * the red never rewrites history. - * - * What that costs depends on the spelling, and the output says so rather than - * flattening it: Part-of and Refs land as a reference and move no card, while a - * CLOSING keyword lands on the surface GitHub's parser reads. The asymmetry is - * why the rule refuses all three at PR time rather than only the contradictory - * ones. - * - * ⛔ Whether a pull request LANDS carrying this red is not this gate's call and - * its output must not make it. The check run is advisory at the branch- - * protection layer — absent from the required-context registry, and its workflow - * subscribes to no `merge_group` event because a queue build carries no body for - * it to judge — so that decision belongs to whoever lands the PR, under the - * rules that bind them. This gate reports. - * - * ## Where the commit messages come from, and why the endpoint and not a walk - * - * The WORKFLOW gathers them and hands them over as data, so the judging path - * stays HTTP-free by construction exactly as it is for the body: this script - * still makes no request, and its inputs are still only the environment and, now, - * a file the environment names. - * - * The gather is one paginated REST read of the pull request's own commits list, - * chosen over a git walk for a measured reason. That endpoint returns exactly - * the set GitHub will squash. The git equivalent needs the merge base present in - * order to exclude what is already on the default branch, and this job checks - * out at depth 1 — so on a branch that has merged the default branch back in, a - * shallow walk cannot perform that exclusion and reports ANOTHER author's landed - * trailers as this pull request's. The alternatives are a deepen-until-found - * loop, which is unbounded, or a full-history clone to read a handful of - * messages. The endpoint is bounded, exact, and needs one added read scope. - * - * The list reaches the script as a FILE named in the environment, not as a value - * in it. Commit messages carry newlines, blank lines and quotes; JSON Lines - * through a file keeps that payload out of the shell and out of the environment - * block, which is the same argument the body's `env:` spelling rests on, applied - * to a payload too big and too multi-line to be an environment value. - * + * ## Where RULE 2 went, and why this gate no longer reads the commit list + * + * There was a third rule here, and it is worth knowing where it went rather + * than discovering its absence. RULE 2 refused a card-relation trailer in ANY + * commit message on the pull request, and it rested on one premise: this + * repository squash-merges by assembling the branch's COMMIT MESSAGES, so a + * trailer left on a pushed commit really does reach the default branch's + * permanent history. + * + * That premise no longer holds. The repository setting + * `squash_merge_commit_message` is `PR_BODY` — paired with `PR_TITLE`, the only + * combination GitHub accepts it in — so every squash, queue merges included, + * lands the pull request BODY and nothing else. A trailer in a branch commit is + * no longer a trailer on the default branch. + * + * What remained was the cost of saying so LATE. This gate reads the pull + * request's commit list, so once a branch is pushed a new commit on top JOINS + * that list and leaves the offending message in it; nothing an author may + * legally do removes it, because the removal is the history rewrite this + * repository forbids. Two rounds, hours apart, each paid a full redo — a new + * branch, the diff re-applied, a new pull request — for a mistake that costs + * one reword when it is caught before the push. A gate whose only reachable + * remedy is a redo is a post-push detector for a pre-push mistake. + * + * So the rule moved to the moment the repair is still free: it is a PRE-PUSH + * hook now, in check-commit-card-trailers, called from .githooks/pre-push. It + * judges exactly the commits a push would publish, and everything it judges is + * by construction unpublished, so its remedy is an ordinary reword rather than + * a rewrite. The rule itself did not weaken — no commit carries a card + * relation, the body is the only carrier — and the contract citation moved with + * it, to the finding that now prints it. + * + * ⚠️ What the move gives up is stated rather than left to be discovered: a hook + * is registered PER CLONE (by `pnpm install`, through the hook registrar), runs + * on the pusher's machine, and reports to nobody, so a clone that never + * installed pushes unchecked. That is the ruled trade — the enforcement lands + * at the only moment where the fix is cheap, and CI holds its self-test. ⛔ It + * is not an argument for re-adding a commit-list read here: with the squash + * message coming from the body, a red on this surface would report a text that + * no longer lands. + * + * The numbering is left alone. RULE 3 keeps its number although RULE 2 is gone, + * exactly as it kept it while reading RULE 1's surface: the numbers are the + * order the rules were learned, and renumbering would rewrite output text this + * file's own self-test pins by name, for no gain. * ## RULE 3 — the same surface as RULE 1, and the half of the class it misses * * RULE 1's own rationale is stated in fully general terms: GitHub's parser @@ -233,28 +172,13 @@ * * ## Exit codes — and why an empty body is a VERDICT, not a skip * - * 0 judged, clean — BOTH rules, over inputs that were really read. - * 1 judged, finding. The PR is red until the body is reworded (RULE 1 and - * RULE 3) or the relation is moved out of the commits and into the body - * (RULE 2). All three findings share this exit: a body that negates the - * relation it also states is the same contradiction class RULE 1 refuses, - * so it is a new finding KIND and not a new exit code. - * 2 NOT WIRED — an input this gate judges is missing, so a rule verified - * nothing. A usage/wiring failure, never a statement about any PR. - * - * Exit 2 covers no PR context at all AND the case where the body arrived but - * the commit list did not. That second one is deliberate and is the whole - * presence semantics of RULE 2: a wiring that forgot the commits has not seen a - * clean commit history, it has seen no commit history, and the two must never - * print the same line. An empty commit list is read the same way rather than as - * a clean PR with nothing in it — every pull request has at least one commit, - * so zero rows means the gather failed, not that the author pushed nothing. - * - * The precedence when both apply — a real finding and a half-wired run — is - * FINDING first, because a finding is a true statement about an input that was - * really read, while exit 2 claims nothing was judged. The unread half is still - * named in the output, so a run can never quietly drop the fact that it read - * only one of the two surfaces. + * 0 judged, clean — both rules, over an input that was really read. + * 1 judged, finding. The PR is red until the body is reworded. Both findings + * share this exit: a body that negates the relation it also states is the + * same contradiction class RULE 1 refuses, so it is a new finding KIND and + * not a new exit code. + * 2 NOT WIRED — the input this gate judges is missing, so nothing was + * judged. A usage/wiring failure, never a statement about any PR. * * The split matters in both directions. A gate that cannot read its input has * verified nothing, and exiting 0 there is the anti-pattern this repo keeps @@ -309,22 +233,13 @@ import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; import process from 'node:process'; -// The relation extractors are the sweep's, imported and not re-spelled. Two -// things ride on that beyond the usual no-fork argument. GitHub's +// Both predicates are the sweep's, imported and not re-spelled. GitHub's // closing-keyword grammar is spelled in three places in this tree and a parity // gate holds those three behaviourally equal; a FOURTH spelling here would be a // fourth thing to keep in step, and it would be the one nobody remembers when -// the grammar next moves. And the `markdown: false` reading these are called -// with is itself a measured contract — a commit message is not markdown, so -// backticks do not neutralise a keyword there — which is the sweep's H23 -// section, not a call this gate is entitled to re-make. -import { - closingKeywordTargets, - h7PartOfWithClosingKeyword, - h21NegatedClosingKeyword, - partOfTargets, - refsTargets, -} from './pm/check-half-states.mjs'; +// the grammar next moves. Importing the PREDICATES rather than the extractors +// takes the same posture one level up: the rule itself has one home. +import { h7PartOfWithClosingKeyword, h21NegatedClosingKeyword } from './pm/check-half-states.mjs'; import { isEntrypoint } from './invoked-as.mjs'; // ── The self-test's own battery roster and floor (#13489) ────────────────── @@ -361,18 +276,12 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'Context reading: presence, not truthiness.': 4, 'The wiring itself. A gate whose workflow step is deleted or whose': 6, 'The predicate source this gate reuses must still be there to reuse.': 1, - 'RULE 2 — every card-relation spelling in a commit message is a finding,': 22, - 'The regression fixture: the squash that assembled a contradiction no': 4, - 'RULE 2 delegates to the sweep extractors at the commit-message reading.': 3, - 'The commit list input. An absent, broken or empty list can never read': 8, - 'The verdict layer over two rules: precedence, and the unread half is': 5, - 'The wiring gathers the commit list and hands it over as a file path.': 5, 'RULE 3 — a closing keyword bound to a card the sentence says it is NOT': 17, }); // DELETING an entry silences that battery's floor exactly as effectively as // zeroing it, so the roster's own size is pinned too. -const SELF_TEST_BATTERY_FLOOR = 16; +const SELF_TEST_BATTERY_FLOOR = 10; // The key an assertion is filed under when no battery is open. It is not a // declared battery, so it reds by the same set difference rather than silently @@ -454,162 +363,10 @@ const PREDICATE_SOURCE = 'scripts/pm/check-half-states.mjs'; /** The wiring that gives this gate a PR to judge. */ const WIRING_WORKFLOW = '.github/workflows/partof-closing-keyword-guard.yml'; -/** - * The file RELATION_CONTRACT quotes — a REAL input, read by the self-test. - * - * Quoted as a path literal on purpose, unlike the paths in the header. The - * derivation turns a quoted path in this file into a watch hint, and this one - * is a hint that tells the truth: editing the sentence at the other end of it - * breaks the citation pin below, so a card touching that file really does want - * this gate run. The header's last section is the authority on the distinction. - */ -const AGENT_RULES_SOURCE = '.claude/agents/os-dev.md'; - export const EXIT_CLEAN = 0; export const EXIT_CONTRADICTION = 1; export const EXIT_NOT_WIRED = 2; -/** The env var naming the file the wiring writes the PR's commit list to. */ -export const COMMITS_FILE_ENV = 'PR_COMMITS_FILE'; - -/** - * The contract RULE 2's finding CITES. It is quoted, not restated: the sentence - * is written down elsewhere in this repository, that copy is the authority, and - * a gate that paraphrased it would become a second source for one rule — the - * exact drift this file refuses elsewhere by importing its predicate instead of - * copying it. Quoted verbatim, in its own language, because a translation of a - * ruling is a rewrite of it. - * - * ⛔ Nothing inside the corner brackets may be written HERE. An earlier revision - * carried a second sentence in them — that the squash concatenates the commit - * messages and can assemble a contradiction out of individually honest parts — - * which was true on the facts and had never been in the rules file at all. That - * is the same failure as a translation, in the other direction: it attributes to - * the ruling a claim the ruling does not make, and it prints that attribution to - * the very population that reads the rules file. The squash fact is this gate's - * own, and this gate already states it in its own words twice — in the RULE 2 - * header section and in the printed finding below — so it is not restated here - * a third time. - * - * The brackets are held to their source MECHANICALLY, not by care: the self-test - * reads AGENT_RULES_SOURCE and requires every sentence between them to appear in - * it verbatim. Comparing the printed finding with this constant cannot do that — - * both sides move together when the constant is edited, which is precisely how - * the added sentence survived a self-test that already claimed to check the - * citation. - * - * The path is named in prose here rather than as a literal because it is spelled - * once, as a literal, at AGENT_RULES_SOURCE; that declaration carries the - * watch-hint note. - */ -const RELATION_CONTRACT = - 'The contract is written down in the agent rules at .claude/agents/os-dev.md — ' - + '「卡片关系只在 PR 正文声明一次:commit ⛔ 不带卡片 trailer,其 trailer pair 一律 model-free。」 ' - + '(The card relation is declared ONCE, in the PR body; a commit carries no card trailer, ' - + 'and its trailer pair is model-free.)'; - -/** - * Every sentence inside the corner brackets of a citation, in order. - * - * Split rather than compared whole so the pin stays honest if the quotation ever - * grows a second sentence legitimately: each is held to the source on its own, - * and a sentence added here without being added there is named individually. - * - * Returns an empty array when there are no brackets at all, which the self-test - * refuses explicitly — an extractor that silently found nothing would make the - * citation pin vacuously green, the phantom-check shape this file exists to - * refuse elsewhere. - */ -function citedSentences(text) { - const quoted = /「([^」]*)」/.exec(String(text ?? '')); - if (quoted === null) return []; - return quoted[1] - .split(/(?<=。)/) - .map((sentence) => sentence.trim()) - .filter((sentence) => sentence !== ''); -} - -/** - * The commit rows the wiring gathered, as JSON Lines — one object per line, - * `{ sha, message }`. - * - * A commit message is multi-line by nature and JSON escapes those newlines, so - * one row really is one line and the format needs no separator of its own. A - * malformed line is a PROBLEM and never a skipped row: a parser that silently - * dropped what it could not read would shrink the population this rule judges - * and report the survivors as the whole PR. - */ -export function parseCommitRecords(text) { - const rows = []; - const lines = String(text ?? '').split('\n'); - for (let i = 0; i < lines.length; i++) { - if (lines[i].trim() === '') continue; - let row; - try { - row = JSON.parse(lines[i]); - } catch (err) { - return { problem: `line ${i + 1} of the commit list is not JSON — ${err.message}` }; - } - if (typeof row?.sha !== 'string' || typeof row?.message !== 'string') { - return { problem: `line ${i + 1} of the commit list has no string \`sha\` and \`message\`.` }; - } - rows.push({ sha: row.sha, message: row.message }); - } - return { rows }; -} - -/** - * Every card-relation trailer one commit message carries, in a fixed order. - * - * All three relations, because all three are the PR body's to declare: a - * closing keyword acts on merge, and Part-of and Refs are read by this repo's - * own board tooling, so a commit carrying either tells the board something its - * author only meant to tell the pull request. Read at `markdown: false` — see - * the import note. - */ -export function commitRelations(message) { - const found = []; - for (const card of partOfTargets(message, { markdown: false })) found.push({ keyword: 'Part of', card }); - for (const card of refsTargets(message, { markdown: false }).keys()) found.push({ keyword: 'Refs', card }); - for (const [card, keyword] of closingKeywordTargets(message, { markdown: false })) found.push({ keyword, card }); - return found; -} - -/** The commit's subject, trimmed to a length that keeps a log line readable. */ -function commitSubject(message) { - const first = String(message ?? '').split('\n', 1)[0].trim(); - return first.length > 72 ? `${first.slice(0, 69)}…` : first; -} - -/** - * RULE 2 — one finding sentence per offending commit, empty when clean. - * - * Per COMMIT rather than per relation: an author fixes a message, not a match, - * and a commit carrying three trailers is one edit and should be one line. - */ -export function commitTrailerFindings(commits) { - const findings = []; - for (const commit of commits) { - const relations = commitRelations(commit.message); - if (relations.length === 0) continue; - const sha = String(commit.sha ?? '').slice(0, 9) || '(unknown sha)'; - const carried = relations.map((r) => `\`${r.keyword} #${r.card}\``).join(', '); - findings.push( - `commit \`${sha}\` ("${commitSubject(commit.message)}") carries ${carried} in its message. ` + - `This repo squash-merges, so every commit message on this PR is concatenated into the one ` + - `message that lands on the default branch — a text no one writes and no one reviews, which ` + - `carries every trailer its inputs carried and can contradict itself where its parts did not. ` + - `The trailer is also live on its own. ${RELATION_CONTRACT} ` + - `Remedy, while this branch is still UNPUSHED: reword this commit message so the relation is ` + - `stated only in the PR body. ` + - `⛔ Do NOT amend, rebase or force-push to remove it — rewriting pushed history is forbidden ` + - `here; on an already-pushed branch nothing removes it, and the summary below is what to do ` + - `about that.`, - ); - } - return findings; -} - /** * The PR context, or null when this process was handed none. * @@ -623,51 +380,9 @@ export function readPrContext(env) { return { number: String(env.PR_NUMBER ?? '').trim(), body: env.PR_BODY ?? '', - ...readCommits(env), }; } -/** - * RULE 2's input: `{ commits }` when a list was really read, else - * `{ commits: null, commitsProblem }` naming which way it was not. - * - * Every failure mode lands in `commitsProblem` rather than throwing, so that a - * missing or broken commit list is REPORTED by the verdict layer as an unjudged - * rule instead of killing the process with a stack trace that reads, to whoever - * finds the red X, exactly like the gate itself being broken. - * - * The `readText` seam exists so the self-test drives every one of these arms - * without a temp file; production passes the real reader. - */ -export function readCommits(env, readText = (p) => readFileSync(p, 'utf8')) { - const path = env[COMMITS_FILE_ENV]; - if (typeof path !== 'string' || path.trim() === '') { - return { - commits: null, - commitsProblem: `${COMMITS_FILE_ENV} names no file, so the PR's commit messages were never read.`, - }; - } - - let text; - try { - text = readText(path.trim()); - } catch (err) { - return { commits: null, commitsProblem: `${COMMITS_FILE_ENV} names ${path.trim()}, which could not be read — ${err.message}` }; - } - - const parsed = parseCommitRecords(text); - if (parsed.problem) return { commits: null, commitsProblem: `the commit list at ${path.trim()} is malformed — ${parsed.problem}` }; - if (parsed.rows.length === 0) { - return { - commits: null, - commitsProblem: - `the commit list at ${path.trim()} is EMPTY. Every pull request has at least one commit, so this ` + - 'is a gather that failed, not a PR with nothing in it — and it must not be read as a clean commit history.', - }; - } - return { commits: parsed.rows, commitsProblem: null }; -} - /** * The verdict: `{ exit, lines }`, pure, so the self-test drives it directly. * @@ -685,8 +400,7 @@ export function judge(ctx) { 'verdict: it says nothing about whether any PR body contradicts itself, and no author caused it.', '', `Fix: run it from the workflow that supplies the context (${WIRING_WORKFLOW}), or locally with`, - ` PR_BODY="$(cat some-body.md)" ${COMMITS_FILE_ENV}=some-commits.jsonl \\`, - ' node scripts/check-partof-closing-keyword.mjs', + ' PR_BODY="$(cat some-body.md)" node scripts/check-partof-closing-keyword.mjs', ], }; } @@ -697,13 +411,8 @@ export function judge(ctx) { // construction — a number already declared Part of is RULE 1's, and H21 skips // it — so one number can never be reported twice in one verdict. const negated = h21NegatedClosingKeyword({ body: ctx.body }); - const commitFindings = ctx.commits ? commitTrailerFindings(ctx.commits) : []; - - // The unread half is named wherever it exists, on EVERY exit path — a run - // that judged one surface must never present itself as one that judged both. - const unread = ctx.commitsProblem ? [` ⚠️ RULE 2 judged nothing: ${ctx.commitsProblem}`] : []; - if (contradiction || negated || commitFindings.length) { + if (contradiction || negated) { const lines = []; if (contradiction) { lines.push( @@ -733,8 +442,8 @@ export function judge(ctx) { ' to a reader while saying nothing to the parser. If the keyword must stay in the prose, put', ' it in BACKTICKS: a pull request body is markdown, the parser does not fire inside a code', ' span, and that was measured live rather than assumed. ⛔ Backticks are NOT the escape in a', - ' commit message — nothing renders one, so they are ordinary characters there and RULE 2', - ' above is the rule that surface answers to.', + ' commit message — nothing renders one, so they are ordinary characters there, and the', + ' pre-push card-trailer refusal is the check that surface answers to.', '', ' Why this is blocking rather than advisory: this body declares no `Part of`, so RULE 1 is', ' silent on it by construction, however plainly the sentence says the card stays open. The', @@ -743,70 +452,7 @@ export function judge(ctx) { ' Editing the body re-runs this check; no push and no re-run are needed.', ); } - if (commitFindings.length) { - if (contradiction || negated) lines.push(''); - for (const finding of commitFindings) lines.push(`::error::${finding}`); - lines.push( - '', - `✗ check:partof-closing-keyword: ${commitFindings.length} commit message(s) on ${where} carry a`, - ' card-relation trailer. The PR body is the only carrier of the relation.', - '', - ); - for (const finding of commitFindings) lines.push(` ${finding}`, ''); - lines.push( - ' ⛔ The repair is NOT a history rewrite. Amend, rebase and force-push are forbidden in this', - ' repository and this gate never asks for one. What it does ask for turns on one thing only:', - '', - ' BRANCH NOT PUSHED YET — reword the commit messages now and push once. Nothing published is', - ' rewritten, the relation goes in the PR body where the contract puts it, and this check is green.', - '', - ' BRANCH ALREADY PUSHED — no author action clears this red, and that is expected rather than a', - " problem to solve. This gate reads the PR's COMMIT LIST, so a new commit on top JOINS that list", - ' and leaves the message above in it; the only thing that would remove it is the rewrite forbidden', - ' above. Three measured facts, so this red can be READ rather than acted on:', - '', - ' 1. This check run is advisory at the branch-protection layer: it is absent from the', - ' required-context registry, and its workflow subscribes to no `merge_group` event because a', - ' queue build carries no PR body for it to judge.', - ' 2. The squash message is assembled from the COMMIT messages, not from the PR body. Measured on', - " 0a61db1f5, the squash of PR #16646: the landed message is the branch commit's, verbatim,", - " carrying its `Refs #16624`; the body's `Fixes #16624` is nowhere in it.", - ' 3. The card relation is safe either way. #16624 closed on that same merge although no commit', - " message named a closing keyword for it — the PR BODY's keyword is what acts. Declaring the", - ' relation once in the body is the whole contract, and it already works.', - '', - ' So the residue of landing this red is the trailer above sitting in the permanent history, and', - ' what that costs depends on its spelling: `Part of` and `Refs` land as a reference and move no', - " card, while a CLOSING keyword lands on the surface GitHub's parser reads. ⛔ Weigh that under the", - ' landing rules that bind you — this gate found a real contradiction between your commits and your', - ' body, and it does not decide whether the pull request merges.', - '', - ' What DISCHARGES it is the merge, and only a merge whose squash message is the PR BODY: then the', - ' trailer above never reaches the default branch. Whether that happens by itself is the repository', - ' setting `squash_merge_commit_message`. At `COMMIT_MESSAGES` (the state fact 2 measured) it takes', - ' the lander replacing the assembled commit list with the PR body BY HAND at the merge button — a', - ' queue merge edits nothing, so there the residue lands. At `PR_BODY` every squash does it, queue', - ' included. Until that merge the red stays on this branch; ⛔ discharging it never rewrites history.', - ); - } - return { exit: EXIT_CONTRADICTION, lines: [...lines, ...(unread.length ? ['', ...unread] : [])] }; - } - - if (ctx.commitsProblem) { - return { - exit: EXIT_NOT_WIRED, - lines: [ - `check:partof-closing-keyword: PARTLY WIRED — ${where}'s body was read and carries no Part-of/closing-keyword`, - 'contradiction, but its COMMIT MESSAGES were not read, so RULE 2 judged nothing. This is a wiring', - 'failure, NOT a verdict: reporting an unread commit history as a clean one is the exact shape this', - 'gate exists to refuse.', - '', - ` ${ctx.commitsProblem}`, - '', - `Fix: run it from the workflow that supplies the context (${WIRING_WORKFLOW}), which writes the PR's`, - ` commits as JSON Lines and names that file in ${COMMITS_FILE_ENV}.`, - ], - }; + return { exit: EXIT_CONTRADICTION, lines }; } const what = ctx.body.trim() === '' ? 'has an empty body, which can carry no' : 'carries no'; @@ -814,8 +460,7 @@ export function judge(ctx) { exit: EXIT_CLEAN, lines: [ `✓ check:partof-closing-keyword: ${where} ${what} Part-of/closing-keyword contradiction and no`, - ' closing keyword bound to a card its own sentence says it is not closing, and its', - ` ${ctx.commits.length} commit message(s) carry no card-relation trailer.`, + ' closing keyword bound to a card its own sentence says it is not closing.', ], }; } @@ -842,20 +487,7 @@ function selfTest() { registerCase(); return cases.push([name, actual, expected]); }; - // Every RULE 1 case below judges a BODY, so each is handed a commit list that - // is present and clean. Without one they would all exit 2 on the unread half - // and stop testing the thing they were written to test — and the day that - // happened they would still print, which is what the battery floor is for. - const CLEAN_COMMITS = [{ sha: 'a1b2c3d4e5f60718', message: 'chore(ci): a subject carrying no card relation\n\nBody prose.\n' }]; - const verdict = (body, number = '1') => judge({ number, body, commits: CLEAN_COMMITS, commitsProblem: null }); - /** A verdict over COMMITS, with a body that is clean under RULE 1. */ - const commitVerdict = (...messages) => - judge({ - number: '1', - body: 'A body with no relation declared in it.', - commits: messages.map((message, i) => ({ sha: `${i}0deadbeef1234567`, message })), - commitsProblem: null, - }); + const verdict = (body, number = '1') => judge({ number, body }); // --- The measured arms. All three were read live on one throwaway PR, in one // body, at one moment, with the PR OPEN and unmerged: the plain-prose target @@ -914,7 +546,7 @@ function selfTest() { 'the verdict is exactly the shipped predicate over every fixture (no forked rule)', bodies.every( (body) => - (judge({ number: '1', body, commits: CLEAN_COMMITS, commitsProblem: null }).exit === EXIT_CONTRADICTION) === + (judge({ number: '1', body }).exit === EXIT_CONTRADICTION) === (h7PartOfWithClosingKeyword({ body }) !== null), ), true, @@ -989,253 +621,6 @@ function selfTest() { battery('The predicate source this gate reuses must still be there to reuse.'); t('the predicate source exists', existsSync(join(ROOT, PREDICATE_SOURCE)), true); - // --- RULE 2. Every spelling the ruling names, plus the shapes that must stay - // green so the rule does not tax ordinary commit prose. - battery('RULE 2 — every card-relation spelling in a commit message is a finding,'); - for (const spelling of ['Fixes', 'Closes', 'Resolves', 'Part of', 'Refs']) { - t( - `a commit message carrying "${spelling}" bound to a card is a finding`, - commitVerdict(`fix(x): a subject\n\n${spelling} #4242\n`).exit, - EXIT_CONTRADICTION, - ); - } - const named = commitVerdict('fix(x): the subject that must be quoted back\n\nFixes #4242\n').lines.join('\n'); - t('the finding names the offending commit by short sha', named.includes('`00deadbee`'), true); - t('the finding quotes the commit subject back', named.includes('the subject that must be quoted back'), true); - t('the finding names the trailer and the card it binds', named.includes('`Fixes #4242`'), true); - t( - 'the finding CITES the contract rather than restating it in its own words', - named.includes(RELATION_CONTRACT), - true, - ); - // The case above compares the printed finding with THIS FILE'S constant, so - // both sides move together whenever the constant is edited: a sentence that - // was never in the rules file passes it, and one did — see the constant's - // docblock. These three hold the quotation to its SOURCE instead. An absent - // rules file reds here rather than passing quietly: a citation check that - // cannot read the cited file has verified nothing. - const agentRulesPath = join(ROOT, AGENT_RULES_SOURCE); - const agentRules = existsSync(agentRulesPath) ? readFileSync(agentRulesPath, 'utf8') : ''; - const citedFromRules = citedSentences(RELATION_CONTRACT); - t(`the cited rules file is readable (${AGENT_RULES_SOURCE})`, agentRules !== '', true); - t('the citation carries quoted sentences at all (never a vacuous zero)', citedFromRules.length > 0, true); - t( - 'every sentence inside the corner brackets is verbatim in the cited rules file', - citedFromRules.filter((sentence) => !agentRules.includes(sentence)), - [], - ); - // The guidance text itself, pinned. This gate once told the author to push - // reworded commits while also forbidding a rewrite — unsatisfiable on a - // pushed branch, and a seat that read it instructed an amend and force-push. - // These six hold the repaired shape: the forbidden action stays forbidden, - // the reachable remedy is scoped to an UNPUSHED branch, the pushed case is - // named as permanent, the measured squash fact replaces the false one, and - // the gate still refuses to decide the merge. - t( - 'the guidance still forbids amend, rebase and force-push', - /⛔ Do NOT amend, rebase or force-push/.test(named), - true, - ); - t( - 'the reachable remedy is scoped to a branch that is NOT pushed yet', - named.includes('UNPUSHED') && named.includes('BRANCH NOT PUSHED YET'), - true, - ); - t( - 'the already-pushed case is named, and named as clearing for nobody', - named.includes('BRANCH ALREADY PUSHED') && named.includes('no author action clears this red'), - true, - ); - t( - 'the guidance no longer claims the squash message comes from the PR body', - /squash message from that body/.test(named), - false, - ); - t( - 'the guidance states the MEASURED squash fact instead', - named.includes('assembled from the COMMIT messages, not from the PR body') && named.includes('0a61db1f5'), - true, - ); - t( - 'the guidance leaves the landing decision to the lander rather than ordering a merge', - named.includes('does not decide whether the pull request merges'), - true, - ); - // The discharge sentence. Saying that nothing clears the red on the branch is - // half of the pushed case; the other half is WHAT discharges it — the merge, - // and only a merge whose squash message is the PR body — and the setting that - // decides whether that is by hand or automatic. It is stated as a CONDITION on - // the setting's value, never as today's value, so these three hold whichever - // way the repository is configured when they run; and the sentence must sit - // beside the prohibition, never in place of it. - t( - 'the already-pushed case says what DISCHARGES the red: a merge whose squash message is the PR body', - named.includes('What DISCHARGES it is the merge') && named.includes('squash message is the PR BODY'), - true, - ); - t( - 'and names the setting that decides by-hand versus automatic, with both of its values', - named.includes('`squash_merge_commit_message`') && - named.includes('`COMMIT_MESSAGES`') && - named.includes('`PR_BODY`') && - named.includes('BY HAND'), - true, - ); - t( - 'and the discharge sentence sits BESIDE the prohibition, never in place of it', - named.includes('discharging it never rewrites history') && /⛔ Do NOT amend, rebase or force-push/.test(named), - true, - ); - t( - 'an ordinary commit message with no card relation is clean', - commitVerdict('fix(cli): stop counting the walk instead of the tree\n\nBody prose about the change.\n').exit, - EXIT_CLEAN, - ); - t( - 'the squash subject marker `(#N)` is not a card relation', - commitVerdict('fix(cli): report key counts off the emitted bytes (#16247)\n').exit, - EXIT_CLEAN, - ); - t( - 'the word closing is still not a closing keyword on this surface either', - commitVerdict('docs: explain why closing #4242 by hand would drop the severe half\n').exit, - EXIT_CLEAN, - ); - t( - 'the harness trailer pair carries no card number and stays clean', - commitVerdict('fix(x): a subject\n\nCo-Authored-By: Someone \nClaude-Session: https://example.invalid/session_0\n').exit, - EXIT_CLEAN, - ); - - // --- The regression fixture. This is the specimen the card was filed on, and - // it is the argument for RULE 2 being WIDER than the sweep's H23: the squash - // carries a closing trailer and NO Part-of, so the contradiction row is silent - // on it while the assembly it produced contradicts itself in plain sight. - battery('The regression fixture: the squash that assembled a contradiction no'); - const FIXTURE_SQUASH = [ - 'fix(cli): report `os i18n extract` key counts off the emitted bytes (#16247)', - '', - '* fix(cli): report i18n extract key counts off the emitted bytes', - '', - 'Two consequences of the same conflation go with it: the emit gate is now', - "the module's own leaf count; and `--json`'s `counts` now counts the", - '`bundles` payload beside it, as `metadataFormsCounts` already counted', - '`metadataForms`.', - '', - 'Fixes #16121', - '', - '* fix(cli): unbreak the pin\'s typecheck, drop a false symmetry claim', - '', - '2. The changeset, the PR body and the `--json` comment all claimed the new', - ' `counts`/`bundles` relationship was "the relationship `metadataFormsCounts`', - ' already had to `metadataForms`". It is not.', - ].join('\n'); - const fixture = commitVerdict(FIXTURE_SQUASH); - t('the fixture squash message is a finding', fixture.exit, EXIT_CONTRADICTION); - t('the fixture finding names the trailer it found', fixture.lines.join('\n').includes('`Fixes #16121`'), true); - t( - 'the fixture is INVISIBLE to the contradiction rule — it declares no Part-of', - partOfTargets(FIXTURE_SQUASH, { markdown: false }).size, - 0, - ); - t( - 'the fixture body alone is clean under RULE 1, which is why RULE 2 exists', - h7PartOfWithClosingKeyword({ body: FIXTURE_SQUASH }), - null, - ); - - // --- Delegation again, on the second surface: the reading must be the sweep's - // `markdown: false`, or an author who "quotes" the trailer gets a green for a - // trailer GitHub still acts on. - battery('RULE 2 delegates to the sweep extractors at the commit-message reading.'); - t( - 'a trailer inside backticks in a COMMIT is still a finding (a commit is not markdown)', - commitVerdict('fix(x): a subject\n\n`Fixes #4242`\n').exit, - EXIT_CONTRADICTION, - ); - t( - 'a trailer inside a fenced block in a COMMIT is still a finding', - commitVerdict('fix(x): a subject\n\n```\nFixes #4242\n```\n').exit, - EXIT_CONTRADICTION, - ); - t( - 'the relations found are exactly the sweep extractors\' union over the message', - commitRelations('Part of #1 and Refs #2 and Fixes #3').map((r) => `${r.keyword} #${r.card}`), - ['Part of #1', 'Refs #2', 'Fixes #3'], - ); - - // --- The commit list input. Every way it can be missing must land on a - // verdict that says the rule judged nothing. - battery('The commit list input. An absent, broken or empty list can never read'); - const noFile = readCommits({}); - t('an absent file name is not a commit list', noFile.commits, null); - t('an absent file name says the messages were never read', noFile.commitsProblem.includes('never read'), true); - t('an empty file name is treated as absent', readCommits({ [COMMITS_FILE_ENV]: ' ' }).commits, null); - t( - 'an unreadable file is a problem, not a crash', - readCommits({ [COMMITS_FILE_ENV]: 'x.jsonl' }, () => { - throw new Error('ENOENT'); - }).commits, - null, - ); - t( - 'a malformed line is a problem, never a silently dropped row', - readCommits({ [COMMITS_FILE_ENV]: 'x.jsonl' }, () => '{"sha":"a","message":"m"}\nnot json\n').commitsProblem !== null, - true, - ); - t( - 'a row missing its message is a problem', - readCommits({ [COMMITS_FILE_ENV]: 'x.jsonl' }, () => '{"sha":"a"}\n').commitsProblem !== null, - true, - ); - t( - 'an EMPTY list is a failed gather, never a clean history', - readCommits({ [COMMITS_FILE_ENV]: 'x.jsonl' }, () => '\n\n').commitsProblem?.includes('EMPTY'), - true, - ); - t( - 'a well-formed list parses to its rows, blank lines ignored', - readCommits({ [COMMITS_FILE_ENV]: 'x.jsonl' }, () => '{"sha":"a","message":"m"}\n\n{"sha":"b","message":"n"}\n').commits - ?.length, - 2, - ); - - // --- The verdict layer: what wins when both apply, and the rule that an - // unjudged half is always SAID. - battery('The verdict layer over two rules: precedence, and the unread half is'); - const partly = judge({ number: '1', body: 'A clean body.', commits: null, commitsProblem: 'the list was not read.' }); - t('a body-only run is NOT WIRED, never clean', partly.exit, EXIT_NOT_WIRED); - t('a body-only run does not read as a clean board', partly.lines.join('\n').includes('✓'), false); - t('a body-only run says which rule judged nothing', partly.lines.join('\n').includes('RULE 2 judged nothing'), true); - const both = judge({ - number: '1', - body: 'Part of #4242 — close #4242 once the rest lands.', - commits: null, - commitsProblem: 'the list was not read.', - }); - t('a real finding outranks a half-wired run', both.exit, EXIT_CONTRADICTION); - t('and the half-wired run is still named in the output', both.lines.join('\n').includes('RULE 2 judged nothing'), true); - - // --- The wiring's second half: the step that gathers the commit list. - battery('The wiring gathers the commit list and hands it over as a file path.'); - t('the wiring names the commit-list file variable', wiring.includes(`${COMMITS_FILE_ENV}:`), true); - t( - 'the wiring reads the PR commits endpoint rather than walking a shallow clone', - /pulls\/[^\n]*\/commits/.test(wiring), - true, - ); - t('the wiring pages the endpoint, so a long PR is not silently truncated', wiring.includes('--paginate'), true); - t( - 'the wiring grants the read scope that endpoint needs', - /permissions:[\s\S]{0,200}?pull-requests:\s*read/.test(wiring), - true, - ); - t( - 'the commit list reaches the script as a PATH in env, never as a body of text in it', - new RegExp(`${COMMITS_FILE_ENV}:\\s*\\$\\{?\\{?\\s*(RUNNER_TEMP|runner\\.temp)`, 'i').test(wiring), - true, - ); - // --- RULE 3 — a closing keyword bound to a card the sentence says it is NOT // closing. The sweep's H21, called. Every fixture below is a real sentence: // the two positives are measured specimens quoted byte-for-byte, and the @@ -1300,7 +685,7 @@ function selfTest() { 'the verdict is exactly the two shipped predicates over every fixture (no forked rule)', negatedBodies.every( (body) => - (judge({ number: '1', body, commits: CLEAN_COMMITS, commitsProblem: null }).exit === EXIT_CONTRADICTION) === + (judge({ number: '1', body }).exit === EXIT_CONTRADICTION) === (h7PartOfWithClosingKeyword({ body }) !== null || h21NegatedClosingKeyword({ body }) !== null), ), true, diff --git a/scripts/pm/check-half-states.mjs b/scripts/pm/check-half-states.mjs index fd4cca05e4..4b1dc895de 100644 --- a/scripts/pm/check-half-states.mjs +++ b/scripts/pm/check-half-states.mjs @@ -1787,8 +1787,37 @@ function closingKeywordRe() { return /\b(clos(?:e|es|ed)|fix(?:es|ed)?|resolv(?:e|es|ed))\b[ \t]*:?[ \t]*#(\d+)\b/gi; } +/** + * `Part of #N` — and every spelling of it this repository's dispatch orders + * forbid, which is not the same set as the one a space-only pattern matched. + * + * MEASURED (2026-09-11, the spec seat, on one commit message fed to the gate + * that reads this): `Part of #17184` exited 1 and `Part-of: #17184` exited 0. + * The hyphen and the colon are the two spellings every `os-dev` dispatch order + * lists as forbidden, so a dev who greps for `Part-of` before pushing, gets 0, + * and trusts the gate to be the net underneath is reading a net with no thread + * in that square. The instruction surface and the enforcement surface were out + * of step at exactly the step that tells the dev to verify it themselves. + * + * The separator is therefore the one `closingKeywordRe` already ships — an + * optional colon between the word and the reference — and the word itself + * accepts a hyphen where it accepts a space. `Part-of:` is also the GIT TRAILER + * spelling (`Token: value`), which is the shape a commit message carries this + * relation in at all, so it is the spelling most likely to be typed. + * + * ⚠️ The widening is a STRICT SUPERSET of what this matched before — the + * whitespace run stays optional-colon-tolerant rather than being replaced by + * `[ \t]`, so no body or message that matched yesterday stops matching today. + * The direction matters: a narrowing here silently empties a `partOf` set and + * drops a row through to a fallback channel (see `partOfDeclarationRe`'s + * measured corpus note), which is a finding lost rather than a finding gained. + * + * `Part` alone, `Parts of` and `part-of` in prose are deliberately NOT widened + * to: the protocol has these spellings, and widening the reader further is how + * a dialect gets a home (`refsRe` makes the same call for `Ref`/`References`). + */ function partOfRe() { - return /\bPart of\s+#(\d+)\b/gi; + return /\bPart[ \t-]of\b[ \t]*:?\s*#(\d+)\b/gi; } /** @@ -1831,7 +1860,7 @@ function partOfRe() { * position is reported instead. */ function partOfDeclarationRe() { - return /^[ \t]*(?:>[ \t]*)*(?:(?:[-*+]|\d{1,9}[.)])[ \t]+)?[*_]{0,3}Part of\s+#(\d+)\b/gim; + return /^[ \t]*(?:>[ \t]*)*(?:(?:[-*+]|\d{1,9}[.)])[ \t]+)?[*_]{0,3}Part[ \t-]of\b[ \t]*:?\s*#(\d+)\b/gim; } /** @@ -1948,7 +1977,10 @@ export function partOfDeclarationTargets(body, { markdown = true } = {}) { * with nothing on the board contradicting anything — H49's row. * * Same strictness as `partOfRe`, on purpose: the word is bound (`\\b`), the - * `#` follows on whitespace with no colon, the read is code-stripped for the + * `#` follows on whitespace or on the optional colon that every other relation + * in this file accepts — `Refs: #N` is the git-trailer spelling of this exact + * declaration and was as invisible here as `Part-of:` was there — the read is + * code-stripped for the * BODY surface (a body QUOTING `Refs #N` in backticks declares nothing), and a * fresh regex per call (the `lastIndex` note at `closingKeywordRe`). `Ref`, * `References` and `See #N` are deliberately NOT this relation: the protocol @@ -1961,7 +1993,7 @@ export function partOfDeclarationTargets(body, { markdown = true } = {}) { * inventing an item. */ function refsRe() { - return /\bRefs\s+#(\d+)\b(?:[ \t]*\(([^()\n]{1,40})\))?/gi; + return /\bRefs\b[ \t]*:?\s*#(\d+)\b(?:[ \t]*\(([^()\n]{1,40})\))?/gi; } /** `#N` -> the `(item k)` annotation beside it, or `null` when bare (first occurrence wins). */ @@ -19911,6 +19943,20 @@ async function selfTest() { t('extractor: `Part of` in a fence is invisible on the body surface', partOfTargets('```\nPart of #1\n```').size, 0); t('extractor: …and visible on the commit surface', partOfTargets('```\nPart of #1\n```', { markdown: false }).size, 1); t('extractor: the default is byte-identical to the pre-option reading', closingKeywordTargets('Fixes #1').get('1'), 'Fixes'); + // The `Part of` spellings every dispatch order forbids, each pinned on the + // COMMIT surface — the one a pre-push refusal reads. The hyphen and the colon + // were both invisible here while every order listed them, so a dev greppping + // for them got 0 and trusted a net with no thread in that square (measured by + // the spec seat on one commit message: the space spelling exited 1, the + // hyphen-colon spelling exited 0). + t('extractor: `Part of #N` — the spelling that always matched', partOfTargets('Part of #17184', { markdown: false }).has('17184'), true); + t('extractor: `Part-of #N` — the hyphen spelling', partOfTargets('Part-of #17184', { markdown: false }).has('17184'), true); + t('extractor: `Part of: #N` — the colon spelling', partOfTargets('Part of: #17184', { markdown: false }).has('17184'), true); + t('extractor: `Part-of: #N` — the git-trailer spelling, the measured miss', partOfTargets('Part-of: #17184', { markdown: false }).has('17184'), true); + t('extractor: the widening is a superset — a line break still reaches the reference', partOfTargets('Part of\n#17184', { markdown: false }).has('17184'), true); + t('extractor: ⛔ `Parts of #N` is not this relation', partOfTargets('Parts of #17184', { markdown: false }).size, 0); + t('extractor: ⛔ nor a `part` that only ends the word', partOfTargets('Counterpart of #17184', { markdown: false }).size, 0); + t('extractor: the declaration subset reads every spelling the relation does', [...partOfDeclarationTargets('Part-of: #17184', { markdown: false })].join(','), '17184'); // The REMEDY TEXT. The realistic regression is someone copying H7's tail // across, so H7's own sentence is asserted to CARRY the clause this one must @@ -25162,7 +25208,16 @@ Mutual exclusion: \`get_comments\` page 747 → \`[]\`, page 746 = my own R+117 t('refs extractor: case-insensitive, exactly as `partOfRe`', refsTargets('refs #16003').has('16003'), true); t('refs extractor: ⛔ `Ref #N` is not the protocol\'s spelling', refsTargets('Ref #16003').size, 0); t('refs extractor: ⛔ nor `References #N`', refsTargets('References #16003').size, 0); - t('refs extractor: ⛔ nor a colon, exactly as `Part of` takes none', refsTargets('Refs: #16003').size, 0); + // ⚠️ This pin USED to assert the opposite — that a colon was not this + // relation's spelling — and it was pinning the gap rather than a decision: + // `Refs: #N` is the git-trailer spelling of the same declaration, and the + // keyword extractor beside it has accepted `Fixes: #N` since the colon was + // measured against GitHub's own parser. Replaced rather than reworded: the + // old case passed for as long as the branch it named existed, which is the + // shape a fixture takes when what it really pins is a missing thread. + t('refs extractor: a colon IS the spelling, as at the keyword extractor', refsTargets('Refs: #16003').has('16003'), true); + t('refs extractor: …and the item annotation still reads through the colon', refsTargets('Refs: #16003 (item 2)').get('16003'), 'item 2'); + t('refs extractor: the word is still bound — `Prefs: #N` is not a reference', refsTargets('Prefs: #16003').size, 0); t('refs extractor: the word is bound — `Prefs #N` is not a reference', refsTargets('Prefs #16003').size, 0); t('refs extractor: several cards in one body are each read', [...refsTargets('Refs #1 (item ①) and Refs #2').keys()].join(','), '1,2'); t('refs extractor: a missing body is empty, never a crash', refsTargets(undefined).size, 0);