From ce3319cbed8ccf7859367a8eb7664254b41ccb6a Mon Sep 17 00:00:00 2001 From: sjat Date: Wed, 10 Jun 2026 13:55:16 +0200 Subject: [PATCH] docs(adr): implementation plan + FRICTION signal for ADR structure Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/FRICTION.md | 9 + .../plans/2026-06-10-adr-structure.md | 536 ++++++++++++++++++ 2 files changed, 545 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-10-adr-structure.md diff --git a/docs/FRICTION.md b/docs/FRICTION.md index b632dd3..8b93fb4 100644 --- a/docs/FRICTION.md +++ b/docs/FRICTION.md @@ -25,6 +25,15 @@ _(append new raw signals here; the next kaizen review consumes them)_ invented a Status header ("Proposed") on the fly because there's no documented convention for how we write ADRs (status lifecycle, required sections). → TODO 10.2 — decide a minimal ADR template / status convention. +- `[recurring]` **Brainstorming's "user reviews spec" gate fires despite a standing + agreement to skip it** (2026-06-10): writing the ADR-structure spec, I stopped to ask + the user to review the finished spec before writing the plan — the + `superpowers:brainstorming` skill scripts that gate. We had previously agreed I should + move directly from the Q/A to the implementation plan once the spec is written. Same + shape as the execution-mode-menu signal: an external skill's script conflicting with a + boma convention, where prose reminders don't hold. → consider a mechanical guard + (Stop-hook family) or a CLAUDE.md/skill-override note that suppresses the spec-review + gate. --- diff --git a/docs/superpowers/plans/2026-06-10-adr-structure.md b/docs/superpowers/plans/2026-06-10-adr-structure.md new file mode 100644 index 0000000..b63c22f --- /dev/null +++ b/docs/superpowers/plans/2026-06-10-adr-structure.md @@ -0,0 +1,536 @@ +# ADR Structure & Lifecycle Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Codify how boma's ADRs are structured — a canonical section set, an Accepted/Superseded/Deprecated lifecycle, a template, a lightweight enforcement check, and a one-time Status backfill of the back-catalogue. + +**Architecture:** Five independent units. (1) A pure-function `adr-structure` check added to the existing `scripts/repo-scan.py` (stdlib only, pytest-tested like its siblings), verifying every numbered ADR has the four mandatory sections and a parseable Status line — presence only, not order. (2) An `adr-template.md` scaffold. (3) ADR-023 itself, written to pass its own check. (4) Wiring into CLAUDE.md and the `/review-repo` command doc. (5) A mechanical backfill adding `## Status` to ADRs 001–018, dated from each file's first git-commit. + +**Tech Stack:** Python 3 stdlib (`scripts/repo-scan.py`), pytest (`.venv/bin/pytest`), Markdown, git. + +**Spec:** `docs/superpowers/specs/2026-06-10-adr-structure-design.md` + +**Branch:** `feat/adr-structure` (already created; the design spec is the first commit). + +**Convention reminders (from CLAUDE.md):** docs-/script-only commits skip the ansible-lint pre-commit hook and need no `rbw` unlock. Imperative subject ≤72 chars. `Co-Authored-By: Claude Opus 4.8 (1M context) ` trailer on every commit. + +--- + +## Decisions locked by the spec (do not re-litigate) + +- **Mandatory sections, in this order:** `## Status`, `## Context`, `## Decision`, `## Consequences`. +- **Optional sections:** `## Related`, `## Scope`, `## Guardrails` / `## Enforcement`, `## What was ruled out`, `## Verified facts (ADR-014)`. +- **Status lifecycle (3 states):** `Accepted (YYYY-MM-DD)` → optionally `Superseded by ADR-NNN (YYYY-MM-DD)` or `Deprecated (YYYY-MM-DD)`. No "Proposed" stage. +- **No silent rewrites:** material reversal = new ADR + `Superseded by` marker; bidirectional link. +- **Enforcement checks presence + parseable Status line, NOT section order.** Order is demonstrated by the template, not machine-enforced. +- **Backfill is Status-header-only** — no decision content touched. + +--- + +## Task 1: `adr-structure` check in repo-scan.py + +**Files:** +- Modify: `scripts/repo-scan.py` (add module-level regexes near the other `_RE` definitions ~line 38–44; add `adr_structure_findings()` next to `deferred_findings()` ~line 96; wire it into `scan()` at the `findings.extend(...)` site ~line 215) +- Test: `tests/test_repo_scan.py` (new) + +- [ ] **Step 1: Write the failing test** + +Create `tests/test_repo_scan.py`: + +```python +import importlib.util +import pathlib + +_PATH = pathlib.Path(__file__).resolve().parent.parent / "scripts" / "repo-scan.py" +_spec = importlib.util.spec_from_file_location("repo_scan", _PATH) +rs = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(rs) + +GOOD = [ + "# ADR-099 — Example\n", "\n", + "## Status\n", "\n", "Accepted (2026-06-10)\n", "\n", + "## Context\n", "\n", "Why.\n", "\n", + "## Decision\n", "\n", "What.\n", "\n", + "## Consequences\n", "\n", "So what.\n", +] + + +def _checks(findings): + return [f for f in findings if f["check"] == "adr-structure"] + + +def test_good_adr_has_no_findings(): + out = rs.adr_structure_findings({"docs/decisions/099-example.md": GOOD}) + assert _checks(out) == [] + + +def test_missing_mandatory_section_is_flagged(): + lines = [ln for ln in GOOD if not ln.startswith("## Consequences")] + out = _checks(rs.adr_structure_findings({"docs/decisions/099-example.md": lines})) + assert len(out) == 1 + assert "Consequences" in out[0]["detail"] + + +def test_unparseable_status_is_flagged(): + lines = [("Designed, not built.\n" if ln == "Accepted (2026-06-10)\n" else ln) + for ln in GOOD] + out = _checks(rs.adr_structure_findings({"docs/decisions/099-example.md": lines})) + assert len(out) == 1 + assert "Status not parseable" in out[0]["detail"] + + +def test_superseded_status_is_accepted(): + lines = [("Superseded by ADR-100 (2026-06-11)\n" if ln == "Accepted (2026-06-10)\n" + else ln) for ln in GOOD] + out = _checks(rs.adr_structure_findings({"docs/decisions/099-example.md": lines})) + assert out == [] + + +def test_non_numbered_file_is_skipped(): + bare = ["# ADR template\n", "\n", "## Status\n", "\n", "\n"] + out = _checks(rs.adr_structure_findings({"docs/decisions/adr-template.md": bare})) + assert out == [] +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `.venv/bin/pytest tests/test_repo_scan.py -q` +Expected: FAIL — `AttributeError: module 'repo_scan' has no attribute 'adr_structure_findings'`. + +- [ ] **Step 3: Add the regexes** + +In `scripts/repo-scan.py`, after the `RESOLVE_WORD_RE = ...` line (~line 44), add: + +```python +# ADR-structure check (ADR-023): numbered ADRs must carry the four mandatory +# sections and a parseable Status line. Presence only — section ORDER is a +# template-demonstrated convention, not machine-enforced. +ADR_FILE_RE = re.compile(r"^\d{3}-.*\.md$") +ADR_REQUIRED_SECTIONS = ("Status", "Context", "Decision", "Consequences") +ADR_STATUS_LINE_RE = re.compile( + r"^(Accepted \(\d{4}-\d{2}-\d{2}\)" + r"|Superseded by ADR-\d{3}" + r"|Deprecated \(\d{4}-\d{2}-\d{2}\))") +``` + +- [ ] **Step 4: Add the check function** + +In `scripts/repo-scan.py`, immediately after the `deferred_findings(...)` function (it ends ~line 96, just before `def walk_files():`), add: + +```python +def adr_structure_findings(adr_files): + """adr_files: {rel_path: [lines]} for docs/decisions/*.md. + Flags numbered ADRs (NNN-*.md) missing a mandatory section or whose Status + section has no parseable lifecycle line. Non-numbered files (e.g. + adr-template.md) are skipped. Section order is NOT checked (ADR-023).""" + out = [] + for rpath, lines in sorted(adr_files.items()): + if not ADR_FILE_RE.match(os.path.basename(rpath)): + continue + headings = {} + for i, line in enumerate(lines): + m = re.match(r"^##\s+(\w+)", line) + if m: + headings.setdefault(m.group(1), i) + missing = [s for s in ADR_REQUIRED_SECTIONS if s not in headings] + if missing: + out.append({"check": "adr-structure", "severity": "medium", + "path": rpath, "line": 1, + "detail": f"missing mandatory section(s): {', '.join(missing)}"}) + if "Status" in headings: + body = [] + for line in lines[headings["Status"] + 1:]: + if line.startswith("## "): + break + body.append(line) + status_text = next((ln.strip() for ln in body if ln.strip()), "") + if not ADR_STATUS_LINE_RE.match(status_text): + out.append({"check": "adr-structure", "severity": "medium", + "path": rpath, "line": headings["Status"] + 1, + "detail": "Status not parseable (want 'Accepted (YYYY-MM-DD)', " + "'Superseded by ADR-NNN', or 'Deprecated (YYYY-MM-DD)'); " + f"got: {status_text[:60]!r}"}) + return out +``` + +- [ ] **Step 5: Run the test to verify it passes** + +Run: `.venv/bin/pytest tests/test_repo_scan.py -q` +Expected: PASS — 5 passed. + +- [ ] **Step 6: Wire the check into `scan()`** + +In `scripts/repo-scan.py`, find (~line 215): + +```python + findings.extend(deferred_findings(adr_files, defer_refs)) + return findings +``` + +Replace with: + +```python + findings.extend(deferred_findings(adr_files, defer_refs)) + findings.extend(adr_structure_findings(adr_files)) + return findings +``` + +- [ ] **Step 7: Confirm the check fires on the real (not-yet-backfilled) repo** + +Run: `python3 scripts/repo-scan.py 2>/dev/null | python3 -c "import json,sys; print(sorted({f['path'] for f in json.load(sys.stdin)['findings'] if f['check']=='adr-structure'}))"` +Expected: a list including `docs/decisions/001-architecture.md` … through `018-logging.md` (001–015 missing Status; 016–018 unparseable Status). 019–022 and 023 must NOT appear. This proves the check works and previews Task 5's worklist. + +- [ ] **Step 8: Commit** + +```bash +git add scripts/repo-scan.py tests/test_repo_scan.py +git commit -m "feat(review): add adr-structure check to repo-scan + +Flags numbered ADRs missing a mandatory section (Status/Context/Decision/ +Consequences) or with an unparseable Status line. Presence only, not order. + +Co-Authored-By: Claude Opus 4.8 (1M context) " +``` + +--- + +## Task 2: ADR template + +**Files:** +- Create: `docs/decisions/adr-template.md` + +- [ ] **Step 1: Write the template** + +Create `docs/decisions/adr-template.md` with exactly: + +```markdown +# ADR-NNN — : <optional clarifying subtitle> + +<!-- Filename: NNN-kebab-title.md (zero-padded, monotonic, never reused). + Register a row in CLAUDE.md "Further reading" when this ADR is created. + Sections below in order. Mandatory: Status, Context, Decision, Consequences. + Delete this comment and any optional section you don't use. --> + +## Status + +Accepted (YYYY-MM-DD) +<!-- Lifecycle: "Accepted (YYYY-MM-DD)" → later "Superseded by ADR-NNN (YYYY-MM-DD)" + or "Deprecated (YYYY-MM-DD)" + one-line why. Optional trailing note OK, e.g. + "Accepted (2026-06-10). Doctrine ADR — pins policy, builds nothing yet." --> + +## Context + +<!-- The forces, the problem, what exists today, why now. --> + +## Decision + +<!-- What we are doing. Use numbered sub-decisions (### 1. ...) for multi-part ADRs. --> + +## Consequences + +<!-- Results, trade-offs explicitly accepted, follow-on work. --> + +<!-- OPTIONAL SECTIONS — uncomment any that genuinely apply; never pad. +## Scope +<!-- Explicit in / out-of-scope boundaries. --> + +## Guardrails +<!-- How the decision is mechanically enforced (lint, CI, hooks). --> + +## What was ruled out +<!-- Rejected alternatives, each with its reason. --> + +## Verified facts (ADR-014) +<!-- verified: <subject> · <tool> <version> · <source> · <YYYY-MM-DD> --> + +## Related +<!-- Links to other ADRs by number; bidirectional for Supersedes/Superseded-by. --> +--> +``` + +- [ ] **Step 2: Confirm the template is skipped by the check** + +Run: `python3 scripts/repo-scan.py 2>/dev/null | python3 -c "import json,sys; print([f for f in json.load(sys.stdin)['findings'] if f['check']=='adr-structure' and 'adr-template' in f['path']])"` +Expected: `[]` (non-numbered filename → skipped). + +- [ ] **Step 3: Commit** + +```bash +git add docs/decisions/adr-template.md +git commit -m "docs(adr): add adr-template.md scaffold (ADR-023) + +Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>" +``` + +--- + +## Task 3: ADR-023 itself + +**Files:** +- Create: `docs/decisions/023-adr-structure.md` + +- [ ] **Step 1: Write ADR-023** + +Create `docs/decisions/023-adr-structure.md`. It must pass its own check (Status/Context/Decision/Consequences present; parseable Status line). Use this content: + +```markdown +# ADR-023 — ADR structure & lifecycle + +## Status + +Accepted (2026-06-10). Meta/doctrine ADR — pins how ADRs are written; the +`adr-structure` check (`scripts/repo-scan.py`) and `docs/decisions/adr-template.md` +ship with it. Resolves the FRICTION signal (2026-05-31) about ADR-writing policy +being unsettled. + +## Context + +boma records architectural decisions as numbered ADRs in `docs/decisions/`, and +CLAUDE.md treats them as load-bearing. Yet no ADR said how an ADR is written. The +newest ADRs (019–022) converged on a clean shape — Status → Context → Decision → +Consequences → Related — but only by imitation, and ADRs 001–018 predate it: 001–015 +carry no `## Status` section at all, and 016–018 have a trailing build-status note +rather than a lifecycle line. The result is structural drift and no uniform way to +tell an active decision from a superseded or deprecated one. + +## Decision + +### 1. Title & filename + +Title line: `# ADR-NNN — <Title>: <optional clarifying subtitle>` (em-dash). Filename: +`NNN-kebab-title.md`, zero-padded 3-digit, monotonic, never reused — a superseded ADR +keeps its number and file. A new ADR is registered as a row in the CLAUDE.md +"Further reading" table. + +### 2. Mandatory sections, in this order + +- `## Status` — `Accepted (YYYY-MM-DD)`, plus an optional one-line note. +- `## Context` — the forces, the problem, what exists today, why now. +- `## Decision` — what we are doing; numbered sub-decisions for multi-part ADRs. +- `## Consequences` — results, trade-offs explicitly accepted, follow-on work. + +### 3. Optional sections (use only where they genuinely apply) + +`## Related`, `## Scope`, `## Guardrails` / `## Enforcement`, `## What was ruled out`, +`## Verified facts (ADR-014)`. + +### 4. Status lifecycle + +Three states; no "Proposed" stage (boma is single-contributor and trunk-based with no +review gate, so an ADR is born committed-to). + +- Born **`Accepted (YYYY-MM-DD)`**. +- Replaced → old ADR's Status becomes **`Superseded by ADR-NNN (YYYY-MM-DD)`**; the new + ADR records `Supersedes ADR-MMM` in its Status and `## Related`. The link is + **bidirectional**. +- Retired with no replacement → **`Deprecated (YYYY-MM-DD)`** + a one-line reason. + +**No silent rewrites.** An Accepted ADR is not edited to reverse its decision. Typo and +clarity fixes are fine; a material reversal requires a new ADR and a `Superseded by` +marker on the old one. + +### 5. Template & enforcement + +`docs/decisions/adr-template.md` is the scaffold for new ADRs. The `/review-repo` +command's pre-scan (`scripts/repo-scan.py`) emits an `adr-structure` finding for any +numbered ADR missing a mandatory section or with an unparseable Status line. It checks +**presence and Status, not section order** — order is a convention the template carries, +deliberately not gated, to keep enforcement lightweight (consistent with boma's other +doctrine ADRs adding no CI gate). + +## Consequences + +- New ADRs have one obvious shape and a scaffold; structural drift stops. +- Every ADR declares its lifecycle state uniformly, and reversals are traceable. +- One-time backfill churn: a Status header added to ADRs 001–018 (header only; no + decision content changed). +- `/review-repo` grows one deterministic check; no new CI machinery. +- This ADR is the first conformant example and is held to its own check. + +## What was ruled out + +- **A "Proposed" draft stage** — no review gate exists for it to serve. +- **A `make lint` / CI gate for ADR structure** — heavier than the risk warrants; + the `/review-repo` check and the template suffice. +- **Machine-enforcing section order** — brittle for marginal value; left as a + template-demonstrated convention. +- **Normalizing the body of 001–018** beyond adding `## Status` — out of scope; the + decisions themselves are untouched. + +## Related + +- ADR-014 — knowledge sourcing (the `Verified facts` optional section). +- ADR-019/020/021/022 — the emergent structure this ADR codifies. +- `docs/decisions/adr-template.md` — the scaffold. +- `scripts/repo-scan.py` — the `adr-structure` enforcement check. +``` + +- [ ] **Step 2: Confirm ADR-023 passes its own check** + +Run: `python3 scripts/repo-scan.py 2>/dev/null | python3 -c "import json,sys; print([f for f in json.load(sys.stdin)['findings'] if f['check']=='adr-structure' and '023-' in f['path']])"` +Expected: `[]`. + +- [ ] **Step 3: Commit** + +```bash +git add docs/decisions/023-adr-structure.md +git commit -m "docs(adr): ADR-023 — ADR structure & lifecycle + +Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>" +``` + +--- + +## Task 4: Wire into CLAUDE.md and the review-repo command doc + +**Files:** +- Modify: `CLAUDE.md` ("Further reading" table) +- Modify: `.claude/commands/review-repo.md` (the deterministic-findings description, ~line 26–28) + +- [ ] **Step 1: Add the CLAUDE.md "Further reading" row** + +In `CLAUDE.md`, in the "Further reading" table, after the `Backup & disaster recovery` row, add: + +```markdown +| ADR structure & lifecycle | `docs/decisions/023-adr-structure.md` | +``` + +- [ ] **Step 2: Mention the new check in review-repo.md** + +In `.claude/commands/review-repo.md`, find (~line 27–28): + +```markdown +(roles, ADRs, runbooks, playbooks, scripts — your shard list) and **exact findings** +(markers, broken refs, unencrypted vaults). Fold these into the report verbatim. +``` + +Replace the parenthetical with: + +```markdown +(roles, ADRs, runbooks, playbooks, scripts — your shard list) and **exact findings** +(markers, broken refs, unencrypted vaults, ADR-structure violations). Fold these into +the report verbatim. +``` + +- [ ] **Step 3: Verify the CLAUDE.md link resolves** + +Run: `test -f docs/decisions/023-adr-structure.md && echo OK` +Expected: `OK`. + +- [ ] **Step 4: Commit** + +```bash +git add CLAUDE.md .claude/commands/review-repo.md +git commit -m "docs(adr): register ADR-023 and note adr-structure check + +Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>" +``` + +--- + +## Task 5: Backfill `## Status` into ADRs 001–018 + +**Files:** +- Modify: `docs/decisions/001-architecture.md` … `015-control-host.md` (insert a Status section) +- Modify: `docs/decisions/016-mesh-vpn.md`, `017-service-ui-verification.md`, `018-logging.md` (prepend a parseable Accepted line to the existing Status section) + +**Per-file date source:** the file's first git-commit (add) date — +`git log --diff-filter=A --format=%as -- <path> | tail -1` (yields `YYYY-MM-DD`). + +- [ ] **Step 1: For each of 001–015, insert a Status section after the title** + +For each file `docs/decisions/NNN-*.md` in 001–015, get its date and insert a Status +section between the title line and the first `##` heading. Worked example for +`001-architecture.md` (its line 2 is blank, line 3 is `## Context`): + +```bash +f=docs/decisions/001-architecture.md +d=$(git log --diff-filter=A --format=%as -- "$f" | tail -1) +# Insert after the title's trailing blank line, before "## Context": +``` + +Use the Edit tool to change, in `001-architecture.md`: + +```markdown +# ADR-001 — Architecture overview + +## Context +``` + +to: + +```markdown +# ADR-001 — Architecture overview + +## Status + +Accepted (<d>) + +## Context +``` + +where `<d>` is the date from the command above. Repeat for 002–015, matching each +file's actual title text and its first `##` heading (the heading is not always +`## Context`). + +- [ ] **Step 2: For 016, 017, 018, make the existing Status section parseable** + +These already have a `## Status` section whose first line is build-state prose. Get the +date (`git log --diff-filter=A --format=%as -- <path> | tail -1`) and prepend a +parseable Accepted line so the existing note becomes a trailing clause. Worked example +for `018-logging.md` — change its Status section from: + +```markdown +## Status + +Designed. **Authorable now:** ... +``` + +to: + +```markdown +## Status + +Accepted (<d>). Designed. **Authorable now:** ... +``` + +Repeat for 016 and 017 with their own dates and existing first lines. + +- [ ] **Step 3: Verify the whole corpus now passes the check** + +Run: `python3 scripts/repo-scan.py 2>/dev/null | python3 -c "import json,sys; v=[f for f in json.load(sys.stdin)['findings'] if f['check']=='adr-structure']; print('adr-structure findings:', len(v)); [print(' ', f['path'], '—', f['detail']) for f in v]"` +Expected: `adr-structure findings: 0`. + +- [ ] **Step 4: Run the full repo-scan test suite** + +Run: `.venv/bin/pytest tests/test_repo_scan.py -q` +Expected: PASS — 5 passed. + +- [ ] **Step 5: Commit** + +```bash +git add docs/decisions/0*.md docs/decisions/1*.md +git commit -m "docs(adr): backfill Status section into ADRs 001-018 + +Status header only (Accepted, dated from each file's first git-commit); +no decision content changed. Brings the back-catalogue to ADR-023 conformance. + +Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>" +``` + +--- + +## Final verification (after all tasks) + +- [ ] **Lint:** `make lint` — Expected: passes (docs + a stdlib script touched; ansible content unchanged). +- [ ] **Full deterministic scan clean for our check:** `python3 scripts/repo-scan.py 2>/dev/null | python3 -c "import json,sys; print('adr-structure:', sum(1 for f in json.load(sys.stdin)['findings'] if f['check']=='adr-structure'))"` → `adr-structure: 0`. +- [ ] **Tests green:** `.venv/bin/pytest tests/ -q` → all pass. +- [ ] **Branch ready:** invoke `superpowers:finishing-a-development-branch` to merge `feat/adr-structure` to `main` (trunk-based, no PR) and delete the branch. + +--- + +## Self-review notes + +- **Spec coverage:** §1 title/filename → Task 3 + template; §2 sections → Tasks 2/3 + check; §3 lifecycle → Task 3; §4 cross-refs → Task 3 `## Related`; §5 template → Task 2; §6 backfill → Task 5; §7 enforcement → Task 1 + Task 4. All covered. +- **Order nuance:** spec says sections come "in this order"; the check enforces presence + Status only. This is intentional and stated in both the spec's enforcement wording ("the four mandatory sections and a parseable Status line") and ADR-023's Decision §5 / "What was ruled out". Not a gap. +- **Type/name consistency:** `adr_structure_findings` and the `"adr-structure"` check key are used identically in the function, the `scan()` wiring, the tests, and both verification one-liners.