docs: record the second review round in the simplification audit
Why: the audit carried stale figures and working-tree citations after the
grill commits landed, and this review round's decisions needed recording.
Implementation Notes: line totals pinned to c07ca07, working-tree
citations replaced with commits, version-location count corrected, and a
new section lists this round's dispositions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -21,17 +21,21 @@ Counting convention: line counts are hand-edited `.apm/` source unless marked "i
|
||||
>
|
||||
> > **Re-measured (2026-09-14, at `a6434e0`):** the right-hand column originally read 31,473 / 6,050 / 3,471 / 2,360 / 923 / 2,083 = 46,360 and was labelled "Today" against "the current working tree". It did not reconcile to its own commit's tree — at `061bb3d`, where it was written, the six plugins measured 31,435 / 6,048 / 3,474 / 2,358 / 926 / 2,087 = 46,328 — and "the current working tree" is a basis that goes stale silently. Re-counted at `a6434e0` and the column now names its SHA. The baseline column is confirmed exact against `9eb8bc7`. Commits after `061bb3d` (`c96ca9c`, which deleted the six plugin-root `.mcp.json` files) account for most of the remaining drift.
|
||||
|
||||
> **Re-derived (2026-09-16, at HEAD on `docs/simplification-audit`):** the 2026-09-15 notes recording finding 14's merge (`467bbd7`, ADR-0025) and the pipefail fix (`4059cb4`) were written without correcting the headlines they annotate, so this pass re-counted every figure those two commits could have moved and corrected each in place above and below. Everything re-measured here came from a command run at HEAD — `git ls-files`, `wc -l`, `grep -c`, and `bash tests/run-tests.sh --strict` — never from an earlier note. What moved: finding 2 (two surviving sync gates → one), the `.pre-commit-config.yaml` hook counts (27/9 → 26/8, then back to 27/9 — see the correction at the end of this note), the skill census (39 → 38 and everything derived from it), finding 11's validator and `sources.md` figures, finding 16's whole numeric basis, and the stale `skill-audit/`, `agent-audit/` and `formatting-and-scripts.md` paths in findings 18, 19 and 33. §1's three rows re-measured: ~~**469**~~ → **471** tracked files (~~465~~ → 467 regular plus the 4 submodule gitlinks) / ~~**74,594**~~ → **75,441** lines; `plugins/` ~~**46,106** (62%)~~ → **46,127** (61%); the 38 `SKILL.md` bodies **2,409** (5.2% of plugin lines); enforcement ~~**20 `tests/test-*.sh` totalling 10,189 lines**~~ → **21 `tests/test-*.sh` totalling 10,608 lines**, the two runners **502** (`run-tests.sh` 283 + `run-bats.sh` 219), and `scripts/` ~~**2,901**~~ → **3,139**; kyberforge's validator scripts and their bats tests ~~**5,861**~~ → **5,876** + **6,015** (the merge deduplicated scripts and left the test corpus larger, not smaller — `git ls-files 'plugins/kyberforge/.apm/skills/*/scripts/*.sh'` and `.../tests/*.bats`). `run-tests.sh --strict` reports ~~**20 passed, 0 skipped, 0 failed**~~ → **21 passed, 0 skipped, 0 failed**.
|
||||
> **Re-derived (2026-09-16, at HEAD on `docs/simplification-audit`):** the 2026-09-15 notes recording finding 14's merge (`467bbd7`, ADR-0025) and the pipefail fix (`4059cb4`) were written without correcting the headlines they annotate, so this pass re-counted every figure those two commits could have moved and corrected each in place above and below. Everything re-measured here came from a command run at HEAD — `git ls-files`, `wc -l`, `grep -c`, and `bash tests/run-tests.sh --strict` — never from an earlier note. What moved: finding 2 (two surviving sync gates → one), the `.pre-commit-config.yaml` hook counts (27/9 → 26/8, then back to 27/9 — see the correction at the end of this note), the skill census (39 → 38 and everything derived from it), finding 11's validator and `sources.md` figures, finding 16's whole numeric basis, and the stale `skill-audit/`, `agent-audit/` and `formatting-and-scripts.md` paths in findings 18, 19 and 33. §1's three rows re-measured: ~~**469**~~ → **471** tracked files (~~465~~ → 467 regular plus the 4 submodule gitlinks) / ~~**74,594**~~ → **75,441** lines (pinned to `c07ca07`; see the note below); `plugins/` ~~**46,106** (62%)~~ → **46,127** (61%); the 38 `SKILL.md` bodies **2,409** (5.2% of plugin lines); enforcement ~~**20 `tests/test-*.sh` totalling 10,189 lines**~~ → **21 `tests/test-*.sh` totalling 10,608 lines**, the two runners **502** (`run-tests.sh` 283 + `run-bats.sh` 219), and `scripts/` ~~**2,901**~~ → **3,139**; kyberforge's validator scripts and their bats tests ~~**5,861**~~ → **5,876** + **6,015** (the merge deduplicated scripts and left the test corpus larger, not smaller — `git ls-files 'plugins/kyberforge/.apm/skills/*/scripts/*.sh'` and `.../tests/*.bats`). `run-tests.sh --strict` reports ~~**20 passed, 0 skipped, 0 failed**~~ → **21 passed, 0 skipped, 0 failed**.
|
||||
>
|
||||
> > **Re-measured (2026-09-16, at HEAD + working tree):** commit `8451169` added `check-skill-version-bump` — a pre-push hook, `scripts/check-skill-version-bump.sh` (238 lines in the working tree) and `tests/test-skill-version-bump.sh` (410) — after the figures above were taken, so each was one short. `.pre-commit-config.yaml` now has **27** `- id:` entries and **9** `stages: [pre-push]` (`grep -c -- "- id:"`; `grep -c "stages: \[pre-push\]"`), all nine repo-authored. The struck figures are replaced from these commands, run against the working tree: `git ls-files | wc -l`; `cat` over every non-gitlink tracked path `| wc -l`; `git ls-files plugins | xargs cat | wc -l`; `git ls-files scripts | xargs wc -l` (no untracked files under `scripts/`); `ls tests/test-*.sh | wc -l` and `cat tests/test-*.sh | wc -l`; `bash tests/run-tests.sh --strict`. The earlier 469 / 74,594 / 46,106 did not reproduce exactly at `8451169^` either (469 / 74,638 / 46,121), so they were taken at an earlier commit than this note's "at HEAD" says. Re-checked and unchanged, so left alone: `docs/research/` inside plugins (19,030) and repo-level `docs/research/` + `docs/notes/` (4,488). Not re-measured, and still carrying their last stated basis: the preload-tax and commit-share rows, §2's timings, and the per-plugin table in the note above.
|
||||
> > **Re-measured (2026-09-16, at `c07ca07`):** commit `8451169` added `check-skill-version-bump` — a pre-push hook, `scripts/check-skill-version-bump.sh` (238 lines) and `tests/test-skill-version-bump.sh` (410) — after the figures above were taken, so each was one short. `.pre-commit-config.yaml` now has **27** `- id:` entries and **9** `stages: [pre-push]` (`grep -c -- "- id:"`; `grep -c "stages: \[pre-push\]"`), all nine repo-authored. The struck figures are replaced from these commands. They were run against the working tree, and every figure reproduces exactly from the committed tree at `c07ca07`: `git ls-files | wc -l`; `cat` over every non-gitlink tracked path `| wc -l`; `git ls-files plugins | xargs cat | wc -l`; `git ls-files scripts | xargs wc -l` (no untracked files under `scripts/`); `ls tests/test-*.sh | wc -l` and `cat tests/test-*.sh | wc -l`; `bash tests/run-tests.sh --strict`. The earlier 469 / 74,594 / 46,106 did not reproduce exactly at `8451169^` either (469 / 74,638 / 46,121), so they were taken at an earlier commit than this note's "at HEAD" says. Re-checked and unchanged, so left alone: `docs/research/` inside plugins (19,030) and repo-level `docs/research/` + `docs/notes/` (4,488). Not re-measured, and still carrying their last stated basis: the preload-tax and commit-share rows, §2's timings, and the per-plugin table in the note above.
|
||||
>
|
||||
> > **Pinned (2026-09-16, review round):** the tracked-lines total is a moving figure, and this file moves it: `b426460` changed only this audit and took the committed total from 75,441 to 75,461. So the figure stays pinned to a commit instead of being chased. At `c07ca07` the committed tree has **471** paths (`git ls-tree -r`), **75,441** lines (every blob, `git cat-file -p … | wc -l`), and **46,127** lines under `plugins/` (61%). The 38 `SKILL.md` bodies there total **2,409** lines. At `c07ca07` the working-tree method above gives the same numbers, because nothing else was uncommitted. The `scripts/`, `tests/` and runner figures in the note above also reproduce at `c07ca07`, and at `b426460` too, since that commit changed no other file.
|
||||
|
||||
> **Reviewed (2026-09-14):** commits `718c79a` and `d2480b8` were put through a five-agent review. Result: **zero skill, agent or hook regressions** — 39 skills before and after, all gates passing, and both hook removals (`validate-plugins`, `check-plugin-content-sync`) genuinely moot rather than merely unenforced. One real functional regression was found — MCP propagation to consumers, broken by the same commit's manifest deletion; see finding 37 — along with the numeric and bookkeeping drift in this document's own 2026-09-14 notes, corrected in place above and below.
|
||||
|
||||
> **Reviewed again (2026-09-16):** the grill commits (`8451169` through `b426460`) went through a second review round. Its dispositions are in §11.
|
||||
|
||||
## 1. The shape of the problem
|
||||
|
||||
| Measure | Value |
|
||||
| ---------------------------------------------------------------------------| -----------------------------------------------------------------------------------------|
|
||||
| Tracked files / lines | ~~820 / 102,000~~ → ~~475 / 73,073~~ → ~~469 / 74,594~~ → 471 / 75,441 |
|
||||
| Tracked files / lines | ~~820 / 102,000~~ → ~~475 / 73,073~~ → ~~469 / 74,594~~ → 471 / 75,441 (at `c07ca07`) |
|
||||
| Lines in `plugins/` | ~~70,600 (69% of repo)~~ → ~~46,301 (63% of repo)~~ → ~~46,106 (62% of repo)~~ → 46,127 (61% of repo) |
|
||||
| Of which the ~~39~~ → 38 `SKILL.md` files a model actually loads | ~~about 2,600 lines (under 4% of plugin lines)~~ → ~~2,509 lines (5.4% of plugin lines)~~ → 2,409 lines (5.2% of plugin lines) |
|
||||
| Generated flat mirror files (byte copies of `.apm/`) | ~~263 files, ~22,000 lines~~ → 0 (deleted 2026-09-14, see below) |
|
||||
@@ -43,7 +47,7 @@ Counting convention: line counts are hand-edited `.apm/` source unless marked "i
|
||||
| Preload tax (39 skill names + descriptions) | 10,987 chars, ~2,750 tokens per session |
|
||||
| Commits since 2026-05-10 / share touching hook, test, gate, vale, or sync | 447 / ~25% |
|
||||
|
||||
> **Corrected then done (2026-09-14):** the mirror row's figure was wrong. The true mirror was **213 files / 20,061 lines**, not 263 / ~22,000 — the original count swept in files that were never mirror output. All 213 were deleted in commit `718c79a` on `docs/simplification-audit` (245 files changed, 298 insertions, 22,602 deletions across the whole change), so the row is now zero. The enforcement row is stale on **both** halves — it was correct at the 2026-09-10 baseline (`9eb8bc7`: 33 `- id:` entries, 14 repo-authored pre-push hooks), but `.pre-commit-config.yaml` today has ~~**27 entries and 9 `stages: [pre-push]`**~~ → ~~**26 entries and 8 `stages: [pre-push]`**~~ → **27 entries and 9 `stages: [pre-push]`** (`467bbd7` removed `check-vale-style-sync` with finding 14's merge; `8451169` then added `check-skill-version-bump`; re-measured 2026-09-16 at HEAD + working tree with `grep -c -- "- id:"` and `grep -c "stages: \[pre-push\]"` on `.pre-commit-config.yaml`). Like for like that is 14 → ~~9~~ → ~~8~~ → 9 repo-authored pre-push hooks. The stage *reports* ~~11~~ → ~~10~~ → 11, because the 2 pre-commit `meta` hooks also run there — a different counting basis; see the corrected §3 target, which states it the same way.
|
||||
> **Corrected then done (2026-09-14):** the mirror row's figure was wrong. The true mirror was **213 files / 20,061 lines**, not 263 / ~22,000 — the original count swept in files that were never mirror output. All 213 were deleted in commit `718c79a` on `docs/simplification-audit` (245 files changed, 298 insertions, 22,602 deletions across the whole change), so the row is now zero. The enforcement row is stale on **both** halves — it was correct at the 2026-09-10 baseline (`9eb8bc7`: 33 `- id:` entries, 14 repo-authored pre-push hooks), but `.pre-commit-config.yaml` today has ~~**27 entries and 9 `stages: [pre-push]`**~~ → ~~**26 entries and 8 `stages: [pre-push]`**~~ → **27 entries and 9 `stages: [pre-push]`** (`467bbd7` removed `check-vale-style-sync` with finding 14's merge; `8451169` then added `check-skill-version-bump`; re-measured 2026-09-16 at HEAD (`b426460`) with `grep -c -- "- id:"` and `grep -c "stages: \[pre-push\]"` on `.pre-commit-config.yaml`). Like for like that is 14 → ~~9~~ → ~~8~~ → 9 repo-authored pre-push hooks. The stage *reports* ~~11~~ → ~~10~~ → 11, because the 2 pre-commit `meta` hooks also run there — a different counting basis; see the corrected §3 target, which states it the same way.
|
||||
|
||||
> **Re-measured (2026-09-14, at `a6434e0`):** this table is a **dated snapshot corrected in place**, not a live figure — every arrow above reads "baseline (2026-09-10, `9eb8bc7`) → value at the stated commit". Three further rows were still carrying baseline values after `d2480b8`/`061bb3d` corrected their neighbours, and are now corrected at `a6434e0`:
|
||||
>
|
||||
@@ -100,8 +104,8 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
|
||||
> **Grilled and closed (2026-09-14):** `apm-audit-ci` — already resolved before this audit was written: `.pre-commit-config.yaml`'s own comment block (added in commit `a155af6`, months before this audit) already rebuts the "overclaimed description" complaint and gives a dated, verified justification for what the hook still checks. Keep, no action. `apm-marketplace-check` — its stated purpose ("the only hook that checks remote package references rather than local-source paths") is void: finding 35 (commit `568ca74`) already removed the only remote package entry, so every `marketplace.packages[]` entry is now a local `./plugins/<name>` path and the hook is pure overlap with `apm-pack-check-clean`. Removed the hook entry, and corrected the now-stale "does NOT join apm-marketplace-check ... on the offline SKIP= list" comment on `apm-audit-ci` (there is no offline skip list any more — every pre-push hook already passes offline per `README.md`). Updated `README.md` (tool table, "Offline?" section) and `docs/spec/gates.md` (hook table, hook counts 13→11 self-authored / 15→13 total, the "Three of these shell out to apm" paragraph, and the "Pushing without a network" section) accordingly. Verified: `apm audit --ci` still passes per-plugin, and the pre-push hook count now matches `.pre-commit-config.yaml`.
|
||||
> **Corrected and closed (2026-09-14, at `a6434e0`):** two things above went stale within hours of being written, and the finding was never given a marker.
|
||||
>
|
||||
> - **"Keep the two `claude plugin validate` hooks"** is void. `718c79a` (ADR-0024) deleted `validate-plugins` — the ADR's own reasoning is that `claude plugin validate` reads manifests only and could never detect the empty-content defect it was credited with guarding, and with the per-plugin manifests gone it has nothing left to read. Only **`validate-marketplace`** survives, over the one manifest this repo still ships (`.claude-plugin/marketplace.json`). Of the six hooks this finding named, three now exist: `validate-marketplace`, `apm-pack-check-clean`, `apm-audit-ci`. Verified against `.pre-commit-config.yaml`: ~~27 `- id:` entries, 9 with `stages: [pre-push]`~~ → ~~**26 `- id:` entries, 8 with `stages: [pre-push]`**~~ → **27 `- id:` entries, 9 with `stages: [pre-push]`** (re-measured 2026-09-16 at HEAD + working tree; `8451169` added `check-skill-version-bump`), no `validate-plugins` entry.
|
||||
> - **The gates.md figures above ("13→11 self-authored / 15→13 total") were correct for `0dffff3` and are no longer current.** `718c79a` removed two more pre-push hooks after that commit, and `docs/spec/gates.md:24` read **11 reported / 9 self-authored** when this note was written; finding 14's merge has since removed `check-vale-style-sync`, and it ~~now reads **10 reported / 8 self-authored**~~ → read **10 reported / 8 self-authored** until `8451169` added `check-skill-version-bump`; at HEAD + working tree (2026-09-16) `gates.md:24-28` reads **11 reported / 9 self-authored** again. Read the count from that file, not from this note.
|
||||
> - **"Keep the two `claude plugin validate` hooks"** is void. `718c79a` (ADR-0024) deleted `validate-plugins` — the ADR's own reasoning is that `claude plugin validate` reads manifests only and could never detect the empty-content defect it was credited with guarding, and with the per-plugin manifests gone it has nothing left to read. Only **`validate-marketplace`** survives, over the one manifest this repo still ships (`.claude-plugin/marketplace.json`). Of the six hooks this finding named, three now exist: `validate-marketplace`, `apm-pack-check-clean`, `apm-audit-ci`. Verified against `.pre-commit-config.yaml`: ~~27 `- id:` entries, 9 with `stages: [pre-push]`~~ → ~~**26 `- id:` entries, 8 with `stages: [pre-push]`**~~ → **27 `- id:` entries, 9 with `stages: [pre-push]`** (re-measured 2026-09-16 at HEAD, `b426460`; `8451169` added `check-skill-version-bump`), no `validate-plugins` entry.
|
||||
> - **The gates.md figures above ("13→11 self-authored / 15→13 total") were correct for `0dffff3` and are no longer current.** `718c79a` removed two more pre-push hooks after that commit, and `docs/spec/gates.md:24` read **11 reported / 9 self-authored** when this note was written; finding 14's merge has since removed `check-vale-style-sync`, and it ~~now reads **10 reported / 8 self-authored**~~ → read **10 reported / 8 self-authored** until `8451169` added `check-skill-version-bump`; at HEAD (`b426460`, 2026-09-16) `gates.md:24-28` reads **11 reported / 9 self-authored** again. Read the count from that file, not from this note.
|
||||
>
|
||||
> Marked `[x]`: all three of this finding's decisions are resolved — `check-manifests` deleted (`e647f14`), `apm-audit-ci` kept on the grill above, `apm-marketplace-check` removed (`0dffff3`).
|
||||
|
||||
@@ -139,7 +143,7 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
|
||||
8. **`docs/spec/gates.md` (1,048 lines) is roughly 15% "what is enforced" and 85% post-mortems** of defects already fixed and pinned by tests. The 60-line hook table is the useful part. Target 200 lines. The same applies to the 106 comment lines in `.pre-commit-config.yaml` and to `scripts/`, where 8 of 15 files are 40 to 60% comments. Effort M.
|
||||
> **Partially done (2026-09-13):** see commit `a35f5e8` on `docs/simplification-audit`. The 85%-post-mortem characterization was stale — the file had already shrunk to 966 lines by other findings, and most of what remained is load-bearing "why this design" rationale cited by ADRs and tests, not dead incident narration. Cut only the two genuinely stale passages: a reproduction paragraph carrying explicitly outdated numbers, and a retrofit-process narrative superseded by current state — a 36-line cut, 966 → 930 as measured at commit `a35f5e8`. Those two figures describe that commit only, not the file: `718c79a` and later findings have edited `gates.md` again, so read its current length from the file rather than quoting a number here. `.pre-commit-config.yaml`'s comments were left untouched; on inspection they're compact constraint notes, not filler. Target of 200 lines not reached and not recommended — would require deleting content the file itself flags as load-bearing.
|
||||
>
|
||||
> **Closed (2026-09-16, grill): done to the extent recommended.** The `gates.md` cut in `a35f5e8` stands; the 200-line target stays rejected (the file is ~~1,113 lines at HEAD~~ → **1,164** lines at HEAD + working tree, `wc -l docs/spec/gates.md`, grown by later findings' sections, and read on demand only). The tests target below is struck: ~~findings 3, 5 and 16 each found dense suites to be named-incident regression coverage~~ → findings 3 and 5 each found dense test suites to be named-incident regression coverage, and finding 16 found the same of dense validator code, whose comments are an incident log. Any future cut to a test suite is its own finding and starts by reading that suite's header.
|
||||
> **Closed (2026-09-16, grill): done to the extent recommended.** The `gates.md` cut in `a35f5e8` stands; the 200-line target stays rejected (the file is ~~1,113 lines at HEAD~~ → **1,164** lines at HEAD (`b426460`), `wc -l docs/spec/gates.md`, grown by later findings' sections, and read on demand only). The tests target below is struck: ~~findings 3, 5 and 16 each found dense suites to be named-incident regression coverage~~ → findings 3 and 5 each found dense test suites to be named-incident regression coverage, and finding 16 found the same of dense validator code, whose comments are an incident log. Any future cut to a test suite is its own finding and starts by reading that suite's header.
|
||||
|
||||
**Proposed target.** ~~Pre-push 14 hooks to 6: `run-tests`, `validate-plugins`, `validate-marketplace`, `apm-pack-check-clean`, `check-plugin-content-sync`, `check-release-needed`.~~
|
||||
|
||||
@@ -147,7 +151,7 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
|
||||
|
||||
> **Superseded count (2026-09-15):** finding 14 deleted `check-vale-style-sync` with the merge into `factory-audit` (ADR-0025), so pre-push ~~is now~~ → was **8 repo-authored hooks** (10 reported). The dated note above is the state on 2026-09-14; see finding 14's note for the correction.
|
||||
>
|
||||
> > **Superseded count (2026-09-16, at HEAD + working tree):** `8451169` added `check-skill-version-bump` (finding 33), so pre-push is **9 repo-authored hooks** (11 reported) — the eight above plus that one. Measured with `grep -c "stages: \[pre-push\]" .pre-commit-config.yaml`.
|
||||
> > **Superseded count (2026-09-16, at HEAD `b426460`):** `8451169` added `check-skill-version-bump` (finding 33), so pre-push is **9 repo-authored hooks** (11 reported) — the eight above plus that one. Measured with `grep -c "stages: \[pre-push\]" .pre-commit-config.yaml`.
|
||||
|
||||
Pre-commit stays roughly as is minus `skill-frontmatter`, and minus `check-ast` once finding 9 removes the only `.py` files. ~~Tests 26 files to about 10 (12,400 to about 5,000 lines).~~ Keep bats and its three submodules; the 351 bats tests ship inside plugins and are the right tool there. ~~Do not port the bash suites to bats; delete them instead.~~ **Struck (2026-09-16, grill):** see finding 8's closing note — the suites are regression coverage (findings 3 and 5; finding 16 found the same of the validators they test).
|
||||
|
||||
@@ -345,7 +349,7 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
>
|
||||
> **"The rules already live in `gates.md`" is backwards.** ~~`docs/spec/gates.md:349-352`~~ → `docs/spec/gates.md:397-400` explicitly *declines* to restate ADR-0020's numbers: *"they live in ADR-0020's Consequences section… Quoting them here would just create a second copy to go stale."* gates.md is a consumer of the ADR, not its replacement. **The index proposal also contradicts a recorded decision** — `docs/spec/architecture.md:90`: *"There is no index file — the directory holds numbered ADRs whose filenames state their decision, so `ls docs/adr/` is the index."*
|
||||
>
|
||||
> > **Repointed (2026-09-16, at HEAD + working tree):** the quoted `gates.md` passage moved from `:349-352` to `:397-400` as later sections were added above it; verified with `grep -n "Quoting them here" docs/spec/gates.md` and `sed -n 397,400p`. `architecture.md:90` still resolves.
|
||||
> > **Repointed (2026-09-16, at HEAD `b426460`):** the quoted `gates.md` passage moved from `:349-352` to `:397-400` as later sections were added above it; verified with `grep -n "Quoting them here" docs/spec/gates.md` and `sed -n 397,400p`. `architecture.md:90` still resolves.
|
||||
>
|
||||
> **No superseded body can be truncated — every one is quoted by content, not merely cited by number.** ADR-0001's body text is quoted verbatim at `docs/adr/0015:5,36`, and `factory-integration-decisions.md:133` lists "Pull-based distribution (ADR-0001)" as settled, a concept living only in its consequences bullets. ADR-0006's version-parity invariant is stated only at `0006:23` and is relied on by `0014:116` and `0024:183-185` — and its banner (17 lines) is already longer than its body (7). ADR-0017's own banner says its diagnosis "is still accurate about how Claude Code's installer works", and ADR-0024 cites its body in eight places. ADR-0002 is only partially superseded and is cited as a design source by a shipped skill.
|
||||
>
|
||||
@@ -357,7 +361,7 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
>
|
||||
> **The framing question this finding never notices:** it proposes reversing a convention the repo *just* re-affirmed — every banner added by the ADR-0024 work ends with some form of "kept below as the historical record". Is a superseded ADR's body a record or dead weight? Nothing here is mechanical; every proposed cut touches text another file quotes.
|
||||
>
|
||||
> **Closed (2026-09-16, grill): not proceeding.** Decision: a superseded or accepted ADR's text is the historical record (the Nygard convention, and what every ADR-0024 banner already says), so no body is truncated, no amendment is deleted, and ADR-0020's Context is left intact. That removes every remaining cut — ADR-0017's amendments are part of its record, and ADR-0024 consequence 7 (`0024:213-252`, ~40 lines) summarises them rather than restating them in full as the note above says. Corrected headline for anyone quoting it: ~~**79% of `docs/adr/` lines sit in ten ADRs over 150 lines** (measured 2026-09-14)~~ → **80% of `docs/adr/` lines (2,983 of 3,709, across 25 files) sit in eleven ADRs over 150 lines** (at HEAD + working tree, 2026-09-16, from `wc -l docs/adr/*.md`; ADR-0025 joined the cohort and the ADR-0019 and ADR-0022 amendments grew the directory). The 79%-in-ten figure was the 2026-09-14 state; the 2,740 figure is the `a3e721e` state only.
|
||||
> **Closed (2026-09-16, grill): not proceeding.** Decision: a superseded or accepted ADR's text is the historical record (the Nygard convention, and what every ADR-0024 banner already says), so no body is truncated, no amendment is deleted, and ADR-0020's Context is left intact. That removes every remaining cut — ADR-0017's amendments are part of its record, and ADR-0024 consequence 7 (`0024:213-252`, ~40 lines) summarises them rather than restating them in full as the note above says. Corrected headline for anyone quoting it: ~~**79% of `docs/adr/` lines sit in ten ADRs over 150 lines** (measured 2026-09-14)~~ → **80% of `docs/adr/` lines (2,983 of 3,709, across 25 files) sit in eleven ADRs over 150 lines** (at HEAD `b426460`, 2026-09-16, from `wc -l docs/adr/*.md`; ADR-0025 joined the cohort and the ADR-0019 and ADR-0022 amendments grew the directory). The 79%-in-ten figure was the 2026-09-14 state; the 2,740 figure is the `a3e721e` state only.
|
||||
|
||||
29. [x] ~~**The same facts are stated in full three or four times.** "Edit `.apm/`, never the mirror": README (2 paragraphs), AGENTS.md (2 paragraphs), architecture.md (2 paragraphs plus the lost-README anecdote), ADR-0017. The apm.lock / SessionStart story: README (11 lines), AGENTS.md, ADR-0018, ADR-0019, gates.md. The offline `SKIP=` command and the three-stage install each appear three times. Rule: README has the how-to, AGENTS.md has one-line rules with links, architecture.md has mechanics. Effort S.~~
|
||||
> **Corrected then partially done (2026-09-14):** independent re-verification found the "edit `.apm/`, never the mirror" and apm.lock/SessionStart clusters confirmed but the third overstated — no file documents an offline `SKIP=` command (the one `SKIP=`-adjacent mention in `gates.md` explicitly says a *different* opt-out "is not `SKIP=`"), and "three-stage install" appears twice, not three times, with no restatement worth trimming. Trimmed the two confirmed clusters: README's "Editing plugin content" and AGENTS.md's "Edit `.apm/`, never the flat mirror" sections cut to the how-to/one-line-plus-link split the finding itself proposed, full mechanics (the `rm -rf` behavior and the `plugins/kyberforge/hooks/README.md` anecdote) staying solely in `docs/spec/architecture.md`. README's "Keeping the install current" and AGENTS.md's apm.lock bullet trimmed to drop the restated `apm outdated`/`apm update --yes` timing narrative, pointing to ADR-0019 as the canonical mechanism instead. No test greps the trimmed wording (checked).
|
||||
@@ -387,20 +391,20 @@ Not covered by the area audits above; found on a final sweep of the root config
|
||||
>
|
||||
> **Do this regardless of anything else: delete the six root `apm.yml` `packages[].version` lines.** Tested in an isolated scratch copy (repo untouched): setting `plugins/lint/apm.yml` to `9.9.9` while root says `1.1.7` **passes `apm pack --check-versions --check-clean` with exit 0**, reports `[matches]`, and emits `1.1.7` — the curator entry wins (`output_mappers.py:163-171`). Deleting the root `version:` line entirely leaves `marketplace.json` **byte-unchanged** (`builder._fetch_local_metadata` reads the plugin's own `apm.yml`). All six are removable with zero output diff. This is unpoliced duplication that silently ships the wrong number on drift. Effort S, no decision needed.
|
||||
>
|
||||
> Corrected headline: **two** hand-maintained per-plugin locations (**three** for kyberforge), not four — the audit's own "already done" note records the `plugin.json` deletion but never fixed the headline. Gitea skills drift across **six** values (`0.1.2, 0.1.3, 0.1.4, 0.1.5, 0.1.6, 1.0.1`), not five — ~~still six at HEAD on 2026-09-16~~ → **five** again at HEAD + working tree on 2026-09-16 (`0.1.2, 0.1.4, 0.1.5, 0.1.6, 1.0.1`), because `8451169` bumped `gitea-branches` 0.1.3 → 0.1.4 under the new version-bump gate and it was the only skill at 0.1.3; re-derived by parsing `metadata.version` out of each `plugins/gitea/.apm/skills/*/SKILL.md` with PyYAML. ~~39 `SKILL.md` files ✓~~ → **38** carry it, and all 38 do (re-measured 2026-09-16; ADR-0025's merge took one). The `0.4.6` duplication between root `version:` and `marketplace.version:` is **forced by apm, not a repo choice** — deleting `marketplace.version` makes `--check-clean` go dirty.
|
||||
> Corrected headline: ~~**two** hand-maintained per-plugin locations (**three** for kyberforge)~~ → **one** hand-maintained per-plugin version location, `plugins/<name>/apm.yml` (**two** for kyberforge, adding the `executables.allow` key), not four. `2def060` deleted the root `packages[].version` lines (corrected 2026-09-16, review round). The root `packages[].description:` duplicates dropped in the same round are a separate duplication, not a version location, so they do not change this count — the audit's own "already done" note records the `plugin.json` deletion but never fixed the headline. Gitea skills drift across **six** values (`0.1.2, 0.1.3, 0.1.4, 0.1.5, 0.1.6, 1.0.1`), not five — ~~still six at HEAD on 2026-09-16~~ → **five** again at HEAD (`b426460`) on 2026-09-16 (`0.1.2, 0.1.4, 0.1.5, 0.1.6, 1.0.1`), because `8451169` bumped `gitea-branches` 0.1.3 → 0.1.4 under the new version-bump gate and it was the only skill at 0.1.3; re-derived by parsing `metadata.version` out of each `plugins/gitea/.apm/skills/*/SKILL.md` with PyYAML. ~~39 `SKILL.md` files ✓~~ → **38** carry it, and all 38 do (re-measured 2026-09-16; ADR-0025's merge took one). The `0.4.6` duplication between root `version:` and `marketplace.version:` is **forced by apm, not a repo choice** — deleting `marketplace.version` makes `--check-clean` go dirty.
|
||||
>
|
||||
> **"Nothing consumes `metadata.version`" is false twice over.** Machine enforcers: ~~`scripts/skill-size-check.sh:1365-1374`~~ → `scripts/skill-size-check.sh:1370-1379` and ~~`skill-audit/scripts/validate.sh:1292-1332`~~ → `plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-checks-skill.sh:235-277`, both FAIL tier, the latter citing ADR-0022 by name, with four dedicated bats cases and ~10 fixture generators baking the field in.
|
||||
> Instruction-level consumers: `skill-author/SKILL.md:60` (bump minor on create, patch on improve), `create.md:89,101`, `improve.md:82`, and `forge/SKILL.md:54` + `references/version-bump.md`. apm parses it for Chatmode/Instruction/Context primitives but not for Skills, and never emits it. Precise statement: the value is written, shape-validated, and never read *downstream* — it is an agent-visible revision counter, and the drift table shows the counter is not being maintained.
|
||||
>
|
||||
> > **Repointed (2026-09-16, at HEAD):** `skill-audit/scripts/validate.sh` no longer exists — ADR-0025's merge moved the ADR-0022 check into `factory-audit`'s skill-side check library, where it is the `SEMVER_RE` block (comment header at `:235`, `fail()` calls at `:261` and `:275`). `skill-size-check.sh` grew by 5 lines above the block since `062ca47`, hence the shifted range there. All five instruction-level citations still resolve at HEAD, verified with `sed -n`.
|
||||
>
|
||||
> **ADR-0022 already considered and rejected dropping the field**, on the grounds that `skill-author` depends on it to decide whether a pass owes a bump — a rationale still live today. Superseding costs: rewrite skill-author's bump rule, delete `forge`'s version-bump route premise, strip two scripts, delete four bats cases, fix ~10 fixture generators, edit the scaffold template, update ~~`gates.md:97`~~ → `gates.md:145` — and re-open the "is this field present here?" question issue #127 closed, just from the other side. *(Repointed 2026-09-16, at HEAD + working tree: the `metadata.version` frontmatter sentence formerly at `gates.md:97` is now at `:143-146`, the field itself on `:145`; verified with `grep -n "metadata.version" docs/spec/gates.md`.)* **Recommendation: keep it and fix the actual defect, which is that nobody bumps it.** Either enforce the bump in the skill-author workflow or declare the values advisory in the ADR.
|
||||
> **ADR-0022 already considered and rejected dropping the field**, on the grounds that `skill-author` depends on it to decide whether a pass owes a bump — a rationale still live today. Superseding costs: rewrite skill-author's bump rule, delete `forge`'s version-bump route premise, strip two scripts, delete four bats cases, fix ~10 fixture generators, edit the scaffold template, update ~~`gates.md:97`~~ → `gates.md:145` — and re-open the "is this field present here?" question issue #127 closed, just from the other side. *(Repointed 2026-09-16, at HEAD `b426460`: the `metadata.version` frontmatter sentence formerly at `gates.md:97` is now at `:143-146`, the field itself on `:145`; verified with `grep -n "metadata.version" docs/spec/gates.md`.)* **Recommendation: keep it and fix the actual defect, which is that nobody bumps it.** Either enforce the bump in the skill-author workflow or declare the values advisory in the ADR.
|
||||
>
|
||||
> **The `tagPattern` claim is refuted — inert, not broken.** Under `versioning.strategy: per_package`, apm never reads it: `version_check.py:262` gates on `strategy == "tag_pattern"`, and `builder.py:641,781` are reachable only for *remote* source entries, while all six packages here are local paths. The `v1.0.0`/`v2.0.0`/`v2.0.1` tags are not "a third scheme" — they are the `.pre-commit-hooks.yaml` external-consumer contract tags from finding 36, a different axis entirely. Latent risk only: if `dependencies.apm` ever gains `ref:` pins, tagPattern goes live against per-package tags that do not exist.
|
||||
>
|
||||
> Also: **`executables.allow` should be kept** — it is version-keyed by apm's design and `check-executables-allow-sync` guards a real silent failure (ADR-0019). And ADR-0006's ADR-0024 amendment asserting *"`apm.yml`'s `version:` is the only version field a plugin has"* is inaccurate while root `packages[].version` exists — fixed by the deletion above.
|
||||
>
|
||||
> **Decided and done (2026-09-16, grill): enforce the bump.** Advisory status and dropping the field were both rejected. The six root `apm.yml` `packages[].version` lines are deleted (`apm pack --check-versions --check-clean` still passes, output unchanged), which also makes ADR-0006's "the only version field a plugin has" true. `scripts/check-skill-version-bump.sh` now runs at pre-push: any skill directory that changed against its merge-base with `main`, `tests/` excluded, must carry a strictly higher `metadata.version` than ~~`main`~~ → the same skill had at that merge-base (not `main`'s current tip; see `gates.md:90` and `:111-112`, and the hook comment at `.pre-commit-config.yaml:203-206`); new, renamed and deleted skills are exempt; every plugin is covered, `bin` included. Recorded as a dated section in ADR-0022, not a new ADR. The 17 skills changed on this branch without a bump took a patch bump in the same commit. The `executables.allow` pin and the inert `tagPattern` are left as the note above recommends.
|
||||
> **Decided and done (2026-09-16, grill): enforce the bump.** Advisory status and dropping the field were both rejected. The six root `apm.yml` `packages[].version` lines are deleted (`apm pack --check-versions --check-clean` still passes, output unchanged), which also makes ADR-0006's "the only version field a plugin has" true. `scripts/check-skill-version-bump.sh` now runs at pre-push: any skill directory that changed against its merge-base with `main`, `tests/` excluded, must carry a strictly higher `metadata.version` than ~~`main`~~ → the same skill had at that merge-base (not `main`'s current tip; see `gates.md:90` and `:111-112`, and the hook comment at `.pre-commit-config.yaml:203-206`. *Amended in the 2026-09-16 review round: the gate now also compares against the `origin/main` tip; see §11.*); new, renamed and deleted skills are exempt; every plugin is covered, `bin` included. Recorded as a dated section in ADR-0022, not a new ADR. The 17 skills changed on this branch without a bump took a patch bump in the same commit. The `executables.allow` pin and the inert `tagPattern` are left as the note above recommends.
|
||||
|
||||
34. **The SessionStart hook auto-updates the install on every startup.** `check-apm-current.sh` runs `apm outdated` (network, 60 s timeout) and then `apm update --yes` (300 s timeout) at every session start, rewriting `apm.lock.yaml`. That is why the lock file is dirty at the start of this session and why `AGENTS.md` has to explain "commit or discard it deliberately". It is a 60-line script with a 368-line test, an ADR (0019), the `executables.allow` pin, and a sync hook behind it. For a repo that is its own source, the update belongs in `install.sh` or a manual `apm update`, not in session startup. Effort S to remove; the design question is whether auto-update at startup is wanted at all.
|
||||
> **Refuted (2026-09-14, at HEAD `062ca47`). The evidence is inverted: the finding cites as proof of over-eagerness a session in which the mechanism did not fire, and the observed state is the exact silent failure ADR-0019 exists to prevent.**
|
||||
@@ -443,7 +447,7 @@ Not covered by the area audits above; found on a final sweep of the root config
|
||||
>
|
||||
> Removal is ~1,000 lines and mechanical: `.pre-commit-hooks.yaml`, `check-release-needed.sh`, both tests, the hook block at ~~`.pre-commit-config.yaml:194-201`~~ → `.pre-commit-config.yaml:186-193`, the ~~`gates.md:83`~~ → `gates.md:96` row and its "External consumers" section (`gates.md:789`). Tags are inert and can stay. **The one real loss:** `test-vale-hooks-consumer.sh` is the sole test exercising the entry-resolution path that once shipped broken — it goes only *with* the manifest, never while it stays. Reversal cost is bounded provided ADR-0014 and `LESSONS.md:101,105` are kept: they preserve the `entry[0]`-only constraint that took three review rounds to find.
|
||||
>
|
||||
> > **Repointed (2026-09-16, at HEAD + working tree):** the line citations in this note were taken at `062ca47` and have shifted. The `check-release-needed` block is now `.pre-commit-config.yaml:186-193` (`grep -n "id: check-release-needed"`); the three internal `repo: local` wirings' `entry:` lines are `:221` (`skill-size-check`), `:254` and `:269` (the two `vale-audit-prefilter-*` hooks, both now on `factory-audit`'s one `vale-wrap.sh`); the `check-release-needed` table row is `gates.md:96` and the "External consumers" section heading is `gates.md:789`. Verified with `grep -n` and `sed -n`. Line counts in this note were not re-measured.
|
||||
> > **Repointed (2026-09-16, at HEAD `b426460`):** the line citations in this note were taken at `062ca47` and have shifted. The `check-release-needed` block is now `.pre-commit-config.yaml:186-193` (`grep -n "id: check-release-needed"`); the three internal `repo: local` wirings' `entry:` lines are `:221` (`skill-size-check`), `:254` and `:269` (the two `vale-audit-prefilter-*` hooks, both now on `factory-audit`'s one `vale-wrap.sh`); the `check-release-needed` table row is `gates.md:96` and the "External consumers" section heading is `gates.md:789`. Verified with `grep -n` and `sed -n`. Line counts in this note were not re-measured.
|
||||
|
||||
37. [x] ~~**Two `.mcp.json` files declare an Obsidian vault server over `docs/`** (root and `plugins/bin/`; the other five plugin `.mcp.json` files are empty stubs), while `AGENTS.md` forbids using an external memory system for this repo. If the Obsidian tools are unused, drop both and the `reinject_mcp_servers` explanation in the bin README; the bin `plugin.json` pair regenerates. Effort S.~~
|
||||
> **Not proceeding (2026-09-13):** premise doesn't hold. The server exposes the repo's own git-tracked `docs/` folder — not an external/off-repo store — so it isn't the "external memory system" AGENTS.md's rule targets. It was deliberately added and versioned (3 commits), is documented as current intended behavior in both READMEs, and ADR-0018 uses it as its only concrete worked example of apm's MCP-dependency propagation mechanism actually working. No skill invokes the Obsidian tools as a workflow step, but that alone doesn't make the config dead. No changes made; recommend a human confirm whether the vault tooling is still wanted before removing it.
|
||||
@@ -569,4 +573,17 @@ The recurring failure mode is worth naming, because it has now produced six wron
|
||||
|
||||
- **~~A live bug in always-on context.~~ Fixed (2026-09-15).** The deployed `core/instructions/governance.md` cited `docs/HUMANS.md`, which does not exist — the file is `docs/wiki/HUMANS.md`. Five occurrences across three files (`governance.md:82`, which was self-inconsistent against its own correct line 73; `CONTROLS.md:5,101,106`; `ai-constitution.md:238`), in a file `@`-imported into every session in every project. All five now point at `docs/wiki/HUMANS.md`. Note the deployed copy under `~/.claude/` no longer matches the repo until `scripts/install.sh` re-runs.
|
||||
- **This checkout's install is stale and there is a branch hazard.** At the time of the wave `apm outdated` reported 6 outdated dependencies, 9 commits behind `main`, with a clean tree and nothing reporting it. **Do not run `apm update` on this branch** — it resolves against `main` and restores the obsidian MCP server that commit `c96ca9c` removed here. Reproduced. The mechanism is worse than "reinstalls `plugins/bin/.mcp.json`": apm never writes into `plugins/`, it re-materialises the file under `apm_modules/` and regenerates the repo-root `/.mcp.json` — which `c96ca9c` gitignored, so the restoration would not appear in `git status` at all. This belongs in ADR-0019's Consequences; see finding 34.
|
||||
> **Landed (2026-09-16):** commit `afcf477` amended ADR-0019's Consequences with the feature-branch hazard, and the discard guidance for a feature branch is now also in `AGENTS.md` and `README.md` (working tree, same date). See finding 34's closing note.
|
||||
> **Landed (2026-09-16):** commit `afcf477` amended ADR-0019's Consequences with the feature-branch hazard, and the discard guidance for a feature branch is now also in `AGENTS.md` and `README.md` (`dd0b923`). The reason those two files and ADR-0019 gave for discarding the lock was wrong, and the review round below corrected it. See finding 34's closing note.
|
||||
|
||||
## 11. Review round on the grill commits (2026-09-16)
|
||||
|
||||
A review of this branch's grill commits (`8451169` through `b426460`) raised the findings below. Each was decided in this round and fixed on this branch. Several agents made the fixes in parallel. This note records the decisions. It does not re-verify the details of fixes it did not make itself: for those, read the named file or ADR.
|
||||
|
||||
- **Version-bump gate baseline.** The gate now also compares against the `origin/main` tip, as well as the merge-base. ADR-0022 is amended to match. This supersedes the merge-base-only wording in finding 33's closing note.
|
||||
- **Version-bump gate parsing.** Frontmatter that starts with a BOM, and version components with leading zeros, are now parsed correctly.
|
||||
- **Version-bump gate tests.** The gaps the review found in `tests/test-skill-version-bump.sh` are closed.
|
||||
- **Plugin patch bumps.** Five plugins took a patch bump: `bin`, `git`, `gitea`, `core` and `lint`. kyberforge was already at `2.0.0` on this branch and needed no further bump.
|
||||
- **Root `apm.yml` descriptions.** The `packages[].description:` duplicates are dropped, following the `version:` lines `2def060` already removed. ADR-0021 is amended. This is not a version location, so finding 33's corrected count (one per plugin, two for kyberforge) is unaffected.
|
||||
- **Remote-entry `version:` guidance.** The `apm-workflow` references now give the right guidance on a remote marketplace entry's `version:`.
|
||||
- **Lock-file discard reasoning.** `README.md`, `AGENTS.md` and ADR-0019's 2026-09-16 amendment used to say to discard the refreshed lock on a feature branch "because it records `main`'s commit, not the branch's". That was wrong: the branch's committed lock records a `main` commit too, just an older one. In this checkout it is `b7bec71`, which `git branch -r --contains` finds on `origin/main`. All three now give the real reasons. Discarding keeps unrelated lock churn out of the branch diff, and it keeps the deployed tree consistent with the lock that `apm pack --check-clean` reads. They also state the cost: the session runs the older `main` until the next session start refreshes again. The SessionStart notice in `check-apm-current.sh` now gives branch-specific advice, and `tests/test-apm-current-hook.sh` pins it. ADR-0019 had two claims that were checked against apm's source. `apm pack` "refuses to run": precisely, it raises a build error before the `--check-clean` gate is reached, and only when a file the lock lists is missing on disk (`bundle/packer.py`, `pack_bundle`). apm "removes a server on its next update": this holds, and it holds for `apm install` as well (`install/mcp/integration.py`, `MCPIntegrator.remove_stale`). The amendment now says both precisely.
|
||||
- **ADR-0022 amendment.** The amendment's placement and the validator names it cites are fixed.
|
||||
|
||||
Reference in New Issue
Block a user