From 1d723fda0c367ca10fd82cfa730266d783995dec Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 1 Sep 2026 21:36:18 -0700 Subject: [PATCH 1/2] ci(adr): make a duplicate ADR number go red MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 the moment the second merged. The same thing had already happened at ADR-018, where the duplicate survived 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. Two checks, because the collision has two distinct lifetimes: 1. Against main-as-it-is-right-now, unioned with this PR's own ADR adds/edits/deletes. Catches a PR claiming a number main already uses. Verified against #1295's head: exactly one error, ADR-025. 2. Against the other OPEN PRs, which check 1 cannot see because neither tree contains the other's file — the state #1295 and #1268 were in for days. Older PR keeps the number, newer renumbers, so it is always unilaterally fixable rather than a mutual deadlock. Deliberately not the merge ref. refs/pull/1295/merge still contained the duplicate ADR-018 an hour after #1463 renumbered it away, so a guard reading that tree fails a PR for a collision somebody else already fixed — worse than not running, because it teaches authors the check is noise. Deliberately not contiguity: main has no 029 and that is fine. Both gh calls fail closed. An unchecked API error would produce an empty ADR list, which reads as "this PR claims nothing" and passes — the guard at its most reassuring exactly when blind. Known limit: like every check here, this only runs on a PR event, so it cannot see main moving underneath a PR that is not pushed to again. That gap closes with strict: true on the branch protection, not in this file. Co-Authored-By: Claude Opus 5 --- .github/workflows/adr-numbering-guard.yml | 142 ++++++++++++++++++++++ scripts/verify-adr-numbering.js | 80 ++++++++++++ 2 files changed, 222 insertions(+) create mode 100644 .github/workflows/adr-numbering-guard.yml create mode 100755 scripts/verify-adr-numbering.js diff --git a/.github/workflows/adr-numbering-guard.yml b/.github/workflows/adr-numbering-guard.yml new file mode 100644 index 0000000000..026053e057 --- /dev/null +++ b/.github/workflows/adr-numbering-guard.yml @@ -0,0 +1,142 @@ +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: + 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: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + adr-numbering: + name: ADR numbers are unique + 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.`); From caa7c4246e6d2fcc113305533b8ddaec30af7077 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Tue, 1 Sep 2026 21:42:57 -0700 Subject: [PATCH 2/2] ci(adr): add the main-side arm the PR arms structurally cannot cover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR arms are triggered by PR events, so a green freezes at the last one. Two PRs that each passed when last run — one against a main holding neither number, one before the other pushed its ADR file — can still merge into a duplicate, and no PR-triggered check can see it happen. This arm cannot prevent that either. 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. Reds main, deliberately: a duplicated 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. Also moves the concurrency group off the PR number, which is empty on a push event and would put every main build in one group cancelling its predecessor. Co-Authored-By: Claude Opus 5 --- .github/workflows/adr-numbering-guard.yml | 25 ++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/workflows/adr-numbering-guard.yml b/.github/workflows/adr-numbering-guard.yml index 026053e057..04d673b8af 100644 --- a/.github/workflows/adr-numbering-guard.yml +++ b/.github/workflows/adr-numbering-guard.yml @@ -12,6 +12,14 @@ name: ADR Numbering Guard # 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 @@ -24,12 +32,27 @@ permissions: pull-requests: read concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + # 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