diff --git a/.github/workflows/adr-numbering-guard.yml b/.github/workflows/adr-numbering-guard.yml new file mode 100644 index 0000000000..04d673b8af --- /dev/null +++ b/.github/workflows/adr-numbering-guard.yml @@ -0,0 +1,165 @@ +name: ADR Numbering Guard + +# An ADR number is a name two documents can claim at once, and every mechanical +# check we own is blind to it. #1295 and #1268 both added +# `docs/adr/ADR-025-*.md` under different slugs: different filenames, so no +# textual conflict, `merge-tree` clean, all checks green — and main carried two +# ADR-025s from the moment the second merged. It had already happened at +# ADR-018, where the duplicate lived long enough that #963's author followed +# the wrong one and shipped a wake-policy regression. +# +# Both were found by a human reading a directory listing. This makes the third +# one go red. + +on: + # The PR arms below cannot fire on base movement, and a PR's green is frozen + # at its last event. So a pair that were each green when last run can still + # merge into a duplicate. This arm cannot prevent that — nothing triggered by + # PR events can — but it makes main say so within a minute instead of waiting + # for someone to read a directory listing, which is how both known duplicates + # were actually found. + push: + branches: [ main ] + pull_request: + branches: [ main ] + # `edited` catches base-branch retargeting, same reasoning as + # package-version-guard.yml: a stacked PR retargeted to main after its + # parent merges otherwise enters this population with no event firing. + types: [opened, synchronize, reopened, ready_for_review, edited] + +permissions: + contents: read + pull-requests: read + +concurrency: + # github.ref, not the PR number: on a push event there is no PR to key on and + # every main build would share one group and cancel its predecessor. + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + # Detection, not prevention. If a duplicate reaches main, this reds main — + # which is loud, and deliberately so: a duplicated ADR number silently + # mis-routes every citation of it, and #963 shipped a wake-policy regression + # because an author followed the wrong member of the ADR-018 pair. + adr-numbering-main: + name: main carries no duplicate ADR number + if: github.event_name == 'push' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - run: node scripts/verify-adr-numbering.js + + adr-numbering: + name: ADR numbers are unique + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # The PR HEAD, not the merge ref. `refs/pull/N/merge` is recomputed + # lazily and can be badly stale: #1295's merge ref still contained the + # duplicate ADR-018 an hour after #1463 renumbered it away on main. A + # guard reading that tree fails the PR for a collision somebody else + # already fixed — worse than not running, because it teaches authors + # that this check is noise. We assemble the post-merge state below + # instead, from main as it is right now. + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 0 + + - name: No two ADRs claim the same number + shell: bash + run: | + set -euo pipefail + + # No --depth: a shallow base makes the fork point unreachable and + # every comparison below meaningless. Same trap as + # package-version-guard.yml (#1113), and no `|| true` for the same + # reason — a base we cannot fetch must stop the guard, not pass it. + git fetch --no-tags origin main + if ! base=$(git merge-base FETCH_HEAD HEAD); then + echo "::error::No merge base between origin/main and this PR head. Not passing on a baseline this guard cannot see." + exit 1 + fi + + # Effective tree = main's ADRs right now, with this PR's own ADR + # additions, edits and deletions applied on top. That is what main + # will hold after the merge, and unlike the merge ref it is fresh as + # of this run. + eff=$(mktemp -d) + mkdir -p "$eff/docs/adr" + git archive FETCH_HEAD docs/adr | tar -x -C "$eff" + + # --no-renames so a rename shows up as delete + add; rename detection + # would hide exactly the "renumbered but the H1 still says the old + # number" case this guard checks for. + git diff --no-renames --name-status "$base" HEAD -- docs/adr \ + | while IFS=$'\t' read -r status file; do + case "$status" in + D) rm -f "$eff/$file" ;; + *) mkdir -p "$eff/$(dirname "$file")" + git show "HEAD:$file" > "$eff/$file" ;; + esac + done + + node scripts/verify-adr-numbering.js --dir "$eff/docs/adr" + + # The check above cannot see a collision between two PRs that are both + # still open — neither tree contains the other's file. That is the state + # #1295 and #1268 were in for days, and the state a human happened to + # catch. Deterministic tiebreak so it is unilaterally fixable: the older + # PR keeps the number, the newer renumbers. Both PRs going red would be a + # deadlock neither author could clear alone. + - name: No OPEN PR is already claiming these numbers + env: + GH_TOKEN: ${{ github.token }} + PR: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + shell: bash + run: | + set -euo pipefail + + # Every gh call is checked. A silent API failure here would yield an + # empty ADR list, which reads as "this PR claims nothing" and PASSES + # — the guard would be at its most reassuring exactly when it had + # gone blind. + added_adrs() { + local raw + if ! raw=$(gh api "repos/$REPO/pulls/$1/files" --paginate \ + --jq '.[] | select(.status == "added") | .filename'); then + echo "::error::gh api failed reading PR #$1. This guard cannot see the open-PR set; it will not pass on a check it could not run." >&2 + return 1 + fi + printf '%s\n' "$raw" | sed -n 's#^docs/adr/ADR-\([0-9]\{3\}\)-.*\.md$#\1#p' | sort -u + } + + mine=$(added_adrs "$PR") + if [ -z "$mine" ]; then + echo "· this PR adds no new ADR file" + exit 0 + fi + echo "this PR claims: $(echo "$mine" | tr '\n' ' ')" + + # Older = lower PR number. Only an older PR can outrank this one. + if ! older=$(gh api "repos/$REPO/pulls?state=open&per_page=100" --paginate \ + --jq ".[] | select(.number < $PR) | .number"); then + echo "::error::gh api failed listing open PRs. Not passing on a blind check." + exit 1 + fi + + fail=0 + for other in $older; do + theirs=$(added_adrs "$other") + [ -z "$theirs" ] && continue + clash=$(comm -12 <(printf '%s\n' "$mine") <(printf '%s\n' "$theirs")) || true + [ -z "$clash" ] && continue + for n in $clash; do + echo "::error file=docs/adr::ADR-$n is already claimed by the older open PR #$other. Two open PRs adding the same number both merge green and leave main with a duplicate — this is how main got two ADR-025s. #$other keeps $n; renumber here." + done + fail=1 + done + + if [ "$fail" = "0" ]; then + echo "✓ no open PR claims these numbers" + fi + exit $fail diff --git a/scripts/verify-adr-numbering.js b/scripts/verify-adr-numbering.js new file mode 100755 index 0000000000..237d1b71a3 --- /dev/null +++ b/scripts/verify-adr-numbering.js @@ -0,0 +1,80 @@ +#!/usr/bin/env node +/** + * ADR numbering guard. + * + * An ADR number is a name two documents can claim at once, and nothing in the + * toolchain notices. #1295 and #1268 both added `docs/adr/ADR-025-*.md` under + * different slugs: no textual conflict, `merge-tree` clean, every check green, + * and main carried two ADR-025s the moment the second one merged. The same + * thing had already happened at ADR-018 (`agent-attention-claims` and + * `agent-identity`), where the duplicate survived long enough that a PR author + * followed the wrong one and shipped a wake-policy regression (#963). + * + * Both were caught by a human reading a directory listing. This makes the + * third one go red. + * + * Checks, against the working tree: + * 1. every file in docs/adr/ is named ADR-NNN-slug.md + * 2. no two files claim the same NNN + * 3. the H1 title's number matches the filename's number + * + * Deliberately NOT checked: contiguity. main is missing 029 and that is fine — + * a gap costs nothing, and requiring density would make every renumber a + * cascade. + */ +const fs = require('fs'); +const path = require('path'); + +// `--dir` lets CI point this at a materialised "what main would look like +// after this merges" tree, rather than at the checkout. See +// .github/workflows/adr-numbering-guard.yml for why the merge ref is not that. +const dirArg = process.argv.indexOf('--dir'); +const DIR = dirArg !== -1 && process.argv[dirArg + 1] + ? path.resolve(process.argv[dirArg + 1]) + : path.join(__dirname, '..', 'docs', 'adr'); +const FILENAME = /^ADR-(\d{3})-[a-z0-9-]+\.md$/; +// Titles use both "ADR-018 — x" and "ADR-018: x". Match the number only. +const H1 = /^#\s+ADR-(\d{3})\b/m; + +const errors = []; +const byNumber = new Map(); + +const files = fs.readdirSync(DIR).filter((f) => f.endsWith('.md') && f !== 'README.md').sort(); + +for (const file of files) { + const m = FILENAME.exec(file); + if (!m) { + errors.push(`${file}: not named ADR-NNN-slug.md (three digits, lowercase hyphenated slug).`); + continue; + } + const num = m[1]; + if (!byNumber.has(num)) byNumber.set(num, []); + byNumber.get(num).push(file); + + const body = fs.readFileSync(path.join(DIR, file), 'utf8'); + const h1 = H1.exec(body); + if (!h1) { + errors.push(`${file}: no H1 of the form "# ADR-${num} — Title".`); + } else if (h1[1] !== num) { + errors.push( + `${file}: filename says ADR-${num}, H1 says ADR-${h1[1]}. A renamed file whose title still ` + + `carries the old number is cited by BOTH numbers and found by neither.` + ); + } +} + +for (const [num, dupes] of [...byNumber].sort()) { + if (dupes.length > 1) { + errors.push( + `ADR-${num} is claimed by ${dupes.length} files: ${dupes.join(', ')}. ` + + `Two documents under one number make every citation of it ambiguous — renumber all but one.` + ); + } +} + +if (errors.length) { + for (const e of errors) console.error(`::error file=docs/adr::${e}`); + console.error(`\n${errors.length} ADR numbering problem(s) in ${files.length} files.`); + process.exit(1); +} +console.log(`✓ ${files.length} ADRs, ${byNumber.size} distinct numbers, no collisions, every H1 agrees with its filename.`);