docs: record the grill and declined-finding decisions in the audit
Findings 11, 17, 23, 25 and 27 are declined. From the grill: 33 and 34 done, 8, 18, 20 and 28 closed, 22 deferred with bin. Open: 15 and 36. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -136,6 +136,8 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
|
||||
|
||||
8. **`docs/spec/gates.md` (1,048 lines) is roughly 15% "what is enforced" and 85% post-mortems** of defects already fixed and pinned by tests. The 60-line hook table is the useful part. Target 200 lines. The same applies to the 106 comment lines in `.pre-commit-config.yaml` and to `scripts/`, where 8 of 15 files are 40 to 60% comments. Effort M.
|
||||
> **Partially done (2026-09-13):** see commit `a35f5e8` on `docs/simplification-audit`. The 85%-post-mortem characterization was stale — the file had already shrunk to 966 lines by other findings, and most of what remained is load-bearing "why this design" rationale cited by ADRs and tests, not dead incident narration. Cut only the two genuinely stale passages: a reproduction paragraph carrying explicitly outdated numbers, and a retrofit-process narrative superseded by current state — a 36-line cut, 966 → 930 as measured at commit `a35f5e8`. Those two figures describe that commit only, not the file: `718c79a` and later findings have edited `gates.md` again, so read its current length from the file rather than quoting a number here. `.pre-commit-config.yaml`'s comments were left untouched; on inspection they're compact constraint notes, not filler. Target of 200 lines not reached and not recommended — would require deleting content the file itself flags as load-bearing.
|
||||
>
|
||||
> **Closed (2026-09-16, grill): done to the extent recommended.** The `gates.md` cut in `a35f5e8` stands; the 200-line target stays rejected (the file is 1,113 lines at HEAD, grown by later findings' sections, and read on demand only). The tests target below is struck: findings 3, 5 and 16 each found dense suites to be named-incident regression coverage. Any future cut to a test suite is its own finding and starts by reading that suite's header.
|
||||
|
||||
**Proposed target.** ~~Pre-push 14 hooks to 6: `run-tests`, `validate-plugins`, `validate-marketplace`, `apm-pack-check-clean`, `check-plugin-content-sync`, `check-release-needed`.~~
|
||||
|
||||
@@ -143,7 +145,7 @@ This is the area you named as hardest to understand and slowest. Root cause: mos
|
||||
|
||||
> **Superseded count (2026-09-15):** finding 14 deleted `check-vale-style-sync` with the merge into `factory-audit` (ADR-0025), so pre-push is now **8 repo-authored hooks** (10 reported). The dated note above is the state on 2026-09-14; see finding 14's note for the correction.
|
||||
|
||||
Pre-commit stays roughly as is minus `skill-frontmatter`, and minus `check-ast` once finding 9 removes the only `.py` files. ~~Tests 26 files to about 10 (12,400 to about 5,000 lines).~~ Keep bats and its three submodules; the 351 bats tests ship inside plugins and are the right tool there. Do not port the bash suites to bats; delete them instead.
|
||||
Pre-commit stays roughly as is minus `skill-frontmatter`, and minus `check-ast` once finding 9 removes the only `.py` files. ~~Tests 26 files to about 10 (12,400 to about 5,000 lines).~~ Keep bats and its three submodules; the 351 bats tests ship inside plugins and are the right tool there. ~~Do not port the bash suites to bats; delete them instead.~~ **Struck (2026-09-16, grill):** see finding 8's closing note — the suites are regression coverage (findings 3, 5, 16).
|
||||
|
||||
> **Re-measured (2026-09-14, at `a6434e0`):** the tests target was stated against the 2026-09-10 baseline and both its numbers are stale. `tests/` now holds **20 `test-*.sh` suites totalling 9,123 lines** (plus the two runners, 490). Six suites have gone since the baseline: `test-check-manifests.sh` (`e647f14`), `test-skill-frontmatter.sh` (`c8a7c9e`), `test-governance-layer.sh` and `test-instructions-and-docs.sh` (`5f9f2b3`), `test-sync-marketplace-mirror.sh` (`0dffff3`), `test-sync-plugin-content.sh` (`718c79a`). Restated on the same basis the target is **20 files to about 10, 9,123 to about 5,000 lines** — the file half of the target is now the closer half, and finding 9's `check-ast` clause is moot anyway, since finding 9 is not proceeding.
|
||||
|
||||
@@ -172,6 +174,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> 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** (about ~~6,393~~ → **7,136** lines deleted across 242 files: the ~~4,641~~ → **5,380** validator and bats lines plus the ~~1,752~~ → **1,756** of `sources.md` measured above, across 196 `source_keys` carriers and ~~46~~ → **45** `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~~ → **1,207 lines of skill `sources.md`, the 1,145-line `lib-provenance-skill.sh`, 87 tests**, re-measured 2026-09-16 at HEAD) and leave the ADR-0010 plugin-root half alone — no ADR supersession needed.
|
||||
>
|
||||
> **Decision (2026-09-16):** Not proceeding — the human declined this finding. The provenance chain (`sources.md`, `source_keys:`, `validate-provenance.sh`) stays, and `research` keeps producing it. This also answers §8's provenance question.
|
||||
|
||||
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.
|
||||
@@ -210,6 +214,7 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> Where the savings actually are: **merge skill-audit + agent-audit (finding 14) → ~~−1,587 lines~~ → landed 2026-09-15 at −2,934 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.
|
||||
> **Decision (2026-09-16):** Not proceeding — the human declined this finding. `forge` and `apm-install` stay as separate skills.
|
||||
|
||||
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:
|
||||
@@ -229,6 +234,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> - **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.
|
||||
>
|
||||
> **Closed (2026-09-16, grill): not proceeding.** Finding 22 is deferred with `bin`, and the folded kyberforge remainder (30–60 thin lines, no skill brought under budget) is too small to stand alone, as the note above already says.
|
||||
|
||||
### 4.3 git and gitea (153 + 93 files, 9,889 + 6,047 lines incl. mirror; source 3,288 + 2,286)
|
||||
|
||||
@@ -251,6 +258,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> 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.
|
||||
>
|
||||
> **Closed (2026-09-16, grill): not proceeding, salvage included.** Both files are loaded on demand only (`git-commits/SKILL.md:46`, `git-history/SKILL.md:33`), and exactly when the agent needs the precise rules; replacing them with a URL pointer trades a local, deterministic answer for a network fetch mid-commit. `bisect.md` is not ~97% generic — every command carries the `rtk` prefix (ADR-0023). `conventional-commits-spec.md` carries the commitlint 11-type set the repo's `conventional-pre-commit` hook enforces. The gitea issue-vs-PR duplication is forced by the no-cross-skill-sharing rule (§9). The saving would be repo lines, not context tokens, at the price of a version bump per file.
|
||||
|
||||
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`.
|
||||
@@ -271,8 +280,11 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> **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.
|
||||
>
|
||||
> **Deferred (2026-09-16, grill).** The human is excluding `bin` from this audit: its skills are "binned for a reason" and will be fixed or relocated as a separate piece of work. Nothing in this finding is executed here; the itemised ~165-line estimate above is the starting point for that work. `bin` is still covered by the version-bump gate (finding 33) until then.
|
||||
|
||||
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.
|
||||
> **Decision (2026-09-16):** Not proceeding — the human declined this finding. `grill-me` and `grill-with-docs` stay separate.
|
||||
|
||||
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.**
|
||||
@@ -288,6 +300,7 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> 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.
|
||||
> **Decision (2026-09-16):** Not proceeding — the human declined this finding. The `lint-runner` agent stays.
|
||||
|
||||
## 5. Prose and docs (9,600 lines, 109,000 words outside plugins)
|
||||
|
||||
@@ -310,6 +323,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> **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.) **Fixed (2026-09-15):** all five corrected to `docs/wiki/HUMANS.md`; the deployed copy under `~/.claude/` is now stale until `scripts/install.sh` re-runs. (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.
|
||||
>
|
||||
> **Decision (2026-09-16):** Not proceeding — the human declined this finding, including the 168-word cross-reference trim. `core/instructions/governance.md` and the other three governance documents stay as they are. The `docs/HUMANS.md` path defect was fixed separately (see §10).
|
||||
|
||||
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.
|
||||
@@ -333,6 +348,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
|
||||
> **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.
|
||||
>
|
||||
> **Closed (2026-09-16, grill): not proceeding.** Decision: a superseded or accepted ADR's text is the historical record (the Nygard convention, and what every ADR-0024 banner already says), so no body is truncated, no amendment is deleted, and ADR-0020's Context is left intact. That removes every remaining cut — ADR-0017's amendments are part of its record, and ADR-0024 consequence 7 (`0024:213-252`, ~40 lines) summarises them rather than restating them in full as the note above says. Corrected headline for anyone quoting it: **79% of `docs/adr/` lines sit in ten ADRs over 150 lines** (measured 2026-09-14); the 2,740 figure is the `a3e721e` state only.
|
||||
|
||||
29. [x] ~~**The same facts are stated in full three or four times.** "Edit `.apm/`, never the mirror": README (2 paragraphs), AGENTS.md (2 paragraphs), architecture.md (2 paragraphs plus the lost-README anecdote), ADR-0017. The apm.lock / SessionStart story: README (11 lines), AGENTS.md, ADR-0018, ADR-0019, gates.md. The offline `SKIP=` command and the three-stage install each appear three times. Rule: README has the how-to, AGENTS.md has one-line rules with links, architecture.md has mechanics. Effort S.~~
|
||||
> **Corrected then partially done (2026-09-14):** independent re-verification found the "edit `.apm/`, never the mirror" and apm.lock/SessionStart clusters confirmed but the third overstated — no file documents an offline `SKIP=` command (the one `SKIP=`-adjacent mention in `gates.md` explicitly says a *different* opt-out "is not `SKIP=`"), and "three-stage install" appears twice, not three times, with no restatement worth trimming. Trimmed the two confirmed clusters: README's "Editing plugin content" and AGENTS.md's "Edit `.apm/`, never the flat mirror" sections cut to the how-to/one-line-plus-link split the finding itself proposed, full mechanics (the `rm -rf` behavior and the `plugins/kyberforge/hooks/README.md` anecdote) staying solely in `docs/spec/architecture.md`. README's "Keeping the install current" and AGENTS.md's apm.lock bullet trimmed to drop the restated `apm outdated`/`apm update --yes` timing narrative, pointing to ADR-0019 as the canonical mechanism instead. No test greps the trimmed wording (checked).
|
||||
@@ -374,6 +391,8 @@ Not covered by the area audits above; found on a final sweep of the root config
|
||||
> **The `tagPattern` claim is refuted — inert, not broken.** Under `versioning.strategy: per_package`, apm never reads it: `version_check.py:262` gates on `strategy == "tag_pattern"`, and `builder.py:641,781` are reachable only for *remote* source entries, while all six packages here are local paths. The `v1.0.0`/`v2.0.0`/`v2.0.1` tags are not "a third scheme" — they are the `.pre-commit-hooks.yaml` external-consumer contract tags from finding 36, a different axis entirely. Latent risk only: if `dependencies.apm` ever gains `ref:` pins, tagPattern goes live against per-package tags that do not exist.
|
||||
>
|
||||
> Also: **`executables.allow` should be kept** — it is version-keyed by apm's design and `check-executables-allow-sync` guards a real silent failure (ADR-0019). And ADR-0006's ADR-0024 amendment asserting *"`apm.yml`'s `version:` is the only version field a plugin has"* is inaccurate while root `packages[].version` exists — fixed by the deletion above.
|
||||
>
|
||||
> **Decided and done (2026-09-16, grill): enforce the bump.** Advisory status and dropping the field were both rejected. The six root `apm.yml` `packages[].version` lines are deleted (`apm pack --check-versions --check-clean` still passes, output unchanged), which also makes ADR-0006's "the only version field a plugin has" true. `scripts/check-skill-version-bump.sh` now runs at pre-push: any skill directory that changed against its merge-base with `main`, `tests/` excluded, must carry a strictly higher `metadata.version` than `main`; new, renamed and deleted skills are exempt; every plugin is covered, `bin` included. Recorded as a dated section in ADR-0022, not a new ADR. The 17 skills changed on this branch without a bump took a patch bump in the same commit. The `executables.allow` pin and the inert `tagPattern` are left as the note above recommends.
|
||||
|
||||
34. **The SessionStart hook auto-updates the install on every startup.** `check-apm-current.sh` runs `apm outdated` (network, 60 s timeout) and then `apm update --yes` (300 s timeout) at every session start, rewriting `apm.lock.yaml`. That is why the lock file is dirty at the start of this session and why `AGENTS.md` has to explain "commit or discard it deliberately". It is a 60-line script with a 368-line test, an ADR (0019), the `executables.allow` pin, and a sync hook behind it. For a repo that is its own source, the update belongs in `install.sh` or a manual `apm update`, not in session startup. Effort S to remove; the design question is whether auto-update at startup is wanted at all.
|
||||
> **Refuted (2026-09-14, at HEAD `062ca47`). The evidence is inverted: the finding cites as proof of over-eagerness a session in which the mechanism did not fire, and the observed state is the exact silent failure ADR-0019 exists to prevent.**
|
||||
@@ -389,6 +408,8 @@ Not covered by the area audits above; found on a final sweep of the root config
|
||||
> **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.
|
||||
>
|
||||
> **Decided and done (2026-09-16, grill): document, do not skip.** Both follow-ups landed in ADR-0019. The refresh figure is re-measured at ~24–26 s (two runs, six packages behind), not the ~18 s above — still inside the 360 s the `timeout: 380` invariant covers. The branch hazard is a new Consequences paragraph, written after it fired in this very session: the startup refresh redeployed `main`'s `skill-audit`/`agent-audit` and the `obsidian` server over this branch. Skipping the refresh off the default branch was rejected — it would freeze the session on an older `main` without making the branch live.
|
||||
|
||||
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.
|
||||
@@ -426,8 +447,10 @@ Not covered by the area audits above; found on a final sweep of the root config
|
||||
## 7. Suggested order
|
||||
|
||||
1. Quick wins, all S, no design decisions needed: findings 9, 10, 26, 30, 31, 29, 12, 13, 1, 6, 4, 35, 37, 38, and the mirror-sync and executables-allow halves of 2. Removes roughly 25,000 to 30,000 lines and 6 hooks.
|
||||
2. Structural changes that need a short discussion: ~~14~~, 15, ~~19~~, 20, 23, 25, 17, ~~3~~, 5, ~~7~~, 33, ~~34~~, 36.
|
||||
3. The real complexity: ~~16 (validators)~~, 11 (provenance), ~~24 (core)~~, 8 and 28 (gates.md and ADRs).
|
||||
2. Structural changes that need a short discussion: ~~14~~, 15, ~~19~~, ~~20~~, ~~23~~, ~~25~~, ~~17~~, ~~3~~, ~~5~~, ~~7~~, ~~33~~, ~~34~~, 36.
|
||||
3. The real complexity: ~~16 (validators)~~, ~~11 (provenance)~~, ~~24 (core)~~, ~~8 and 28 (gates.md and ADRs)~~.
|
||||
|
||||
> **Status (2026-09-16, after the grill on 33, 28, 22/18, 20, 8, 34):** open findings are **15** (merge `skill-author` + `agent-author`) and **36** (release-tag mechanism, decision deferred by the human). **22** is deferred with the rest of `bin`. Every other finding is done, closed, or refuted at its own note.
|
||||
|
||||
> **Re-derived (2026-09-16, at HEAD):** this ordering was written before the findings were worked, and seven of its entries are now closed. Struck above: **14** landed (`467bbd7`, ADR-0025); **7** was superseded then done (`718c79a`); **3** and **19** are not proceeding on refuted premises; **34**, **16** and **24** are refuted outright. **5** is left standing but is downstream of 16 by its own note, so it cannot be taken in this bucket's order. Read each finding's own marker, not this list — it is a plan of record, not a status board. Bucket 1 is left as written: every entry in it is marked `[x]` or carries a decision note at its own finding.
|
||||
|
||||
@@ -445,7 +468,8 @@ Findings 9, 10, 11, and 12 are coupled through the provenance validator and the
|
||||
> **The mechanism is host-independent, which is why this bullet had to change.** ADR-0024 consequence 1 and §9's first residual both state it for Claude Code: a native registration succeeds and installs six plugins containing zero skills, silently. Nothing in that chain is Claude-specific. The catalogue is a list of plugin *roots*; discovery of content inside a root is a convention-scan of flat `skills/`/`agents/`/`hooks/` directories, and that is the layout `718c79a` deleted. Whichever of the four paths a host resolves the catalogue through, it lands on the same empty roots. Copilot was in fact always the weaker case — ADR-0017's own `hooks` amendment records that the mirror only ever partially served it.
|
||||
>
|
||||
> **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.
|
||||
- [x] ~~**Provenance chain.** Is "which upstream informed this file" a requirement you still want, or was it a governance experiment? Finding 11 hinges on this.~~
|
||||
> **Answered (2026-09-16):** keep it. The human declined finding 11; the chain and its validators stay.
|
||||
> **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?
|
||||
> **Closed on the 14/15 half (2026-09-16, at HEAD):** finding 14 landed as `factory-audit` on 2026-09-15 (`467bbd7`, ADR-0025), and the design question this bullet holds open was answered by doing it — the merged description ships at 241 characters, inside the 250 SUGGESTION target, and the binding ceiling turned out to be the 900-word **body**, solved with a dispatch body over `skill-*`/`agent-*` reference files. See finding 14's own note. What remains open here is finding 15 (`skill-author` + `agent-author`) and the ADR-0012 question below, which the next note already answers.
|
||||
|
||||
Reference in New Issue
Block a user