Compare commits
4 Commits
e62188c3c3
...
1614bcef23
| Author | SHA1 | Date | |
|---|---|---|---|
| 1614bcef23 | |||
| 82b7bbcf5c | |||
| 3920dfab20 | |||
| ea119d83b0 |
@@ -215,7 +215,7 @@ repos:
|
|||||||
- id: skill-size-check
|
- id: skill-size-check
|
||||||
stages: ['pre-commit']
|
stages: ['pre-commit']
|
||||||
name: SKILL.md size and context-budget ceilings
|
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
|
entry: scripts/skill-size-check.sh
|
||||||
language: script
|
language: script
|
||||||
files: '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$'
|
files: '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$'
|
||||||
|
|||||||
@@ -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`.
|
- **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.
|
- **`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.
|
- **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.
|
- **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
|
## Key documents
|
||||||
|
|||||||
15
apm.yml
15
apm.yml
@@ -36,10 +36,17 @@ dependencies:
|
|||||||
# executables deploy" until an `executables:` block exists.
|
# executables deploy" until an `executables:` block exists.
|
||||||
#
|
#
|
||||||
# kyberforge ships the SessionStart hook that keeps this install level with the
|
# 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
|
# remote (ADR-0019). The `#2.0.0` suffix below is cosmetic as far as apm is
|
||||||
# kyberforge version bump makes this entry stop matching and the hook stops
|
# concerned: grants are version-BLIND in apm 0.28.0. `_map_grants`
|
||||||
# deploying until the version here is bumped too. If skills silently go stale
|
# (apm_cli/security/executables.py) matches the exact key, the version-blind
|
||||||
# after a kyberforge release, check this first.
|
# 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:
|
executables:
|
||||||
allow:
|
allow:
|
||||||
kyberforge#2.0.0:
|
kyberforge#2.0.0:
|
||||||
|
|||||||
@@ -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
|
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
|
CC-vs-Copilot field-placement split, dual-file mirroring) are obsolete under `apm.yml`'s
|
||||||
single-manifest model and were deliberately dropped.
|
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
|
- **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 —
|
(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
|
`apm` doesn't enforce it, and has no native version-bump automation at all — so rather than
|
||||||
|
|||||||
@@ -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
|
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
|
(`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
|
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
|
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
|
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
|
`scripts/check-executables-allow-sync.sh` runs at pre-push, parsing `version:` out of
|
||||||
|
|||||||
@@ -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.
|
`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
|
**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
|
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
|
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
|
to read `dependencies.apm` when resolving the boundary-target universe — none of them reads the
|
||||||
|
|||||||
@@ -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
|
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.
|
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-
|
**4. No version bumps.** There *is* a standing rule, and it is not triggered here.
|
||||||
change convention this repo once followed was ADR-0006's, and ADR-0015 explicitly retired it as a
|
`plugins/kyberforge/.apm/skills/apm-workflow/references/configure.md` states it: **bump a package's
|
||||||
dual-manifest artifact: "Conventions that existed only because of hand-authored dual manifests
|
own `apm.yml` `version:` whenever anything that reaches its compiled output changes** — either its
|
||||||
(ADR-0006's version-parity/patch-bump rule …) are obsolete under `apm.yml`'s single-manifest model
|
`.apm/` content (a new or removed skill/agent/hook, or a substantive edit to one) or its own
|
||||||
and were deliberately dropped." ADR-0015 also records that apm "has no native version-bump
|
manifest metadata (`description`, `keywords`, `author`, `license`, `homepage`, `repository`, all
|
||||||
automation at all", so nothing mechanical demands one either. What remains is the substantive test,
|
compiled verbatim into `plugin.json`). This change touches neither: nothing under `.apm/` is edited,
|
||||||
and it is satisfied independently: nothing under `.apm/` is touched here, only compiled artifacts are
|
no manifest metadata changes, and only compiled artifacts are removed, so the content every apm
|
||||||
removed, so the content every apm consumer receives is byte-identical before and after. This also
|
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`
|
avoids triggering the `executables.allow`
|
||||||
`kyberforge#<version>` pin cascade ADR-0019 describes, which would otherwise turn a cleanup into a
|
`kyberforge#<version>` pin cascade ADR-0019 describes, which would otherwise turn a cleanup into a
|
||||||
multi-file coordinated edit for no functional gain.
|
multi-file coordinated edit for no functional gain.
|
||||||
|
|||||||
@@ -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
|
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
|
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
|
`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.
|
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 |
|
| Class | Old line | What it asserted | Now |
|
||||||
|---|---|---|---|
|
|---|---|---|---|
|
||||||
| **Moot (6)** | 19 | `REPO_ROOT` is a directory | nothing to guard; no script |
|
| **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
|
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
|
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
|
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
|
an earlier revision of this ADR implied. `598a7c3` (2026-09-01, the squash of PR #129) is where the
|
||||||
copies with two different spellings of the loop: a temporary `rest` in skill-audit and an inline
|
bullet-form parser landed on a published branch, in both copies, already carrying the
|
||||||
slice in agent-audit. The two were behaviourally identical. `598a7c3` (2026-09-01) unified the
|
`SHARED CONTRIBUTING-FILES PARSER` markers that 1b hashed. The drift it is named for happened inside
|
||||||
spellings and added the `SHARED CONTRIBUTING-FILES PARSER` markers that 1b hashed. From then until
|
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
|
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
|
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.
|
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
|
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.
|
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
|
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
|
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
|
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
|
a one-arrow form would leave `agent-author` checked by nothing. The real blocker was the body: 1,532
|
||||||
|
|||||||
@@ -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-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.
|
> > **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 .../'`.)
|
> **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.
|
> 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.
|
> 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.~~
|
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 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).
|
> **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.~~
|
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.
|
> 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.
|
> 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.
|
> **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.**
|
**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.
|
- **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.
|
**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
|
### 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.
|
- **~~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.
|
> **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)
|
## 11. Review round on the grill commits (2026-09-16)
|
||||||
|
|
||||||
|
|||||||
@@ -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/`.
|
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
|
## Reference conventions
|
||||||
|
|
||||||
@@ -87,4 +87,4 @@ The stated convention is that files referencing other files declare those refere
|
|||||||
|
|
||||||
## Architectural decisions
|
## 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).
|
||||||
|
|||||||
@@ -403,10 +403,13 @@ findings.
|
|||||||
|
|
||||||
### Duplicated constants
|
### Duplicated constants
|
||||||
|
|
||||||
`factory-audit`'s `validate.sh` holds a second copy of the four ADR-0020 constants
|
`factory-audit` holds a second copy of the four ADR-0020 constants
|
||||||
(`DESC_SUGGEST_CHARS` / `DESC_MAX_CHARS` / `BODY_SUGGEST_WORDS` / `BODY_MAX_WORDS`) — the two
|
(`DESC_SUGGEST_CHARS` / `DESC_MAX_CHARS` / `BODY_SUGGEST_WORDS` / `BODY_MAX_WORDS`). They are not in
|
||||||
description constants apply to both artifact types it handles, the two body constants only to
|
its `validate.sh`, which carries none of them: they live in the mode libraries it sources —
|
||||||
skills. They are copied rather than imported because a skill's files may not reach outside that skill's own
|
`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
|
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,
|
`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.
|
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** —
|
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
|
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
|
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
|
exact count-preserving edit on a copy and requires the check to fail on it. The suite's case count
|
||||||
cases. It stood at 27 before the 2026-09-16 change and 29 after it, which replaced the two-copy
|
runs **28 → 27 → 29**, and is **29** at HEAD: 28 at `620f20b` (the ADR-0025 merge), 27 after
|
||||||
hash and its line-count floor with the six one-copy assertions above.
|
`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
|
### `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
|
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.
|
`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 the **root** manifest, **10 checks**: `lockfile-exists`, `ref-consistency`,
|
||||||
on a dependency entry missing its `git`/`path`/`registry` field (`Cannot parse apm.yml`).
|
`deployment-ledger-owners`, `deployed-files-present`, `no-orphaned-packages`,
|
||||||
- **lockfile-exists** — any package declaring dependencies has a consistent `apm.lock.yaml`.
|
`skill-subset-consistency`, `config-consistency`, `content-integrity`, `includes-consent`, `drift`.
|
||||||
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
|
On each **plugin** manifest, **1 check**: `lockfile-exists`. Conditional, and vacuous while every
|
||||||
`plugins/lint/apm.yml`).
|
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
|
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:
|
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
|
`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.
|
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`
|
**It does scan for hidden Unicode.** An earlier revision of this section said the opposite. The
|
||||||
refuses to combine with `--file`/`--strip`/`--dry-run`/`PACKAGE`), and plain `apm audit` here reports
|
`content-integrity` check in the root table *is* that scan — it reports `No critical hidden Unicode
|
||||||
`No apm.lock.yaml found -- nothing to scan` and exits 0. Adding it would buy a second vacuous check.
|
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`
|
### `check-executables-allow-sync`
|
||||||
|
|
||||||
apm gates a package's `hooks/` and `bin/` on an **exact `<package>#<version>` dictionary lookup** in
|
apm gates a package's `hooks/` and `bin/` on root `apm.yml`'s `executables.allow`
|
||||||
root `apm.yml`'s `executables.allow` (`apm_cli/security/executables.py`, `is_package_approved`).
|
(`apm_cli/security/executables.py`). `is_package_approved` is itself an exact dictionary lookup, but
|
||||||
There is no wildcard and no version-less form.
|
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
|
**Correction (2026-09-19):** verified against apm 0.28.0, a kyberforge version bump therefore does
|
||||||
entry simply stops matching, the gate blocks the hook, kyberforge's `SessionStart` hook stops
|
*not* stop the entry matching — approving `owner/repo#2.0.0` also covers `owner/repo#2.1.0` through
|
||||||
deploying, and the apm install goes quietly stale — the exact failure ADR-0019 exists to end,
|
the version-blind alias. The earlier claim here ("no wildcard and no version-less form", so the
|
||||||
reintroduced through the mechanism meant to secure it. ADR-0019 records this as a live failure mode;
|
entry silently stops matching and the `SessionStart` hook stops deploying) described apm's behaviour
|
||||||
the release that shipped the hook hit it immediately.
|
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
|
`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
|
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`
|
### 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
|
It is in the **second and last alternation** in that hook's `exclude:` pattern, and that alternation
|
||||||
for a reason other than "generated manifest". Mind which number you are quoting: **two alternations,
|
is the only one there for a reason other than "generated manifest". Mind which number you are
|
||||||
expanding to two real files** — `.claude-plugin/marketplace.json`, plus this one.
|
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
|
`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
|
integrator emits insertion order (`matcher` before `hooks`, `type` before `command`). Leaving the
|
||||||
@@ -1108,11 +1145,18 @@ fix.
|
|||||||
|
|
||||||
## Pushing without a network
|
## Pushing without a network
|
||||||
|
|
||||||
No pre-push hook needs the network. Every entry in root `apm.yml`'s `marketplace.packages[]`
|
No pre-push hook needs the network **once `apm install` has populated `apm_modules/`**. Every entry
|
||||||
resolves from a local `./plugins/<name>` path, so `apm-pack-check-clean` never calls `git ls-remote`.
|
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
|
`apm-audit-ci` calls `apm` too, and its org-policy discovery resolves nothing on this remote before
|
||||||
this remote before any network call.
|
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
|
styles
|
||||||
- `docs/adr/0025-skill-audit-and-agent-audit-merge-into-factory-audit.md` — the audit-pair merge that
|
- `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
|
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
|
- `docs/spec/architecture.md` — directory structure, install pipeline, what is generated and what is
|
||||||
hand-authored
|
hand-authored
|
||||||
- `.pre-commit-config.yaml` — the hooks themselves, with inline rationale comments
|
- `.pre-commit-config.yaml` — the hooks themselves, with inline rationale comments
|
||||||
|
|||||||
@@ -55,7 +55,7 @@ When invoked, you:
|
|||||||
- remotes: add-remote, remove-remote, rename-remote, set-remote-url, push, pull, fetch
|
- 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
|
- submodules: add-submodule, init-submodule, update-submodule, sync-submodule, remove-submodule, submodule-status
|
||||||
- **parameters:** object, operation-specific arguments (branch name, commit message, etc.)
|
- **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)
|
- **confirm:** boolean (optional), explicit confirmation for destructive operations (required if not set for force-push, branch deletion, rebase with history loss, force-checkout)
|
||||||
|
|
||||||
## Process
|
## Process
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ description: >
|
|||||||
Not a Gitea remote's branches -> `gitea-branches`.
|
Not a Gitea remote's branches -> `gitea-branches`.
|
||||||
|
|
||||||
metadata:
|
metadata:
|
||||||
version: "1.0.4"
|
version: "1.0.5"
|
||||||
category: git
|
category: git
|
||||||
source_keys:
|
source_keys:
|
||||||
- context7-git-htmldocs
|
- context7-git-htmldocs
|
||||||
|
|||||||
@@ -8,8 +8,8 @@ source_keys:
|
|||||||
One command per action. Where two forms exist, the first is the default and the second the escape
|
One command per action. Where two forms exist, the first is the default and the second the escape
|
||||||
hatch.
|
hatch.
|
||||||
|
|
||||||
- **create** — `rtk git switch -c <branch> <base>`. Base comes from the config's `base_branch`
|
- **create** — `rtk git switch -c <branch> <base>`. Base is `main` under GitHub Flow, or `develop`
|
||||||
(`main` under GitHub Flow, usually `develop` under Gitflow).
|
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
|
- **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.
|
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,
|
- **delete (local)** — `rtk git branch -d <branch>` refuses when the branch holds unmerged commits,
|
||||||
|
|||||||
@@ -9,8 +9,7 @@ source_keys:
|
|||||||
|
|
||||||
Which pattern is in play decides the base branch, the branch name prefix, and whether merges are
|
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.
|
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
|
Fall back to Gitflow only when the repo already carries `develop` or `release/*` branches.
|
||||||
`release/*` branches.
|
|
||||||
|
|
||||||
## GitHub Flow
|
## GitHub Flow
|
||||||
|
|
||||||
|
|||||||
@@ -13,7 +13,7 @@ Request:
|
|||||||
{
|
{
|
||||||
"action": "create|switch|delete|rename|track|list|get-intent",
|
"action": "create|switch|delete|rename|track|list|get-intent",
|
||||||
"branch": "<branch-name>",
|
"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>",
|
"intent": "<human-readable intent, optional>",
|
||||||
"confirm": "<true for destructive ops, omit for read ops>"
|
"confirm": "<true for destructive ops, omit for read ops>"
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ description: >
|
|||||||
Not branch lifecycle -> `git-branches`.
|
Not branch lifecycle -> `git-branches`.
|
||||||
|
|
||||||
metadata:
|
metadata:
|
||||||
version: "0.1.6"
|
version: "0.1.7"
|
||||||
category: git
|
category: git
|
||||||
source_keys:
|
source_keys:
|
||||||
- conventional-commits-spec
|
- conventional-commits-spec
|
||||||
|
|||||||
@@ -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.
|
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`.
|
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.
|
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`.
|
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`.
|
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`.
|
||||||
|
|||||||
@@ -19,7 +19,7 @@ metadata:
|
|||||||
- gitea-mcp-slim-go
|
- gitea-mcp-slim-go
|
||||||
- context7-websites-gitea
|
- context7-websites-gitea
|
||||||
- context7-gitea-tea-cli
|
- 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
|
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
|
## 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`.
|
- **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.
|
- **`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
|
## Step 1 — Resolve owner and repo
|
||||||
|
|||||||
@@ -56,12 +56,21 @@ emit() {
|
|||||||
# is churn unrelated to the branch and should be discarded. The branch name only
|
# 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,
|
# selects between fixed strings and is never interpolated. Outside a git checkout,
|
||||||
# or on a detached HEAD, the neutral advice stands.
|
# 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."
|
lock_advice="commit it or discard it deliberately."
|
||||||
current_branch="$(git symbolic-ref --short -q HEAD 2> /dev/null || true)"
|
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="$(git symbolic-ref --short -q refs/remotes/origin/HEAD 2> /dev/null || true)"
|
default_branch="${default_branch#origin/}"
|
||||||
default_branch="${default_branch#origin/}"
|
if [[ -n "$current_branch" && -n "$default_branch" ]]; then
|
||||||
if [[ "$current_branch" == "${default_branch:-main}" ]]; then
|
if [[ "$current_branch" == "$default_branch" ]]; then
|
||||||
lock_advice="this is the default branch, so commit it or discard it deliberately."
|
lock_advice="this is the default branch, so commit it or discard it deliberately."
|
||||||
else
|
else
|
||||||
lock_advice="this is a feature branch, so discard it: git checkout -- apm.lock.yaml && apm install"
|
lock_advice="this is a feature branch, so discard it: git checkout -- apm.lock.yaml && apm install"
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ description: >
|
|||||||
Not read-only review -> `factory-audit`. Not agent files -> `agent-author`.
|
Not read-only review -> `factory-audit`. Not agent files -> `agent-author`.
|
||||||
allowed-tools: Bash Read Write Edit
|
allowed-tools: Bash Read Write Edit
|
||||||
metadata:
|
metadata:
|
||||||
version: "1.0.2"
|
version: "1.0.3"
|
||||||
category: factory
|
category: factory
|
||||||
source_keys:
|
source_keys:
|
||||||
- agentskills-home
|
- agentskills-home
|
||||||
|
|||||||
@@ -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_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. |
|
| `${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
|
## Standalone mode
|
||||||
|
|
||||||
|
|||||||
@@ -4,7 +4,8 @@
|
|||||||
|
|
||||||
Use this for CLI tools, helper scripts, or MCP server entry points bundled with the plugin.
|
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 —
|
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.
|
do not use relative paths from the repo root, as they will break after install.
|
||||||
|
|
||||||
|
|||||||
@@ -6,16 +6,29 @@ set -euo pipefail
|
|||||||
# is the gate that holds the rule, since skill-size-check only checks presence
|
# is the gate that holds the rule, since skill-size-check only checks presence
|
||||||
# and shape.
|
# and shape.
|
||||||
#
|
#
|
||||||
# Baseline: `git merge-base <main> <pushed commit>`, where <main> is origin/main
|
# Baseline: `git merge-base --all <main> <pushed commit>`, where <main> is
|
||||||
# when it resolves and the local `main` branch otherwise. Readers install
|
# origin/main when it resolves and the local `main` branch otherwise. Readers
|
||||||
# skills from main, so "changed" means changed relative to what main ships, not
|
# install skills from main, so "changed" means changed relative to what main
|
||||||
# relative to the remote branch's current tip. Diffing from PRE_COMMIT_FROM_REF
|
# ships, not relative to the remote branch's current tip. Diffing from
|
||||||
# would let the second push of a feature branch excuse a change the first push
|
# PRE_COMMIT_FROM_REF would let the second push of a feature branch excuse a
|
||||||
# already carried unbumped. The check runs on every push whatever the target
|
# change the first push already carried unbumped. The check runs on every push
|
||||||
# branch — nothing here reads PRE_COMMIT_REMOTE_BRANCH — so it also runs under
|
# whatever the target branch — nothing here reads PRE_COMMIT_REMOTE_BRANCH — so
|
||||||
# a manual `pre-commit run --hook-stage pre-push` (against HEAD, since no
|
# it also runs under a manual `pre-commit run --hook-stage pre-push` (against
|
||||||
# PRE_COMMIT_TO_REF is set). A missing bump is cheapest to fix on the branch,
|
# HEAD, since no PRE_COMMIT_TO_REF is set). A missing bump is cheapest to fix on
|
||||||
# before review.
|
# 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
|
# 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
|
# version must exceed its version there too (ADR-0022, second 2026-09-16
|
||||||
@@ -134,38 +147,70 @@ if [[ -z "$MAIN_REF" ]]; then
|
|||||||
exit 1
|
exit 1
|
||||||
fi
|
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 "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
|
echo " Fix: ensure full history is available (e.g. git fetch --unshallow) and retry." >&2
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
if [[ "$MAIN_REF" == "main" && "$BASELINE" == "$PUSHED_COMMIT" ]]; then
|
# A pushed commit that is an ancestor of <main> is itself the only merge-base,
|
||||||
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
|
# so this fires on exactly the case it always did.
|
||||||
echo " Fix: git fetch origin main and retry." >&2
|
if [[ "$MAIN_REF" == "main" ]]; then
|
||||||
exit 1
|
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
|
fi
|
||||||
|
|
||||||
CHANGED_FILE="$(mktemp)"
|
CHANGED_FILE="$(mktemp)"
|
||||||
trap 'rm -f "$CHANGED_FILE"' EXIT
|
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_PATH_RE='^(plugins/[^/]+/\.apm/skills/[^/]+)/(.+)$'
|
||||||
SKILL_DIRS=()
|
|
||||||
while IFS= read -r -d '' path; do
|
# skill_dirs_at <base>: sets SKILL_DIRS_ONE to the skill directories differing
|
||||||
[[ "$path" =~ $SKILL_PATH_RE ]] || continue
|
# between <base> and the pushed commit.
|
||||||
[[ "${BASH_REMATCH[2]}" == tests/* ]] && continue
|
skill_dirs_at() {
|
||||||
dir="${BASH_REMATCH[1]}"
|
local path dir existing seen
|
||||||
seen=false
|
SKILL_DIRS_ONE=()
|
||||||
for existing in ${SKILL_DIRS[@]+"${SKILL_DIRS[@]}"}; do
|
if ! git diff -z --no-renames --name-only "$1" "$PUSHED_COMMIT" -- plugins > "$CHANGED_FILE"; then
|
||||||
[[ "$existing" == "$dir" ]] && { seen=true; break; }
|
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
|
done
|
||||||
$seen || SKILL_DIRS+=("$dir")
|
SKILL_DIRS=(${KEPT[@]+"${KEPT[@]}"})
|
||||||
done < "$CHANGED_FILE"
|
done
|
||||||
|
|
||||||
[[ ${#SKILL_DIRS[@]} -eq 0 ]] && exit 0
|
[[ ${#SKILL_DIRS[@]} -eq 0 ]] && exit 0
|
||||||
|
|
||||||
@@ -238,35 +283,64 @@ in_tree() {
|
|||||||
|
|
||||||
OFFENDERS=()
|
OFFENDERS=()
|
||||||
for dir in ${SKILL_DIRS[@]+"${SKILL_DIRS[@]}"}; do
|
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
|
at_tip=false
|
||||||
in_tree "$BASELINE" "$dir/SKILL.md" && at_base=true
|
tip_is_base=false
|
||||||
# When main has not moved since the merge-base, the tip is the same baseline.
|
for base in ${BASES[@]+"${BASES[@]}"}; do
|
||||||
[[ "$MAIN_TIP" != "$BASELINE" ]] && in_tree "$MAIN_TIP" "$dir/SKILL.md" && at_tip=true
|
if [[ "$MAIN_TIP" == "$base" ]]; then
|
||||||
# Absent at both baselines: new, renamed-to, or merged-into. Exempt.
|
tip_is_base=true
|
||||||
$at_base || $at_tip || continue
|
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.
|
# Directory absent at pushed commit: deleted or renamed-from. Exempt.
|
||||||
[[ "$(git cat-file -t "$PUSHED_COMMIT:$dir" 2>/dev/null)" == "tree" ]] || continue
|
[[ "$(git cat-file -t "$PUSHED_COMMIT:$dir" 2>/dev/null)" == "tree" ]] || continue
|
||||||
|
|
||||||
base_ver=""
|
|
||||||
tip_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 $at_tip; then version_at "$MAIN_TIP" "$dir/SKILL.md"; tip_ver="$VERSION"; fi
|
||||||
|
|
||||||
if ! in_tree "$PUSHED_COMMIT" "$dir/SKILL.md"; then
|
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
|
continue
|
||||||
fi
|
fi
|
||||||
version_at "$PUSHED_COMMIT" "$dir/SKILL.md"
|
version_at "$PUSHED_COMMIT" "$dir/SKILL.md"
|
||||||
cur_ver="$VERSION"
|
cur_ver="$VERSION"
|
||||||
|
|
||||||
if [[ -z "$cur_ver" ]]; then
|
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
|
continue
|
||||||
fi
|
fi
|
||||||
if [[ -n "$base_ver" ]] && ! semver_gt "$cur_ver" "$base_ver"; then
|
# Named by sha only when there is more than one base to tell apart; a
|
||||||
OFFENDERS+=("$dir: $base_ver -> $cur_ver (not above merge-base)")
|
# criss-cross history is the only case where "which merge-base" is a question
|
||||||
fi
|
# 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
|
if [[ -n "$tip_ver" ]] && ! semver_gt "$cur_ver" "$tip_ver"; then
|
||||||
OFFENDERS+=("$dir: $tip_ver -> $cur_ver (not above $MAIN_REF tip)")
|
OFFENDERS+=("$dir: $tip_ver -> $cur_ver (not above $MAIN_REF tip)")
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -169,6 +169,67 @@ done < <(
|
|||||||
| sort
|
| 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
|
# 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
|
# 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
|
# run concurrently in fixed-size batches instead of one at a time. Dispatch and
|
||||||
|
|||||||
@@ -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"
|
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
|
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 ""
|
||||||
echo "Results: $PASS passed, $FAIL failed"
|
echo "Results: $PASS passed, $FAIL failed"
|
||||||
[[ $FAIL -eq 0 ]]
|
[[ $FAIL -eq 0 ]]
|
||||||
|
|||||||
@@ -150,6 +150,23 @@ if command -v git > /dev/null 2>&1; then
|
|||||||
commit -q --allow-empty -m init
|
commit -q --allow-empty -m init
|
||||||
touch "$REPO/apm.lock.yaml"
|
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")"
|
out="$(run_hook_in "$REPO" "$REPO")"
|
||||||
grep -q "default branch, so commit it or discard it deliberately" <<< "$(advice_of "$out")" \
|
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" \
|
&& 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")" \
|
grep -q "default branch, so commit it" <<< "$(advice_of "$out")" \
|
||||||
&& pass "reads the default branch from origin/HEAD when it is set" \
|
&& 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")"
|
|| 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
|
else
|
||||||
echo " (git not on PATH — branch-specific advice cases not run)"
|
echo " (git not on PATH — branch-specific advice cases not run)"
|
||||||
fi
|
fi
|
||||||
|
|||||||
@@ -703,6 +703,74 @@ else
|
|||||||
pass "a root under .claude/worktrees/ runs its own suites and skips nested worktrees"
|
pass "a root under .claude/worktrees/ runs its own suites and skips nested worktrees"
|
||||||
fi
|
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 ""
|
||||||
echo "Results: $PASS passed, $FAIL failed"
|
echo "Results: $PASS passed, $FAIL failed"
|
||||||
[[ $FAIL -eq 0 ]]
|
[[ $FAIL -eq 0 ]]
|
||||||
|
|||||||
@@ -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"
|
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"
|
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 ""
|
||||||
echo "Results: $PASS passed, $FAIL failed"
|
echo "Results: $PASS passed, $FAIL failed"
|
||||||
[[ $FAIL -eq 0 ]]
|
[[ $FAIL -eq 0 ]]
|
||||||
|
|||||||
@@ -1783,7 +1783,14 @@ fi
|
|||||||
# merge took out with the script, and cases 28-30 cannot backstop it. They key
|
# merge took out with the script, and cases 28-30 cannot backstop it. They key
|
||||||
# on `Kyberforge.VagueWording` and `KyberforgeCopilot.ProactivePhrase`, so
|
# on `Kyberforge.VagueWording` and `KyberforgeCopilot.ProactivePhrase`, so
|
||||||
# DescriptionOpener, PaddingPhrase, SentenceOpenerThereIs and CompositionNote
|
# 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:
|
# The original's comments, verbatim:
|
||||||
#
|
#
|
||||||
@@ -2364,6 +2371,124 @@ EOF_MUT34
|
|||||||
fi
|
fi
|
||||||
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 ""
|
||||||
echo "Results: $PASS passed, $FAIL failed"
|
echo "Results: $PASS passed, $FAIL failed"
|
||||||
[[ $FAIL -eq 0 ]] || exit 1
|
[[ $FAIL -eq 0 ]] || exit 1
|
||||||
|
|||||||
Reference in New Issue
Block a user