docs: record findings 5, 18, 32 and the ten-finding verification wave
Why: this audit was written read-only and its scope estimates proved systematically optimistic. Ten open findings with claimed yield were re-verified against the files by independent agents. One premise of ten survived, and the headline figure was wrong in at most eight of the ten. Implementation notes: per-finding verification notes on 11, 16, 20, 22, 24, 27, 28, 33, 34 and 36. Finding 5 marked not proceeding, on the same grounds as finding 3 -- its six suites are split by failure class, not ADR section, and five of the six headers name the incident they guard. Finding 18 re-scoped and folded into finding 22 under three exemptions (audit criteria, assets/templates and sourced spec restatement, the last now carrying a decidable test rather than resting on the presence of source_keys). Finding 32 closed with its premise corrected. Section 8 questions updated where measurement answered them: ADR-0012 is moot, git/gitea granularity fails an enforced gate at 4.9x, and the external-consumer question has its evidence but awaits a decision. New section 10 records the wave, the recurring failure mode behind six wrong findings, and where the remaining opportunity actually sits. The wave's own notes were then re-verified by a second independent round, and this commit carries those corrections. The notes had an error rate comparable to the findings they corrected. Four errors changed a verdict. Finding 11's note anchored its search at column 0 and so missed every source_keys carrier nested under metadata:, producing "172 carriers" (196), "zero of 40 SKILL.md files carry source_keys" (28 of 39) and "check 2 is dead code" (live, with bats coverage); its double-counting accusation was a misreading of the word "plus" and is withdrawn. Finding 28's note claimed 2,740 lines "has never matched any commit" -- it is exact ata3e721e, the unique commit of the 67 touching docs/adr/ that yields it, and where all of the finding's headline figures reproduce simultaneously; the finding went stale, it was not fabricated. Finding 20's note argued the gitea split was blocked a fortiori by ADR-0011, which inverts that ADR's reasoning (its objection is to a boundary being crossed, not to bundle size) -- withdrawn and replaced with the same objection aimed at the correct seam, in the note and in section 8. Finding 18's "sourced spec restatement" exemption collided with finding 20's own salvage recommendation in the same commit and now carries a test that separates them. Bookkeeping corrected throughout: the dangling docs/HUMANS.md path is five occurrences across three files, not four (the sentence enumerated five while stating four); finding 16'sc8a7c9echronology was inverted, and its resolver core is 549 executable lines, not 357, making it 2.7x the proposed budget rather than 1.8x; finding 24's Q1-Q5 coverage is 20 tests and ~67%, not 24 and ~76%; finding 22's estimate is ~150-180 lines with its components summing, and its RED/GREEN rebuttal no longer depends on ignoring the two diagrams the finding most plausibly named; finding 27's preamble is 43 words; finding 28's proposal is a wash (+5 to -1) rather than a firm +5; finding 32's citation is architecture.md:22 and its net is 6 lines. Section 10's table reconciled against every corrected note. Impact: no code, gate or behaviour changes. Two defects are flagged for independent fixing -- the deployed core/instructions/governance.md cites docs/HUMANS.md, which does not exist, in five places across three files; and apm update on this branch resolves against main and would restore the obsidian MCP server removed inc96ca9c, via the regenerated repo-root .mcp.json, which is gitignored and so would not appear in git status. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
This commit is contained in:
@@ -120,6 +120,7 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
|
||||
> **Done (2026-09-12):** see commit `c8a7c9e` on `docs/simplification-audit`. Added a ~20-line required-frontmatter check (`name`, `description`, `metadata.version` as three-part semver) to `scripts/skill-size-check.sh`, reusing the YAML mapping `description_value()` already parses. Removed the inline `skill-frontmatter` hook (~80 lines) from `.pre-commit-config.yaml` and deleted `tests/test-skill-frontmatter.sh` (366 lines). Removed the 79-line "the other hook on that scope" discussion from `docs/spec/gates.md` and its now-dangling cross-reference, replacing both with a one-line note of the fold; updated the pre-push hook counts there. Updated fixture builders in `tests/test-skill-size-check.sh`, `tests/test-adr0020-body-checks.sh`, `tests/test-adr0020-targets.sh`, `tests/test-adr0020-differential.sh`, and `tests/test-vale-hooks-consumer.sh` to carry valid `metadata.version` so the new check doesn't spuriously fail existing fixtures.
|
||||
|
||||
5. **`skill-size-check.sh` has six test files totalling 3,589 lines for one 1,497-line script**, split by ADR section rather than behaviour. `test-adr0020-differential.sh` is 452 lines for 12 assertions. Merge to two files. Effort M.
|
||||
> **Not proceeding (2026-09-14):** premise doesn't hold, in the same way finding 3's did not. The six suites are **not** split by ADR section — they are split by failure class, and five of the six headers name the incident they guard. (The exception is `tests/test-skill-size-check.sh`, whose header names no incident: it describes the two gate families the script must not conflate and flags the constant-agreement block as the load-bearing part.) `test-adr0020-contract.sh` defends *structural* claims that "each one fails silently": that the resolver block copied verbatim into three scripts has not drifted, that both interpreter preflights still exist, that `verbose: true` is still set on the hook (the entire delivery mechanism for the SUGGESTION tier). It records that the `validate-provenance.sh` pair "had already drifted" once. `test-adr0020-differential.sh` compares *verdicts* between `skill-size-check.sh` and `validate.sh` on real files, and its header states that constant-agreement is "necessary but demonstrably not sufficient — a previous review found the two scripts disagreeing on real files while every constant matched perfectly", with two ceilings excluded "until a real divergence shipped behind the exclusion". The suites also do not cover the same scripts: `contract` reaches `validate-provenance.sh` (`tests/test-adr0020-contract.sh:115-116` byte-compares both copies of it). Merging by subject would delete exactly the guards against silent drift between hand-duplicated validators. Re-measured at HEAD: **3,619 lines** across six suites against a **1,517-line** script, not 3,589/1,497. That ratio is the cost of the duplication, not an independent defect — it is deleted by **finding 16**, which removes the thing being differentially compared. **#5 is downstream of #16 and should be reconsidered only after it.** The one salvageable part is a performance change, not a coverage change: `test-adr0020-differential.sh` spends 29 s of every push re-running two validators over the live corpus, and could be sped up with no coverage loss. That is a different finding than the one written here.
|
||||
|
||||
6. [x] ~~**Prose-grep tests.** `test-governance-layer.sh` and `test-instructions-and-docs.sh` (583 lines) grep markdown for phrases, including a one-shot "issue 0015 refactor incomplete" assertion made permanent and an assertion that `docs/notes/` exists. Delete both.~~ `check-apm-agents-valid.sh` (161 + 264 test lines) is a loop plus fail-closed guards around `validate.sh`; it folds into the merged audit skill's own tests (finding 14). Effort S.
|
||||
> **Done (2026-09-12):** see commit `5f9f2b3` on `docs/simplification-audit`. Deleted `tests/test-governance-layer.sh` (270 lines) and `tests/test-instructions-and-docs.sh` (313 lines); no other file referenced either. `check-apm-agents-valid.sh` was left untouched — its fate is tied to the separate, out-of-scope skill-merge finding 14.
|
||||
@@ -151,6 +152,17 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> **Done (2026-09-12):** see commit `edcc57c` on `docs/simplification-audit`. Deleted the 48 per-skill/reference READMEs plus 2 scaffold templates; dropped the README criterion from `skill-audit`'s `file-structure.md` and `finding-criteria.md` and the README-generation step from `new-skill.sh`; updated `new-skill.bats` to match. Plugin-root READMEs were kept, not part of this finding.
|
||||
|
||||
11. **Drop the provenance chain: `sources.md`, `source_keys` frontmatter, `validate-provenance.sh`.** 32 plugin and skill `sources.md` files (about 1,300 lines) plus 9 research indexes, 216 source files with `source_keys`, two copies of the validator (1,198 and 632 lines) with ten checks, and 125 bats tests exist to track which upstream informed which file. Git blame and a URL in the README do the same job. This is more code than the content it tracks. Effort M (touches skill-audit, both validator copies, two repo tests, and every skill's frontmatter).
|
||||
> **Verified (2026-09-14, at HEAD `062ca47`):** direction defensible, two scope figures wrong, and **blocked on a decision the finding never poses**. The `sources.md` census below is exact, and so are the finding's own validator and bats figures (1,198 / 632 lines, 125 bats tests); the scope errors are narrower than an earlier revision of this note claimed.
|
||||
>
|
||||
> Corrected figures: **46 `sources.md` files / 1,752 lines** in three distinct classes — 29 skill `references/sources.md` (1,217 lines), 13 research indexes (435), 4 plugin-root files (100, ADR-0010). The finding does **not** double-count: it states two disjoint classes additively ("32 plugin and skill `sources.md` files (about 1,300 lines) **plus** 9 research indexes"), and that plugin-and-skill subtotal is really **33 files / 1,317 lines**, matching its "about 1,300" exactly — had the 32 swept in the research indexes the figure would have been ~1,750. Its real errors there are an off-by-one (32 should be 33) and an omission: it missed the 4 vendored example indexes under `kyberforge/docs/research/examples/skill-write/`, so 9 should be 13. Carriers of `source_keys` in YAML frontmatter: **196** — 168 at column 0 and 28 nested two spaces under `metadata:` — so the finding's 216 is closer to the truth than it looks. (219 files merely *mention* the string. A naive `^[[:space:]]*source_keys:` grep returns 200, but 4 of those are heredoc or fixture text rather than frontmatter: both `validate-provenance.bats` copies, `scripts/check-scope-walkup-sync.sh`, and a fenced example in `plugins/bin/.apm/skills/research/references/file-format.md`.) Checks: **16 across the two copies** (skill-audit 0–9, agent-audit 0–5), not ten. Validator line counts (1,198 / 632) and 125 bats tests are exact.
|
||||
>
|
||||
> **"Touches every skill's frontmatter" is roughly right.** **28 of the 39 real skills carry `source_keys` in frontmatter**, nested under `metadata:` — see `plugins/git/.apm/skills/git-commits/SKILL.md:10-17`, where `metadata:` → `source_keys:` carries four slugs. (44 tracked files match `*SKILL.md`; subtract `skill-author/assets/templates/SKILL.md` and the 4 vendored under `kyberforge/docs/research/examples/skill-write/`, leaving 39 real skills.) The 11 without it are exactly the `plugins/bin/` skills. Check 2 in the skill-audit validator (SKILL.md `source_keys` → slug in `sources.md`) is correspondingly **live**, not dead code: `parse_source_keys()` at `plugins/kyberforge/.apm/skills/skill-audit/scripts/validate-provenance.sh:242-270` handles both spellings explicitly — the metadata-nested branch at `:257`, the top-level branch at `:260`, and a docstring that says "handles metadata.source_keys and top-level" — check 2 at `:766` runs against all 28 carrier skills, every one of which has a `references/sources.md`, and bats pins it at `validate-provenance.bats:222` ("FAIL: source_keys slug in SKILL.md not present as H2 in sources.md") and `:1313` (a BOM must not silently disable check 2). The imbalance the finding names is real and **worse** than claimed: 4,641 validator+bats lines against 1,752 of metadata, a 2.6:1 ratio.
|
||||
>
|
||||
> **Omitted entirely: the chain has a producer.** `plugins/bin/.apm/skills/research/` *specifies* the `sources.md` + `source_keys:` output format, and `plugins/bin/evals/research/research/eval.yaml` carries three criteria asserting it. **This is the blocking decision: does `research` keep emitting `sources.md`?** If yes, the chain is not dropped — only unenforced, and the finding collapses to "delete the validators." If no, the research skill's output contract and its evals need redesigning.
|
||||
>
|
||||
> Also breaks: `check-scope-walkup-sync` loses one of four walk-up ports (the hook exists because three scripts drifted); `tests/test-adr0020-contract.sh` loses its parser byte-identity assertion; `tests/test-check-scope-walkup-sync.sh` must re-base its fixture; ADR-0010 is superseded outright and ADR-0009/0016 need amending (`field-inventory.md`'s allowlist data line carries `source_keys`). `LESSONS.md:73` records this validator as the **only** thing that catches a skill authored outside `skill-author` — a failure that "recurred twice in one session" — so "git blame + a README URL do the same job" is false for the one thing the chain demonstrably catches. Side effect: 55 reference files have frontmatter containing *only* `source_keys:`, leaving empty `---\n---` blocks to delete.
|
||||
>
|
||||
> **Effort L, not M** (~6,393 lines deleted across 242 files: the 4,641 validator and bats lines plus the 1,752 of `sources.md` measured above, across 196 `source_keys` carriers and 46 `sources.md` files. An earlier revision of this note said ~4,600 lines across ~230 files, which was internally inconsistent — 4,600 is validator-plus-bats only and silently drops the `sources.md` this same note measures, and ~230 inherited a carrier count of 172 that missed every `metadata:`-nested file.) Smaller alternative worth considering: scope the drop to the skill half only (1,217 lines, 1,198-line validator, 82 tests) and leave the ADR-0010 plugin-root half alone — no ADR supersession needed.
|
||||
|
||||
12. [x] ~~**Strip ADR and changelog narration from model-facing files.** `ADR-0020` is cited in 3 of 7 kyberforge SKILL.md files and 16 references; ADR-0023 is cited inline 21 times in the git plugin. Examples: "was the old house rule and ADR-0020 deleted it", "were removed per ADR-0015 once issue #90 landed", "this file previously recorded `list_issues` as having neither a `type` nor a `milestones` parameter". `skill-author/references/retrofit.md` (197 lines) is a one-time migration guide; it is loaded from `improve.md` and listed in `sources.md`, so remove those in the same change. These belong in git history or the ADR, not in context. Effort S.~~
|
||||
> **Done (2026-09-12):** see commit `edcc57c` on `docs/simplification-audit`. Historical narration stripped from kyberforge (ADR-0020) and git (ADR-0023) skill content; `retrofit.md` deleted along with its load-step and `sources.md` entries. Caught in review: some `ADR-0023` tags were not narration but the `check-rtk-prefix` hook's required opt-out marker for intentionally-bare git commands — those 12 were restored, not left stripped.
|
||||
@@ -165,10 +177,39 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
15. **Merge `skill-author` + `agent-author` likewise.** `contract.md` shares most of its Description section; `new-skill.sh` and `new-agent.sh` implement the same package-root walk-up with different mode names; step 1 dispatch tables and step 3 gates are near-identical. Keep the agent scope logic (plugin vs project/user) as its own reference. Effort M.
|
||||
|
||||
16. **Cut the validators by an order of magnitude.** `validate.sh` is 1,677 lines of bash with embedded Python, ported twice; `skill-size-check.sh` is 1,497. Target about 200 lines total: frontmatter present, size ceilings, boundary targets resolve. The 526-line `vale-wrap.sh` exists to work around folded `>` scalars in descriptions; writing descriptions as `|` literal blocks removes the folding problem, but the wrapper is also the exported hook entry in `.pre-commit-hooks.yaml` and carries the NOT RUN guard the audits depend on, so it shrinks rather than disappears. This is where the real complexity lives and is the item most worth discussing. Effort L.
|
||||
> **Refuted (2026-09-14, at HEAD `062ca47`). Finding 16 has no independent content — its only safe saving belongs to finding 14.**
|
||||
>
|
||||
> The three validators are **not three implementations**. They contain **one block, 1,061 lines, byte-identical in all three**, delimited by `# ===== BEGIN/END ADR-0020 SHARED BOUNDARY RESOLVER =====` and hashed by `tests/test-adr0020-contract.sh`. So 3,183 of 4,932 validator lines (65%) are that block × 3, and **what is left once the resolver is excised is 1,749 lines across all three** — 1,580 non-blank, 992 with comments and blanks both stripped. The duplication is forced by the self-containment constraint, which is why *merging* is the lever and *shrinking* is not.
|
||||
>
|
||||
> Corrected figures: `skill-size-check.sh` is **1,517**. The finding's 1,497 was correct when written — `git show 9eb8bc7:scripts/skill-size-check.sh` is 1,497 lines, and `9eb8bc7` (2026-09-10) is this audit's own first commit. It went stale two days *after*, at `c8a7c9e` (2026-09-12), the commit that folded `skill-frontmatter` in — which is why the finding's "frontmatter present" target is now work already done, not why its number was wrong. agent-audit's `validate.sh` is **1,738**, a superset, not a 1,677-line port. `vale-wrap.sh` 526 × 2 is exact.
|
||||
>
|
||||
> **The 200-line target is off by an order of magnitude.** The resolver's executable core alone, every comment and blank stripped, is **549 lines** (of its 1,061: 411 comment, 101 blank) — **2.7×** the whole proposed budget, before any of the three scripts' own diagnostics. Counting call sites to `error`/`fail`, `suggest` and `info`, those number 23 in `skill-size-check.sh`, 31 in skill-audit's `validate.sh` and 38 in agent-audit's. ADR-0020 mandates **10** deterministic gates — its Enforcement table has 15 content rows: 10 `deterministic`, 1 `prose pattern`, 4 `auditor judgment` — and explicitly warns against "a rule filed under 'Enforcement' that no validator implements". **5,506 lines of tests** guard these scripts: the six repo suites over `skill-size-check.sh` (3,619) plus the two in-skill `validate.bats` (965 + 922). Realistic floor with prose intact and duplication removed: **~2,600–2,900**; comment-stripped and fully merged, ~1,100–1,200.
|
||||
>
|
||||
> **The comment volume is an incident log, not filler** — 26 verbatim failing input strings and four filed issues (#99, #107, #108, #110). Samples: deriving the skill universe from `$BASH_SOURCE` "leaked holocron's 39-skill universe into every consumer repo"; a worktree named `feature[2]` turned a glob into a character class and "silently disabled the gate tree-wide"; `description:` followed by `model: sonnet` "exited 0 with zero output on a BLOCKING pre-push gate"; a 550-char description with a 1,000-word body "exited 0 behind a BOM". Exactly one clause argues for speculative retention and flags itself as unexercised. Same pattern as findings 3, 5 and 24.
|
||||
>
|
||||
> **The `vale-wrap` half is wrong on its conclusion.** `|` literal blocks do fix the folding case — the script says so and deliberately no-ops on them — but the wrapper handles **four** affected scalar forms (folded `>`, bare plain, double- and single-quoted continuation lines), and **277 of its 526 lines are argv handling unrelated to folding** (cwd-relative absolutization, the `is_builtin_output` guard, scratch-tree mirroring, path relativization), each with its own incident record. Decisively, `.pre-commit-hooks.yaml` exports these hooks to external consumer repos whose scalar style this repo cannot dictate. Converting the 40 in-repo descriptions to `|` is a fine independent change; **it does not shrink the wrapper.**
|
||||
>
|
||||
> Where the savings actually are: **merge skill-audit + agent-audit (finding 14) → −1,587 lines, zero coverage loss.** A second option — sourcing the resolver into `scripts/skill-size-check.sh` rather than embedding it (−1,061) — is technically possible but couples the root hook to plugin layout and dismantles the byte-identity contract test's design; needs a decision, not an assumption.
|
||||
|
||||
17. **Fold `forge` and `apm-install`.** `forge` is a four-row routing table plus 207 lines of references explaining fork vs inline; it should be 25 lines with no references. `apm-install` (53 lines + 17-line sources) becomes a sixth dispatch row in `apm-workflow`. Effort S.
|
||||
|
||||
18. **Delete prose the model already knows.** "Valid characters: lowercase letters, numbers, hyphens"; what pipx does and PEP 668; "code blocks carry a language tag"; "data to stdout, diagnostics to stderr". Ironically `body-discipline.md` instructs auditors not to include "concepts the agent already knows". Effort S.
|
||||
> **Re-scoped and folded into finding 22 (2026-09-14).** All four named examples were located, and they are four different classes of thing — only one is what the finding describes:
|
||||
>
|
||||
> | Example | Location | What it actually is |
|
||||
> |---|---|---|
|
||||
> | "Code blocks carry a language tag" | `skill-audit/references/formatting-and-scripts.md:19` | An **audit criterion** |
|
||||
> | "Data to stdout, diagnostics to stderr" | same file, line 39 | An **audit criterion** |
|
||||
> | "Valid characters: lowercase letters…" | `skill-author/assets/templates/SKILL.md:7` | A **scaffold comment** emitted into new skills by `new-skill.sh`; never loaded as model context |
|
||||
> | pipx / PEP 668 | `apm-install/SKILL.md:35-36`, `skill-author/references/scripts.md` | Generic tool prose — the only true positive |
|
||||
>
|
||||
> Three exemptions agreed, which is what re-scopes the finding:
|
||||
>
|
||||
> - **Audit criteria are exempt.** `body-discipline.md:14` frames the rule as "Would the agent get this wrong without this instruction?" — an auditor *would*, because the criterion is what it reports against. Cutting criteria is a redesign of what `skill-audit` checks, which belongs with finding 14.
|
||||
> - **`assets/templates/` is exempt.** Scaffold output, not context.
|
||||
> - **Sourced restatement of a spec this repo's own artifacts are built to is exempt.** `skill-author/references/scripts.md` carries `source_keys: agentskills-using-scripts` and deliberately restates the agentskills.io spec — the contract every skill here is written against, so the restatement governs this repo's artifacts and has to be in front of the author. **`source_keys:` alone is not the test**, and cannot be: `conventional-commits-spec.md` and `bisect.md` both carry it too, and finding 20 recommends reducing both to a pointer plus the house delta. The decidable line is what the content governs — a spec this repo's artifacts must satisfy (agentskills.io) is exempt; documentation of an external tool the model already has (Conventional Commits, `git bisect`) is not. Grounding, stated honestly: findings 9 and 26 closed as "Keep — vendored upstream content is intentional", but both closed over the `docs/research/` and `docs/notes/` *directories*, not over skill `references/*.md`; extending them to `scripts.md` is this note's inference, not a recorded decision. (An earlier revision added "finding 11 re-decides this content's status anyway" — withdrawn: finding 11 proposes dropping the provenance *metadata and validators*, not the sourced prose.)
|
||||
>
|
||||
> What remains is unsourced explanatory prose in skill bodies and non-criteria references — roughly **30–60 lines across kyberforge**, where `apm-install/SKILL.md` yields about one clause. Too small to stand alone, and the same class of writing as finding 22 with a larger surface and no sourced-content conflict. **Merged into finding 22 under these exemptions; not a separate work item.** Safety note established while scoping: `validate-provenance.sh` is not a pre-push gate (the only `.pre-commit-config.yaml` reference is `check-scope-walkup-sync`, over the walk-up port) and validates `sources.md` structure, never line-level traceability — so trimming sourced prose trips no gate provided frontmatter and `sources.md` are left intact. **Loose end in the fold, stated so it is not lost:** finding 22's total is computed over five `bin` skills (1,018 lines) and its implementation sizing names two agents, neither touching kyberforge — so these 30–60 lines sit outside the scope finding 22 states. Track them there as a separate line item with its own estimate; they are not covered by "bin: strip generic process theatre" as written.
|
||||
|
||||
### 4.3 git and gitea (153 + 93 files, 9,889 + 6,047 lines incl. mirror; source 3,288 + 2,286)
|
||||
|
||||
@@ -176,6 +217,21 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> **Not proceeding (2026-09-13):** premise doesn't hold. There are no separate "router skills" — only two `.agent.md` files. `git-orchestrate` is not a dangling boundary-clause reference; it's `git-workflow` step 5's actual execution backend (documented both directions), so deleting it breaks `git-workflow`'s only execution path rather than tidying an orphan. `gitea-orchestrate` is intentional per ADR-0011 (agent-facing counterpart for agent callers) even though `gitea-workflow` doesn't call it. A third, undocumented instance of the same pattern (`apm-orchestrate`) exists and isn't addressed by this finding. The four boundary-clause locations named above don't actually reference either agent. No changes made. This needs the "short discussion" §7 bucket 2 implies, not a mechanical delete.
|
||||
|
||||
20. **Collapse git 7 skills to 1; gitea 7 to 2.** Git references are man-page restatement: `git-log-format.md` (242 lines listing `%H`, `%ar`), `conventional-commits-spec.md` (170 lines), `worktrees.md` (178), `merging.md` explaining fast-forward. Roughly 60% of the plugin is generic. The genuinely house-specific content fits in about 150 lines: the `rtk` rule and ADR-0023 exceptions, main/master refusal, `--no-verify`, the `-i --autosquash` 2.39.5 trap, `--force-with-lease --force-if-includes`, bisect exit codes, submodule push ordering, the detached-HEAD worktree trap. Gitea is more legitimately specific (MCP schema quirks: `tree_sha`, `withLines`, silent drops on PR create, `per_page` 20 vs 30, 404 means 403) and splits naturally into `gitea-tracker` (issues, PRs, labels, milestones) and `gitea-repo` (branches, files, releases). Risk: one description must carry all trigger phrases; keep a dispatch table at the top of the body. Keep `pc-author` and `pc-run` (finding 38). Effort M.
|
||||
> **Refuted as specified (2026-09-14, at HEAD `062ca47`). The routing concern is not a risk to mitigate — it is a blocking gate failure.**
|
||||
>
|
||||
> What holds: skill counts (git 7 `git-*` + `pc-*`, gitea 7); the four named git reference files at their stated sizes (`git-log-format.md` 242, `conventional-commits-spec.md` 170, `worktrees.md` 178; `merging.md` is 31, among the smallest). "Roughly 60% generic" holds at the top of its range — two independent methods give **54–60%**. Gitea being "more legitimately specific" holds and is **understated**: gitea is ~**72% house-specific**, the inverse of git, with ~50 MCP quirks beyond the five named (no `method:"close"` on `issue_write`; `draft:true` is literally a `"WIP:"` title prefix; **no update tool for releases exists at all**; `replace_labels` clears unlisted labels; org-label methods take `org` not `owner`). Note commit `6cfc357` (finding 13) touched none of the four named files, so its trim does not deflate this evidence.
|
||||
>
|
||||
> **The 150-line target fails on the finding's own arithmetic.** 60% generic of 1,891 non-`sources.md` lines leaves **756** house-specific; an independent full read puts the floor at **~918**. Off by 5–6×. The house-specific list is also not exhaustive — it misses the rest of the ADR-0023 bare-`git` exceptions (**~23 sites across nine files**, which collapse to about **ten distinct documented reasons**, not one per site: the four `git log -L` lines share one reason, the four `--word-diff` lines another, the two `git diff --name-*` lines a third — the genuinely distinct ones include `git branch --list`'s phantom `* ` line, `stash pop` swallowing the conflict report, `stash list` printing `No stashes` where git prints nothing, and the inner `$(git config remote.origin.url)` substitution, where output rewriting would poison a remote URL), a second version trap (`worktree add --orphan` needs 2.42+, exits 129 on 2.39.5), `commit-template.md` (66 lines, wholly house), **three** specified JSON result shapes across the six skills that carry an output section (`git-commits/SKILL.md:54`, `git-remotes:48`, `git-branches:61`) plus the request schema in `git-branches/references/orchestrator-contract.md`, and every cross-skill dispatch clause. Conversely one item **on** the list is misfiled: **bisect exit codes restate `git bisect run`'s own docs** (`bisect.md` is ~97% generic).
|
||||
>
|
||||
> **Git 7→1 is uncommittable.** Measured against `skill-size-check`'s FAIL tiers: description **1,950 chars = 4.9× the 400 ceiling**, body **3,381 words = 3.8× the 900 ceiling**, and the plugin is already at 492 of 500 whole-file lines. The finding's own mitigation — "keep a dispatch table at the top of the body" — *adds* body words to a budget already 3.8× over, and ADR-0020 makes stating the same trigger in two registers a FAIL in its own right. The smallest existing git description is 214 chars for **one** domain.
|
||||
>
|
||||
> **Gitea 7→2 fails the same gate and puts a seam through the commonest workflow.** Both halves FAIL the description ceiling: `gitea-tracker` (issues + PRs + labels/milestones) sums to **991 chars**, `gitea-repo` (branches + files + releases) to **989** — 2.5× the 400 ceiling, measured as `skill-size-check`'s `description_value()` measures it (YAML-folded, whitespace-collapsed). The proposal also silently drops `gitea-workflow`, a seventh skill it never places, so "7→2" is really 7→2-plus-a-deletion. `gitea-repo` would carry 20 of 32 MCP tools across three unrelated families. And the structural objection: the split puts a hard cross-skill boundary through the commonest real workflow — edit-a-file-then-open-a-PR lands `gitea-files` and `gitea-prs` on opposite sides of the `tracker`/`repo` seam, forcing the same duplicate-or-reach-across choice ADR-0011 rejected the 5-skill option for, at a different seam. **What this is not: blocked by ADR-0011's reasoning.** An earlier revision argued that ADR-0011 rejected a 5-skill split and `gitea-tracker` "bundles strictly more", so it is rejected a fortiori — withdrawn, the premise is false. ADR-0011's stated reason is that bundling labels under `gitea-issues` "forc[es] `gitea-prs` to either duplicate the guide or reach into `gitea-issues`' `references/` — breaking the self-contained skill boundary": an objection to a *boundary being crossed*, not to bundle size. `gitea-tracker` puts issues, PRs, labels and milestones in one skill, so there is no boundary to cross and no guide to duplicate. What remains of ADR-0011 here is procedural: reversing the recorded 7-skill split needs a superseding ADR. Merging concatenates; it does not compress.
|
||||
>
|
||||
> **Interaction with rejected finding 19, unacknowledged:** collapsing git to one skill absorbs `git-workflow` itself — one of the seven — so its 8-row Domains table would route to itself, and leaves `git-orchestrate` a dispatcher with exactly one target, its contract vacuous. Finding 19 was rejected for doing less than this, mechanically.
|
||||
>
|
||||
> Blast radius if ever revisited: **68 backticked references to git skill names, 99 to gitea names** under `plugins/`. Only the ones in a `SKILL.md` are boundary targets `skill-size-check` resolves and FAILs on if dangling — its `files:` regex is `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$`, so it opens nothing else — and that is **35 of the 68 git mentions and 26 of the 99 gitea ones**. The remaining 33 and 73 live in `references/*.md`, the two orchestrate agents (10 and 23 on their own), the plugin READMEs, kyberforge's audit and author references, and two `validate.sh` copies — none of which this gate opens: they still have to be rewritten by hand, but they fail no hook. Plus `AGENTS.md:16,18`, `README.md:21-22`, `CONTEXT.md:168` (uses `gitea-prs` as the naming exemplar), `architecture.md:34` (uses `git-branches` vs `gitea-branches` as the canonical boundary example), and ADRs 0011, 0020, 0021, 0022, 0023. Two false alarms not worth chasing: `tests/test-check-rtk-prefix.sh:97` reads from a pinned historical SHA, and `scripts/check-rtk-prefix.sh`'s mention is in a comment.
|
||||
>
|
||||
> **Salvageable independently, ~230 lines:** `conventional-commits-spec.md` (~98% generic) and `bisect.md` (~97%) are the only two files where the generic-restatement thesis fully holds — reduce each to a pointer plus the house delta. Also worth a finding-13-style trim-in-place: the issue-vs-PR disambiguation duplicated across 4 gitea files. Neither needs a merge.
|
||||
|
||||
21. [x] ~~**Delete `config.example.json` / `.claude/plugins/git/config.json`.** Read by two steps, written by nothing. Default to GitHub Flow with the existing `develop` / `release/*` inference. Effort S.~~
|
||||
> **Done (2026-09-12):** see commit `f5e4d0d`. Deleted `plugins/git/config.example.json` (the runtime `.claude/plugins/git/config.json` was never a tracked file). Removed the config-read step from `git-orchestrate`'s Process and from `git-branches`' Step 1, leaving the existing default-inference logic (GitHub Flow, with Gitflow inferred from a `develop`/`release/*` branch) as the sole path; updated `git-workflow`'s description of the orchestrator's behaviour to match. Dropped the now-dangling `applied_config` field from `git-orchestrate`'s output shape and the `config.example.json` example from `docs/spec/architecture.md`.
|
||||
@@ -183,10 +239,34 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
### 4.4 bin, core, lint (88 + 49 + 31 files incl. mirror)
|
||||
|
||||
22. **bin: strip generic process theatre.** `write-docs` is 109 lines, mostly form-filling sections plus a 15-line source provenance block; its rules fit in 25 lines. `tdd` is about 70% textbook (RED/GREEN diagram, "good tests are integration-style", five thin references restating textbook design advice). `diagnose` 40%, `prototype` 50% (pixel-level UI switcher spec), `grill-with-docs` 35%. Keep the opinionated parts: "no horizontal slicing", "no phase 2 without a loop", `[DEBUG-xxxx]` tags, "never infer the output path", the triage state machine. Effort M.
|
||||
> **Verified (2026-09-14, at HEAD `062ca47`): percentages inflated 3–4×, target set wrong, but a real and better defect found. This finding also now carries finding 18's re-scoped remainder** (unsourced explanatory prose only — audit criteria, `assets/templates/` and sourced spec restatement are exempt; see finding 18).
|
||||
>
|
||||
> Corrected percentages, by a stated method (a line counts generic only if it states a general SE principle with no repo-specific term, no named house convention, and would survive unchanged in any textbook): `diagnose` **~10–15%**, not 40%. `prototype` **~13%**, not 50% — and its `?variant=` switcher spec is a prescriptive house convention (floating bar, arrow keys, `NODE_ENV` gating), not theatre. `grill-with-docs` **~10%**, not 35%. `tdd` **~25%** in the body, ~53% only at directory scope, not 70%. On the "RED/GREEN diagram" the finding names for deletion: `tdd/SKILL.md` has three diagrams, so the name is ambiguous. The horizontal-vs-vertical block (`:36-46`, whose own rows are labelled `RED:`/`GREEN:`) is the clearest statement of the skill's central opinion and should stay; the two literal `RED:`/`GREEN:` loop blocks at `:71-74` and `:82-85` are what the finding most plausibly means, and those two are textbook and go — eight lines with their fences. So the rebuttal stands only for the horizontal-vs-vertical block; it is an eight-line cut either way, not a case against the skill's one original diagram. `write-docs` is exactly 109 lines ✓, but "its rules fit in 25 lines" is wrong — the Process section alone is 17 lines of real content; floor is ~55–60.
|
||||
>
|
||||
> **Two of five keep-list items name skills this finding never targets** — "never infer the output path" is `research/SKILL.md:25`, the triage state machine is in `triage`. And **the target set does not match the repo's own over-budget list**: `skill-size-check` flags six `bin` skills; this finding names two that pass clean (`prototype`, `grill-with-docs`) and misses three that are over — `improve-codebase-architecture` (730w + 316-char desc), `research` (703w), `triage` (712w).
|
||||
>
|
||||
> **The real defect, which "form-filling sections" understates: `write-docs` is 54% restatement.** 431 of the **801** body words the gate counts (`skill-size-check` reports 801; the 775 an earlier revision used omits the heading words the gate includes). Constraints (126w), When-to-use (110w), Failure handling (104w) and Self-check (91w) each restate the Process section or the description. Measured claim by claim, the repetition is wider than "four claims four times": the Reader-Testing scoping rule appears **five** times (lines 63/80/88/96/107); file approval before reading **four** (58/68/93/103); the delta summary **four** (62/78/87/106); stage-skip logging **four** (59/72/94/104); "every claim traceable, never invent behaviour" **four** (34/57/101/102); show-the-full-section-before-gating **three** (60/76/105). `contract.md:119` explicitly forbids this: *"Exclude: Restatements of the description — it is already in context."* It is also the corpus's only outlier frontmatter — the sole `SKILL.md` of the **39** real skills carrying `when:`, `updated:` or `source:` (44 files are named `SKILL.md` under `plugins/`; 1 is a template under `skill-author/assets/templates/` and 4 are vendored under `kyberforge/docs/research/examples/skill-write/`, the same 39 `architecture.md:82` states).
|
||||
>
|
||||
> **Realistic total ~150–180 lines of 1,018 (15–18%)**, itemised so it adds up: `tdd`'s three textbook references **74 lines** (`refactoring.md` 10 — a Fowler smell list; `deep-modules.md` 33 — self-declared *"From 'A Philosophy of Software Design'"*; `interface-design.md` 31 — generic DI advice), where `improve-codebase-architecture` already carries richer house-specific treatments of both concepts; `write-docs` 109 → ~58, so **~51**; `tdd`'s body at ~25% generic, **~25**; `diagnose` **~15**. That sums to ~165. The three textbook references are **~45%** of the cut, not the ~75% an earlier revision claimed — 74 of ~165. `prototype` and `grill-with-docs` contribute nothing, per the scope recommendation below, and finding 18's folded kyberforge remainder (30–60 lines) sits outside this 1,018-line denominator.
|
||||
>
|
||||
> **Coupling — the important caution.** This corpus has already been through two trim passes, and the last one broke two of these five targets the same way. PR #129 (`598a7c3`) records: *"`prototype` and `vale-config` deleted rules outright that survived nowhere."* This finding proposes redoing that operation on `prototype` and `diagnose`. **Recommend dropping `prototype` and `grill-with-docs` from scope entirely** — both pass every gate and both have prior-regression history. Also: deleting a `references/*.md` named in a body is a hard ERROR (`skill-size-check.sh:1265`), so `tdd`'s reference deletions and its SKILL.md relinks must land in one commit; `improve-codebase-architecture/SKILL.md:77,79` hard-name `grill-with-docs`'s `context-format.md` and `adr-format.md` by path, so neither can be renamed; and shrinking `write-docs` falsifies live comments at `skill-size-check.sh:64,596`, `tests/test-adr0020-targets.sh:592` and `architecture.md:82`. Unlike finding 23's `caveman`/`zoom-out`, **none of these five is cited as a convention exemplar** anywhere.
|
||||
>
|
||||
> Sizing if implemented: two parallelizable agents over disjoint files — A on `write-docs` (self-contained, no references), B on `tdd` (reference deletion + same-commit relink, ERROR-gated, cannot be split). `diagnose` is ~15 lines, too small for its own agent.
|
||||
|
||||
23. **bin: merge `grill-me` into `grill-with-docs`.** `grill-me` is 16 lines and a subset of the docs flow; `grill-with-docs` creates `CONTEXT.md` when missing, so the merged skill needs a no-write opt-out. `caveman` (50 lines) and `zoom-out` (9) are hand-invoked prompts rather than workflow skills; they are also the repo's `disable-model-invocation` exemplars in `CONTEXT.md`, `contract.md`, ADR-0020, ADR-0021, and `gates.md`, and `install.sh` has no path for `~/.claude/commands/`, so moving them means picking a new exemplar. `improve-codebase-architecture` defines its glossary twice (inline and in `language.md`; the README documents the split as intentional). Effort S.
|
||||
|
||||
24. **core: `provider-adapter-author` is a 1,200-line wrapper around one instruction** ("replace duplicated lines with `@AGENTS.md`, keep provider-specific lines"): a 496-line validator with a 519-line bats suite for a check that is a grep. `agentsmd-author` already calls `agentsmd-audit` as mandatory closeout, and both route to `provider-adapter-author` in boundary clauses that must change with it. Target: one `agentsmd` skill with an audit mode, adapter conversion as a step, validator about 40 lines. Needs an ADR-0012 revisit. Effort L.
|
||||
> **Refuted (2026-09-14, at HEAD `062ca47`). Not deferred — the target fails the repo's own gate before any judgment call is reached, so the ADR-0012 §8 question is moot for this finding.**
|
||||
>
|
||||
> **The merge is arithmetically impossible as specified.** Body word counts: `agentsmd-author` 485 + `agentsmd-audit` 361 + `provider-adapter-author` 514 = **1,360 words against `BODY_MAX_WORDS=900`** (ADR-0020 hard FAIL). Descriptions: 251 + 275 + 280 = **806 chars into a field capped at 400**. Relocating the overflow into `references/` is PR #129's named anti-goal, and issue #117 records that `references/` is where neither the size gate nor Vale looks.
|
||||
>
|
||||
> **Both factual anchors describe a validator that no longer exists.** `scripts/validate-adapter.sh` was **141 lines at birth** (`6fd6876`) and in that form *was* approximately a grep — which is why it shipped two recorded defects: `a8cd5e8` (a UTF-8 BOM hid the import line, so a `CLAUDE.md` whose first line was `@AGENTS.md` failed with "no reference to AGENTS.md" and was told to add the line already in front of it) and issue **#115** (`c59e4bf`: the `--no-import-syntax` flag was a proven no-op — "both branches reduce to the same expression"). 141 → 496 is the fix for those. **"Validator about 40 lines" targets below the version whose defects are on the record.** Line counts otherwise exact: validator 496, bats 519 — but "1,200-line wrapper" is 1,164, of which the *wrapper* is 52; **1,015 are validator + tests** (1,071 with the two READMEs, 28 each), and the remaining 41 are `references/`.
|
||||
>
|
||||
> Coverage given up by a 40-line validator: **~67% of the 42-test suite**. **20** tests sit under explicit `Q1`–`Q5` headers — Q1 inert fenced/indented/HTML-comment regions (5), Q2 valid-UTF-8-but-not-UTF-8 encodings (4), Q3 exists-but-unreadable (1), Q4 path resolution (4), Q5 pointer-vs-mention (6); 8 more are hardening, so 28 of 42. Every one was proven non-vacuous by deliberate mutation under PR #129. Representative guards: a ```-fenced `@AGENTS.md` "exited 0"; `@NOTAGENTS.md` counted as an import for want of a path-segment boundary; `"AGENTS.md" in ln` passed *"Do NOT read AGENTS.md; it is obsolete."*; BOM-less UTF-16LE decodes as valid UTF-8 and produced a false diagnosis; exit 2/3 split from 1 "because the skill's closeout tells the agent to fix every non-zero exit by editing the provider file, which for a mistyped flag edits the wrong file forever".
|
||||
>
|
||||
> **The self-containment constraint does not support this finding the way it supports 14/15** — there is no cross-skill duplication here to merge away. `agentsmd-audit`'s three scripts share essentially nothing with `validate-adapter.sh` (no `read_text`, no BOM handling, no NUL check; they exit 1 on usage errors). Merging would *expose* that they are unhardened — costing lines, not saving them.
|
||||
>
|
||||
> Two further blockers if it were ever revisited: the merge dissolves `agentsmd-author`'s standing prohibition *"Never write to a provider file yourself, in any circumstance"* (SKILL.md:21), a hazard `c59e4bf` closed after the validator's own size-FAIL remediation text "actively invited the prohibited edit"; and `skill-size-check.sh:121` + `tests/test-skill-size-check.sh:729` both cite `a8cd5e8`'s exit-2 split as precedent for their own, so deleting it orphans two live cross-references.
|
||||
|
||||
25. **lint: delete the `lint-runner` agent.** Its body is "call `vale-run`, reformat output", which `--output=JSON` already gives; it exists for backends that do not exist. It is the example boundary clause in three `agent-author` templates and ADR-0016, so those need a new example. About 40% of `vale-config` is install tables and settings lists the model can fetch from vale.sh. Keep the house-verified matrices (`E100`/`E201`, `Packages` below glob, frontmatter, ignore paths). `lint/docs/research/docs/vale/` overlaps the skill's own references by about two thirds. Effort S.
|
||||
|
||||
@@ -196,11 +276,44 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> **Decision (2026-09-12):** Keep. Same reasoning as finding 9 — these docs are intentional context for sourced work. Not proceeding.
|
||||
|
||||
27. **Four governance documents say one thing.** `core/instructions/governance.md` (949 words, always-on), `docs/ai-constitution.md` (2,906), `docs/wiki/HUMANS.md` (1,413), `CONTROLS.md` (1,224), with near-identical preambles and, in three of the four, a "what this file does not govern" block pointing at the others. The constitution repeats one of its own principle lead sentences. Keep `governance.md` as the operative file, trimmed to about 50 lines (drop the classification table that repeats the bullets above it, the footer, the non-governance block). Dedupe the constitution by about 20%. Effort M.
|
||||
> **Refuted as framed (2026-09-14, at HEAD `062ca47`). All four word counts are exact — the first finding in this audit whose figures survive checking — and everything built on them fails.**
|
||||
>
|
||||
> **"Four documents say one thing" misreads audience separation as duplication.** They are one principle set projected onto four execution surfaces, and each projection is load-bearing: `governance.md` is imperative *to the model* and injected into every session; `HUMANS.md` is imperative *to a person* on a wiki; `CONTROLS.md` is a declarative spec *for CI tooling*; the constitution is the justification layer with citations. Take "secrets never enter AI context": the constitution states it with evidence, `governance.md` tells the model never to emit one, `HUMANS.md` tells the person never to paste one, `CONTROLS.md` specifies the pre-commit hook that catches both when the first two fail. `CONTROLS.md:8` names this explicitly — *"Agent instructions and human practitioner rules are probabilistic… A control that runs automatically in CI enforces a principle more reliably than any instruction in any file."* The three "what this file does not govern" blocks are the seams that keep the four from bleeding together, each pointing at a different file for a different reason. Real overlap is ~15%.
|
||||
>
|
||||
> **22% of the finding's word count is not this repo's to edit.** `docs/wiki` is a submodule pointing at a separate Gitea *wiki* repo, concurrently editable through the web UI. `HUMANS.md`'s 1,413 words are out of scope for any change made here.
|
||||
>
|
||||
> **"Drop the classification table" would delete live rules.** The table has four rows; only Confidential and Restricted restate the bullets above it. **Public and Internal exist nowhere else in the file** — dropping it removes the only statement of `Internal | Operational data, anonymised logs | Enterprise AI tools only; not consumer/free-tier` from always-on agent context, in every project.
|
||||
>
|
||||
> **The 50-line target is arithmetically unreachable and contradicts the finding's own keep-list.** The file is 82 lines; the three named cuts total **16 lines** counting only their own content — the table rows (31–36, 6), the non-governance block (71–76, 6), the footer (79–82, 4) — landing at 66, or at most **23** if each cut also takes its heading, surrounding blanks and the `---` rules, landing at 59. Both include the table cut that shouldn't happen. Reaching 50 means cutting ~9–16 more from Hard Prohibitions (18 lines) or Required Behaviours (24), the operative rules the finding says to keep. **"Dedupe the constitution by about 20%" overstates by 4×** — verifiable duplication is **133 words (4.6%)**, in two adjacent principle pairs (§5 lines 114/117 byte-identical; §4 lines 89/92), both merge artifacts. §§1–10 are ten distinct domains with near-zero cross-section.
|
||||
>
|
||||
> **Any cut to `governance.md` is a global agent-behaviour change, not a docs edit.** Verified chain: `scripts/deploy-manifest.sh:21` maps `core:.claude/core`, and both `providers/claude-code/CLAUDE.md` and the live `~/.claude/CLAUDE.md` carry `@~/.claude/core/instructions/governance.md`. Repo and deployed copies are byte-identical (6,590 bytes). All 949 words are injected into every session in every project. Needs explicit sign-off on that basis.
|
||||
>
|
||||
> **Honest ceiling: 168 words / 17.7%** of the file's 949 — the cross-reference scaffolding only: preamble **43** (lines 3–5), non-governance block **77** (71–76), footer **48** (79–82), landing at ~67 lines with no rule loss. (An earlier revision said 47 for the preamble, which is only reachable by counting lines 1–7 — that sweeps in the `#` glyph and a `---` rule as words.) Plus 133 words from the constitution. Not 50 lines, not 20%.
|
||||
>
|
||||
> **Two defects the finding missed, both worth fixing independently of it.** (1) **A live bug: `docs/HUMANS.md` does not exist** — the file is `docs/wiki/HUMANS.md`. The wrong path appears **five times across three files**, including the **deployed** `core/instructions/governance.md:82`, which is self-inconsistent (line 73 correct, line 82 broken); the other four are `CONTROLS.md:5,101,106` and `ai-constitution.md:238`. (An earlier revision said "four times" while enumerating all five.) (2) The deployed always-on file carries **repo-relative pointers that dangle in every project but this one** — an agent told to "read it when making decisions not covered here" cannot. That is the substantive question this finding should have asked. The footer is additionally self-referential: `governance.md:80` lists the file as compatible with itself.
|
||||
|
||||
28. **ADRs: 2,740 lines, 72% in eight ADRs over 150 lines.** ADR-0020 is 513 lines with a 71-line measurement log as Context; ADR-0017 has 173 lines of amendments against 45 of decision. ADR-0001 is superseded and ADR-0006 moot, both keeping full text below the banner. ADR-0002 is three lines. Truncate superseded ones to the banner, fold amendments into the decision, cap Context at 20 lines, add a 25-line `docs/adr/README.md` index with status. The rules already live in `gates.md`; the ADRs need only decision and consequences. Effort M.
|
||||
> **Moved backwards (measured 2026-09-14 over `afa7187^`..`a6434e0`):** today's ADR-0024 work did the opposite of this finding on every axis, and that is recorded here so it is a known trade rather than a surprise. `docs/adr/` went from **23 files / 2,748 lines** to **24 / 3,084** — one new ADR (0024, 259 lines) plus amendment and banner text across **eleven existing ADRs** (0001, 0006, 0011, 0013, 0014, 0015, 0017, 0018, 0019, 0020, 0021 — 87 lines added, 10 removed, net **+77**), for a total of net **+336 lines (+12%)**. The two ADRs this finding names for truncation both grew *below* their banners instead: **ADR-0001 26 → 27** lines and **ADR-0006 22 → 27**, each gaining a fresh "as of ADR-0024" paragraph rather than losing the historical body beneath it. ADR-0017 gained a supersession banner while keeping its four amendments in full — the exact shape this finding proposes to fold.
|
||||
>
|
||||
> Not a defect in that work: a supersession has to be recorded somewhere, and an unread stale ADR is worse than a long one. But it does mean the finding's estimate is now conservative and its "truncate superseded ones to the banner" step has more to remove than when it was written — ADR-0001, ADR-0006 and ADR-0017 are all superseded-with-full-body today. **State the basis when re-measuring:** this is a two-SHA measurement, not a standing count, and further ADR amendments were being written by other sessions while it was taken. Re-derive with `git ls-tree -r --name-only <sha> docs/adr` before acting on it.
|
||||
>
|
||||
> **Verified (2026-09-14, at HEAD `062ca47`): as written this finding saves nothing and breaks citations in four files.** The size of the saving depends on a convention the finding never states. Truncating ADR-0001 and ADR-0006 to their banners removes **20 lines** if the `---` separator and its trailing blank are kept (13 + 7), or **24–26** if truncation drops those too, which is the natural edit (15–16 + 9–10). The proposed `docs/adr/README.md` index costs 25. So the range is **+5 to −1 lines** — the robust conclusion is that the proposal is a wash, not that it nets +5.
|
||||
>
|
||||
> **The finding's headline was accurately measured; it has since gone stale.** Current state is **3,118 lines / 24 files**. The 2,740 was correct at commit **`a3e721e`** (2026-09-09, "docs: retire the META.md guidance ADR-0022 overruled"), an ancestor of HEAD: `docs/adr/` there is exactly **2,740 lines across 23 files**, exactly **8** ADRs exceed 150 lines, the top-eight share is **71.90%** (1,970/2,740 — the finding says 72%), and **ADR-0020 is 513 lines**. Every headline figure reproduces at that one commit, which rules out coincidence, and across all 67 commits touching `docs/adr/` `a3e721e` is the **unique** one yielding 2,740 (neighbours: `ed8c99e` 2,732, `a3e721e` 2,740, `568ca74` 2,747, `af80d27` 2,748). What moved the numbers is the ADR-0024 wave, already recorded in the note above. Re-derive with `git show a3e721e:<path>` rather than assuming the figure was invented.
|
||||
>
|
||||
> Today those same figures read: **ten** ADRs exceed 150 lines, not eight; top-eight share is 68.3%, the over-150 cohort 79.2%. ADR-0020 is **514** lines. Its Context is **72** lines counting the `## Context` heading and **71** without — a counting convention, not drift: the section is byte-identical at `a3e721e` and at HEAD (`## Context` at :11 through `## Decision` at :83), so the finding's 71 and this note's 72 are the same span counted two ways. ADR-0017's "173 amendment lines against 45 of decision" and ADR-0002's three lines are exact.
|
||||
>
|
||||
> **"The rules already live in `gates.md`" is backwards.** `docs/spec/gates.md:349-352` 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."*
|
||||
>
|
||||
> **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.
|
||||
>
|
||||
> **Nothing in `tests/` or `.pre-commit-config.yaml` reads `docs/adr/`** — `grep -n "docs/adr" tests/test-adr0020-*.sh` returns nothing; the ADR-0020 gate family tests `skill-size-check.sh` and the embedded resolver copies. **Editing ADR prose breaks no gate.** The only constraint is citation integrity.
|
||||
>
|
||||
> **"Cap Context at 20 lines" would destroy a derivation three scripts depend on.** ADR-0020's Context pins base commit `f9b919d7e3b`, states the summation method and the token approximation, and derives the 2,770 gate from 7.22 chars/word × 20,000 — stating body-only vs whole-file explicitly *because conflating them is the defect the ADR exists to stop*. Only the four illustrative anecdotes (~42 lines) are trimmable, and those are the argument, not the measurement.
|
||||
>
|
||||
> **Honest ceiling ~235 lines (7.5%)**, and the one real win is not in the finding: **delete ADR-0017's four amendments (−173) now that ADR-0024 consequence 7 has restated them in full**, re-pointing eight citations. Plus ADR-0001/0006 compressed to banner-plus-one-line (−20) and ADR-0020's anecdotes (−42). No README index. Restate the headline as **79% in ten ADRs**.
|
||||
>
|
||||
> **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.
|
||||
|
||||
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).
|
||||
@@ -212,7 +325,13 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> **Corrected then done (2026-09-13):** see commits `124ce6e` and follow-up on `docs/simplification-audit`. Independent re-verification found "most used only by gates.md/scripts/tests" overstated: 13 of 28 terms are actually referenced from model-facing `references/*.md` files skills load in normal use (Routing target, Hand-invoked skill, Dispatch body, Near-miss, Thin adapter, Provenance chain, Output profile, apm package, Plugin marketplace, HITL, Skill composition, Delegation discipline, holocron) and were kept untouched. Only the 9 terms confirmed as true orphans were removed after a fresh independent grep: Content mirror, apm-consumed install, Vale audit prefilter, Vacuous green, Management Application, Sycophancy, HOTL, Preload tax, Skill context contract — 28 → 19 terms.
|
||||
> **Re-counted (2026-09-14, at `a6434e0`): 18 terms, not 19.** The "28 → 19" above is an accurate record of this finding's own commit (`124ce6e`) and is left standing. `718c79a` then removed a twentieth-to-nineteenth entry this finding never touched: the standalone **Plugin** term, folded into **apm package** when ADR-0024 made "plugin" and "apm package" the same thing. Counted as bolded term entries between `## Language` and `## Relationships` in `CONTEXT.md`: 19 at `124ce6e`, 18 at `718c79a` and unchanged at `a6434e0`. The finding's own target ("about 20 one-line terms") is met either way. The preload-tax self-contradiction (quotes 23,427/10,478-char figures then says not to quote either) was confirmed verbatim and resolved by the entry's own deletion. The "example dialogue" and "flagged ambiguities" sections were found to be mandated by `grill-with-docs/references/context-format.md`'s template spec, not grill residue — left untouched, except one dangling bolded cross-reference to the now-deleted "Preload tax" term in a Flagged-ambiguities line, which was unbolded/de-referenced in place (the ambiguity resolution itself still holds without a defined glossary entry to point at).
|
||||
|
||||
32. **Structure is described three ways** (README layout table, architecture.md plugin table, AGENTS.md structure bullets), and `VISION.md` carries a 35-line stack spec for a product that lives in another repo. One layout table in README; architecture.md keeps mechanics only; VISION drops the stack detail. Effort S.
|
||||
32. [x] ~~**Structure is described three ways** (README layout table, architecture.md plugin table, AGENTS.md structure bullets), and `VISION.md` carries a 35-line stack spec for a product that lives in another repo. One layout table in README; architecture.md keeps mechanics only; VISION drops the stack detail. Effort S.~~
|
||||
> **Premise corrected, residual done (2026-09-14).** Both halves were inflated; most of the proposed split already existed.
|
||||
>
|
||||
> - **"Described three ways" overstates it.** `docs/spec/architecture.md` had already been differentiated, and says so in the file: its plugin table is prefaced "These are routing boundaries, not inventories — they answer 'where does a new skill go', so they deliberately do not enumerate what each plugin ships today… For what a plugin ships today, read `plugins/<name>/.apm/skills/` or the plugin list in `README.md`." That *is* the split this finding proposes, already implemented and self-documenting. README holds a path→contents table plus the six-plugin inventory; architecture.md holds a plugin→scope routing table that delegates inventory to README. `AGENTS.md`'s `## Structure` was **two bullets**, not a third description.
|
||||
> - **"A 35-line stack spec" counted the wrong thing.** `docs/VISION.md` is **71 lines total**. The whole `## Long-term: Management Application` section is 35 lines; the stack detail inside it was **5 lines** (Stack, Stack rationale, Deployment, Hosting, Users).
|
||||
>
|
||||
> The one genuine duplicate was `AGENTS.md`'s `plugins/` bullet restating apm-install mechanics owned by `docs/spec/architecture.md:22` and `README.md:55`. Done: that section cut to two actionable one-liners plus pointers to the README layout table and architecture.md — keeping the load-bearing session rule (`.claude/skills/` and `.claude/agents/` are install output, never edit them), which finding 29's earlier trim had left the Structure bullet carrying implicitly. In `VISION.md`, the stack/framework/deployment lines were replaced with a one-line scope statement deferring those choices to that product's own repo, and the Phase 1 "Mobile/desktop (Phase 3)" line was dropped as an intra-file duplicate of the Phase 3 section. Net **6 lines** (`f91babc`: 2 files changed, 5 insertions, 11 deletions); README and architecture.md untouched, both already correct.
|
||||
|
||||
## 6. Distribution, versioning, and session startup
|
||||
|
||||
@@ -220,8 +339,34 @@ Not covered by the area audits above; found on a final sweep of the root config
|
||||
|
||||
33. **Every plugin version lives in four places (five for kyberforge), plus one per skill.** `plugins/<name>/apm.yml`, two generated `plugin.json` files, the root `apm.yml` packages list, the `executables.allow` key (`kyberforge#1.6.2`), and a `metadata.version` in all 39 SKILL.md files (ADR-0022) that nothing consumes and that drifts freely (gitea skills sit at five different values). Repo tags (`v2.0.1`) follow a third scheme that the declared `tagPattern: v{version}` can never match under `per_package` versioning. ADR-0006, ADR-0022, `check-executables-allow-sync`, `skill-frontmatter`, and `apm pack --check-versions` all exist to police this. Proposal: one version per plugin in its `apm.yml`; drop `metadata.version` and ADR-0022; let `apm pack` derive the rest. Effort M.
|
||||
> **Partially advanced (2026-09-14):** see commit `718c79a` on `docs/simplification-audit`. Two of the four locations per plugin are gone: the twelve generated `plugin.json` manifests (`plugins/*/.claude-plugin/` and `plugins/*/.github/plugin/`) were deleted with the mirror. ADR-0006 needed no action — it was already moot and governed only those two now-deleted manifests, so no version bumps were required by the change. **Not closed.** Still outstanding: `plugins/<name>/apm.yml`, the root `apm.yml` packages list, the `executables.allow` pin, and `metadata.version` in all 39 SKILL.md files (still unconsumed, still drifting), plus ADR-0022 and the `v{version}` `tagPattern` mismatch.
|
||||
> **Verified (2026-09-14, at HEAD `062ca47`): headline wrong, central claim inverted — and it contains the one zero-risk, empirically-verified win in this audit.**
|
||||
>
|
||||
> **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. 39 `SKILL.md` files ✓. 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` and `skill-audit/scripts/validate.sh:1292-1332`, 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.
|
||||
>
|
||||
> **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` — and re-open the "is this field present here?" question issue #127 closed, just from the other side. **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.
|
||||
|
||||
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.**
|
||||
>
|
||||
> **The update is conditional, not unconditional.** `check-apm-current.sh:42-43` captures `apm outdated` and `exit 0`s unless the output matches `outdated dependenc(y|ies) found`. The staleness test is a real SHA comparison (`apm_cli/commands/outdated.py`, git-branch branch) of the lockfile's `resolved_commit` against the remote tip. On a current install the cost is one **~0.8 s** check and **no lock rewrite** — confirmed by timed probe. `hooks.json` also declares `"matcher": "startup"` only, so `--resume`/`--continue`/post-compact sessions never fire it (ADR-0019 sub-decision 3).
|
||||
>
|
||||
> **"That is why the lock file is dirty at the start of this session" is false.** The session-start `git status` reads `(clean)` and `apm.lock.yaml` was unmodified. Meanwhile `apm outdated` reports **6 outdated dependencies — the install sits 9 commits behind `main`**, right now, with nothing reporting it. The hook did not run.
|
||||
>
|
||||
> **ADR-0019 pre-answers the finding's core argument**, Context ¶3: *"Refreshing on push assumes the person who pushes is the person who goes stale, which is backwards: your install goes stale when someone else merges, and a push of your own is neither necessary nor sufficient for it to have happened."* "For a repo that is its own source" conflates authoring source with installed content — under ADR-0018 this repo consumes its own plugins as unpinned git refs against the remote default branch, so a session loads `main`, never the working tree (AGENTS.md states this). Being its own source makes it *more* exposed, not less: it is the only consumer whose authors routinely hold uninstalled edits and may assume they are live. The ADR also pre-rejects a manual `apm update` (sub-decision 1), accepting the dirty-lock cost deliberately — the AGENTS.md line the finding reads as evidence of a problem is the ADR's documented consequence.
|
||||
>
|
||||
> **`scripts/install.sh` has no apm step at all** — it installs git hooks and deploys `providers/claude-code/`, and runs once at setup, so it structurally cannot address staleness caused by someone else merging later. The proposed destination does not exist. Footprint is also understated: six files, 1,096 lines. And the claimed saving largely evaporates — deleting the `executables.allow` block turns apm's trust gate **off** for all six packages, a security regression ADR-0019 deliberately closed, so it must be retained in some form and `check-executables-allow-sync` (222 + 243) only becomes droppable if reduced to a non-version-keyed form.
|
||||
>
|
||||
> **Recommendation: keep the hook.** Cost is 0.8 s on a current install; offline it fails fast (0.81 s, status `unknown`, grep misses, exit 0 — the 60 s timeout is a bound, not a latency). The benefit guards a failure that is silent by construction and that the repo is exhibiting right now.
|
||||
>
|
||||
> **Two things worth fixing, neither of which is removal.** (1) ADR-0019's ~10.4 s refresh figure is now **~18 s** measured warm on a LAN remote — it is quoted in the `timeout: 380` invariant reasoning and understates by 75%. (2) **An undocumented branch hazard, and the strongest argument the finding could have made:** the hook resolves against the remote *default* branch, so on a feature branch that changes `plugins/`, an auto-refresh reinstalls `main`'s version over it. Reproduced — running `apm update` today re-installs `main`'s `plugins/bin/.mcp.json` and writes back the obsidian MCP server that commit `c96ca9c` removed on this branch. That deserves a line in ADR-0019's Consequences; the proportionate fix if it bites is ~3 lines skipping the refresh when `HEAD` is not the default branch.
|
||||
|
||||
35. [x] ~~**Outputs and packages for consumers that do not exist.** The `codex` output profile generates `.agents/plugins/marketplace.json` (95 lines) although Codex is not a supported consumer. The `mattpocock-skills` remote package entry is the only reason `apm-marketplace-check` needs the network, and its pin is advanced by hand (ADR-0015). The `.github/plugin/marketplace.json` mirror is a legacy path (finding 2). Removing all three leaves one generated marketplace manifest (the per-plugin `plugin.json` pairs remain) and no network-dependent hook. Effort S.~~
|
||||
> **Done (2026-09-13):** see commit `568ca74` on `docs/simplification-audit`. Removed the `codex` output profile from root `apm.yml` and its compiled `.agents/plugins/marketplace.json` (95 lines), and the `mattpocock-skills` remote package entry — the only remote marketplace entry, so `apm-marketplace-check` and `apm-pack-check-clean` no longer need network access at all. Updated `README.md`, `AGENTS.md`, `docs/spec/gates.md`, and `docs/spec/architecture.md` accordingly; added one-line superseded/updated notes to ADR-0015 and ADR-0021. Left `.github/plugin/marketplace.json` untouched — that's the Copilot legacy-path question in finding 2/§8, out of scope here; only re-ran the sync script to keep it consistent. `apm.lock.yaml` unaffected (`marketplace.packages[]` isn't part of the lockfile). Verified via `apm install`, `apm pack --marketplace=claude --check-versions`, and all four affected pre-push hooks.
|
||||
@@ -231,6 +376,21 @@ Not covered by the area audits above; found on a final sweep of the root config
|
||||
> - **"Left `.github/plugin/marketplace.json` untouched … out of scope here"** is void the same day: `0dffff3` deleted it under finding 2c, along with `scripts/sync-marketplace-mirror.sh` and its test. The "only re-ran the sync script to keep it consistent" step above refers to `sync-plugin-content.sh`, itself deleted in `718c79a`.
|
||||
|
||||
36. **The release-tag mechanism guards an external contract with no known consumer.** `.pre-commit-hooks.yaml` exports three hooks for other repos to pin by `rev: <tag>`. `check-release-needed` (242 lines + 442 test), `test-vale-hooks-consumer` (270 lines), ADR-0014, and three tags exist to serve that. If no other repo pins these hooks today, the whole mechanism can be deferred until one does. Effort S.
|
||||
> **Verified (2026-09-14, at HEAD `062ca47`): premise holds — the only premise in this audit to survive verification, though not the finding whole: `test-vale-hooks-consumer.sh` is 272 lines, not 270. Not yet decided; deferred by the human on 2026-09-14.**
|
||||
>
|
||||
> Exact: three exported hooks (`kyberforge-vale-audit-skill`, `kyberforge-vale-audit-agent`, `kyberforge-skill-size-check`), `check-release-needed.sh` 242, its test 442, three tags (`v1.0.0`, `v2.0.0`, `v2.0.1`). `test-vale-hooks-consumer.sh` is **272** lines, not 270.
|
||||
>
|
||||
> **Consumer evidence: none found, near-conclusive for this instance.** The Gitea instance holds exactly two repos; the other (`Defame1297/ansible-homelab-mono`) pins seven hook repos in its `.pre-commit-config.yaml` — conventional-pre-commit, gitleaks, jumanjihouse, yamllint, ansible-lint, pre-commit/pre-commit-hooks, plus `local` and `meta` — **none referencing this repo or any of the three hook ids**. All 13 commits touching the mechanism are self-authored fixes found by this repo's own tests; none traces to a reported external breakage. `test-vale-hooks-consumer.sh` builds a synthetic consumer in `mktemp` — a genuine regression test that caught a genuine shipped bug (`LESSONS.md:101`), simulating nobody who exists. Off-instance clones remain undeterminable. **ADR-0024 already ruled this standard sufficient** four commits earlier, deleting the 20,000-line mirror as "maintained for an audience of zero".
|
||||
>
|
||||
> **The README documents a contradictory contract** — "For external consumers" says consume through apm, "apm is the only supported install path", and never mentions `.pre-commit-hooks.yaml` or `rev:` pinning.
|
||||
>
|
||||
> **The mechanism is already failing at its one job.** `scripts/skill-size-check.sh` changed on `origin/main` in `598a7c3` after `v2.0.1`, with no tag cut since — a consumer pinning `rev: v2.0.1` gets a stale hook today. The gate cannot fire: it is wholly gated on `PRE_COMMIT_REMOTE_BRANCH == refs/heads/main`, and PRs merge through Gitea's server-side button, which sets nothing. The script's own header documents this as needing "a server-side CI job, which this repo does not have yet".
|
||||
>
|
||||
> **The premise that it serves only the external contract holds** — all three exported hooks are *separately* wired internally via `repo: local` (`.pre-commit-config.yaml:216,249,258`), so deleting the export costs **zero** internal lint coverage.
|
||||
>
|
||||
> **Correction to the finding: ADR-0014 gets amended, not retired.** Its primary decision — moving Vale config/styles/wrapper into `skill-audit/assets/vale/` and `agent-audit/assets/vale/`, self-locating from `${BASH_SOURCE[0]}` so the prefilter works at *runtime* in any repo installing kyberforge — is independent of the release-tag mechanism and stands on its own. Only the `.pre-commit-hooks.yaml` half and the tag consequence retire.
|
||||
>
|
||||
> 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`, the `gates.md:83` row and its "External consumers" section. 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.
|
||||
|
||||
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.
|
||||
@@ -262,10 +422,15 @@ Findings 9, 10, 11, and 12 are coupled through the provenance validator and the
|
||||
>
|
||||
> **The delete still holds**, for a stronger reason than the one given: the file was a preferred discovery path to content that no longer exists. What changed is the accepted cost — this is no longer "preference lost", it is the same accepted silent-empty-install residual §9 records, now known to apply to Copilot as well.
|
||||
- **Provenance chain.** Is "which upstream informed this file" a requirement you still want, or was it a governance experiment? Finding 11 hinges on this.
|
||||
> **Sharpened (2026-09-14):** still open, but ask it of the **producer** first. `plugins/bin/.apm/skills/research/` specifies the `sources.md` + `source_keys:` format and three evals in `plugins/bin/evals/research/research/eval.yaml` assert it. If `research` keeps emitting the chain, finding 11 collapses to "delete the validators" and the metadata stays. See finding 11's verification note.
|
||||
- **ADR-0012 (three core skills) and the one-script-per-skill install constraint.** ~~The merges in 14, 15, and 24 need the first revisited and are the only way around the second.~~ **Corrected (2026-09-14):** this grouping was wrong, and finding 2b's note has said so since `0dffff3` while this bullet said the opposite. ADR-0012 governs only the `core` plugin's three `agentsmd-*` skills (`agentsmd-author`, `agentsmd-audit`, `provider-adapter-author`) — read it: it names those three and nothing else. **Only finding 24 touches them, so only finding 24 needs ADR-0012 revisited.** Findings 14 and 15 merge kyberforge's `skill-audit`/`agent-audit` and `skill-author`/`agent-author`, which ADR-0012 does not govern; what constrains them is the self-containment rule, and merging is the way *around* it rather than a reason to reverse anything. That rule survives ADR-0024 — see §9's negative result and ADR-0024 consequence 6, which also correct its source: it is the agentskills.io spec for APM package mode, not a property of Claude Code's plugin cache-install as finding 2b's note assumed. The open question for 14/15 is a design one — one `description` carrying both skills' trigger phrases — not an ADR supersession. Are you open to superseding ADR-0012, for finding 24?
|
||||
> **Moot (2026-09-14):** finding 24 is refuted on arithmetic before this question is reached — the three `core` bodies total 1,360 words against `BODY_MAX_WORDS=900`, and their descriptions 806 chars against a 400 cap. Nothing needs superseding because the merge it would unblock cannot be committed. Question closed unless finding 24 is rewritten.
|
||||
- **Granularity of git/gitea skills.** One `git` skill vs seven trades routing precision for size. Is one broad description acceptable?
|
||||
> **Answered by measurement (2026-09-14): no, and it is not a preference question.** A merged git description measures **1,950 chars against a 400-char FAIL ceiling (4.9×)** and a 3,381-word body against 900 (3.8×). Both proposed gitea halves also FAIL at 2.5×, and the gitea split additionally puts a hard boundary through the edit-a-file-then-open-a-PR workflow. (An earlier revision also called the gitea split "blocked by ADR-0011, which already rejected a *smaller* bundling" — withdrawn; ADR-0011's objection is to a boundary being crossed, not to bundle size. See finding 20's verification note.)
|
||||
- **Auto-update at session start.** Do you want the install refreshed from the remote every time a session opens (finding 34), or is a manual `apm update` acceptable?
|
||||
> **Recommendation on evidence (2026-09-14): keep it; finding 34 refuted.** The premise that it runs on every startup is false (the update is conditional on a real SHA check), the lock was not dirty, the hook did not fire this session, and the install is currently **9 commits behind `main` with nothing reporting it** — the failure the hook exists to prevent. `install.sh`, the proposed alternative host, has no apm step. Still formally the human's call, but the factual basis for removing it does not survive. See finding 34.
|
||||
- **External hook consumers.** Does any other repo pin this repo's `.pre-commit-hooks.yaml` by tag today? If not, finding 36 defers the release mechanism entirely.
|
||||
> **Evidence gathered, decision deferred (2026-09-14).** No consumer found: the Gitea instance holds two repos, and the other pins seven hook repos, none of them this one. No consumer-driven commit in the 13 touching the mechanism. Off-instance clones undeterminable — but ADR-0024 accepted exactly this standard when it deleted the mirror. The mechanism is additionally **already broken** (a consumer pinning `rev: v2.0.1` gets a stale `skill-size-check.sh`, and the guard cannot fire through Gitea's merge button). The human deferred the decision on 2026-09-14; the finding is ready to execute when it is taken. See finding 36.
|
||||
- [x] ~~**Obsidian MCP.** Are the Obsidian tools over `docs/` used by anyone? If not, finding 37 is a pure delete.~~
|
||||
> **Answered (2026-09-14):** not used — remove entirely. All seven `.mcp.json` files are deleted and the repo-root path is gitignored; see finding 37, which also records the functional regression this uncovered (since `718c79a` deleted the per-plugin manifests, apm no longer propagated the server to consumers at all).
|
||||
|
||||
@@ -283,3 +448,48 @@ Recorded here so they are not rediscovered as defects. All follow from commit `7
|
||||
**Accepted gap — symlinks under `.apm/`.** ADR-0017's `check_apm_symlinks()` was the only thing reporting that symlinks under `.apm/` do not survive to a consumer. It is gone, and no replacement guard is being added — the human decided to accept the gap.
|
||||
|
||||
The mechanism is not the bundle exporter, as ADR-0017 assumed; it is the **install** path, and it has since been verified. `apm_cli/security/gate.py`'s `ignore_non_content()` is a `shutil.copytree` ignore callback whose docstring says "Excludes symlinks (security)"; it is used at `apm_cli/integration/skill_integrator.py:424`, `:791` and `:1152`. Materialization into `apm_modules/` dereferences first, so symlinked content survives *there* and is dropped when skills are deployed out of it. ADR-0024 flagged the prediction as unverified; it holds, with that corrected attribution. No symlinks exist under any `.apm/` today, so nothing is broken now — but the next one added there will silently not reach consumers, and nothing will say so.
|
||||
|
||||
## 10. Verification wave (2026-09-14)
|
||||
|
||||
Ten open findings with claimed yield — 11, 16, 20, 22, 24, 27, 28, 33, 34, 36 — were each re-checked against the files by an independent read-only agent, at HEAD `062ca47`. Findings 14 and 15 were deliberately excluded: their blocker is a design decision, not a premise. Results are recorded in each finding's own note above.
|
||||
|
||||
**Read this section before acting on any remaining finding.**
|
||||
|
||||
### Why the wave was run
|
||||
|
||||
This audit was written read-only, and its scope estimates are systematically optimistic. Before the wave, ten findings had been examined closely in the course of implementing them: 3, 19 and 37 each cost a full agent run to conclude "premise doesn't hold", and 2d, 29, 31 and 38 each needed correcting mid-implementation. Findings 5, 18 and 32 were then examined during a grill on 2026-09-14 and all three collapsed — 18's four examples were three different classes of load-bearing content, 5's suites turned out to be split by failure class rather than ADR section, and 32's proposed split already existed and was self-documenting in `architecture.md`.
|
||||
|
||||
That base rate made "effort S, no decisions needed" an unreliable signal, and §7's bucket 1 an unreliable plan. **Dispatching implementation agents against unverified premises costs more than verifying first.**
|
||||
|
||||
### Outcome
|
||||
|
||||
| Finding | Verdict | Verified yield |
|
||||
|---|---|---|
|
||||
| 36 | **Premise holds** — the only one whose premise survived; one supporting figure wrong (272 lines, not 270) | ~1,000 lines |
|
||||
| 33 | Headline wrong; one item empirically verified zero-risk | **6 lines, zero output diff** |
|
||||
| 28 | Headline accurate when measured (`a3e721e`), now stale; as written it is a wash (+5 to −1 lines); real win is elsewhere | ~235 lines |
|
||||
| 22 | Percentages 3–4× inflated, wrong target set; better defect found | ~150–180 lines |
|
||||
| 20 | Refuted — git 7→1 is 4.9× the description FAIL ceiling | ~230 lines salvage |
|
||||
| 27 | Refuted — audience separation misread as duplication | ~168 words + a live bug |
|
||||
| 11 | Direction defensible, two scope figures wrong; blocked on a decision it never poses | ~6,393 if unblocked |
|
||||
| 16 | Refuted — its only safe saving belongs to finding 14 | 0 independent |
|
||||
| 24 | Refuted — arithmetically impossible (1,360w vs a 900 cap) | 0 |
|
||||
| 34 | Refuted — evidence inverted | 0 |
|
||||
|
||||
**One premise of ten survived — finding 36's — but not the finding whole: its supporting figure was wrong (`test-vale-hooks-consumer.sh` is 272 lines, not 270). The other nine premises failed.**
|
||||
|
||||
**The headline figure was wrong in at most eight of the ten, not all ten.** Two exceptions, stated so the claim is not overstated:
|
||||
|
||||
- **Finding 36 states no headline figure.** Its headline is a claim — "the release-tag mechanism guards an external contract with no known consumer" — and the numbers appear only in a supporting sentence.
|
||||
- **Finding 34's stated figures are exact.** "A 60-line script with a 368-line test" checks out at both ends (`plugins/kyberforge/.apm/hooks/check-apm-current.sh` 60, `tests/test-apm-current-hook.sh` 368). Its note's only figure correction runs the *other* way — the footprint is **understated** at six files / 1,096 lines. What is refuted in 34 is the mechanism claim ("on every startup"), not an arithmetic error.
|
||||
|
||||
Three findings (16, 24, 34) are refuted outright; two (22, 28) contain a real finding different from the one written. Finding 28 is the one case where the headline was *accurately measured and went stale*: 2,740 lines / 72% in eight ADRs is exact at `a3e721e`, and the ADR-0024 wave moved it afterwards.
|
||||
|
||||
The recurring failure mode is worth naming, because it has now produced six wrong findings (3, 5, 16, 22, 24, and by implication 28): **dense validator and test code with heavy comments reads as over-engineering when skimmed, and turns out to be regression coverage whose comments name the incident.** Findings 16 and 24 propose reverting validators to sizes whose defects are on the commit record. Before proposing to cut any script or suite in this repo, read its header.
|
||||
|
||||
**Where the real remaining opportunity is:** finding 14 (merge `skill-audit` + `agent-audit`) at **−1,587 lines with zero coverage loss**, which is also where finding 16's savings actually live. Its blocker is the design question in §8 — one `description` carrying both skills' trigger phrases — not an ADR supersession.
|
||||
|
||||
### Two defects to fix independently of any finding
|
||||
|
||||
- **A live bug in always-on context.** The deployed `core/instructions/governance.md` cites `docs/HUMANS.md`, which does not exist — the file is `docs/wiki/HUMANS.md`. **Five occurrences across three files**; `governance.md` is self-inconsistent (line 73 correct, line 82 broken), and it is `@`-imported into every session in every project. Also `CONTROLS.md:5,101,106` and `ai-constitution.md:238`.
|
||||
- **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.
|
||||
|
||||
Reference in New Issue
Block a user