From 72a4a7c84ed02ec7538c82cf3a79518a60b3907a Mon Sep 17 00:00:00 2001 From: Lauri Gates Date: Sat, 4 Jul 2026 20:26:21 +0300 Subject: [PATCH] feat(blueprint-plugin): guard ADR-number collisions in blueprint-adr-validate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR numbers are chosen at branch time but only claimed at merge time, so two in-flight ADR PRs can pick the same number and both land (issue #1585 — the FVH infrastructure #2015 collision where two ADRs both claimed 0038 and the README index silently missed 0039/0040 for a week). Adds the lightweight validation-only guard from the issue's "lighter alternative" — no renaming automation, just deterministic collision + index detection: - check-adr-numbers.sh reports duplicate_adr_number (two working-tree files share a number, ERROR), adr_number_collision (a working-tree ADR's number is held by a different filename on origin/main — the pre-merge parallel-PR case, ERROR), and adr_missing_index_row (ADR absent from the README index, WARN). - Resolves docs/adrs/ or docs/adr/, emits the STATUS=/ISSUE_COUNT= convention, degrades to OK when no ADR dir / base ref exists (parallel-safe). - Wired into blueprint-adr-validate SKILL.md Step 2b + Agentic Optimizations, documented in REFERENCE.md. - Regression test (git fixture, 8 assertions) + plugin-compliance-check guard asserting the SKILL.md retains the invocation. Closes #1585 Co-Authored-By: Claude Opus 4.8 --- .claude/rules/regression-testing.md | 1 + .../blueprint-adr-validate/REFERENCE.md | 25 +++ .../skills/blueprint-adr-validate/SKILL.md | 29 ++- .../scripts/check-adr-numbers.sh | 172 ++++++++++++++++++ .../scripts/tests/test-check-adr-numbers.sh | 113 ++++++++++++ scripts/plugin-compliance-check.sh | 14 ++ 6 files changed, 351 insertions(+), 3 deletions(-) create mode 100755 blueprint-plugin/skills/blueprint-adr-validate/scripts/check-adr-numbers.sh create mode 100755 blueprint-plugin/skills/blueprint-adr-validate/scripts/tests/test-check-adr-numbers.sh diff --git a/.claude/rules/regression-testing.md b/.claude/rules/regression-testing.md index 4a53e59b4..59db7ae79 100644 --- a/.claude/rules/regression-testing.md +++ b/.claude/rules/regression-testing.md @@ -180,6 +180,7 @@ The audit script in this repo is the canonical example — it pairs a human repo | The `obsidian-plugin` read-only vault audit skills (`vault-orphans`, `vault-wikilinks`, `vault-mocs`, `vault-tags`, `vault-stubs`) leaned on the `obsidian` CLI / `vault-agent` analyzers, which require the **running Obsidian app** — with it closed the CLI errors (`The CLI is unable to find Obsidian…`) and the whole audit family is unusable for headless / batch / scheduled vault-health work. The detection *methodology* was sound; only the **data source** needed a file-parsing path (the one `vault-frontmatter` already models). | Added an `## Offline Fallback (App Closed)` section to each of the five audit skills documenting the direct `.md`-parsing path — frontmatter (`tags`/`aliases`/`context`) plus `[[wikilink]]` resolution (basename + relpath + alias, case-insensitive; `![[embed]]` against attachments) — framed as: the CLI / analyzers are the live-index path, direct parsing is the deterministic headless default. The CLI-bound `search-discovery` skill gained an `## Offline Limitation` section pointing at that path. Documentation-only; a self-contained Python parser was deliberately deferred (it would duplicate the external `vault-agent` analyzers). | `scripts/check-vault-offline-fallback.sh` (wired into `.pre-commit-config.yaml`) asserts each of the five audit skills retains the `## Offline Fallback (App Closed)` heading with the live-index/headless framing (catches a stub heading), and that `search-discovery` retains `## Offline Limitation` + the offline-path pointer. | issue #1727 | | `comfyui-node-scaffold`'s `scaffold.py` produced a CI-green pack that was invisibly missing the registry-ready / fleet-consistent **finishing pass**: `[tool.comfy] Icon = ""` (empty), no banner, no `registry-health.yml` / `clear-autorelease-labels.yml`, and a `## What it does\n\nTODO` README stub. Nothing failed — the pack published looking unfinished on registry.comfy.org and drifting from every sibling (`comfyui-touch-manager` was the lone pack missing all four for weeks with zero CI signal). Fixing each deployed pack by hand leaves every *future* pack broken the same way; the generator is the right place. | The scaffold now EMITS the deterministic pieces — `icon.svg` + `banner.svg` (family-style placeholders) with `Icon`/`Banner` wired to the `…/main/{icon,banner}.png` raw-GitHub URLs, a `just assets` rsvg-convert recipe, and the `registry-health.yml` + `clear-autorelease-labels.yml` workflows — replaces the bare README TODO with a real `## What it does`, and prints a **finishing-pass audit** at the end of every run that flags the two pieces it can't do from stdlib (PNG rasterization; the heavy pack-specific screenshot pipeline) and diffs the new pack's top-level entries against a mature sibling. | `comfyui-plugin/skills/comfyui-node-scaffold/scripts/tests/test-finishing-pass.sh` (auto-discovered via `*/skills/*/scripts/tests/test-*.sh`) EXECUTES the generator and asserts icon.svg/banner.svg emit as valid XML, `Icon`/`Banner` point at the raw-GitHub PNG URLs (never `Icon = ""`), the two workflows + `renovate.json` emit with no `dependabot.yml`, the `just assets` recipe is present, the README `What it does` is not a bare TODO, and the audit prints. `plugin-compliance-check.sh` `check_skill_body()` pins the four finishing-pass file-map entries + non-empty `Icon`/`Banner` + `print_finishing_pass_audit`. | issue #1877 | | `task-reconcile`'s `reconcile.sh` only saw tasks carrying a `ghid`/`ghpr` UDA (`select(.ghid != null or .ghpr != null)`) and checked every ref against the **CWD** repo, so it silently missed the majority of real stale tasks — those referencing an issue/PR only in the **description/annotation text** (`Merge PR #16`, `+gh-issue #292`) — and mis-resolved **cross-repo** refs (a `project:immeral` task monitoring `foundryvtt-mcp#142/143/144` was checked against immeral's repo). In a 14-task portfolio reconcile, ~10 closes came from description-text/cross-repo refs the skill could not have found. | Added a `refs` jq function that extracts refs from `ghpr`/`ghid` UDAs **and** three unambiguous text forms (`github.com/owner/repo/(issues\|pull)/N`, `owner/repo#N`, bare `#N` via negative-lookbehind so shorthand like `prompt-editor#42` is NOT matched → safe false-negative). Selection broadened to `select(refs \| length > 0)`; `ref_state repo kind num` resolves each ref (with `-R owner/repo` for cross-repo; pr-then-issue fallback for unknown kind, cached per `repo\|kind\|num`). Multi-ref tasks are stale only when EVERY ref is done (any open → live; any unreadable → keep; any `pr-closed` → aggregate `pr-closed`, kept out of the bounded `--only-verdicts` auto-apply). | `taskwarrior-plugin/scripts/tests/test-reconcile-refs.sh` (auto-discovered via `*-plugin/scripts/tests/test-*.sh`; stubs `task`+`gh`) pins: bare-`#N` description task selected + classified (gap 1); ghid/ghpr UDA back-compat; cross-repo `owner/repo#N` resolved with `-R owner/repo` (gap 2, asserted against a `gh` invocation log) + kind fallback; multi-ref with an open ref → `live` (never closed); multi-ref all-done → stale; multi-ref merged+closed → `pr-closed` kept out of the bounded apply; shorthand `prompt-editor#60` NOT selected. `test-scheduled-reconcile.sh` (28 assertions) still green (UDA path + `--only-verdicts` back-compat). | reconcile-desc-crossrepo-refs | +| ADR numbers are chosen at branch time but claimed only at merge time, so two in-flight ADR PRs can pick the same number and both land — the FVH infrastructure #2015 collision where two ADRs both claimed `0038`, a third was assigned `0040` over the apparent gap, and the README index silently missed 0039/0040 for a week. `blueprint-adr-validate` validated ADR *relationships* (supersedes/extends/cycles) but had no numbering-collision guard, so the race was invisible until implementation time. | Added `blueprint-plugin/skills/blueprint-adr-validate/scripts/check-adr-numbers.sh` (issue #1585) — the lightweight validation-only variant: it detects `duplicate_adr_number` (two working-tree files share a number, ERROR), `adr_number_collision` (a working-tree ADR's number is held by a **different** filename on `origin/main` — the pre-merge parallel-PR case, ERROR), and `adr_missing_index_row` (ADR absent from the README index, WARN). Resolves `docs/adrs/` or `docs/adr/`, emits the `STATUS=`/`ISSUE_COUNT=` convention, degrades to OK when no ADR dir / base ref exists. Wired into `blueprint-adr-validate/SKILL.md` Step 2b + Agentic Optimizations. | `blueprint-plugin/skills/blueprint-adr-validate/scripts/tests/test-check-adr-numbers.sh` (auto-discovered via `*/skills/*/scripts/tests/test-*.sh`) builds a git fixture and pins: clean tree → OK; base-ref collision → ERROR + exit 1; within-tree duplicate → ERROR; missing index row → WARN + exit 0; no ADR dir → OK; absent base ref → graceful degrade. `plugin-compliance-check.sh` `check_skill_body()` asserts the SKILL.md retains the `check-adr-numbers.sh` invocation. | issue #1585 | ## How to Add a Regression Check diff --git a/blueprint-plugin/skills/blueprint-adr-validate/REFERENCE.md b/blueprint-plugin/skills/blueprint-adr-validate/REFERENCE.md index 96bc9b6b6..ff865242f 100644 --- a/blueprint-plugin/skills/blueprint-adr-validate/REFERENCE.md +++ b/blueprint-plugin/skills/blueprint-adr-validate/REFERENCE.md @@ -24,6 +24,31 @@ - Broken reference: Target ADR doesn't exist - Inconsistent supersession: Supersedes but target not marked Superseded +### ADR-Number Collisions (issue #1585) + +`scripts/check-adr-numbers.sh` guards the parallel-PR numbering race. ADR +numbers are chosen at branch time but claimed at merge time, so two in-flight +ADR PRs can pick the same number and both land (the FVH infrastructure #2015 +collision: two ADRs both numbered `0038`). The check is deterministic and +emits the `=== ADR NUMBER AUDIT ===` / `STATUS=` / `ISSUE_COUNT=` convention. + +| Type | Severity | Meaning | +|------|----------|---------| +| `duplicate_adr_number` | ERROR | Two files in the working tree lead with the same `NNNN-`. | +| `adr_number_collision` | ERROR | A working-tree ADR's number is already held by a **different** filename on the base ref (`origin/main`) — the pre-merge parallel-PR case. | +| `adr_missing_index_row` | WARN | An ADR file is not referenced from the ADR directory's `README.md` index. | + +It resolves the ADR directory as `docs/adrs/` (blueprint canonical) or +`docs/adr/`, degrades to `STATUS=OK` when neither exists, and skips the base-ref +comparison (still checking duplicates + index) when `origin/main` is +unavailable. Flags: + +- `--project-dir ` — repo root (default: cwd). +- `--base-ref ` — collision comparison ref (default: `origin/main`). + +Remediation for an ERROR: renumber the newer ADR to the next free sequential +number, rewrite its `# ADR-NNNN:` title, and backfill the README index row. + ## Report Format ``` diff --git a/blueprint-plugin/skills/blueprint-adr-validate/SKILL.md b/blueprint-plugin/skills/blueprint-adr-validate/SKILL.md index 55f07840f..eaa3ab467 100644 --- a/blueprint-plugin/skills/blueprint-adr-validate/SKILL.md +++ b/blueprint-plugin/skills/blueprint-adr-validate/SKILL.md @@ -1,8 +1,8 @@ --- created: 2026-01-15 -modified: 2026-06-18 -reviewed: 2026-04-12 -description: Validate ADR relationships and domain consistency. Use when auditing ADRs before release, finding broken supersedes/extends links, or detecting cycles. +modified: 2026-07-04 +reviewed: 2026-07-04 +description: Validate ADR relationships, domain consistency, and duplicate ADR numbers. Use when auditing ADRs before release, finding broken supersedes/extends links, cycles, or number collisions. args: "[--report-only]" argument-hint: "--report-only to validate without prompting for fixes" allowed-tools: Read, Bash, Glob, Grep, Edit, AskUserQuestion @@ -71,6 +71,28 @@ For each ADR, validate: See [REFERENCE.md](REFERENCE.md#validation-rules) for detailed checks. +### Step 2b: Detect ADR-number collisions and index drift + +ADR numbers are chosen at branch time but only claimed at merge time, so two +in-flight ADR PRs can pick the same number and both land (issue #1585 — the FVH +infrastructure #2015 collision where two ADRs both claimed `0038`). Run the +deterministic guard: + +```bash +bash ${CLAUDE_SKILL_DIR}/scripts/check-adr-numbers.sh --project-dir "$(pwd)" +``` + +It emits the structured `STATUS=` / `ISSUE_COUNT=` convention and reports three +classes (see [REFERENCE.md](REFERENCE.md#validation-rules)): + +- `duplicate_adr_number` (ERROR) — two files in the working tree claim the same number. +- `adr_number_collision` (ERROR) — a working-tree ADR claims a number a **different** filename already holds on the base ref (`origin/main`). This is the pre-merge parallel-PR case, caught before the second PR merges. +- `adr_missing_index_row` (WARN) — an ADR is not referenced from the ADR directory's README index (how the `0038` collision went unnoticed for a week). + +Fold any findings into the Step 4 report. A `STATUS=ERROR` is a blocking +collision; resolve it by renumbering the newer ADR to the next free sequential +number and backfilling the README index row. + ### Step 3: Analyze domains 1. Group ADRs by domain field @@ -135,6 +157,7 @@ Report all changes made: | Extract frontmatter | `head -50 {file} \| grep -m1 "^field:" \| sed 's/^[^:]*:[[:space:]]*//'` | | Find by domain | `grep -l "^domain: {domain}" docs/adrs/*.md` | | Detect cycles | Build supersession graph and traverse | +| Number collisions | `bash ${CLAUDE_SKILL_DIR}/scripts/check-adr-numbers.sh --project-dir "$(pwd)"` | --- diff --git a/blueprint-plugin/skills/blueprint-adr-validate/scripts/check-adr-numbers.sh b/blueprint-plugin/skills/blueprint-adr-validate/scripts/check-adr-numbers.sh new file mode 100755 index 000000000..9c6602b60 --- /dev/null +++ b/blueprint-plugin/skills/blueprint-adr-validate/scripts/check-adr-numbers.sh @@ -0,0 +1,172 @@ +#!/usr/bin/env bash +# check-adr-numbers.sh — detect ADR-number collisions and index drift. +# +# ADR numbers are chosen at branch time but only claimed at merge time, so two +# in-flight ADR PRs can pick the same number and both land (the FVH +# infrastructure #2015 collision: two 0038 ADRs). This is the lightweight +# validation-only guard from issue #1585 — no renaming automation, just +# collision + index-drift detection. +# +# It reports three classes: +# 1. duplicate_adr_number — two files in the working tree claim the same +# number (post-merge collision, or a hand-numbering slip). ERROR +# 2. adr_number_collision — a working-tree ADR claims a number a DIFFERENT +# filename already holds on the base ref (origin/main). This is the +# pre-merge parallel-PR case — caught before the second PR merges. ERROR +# 3. adr_missing_index_row — an ADR file is not referenced from the ADR +# directory's README index (how the 0038 collision went unnoticed). WARN +# +# Emits the structured KEY=VALUE / STATUS= / ISSUE_COUNT= convention +# (.claude/rules/structured-script-output.md). Exit 0 on OK/WARN, 1 on ERROR +# (parallel-safe per .claude/rules/parallel-safe-queries.md). +set -uo pipefail + +project_dir="$(pwd)" +base_ref="origin/main" + +while [ $# -gt 0 ]; do + case "$1" in + --project-dir) project_dir="$2"; shift 2 ;; + --base-ref) base_ref="$2"; shift 2 ;; + --project-dir=*) project_dir="${1#*=}"; shift ;; + --base-ref=*) base_ref="${1#*=}"; shift ;; + *) shift ;; + esac +done + +emit() { printf '%s\n' "$1"; } + +echo "=== ADR NUMBER AUDIT ===" + +# Locate the ADR directory — blueprint canonical is docs/adrs/, but docs/adr/ +# is also common (the FVH infrastructure repo where #1585 originated uses it). +adr_dir="" +for cand in "$project_dir/docs/adrs" "$project_dir/docs/adr"; do + if [ -d "$cand" ]; then adr_dir="$cand"; break; fi +done + +if [ -z "$adr_dir" ]; then + emit "ADR_DIR=none" + emit "STATUS=OK" + emit "ISSUE_COUNT=0" + emit "=== END ADR NUMBER AUDIT ===" + exit 0 +fi + +adr_rel="${adr_dir#"$project_dir"/}" +emit "ADR_DIR=$adr_rel" + +# Normalize a "NNNN-title.md" basename to a bare integer, or empty if the file +# does not lead with a number (README.md, validation-report.md, templates). +adr_number() { + local base="$1" num + num="$(printf '%s' "$base" | sed -nE 's/^0*([0-9]+)[-_].*/\1/p')" + [ -n "$num" ] && printf '%d' "$((10#$num))" +} + +# --- Collect working-tree ADRs: "numberbasename" ------------------------- +wt_pairs="" +adr_count=0 +for f in "$adr_dir"/*.md; do + [ -e "$f" ] || continue + base="$(basename "$f")" + num="$(adr_number "$base")" + [ -n "$num" ] || continue + adr_count=$((adr_count + 1)) + wt_pairs="${wt_pairs}${num} ${base}"$'\n' +done +emit "ADR_COUNT=$adr_count" + +issues="" +issue_count=0 +has_error=0 +add_issue() { # severity type msg + issues="${issues} - SEVERITY=$1 TYPE=$2 MSG=$3"$'\n' + issue_count=$((issue_count + 1)) + [ "$1" = "ERROR" ] && has_error=1 + return 0 +} + +# --- Check 1: duplicate numbers within the working tree ----------------------- +dup_numbers="$(printf '%s' "$wt_pairs" | awk -F'\t' 'NF==2{c[$1]++} END{for(n in c) if(c[n]>1) print n}' | sort -n)" +for n in $dup_numbers; do + files="$(printf '%s' "$wt_pairs" | awk -F'\t' -v n="$n" '$1==n{printf "%s ", $2}')" + add_issue ERROR duplicate_adr_number "ADR number $n claimed by multiple files: ${files% }" +done + +# --- Check 2: collision with a different filename on the base ref -------------- +base_available=false +base_pairs="" +if git -C "$project_dir" rev-parse --verify --quiet "$base_ref" >/dev/null 2>&1; then + base_available=true + # git ls-tree paths are repo-relative; adr_rel is project-dir-relative. When + # the project dir is the repo root these coincide (the common case). + while IFS= read -r path; do + [ -n "$path" ] || continue + b="$(basename "$path")" + num="$(adr_number "$b")" + [ -n "$num" ] || continue + base_pairs="${base_pairs}${num} ${b}"$'\n' + done < <(git -C "$project_dir" ls-tree -r --name-only "$base_ref" -- "$adr_rel" 2>/dev/null) +fi +emit "BASE_REF=$base_ref" +emit "BASE_REF_AVAILABLE=$base_available" + +if [ "$base_available" = true ]; then + # For each working ADR, flag if the base ref holds the same number under a + # basename that is NOT present in the working tree for that number. + while IFS=$'\t' read -r wnum wbase; do + [ -n "${wnum:-}" ] || continue + while IFS=$'\t' read -r bnum bbase; do + [ -n "${bnum:-}" ] || continue + [ "$bnum" = "$wnum" ] || continue + [ "$bbase" = "$wbase" ] && continue + # base holds this number under a different filename. Confirm that other + # filename is not itself in the working tree (i.e. it is a genuine + # cross-boundary collision, not an unrelated rename we already track). + if ! printf '%s' "$wt_pairs" | awk -F'\t' -v n="$wnum" -v b="$bbase" '$1==n && $2==b{f=1} END{exit !f}'; then + add_issue ERROR adr_number_collision "ADR number $wnum in '$wbase' also claimed on $base_ref by '$bbase'" + fi + done < <(printf '%s' "$base_pairs") + done < <(printf '%s' "$wt_pairs") +fi + +# --- Check 3: ADRs missing from the README index ------------------------------ +readme="" +for cand in "$adr_dir/README.md" "$adr_dir/readme.md"; do + [ -f "$cand" ] && { readme="$cand"; break; } +done +if [ -n "$readme" ]; then + emit "INDEX=$( [ -n "$readme" ] && echo present || echo absent )" + while IFS=$'\t' read -r num base; do + [ -n "${num:-}" ] || continue + # Indexed if the README mentions the filename OR an ADR-NNNN / ADR NNNN token + # matching this number (zero-padding-insensitive). + padded="$(printf '%04d' "$num")" + if grep -qF "$base" "$readme" 2>/dev/null; then continue; fi + if grep -qiE "ADR[- ]0*${num}([^0-9]|\$)" "$readme" 2>/dev/null; then continue; fi + if grep -qF "$padded" "$readme" 2>/dev/null; then continue; fi + add_issue WARN adr_missing_index_row "ADR '$base' (number $num) is not referenced in $(basename "$readme")" + done < <(printf '%s' "$wt_pairs") +else + emit "INDEX=absent" +fi + +# --- Roll-up ------------------------------------------------------------------ +if [ "$has_error" -eq 1 ]; then + status="ERROR" +elif [ "$issue_count" -gt 0 ]; then + status="WARN" +else + status="OK" +fi +emit "STATUS=$status" +emit "ISSUE_COUNT=$issue_count" +if [ "$issue_count" -gt 0 ]; then + emit "ISSUES:" + printf '%s' "$issues" +fi +emit "=== END ADR NUMBER AUDIT ===" + +[ "$has_error" -eq 1 ] && exit 1 +exit 0 diff --git a/blueprint-plugin/skills/blueprint-adr-validate/scripts/tests/test-check-adr-numbers.sh b/blueprint-plugin/skills/blueprint-adr-validate/scripts/tests/test-check-adr-numbers.sh new file mode 100755 index 000000000..40fcb773b --- /dev/null +++ b/blueprint-plugin/skills/blueprint-adr-validate/scripts/tests/test-check-adr-numbers.sh @@ -0,0 +1,113 @@ +#!/usr/bin/env bash +# Regression test for check-adr-numbers.sh (issue #1585). +# Auto-discovered by scripts/run-skill-script-tests.sh via +# */skills/*/scripts/tests/test-*.sh. +set -uo pipefail + +# Neutralize any inherited git context so `git -C "$sandbox"` cannot be hijacked +# into the real shared .git (issue #1745). +unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_OBJECT_DIRECTORY GIT_COMMON_DIR GIT_NAMESPACE GIT_PREFIX + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +CHECK="$SCRIPT_DIR/../check-adr-numbers.sh" + +pass=0 +fail=0 +ok() { printf 'ok - %s\n' "$1"; pass=$((pass + 1)); } +bad() { printf 'FAIL - %s\n' "$1"; fail=$((fail + 1)); } + +# --- fixture: a git repo with a base commit (origin/main-like) ---------------- +SANDBOX="$(mktemp -d)" +[ -n "$SANDBOX" ] || { echo "mktemp failed"; exit 1; } +trap 'rm -rf "$SANDBOX"' EXIT + +git -C "$SANDBOX" init -q +git -C "$SANDBOX" config user.email t@t.t +git -C "$SANDBOX" config user.name t +mkdir -p "$SANDBOX/docs/adrs" + +# Base ref holds ADR 0038 under one filename, indexed in the README. +cat > "$SANDBOX/docs/adrs/0038-declarative-bootstrapping.md" <<'MD' +# ADR-0038: Declarative bootstrapping +status: Accepted +MD +cat > "$SANDBOX/docs/adrs/README.md" <<'MD' +# ADRs +- [ADR-0038](0038-declarative-bootstrapping.md) +MD +git -C "$SANDBOX" add -A +git -C "$SANDBOX" commit -q -m "base" +git -C "$SANDBOX" branch base-main + +run() { bash "$CHECK" --project-dir "$SANDBOX" --base-ref "$1"; } + +# --- TEST A: clean tree matching base → STATUS=OK ----------------------------- +out="$(run base-main)" +if grep -q '^STATUS=OK$' <<<"$out"; then ok "A: clean tree → OK"; else bad "A: clean tree should be OK"; echo "$out"; fi + +# --- TEST B: parallel-PR collision — a PR branched before the base's 0038 +# merged, so its working tree has 0038-sensor but NOT the 0038-declarative that +# now sits on base-main under a different filename. +rm -f "$SANDBOX/docs/adrs/0038-declarative-bootstrapping.md" +cat > "$SANDBOX/docs/adrs/0038-sensor-pipeline.md" <<'MD' +# ADR-0038: Sensor pipeline +status: Accepted +MD +# index the new one so we isolate the collision (not the missing-index) signal +cat > "$SANDBOX/docs/adrs/README.md" <<'MD' +# ADRs +- [ADR-0038 sensor](0038-sensor-pipeline.md) +MD +out="$(run base-main)"; rc=$? +if grep -q 'TYPE=adr_number_collision' <<<"$out" && grep -q '^STATUS=ERROR$' <<<"$out"; then + ok "B: base-ref collision detected (ERROR)" +else bad "B: expected adr_number_collision ERROR"; echo "$out"; fi +if [ "$rc" -eq 1 ]; then ok "B: exit 1 on collision"; else bad "B: expected exit 1, got $rc"; fi +rm -f "$SANDBOX/docs/adrs/0038-sensor-pipeline.md" +git -C "$SANDBOX" checkout -q -- docs/adrs/ + +# --- TEST C: two files in the working tree claim the same number -------------- +cat > "$SANDBOX/docs/adrs/0041-alpha.md" <<'MD' +# ADR-0041: Alpha +MD +cat > "$SANDBOX/docs/adrs/0041-beta.md" <<'MD' +# ADR-0041: Beta +MD +printf -- '- [ADR-0041 a](0041-alpha.md)\n- [ADR-0041 b](0041-beta.md)\n' >> "$SANDBOX/docs/adrs/README.md" +out="$(run base-main)" +if grep -q 'TYPE=duplicate_adr_number' <<<"$out" && grep -q '^STATUS=ERROR$' <<<"$out"; then + ok "C: within-tree duplicate detected (ERROR)" +else bad "C: expected duplicate_adr_number ERROR"; echo "$out"; fi +rm -f "$SANDBOX/docs/adrs/0041-alpha.md" "$SANDBOX/docs/adrs/0041-beta.md" +git -C "$SANDBOX" checkout -q -- docs/adrs/README.md + +# --- TEST D: an ADR absent from the README index → WARN ----------------------- +cat > "$SANDBOX/docs/adrs/0042-unindexed.md" <<'MD' +# ADR-0042: Unindexed +MD +out="$(run base-main)"; rc=$? +if grep -q 'TYPE=adr_missing_index_row' <<<"$out" && grep -q '^STATUS=WARN$' <<<"$out"; then + ok "D: missing index row detected (WARN)" +else bad "D: expected adr_missing_index_row WARN"; echo "$out"; fi +if [ "$rc" -eq 0 ]; then ok "D: WARN exits 0 (parallel-safe)"; else bad "D: WARN should exit 0, got $rc"; fi +rm -f "$SANDBOX/docs/adrs/0042-unindexed.md" + +# --- TEST E: no docs/adrs dir → OK, no false positive ------------------------- +E_SANDBOX="$(mktemp -d)" +[ -n "$E_SANDBOX" ] || { echo "mktemp failed"; exit 1; } +git -C "$E_SANDBOX" init -q +out="$(bash "$CHECK" --project-dir "$E_SANDBOX" --base-ref HEAD 2>/dev/null)" +if grep -q 'ADR_DIR=none' <<<"$out" && grep -q '^STATUS=OK$' <<<"$out"; then + ok "E: no ADR dir → OK" +else bad "E: missing ADR dir should be OK"; echo "$out"; fi +rm -rf "$E_SANDBOX" + +# --- TEST F: base ref unavailable → still runs (dupes/index only) -------------- +out="$(bash "$CHECK" --project-dir "$SANDBOX" --base-ref does-not-exist 2>/dev/null)" +if grep -q '^BASE_REF_AVAILABLE=false$' <<<"$out" && grep -q '^STATUS=OK$' <<<"$out"; then + ok "F: absent base ref degrades gracefully" +else bad "F: absent base ref should degrade to OK"; echo "$out"; fi + +echo "---" +echo "PASS=$pass FAIL=$fail" +[ "$fail" -eq 0 ] diff --git a/scripts/plugin-compliance-check.sh b/scripts/plugin-compliance-check.sh index c4b0a23a5..5984d7861 100755 --- a/scripts/plugin-compliance-check.sh +++ b/scripts/plugin-compliance-check.sh @@ -387,6 +387,20 @@ check_skill_body() { fi fi + # Regression: blueprint-adr-validate must retain the ADR-number collision + # guard (issue #1585). ADR numbers are claimed at merge time, so two + # parallel ADR PRs can pick the same number and both land (the FVH #2015 + # 0038 collision). The deterministic check-adr-numbers.sh detects + # within-tree duplicates, base-ref (origin/main) collisions, and index + # drift. A bulk edit dropping the script invocation silently removes the + # guard, so the semantic invariant is that the skill body still wires it in. + if [ "$skill_name" = "blueprint-adr-validate" ]; then + if ! grep -q "check-adr-numbers.sh" "$skill_file"; then + issues+=("❌ ${plugin}/${skill_name}: SKILL.md must wire in the ADR-number collision guard (check-adr-numbers.sh) (issue #1585)") + has_errors=true + fi + fi + # Regression: task-reconcile is the only skill that ACTS on the stale-task # drift task-status detects. Its load-bearing invariants are (1) a bulk # `task import` round-trip for leaf tasks, (2) a default-safe dry-run with an