Skip to content

Commit 3553298

Browse files
huangyiireneclaude
andauthored
fix(devx): teach check:adr-anchors that a tombstone is not a decision (#7329) (#7838)
* fix(devx): teach `check:adr-anchors` that a tombstone is not a decision (#7329) `docs/adr/0107-withdrawn-hook-body-write-set-static-gap.md` (#6676) is the corpus's first tombstone — a file whose entire content is "this number is withdrawn, do not reuse". It was written so the historical `ADR-0107` citations resolve, and it does that. But the gate assembled its record set from filenames alone, so the same file also satisfied the anchor loop's `records.has(...)` guard, which was never meant to accept it. Measured on `main` @ 69fde55: a shard in `scripts/adr-anchors/` citing `ADR-0107` made the gate print OK and exit 0 — live content anchored to a number whose own record says nothing in it is in force. The only thing standing in the way was a reader noticing that the required `invariant` field cannot be written truthfully for a number that decided nothing. The two audits want opposite answers about the same file, so one set was not enough. A tombstone number is now in `records` (citations resolve — the job it exists for) and in `nonDecisions` (anchors refused). No live record's treatment changes. The marker is the filename, `NNNN-withdrawn-<slug>.md`, for the reason the `.vN` rule and the cross-repo qualifier are also spellings rather than lists: a structural signal reaches every future record without editing the gate. A `status:` field in YAML front-matter was the more obvious design and lost on three measurements — zero of the 119 records carry front-matter today; the records' status lines are free prose and already outside this gate's rules; and #6741 routes any `docs/adr/**` diff to the maintainer's own merge, coupling a script-only fix to a governance approval while failing open the same way. An explicit list was rejected as the one option needing a gate edit per tombstone. The anchor loop's ADR-id judgement is extracted into `anchorIdProblem` so `--self-test` drives the real function: 15 new assertions cover both directions over the same file, the `.vN` case, the prefix-not-substring boundary, the contested-number case (a tombstone sharing a number with a live record is a collision, and the live record keeps its anchors), and an ablation on the real tree replaying the exact probe that came back green on `main`. Refs #6676, #6634, #5992. * chore(devx): drop the empty changeset — route 2 (#7329) This PR touches `scripts/` only and releases nothing, so it takes the `skip-changeset` route the gate names rather than declaring a release it does not make. An empty-frontmatter changeset is a real input to changesets/action and an all-empty set stalls the release silently and greenly (#4898); the pre-existing empty ones on the base commit are grandfathered and this gate judges only what a PR newly introduces (#5471). The reasoning that changeset carried is in the commit message and the PR body, which is where it is readable without a release cycle. --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 255f2d7 commit 3553298

1 file changed

Lines changed: 249 additions & 21 deletions

File tree

scripts/check-adr-anchors.mjs

Lines changed: 249 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,43 @@
109109
// resolves the historical citations and makes re-use collide loudly under
110110
// the number-uniqueness audit instead of merely going stale here.
111111
//
112+
// ## The fourth thing it checks: a tombstone is NOT anchorable (#7329)
113+
//
114+
// The tombstone remedy above buys the citation audit exactly what it wanted —
115+
// `ADR-0107` in a changeset now resolves — and it did so by putting a FILE under
116+
// `docs/adr/`. But `records` is assembled from filenames alone, so the same file
117+
// also satisfied the anchor loop's `records.has(...)` guard, which was never meant
118+
// to accept it: an entry in `adr-anchors/` could anchor live code to a number whose
119+
// record says, in its own words, that "nothing in it is in force". Measured on
120+
// `main` @ `69fde55` — a shard citing `ADR-0107` made this gate print OK and exit 0.
121+
//
122+
// The two audits want opposite answers about the same file, so one set is not
123+
// enough. A tombstone number is now in `records` (citations resolve — wanted) AND
124+
// in `nonDecisions` (anchors are refused — the gap this closes). Nothing about a
125+
// live record's treatment changes.
126+
//
127+
// **The marker is the filename**, `NNNN-withdrawn-<slug>.md`, for the reason the
128+
// `.vN` rule and the cross-repo qualifier are also spellings rather than lists: a
129+
// structural signal is available to every future record without editing this file,
130+
// and it is legible to a human reading `ls docs/adr/`. Two alternatives were
131+
// weighed and rejected:
132+
//
133+
// - **A `status:` field in YAML front-matter.** Self-describing, and the more
134+
// obvious design — but ZERO of the 119 records carry front-matter today, so it
135+
// is a new repo-wide convention introduced for one file; the records' status
136+
// lines are deliberately free prose and deliberately outside this gate's rules
137+
// (see the collision section above); and #6741 routes ANY diff touching
138+
// `docs/adr/**` to the maintainer's own merge, so marking the tombstone would
139+
// couple a script-only fix to a governance approval. It also fails open in
140+
// exactly the way the filename does — a future tombstone can forget either one.
141+
// - **An explicit `NON_DECISION_RECORDS` list here.** Cannot misfire, but it is
142+
// the one option that needs this file edited for every future tombstone, which
143+
// is the property the other two are chosen for.
144+
//
145+
// A number counts as a non-decision only when EVERY stem claiming it is a
146+
// tombstone. A tombstone sharing its number with a live record is a collision, and
147+
// the audit above already reports that, loudly and about the right fact.
148+
//
112149
// ## Where the registry lives (#6957)
113150
//
114151
// `scripts/adr-anchors/` — **one JSON file per anchor**, named after the path it
@@ -173,6 +210,24 @@ const ADR_FILENAME = /^(\d{4})-([a-z0-9]+(?:-[a-z0-9]+)*)(?:\.v(\d+))?\.md$/;
173210
*/
174211
const NON_RECORD_FILES = new Set(['PRIORITIZATION.md']);
175212

213+
/**
214+
* The slug prefix that marks a record as a TOMBSTONE — a file whose whole content
215+
* is "this number is withdrawn, do not reuse" (`0107-withdrawn-hook-body-write-
216+
* set-static-gap.md`, the first one, #6676).
217+
*
218+
* A tombstone is a record for the purpose of RESOLVING citations, which is the
219+
* job it exists to do, and is NOT a record for the purpose of ANCHORING code:
220+
* an anchor's `invariant` has to state what the ADR decided, and this number
221+
* decided nothing. See the "#7329" section of the header for why the marker is
222+
* the filename and not a status field or a list.
223+
*
224+
* It is a prefix of the SLUG, not a substring of the name, so a live decision
225+
* *about* withdrawing something (`0123-withdrawn-plugin-cleanup-policy.md` would
226+
* be the collision to worry about) has a spelling that stays clear of it: name
227+
* the decision for what it decides.
228+
*/
229+
const TOMBSTONE_SLUG_PREFIX = 'withdrawn-';
230+
176231
/**
177232
* An ADR citation as written in prose or a code comment, with the word before it
178233
* (if any) captured so a SIBLING REPO's registry can be recognised.
@@ -238,6 +293,9 @@ const UNRESOLVED_ADR_CITATIONS = [
238293
// awaiting release plus an audit, so the grandfather clause would have expired
239294
// on its own and quietly re-freed the number. A record does not expire, and a
240295
// re-use now collides in `auditAdrDirectory` — loud, and about the right fact.
296+
// The one thing the tombstone bought that this list never could — resolving the
297+
// citations — is also the one thing it must NOT buy for anchors, which is why
298+
// `nonDecisions` exists (#7329).
241299
];
242300

243301
/**
@@ -304,14 +362,18 @@ const { anchors, errors: registryErrors } = loadAnchors(ROOT);
304362
*
305363
* @param {string[]} filenames
306364
* @param {{ number: string, decisions: string[], why: string }[]} allowlist
307-
* @returns {{ records: Set<string>, errors: string[] }} `records` is the set of
308-
* numbers that name a real record, as the anchor loop below needs it.
365+
* @returns {{ records: Set<string>, nonDecisions: Set<string>, errors: string[] }}
366+
* `records` is the set of numbers that name a real record, as the citation audit
367+
* needs it; `nonDecisions` is the subset of those whose every claimant is a
368+
* tombstone, as the anchor loop needs it (#7329).
309369
*/
310370
function auditAdrDirectory(filenames, allowlist) {
311371
const errors = [];
312372
const records = new Set();
313373
/** number → set of decision stems claiming it. */
314374
const stemsByNumber = new Map();
375+
/** number → set of those stems that are tombstones. */
376+
const tombstoneStemsByNumber = new Map();
315377

316378
for (const name of [...filenames].sort()) {
317379
if (!name.endsWith('.md') || NON_RECORD_FILES.has(name)) continue;
@@ -332,6 +394,18 @@ function auditAdrDirectory(filenames, allowlist) {
332394
const stem = `${number}-${slug}`;
333395
if (!stemsByNumber.has(number)) stemsByNumber.set(number, new Set());
334396
stemsByNumber.get(number).add(stem);
397+
if (slug.startsWith(TOMBSTONE_SLUG_PREFIX)) {
398+
if (!tombstoneStemsByNumber.has(number)) tombstoneStemsByNumber.set(number, new Set());
399+
tombstoneStemsByNumber.get(number).add(stem);
400+
}
401+
}
402+
403+
// A number is a non-decision only if EVERY stem claiming it is a tombstone. A
404+
// tombstone sharing a number with a live record is a collision, reported as one
405+
// below — and until it is resolved, the live record keeps its anchors.
406+
const nonDecisions = new Set();
407+
for (const [number, tombstones] of tombstoneStemsByNumber) {
408+
if (tombstones.size === stemsByNumber.get(number).size) nonDecisions.add(number);
335409
}
336410

337411
/** number → the sorted stems sharing it, for numbers claimed more than once. */
@@ -386,7 +460,39 @@ function auditAdrDirectory(filenames, allowlist) {
386460
}
387461
}
388462

389-
return { records, errors };
463+
return { records, nonDecisions, errors };
464+
}
465+
466+
/**
467+
* Decide whether an anchor entry may cite `adr` — the whole ADR-id judgement of
468+
* the anchor loop, extracted so its red paths are exercised by `--self-test`
469+
* rather than only by a real registry someone would have to break on purpose.
470+
*
471+
* @param {string} adr the id as written in the shard, e.g. `ADR-0090`
472+
* @param {Set<string>} records numbers that name a real record
473+
* @param {Set<string>} nonDecisions the subset that are tombstones
474+
* @returns {string|null} why it may not, or null if it may
475+
*/
476+
function anchorIdProblem(adr, records, nonDecisions) {
477+
const m = ADR_ID.exec(adr);
478+
if (!m) return `"${adr}" is not an ADR id (expected e.g. ADR-0090).`;
479+
// Anchoring a never-written or deleted record sends the next author to a
480+
// dead end (cf. ADR-0001, whose record was deleted in the 2026-02-11
481+
// permission-protocol rewrite and never restored).
482+
if (!records.has(m[1])) return `${adr} has no record under ${ADR_DIR}/ — anchor a decision that exists.`;
483+
if (nonDecisions.has(m[1])) {
484+
return (
485+
`${adr} names a WITHDRAWN record, not a decision — nothing may be anchored to it (#7329).\n` +
486+
` ${ADR_DIR}/${m[1]}-${TOMBSTONE_SLUG_PREFIX}*.md is a tombstone: it exists so the number RESOLVES for ` +
487+
'the citations that discuss its withdrawal, and so the number can never be handed to an unrelated ' +
488+
'decision. Nothing in it is in force, so no code can be governed by it and no "invariant" can be ' +
489+
'written for it truthfully.\n' +
490+
' If the code really is governed by a decision, that decision has a live record — anchor that one. ' +
491+
'If the decision was never written down, write it under the next free number; reviving this one would ' +
492+
'retroactively re-point every historical citation of it (#6634).'
493+
);
494+
}
495+
return null;
390496
}
391497

392498
/**
@@ -552,7 +658,7 @@ try {
552658
}
553659

554660
/** Decision records that actually exist, by number: `0090` → `0090-permission-model-...md`. */
555-
const { records, errors: numberErrors } = auditAdrDirectory(adrFiles, KNOWN_NUMBER_COLLISIONS);
661+
const { records, nonDecisions, errors: numberErrors } = auditAdrDirectory(adrFiles, KNOWN_NUMBER_COLLISIONS);
556662

557663
const errors = [...registryErrors, ...numberErrors];
558664
let checked = 0;
@@ -591,23 +697,12 @@ for (const entry of anchors) {
591697

592698
const body = readFileSync(abs, 'utf8');
593699
for (const adr of adrs) {
594-
const m = ADR_ID.exec(adr);
595-
if (!m) {
596-
errors.push(`${shard}: "${adr}" is not an ADR id (expected e.g. ADR-0090).`);
597-
continue;
598-
}
599-
// Anchoring a never-written or deleted record sends the next author to a
600-
// dead end (cf. ADR-0001, whose record was deleted in the 2026-02-11
601-
// permission-protocol rewrite and never restored).
602-
//
603-
// ⚠️ This resolves against the FILENAME only, so a tombstone — a record whose
604-
// whole content is "this number is withdrawn, do not reuse" — reads here as a
605-
// decision that exists (#6676 added the first, ADR-0107). Anchoring code to
606-
// one is not caught mechanically; what catches it is the `invariant` field,
607-
// which has to state what the ADR decided and cannot be written truthfully
608-
// for a number that decided nothing.
609-
if (!records.has(m[1])) {
610-
errors.push(`${shard}: ${adr} has no record under ${ADR_DIR}/ — anchor a decision that exists.`);
700+
// Resolution — the id parses, names a record, and that record is a DECISION
701+
// rather than a tombstone (#7329). All three live in `anchorIdProblem` so
702+
// `--self-test` drives the real judgement.
703+
const problem = anchorIdProblem(adr, records, nonDecisions);
704+
if (problem) {
705+
errors.push(`${shard}: ${problem}`);
611706
continue;
612707
}
613708
if (!body.includes(adr)) {
@@ -765,6 +860,105 @@ function selfTest() {
765860
);
766861
}
767862

863+
// ── A tombstone resolves citations but is not anchorable (#7329) ─────────
864+
//
865+
// The two halves pull in OPPOSITE directions over the SAME file, which is
866+
// exactly why one `records` set was not enough — so both are pinned, and a
867+
// test of only the red half would pass on an implementation that broke the
868+
// citation resolution the tombstone exists for.
869+
{
870+
const TOMB = [...BASE, '0003-withdrawn-something.md'];
871+
const { errors: e, records, nonDecisions } = audit(TOMB);
872+
assert('tombstone-directory-is-green', e.length === 0, `a tombstone is a legal record, got:\n${joined(e)}`);
873+
assert(
874+
'tombstone-number-still-resolves-citations',
875+
records.has('0003'),
876+
`a tombstone must stay in \`records\` — resolving citations is the job it exists for; got {${[...records].join(',')}}`,
877+
);
878+
assert(
879+
'tombstone-number-is-a-non-decision',
880+
nonDecisions.has('0003'),
881+
`got {${[...nonDecisions].join(',')}}`,
882+
);
883+
assert(
884+
'live-records-are-not-non-decisions',
885+
!nonDecisions.has('0001') && !nonDecisions.has('0002') && nonDecisions.size === 1,
886+
`only the tombstone may be flagged, got {${[...nonDecisions].join(',')}}`,
887+
);
888+
}
889+
890+
{
891+
// A revised tombstone is still a tombstone: `.vN` repeats the stem.
892+
const { nonDecisions } = audit([...BASE, '0003-withdrawn-something.md', '0003-withdrawn-something.v2.md']);
893+
assert('versioned-tombstone-is-still-a-non-decision', nonDecisions.has('0003'), 'the `.vN` rule must carry the marker');
894+
}
895+
896+
{
897+
// The marker is a SLUG PREFIX, not a substring: a live decision whose slug
898+
// merely contains the word keeps its anchors.
899+
const { nonDecisions } = audit([...BASE, '0003-policy-for-withdrawn-plugins.md']);
900+
assert(
901+
'marker-is-a-prefix-not-a-substring',
902+
!nonDecisions.has('0003'),
903+
'a live decision that merely mentions withdrawal must not be disarmed',
904+
);
905+
}
906+
907+
{
908+
// A tombstone sharing its number with a live record is a COLLISION, and the
909+
// live record must keep its anchors rather than being silently disarmed by
910+
// its squatter — the number audit is the check that speaks to this.
911+
const { errors: e, nonDecisions } = audit([...BASE, '0002-withdrawn-something.md']);
912+
assert(
913+
'tombstone-colliding-with-a-live-record-is-red',
914+
e.length === 1 && joined(e).includes('0002-withdrawn-something'),
915+
`expected the collision to be reported, got:\n${joined(e)}`,
916+
);
917+
assert(
918+
'a-contested-number-is-not-a-non-decision',
919+
!nonDecisions.has('0002'),
920+
'only a number whose EVERY claimant is a tombstone may be disarmed',
921+
);
922+
}
923+
924+
{
925+
// The anchor loop's own judgement, over the real function.
926+
const id = (n) => 'ADR-' + n;
927+
const RECS = new Set(['0090', '0107']);
928+
const TOMBS = new Set(['0107']);
929+
930+
assert(
931+
'anchor-to-a-live-record-is-allowed',
932+
anchorIdProblem(id('0090'), RECS, TOMBS) === null,
933+
`got: ${anchorIdProblem(id('0090'), RECS, TOMBS)}`,
934+
);
935+
assert(
936+
'anchor-to-a-missing-record-is-refused',
937+
/has no record under/.test(anchorIdProblem(id('0202'), RECS, TOMBS) ?? ''),
938+
`got: ${anchorIdProblem(id('0202'), RECS, TOMBS)}`,
939+
);
940+
assert(
941+
'anchor-to-a-malformed-id-is-refused',
942+
/is not an ADR id/.test(anchorIdProblem('ADR-90', RECS, TOMBS) ?? ''),
943+
`got: ${anchorIdProblem('ADR-90', RECS, TOMBS)}`,
944+
);
945+
946+
// The point of the whole change: this returned null before #7329.
947+
const tomb = anchorIdProblem(id('0107'), RECS, TOMBS) ?? '';
948+
assert('anchor-to-a-tombstone-is-refused', tomb !== '', 'a tombstone must not satisfy the anchor guard');
949+
assert('tombstone-message-names-the-id', tomb.includes('0107'), `message lacks the id:\n${tomb}`);
950+
assert(
951+
'tombstone-message-says-what-to-do-instead',
952+
/next free number/.test(tomb) && /anchor that one/.test(tomb),
953+
`the author must be told to anchor the live decision or write a new record, not just that this is wrong:\n${tomb}`,
954+
);
955+
assert(
956+
'tombstone-message-is-distinct-from-the-missing-record-one',
957+
!/has no record under/.test(tomb),
958+
`"the record is missing" and "the record decided nothing" are different facts and must read differently:\n${tomb}`,
959+
);
960+
}
961+
768962
// ── Cited numbers resolve (#6634) ────────────────────────────────────────
769963
//
770964
// ⚠️ Fixture ids are BUILT, never written literally: this file is itself in
@@ -901,6 +1095,40 @@ function selfTest() {
9011095
);
9021096
}
9031097

1098+
// ── Tombstones on the real tree (#7329) ───────────────────────────────
1099+
//
1100+
// The live half of the synthetic assertions above: ADR-0107 is the corpus's
1101+
// only tombstone today, and the shipped registry must be clean under the
1102+
// new rule — a green build here is what says the rule costs nothing to keep.
1103+
{
1104+
const { records: liveRecs, nonDecisions: liveTombs } = audit(liveFiles, KNOWN_NUMBER_COLLISIONS);
1105+
assert(
1106+
'live-tombstone-is-flagged',
1107+
liveTombs.has('0107'),
1108+
`docs/adr/0107-withdrawn-*.md is the corpus's tombstone; got {${[...liveTombs].join(',')}}`,
1109+
);
1110+
assert(
1111+
'live-tombstone-still-resolves-citations',
1112+
liveRecs.has('0107'),
1113+
'flagging the tombstone must not un-resolve the citations it was written to resolve',
1114+
);
1115+
const anchored = (anchors ?? []).flatMap((entry) => (entry?.adrs ?? []).map((adr) => `${entry.file}${adr}`));
1116+
const disarmed = anchored.filter((hit) => anchorIdProblem(hit.split(' → ')[1], liveRecs, liveTombs) !== null);
1117+
assert(
1118+
'live-registry-anchors-only-decisions',
1119+
disarmed.length === 0,
1120+
`no shipped anchor may name a tombstone, got:\n ${disarmed.join('\n ')}`,
1121+
);
1122+
1123+
// Ablation, predicted RED: an anchor to the tombstone must be refused. This
1124+
// is the exact probe run on `main` @ `69fde55`, where it came back GREEN.
1125+
assert(
1126+
'ablation-anchoring-the-live-tombstone-is-red',
1127+
anchorIdProblem('ADR-' + '0107', liveRecs, liveTombs) !== null,
1128+
'the #7329 gap is open again — a shard citing the tombstone would pass',
1129+
);
1130+
}
1131+
9041132
// ── The citation scan, over the real tree (#6634) ─────────────────────
9051133
const liveRecords = audit(liveFiles, KNOWN_NUMBER_COLLISIONS).records;
9061134
const liveCitations = collectCitations();

0 commit comments

Comments
 (0)