4 Commits

Author SHA1 Message Date
1614bcef23 docs: correct the executables.allow version-pinning claim
gates.md's check-executables-allow-sync section and ADR-0019 both stated
that apm matches executables.allow on an exact `<package>#<version>`
dictionary lookup with no wildcard and no version-less form, and drew the
conclusion that a kyberforge version bump silently stops the entry
matching and the SessionStart hook deploying.

Verified against apm 0.28.0: is_package_approved is an exact lookup, but
install/exec_gate.py calls it across a candidate list carrying the
version-blind name, materialize_exec_map stores each approved key under
its version-blind name as well, and _map_grants matches exact key,
version-blind name, or any stored key sharing that name. Approving
kyberforge#2.0.0 therefore keeps covering kyberforge#2.1.0.

The decision is unchanged: check-executables-allow-sync stays, justified
by this repo's own requirement that the key track plugins/kyberforge/
apm.yml's version:, rather than by an apm-level failure mode. ADR-0019
keeps its original text with a dated correction, since whether apm
behaved this way when it was written was not established.

Follows the same correction applied to root apm.yml's comment in 82b7bbc.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
2026-09-19 21:33:43 +00:00
82b7bbcf5c docs: close the PR #135 documentation review findings
Group 3 of the validated PR #135 review fixes. Every figure and commit
citation below was re-verified at HEAD before being written.

ADR and architecture:
- #7 ADR-0025 cited 61b0b9c, which no published branch reaches. Repointed
  to 620f20b (identical parent tree, reachable from the PR branch), with a
  note that neither is reachable from origin/main. The parser-drift
  paragraph now credits 598a7c3 (the reachable PR #129 squash) and keeps
  484357a only as a pre-squash parenthetical.
- #8 architecture.md dropped the pointer at the LESSONS.md entry this
  branch deleted.
- #9 architecture.md's ADR entry points now name ADR-0015 (the one
  compiler) and ADR-0024, and list ADR-0024 as superseding ADR-0017.
- #10 ADR-0024 section 4 rewritten: the standing patch-bump rule is
  apm-workflow's configure.md, not ADR-0006's, and this change does not
  trigger it. ADR-0015:93 carries a correction for the misattribution.
- N5 ADR-0021 gained a Correction note for the deleted
  scripts/check-manifests.sh (e647f14).

gates.md:
- #11a the four ADR-0020 constants live in lib-checks-skill.sh:313-316 and
  lib-checks-agent.sh:164-165, not in validate.sh.
- #11b the pretty-format-json exclude is two alternations expanding to
  three tracked files, including .claude/apm-hooks.json.
- #11c the ADR-0020 contract suite runs 28 -> 27 -> 29 (620f20b,
  4de5b6b, ef27c97), 29 at HEAD; the unverifiable 25 is dropped.
- #11d the boundary resolver is one copy since ef27c97.
- #12 apm-audit-ci documents the 10 root checks and the 1 plugin check
  apm 0.28.0 actually runs, that content-integrity IS the hidden-Unicode
  scan, that manifest-parse is not a named check, and that the hook needs
  a completed apm install. The offline claim is qualified accordingly.
- N9 gates.md:142-146 verified to still match the hook description.

AGENTS.md:
- #12 the no-network session rule is qualified to a populated
  apm_modules/.

Audit note:
- A1 hook counts corrected to 27/9 -> 26/8 -> 27/9 -> 26/8 (26 and 8 at
  HEAD) and the dangling pointer dropped.
- A2 skill-size-check.sh is 509 lines with the resolver sourced, not 1,522
  embedded; citations repointed to skill-size-check.sh:323-335 and
  lib-checks-skill.sh:235-283 (fail() at :265 and :280), and that library
  is 627 lines.
- A3 consumers receive 15 test files across 5 skills; 16 tracked test
  paths repo-wide.
- A4 the "do not run apm update on this branch" instruction is marked
  superseded, with the branch-aware guidance in its place.
- Finding 31's "true orphans" claim corrected for HOTL and Sycophancy,
  both still used in core/ai-constitution.md.

Same class, found during group 2:
- skill-author's deployment-modes.md no longer points at .mcp.json
  configs (deleted in c96ca9c); metadata.version 1.0.2 -> 1.0.3.
- git-orchestrate's context contract clarifies that
  user_config_overrides is caller-supplied session state, not a config
  read. The field name is unchanged.
- B3 root apm.yml's executables.allow comment: grants are version-blind
  in apm 0.28.0, so the #2.0.0 suffix is cosmetic to apm and a bump does
  not break the hook; the suffix stays because
  check-executables-allow-sync.sh requires it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
2026-09-19 21:30:29 +00:00
3920dfab20 fix(skills): drop references to deleted config and .mcp.json files
Closes four PR #135 review findings in skill content.

#2 — plugins/git/config.example.json was deleted in f5e4d0d, but four
git-plugin files still told the agent to read it. The file only ever
carried branching_pattern, commit_style and rebase_strategy, so the
`base_branch` and scope instructions were wrong even before the
deletion. Each site now describes what the skill actually does: base is
`main` under GitHub Flow or `develop` when Gitflow is inferred, the
Gitflow fallback keys off the repo's own branches, the orchestrator
contract's `base` defaults to the inferred base branch, and the commit
scope is inferred from the changed files.

N6 — gitea-prs was the one gitea skill with no permission-scope caveat
on a 404. Added one alongside the existing issue/PR number-space
guidance rather than replacing it: a 404 is only evidence of
"that number is an issue" once write:repository scope is confirmed.

N4 — plugins/kyberforge/bin/README.md pointed at `.mcp.json`, but all
six plugin-root .mcp.json files were deleted in c96ca9c (ADR-0018).
${CLAUDE_PLUGIN_ROOT} itself is still live, so the sentence now points
at .apm/hooks/hooks.json, which kyberforge's own hook already uses.

N7 — not applied. The finding claimed a marketplace field override
emits a verbose BuildDiagnostic that `apm pack -v` surfaces, so
"silently wins" was wrong. apm 0.28.0 does construct the diagnostic in
marketplace/output_mappers.py, but nothing renders it:
_render_marketplace_result in commands/pack.py iterates `warnings`
only, and BuildReport.diagnostics has no consumer. Confirmed on a
fixture — neither `apm pack -v` nor APM_LOG_LEVEL=DEBUG prints the
override, and --check-versions reports [matches]. The existing wording
in configure.md and marketplace.md is correct, so both are unchanged.

Version bumps required by check-skill-version-bump.sh: git-branches
1.0.4 -> 1.0.5, git-commits 0.1.6 -> 0.1.7, gitea-prs 0.1.4 -> 0.1.5.
bin/README.md is outside any skill directory and needs no bump.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
2026-09-19 21:17:14 +00:00
ea119d83b0 fix(gates): close six PR #135 review findings in gates and their tests
B1: check-skill-version-bump.sh resolves every merge-base with `git merge-base
--all` instead of the single base git happens to pick. A criss-cross history has
two, so the verdict turned on that choice: a skill byte-identical to main's tip
could still be reported "not above merge-base" / "not above main tip" and fail a
push that should pass. A skill now counts as changed only when it differs from
EVERY base, and its version must exceed the version at every base it exists at
as well as at the main tip; with more than one base the failure names which one.
Case 40 in tests/test-skill-version-bump.sh builds the criss-cross fixture and
pins both directions.

B2: check-apm-current.sh no longer assumes the remote default branch is `main`
when origin/HEAD is unset. A checkout whose default is `master` was standing on
its default branch and being told "this is a feature branch, so discard it" --
to throw away a real lock update. With origin/HEAD unset nothing is asserted and
the neutral advice stands. tests/test-apm-current-hook.sh covers the unset case
on both `main` and `master`.

#4: the required-frontmatter checks folded into skill-size-check.sh by c8a7c9e
were untested apart from the leading-zero shape -- mutating the missing-version
ERROR into a no-op left every suite green. tests/test-adr0020-frontmatter.sh now
pins name presence and non-emptiness, metadata.version presence and semver
shape, and the four grep defects the deleted test-skill-frontmatter.sh named.

#5: nothing asked whether a Vale rule still MATCHES anything -- rewriting
CompositionNote.yml's tokens to match nothing left test-vale-wrap.sh at 63/63.
Case 35 enumerates the rule files under the Kyberforge* style directories at run
time, requires an alert from each on its own fixture, and fails when a
discovered rule has no fixture row. The stale comment at case 31 is corrected.

#6: tests/run-tests.sh --strict exited 0 when discovery found no test-*.sh at
all; strictness only ever acted on skips, and with no suites there were none. It
now cross-checks the git index the way run-bats.sh does and fails
unconditionally on an empty set, naming the search root.

N9: the skill-size-check hook description in .pre-commit-config.yaml covered
only the size, context-budget and boundary-target gates. It now also names the
required frontmatter fields, matching docs/spec/gates.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
2026-09-19 21:08:10 +00:00
30 changed files with 861 additions and 123 deletions

View File

@@ -215,7 +215,7 @@ repos:
- id: skill-size-check
stages: ['pre-commit']
name: SKILL.md size and context-budget ceilings
description: Enforce agentskills.io's 500-line/2,770-whole-file-word spec ceilings AND ADR-0020's context budget -- description 250 chars SUGGESTION / 400 FAIL, body-only 600 words SUGGESTION / 900 FAIL, and every boundary-clause routing target resolving to a real skill or agent under plugins/*/.apm/
description: Enforce agentskills.io's 500-line/2,770-whole-file-word spec ceilings AND ADR-0020's context budget -- description 250 chars SUGGESTION / 400 FAIL, body-only 600 words SUGGESTION / 900 FAIL, and every boundary-clause routing target resolving to a real skill or agent under plugins/*/.apm/ -- plus the required frontmatter fields folded in from the former skill-frontmatter hook, namely name, a non-empty description, and a metadata.version matching three-part semver (1.0.0)
entry: scripts/skill-size-check.sh
language: script
files: '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$'

View File

@@ -30,7 +30,7 @@ Fall back to raw shell only when no skill covers it.
- **Do not add repo-owned keys to `.claude/settings.json`.** apm treats it as its own deployed artifact and `apm audit --ci` replays the install and diffs, so anything apm would not have written is permanent drift that fails the `apm-audit-ci` pre-push hook. A hook you want here is authored in `plugins/<name>/.apm/hooks/` and deployed by apm, never hand-written into that file. The `SessionStart` entry already in it is exactly that: kyberforge authors it in `plugins/kyberforge/.apm/hooks/hooks.json` and apm merges it in, so it is apm's own output, it is what the replay expects, and it belongs in the commit — do not strip it (ADR-0019). Machine-specific settings go in the gitignored `.claude/settings.local.json`; shared enforcement goes in `.pre-commit-config.yaml`.
- **`apm.lock.yaml` turning up modified is expected, not a bug.** kyberforge's `SessionStart` hook keeps the install current on launch and rewrites the lock in the process (ADR-0019). On `main`, commit or discard it deliberately. On a feature branch, discard it (`git checkout -- apm.lock.yaml`, then `apm install`). This keeps unrelated lock churn out of the branch diff and keeps `apm pack --check-clean` consistent with the committed lock. The session then runs the older `main` that the lock records, which is accepted on a branch, and the next session start refreshes again.
- **A `.apm/` edit is not live until it is on the remote's `main`.** The six dependencies resolve from the holocron remote, unpinned against the default branch, so pushing a feature branch does not deploy it (ADR-0019). `apm install` deploys from the lock; `apm update` is what re-resolves refs.
- **No pre-push hook needs the network.** Root `apm.yml`'s marketplace has no remote package entries, so every hook resolves locally.
- **No pre-push hook needs the network — once `apm install` has run.** Root `apm.yml`'s marketplace has no remote package entries, so every hook resolves locally. The guarantee is a property of a populated `apm_modules/`, not of the hook set: on a fresh clone `apm-audit-ci`'s `deployed-files-present` fails outright, and its `drift` and `config-consistency` install-replays have no cache to replay from and clone from the remote. Run `apm install` once on a new checkout and the offline guarantee holds from then on (`docs/spec/gates.md`, "Pushing without a network").
- **This repo and Gitea are the only source of truth.** All project state, decisions, and working conventions live here. Do not use an external memory system for this project — cached state diverges from the repo and you get a split brain. Before answering any design or architecture question, check `docs/adr/` for an existing decision.
## Key documents

15
apm.yml
View File

@@ -36,10 +36,17 @@ dependencies:
# executables deploy" until an `executables:` block exists.
#
# kyberforge ships the SessionStart hook that keeps this install level with the
# remote (ADR-0019). The key is version-pinned by apm's own design, so a
# kyberforge version bump makes this entry stop matching and the hook stops
# deploying until the version here is bumped too. If skills silently go stale
# after a kyberforge release, check this first.
# remote (ADR-0019). The `#2.0.0` suffix below is cosmetic as far as apm is
# concerned: grants are version-BLIND in apm 0.28.0. `_map_grants`
# (apm_cli/security/executables.py) matches the exact key, the version-blind
# name, or any stored key sharing that name, and `materialize_exec_map` also
# stores the version-blind name — so approving `kyberforge` covers
# `kyberforge#2.0.0` and vice-versa, and a kyberforge version bump does NOT
# make this entry stop matching or stop the hook deploying. Do not delete the
# suffix anyway: `scripts/check-executables-allow-sync.sh` is a repo-authored
# pre-push hook that asserts this key carries the version in
# plugins/kyberforge/apm.yml, so a bump here is a repo convention to keep, not
# an apm mechanic.
executables:
allow:
kyberforge#2.0.0:

View File

@@ -93,6 +93,13 @@ correction) sorted what they document into three buckets:
because of hand-authored dual manifests (ADR-0006's version-parity/patch-bump rule, the
CC-vs-Copilot field-placement split, dual-file mirroring) are obsolete under `apm.yml`'s
single-manifest model and were deliberately dropped.
> **Correction (2026-09-19):** "ADR-0006's version-parity/patch-bump rule" misattributes the
> patch-bump half. ADR-0006 states a version-*parity* rule and nothing about patch bumps — the
> string `patch` does not appear in it (`git show origin/main:docs/adr/0006-plugin-version-parity.md`).
> Only the parity half was ADR-0006's, and only that half was dropped. A patch-bump rule does
> exist and is live: `plugins/kyberforge/.apm/skills/apm-workflow/references/configure.md` —
> bump a package's own `apm.yml` `version:` whenever anything reaching its compiled output
> changes. ADR-0024 §4 repeated this misattribution and is corrected there too.
- **Holocron policy choice — resolved in #90.** `marketplace-author`'s catalog-version convention
(minor bump for package add/remove, patch bump for field-only updates) isn't an APM mechanic —
`apm` doesn't enforce it, and has no native version-bump automation at all — so rather than

View File

@@ -75,7 +75,17 @@ to end, reintroduced through the mechanism meant to secure it.
Matching is an exact dictionary lookup on the composed `name#version` string
(`apm_cli/security/executables.py`, `is_package_approved`), so there is no wildcard or
version-less key that would sidestep this — the key has to be edited on every bump, and the
question is only what catches a missed edit. A comment in the `executables:` block is not enough:
question is only what catches a missed edit.
> **Correction (2026-09-19):** the mechanism above is wrong for apm 0.28.0, verified in source.
> `is_package_approved` is an exact lookup, but `install/exec_gate.py` calls it across a candidate
> list that includes the version-blind name, `materialize_exec_map` stores each approved key under
> its version-blind name too, and `_map_grants` matches exact key, version-blind name, or any stored
> key sharing that name. So approving `kyberforge#2.0.0` keeps covering `kyberforge#2.1.0`: a bump
> does not silently stop the hook deploying. Whether apm behaved this way when this ADR was written
> was not established. **The decision stands** — `scripts/check-executables-allow-sync.sh` is now
> justified by this repo's own requirement that the key track `plugins/kyberforge/apm.yml`'s
> `version:`, not by an apm-level failure mode. `docs/spec/gates.md` carries the same correction. A comment in the `executables:` block is not enough:
this repo gates generated-content drift, marketplace mirror drift and vale style drift
deterministically, and a silent-staleness failure is strictly worse than any of them. So
`scripts/check-executables-allow-sync.sh` runs at pre-push, parsing `version:` out of

View File

@@ -85,7 +85,15 @@ unnamed in `git`'s corrected description, though `65bac15`'s own commit message
`gitea`'s. Across the three plugins, 23 of 27 skills are named at the third attempt.
**Nothing checks any of this.** `scripts/check-manifests.sh` does not contain the string
`description`. The three ADR-0020 validators (`scripts/skill-size-check.sh` and skill-audit's and
`description`.
**Correction (2026-09-19): that script no longer exists.** `e647f14` deleted
`scripts/check-manifests.sh` (282 lines), `tests/test-check-manifests.sh` (771 lines) and the
`check-manifests` pre-commit hook entry with them. The conclusion is unchanged and now holds a
fortiori: the gate that did not read `description:` is gone, so nothing in its place reads it
either.
The three ADR-0020 validators (`scripts/skill-size-check.sh` and skill-audit's and
agent-audit's `validate.sh` — two since ADR-0025 merged the audit pair into `factory-audit`, whose
single auto-detecting `validate.sh` carries both) gate on SKILL.md and agent frontmatter; they do open `apm.yml`, but only
to read `dependencies.apm` when resolving the boundary-target universe — none of them reads the

View File

@@ -179,14 +179,15 @@ now discoverable in the install output and would otherwise be found and double-r
they do not belong to — exactly the `apm_modules/` problem ADR-0018 recorded, arriving by a second
route. Any future script that walks this repo's tree needs both exclusions.
**4. No version bumps.** There is no standing rule that would require one. The patch-bump-on-content-
change convention this repo once followed was ADR-0006's, and ADR-0015 explicitly retired it as a
dual-manifest artifact: "Conventions that existed only because of hand-authored dual manifests
(ADR-0006's version-parity/patch-bump rule …) are obsolete under `apm.yml`'s single-manifest model
and were deliberately dropped." ADR-0015 also records that apm "has no native version-bump
automation at all", so nothing mechanical demands one either. What remains is the substantive test,
and it is satisfied independently: nothing under `.apm/` is touched here, only compiled artifacts are
removed, so the content every apm consumer receives is byte-identical before and after. This also
**4. No version bumps.** There *is* a standing rule, and it is not triggered here.
`plugins/kyberforge/.apm/skills/apm-workflow/references/configure.md` states it: **bump a package's
own `apm.yml` `version:` whenever anything that reaches its compiled output changes** — either its
`.apm/` content (a new or removed skill/agent/hook, or a substantive edit to one) or its own
manifest metadata (`description`, `keywords`, `author`, `license`, `homepage`, `repository`, all
compiled verbatim into `plugin.json`). This change touches neither: nothing under `.apm/` is edited,
no manifest metadata changes, and only compiled artifacts are removed, so the content every apm
consumer receives is byte-identical before and after. apm also "has no native version-bump
automation at all" (ADR-0015), so nothing mechanical demands one either. This also
avoids triggering the `executables.allow`
`kyberforge#<version>` pin cascade ADR-0019 describes, which would otherwise turn a cleanup into a
multi-file coordinated edit for no functional gain.

View File

@@ -46,9 +46,16 @@ On top of that, `scripts/check-vale-style-sync.sh` (413 lines) and
gate was a copy diff**; it was not, and saying so would overstate the case for deleting it. The
script has **17 assertion sites**: 13 `err` calls and 4 hard-fail exits. Its closing
`exit 1` only reports the `err` count, so it is not an assertion. Count them with
`git show 61b0b9c^:scripts/check-vale-style-sync.sh`. An earlier revision of this ADR said 18. No
`git show 620f20b^:scripts/check-vale-style-sync.sh`. An earlier revision of this ADR said 18. No
reproducible counting rule gives 18, and it is corrected here.
> **Repointed (2026-09-19):** this ADR originally cited `61b0b9c^`. `61b0b9c` is a pre-squash commit
> that no published branch reaches, so the `git show` failed for anyone but its author. `620f20b` is
> the reachable squash of the same work on `docs/simplification-audit`, and `61b0b9c^` and `620f20b^`
> have identical trees (`git diff 61b0b9c^ 620f20b^` is empty), so every figure taken at the old
> parent reproduces at the new one. Note that `620f20b` is not reachable from `origin/main` either —
> fetch the PR branch (`git fetch origin docs/simplification-audit`) before running the command.
| Class | Old line | What it asserted | Now |
|---|---|---|---|
| **Moot (6)** | 19 | `REPO_ROOT` is a directory | nothing to guard; no script |
@@ -217,10 +224,13 @@ it stops pinning that two `validate-provenance.sh` copies of the Contributing-fi
byte-identical, and starts pinning that `lib-contributing-files.sh` is a single sourced copy that has
not been re-inlined into either mode library. The claim it protects is the same one — the parser has
exactly one authority — stated against the new structure. The drift history behind it is smaller than
an earlier revision of this ADR implied. `484357a` (2026-08-30) added the bullet-form parser to both
copies with two different spellings of the loop: a temporary `rest` in skill-audit and an inline
slice in agent-audit. The two were behaviourally identical. `598a7c3` (2026-09-01) unified the
spellings and added the `SHARED CONTRIBUTING-FILES PARSER` markers that 1b hashed. From then until
an earlier revision of this ADR implied. `598a7c3` (2026-09-01, the squash of PR #129) is where the
bullet-form parser landed on a published branch, in both copies, already carrying the
`SHARED CONTRIBUTING-FILES PARSER` markers that 1b hashed. The drift it is named for happened inside
that PR's own history: `484357a` (2026-08-30, pre-squash, not on any published branch) added the
parser with two different spellings of the loop — a temporary `rest` in skill-audit and an inline
slice in agent-audit — which were behaviourally identical, and a later commit on the same branch
unified the spellings before the squash. From then until
the merge's parent the two marker blocks were byte-identical (`md5 0857272d…` both). So the parser
never *parsed* differently. What the gate never covered was the prose around the block, and a
docstring there asserted identity the loop did not have. One sourced library removes the question.
@@ -388,7 +398,8 @@ so the correction is not re-derived from scratch later.**
description content rather than accumulating it: the `Not a skill directory -> skill-audit` clause
loses its referent, and the `"is this ready to ship"` trigger was duplicated verbatim across both.
The two descriptions it replaces measure **239** (skill-audit) and **250** (agent-audit) at
`61b0b9c^`. The description this skill ships measures **241**, inside the 250 SUGGESTION target.
`620f20b^` (see the repointing note above). The description this skill ships measures **241**,
inside the 250 SUGGESTION target.
It carries one arrow per boundary target (`Not applying skill fixes -> skill-author. Not applying
agent fixes -> agent-author.`), because ADR-0020 resolves only the first target after an arrow, so
a one-arrow form would leave `agent-author` checked by nothing. The real blocker was the body: 1,532

View File

@@ -23,7 +23,7 @@ Counting convention: line counts are hand-edited `.apm/` source unless marked "i
>
> > **Re-measured (2026-09-14, at `a6434e0`):** the right-hand column originally read 31,473 / 6,050 / 3,471 / 2,360 / 923 / 2,083 = 46,360 and was labelled "Today" against "the current working tree". It did not reconcile to its own commit's tree — at `061bb3d`, where it was written, the six plugins measured 31,435 / 6,048 / 3,474 / 2,358 / 926 / 2,087 = 46,328 — and "the current working tree" is a basis that goes stale silently. Re-counted at `a6434e0` and the column now names its SHA. The baseline column is confirmed exact against `9eb8bc7`. Commits after `061bb3d` (`c96ca9c`, which deleted the six plugin-root `.mcp.json` files) account for most of the remaining drift.
> **Re-derived (2026-09-16, at HEAD on `docs/simplification-audit`):** the 2026-09-15 notes recording finding 14's merge (~~`467bbd7`~~ → `620f20b`, ADR-0025) and the pipefail fix (~~`4059cb4`~~ → `ffcbed6`) were written without correcting the headlines they annotate, so this pass re-counted every figure those two commits could have moved and corrected each in place above and below. Everything re-measured here came from a command run at HEAD — `git ls-files`, `wc -l`, `grep -c`, and `bash tests/run-tests.sh --strict` — never from an earlier note. What moved: finding 2 (two surviving sync gates → one), the `.pre-commit-config.yaml` hook counts (27/9 → 26/8, then back to 27/9 — see the correction at the end of this note), the skill census (39 → 38 and everything derived from it), finding 11's validator and `sources.md` figures, finding 16's whole numeric basis, and the stale `skill-audit/`, `agent-audit/` and `formatting-and-scripts.md` paths in findings 18, 19 and 33. §1's three rows re-measured: ~~**469**~~ → **471** tracked files (~~465~~ → 467 regular plus the 4 submodule gitlinks) / ~~**74,594**~~ → **75,441** lines (pinned to `c07ca07`; see the note below); `plugins/` ~~**46,106** (62%)~~ → **46,127** (61%); the 38 `SKILL.md` bodies **2,409** (5.2% of plugin lines); enforcement ~~**20 `tests/test-*.sh` totalling 10,189 lines**~~ → ~~**21 `tests/test-*.sh` totalling 10,608 lines**~~ → **19 totalling 10,088** at `baa2f5d`, the two runners **502** (`run-tests.sh` 283 + `run-bats.sh` 219), and `scripts/` ~~**2,901**~~ → **3,139**; kyberforge's validator scripts and their bats tests ~~**5,861**~~ → **5,876** + **6,015** (the merge deduplicated scripts and left the test corpus larger, not smaller — `git ls-files 'plugins/kyberforge/.apm/skills/*/scripts/*.sh'` and `.../tests/*.bats`). `run-tests.sh --strict` reports ~~**20 passed, 0 skipped, 0 failed**~~ → ~~**21 passed, 0 skipped, 0 failed**~~ → **19 passed, 0 skipped, 0 failed** at `baa2f5d` (`4de5b6b` deleted two suites).
> **Re-derived (2026-09-16, at HEAD on `docs/simplification-audit`):** the 2026-09-15 notes recording finding 14's merge (~~`467bbd7`~~ → `620f20b`, ADR-0025) and the pipefail fix (~~`4059cb4`~~ → `ffcbed6`) were written without correcting the headlines they annotate, so this pass re-counted every figure those two commits could have moved and corrected each in place above and below. Everything re-measured here came from a command run at HEAD — `git ls-files`, `wc -l`, `grep -c`, and `bash tests/run-tests.sh --strict` — never from an earlier note. What moved: finding 2 (two surviving sync gates → one), the `.pre-commit-config.yaml` hook counts (27/9 → 26/8 → 27/9 → 26/8, the chain spelled out in §3's table note below; **26** `- id:` entries and **8** `stages: [pre-push]` at HEAD, `grep -c -- "- id:"` and `grep -c "stages: \[pre-push\]"`), the skill census (39 → 38 and everything derived from it), finding 11's validator and `sources.md` figures, finding 16's whole numeric basis, and the stale `skill-audit/`, `agent-audit/` and `formatting-and-scripts.md` paths in findings 18, 19 and 33. §1's three rows re-measured: ~~**469**~~ → **471** tracked files (~~465~~ → 467 regular plus the 4 submodule gitlinks) / ~~**74,594**~~ → **75,441** lines (pinned to `c07ca07`; see the note below); `plugins/` ~~**46,106** (62%)~~ → **46,127** (61%); the 38 `SKILL.md` bodies **2,409** (5.2% of plugin lines); enforcement ~~**20 `tests/test-*.sh` totalling 10,189 lines**~~ → ~~**21 `tests/test-*.sh` totalling 10,608 lines**~~ → **19 totalling 10,088** at `baa2f5d`, the two runners **502** (`run-tests.sh` 283 + `run-bats.sh` 219), and `scripts/` ~~**2,901**~~ → **3,139**; kyberforge's validator scripts and their bats tests ~~**5,861**~~ → **5,876** + **6,015** (the merge deduplicated scripts and left the test corpus larger, not smaller — `git ls-files 'plugins/kyberforge/.apm/skills/*/scripts/*.sh'` and `.../tests/*.bats`). `run-tests.sh --strict` reports ~~**20 passed, 0 skipped, 0 failed**~~ → ~~**21 passed, 0 skipped, 0 failed**~~ → **19 passed, 0 skipped, 0 failed** at `baa2f5d` (`4de5b6b` deleted two suites).
>
> > **Re-measured (2026-09-16, at `c07ca07`):** commit `8451169` added `check-skill-version-bump` — a pre-push hook, `scripts/check-skill-version-bump.sh` (238 lines) and `tests/test-skill-version-bump.sh` (410) — after the figures above were taken, so each was one short. `.pre-commit-config.yaml` now has **27** `- id:` entries and **9** `stages: [pre-push]` (`grep -c -- "- id:"`; `grep -c "stages: \[pre-push\]"`), all nine repo-authored. The struck figures are replaced from these commands. They were run against the working tree, and every figure reproduces exactly from the committed tree at `c07ca07`: `git ls-files | wc -l`; `cat` over every non-gitlink tracked path `| wc -l`; `git ls-files plugins | xargs cat | wc -l`; `git ls-files scripts | xargs wc -l` (no untracked files under `scripts/`); `ls tests/test-*.sh | wc -l` and `cat tests/test-*.sh | wc -l`; `bash tests/run-tests.sh --strict`. The earlier 469 / 74,594 / 46,106 did not reproduce exactly at `8451169^` either (469 / 74,638 / 46,121), so they were taken at an earlier commit than this note's "at HEAD" says. Re-checked and unchanged, so left alone: `docs/research/` inside plugins (19,030) and repo-level `docs/research/` + `docs/notes/` (4,488). Not re-measured, and still carrying their last stated basis: the preload-tax and commit-share rows, §2's timings, and the per-plugin table in the note above.
>
@@ -227,6 +227,8 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
>
> **Re-measured (2026-09-16, at HEAD) — the basis of every figure below changed when ADR-0025 landed; the refutation is unaffected.** There are no longer three validators or two `vale-wrap.sh` copies. The headline's "ported twice" is void, and its `1,677` and `526` no longer name anything. At HEAD: `scripts/skill-size-check.sh` is **1,522** (the note below's 1,517 was correct at `a6434e0`); `factory-audit`'s validator is **2,663** lines across four files (`validate.sh` 255 + `lib-checks-skill.sh` 621 + `lib-checks-agent.sh` 683 + `lib-boundary-resolver.sh` 1,104); `vale-wrap.sh` is **535**, one copy. Validator total **4,185**, of which the resolver is **2,165** (the 1,061-line block still embedded in `skill-size-check.sh`, plus `lib-boundary-resolver.sh`'s 1,104 — the same 1,061 block wrapped in 43 lines of library preamble, which is why the byte-identity test compares the block and not the files). So the resolver is now **52%** of validator lines, not 65%, and **2,020** lines remain once it is excised, not 1,749. Tests: the six repo suites over `skill-size-check.sh` are **3,907** (was 3,619) and the two in-skill validator bats files **2,248** (`validate-skill.bats` 1,029 + `validate-agent.bats` 1,219), for **6,155**, not 5,506. The 200-line target is off by the same order of magnitude it was. (All figures `wc -l`; the resolver block by `awk '/BEGIN ADR-0020 SHARED BOUNDARY RESOLVER/,/END .../'`.)
>
> > **Superseded by `ef27c97` (re-measured 2026-09-19, at HEAD).** The paragraph above is a dated snapshot and its two load-bearing claims no longer hold. `scripts/skill-size-check.sh` is **509** lines, not 1,522 — it shrank by 1,013 — and the resolver is **no longer embedded in it**: `ef27c97` excised the 1,061-line block and the hook now sources `factory-audit`'s `lib-boundary-resolver.sh` by path (`RESOLVER_LIB` at `:483`, `. "$RESOLVER_LIB"` at `:492`), failing closed if the library is missing or defines no resolver. The single remaining `BEGIN ADR-0020 SHARED BOUNDARY RESOLVER` string in the hook is that fail-closed guard, not a copy. `factory-audit`'s four validator files now total **2,671** (`validate.sh` 255 + `lib-checks-skill.sh` 627 + `lib-checks-agent.sh` 685 + `lib-boundary-resolver.sh` 1,104) and `vale-wrap.sh` is **536**. So there is **one** resolver copy repo-wide, not two, and the "resolver is 52% of validator lines" arithmetic above is void along with its inputs. Only the refutation of finding 16 survives all of this unchanged.
>
> 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.
@@ -391,6 +393,7 @@ The shared pattern: per-skill `README.md` files no model reads, a `docs/research
31. [x] ~~**`CONTEXT.md`: 28 terms, most used only by gates.md, scripts, or tests rather than by skills;** two (Preload tax, Skill context contract) are never used outside `CONTEXT.md` and ADR-0020. The preload-tax entry quotes two dated numbers then says not to quote them. The example dialogue and flagged-ambiguities sections are grill residue. Cut to about 20 one-line terms. Effort S.~~
> **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.
> > **Corrected (2026-09-19) — "true orphans" is wrong for two of the nine.** **HOTL** and **Sycophancy** are both still used in `core/ai-constitution.md` (HOTL spelled out at `:111-112`, sycophancy at `:72-87`), and HOTL also in `docs/research/governance_principles/ai-governance-research.md:340-344`. The removals themselves were still right, for a different reason than the one given: the constitution **defines both terms itself, at the point of use**, so a second definition in `CONTEXT.md` was duplication rather than the only authority. Only the orphan justification is corrected here; the other seven and the 28 → 19 count are unaffected.
> **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. [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.~~
@@ -413,10 +416,10 @@ Not covered by the area audits above; found on a final sweep of the root config
>
> Corrected headline: ~~**two** hand-maintained per-plugin locations (**three** for kyberforge)~~ → **one** hand-maintained per-plugin version location, `plugins/<name>/apm.yml` (**two** for kyberforge, adding the `executables.allow` key), not four. `2def060` deleted the root `packages[].version` lines (corrected 2026-09-16, review round). The root `packages[].description:` duplicates dropped in the same round are a separate duplication, not a version location, so they do not change this count — the audit's own "already done" note records the `plugin.json` deletion but never fixed the headline. Gitea skills drift across **six** values (`0.1.2, 0.1.3, 0.1.4, 0.1.5, 0.1.6, 1.0.1`), not five — ~~still six at HEAD on 2026-09-16~~ → **five** again at HEAD (`b426460`) on 2026-09-16 (`0.1.2, 0.1.4, 0.1.5, 0.1.6, 1.0.1`), because `8451169` bumped `gitea-branches` 0.1.3 → 0.1.4 under the new version-bump gate and it was the only skill at 0.1.3; re-derived by parsing `metadata.version` out of each `plugins/gitea/.apm/skills/*/SKILL.md` with PyYAML. ~~39 `SKILL.md` files ✓~~ → **38** carry it, and all 38 do (re-measured 2026-09-16; ADR-0025's merge took one). The `0.4.6` duplication between root `version:` and `marketplace.version:` is **forced by apm, not a repo choice** — deleting `marketplace.version` makes `--check-clean` go dirty.
>
> **"Nothing consumes `metadata.version`" is false twice over.** Machine enforcers: ~~`scripts/skill-size-check.sh:1365-1374`~~ → `scripts/skill-size-check.sh:1370-1379` and ~~`skill-audit/scripts/validate.sh:1292-1332`~~ → `plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-checks-skill.sh:235-277`, both FAIL tier, the latter citing ADR-0022 by name, with four dedicated bats cases and ~10 fixture generators baking the field in.
> **"Nothing consumes `metadata.version`" is false twice over.** Machine enforcers: ~~`scripts/skill-size-check.sh:1365-1374`~~ → ~~`scripts/skill-size-check.sh:1370-1379`~~ → `scripts/skill-size-check.sh:323-335` and ~~`skill-audit/scripts/validate.sh:1292-1332`~~ → `plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-checks-skill.sh:235-283`, both FAIL tier, the latter citing ADR-0022 by name, with four dedicated bats cases and ~10 fixture generators baking the field in.
> Instruction-level consumers: `skill-author/SKILL.md:60` (bump minor on create, patch on improve), `create.md:89,101`, `improve.md:82`, and `forge/SKILL.md:54` + `references/version-bump.md`. apm parses it for Chatmode/Instruction/Context primitives but not for Skills, and never emits it. Precise statement: the value is written, shape-validated, and never read *downstream* — it is an agent-visible revision counter, and the drift table shows the counter is not being maintained.
>
> > **Repointed (2026-09-16, at HEAD):** `skill-audit/scripts/validate.sh` no longer exists — ADR-0025's merge moved the ADR-0022 check into `factory-audit`'s skill-side check library, where it is the `SEMVER_RE` block (comment header at `:235`, `fail()` calls at `:261` and `:275`). `skill-size-check.sh` grew by 5 lines above the block since `062ca47`, hence the shifted range there. All five instruction-level citations still resolve at HEAD, verified with `sed -n`.
> > **Repointed (2026-09-16, at HEAD; re-verified and corrected 2026-09-19):** `skill-audit/scripts/validate.sh` no longer exists — ADR-0025's merge moved the ADR-0022 check into `factory-audit`'s skill-side check library, where it is the `SEMVER_RE` block: comment header at `:235`, `SEMVER_RE` itself at `:254`, `fail()` calls at ~~`:261` and `:275`~~ → `:265` and `:280`, the block running `:235-283` (the next section header, `# SKILL.md size ceilings`, is at `:285`). That library is **627** lines, not 621. In `skill-size-check.sh` the check is at `:323-335`; the earlier note said the file "grew by 5 lines above the block", which is the wrong direction by two orders of magnitude — `ef27c97` excised the embedded resolver and the file **shrank** from 1,522 to **509** lines, which is why the range moved from the 1,300s to the 320s. All five instruction-level citations still resolve at HEAD, verified with `sed -n`.
>
> **ADR-0022 already considered and rejected dropping the field**, on the grounds that `skill-author` depends on it to decide whether a pass owes a bump — a rationale still live today. Superseding costs: rewrite skill-author's bump rule, delete `forge`'s version-bump route premise, strip two scripts, delete four bats cases, fix ~10 fixture generators, edit the scaffold template, update ~~`gates.md:97`~~ → ~~`gates.md:145`~~ → `gates.md:146` — and re-open the "is this field present here?" question issue #127 closed, just from the other side. *(Repointed 2026-09-16, at HEAD `b426460`: the `metadata.version` frontmatter sentence formerly at `gates.md:97` was at `:143-146`, the field itself on `:145`, and is at `:143-147` / `:146` at `4b17703`; verified with `grep -n "metadata.version" docs/spec/gates.md`.)* **Recommendation: keep it and fix the actual defect, which is that nobody bumps it.** Either enforce the bump in the skill-author workflow or declare the values advisory in the ADR.
>
@@ -543,7 +546,7 @@ Recorded here so they are not rediscovered as defects. All follow from commit `7
**Two accepted residuals.**
- **Native install still half-works, and cannot be prevented.** apm reuses Claude's catalogue format by design, so a Claude Code user can still register holocron natively and will install six plugins containing zero skills. Accepted, not overlooked: no schema change closes this, because the format that makes it possible is the format apm's own consumers need.
- **Consumers now receive test fixtures.** apm installs from `.apm/`, which carries the `tests/` directories the mirror used to strip, so a consumer installing from this branch receives **10 `.bats` files across 6 skills**, plus those skills' 6 `tests/README.md` files — 16 files. (Repo-wide, 17 tracked paths contain `/tests/`: the 10 `.bats` and 7 `README.md`, one of which is a template asset under `skill-author/assets/templates/tests/` and is not a test fixture.) This is what consumers *receive*, not what this checkout shows: `.claude/skills/` here currently holds zero `.bats` files, because that deployed tree is stale and predates this branch. The mechanism was confirmed empirically on a ref-pinned consumer clone — the 16 files are absent at the parent commit and present at HEAD. Suppressing them means switching all six `apm.yml` files from `includes: auto` to explicit lists, where a wrong list silently drops content — worse failure mode than the noise. Deferred deliberately.
- **Consumers now receive test fixtures.** apm installs from `.apm/`, which carries the `tests/` directories the mirror used to strip, so a consumer installing from this branch receives **10 `.bats` files across 5 skills**, plus those skills' 5 `tests/README.md` files — ~~16~~ → **15** files. (Repo-wide, ~~17~~ → **16** tracked paths contain `/tests/`: the 10 `.bats` and 6 `README.md`, one of which is a template asset under `skill-author/assets/templates/tests/` and is not a test fixture. Re-counted 2026-09-19 at HEAD with `git ls-files | grep '/tests/'`; the earlier figures predate ADR-0025's merge, which collapsed `skill-audit` and `agent-audit` into one skill and took the skill count from 6 to 5.) This is what consumers *receive*, not what this checkout shows: `.claude/skills/` here currently holds zero `.bats` files, because that deployed tree is stale and predates this branch. The mechanism was confirmed empirically on a ref-pinned consumer clone — the files are absent at the parent commit and present at HEAD. Suppressing them means switching all six `apm.yml` files from `includes: auto` to explicit lists, where a wrong list silently drops content — worse failure mode than the noise. Deferred deliberately.
**Negative result — do not re-litigate.** Deleting native install does *not* relax the self-containment constraint. `plugins/kyberforge/.apm/skills/skill-author/references/deployment-modes.md`, sourced from the agentskills.io spec, states it independently for APM package mode: the spec defines no cross-skill sharing. So ~~findings 14 and 15 still require~~ → finding 14 required *merging* skills (done, ADR-0025), and finding 15 would have too (refuted on measurement, 2026-09-16); sharing one file between two skills remains impossible, and §8's "one-script-per-skill install constraint" bullet is unchanged by this decision.
@@ -604,8 +607,9 @@ The recurring failure mode is worth naming, because it has now produced six wron
### Two defects to fix independently of any finding
- **~~A live bug in always-on context.~~ Fixed (2026-09-15).** The deployed `core/instructions/governance.md` cited `docs/HUMANS.md`, which does not exist — the file is `docs/wiki/HUMANS.md`. Five occurrences across three files (`governance.md:82`, which was self-inconsistent against its own correct line 73; `CONTROLS.md:5,101,106`; `ai-constitution.md:238`), in a file `@`-imported into every session in every project. All five now point at `docs/wiki/HUMANS.md`. ~~Note the deployed copy under `~/.claude/` no longer matches the repo until `scripts/install.sh` re-runs.~~ **Deployed (2026-09-16):** the fixed file was copied to `~/.claude/core/instructions/governance.md` and `diff -rq core ~/.claude/core` is clean. `install.sh` itself was deliberately not run: it overwrites `~/.claude/settings.json` wholesale, and the deployed copy carried machine-local keys (`model`, `extraKnownMarketplaces`, `autoMemoryEnabled`, notification flags) that the repo's `providers/claude-code/settings.json` does not.
- **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.
- **This checkout's install is stale and there is a branch hazard.** At the time of the wave `apm outdated` reported 6 outdated dependencies, 9 commits behind `main`, with a clean tree and nothing reporting it. ~~**Do not run `apm update` on this branch**~~ — it resolves against `main` and restores the obsidian MCP server that commit `c96ca9c` removed here. Reproduced. The mechanism is worse than "reinstalls `plugins/bin/.mcp.json`": apm never writes into `plugins/`, it re-materialises the file under `apm_modules/` and regenerates the repo-root `/.mcp.json` — which `c96ca9c` gitignored, so the restoration would not appear in `git status` at all. This belongs in ADR-0019's Consequences; see finding 34.
> **Landed (2026-09-16):** commit `afcf477` amended ADR-0019's Consequences with the feature-branch hazard, and the discard guidance for a feature branch is now also in `AGENTS.md` and `README.md` (`dd0b923`). The reason those two files and ADR-0019 gave for discarding the lock was wrong, and the review round below corrected it. See finding 34's closing note.
> **Superseded (2026-09-19) — the "do not run `apm update`" instruction above no longer stands.** It was never enforceable and is now contradicted three ways. kyberforge's `SessionStart` hook runs `apm update --yes` on **every** branch, so the command runs on this branch at every session start whether or not anyone types it. ADR-0019's 2026-09-16 amendment considered skipping the refresh off the default branch and **explicitly rejected it**: it would not make the branch live, only freeze the session on an older `main` — the silent staleness the ADR exists to prevent. And `AGENTS.md`'s session rules and `README.md`'s install section now carry the branch-aware guidance that replaces the prohibition: on a feature branch, discard the rewritten lock (`git checkout -- apm.lock.yaml`, then `apm install`), which keeps unrelated lock churn out of the branch diff and keeps `apm pack --check-clean` consistent with the committed lock. **Current guidance: let the refresh run, then discard the lock on a feature branch.** The observation the instruction was built on is untouched and still worth reading — the obsidian server does come back, apm re-materialises it under `apm_modules/` and regenerates the gitignored root `.mcp.json`, and `git status` shows none of it. The redeployed content goes away once the branch merges, and the next `apm update`/`apm install` that resolves a tree no longer declaring the server removes it via `MCPIntegrator.remove_stale`.
## 11. Review round on the grill commits (2026-09-16)

View File

@@ -75,7 +75,7 @@ Both `CLAUDE.md` files are thin adapters: they import from their respective `AGE
This repo also has a `CLAUDE.md` at its root — the Claude Code entry point for working in this repo. It imports `AGENTS.md` and nothing else; there is no `@CONTEXT.md` import. It is not import-only either: below the import sits a fenced `<!-- rtk-instructions v2 -->` … `<!-- /rtk-instructions -->` block carrying the RTK command-prefix convention, which is tool-specific content with no `AGENTS.md` source. This is distinct from `providers/claude-code/CLAUDE.md`, which is the global config deployed to `~/.claude/`.
`CONTEXT.md` is therefore **not** always-loaded. `AGENTS.md` instructs agents to read it at session start, which is a behavioural instruction, not an `@import` guarantee — `LESSONS.md`'s 2026-05-17 entry proposed adding the import and it was never applied. Treat that entry as open work rather than a record of a landed change.
`CONTEXT.md` is therefore **not** always-loaded. `AGENTS.md` instructs agents to read it at session start, which is a behavioural instruction, not an `@import` guarantee.
## Reference conventions
@@ -87,4 +87,4 @@ The stated convention is that files referencing other files declare those refere
## Architectural decisions
Key hard-to-reverse decisions are recorded as ADRs in `docs/adr/`. There is no index file — the directory holds numbered ADRs whose filenames state their decision, so `ls docs/adr/` is the index. Read a superseding ADR before the one it supersedes: ADR-0015 (apm as the authoring source of truth) supersedes ADR-0001 and moots ADR-0006, ADR-0017 corrects ADR-0015's host-discovery gap, and ADR-0019 supersedes one claim in ADR-0018 (that `.claude/settings.json`'s committed content is exactly `{"hooks": {}}`) while keeping the rule behind it. Entry points for the structure described on this page: ADR-0002 (two-tier CLAUDE.md), ADR-0003 (AGENTS.md as the provider-agnostic entry point), ADR-0015 and ADR-0017 (the two compilers behind the plugin roots).
Key hard-to-reverse decisions are recorded as ADRs in `docs/adr/`. There is no index file — the directory holds numbered ADRs whose filenames state their decision, so `ls docs/adr/` is the index. Read a superseding ADR before the one it supersedes: ADR-0015 (apm as the authoring source of truth) supersedes ADR-0001 and moots ADR-0006, ADR-0024 supersedes ADR-0017 (which had corrected ADR-0015's host-discovery gap with a compiled flat content mirror, now deleted), and ADR-0019 supersedes one claim in ADR-0018 (that `.claude/settings.json`'s committed content is exactly `{"hooks": {}}`) while keeping the rule behind it. Entry points for the structure described on this page: ADR-0002 (two-tier CLAUDE.md), ADR-0003 (AGENTS.md as the provider-agnostic entry point), ADR-0015 (`apm pack`, the one compiler behind the plugin roots) and ADR-0024 (apm as the only install path).

View File

@@ -403,10 +403,13 @@ findings.
### Duplicated constants
`factory-audit`'s `validate.sh` holds a second copy of the four ADR-0020 constants
(`DESC_SUGGEST_CHARS` / `DESC_MAX_CHARS` / `BODY_SUGGEST_WORDS` / `BODY_MAX_WORDS`) — the two
description constants apply to both artifact types it handles, the two body constants only to
skills. They are copied rather than imported because a skill's files may not reach outside that skill's own
`factory-audit` holds a second copy of the four ADR-0020 constants
(`DESC_SUGGEST_CHARS` / `DESC_MAX_CHARS` / `BODY_SUGGEST_WORDS` / `BODY_MAX_WORDS`). They are not in
its `validate.sh`, which carries none of them: they live in the mode libraries it sources —
`scripts/lib-checks-skill.sh:313-316` carries all four, and `scripts/lib-checks-agent.sh:164-165`
carries the two description constants only. That split is the contract stated directly: the two
description constants apply to both artifact types `factory-audit` handles, the two body constants
only to skills. They are copied rather than imported because a skill's files may not reach outside that skill's own
directory (the self-contained constraint in `docs/spec/architecture.md`, "Plugin model"), and
`scripts/skill-size-check.sh` does not ship with the plugin. `tests/test-skill-size-check.sh` asserts the copies agree,
so drift fails CI rather than silently letting an audit bless a skill the commit hook then rejects.
@@ -428,9 +431,14 @@ and duplicate the `skill)` arm's, and the file-wide count is still 2 and the ass
with the agent path running no resolver or some other one. It is now a **per-arm structural check** —
each arm of `validate.sh`'s `case "$MODE" in` block must carry exactly one `source` line inside its
own body, and the file must carry exactly those two — with a mutation self-test that performs that
exact count-preserving edit on a copy and requires the check to fail on it. The suite went 25 → 28
cases. It stood at 27 before the 2026-09-16 change and 29 after it, which replaced the two-copy
hash and its line-count floor with the six one-copy assertions above.
exact count-preserving edit on a copy and requires the check to fail on it. The suite's case count
runs **28 → 27 → 29**, and is **29** at HEAD: 28 at `620f20b` (the ADR-0025 merge), 27 after
`4de5b6b` retired the `.pre-commit-hooks.yaml` export, and 29 after `ef27c97` replaced the two-copy
hash and its line-count floor with the six one-copy assertions above. There are two 2026-09-16
changes here, not one, which is what an earlier revision of this section conflated. Each figure is
`bash tests/test-adr0020-contract.sh` run in a worktree at that commit, reading its `Results:` line.
An earlier revision also opened the chain at 25; that predates the branch squash, no reachable
commit reproduces it, and it is dropped as unverifiable rather than carried.
### `python3` and PyYAML are hard requirements
@@ -1030,14 +1038,29 @@ because the root-only invocation audits the marketplace manifest and **nothing e
dependency entry passes `apm pack --check-versions --check-clean --dry-run` and fails
`apm audit --ci` in that package's directory. Costs ~0.5s per package.
It verifies **exactly two things** per manifest and claims no more:
**What it actually runs is asymmetric**, and the two manifest classes are not comparable. Verified by
running `apm audit --ci` (apm 0.28.0) at the repo root and in `plugins/lint/`, reading the check
names straight off its own compliance table:
- **manifest-parse** — each `apm.yml` parses as a valid APM manifest. Unconditional; verified to fire
on a dependency entry missing its `git`/`path`/`registry` field (`Cannot parse apm.yml`).
- **lockfile-exists** — any package declaring dependencies has a consistent `apm.lock.yaml`.
Conditional, and vacuous while every plugin `apm.yml` declares `dependencies: {apm: [], mcp: []}`;
it arms itself the moment one does not (verified by adding a git dependency to
`plugins/lint/apm.yml`).
On the **root** manifest, **10 checks**: `lockfile-exists`, `ref-consistency`,
`deployment-ledger-owners`, `deployed-files-present`, `no-orphaned-packages`,
`skill-subset-consistency`, `config-consistency`, `content-integrity`, `includes-consent`, `drift`.
On each **plugin** manifest, **1 check**: `lockfile-exists`. Conditional, and vacuous while every
plugin `apm.yml` declares `dependencies: {apm: [], mcp: []}` — it reports `No dependencies declared
-- lockfile not required` and arms itself the moment one does not (verified by adding a git
dependency to `plugins/lint/apm.yml`). Everything else in the list above is root-only, because it is
the root install that has a lockfile, a deployment ledger and deployed files to check.
**`manifest-parse` is not a named check** in apm 0.28.0's output, and an earlier revision of this
section listed it as one. Parsing is still enforced — a dependency entry missing its
`git`/`path`/`registry` field fails with `Cannot parse apm.yml` — but it fails the invocation before
the table is built rather than appearing as a row in it.
**The hook needs a completed `apm install`.** `deployed-files-present` checks the install output on
disk, so on a fresh clone it fails with `303 deployed file(s) missing` and takes the push with it.
That is not a defect in the gate; it is the gate correctly reporting that nothing has been installed
yet. Run `apm install` before the first push from a new checkout.
It does **not** enforce an org policy. apm discovers one from the git remote and only understands
github.com and Azure DevOps, so against this repo's self-hosted Gitea remote it prints:
@@ -1052,21 +1075,32 @@ not make the check meaningful, it makes it permanently red — `apm audit --ci`
`No org policy found at unknown (policy.fetch_failure_default=block)` on every push, forever. A gate
that can never go green is not a gate. Revisit only if this repo gains a policy source apm can reach.
It also does not scan for hidden Unicode: that scan is plain `apm audit`, a different mode (`--ci`
refuses to combine with `--file`/`--strip`/`--dry-run`/`PACKAGE`), and plain `apm audit` here reports
`No apm.lock.yaml found -- nothing to scan` and exits 0. Adding it would buy a second vacuous check.
**It does scan for hidden Unicode.** An earlier revision of this section said the opposite. The
`content-integrity` check in the root table *is* that scan — it reports `No critical hidden Unicode
or hash drift detected` — so the root invocation already covers it and nothing needs adding. What
remains true is that the *standalone* mode is different: plain `apm audit` (`--ci` refuses to combine
with `--file`/`--strip`/`--dry-run`/`PACKAGE`) run in a plugin directory reports
`No apm.lock.yaml found -- nothing to scan` and exits 0, because only the root has a lockfile.
Plugin manifests get `lockfile-exists` and nothing else; they are not Unicode-scanned.
### `check-executables-allow-sync`
apm gates a package's `hooks/` and `bin/` on an **exact `<package>#<version>` dictionary lookup** in
root `apm.yml`'s `executables.allow` (`apm_cli/security/executables.py`, `is_package_approved`).
There is no wildcard and no version-less form.
apm gates a package's `hooks/` and `bin/` on root `apm.yml`'s `executables.allow`
(`apm_cli/security/executables.py`). `is_package_approved` is itself an exact dictionary lookup, but
it is never called with a single key: `install/exec_gate.py` builds a candidate list that includes
the version-blind name alongside `<package>#<version>`, and `materialize_exec_map` stores every
approved key **under its version-blind name as well**. `_map_grants` matches the same three ways.
So bumping `plugins/kyberforge/apm.yml`'s `version:` without bumping the key **errors nowhere**: the
entry simply stops matching, the gate blocks the hook, kyberforge's `SessionStart` hook stops
deploying, and the apm install goes quietly stale — the exact failure ADR-0019 exists to end,
reintroduced through the mechanism meant to secure it. ADR-0019 records this as a live failure mode;
the release that shipped the hook hit it immediately.
**Correction (2026-09-19):** verified against apm 0.28.0, a kyberforge version bump therefore does
*not* stop the entry matching — approving `owner/repo#2.0.0` also covers `owner/repo#2.1.0` through
the version-blind alias. The earlier claim here ("no wildcard and no version-less form", so the
entry silently stops matching and the `SessionStart` hook stops deploying) described apm's behaviour
wrongly, and ADR-0019 carries the same correction.
The gate is still required, for a repo-level reason rather than an apm-level one:
`scripts/check-executables-allow-sync.sh` asserts the key matches `plugins/kyberforge/apm.yml`'s
`version:`, so a bump without a key edit fails *this repo's* pre-push, and the key stays an accurate
record of what was approved.
`scripts/check-executables-allow-sync.sh` parses `version:` out of `plugins/kyberforge/apm.yml` and
asserts root `apm.yml` carries the matching `kyberforge#<version>` key. A comment in the
@@ -1092,9 +1126,12 @@ does not deploy and the replay does not compare; shared enforcement belongs in
### Why it is excluded from `pretty-format-json`
It is the **second and last alternation** in that hook's `exclude:` pattern, and the only one there
for a reason other than "generated manifest". Mind which number you are quoting: **two alternations,
expanding to two real files** — `.claude-plugin/marketplace.json`, plus this one.
It is in the **second and last alternation** in that hook's `exclude:` pattern, and that alternation
is the only one there for a reason other than "generated manifest". Mind which number you are
quoting: the pattern is `^(\.claude-plugin/marketplace\.json|\.claude/(settings|apm-hooks)\.json)$`
— **two top-level alternations, expanding to three real tracked files**:
`.claude-plugin/marketplace.json`, this one, and its committed `.claude/apm-hooks.json` sidecar,
which is apm output under the same byte-for-byte replay and is excluded for the same reason.
`pretty-format-json --autofix` sorts object keys unless `--no-sort-keys` is passed, while apm's hook
integrator emits insertion order (`matcher` before `hooks`, `type` before `command`). Leaving the
@@ -1108,11 +1145,18 @@ fix.
## Pushing without a network
No pre-push hook needs the network. Every entry in root `apm.yml`'s `marketplace.packages[]`
resolves from a local `./plugins/<name>` path, so `apm-pack-check-clean` never calls `git ls-remote`.
No pre-push hook needs the network **once `apm install` has populated `apm_modules/`**. Every entry
in root `apm.yml`'s `marketplace.packages[]` resolves from a local `./plugins/<name>` path, so
`apm-pack-check-clean` never calls `git ls-remote`.
`apm-audit-ci` calls `apm` too but was always local: its org-policy discovery resolves nothing on
this remote before any network call.
`apm-audit-ci` calls `apm` too, and its org-policy discovery resolves nothing on this remote before
any network call. But it is local only against a populated install: `drift` and `config-consistency`
replay the install to diff scratch against the working tree, and that replay is cache-only —
`[>] Replaying install (cache-only)` — which is exactly why it costs no network here. On a **fresh
clone** there is no cache to replay from, so the replay clones from the holocron remote and those two
checks fail offline, with `deployed-files-present` already failing for the same reason (see
`apm-audit-ci` above). The offline guarantee is a property of a populated `apm_modules/`, not of the
hook set: run `apm install` once on a new checkout and it holds from then on.
---
@@ -1131,7 +1175,8 @@ this remote before any network call.
styles
- `docs/adr/0025-skill-audit-and-agent-audit-merge-into-factory-audit.md` — the audit-pair merge that
collapsed the two Vale copies to one, removed the `check-vale-style-sync` hook, and took the shared
boundary resolver from three copies to two. It amends ADR-0014 and ADR-0020 on those points
boundary resolver from three copies to two (one since `ef27c97`). It amends ADR-0014 and ADR-0020
on those points
- `docs/spec/architecture.md` — directory structure, install pipeline, what is generated and what is
hand-authored
- `.pre-commit-config.yaml` — the hooks themselves, with inline rationale comments

View File

@@ -55,7 +55,7 @@ When invoked, you:
- remotes: add-remote, remove-remote, rename-remote, set-remote-url, push, pull, fetch
- submodules: add-submodule, init-submodule, update-submodule, sync-submodule, remove-submodule, submodule-status
- **parameters:** object, operation-specific arguments (branch name, commit message, etc.)
- **context:** object (optional), workflow state to carry forward (current_branch, branch_intent, user_config_overrides)
- **context:** object (optional), workflow state to carry forward (current_branch, branch_intent, user_config_overrides). All three are supplied by the caller for this request only — `user_config_overrides` is caller-supplied session state, not a read of any plugin config file; no such file exists and step 4 below is explicit that domain skills infer their conventions rather than reading shared config.
- **confirm:** boolean (optional), explicit confirmation for destructive operations (required if not set for force-push, branch deletion, rebase with history loss, force-checkout)
## Process

View File

@@ -9,7 +9,7 @@ description: >
Not a Gitea remote's branches -> `gitea-branches`.
metadata:
version: "1.0.4"
version: "1.0.5"
category: git
source_keys:
- context7-git-htmldocs

View File

@@ -8,8 +8,8 @@ source_keys:
One command per action. Where two forms exist, the first is the default and the second the escape
hatch.
- **create** — `rtk git switch -c <branch> <base>`. Base comes from the config's `base_branch`
(`main` under GitHub Flow, usually `develop` under Gitflow).
- **create** — `rtk git switch -c <branch> <base>`. Base is `main` under GitHub Flow, or `develop`
when Gitflow is inferred from the repo — see `references/branch-patterns.md`.
- **switch** — `rtk git switch <branch>` moves to an existing local branch; it aborts rather than
clobbering conflicting local changes. `rtk git switch -` returns to the previous branch.
- **delete (local)** — `rtk git branch -d <branch>` refuses when the branch holds unmerged commits,

View File

@@ -9,8 +9,7 @@ source_keys:
Which pattern is in play decides the base branch, the branch name prefix, and whether merges are
allowed to fast-forward. Default to GitHub Flow — simpler, and what CI/CD-oriented repos expect.
Fall back to Gitflow only when the config says so or the repo already carries `develop` or
`release/*` branches.
Fall back to Gitflow only when the repo already carries `develop` or `release/*` branches.
## GitHub Flow

View File

@@ -13,7 +13,7 @@ Request:
{
"action": "create|switch|delete|rename|track|list|get-intent",
"branch": "<branch-name>",
"base": "<base branch, optional, defaults to config>",
"base": "<base branch, optional, defaults to the inferred base branch>",
"intent": "<human-readable intent, optional>",
"confirm": "<true for destructive ops, omit for read ops>"
}

View File

@@ -8,7 +8,7 @@ description: >
Not branch lifecycle -> `git-branches`.
metadata:
version: "0.1.6"
version: "0.1.7"
category: git
source_keys:
- conventional-commits-spec

View File

@@ -9,7 +9,7 @@ source_keys:
1. **Gather context** — what changed and why, from the staged diff, the PR description, or the issue. Confirm the staged diff is one logical, independently reviewable and reversible change that leaves the repository buildable and testable. If it bundles unrelated work, suggest splitting it before going further.
2. **Determine the type** — read it off the change itself: a new user-visible feature is `feat`, a bug fix is `fix`. For the full 11-type set and each type's SemVer impact, read `references/conventional-commits-spec.md`.
3. **Determine the scope** — use the scope from plugin config where one is set, otherwise infer it from the files changed (`api`, `db`, `cli`, `config`). Scope is optional, but it identifies which part of the system moved and is worth setting.
3. **Determine the scope** — infer it from the files changed (`api`, `db`, `cli`, `config`). Scope is optional, but it identifies which part of the system moved and is worth setting.
4. **Write the description** — imperative mood, no trailing period: "add user authentication", "fix race condition in cache". Neither source spec sets a target below the 100-character header maximum, but convention favours roughly 50 characters so `git log --oneline` stays readable.
5. **Add a body when the change is non-trivial** — blank line first, wrapped at 100 characters. Explain *why*, not what: the diff already shows what changed, and the message's job is the context the diff cannot carry — motivation, root cause, tradeoffs. Follow the Why / Implementation Notes / Impact structure in `references/commit-template.md`.
6. **Add footers where they apply** — `Fixes: #123`, `Refs: #123`, `ADR: 0012`, `Co-authored-by: Name <email>`, `BREAKING CHANGE: description`. For the full trailer list, read `references/commit-template.md`.

View File

@@ -19,7 +19,7 @@ metadata:
- gitea-mcp-slim-go
- context7-websites-gitea
- context7-gitea-tea-cli
version: "0.1.4"
version: "0.1.5"
allowed-tools: Bash mcp__gitea__list_pull_requests mcp__gitea__pull_request_read mcp__gitea__pull_request_write mcp__gitea__pull_request_review_write
---
@@ -27,6 +27,7 @@ allowed-tools: Bash mcp__gitea__list_pull_requests mcp__gitea__pull_request_read
## Gotchas
- **Issues and PRs share one number space.** `#42` may be an issue rather than a PR. When unsure, call `pull_request_read method: "get"` and read a 404 as "that number is an issue" — hand it to `gitea-issues`.
- **404 may also mean 403.** Gitea masks permission errors as not-found, so a 404 is only evidence of an issue-not-PR once `write:repository` scope is confirmed — check the token scope before reporting a PR missing or handing the number to `gitea-issues`.
- **`pull_request_write method: "create"` discards most optional parameters in silence.** `milestone`, `assignee`, `assignees`, `reviewers` and `team_reviewers` are accepted, dropped, and left out of the response, so a drop is indistinguishable from never passing them. `labels` *does* apply on `"create"`, so labels landing is no evidence the milestone did.
## Step 1 — Resolve owner and repo

View File

@@ -56,12 +56,21 @@ emit() {
# is churn unrelated to the branch and should be discarded. The branch name only
# selects between fixed strings and is never interpolated. Outside a git checkout,
# or on a detached HEAD, the neutral advice stands.
#
# So does an UNSET origin/HEAD, which is the common state: git only writes it on
# clone, and `git remote add` never does. The fallback here used to be `main`,
# which is a guess, and it is wrong in exactly the repos that would notice — a
# checkout whose default branch is `master` was told "this is a feature branch,
# so discard it" while standing on its default branch, i.e. told to throw away a
# real lock update. There is no cheap way to learn the remote's default without
# the network, so nothing is asserted: the advice stays neutral and the reader
# decides.
lock_advice="commit it or discard it deliberately."
current_branch="$(git symbolic-ref --short -q HEAD 2> /dev/null || true)"
if [[ -n "$current_branch" ]]; then
default_branch="$(git symbolic-ref --short -q refs/remotes/origin/HEAD 2> /dev/null || true)"
default_branch="${default_branch#origin/}"
if [[ "$current_branch" == "${default_branch:-main}" ]]; then
default_branch="$(git symbolic-ref --short -q refs/remotes/origin/HEAD 2> /dev/null || true)"
default_branch="${default_branch#origin/}"
if [[ -n "$current_branch" && -n "$default_branch" ]]; then
if [[ "$current_branch" == "$default_branch" ]]; then
lock_advice="this is the default branch, so commit it or discard it deliberately."
else
lock_advice="this is a feature branch, so discard it: git checkout -- apm.lock.yaml && apm install"

View File

@@ -6,7 +6,7 @@ description: >
Not read-only review -> `factory-audit`. Not agent files -> `agent-author`.
allowed-tools: Bash Read Write Edit
metadata:
version: "1.0.2"
version: "1.0.3"
category: factory
source_keys:
- agentskills-home

View File

@@ -40,7 +40,7 @@ These variables are injected when the plugin is loaded from an install cache. Th
| `${CLAUDE_PLUGIN_ROOT}` | Absolute path to the plugin's install directory. Changes on update. |
| `${CLAUDE_PLUGIN_DATA}` | Persistent directory that survives updates. Use for `node_modules`, generated state, caches. |
Use `${CLAUDE_PLUGIN_ROOT}` only in hook commands and `.mcp.json` configs — not in SKILL.md body text, since standalone deployments won't have it.
Use `${CLAUDE_PLUGIN_ROOT}` only in hook commands — not in SKILL.md body text, since standalone deployments won't have it.
## Standalone mode

View File

@@ -4,7 +4,8 @@
Use this for CLI tools, helper scripts, or MCP server entry points bundled with the plugin.
Reference files in this directory from `.mcp.json` or hooks using `${CLAUDE_PLUGIN_ROOT}/bin/<file>`.
Reference files in this directory from a hook command in `.apm/hooks/hooks.json` using
`${CLAUDE_PLUGIN_ROOT}/bin/<file>`.
The `${CLAUDE_PLUGIN_ROOT}` variable resolves to the plugin's install cache path at runtime —
do not use relative paths from the repo root, as they will break after install.

View File

@@ -6,16 +6,29 @@ set -euo pipefail
# is the gate that holds the rule, since skill-size-check only checks presence
# and shape.
#
# Baseline: `git merge-base <main> <pushed commit>`, where <main> is origin/main
# when it resolves and the local `main` branch otherwise. Readers install
# skills from main, so "changed" means changed relative to what main ships, not
# relative to the remote branch's current tip. Diffing from PRE_COMMIT_FROM_REF
# would let the second push of a feature branch excuse a change the first push
# already carried unbumped. The check runs on every push whatever the target
# branch — nothing here reads PRE_COMMIT_REMOTE_BRANCH — so it also runs under
# a manual `pre-commit run --hook-stage pre-push` (against HEAD, since no
# PRE_COMMIT_TO_REF is set). A missing bump is cheapest to fix on the branch,
# before review.
# Baseline: `git merge-base --all <main> <pushed commit>`, where <main> is
# origin/main when it resolves and the local `main` branch otherwise. Readers
# install skills from main, so "changed" means changed relative to what main
# ships, not relative to the remote branch's current tip. Diffing from
# PRE_COMMIT_FROM_REF would let the second push of a feature branch excuse a
# change the first push already carried unbumped. The check runs on every push
# whatever the target branch — nothing here reads PRE_COMMIT_REMOTE_BRANCH — so
# it also runs under a manual `pre-commit run --hook-stage pre-push` (against
# HEAD, since no PRE_COMMIT_TO_REF is set). A missing bump is cheapest to fix on
# the branch, before review.
#
# `--all`, not the single base git would otherwise pick for it. A criss-cross
# history — main merges a branch while that branch merges a main commit — has
# TWO merge bases, and which one `git merge-base` prints is an implementation
# detail. Picking one made the verdict a coin flip: a skill byte-identical to
# main's tip was still reported "not above merge-base" whenever the losing base
# happened to be chosen, so an already-merged bump failed the push it should
# have passed. So a skill counts as CHANGED only when it differs from EVERY
# base — differing from none of them, or from only some, means one base already
# carries the pushed content — and a changed skill's version must exceed the
# version at every base it exists at. Both directions are conservative: the
# intersection cannot exempt a skill that genuinely changed since all of main's
# reachable history, and requiring every base keeps the ratchet.
#
# Second baseline: the tip of that same <main> ref. A changed skill's pushed
# version must exceed its version there too (ADR-0022, second 2026-09-16
@@ -134,38 +147,70 @@ if [[ -z "$MAIN_REF" ]]; then
exit 1
fi
if ! BASELINE="$(git merge-base "$MAIN_REF" "$PUSHED_COMMIT" 2>/dev/null)"; then
BASES=()
while IFS= read -r base; do
[[ -n "$base" ]] && BASES+=("$base")
done < <(git merge-base --all "$MAIN_REF" "$PUSHED_COMMIT" 2>/dev/null || true)
if [[ ${#BASES[@]} -eq 0 ]]; then
echo "FAIL: no merge-base between $MAIN_REF and $PUSHED_REF, so there is no baseline to compare skill versions against." >&2
echo " Fix: ensure full history is available (e.g. git fetch --unshallow) and retry." >&2
exit 1
fi
if [[ "$MAIN_REF" == "main" && "$BASELINE" == "$PUSHED_COMMIT" ]]; then
echo "FAIL: origin/main does not resolve and $PUSHED_REF is already contained in local main, so local main cannot serve as an independent baseline — the diff would be empty by construction." >&2
echo " Fix: git fetch origin main and retry." >&2
exit 1
# A pushed commit that is an ancestor of <main> is itself the only merge-base,
# so this fires on exactly the case it always did.
if [[ "$MAIN_REF" == "main" ]]; then
for base in ${BASES[@]+"${BASES[@]}"}; do
if [[ "$base" == "$PUSHED_COMMIT" ]]; then
echo "FAIL: origin/main does not resolve and $PUSHED_REF is already contained in local main, so local main cannot serve as an independent baseline — the diff would be empty by construction." >&2
echo " Fix: git fetch origin main and retry." >&2
exit 1
fi
done
fi
CHANGED_FILE="$(mktemp)"
trap 'rm -f "$CHANGED_FILE"' EXIT
if ! git diff -z --no-renames --name-only "$BASELINE" "$PUSHED_COMMIT" -- plugins > "$CHANGED_FILE"; then
echo "FAIL: could not diff $BASELINE..$PUSHED_REF (see git error above)." >&2
exit 1
fi
SKILL_PATH_RE='^(plugins/[^/]+/\.apm/skills/[^/]+)/(.+)$'
SKILL_DIRS=()
while IFS= read -r -d '' path; do
[[ "$path" =~ $SKILL_PATH_RE ]] || continue
[[ "${BASH_REMATCH[2]}" == tests/* ]] && continue
dir="${BASH_REMATCH[1]}"
seen=false
for existing in ${SKILL_DIRS[@]+"${SKILL_DIRS[@]}"}; do
[[ "$existing" == "$dir" ]] && { seen=true; break; }
# skill_dirs_at <base>: sets SKILL_DIRS_ONE to the skill directories differing
# between <base> and the pushed commit.
skill_dirs_at() {
local path dir existing seen
SKILL_DIRS_ONE=()
if ! git diff -z --no-renames --name-only "$1" "$PUSHED_COMMIT" -- plugins > "$CHANGED_FILE"; then
echo "FAIL: could not diff $1..$PUSHED_REF (see git error above)." >&2
exit 1
fi
while IFS= read -r -d '' path; do
[[ "$path" =~ $SKILL_PATH_RE ]] || continue
[[ "${BASH_REMATCH[2]}" == tests/* ]] && continue
dir="${BASH_REMATCH[1]}"
seen=false
for existing in ${SKILL_DIRS_ONE[@]+"${SKILL_DIRS_ONE[@]}"}; do
[[ "$existing" == "$dir" ]] && { seen=true; break; }
done
$seen || SKILL_DIRS_ONE+=("$dir")
done < "$CHANGED_FILE"
}
# Intersected across every base: a skill matching any ONE base is already
# shipped by that base and has nothing left to bump.
skill_dirs_at "${BASES[0]}"
SKILL_DIRS=(${SKILL_DIRS_ONE[@]+"${SKILL_DIRS_ONE[@]}"})
for ((i = 1; i < ${#BASES[@]}; i++)); do
[[ ${#SKILL_DIRS[@]} -eq 0 ]] && break
skill_dirs_at "${BASES[i]}"
KEPT=()
for dir in ${SKILL_DIRS[@]+"${SKILL_DIRS[@]}"}; do
for existing in ${SKILL_DIRS_ONE[@]+"${SKILL_DIRS_ONE[@]}"}; do
[[ "$existing" == "$dir" ]] && { KEPT+=("$dir"); break; }
done
done
$seen || SKILL_DIRS+=("$dir")
done < "$CHANGED_FILE"
SKILL_DIRS=(${KEPT[@]+"${KEPT[@]}"})
done
[[ ${#SKILL_DIRS[@]} -eq 0 ]] && exit 0
@@ -238,35 +283,64 @@ in_tree() {
OFFENDERS=()
for dir in ${SKILL_DIRS[@]+"${SKILL_DIRS[@]}"}; do
at_base=false
# Bases the skill exists at, in merge-base order, with the version read at
# each. Index-matched arrays rather than one map: bash 3.2 has no `declare -A`.
base_at=()
base_vers=()
first_base_ver=""
for base in ${BASES[@]+"${BASES[@]}"}; do
in_tree "$base" "$dir/SKILL.md" || continue
version_at "$base" "$dir/SKILL.md"
base_at+=("$base")
base_vers+=("$VERSION")
[[ -n "$first_base_ver" ]] || first_base_ver="$VERSION"
done
# When main has not moved since a merge-base, the tip IS that baseline and the
# skill is checked once.
at_tip=false
in_tree "$BASELINE" "$dir/SKILL.md" && at_base=true
# When main has not moved since the merge-base, the tip is the same baseline.
[[ "$MAIN_TIP" != "$BASELINE" ]] && in_tree "$MAIN_TIP" "$dir/SKILL.md" && at_tip=true
# Absent at both baselines: new, renamed-to, or merged-into. Exempt.
$at_base || $at_tip || continue
tip_is_base=false
for base in ${BASES[@]+"${BASES[@]}"}; do
if [[ "$MAIN_TIP" == "$base" ]]; then
tip_is_base=true
break
fi
done
if [[ "$tip_is_base" == false ]] && in_tree "$MAIN_TIP" "$dir/SKILL.md"; then
at_tip=true
fi
# Absent at every baseline: new, renamed-to, or merged-into. Exempt.
[[ ${#base_at[@]} -gt 0 ]] || $at_tip || continue
# Directory absent at pushed commit: deleted or renamed-from. Exempt.
[[ "$(git cat-file -t "$PUSHED_COMMIT:$dir" 2>/dev/null)" == "tree" ]] || continue
base_ver=""
tip_ver=""
if $at_base; then version_at "$BASELINE" "$dir/SKILL.md"; base_ver="$VERSION"; fi
if $at_tip; then version_at "$MAIN_TIP" "$dir/SKILL.md"; tip_ver="$VERSION"; fi
if ! in_tree "$PUSHED_COMMIT" "$dir/SKILL.md"; then
OFFENDERS+=("$dir: SKILL.md missing at $PUSHED_REF (baseline: ${base_ver:-none})")
OFFENDERS+=("$dir: SKILL.md missing at $PUSHED_REF (baseline: ${first_base_ver:-none})")
continue
fi
version_at "$PUSHED_COMMIT" "$dir/SKILL.md"
cur_ver="$VERSION"
if [[ -z "$cur_ver" ]]; then
OFFENDERS+=("$dir: metadata.version missing or not MAJOR.MINOR.PATCH at $PUSHED_REF (baseline: ${base_ver:-none})")
OFFENDERS+=("$dir: metadata.version missing or not MAJOR.MINOR.PATCH at $PUSHED_REF (baseline: ${first_base_ver:-none})")
continue
fi
if [[ -n "$base_ver" ]] && ! semver_gt "$cur_ver" "$base_ver"; then
OFFENDERS+=("$dir: $base_ver -> $cur_ver (not above merge-base)")
fi
# Named by sha only when there is more than one base to tell apart; a
# criss-cross history is the only case where "which merge-base" is a question
# the reader cannot answer from the branch alone.
for ((i = 0; i < ${#base_at[@]}; i++)); do
base_ver="${base_vers[i]}"
[[ -n "$base_ver" ]] || continue
semver_gt "$cur_ver" "$base_ver" && continue
if [[ ${#BASES[@]} -gt 1 ]]; then
OFFENDERS+=("$dir: $base_ver -> $cur_ver (not above merge-base ${base_at[i]})")
else
OFFENDERS+=("$dir: $base_ver -> $cur_ver (not above merge-base)")
fi
done
if [[ -n "$tip_ver" ]] && ! semver_gt "$cur_ver" "$tip_ver"; then
OFFENDERS+=("$dir: $tip_ver -> $cur_ver (not above $MAIN_REF tip)")
fi

View File

@@ -169,6 +169,67 @@ done < <(
| sort
)
# Discovering NOTHING is never a clean run, and it used to be the quietest
# possible pass: the loops below iterate zero times, nothing is printed between
# the bats block and the summary, and `Summary: 0 passed, 0 failed` exits 0 --
# under --strict too, because strictness only ever turned SKIPS into failures
# and there were no suites to skip. A wrong TEST_DIR, a mistyped `find` pattern,
# an exclusion that grew to swallow tests/, and a gutted checkout all land here.
# tests/run-bats.sh has carried this guard for its own .bats discovery; this is
# the same guard one file over, and the run-tests pre-push hook is the caller
# that needs it.
#
# Two checks, in the same order and for the same reasons as run-bats.sh's.
# First, the derived one: every test-*.sh in the git index must have been
# discovered. The direction matters -- a discovered file need NOT be tracked
# (work in progress is ordinary), and a file removed with `git rm` leaves the
# index, so a deliberate removal passes while an accidental disappearance
# fails. It only runs when SEARCH_ROOT is itself the git worktree root, which
# is what keeps it off the mktemp fixture trees in tests/test-run-tests.sh --
# those hold one or two test-*.sh files by design and git resolves no worktree
# for them. The same exclusions are reapplied to the index listing so both
# sides cover the same universe.
EXPECTED_SCRIPTS=()
GIT_TOPLEVEL="$(git -C "$SEARCH_ROOT" rev-parse --show-toplevel 2> /dev/null || true)"
if [[ -n "$GIT_TOPLEVEL" && "$GIT_TOPLEVEL" == "$SEARCH_ROOT" ]]; then
while IFS= read -r f; do
[[ -n "$f" ]] && EXPECTED_SCRIPTS+=("$SEARCH_ROOT/$f")
done < <(
git -C "$SEARCH_ROOT" ls-files -- 'test-*.sh' '*/test-*.sh' \
| grep -Ev '(^|/)\.claude/worktrees/|(^|/)apm_modules/|(^|/)\.claude/skills/' \
| sort || true
)
fi
if [[ ${#EXPECTED_SCRIPTS[@]} -gt 0 ]]; then
MISSING_SCRIPTS=()
for expected in ${EXPECTED_SCRIPTS[@]+"${EXPECTED_SCRIPTS[@]}"}; do
found=false
for actual in ${SCRIPTS[@]+"${SCRIPTS[@]}"}; do
if [[ "$actual" == "$expected" ]]; then
found=true
break
fi
done
[[ "$found" == true ]] || MISSING_SCRIPTS+=("${expected#"$SEARCH_ROOT"/}")
done
if [[ ${#MISSING_SCRIPTS[@]} -gt 0 ]]; then
echo "Error: ${#MISSING_SCRIPTS[@]} of ${#EXPECTED_SCRIPTS[@]} tracked test-*.sh file(s) were not discovered under $SEARCH_ROOT — they were deleted without being removed from the index, or the search path/exclusions above no longer reach them:" >&2
for m in ${MISSING_SCRIPTS[@]+"${MISSING_SCRIPTS[@]}"}; do
echo " $m" >&2
done
exit 1
fi
fi
# Second, unconditional and separate: a tree with nothing tracked (a tarball
# export, a fresh scaffold) still must not run on an empty set and call it
# green.
if [[ ${#SCRIPTS[@]} -eq 0 ]]; then
echo "Error: found 0 test-*.sh file(s) under $SEARCH_ROOT — the search path is wrong or the suite has been gutted" >&2
exit 1
fi
# Each test-*.sh is independent (fixtures live under its own mktemp dir, none
# write back into the live repo tree -- verified before adding this), so they
# run concurrently in fixed-size batches instead of one at a time. Dispatch and

View File

@@ -441,6 +441,231 @@ else
fail "validate.sh agent mode exited $SILENT_RC with output '${SILENT_OUT:-<empty>}' — the original defect was exit 0 and total silence on a blocking pre-push gate"
fi
# ---------------------------------------------------------------------------
# 3. The REQUIRED-FIELD checks, folded in from the deleted `skill-frontmatter`
# ---------------------------------------------------------------------------
# Commit c8a7c9e retired the standalone `skill-frontmatter` hook and moved its
# two presence checks — `name` non-empty, `metadata.version` present and
# three-part semver — into skill-size-check.sh, beside the ADR-0020 gates. The
# hook's own suite went with it, and only the leading-zero shape was left
# covered (tests/test-skill-size-check.sh). Measured: mutating the
# missing-version ERROR to a no-op left every suite in the repo green. These
# cases are that behaviour pinned back down, on the same fixtures the deleted
# suite used.
#
# Probed against the HOOK alone, deliberately. validate.sh's skill mode has its
# own metadata.version check with its own wording, and its agent mode has none
# at all — ADR-0022 binds skills, not agents — so probe_all's "all three must
# agree" contract does not hold for this family and asserting it would be
# asserting something the ADRs contradict.
#
# The four grep defects the deleted suite named are kept as cases because the
# YAML-parsed implementation must not regress into any of them: a `metadata:`
# block quoted in the BODY, a `version:` under a following `source:` list, a
# deeper-indented `version:`, and the mirror image — a `version:` far down a
# long metadata block, which the old `-A10` grep reported MISSING.
# write_required <name> <frontmatter> [body] — a SKILL.md whose only interesting
# property is its frontmatter. The description and body sit well inside every
# ADR-0020 ceiling, so a finding here is the required-field check and nothing
# else; a fixture that also tripped a ceiling would satisfy "exits non-zero" for
# the wrong reason.
write_required() {
local name="$1" frontmatter="$2" body="${3:-Body text.}"
local dir="$TMPDIR_T/required/$name"
mkdir -p "$dir"
{
printf -- '---\n'
printf '%s\n' "$frontmatter"
printf -- '---\n\n'
printf '%s\n' "$body"
} > "$dir/SKILL.md"
printf '%s' "$dir/SKILL.md"
}
# write_required_raw <name> <whole-file> — for the shapes that must NOT have a
# closing marker written for them.
write_required_raw() {
local name="$1"
local dir="$TMPDIR_T/required/$name"
mkdir -p "$dir"
printf '%s' "$2" > "$dir/SKILL.md"
printf '%s' "$dir/SKILL.md"
}
REQ_DESC='Use when probing the required-field checks. Do not use for anything else.'
# require_finding <label> <file> <needle> — non-zero exit AND the named message.
# The needle is the message, not the exit code: the mutation this case exists to
# catch turns the ERROR into a no-op, and a file that also failed some other gate
# would still exit non-zero with the check gone.
require_finding() {
local label="$1" file="$2" needle="$3" out status=0
set +e
out="$(bash "$HOOK" "$file" 2>&1)"
status=$?
set -e
if [[ $status -eq 0 ]]; then
fail "$label — the hook exited 0: ${out:-<no output>}"
elif [[ "$out" != *"$needle"* ]]; then
fail "$label — the hook failed but never said '$needle': $out"
else
pass "$label"
fi
}
# require_clean <label> <file> — exits 0 with no finding at all.
require_clean() {
local label="$1" file="$2" out status=0
set +e
out="$(bash "$HOOK" "$file" 2>&1)"
status=$?
set -e
if [[ $status -eq 0 ]]; then
pass "$label"
else
fail "$label — expected exit 0, got $status: ${out:-<no output>}"
fi
}
echo ""
echo "--- control: a SKILL.md carrying both required fields passes ---"
# Without this, every case below could be passing because the fixture generator
# is broken rather than because the checks fire.
require_clean "a well-formed name + metadata.version passes" \
"$(write_required valid "name: valid
description: $REQ_DESC
metadata:
version: \"1.0.0\"")"
echo ""
echo "--- metadata.version missing, in each of the shapes that used to satisfy the old grep ---"
require_finding "no metadata block at all is reported missing" \
"$(write_required no-metadata "name: no-metadata
description: $REQ_DESC")" \
"metadata.version field is missing"
require_finding "a metadata block with other keys but no version is reported missing" \
"$(write_required metadata-no-version "name: metadata-no-version
description: $REQ_DESC
metadata:
author: someone")" \
"metadata.version field is missing"
# `version:` with no value parses to None, which is absent, not malformed —
# reporting it as a bad VALUE would send the author looking for a typo in a
# value that is not there.
require_finding "a valueless 'version:' is reported missing, not malformed" \
"$(write_required empty-version "name: empty-version
description: $REQ_DESC
metadata:
version:")" \
"metadata.version field is missing"
# skill-author's own docs quote a metadata block verbatim; under the old
# whole-file grep that quotation satisfied the check for the file quoting it.
require_finding "a metadata block quoted in the BODY does not satisfy the check" \
"$(write_required fenced-metadata "name: fenced-metadata
description: $REQ_DESC" '# Fenced
Skills declare their version like this:
```yaml
metadata:
version: "1.0.0"
```')" \
"metadata.version field is missing"
# `-A10` ran ten lines past `metadata:` regardless of where the block ended, and
# write-docs and research both carry a `source:` list immediately after it.
require_finding "a version: belonging to a following source[] does not satisfy the check" \
"$(write_required source-list "name: source-list
description: $REQ_DESC
metadata:
author: someone
source:
- name: upstream
version: \"2.3.4\"")" \
"metadata.version field is missing"
# `grep -q \" version:\"` was an unanchored substring match, so any indentation
# of two spaces or more matched.
require_finding "a four-space-indented version: one level deeper does not satisfy the check" \
"$(write_required deep-indent "name: deep-indent
description: $REQ_DESC
metadata:
provenance:
version: \"1.0.0\"")" \
"metadata.version field is missing"
# The mirror image, and the reason this one asserts a PASS: the old grep's
# ten-line window reported a real version missing once the block grew past it.
require_clean "a version: thirteen lines into the metadata block is found" \
"$(write_required long-metadata "name: long-metadata
description: $REQ_DESC
metadata:
a: 1
b: 2
c: 3
d: 4
e: 5
f: 6
g: 7
h: 8
i: 9
j: 10
k: 11
version: \"1.0.0\"")"
echo ""
echo "--- present is not well formed: a non-semver metadata.version is its own finding ---"
# plugins/bin/.apm/skills/write-docs/SKILL.md carried `version: "1.0"` through a
# whole PR under a presence-only check: present, well-nested, and not a version.
# The value is quoted back so the author does not have to guess which key.
require_finding "'1.0' is rejected as malformed and the message quotes it" \
"$(write_required two-part "name: two-part
description: $REQ_DESC
metadata:
version: \"1.0\"")" \
"metadata.version is malformed ('1.0')"
require_finding "'latest' is rejected as malformed and the message quotes it" \
"$(write_required word-version "name: word-version
description: $REQ_DESC
metadata:
version: latest")" \
"metadata.version is malformed ('latest')"
echo ""
echo "--- the name field is required and must not be empty ---"
require_finding "an absent name is reported" \
"$(write_required no-name "description: $REQ_DESC
metadata:
version: \"1.0.0\"")" \
"name field is missing or empty"
require_finding "an empty name is reported" \
"$(write_required empty-name "name: \"\"
description: $REQ_DESC
metadata:
version: \"1.0.0\"")" \
"name field is missing or empty"
echo ""
echo "--- a file whose frontmatter block cannot be read reports THAT, not a missing field ---"
# The required-field checks run downstream of the frontmatter match, so a file
# with no readable block must land on the parse error rather than being reported
# as a skill that merely forgot its version — and must never report green.
require_finding "an unterminated frontmatter block is a parse error" \
"$(write_required_raw unterminated "---
name: unterminated
description: $REQ_DESC
metadata:
version: \"1.0.0\"
Body text.
")" \
"no parseable YAML frontmatter block"
require_finding "an empty '---/---' block is a parse error" \
"$(write_required_raw empty-block "---
---
Body text.
")" \
"no parseable YAML frontmatter block"
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]

View File

@@ -150,6 +150,23 @@ if command -v git > /dev/null 2>&1; then
commit -q --allow-empty -m init
touch "$REPO/apm.lock.yaml"
# origin/HEAD is unset here — git writes it on clone and `git remote add` does
# not — so the hook cannot know what the default branch is and must not guess.
# It used to assume `main`, which is why the `master` case further down was a
# live defect.
out="$(run_hook_in "$REPO" "$REPO")"
advice="$(advice_of "$out")"
grep -q "commit it or discard it deliberately" <<< "$advice" \
&& pass "with origin/HEAD unset, keeps the neutral lock advice" \
|| fail "with origin/HEAD unset the advice should stay neutral: $advice"
# Needles are the two DECISION phrases, not the bare words: the fixed prefix
# of every notice already says "behind the remote default branch".
grep -qE "this is (the default|a feature) branch, so" <<< "$advice" \
&& fail "with origin/HEAD unset the hook must not claim to know which branch this is: $advice" \
|| pass "with origin/HEAD unset, claims nothing about which branch this is"
git -C "$REPO" update-ref refs/remotes/origin/main HEAD
git -C "$REPO" symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/main
out="$(run_hook_in "$REPO" "$REPO")"
grep -q "default branch, so commit it or discard it deliberately" <<< "$(advice_of "$out")" \
&& pass "on main, says to commit or discard the lock deliberately" \
@@ -174,6 +191,26 @@ if command -v git > /dev/null 2>&1; then
grep -q "default branch, so commit it" <<< "$(advice_of "$out")" \
&& pass "reads the default branch from origin/HEAD when it is set" \
|| fail "should treat origin/HEAD's branch as the default: $(advice_of "$out")"
# The regression the `${default_branch:-main}` fallback caused: a repo whose
# default branch is `master`, with origin/HEAD unset (no clone wrote it), was
# standing on its DEFAULT branch and was told to discard the lock as feature
# churn. Assuming `main` is the only way to reach that verdict, so the case is
# pinned on the branch name that makes the assumption wrong.
MASTER_REPO="$WORK/master-repo"
mkdir -p "$MASTER_REPO"
git -C "$MASTER_REPO" init -q -b master
git -C "$MASTER_REPO" -c user.email=probe@example.invalid -c user.name=probe \
commit -q --allow-empty -m init
touch "$MASTER_REPO/apm.lock.yaml"
out="$(run_hook_in "$MASTER_REPO" "$MASTER_REPO")"
advice="$(advice_of "$out")"
grep -qF "feature branch, so discard it" <<< "$advice" \
&& fail "on master with origin/HEAD unset the hook assumed main and told the reader to discard a real lock update: $advice" \
|| pass "on master with origin/HEAD unset, does not misread the default branch as a feature branch"
grep -q "commit it or discard it deliberately" <<< "$advice" \
&& pass "on master with origin/HEAD unset, falls back to the neutral lock advice" \
|| fail "on master with origin/HEAD unset the advice should be neutral: $advice"
else
echo " (git not on PATH — branch-specific advice cases not run)"
fi

View File

@@ -703,6 +703,74 @@ else
pass "a root under .claude/worktrees/ runs its own suites and skips nested worktrees"
fi
# --- 13. Discovering ZERO test-*.sh files is a hard error, not a green run ---
# The quietest vacuous pass in the dispatcher, and the one --strict did not
# reach: strictness only ever turned SKIPS into failures, so with no suites to
# skip there was nothing for it to act on. The loops iterated zero times, the
# run printed `Summary: 0 passed, 0 skipped, 0 failed` and exited 0 -- from the
# run-tests pre-push hook, which prints nothing for a passing hook, that is
# indistinguishable from every suite passing. A wrong TEST_DIR, a mistyped find
# pattern, an exclusion that grew to swallow tests/, and a gutted checkout all
# land here. tests/run-bats.sh has guarded its own discovery this way since it
# hit the same hole; this is the sibling.
#
# The bats runner is the healthy stub, so the only thing wrong with the fixture
# is that it has no case scripts -- a failure here cannot be the bats leg.
echo ""
echo "--- discovering zero test-*.sh files fails the run and names the search root ---"
DIR13="$(make_fake_repo)"
FIXTURES+=("$DIR13")
install_healthy_bats_runner "$DIR13"
run_fake "$DIR13"
if [[ $FAKE_RC -eq 0 ]]; then
fail "an empty search root exited 0 — a run that verified nothing reports the same as a run that verified everything: $FAKE_OUT"
elif ! grep -q "found 0 test-\*\.sh file(s)" <<< "$FAKE_OUT"; then
fail "an empty search root failed without saying that nothing was discovered: $FAKE_OUT"
elif ! grep -qF "$DIR13/cases" <<< "$FAKE_OUT"; then
fail "the zero-discovery error did not name the root it searched, which is the one fact needed to fix it: $FAKE_OUT"
elif grep -q "^=== Summary:" <<< "$FAKE_OUT"; then
fail "an empty search root still printed a summary line, so a reader scanning for the verdict sees a green one: $FAKE_OUT"
else
pass "zero discovered test-*.sh files fails the run, names the search root, and prints no summary"
fi
# And under --strict too. Asserted separately because the ONLY lever --strict
# had was the skip list, so "it fails now" and "it fails under the gate's own
# invocation" were genuinely different questions here.
echo ""
echo "--- ... and under --strict, which previously had no lever on this at all ---"
DIR13B="$(make_fake_repo)"
FIXTURES+=("$DIR13B")
install_healthy_bats_runner "$DIR13B"
run_fake "$DIR13B" --strict
if [[ $FAKE_RC -eq 0 ]]; then
fail "an empty search root exited 0 under --strict, the invocation the run-tests pre-push hook uses: $FAKE_OUT"
elif grep -q "found 0 test-\*\.sh file(s)" <<< "$FAKE_OUT"; then
pass "--strict also fails a run that discovered nothing"
else
fail "--strict failed an empty search root for some other reason: $FAKE_OUT"
fi
# The control: one discovered case is enough to get past the guard, so the two
# cases above fail on the count and not on something else the fixture lacks.
echo ""
echo "--- ... while a single discovered case still passes ---"
DIR13C="$(make_fake_repo)"
FIXTURES+=("$DIR13C")
install_healthy_bats_runner "$DIR13C"
add_case "$DIR13C" test-one.sh <<'EOF'
#!/usr/bin/env bash
echo "one case ran"
EOF
run_fake "$DIR13C"
if [[ $FAKE_RC -ne 0 ]]; then
fail "a fixture with exactly one case failed: $FAKE_OUT"
elif grep -q "found 0 test-\*\.sh file(s)" <<< "$FAKE_OUT"; then
fail "the zero-discovery guard fired on a root that has one case script: $FAKE_OUT"
else
pass "one discovered case script is enough to get past the zero-discovery guard"
fi
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]

View File

@@ -588,6 +588,51 @@ expect_fail "unbumped skill with leading blank lines is held to its baseline ver
write_skill "$F" demo alpha 'version: "1.0.1"' "lead body 2"; blank_lead "$F" alpha; commit "$F"
expect_pass "bumped skill with leading blank lines passes" "$F"
echo ""
echo "--- 40. a criss-cross history is judged against EVERY merge-base ---"
# Two merge bases, and which one plain `git merge-base` prints is git's choice,
# not a property of the history. The gate used to take that single answer, so
# the verdict turned on it: here `git diff main feature -- plugins` is EMPTY
# (main already carries the bump, via its merge of the feature branch) and the
# push still failed with "not above main tip", because the base git picked was
# the one that predates the bump. `--all` plus the intersection rule makes the
# answer the same whichever base git would have named.
#
# C0 alpha 1.0.0
# +-- feature: F1 bumps alpha to 1.0.1
# +-- main: M1 unrelated, then M2 merges F1
# feature: F2 merges M1 -> merge bases {M1, F1}
F="$(mktemp -d)"; CLEANUP_DIRS+=("$F")
(cd "$F" && git init -q -b main && git config user.email t@t.t && git config user.name t)
write_skill "$F" demo alpha 'version: "1.0.0"'; commit "$F" C0
(cd "$F" && git checkout -q -b feature)
write_skill "$F" demo alpha 'version: "1.0.1"' "new body"; commit "$F" F1
F1_SHA="$(cd "$F" && git rev-parse HEAD)"
(cd "$F" && git checkout -q main)
echo unrelated > "$F/m1.txt"; commit "$F" M1
M1_SHA="$(cd "$F" && git rev-parse HEAD)"
(cd "$F" && git merge -q --no-edit "$F1_SHA" -m M2 > /dev/null)
(cd "$F" && git checkout -q feature && git merge -q --no-edit "$M1_SHA" -m F2 > /dev/null)
BASES40="$(cd "$F" && git merge-base --all main feature | sort)"
if [[ "$(printf '%s\n' "$BASES40" | wc -l)" -eq 2 ]]; then
pass "fixture check: the history really does have two merge bases"
else
fail "fixture check: expected two merge bases, got: $BASES40"
fi
if [[ -z "$(cd "$F" && git diff main feature -- plugins)" ]]; then
pass "fixture check: nothing under plugins/ differs between main and the branch"
else
fail "fixture check: plugins/ differs between main and the branch, so this is not the case under test"
fi
expect_pass "a skill identical to main's tip passes whichever merge-base git would pick" "$F"
# And the ratchet still holds on the same shape: a further edit with no bump
# differs from BOTH bases, so it is not excused by the criss-cross.
write_skill "$F" demo alpha 'version: "1.0.1"' "later body"; commit "$F" F3
expect_fail "an unbumped edit on a criss-cross branch still fails, naming the baseline sha" \
"alpha: 1\.0\.1 -> 1\.0\.1 \(not above merge-base [0-9a-f]{40}\)" "$F"
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]]

View File

@@ -1783,7 +1783,14 @@ fi
# merge took out with the script, and cases 28-30 cannot backstop it. They key
# on `Kyberforge.VagueWording` and `KyberforgeCopilot.ProactivePhrase`, so
# DescriptionOpener, PaddingPhrase, SentenceOpenerThereIs and CompositionNote
# can each be retired underneath a passing probe.
# are invisible to them.
#
# Which is a statement about the LEVEL of each rule, and only that. It used to
# read as though those four rules were uncovered outright, and they were: case
# 35 at the end of this file is what closed that, enumerating the style
# directories at run time and demanding an alert from every rule it finds. The
# two cases are complementary and neither subsumes the other — 35 proves a rule
# still matches text, this one proves the match is still blocking.
#
# The original's comments, verbatim:
#
@@ -2364,6 +2371,124 @@ EOF_MUT34
fi
fi
# --- 35. Every shipped rule actually fires on a fixture ---------------------
#
# The last coverage class the deleted scripts/check-vale-style-sync.sh and its
# consumer suite took with them (ADR-0025). Cases 28-31 keep a rule LOADED, at
# `error`, and in scope; none of them asks whether the rule still MATCHES
# anything. Measured: rewriting CompositionNote.yml's tokens so they match no
# text left this suite at 63/63 passed — the rule shipped, was loaded, was
# blocking, and was inert.
#
# The rule list is discovered from the style directories at run time, never
# hardcoded, and a discovered rule with no fixture row is a FAILURE rather than
# a silent skip. That direction is the one that decays: a hardcoded list lets
# rule #7 ship uncovered, and a fixture table read as "check the rows I have"
# does exactly the same.
VALE_ASSETS35="$FACTORY_AUDIT/assets/vale"
# `<Style>.<Rule>` for every rule file under styles/Kyberforge*/, which is how
# vale itself names an alert.
discovered_rules35() {
local dir rule style name
for dir in "$VALE_ASSETS35"/styles/Kyberforge*/; do
[[ -d "$dir" ]] || continue
style="${dir%/}"
style="${style##*/}"
for rule in "$dir"*.yml; do
[[ -f "$rule" ]] || continue
name="${rule##*/}"
printf '%s.%s\n' "$style" "${name%.yml}"
done
done | sort
}
# `<Style>.<Rule>|<path>|<description>|<body>`. The path decides which [glob]
# section of .vale.ini applies, so KyberforgeCopilot's row has to be an
# .agent.md file — that style is loaded nowhere else (ADR-0013). Each fixture
# carries exactly the one trigger its rule is about; the rest of the text is
# deliberately clean, so an alert for the wrong rule cannot satisfy the row.
RULE_FIXTURES35="$(
cat << 'EOF_FIX35'
Kyberforge.CompositionNote|composition/SKILL.md|Use when the caller wants a cross-cutting probe. Do not use for anything else.|Body text.
Kyberforge.DescriptionOpener|opener/SKILL.md|This is the description opener under test. Do not use for anything else.|Body text.
Kyberforge.PaddingPhrase|padding/SKILL.md|Use when the caller wants a probe. Do not use for anything else.|See references for more info.
Kyberforge.SentenceOpenerThereIs|sentence-opener/SKILL.md|Use when the caller wants a probe. Do not use for anything else.|There is a defect here.
Kyberforge.VagueWording|vague/SKILL.md|Use when the caller helps with a probe. Do not use for anything else.|Body text.
KyberforgeCopilot.ProactivePhrase|proactive/probe.agent.md|Use when the caller wants a probe. Use proactively.|Body text.
EOF_FIX35
)"
echo ""
echo "--- every shipped Vale rule has a fixture, and every fixture names a shipped rule (no vale needed) ---"
DISCOVERED35="$(discovered_rules35)"
UNCOVERED35=""
STALE_FIXTURES35=""
while IFS= read -r RULE35; do
[[ -n "$RULE35" ]] || continue
FOUND35=false
while IFS='|' read -r FID35 _; do
[[ "$FID35" == "$RULE35" ]] && { FOUND35=true; break; }
done <<EOF_COV35
$RULE_FIXTURES35
EOF_COV35
[[ "$FOUND35" == true ]] || UNCOVERED35+="$RULE35 "
done <<EOF_RULES35
$DISCOVERED35
EOF_RULES35
while IFS='|' read -r FID35 _; do
[[ -n "$FID35" ]] || continue
grep -qxF "$FID35" <<< "$DISCOVERED35" || STALE_FIXTURES35+="$FID35 "
done <<EOF_STALE35
$RULE_FIXTURES35
EOF_STALE35
if [[ -z "$DISCOVERED35" ]]; then
fail "no rule files were found under $VALE_ASSETS35/styles/Kyberforge*/ — either the styles were gutted or this discovery no longer reaches them, and every assertion below would be vacuous"
elif [[ -n "$UNCOVERED35" ]]; then
fail "shipped Vale rule(s) have no fixture row, so nothing proves they still match anything: $UNCOVERED35"
elif [[ -n "$STALE_FIXTURES35" ]]; then
fail "fixture row(s) name a rule that no longer ships, so those rows prove nothing about the live styles: $STALE_FIXTURES35"
else
pass "all $(printf '%s\n' "$DISCOVERED35" | wc -l | tr -d ' ') shipped rule(s) have a fixture row, and every row names a live rule"
fi
if [[ "$VALE_READY" == true ]]; then
echo ""
echo "--- and each of those fixtures actually raises its own rule's alert ---"
TREE35="$(mktemp -d)"
new_fixture "$TREE35"
INERT35=""
while IFS='|' read -r RID35 RPATH35 RDESC35 RBODY35; do
[[ -n "$RID35" ]] || continue
mkdir -p "$TREE35/$(dirname "$RPATH35")"
{
echo "---"
echo "name: probe"
echo "description: $RDESC35"
echo "---"
echo ""
echo "$RBODY35"
} > "$TREE35/$RPATH35"
# vale exits non-zero merely for HAVING alerts, which is the expected
# outcome for every row here, hence the `|| true`.
REPORT35="$( { (cd "$TREE35" && vale --config "$VALE_ASSETS35/.vale.ini" "$RPATH35" 2>&1) | sed -E 's/\x1b\[[0-9;]*m//g'; } || true)"
if ! grep -qE 'in [0-9]+ files?\.' <<< "$REPORT35"; then
INERT35+="[$RID35: vale printed no summary line for $RPATH35, so it did not run: ${REPORT35:-<no output>}] "
elif ! grep -qF "$RID35" <<< "$REPORT35"; then
INERT35+="[$RID35 raised no alert on its own fixture: ${REPORT35:-<no output>}] "
fi
done <<EOF_FIRE35
$RULE_FIXTURES35
EOF_FIRE35
if [[ -n "$INERT35" ]]; then
fail "a shipped rule matched nothing on the fixture written for it — it is loaded and blocking but inert, which is indistinguishable from a passing file: $INERT35"
else
pass "every shipped Vale rule raises its own alert on the fixture written for it"
fi
fi
echo ""
echo "Results: $PASS passed, $FAIL failed"
[[ $FAIL -eq 0 ]] || exit 1