Skip to content

fix(blueprint-plugin): ADR collision guard is basename- and single-directory-scoped — missed a real ADR-0016 double-assignment #2129

Description

@laurigates

What

Two gaps in the ADR collision guard that shipped for #1585 (blueprint-adr-validate/scripts/check-adr-numbers.sh):

  1. Detection is basename-scoped, so an ADR that claims its number in frontmatter rather than in its filename is invisible to the guard.
  2. adr_missing_index_row can only warn, never repair — and a stale index is what blueprint-plugin: placeholder ADR numbers with merge-time assignment (parallel-PR collision guard) #1585 itself named as how the original 0038 collision went unnoticed for a week.

Why

Dogfooding on skullcaps-native hit a real ADR-0016 double-assignment that the guard reported STATUS=OK for:

  • docs/blueprint/adrs/0016-in-engine-ui-not-win32-dialogs.md — frontmatter id: ADR-0016, registered in the manifest
  • docs/adrs/objs-format.md — frontmatter id: ADR-0016, unregistered, and cited as ADR-0016 by two other documents

Both are genuine ADRs. The guard missed it for two compounding reasons:

  • normalize() derives the number from the NNNN-title.md basename (script line ~59), so objs-format.md contributes no number at all.
  • ADR_DIR resolves to a single directory, so the two claimants were never compared.

The cause is documented inside one of the colliding files0016-in-engine-ui-not-win32-dialogs.md:17 records that its author computed "next free slot" from the manifest, not knowing objs-format.md had already self-assigned 0016 in its own frontmatter. That's precisely #1585's branch-time-vs-claim-time race, in a form the shipped guard can't see.

Same repo, same run: INDEX=absent for docs/adrs/, while the other ADR directory's README index listed 8 of 25 ADRs, carried a stale status for one, and offered an awk one-liner as the regeneration recipe whose END block prints a single row regardless of input.

How

1. Detect frontmatter id claims. Union the basename-derived number with a frontmatter id: ADR-NNNN parse. A file claiming a number in frontmatter that a different file claims by filename is the same duplicate_adr_number class already implemented — only the collection step needs widening.

2. Support multiple ADR directories. Accept repeated --adr-dir, or read the set from the manifest. Cross-directory is where this bug lived; a single-directory assumption makes it structurally undetectable. (Relatedly: this repo's ADR ids span docs/adrs/ and docs/blueprint/adrs/, which #1357 already touched from the writer side.)

3. Prefer the manifest as the arbiter. id_registry is the only place that resolves id→path unambiguously; two files can both say ADR-0016 but only one can be registered. Reporting "file X claims ADR-NNNN but the registry maps NNNN to Y" catches this in one comparison and needs no filename convention at all.

4. Generate the index rather than warning about it. A small generator that rebuilds the index table from frontmatter between <!-- adr-index:start --> / <!-- adr-index:end --> markers, with a --check mode for CI. That turns adr_missing_index_row from a warning into a fixable state and removes the hand-maintained table that rotted here.

Status

Working implementation of (4) exists locally (~130 lines, --check mode, wired to pre-commit + CI; regenerates 25 ADRs across two directories). (1)-(3) are small changes to the existing script. Happy to PR either or both with tests matching scripts/tests/.

Not urgent — the collision is fixed downstream by renumbering to ADR-0035. Filing because the guard reported OK on a real collision, which is the kind of false-negative worth closing while the evidence is concrete.

Metadata

Metadata

Assignees

No one assigned

    Labels

    blueprint-pluginBlueprint plugin relatedbugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions