Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/rules/regression-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 25 additions & 0 deletions blueprint-plugin/skills/blueprint-adr-validate/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <path>` — repo root (default: cwd).
- `--base-ref <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

```
Expand Down
29 changes: 26 additions & 3 deletions blueprint-plugin/skills/blueprint-adr-validate/SKILL.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)"` |

---

Expand Down
Original file line number Diff line number Diff line change
@@ -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: "number<TAB>basename" -------------------------
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
Loading
Loading