read_version required --- at byte 0 while skill-size-check accepts
leading blank lines, so a file one gate passed the other reported as
unversioned, and an unversioned merge-base side let an unbumped change
through. Match FRONTMATTER_RE, add case 39, and describe the main-tip
check and fail-closed cases in the hook entry.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apm recognises its own settings.json hook entries only through the
.claude/apm-hooks.json sidecar. With the sidecar gitignored, apm install
in a fresh clone keeps the committed SessionStart entry as user-owned and
appends a duplicate, so apm audit --ci reports drift and the apm-audit-ci
pre-push hook fails. Reproduced on main and this branch with apm 0.28.0.
Commit the sidecar in apm's exact serialisation, exclude it from
pretty-format-json alongside settings.json, and record the correction
in ADR-0019.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Why: a review of 4de5b6b and 3c5f6a6 found seven places that still
described the retired external hook contract as current, or that could
pass without checking anything.
Implementation Notes:
- gates.md: drop the "one caveat below" pointer; the caveat is gone.
- SIMPLIFICATION-AUDIT.md: the §7 note now says all but one entry is
closed and lists 36 (4de5b6b) as struck. Finding 2's count chain gets
a closing note: pre-push is 8 repo-authored hooks (10 reported).
- .pre-commit-config.yaml: the check-vale-style-sync comment points at
case 32 (one-plugin narrowing guard), not the deleted case 33.
- ADR-0014: the retirement pointer now covers the ADR-0025 amendment
above it too, naming case 33 and the exported hook IDs. ADR-0022 gets
a reciprocal amended-by note on its check-release-needed comparison.
Historical body text is unchanged.
- test-vale-wrap.sh case 32: property 3 fails when a class's corpus
regex matches no tracked file, instead of passing vacuously after a
layout move. The corpus regexes are now globals so a new Part D can
point them at a missing layout and require that failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Why: .pre-commit-hooks.yaml and its release-tag gate served external
consumers that do not exist. No repo on the Gitea instance pins these
hooks, and the README names apm as the only supported install path. The
mechanism was also already failing: skill-size-check.sh changed after
v2.0.1 with no tag cut, and the gate cannot fire through Gitea's merge
button. (Simplification audit finding 36.)
Implementation Notes:
- Delete .pre-commit-hooks.yaml, scripts/check-release-needed.sh,
tests/test-check-release-needed.sh and tests/test-vale-hooks-consumer.sh,
and remove the check-release-needed pre-push hook. The repo: local
skill-size-check and vale-audit-prefilter-* hooks are unchanged.
- ADR-0014 is amended, not retired: its runtime decision to bundle Vale
inside factory-audit stands. The amendment keeps the entry[0]-only
constraint (LESSONS.md:101,105) in case the export returns. ADR-0025
gets a pointer.
- test-vale-wrap.sh: drop case 33 (the cross-manifest drift check) and
case 28's hook-scope half, which read the published manifest. Case 32
now also requires each hook to select every tracked file of its class,
which keeps case 33's one-plugin-narrowing guard, with a mutation test.
- test-skill-size-check.sh and test-adr0020-contract.sh now assert the
hook contract and verbose: true on .pre-commit-config.yaml only.
- gates.md: pre-push count goes from 9 to 8 authored hooks (11 to 10
reported), and the Release table, the External consumers section and
the two-manifest scope table are removed. README and script/test
comments no longer describe the export as live. The resolver comment
is edited identically in both copies.
- The v1.0.0/v2.0.0/v2.0.1 tags are left in place; they are inert.
ADR: 0014
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
check-skill-version-bump fails a push when a skill directory changed
against its merge-base with main (tests/ excluded) without a strictly
higher metadata.version than main. New, renamed and deleted skills are
exempt; every plugin is covered. Recorded as a dated section in
ADR-0022 and documented in gates.md.
Patch-bumps the 17 skills that changed on this branch without a bump,
so the branch passes its own gate. Simplification audit finding 33.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Why
The two audit skills carried 1,724 lines of byte-identical duplication: the ADR-0020 boundary
resolver (1,061), vale-wrap.sh (526), the Vale style rules (44) and the Contributing-files parser
(93). Nothing shared them — they were held in sync by a 413-line pre-push gate and its 797-line
test suite. Sync-by-gate had already failed once: at 484357a the two parser copies drifted into
different spellings of the bullet loop while a docstring asserted they were identical. That drift
was behaviour-neutral and was re-unified by hand at 598a7c3, so the copies were identical at merge
time — but nothing had caught it, and the next drift need not be neutral.
Implementation Notes
Self-containment binds BETWEEN skills, not within one. The agentskills.io spec forbids reaching
across skill directories, which is why two separate skills needed embedded copies; two files inside
ONE skill may source a third. That is the whole reason the merge removes duplication rather than
relocating it.
The union of both bodies measured 1,532 words against BODY_MAX_WORDS=900, and only 211 of those
words were shared, so SKILL.md is a dispatch body. Step 0 resolves the flow from the target path
before any validation, and its table mirrors validate.sh's detection exactly: a directory holding
SKILL.md or a SKILL.md file (skill); a *.agent.md, or a .md directly under an agents/ directory
(agent); anything else stops without running a validator. Steps 1-3 live in
references/skill-flow.md and references/agent-flow.md, and gotchas that apply to one flow live in
that flow's file, since it is loaded on every invocation anyway. If validate.sh reports on the
other artifact type, the body restarts at Step 0.
Named factory-audit rather than forge-audit because forge is a live skill, and a family prefix that
matches a live sibling reads as ownership rather than membership.
The description carries one arrow per boundary target, because ADR-0020 resolves only the first
target after an arrow. It drops the quoted "audit this skill"-style phrases, which restated
"audited" in a second register (ADR-0020's duplicate-register rule). 241 characters, Gotchas 16%
of the body: no size SUGGESTIONs.
The boundary resolver stays embedded in two files rather than imported: a cache-installed plugin
cannot read outside its own directory, and the repo-root hook resolves via .pre-commit-hooks.yaml
where entry[0] is the only token pre-commit rewrites, so no single file is reachable by both.
tests/test-adr0020-contract.sh hashes both copies for byte-identity, and asserts validate.sh sources
the resolver and that no third copy exists.
The entry scripts classify the target from its resolved parent directory, so a bare agent filename
typed inside agents/ works; resolve SCRIPT_DIR CDPATH-safely; and exit 2 when a lib-*.sh is
missing, rather than dying with exit 1, the tier the flows relay as real findings.
The provenance run functions stash their findings code in KYBERFORGE_PROV_RC and
return 0, so validate-provenance.sh calls them UNTESTED. Testing a function's
status (`f || RC=$?`) disables errexit for its entire body, and no subshell or
`set -e` inside can re-arm it once the call sits in a condition context
(measured, both spellings). Their error paths use `exit`, which is unaffected
either way; this keeps errexit armed for anything added later.
Case 0's readability guard reads the file instead of asking `[[ -r ]]`. `-r` is
access(2), which answers yes for uid 0 even on a mode-000 file, and this repo's
dev environment is root -- so the guard could never fire where it exists to fire.
A read attempt is also the stricter question, catching EIO. This is the reasoning
scripts/check-vale-style-sync.sh carried before this commit deleted it; the
hazard did not go with it.
All three entry scripts are CDPATH-safe, vale-wrap.sh included: both of its cd sites are cleared,
the --config resolution and the directory-mirror walk, where an exported CDPATH would otherwise
print a decoy path into the -print0 stream and build the mirror from the decoy's files. The two
remaining bare cd calls take absolute paths, which CDPATH is never consulted for.
Impact
BREAKING: skill-audit and agent-audit no longer exist as invocable skills. kyberforge goes to
2.0.0 (catalog 0.4.7).
Check logic is unchanged: differential runs of the old and new validators across every skill and
agent produced byte-identical stdout, stderr and exit codes, and the reconstructed Python payloads
differ only in comments and the references/field-inventory.md -> agent-field-inventory.md rename.
One doctrine governs the tiers: exit 0 is audited and clean, exit 1 is audited with findings OR a
target present but unreadable, exit 2 is that nothing was audited at all. Edge paths DID change,
deliberately (full table in ADR-0025):
- a missing target exits 2 (never ran), not 1, under its own "does not exist" message; detection is
by path shape, so a shape-matching path that is simply absent used to reach the validator and come
back as a FAIL against a file that never existed;
- an unshaped target exits 2 under the generic "matches neither" message, and a directory with no
SKILL.md under a third, distinct one -- three exit-2 messages, not one;
- a dangling symlink or a symlink loop stays exit 1: it is present but broken, which is a finding
about the artifact rather than a usage error;
- a SKILL.md file path is audited as its skill directory instead of refused;
- a .md agent outside an agents/ directory is refused rather than audited;
- a missing script library, a missing python3, a missing PyYAML, and no argument at all each exit 2.
validate-provenance.sh already exited 2 for the last two; validate.sh now matches it.
.pre-commit-hooks.yaml is a published contract consumed by external repos. Both hook IDs and both
files: regexes are unchanged; only entry: and description: moved.
scripts/check-vale-style-sync.sh (413), scripts/sync-vale-styles.sh (21),
tests/test-check-vale-style-sync.sh (797) and agent-audit/scripts/README.md (47) are deleted. The
checker made 17 assertions: 6 compared the two Vale copies and are moot; 10 are rehomed into
tests/test-vale-wrap.sh (case 0, cases 28-31, and the suite's Vale-absent skip); and the
cross-manifest files: agreement check, which selected hooks by entry: and so could not survive both
hooks sharing one, is ported as case 33 pairing hooks by id:. Cases 28, 30 and 33 carry mutation
self-tests; narrowing the local skill prefilter to 6 of 38 SKILL.md files now fails the suite.
Skills go 39 to 38. Pre-push goes 9 repo-authored hooks to 8.
ADR: 0025
BREAKING-CHANGE: the skill-audit and agent-audit skills are removed. Both flows are served by
factory-audit, which auto-detects whether it was handed a skill directory or an agent file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
A five-agent review of 718c79a and d2480b8 found no skill, agent or hook
regressions (39 skills before and after) and confirmed both hook removals
are genuinely moot -- verified against the tree, not taken on the commit's
word. It did find one functional regression (fixed separately) and this
documentation drift.
Counting errors, all from a git pathspec `*` crossing `/`:
- 17 .bats files shipped to consumers is really 10; 17 counted tracked
paths merely containing /tests/, one of them a template asset
- "roughly 88s off every push" is ~92.4s; 88 omitted validate-plugins
- "roughly 70% of each plugin remains live" holds only for kyberforge;
the real spread is 44.3% (bin) to 70.6%, now a table
- the pre-push enforcement row was half-corrected: 33 entries stood
unstruck (now 27) and 14 -> 11 switched counting basis mid-sentence
- the root .claude-plugin/plugin.json was described as "kept"; it has
never been tracked
gates.md said "Ten hooks" above a nine-row table (11 was decremented for
one removal, not two), and "both need the claude CLI" for one remaining
validator. Its pretty-format-json exclude rationale claimed six
alternations expanding to sixteen files in a passage headed "Mind which
number you are quoting" -- four alternations, two live files; the two
dead ones are dropped from the pattern. check-useless-excludes could not
catch this: it only flags an exclude matching nothing at all.
ADR-0024 cited ADR-0006 for a patch-bump rule it does not contain and
which ADR-0015 explicitly retired; stated apm's marketplace probe order
backwards (.claude-plugin/ is the last candidate, not the first, so the
earlier .github/plugin/ deletion only demoted resolution); undercounted
apm's skill-deploying targets as seven when there are fifteen; and never
recorded that validate-plugins was removed. The symlink hedge is resolved:
apm_cli/security/gate.py's ignore_non_content() drops symlinks silently on
deploy while apm_modules/ materialization dereferences them, so content
survives that far and vanishes at install. Accepted with no replacement
guard, per decision -- kyberforge/docs/hooks.md previously asserted a
guard that had been deleted with its script.
Four plugin READMEs still advertised `claude plugin install`; ADRs 0001,
0006, 0013, 0014, 0015 and 0019 described deleted machinery in the present
tense, 0019 most consequentially as the live justification for the
SessionStart hook's .apm/ path. CONTEXT.md's "apm package" entry forbade
"plugin" while using it in its own body, and "Output profile" lost the
antecedent for "one catalogue serves both".
run-tests.sh gains the .claude/skills/ exclusion run-bats.sh already had.
Latent today -- no test-*.sh lives under any .apm/skills/*/tests/ -- but
apm now deploys those directories, so one would be discovered twice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
apm becomes the only supported install path. The flat mirror at each plugin
root existed solely so Claude Code's native `claude plugin install` could
convention-scan plugin content (ADR-0017). With no native consumers, it cost
~20,000 tracked lines plus ~2,100 lines of sync tooling and ~88s of every
push to guard content apm never reads — and its only automated gate,
`claude plugin validate --strict`, passes on a plugin with zero content, so
it could not detect the defect ADR-0017 was created to fix.
Removes the mirror (213 files), the six per-plugin manifest pairs,
sync-plugin-content.sh, its 1,289-line test, the orphaned
marketplace-plugins.sh, and the check-plugin-content-sync and
validate-plugins pre-push hooks. The root `marketplace:` block and
.claude-plugin/ catalogue stay: apm's own marketplace consumers read that
same file, so `<name>@holocron` short names keep working.
tests/run-bats.sh now excludes .claude/skills/. apm installs from .apm/,
which carries the tests/ dirs the mirror stripped, so deployed .bats files
would otherwise be discovered and double-run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
apm-marketplace-check checked network reachability of remote marketplace
refs, but finding 35 already removed the only remote package entry, so
every marketplace.packages[] source is local and the hook is pure
overlap with apm-pack-check-clean. apm-audit-ci was re-examined and kept
as-is -- its pre-commit-config.yaml comment already carries a dated,
verified justification the audit had missed.
check-marketplace-mirror-sync guarded .github/plugin/marketplace.json
against drift from .claude-plugin/marketplace.json. Verified against
current GitHub Copilot CLI docs: Copilot's marketplace discovery already
falls back through .github/plugin/marketplace.json to
.claude-plugin/marketplace.json, which this repo generates anyway -- the
dedicated mirror bought a discovery-order preference, not a capability.
Deleted the mirror file, its sync script, its test, and the hook.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
e647f14 deleted scripts/check-manifests.sh but missed three live files
that still named it: the apm-marketplace-check hook description in
.pre-commit-config.yaml, and comments in sync-plugin-content.sh and
lib/marketplace-plugins.sh explaining design decisions by pointing at
a script that no longer exists.
Also deletes list_marketplace_remote_plugin_names from
marketplace-plugins.sh — its only caller was check-manifests.sh, so
it's been dead code since that commit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
skill-frontmatter was a 62-line bash script inlined in
.pre-commit-config.yaml, re-parsing SKILL.md frontmatter with grep and
awk to check for name/description/metadata.version fields.
skill-size-check.sh already parses the same frontmatter block with
PyYAML for its ADR-0020 checks, so the two checks belonged in one
script.
Adds a ~20-line required-frontmatter check (name, description,
metadata.version as three-part semver) to scripts/skill-size-check.sh.
Removes the inline skill-frontmatter hook from .pre-commit-config.yaml
and deletes tests/test-skill-frontmatter.sh (366 lines). Removes the
79-line "the other hook on that scope" discussion from
docs/spec/gates.md and its now-dangling cross-reference, replacing
both with a one-line note of the fold, and updates the pre-push hook
counts there.
Updates fixture builders in test-skill-size-check.sh,
test-adr0020-body-checks.sh, test-adr0020-targets.sh,
test-adr0020-differential.sh, and test-vale-hooks-consumer.sh to carry
valid metadata.version so the new check doesn't spuriously fail
existing fixtures that predate it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
Six pre-push hooks were validating overlapping sets of the same
manifests. check-manifests (marketplace.json/plugin.json path checks)
is redundant with validate-plugins (claude plugin validate) and
apm-pack-check-clean, which already cover the same ground.
Deletes the check-manifests hook entry, scripts/check-manifests.sh
(282 lines), and tests/test-check-manifests.sh (771 lines).
scripts/lib/marketplace-plugins.sh is kept — it is still sourced by
sync-plugin-content.sh. Updates the now-stale check-manifests.sh
mentions and hook counts in README.md and docs/spec/gates.md.
The apm-audit-ci and apm-marketplace-check hooks named in the same
finding are left untouched — the audit flags them as needing a
separate decision.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YR2CjVumUbEGWcMikcoXBD
The #113 sweep rested on CLAUDE.md's premise that rtk either filters or passes through unchanged,
so prefixing is always safe. Measured against rtk 0.42.4, that premise is false for several of the
commands the sweep prefixed, and two skills were left giving wrong answers silently.
Why:
- `rtk git worktree list --porcelain -z` discards both flags and renders its own format. The
`locked`/`lock_reason` fields git-worktrees Step 2 must emit are absent entirely, and paths under
$HOME are abbreviated to `~/`.
- `rtk git branch --list <name>` prints a phantom `* ` line even when nothing matches, so
git-branches' stated ambiguity test — "output from both means the name is ambiguous" — reported
every name as ambiguous. `tag --list` is a clean passthrough, so only one half broke.
- `rtk git diff --name-only`/`--name-status` append a `Changes:` trailer to output documented as
"one per line"; `--word-diff` emits none of the `[-removed-] {+added+}` markers its table
describes; `rtk git log -L` truncates each line at ~72 chars, on the one command whose purpose is
showing line content.
- `rtk git stash pop` prints only `FAILED: git stash pop`, swallowing the conflict diagnostic and
retained-entry message the surrounding prose tells the agent to rely on.
Implementation notes:
- Eleven sites reverted to bare `git`, each carrying its reason inline so the next sweep does not
undo it. `mergetool` and `rebase -i` are reverted on clause 3's interactive limb only: the TTY
defect does not reproduce — rtk filters exactly twelve subcommands and execs the rest — and
ADR-0023 records that measurement rather than a convenient one.
- ADR-0023 states the rule repo-wide with a third clause: a command whose output the skill parses,
or which is interactive, stays bare. `plugins/git/README.md` is reduced to a pointer; its claim
that gitea skills "contain no git/rtk mentions at all" was false, and its citation of
`hard-rules.md` pointed at a file containing no occurrence of "rtk".
- Eight gitea sites swept, all verified byte-identical passthroughs first.
- `scripts/check-rtk-prefix.sh` gates clause 1. Run against main's pre-sweep corpus it reports 99
findings including every gitea site, so it would have caught the drift #113 was filed about.
Impact: the gate covers clause 1 only, in shell-tagged fences and the opening span of Run cells.
Clause 2 is not gateable — "Run `git switch`" and "`git switch` refuses" are the same tokens — and
prose bullets are invisible to it. Both limits are recorded in gates.md rather than left implied.
Refs: #113
ADR: 0023
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeH8SCbcrCAQrtymkNuhKP
The hook is `entry: bash` with `args: ['-c', <script>]`. pre-commit appends filenames after the
script string, so the first becomes `$0` and never enters `"$@"` — on a single-file commit, the
common case, the loop body never ran and the hook reported Passed having measured nothing.
ADR-0022 leans on this hook as the enforcement for a mandatory `metadata.version`, so the vacuous
green was the whole gate.
Implementation notes:
- An arg0 placeholder absorbs `$0` so every filename lands in `"$@"`.
- Checks now run against the YAML frontmatter block only, extracted with awk. The old
`grep -A10 "^metadata:"` matched a `metadata:` inside a body code fence, spanned past the block
into a following `source:` entry's `version:`, accepted any indentation, and missed a `version:`
more than ten lines in. An unreadable frontmatter block is now an error, never a pass.
- The value is asserted against three-part semver. `write-docs` carried "1.0" through the entire
ADR-0022 retrofit undetected, which a presence-only check cannot catch.
Impact: `tests/test-skill-frontmatter.sh` is the first test this hook has ever had. It drives the
real `entry`/`args` composition read out of the config rather than a copy of the script, which is
the only shape that catches the arg0 bug; against the pre-fix hook it scores 7/20.
gates.md described the hook wrongly in both directions and is rewritten, with a carve-out
explaining why this one stays a shell parser next to the "python3 and PyYAML are hard
requirements" reasoning that argues otherwise.
Refs: #127
ADR: 0022
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeH8SCbcrCAQrtymkNuhKP
Only 12 of 39 skills carried metadata.version, and adoption tracked
which plugin a skill lived in rather than any stated rule: core,
gitea and lint were consistent adopters, bin and kyberforge were
consistent non-adopters, git was split with one outlier. There was
no documented convention, and skill-author's own bump logic was
already written as if presence were conditional.
metadata.version is now required on every skill. The 19 skills here
that never carried one (bin, kyberforge, gitea-files) are seeded at
1.0.0, not 0.1.0 -- that value stays reserved for a skill's actual
creation point under skill-author's existing convention. The
skill-frontmatter pre-commit hook now fails a SKILL.md missing the
field, the same class of failure as a missing name/description.
Full rationale in the new ADR. The git-plugin skills that also need
this field follow in the next commit, bundled with issue #113's rtk
normalization since both touch the same files.
Refs: #127
ADR: 0022
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeH8SCbcrCAQrtymkNuhKP
Why: eight comments and one status note cited CONTEXT.md or AGENTS.md text that
b9c7762 and 1929ffd moved or deleted. All are inert at runtime, but they are the
rationale comments that tell the next maintainer why an assertion exists, and
they now name a file that no longer explains it.
Implementation notes: re-anchored by what the citation is for, not uniformly.
- Four sites quoted facts ADR-0013 owns — every rule is `level: error` with no
ignorable tier (ADR-0013:59-70), and KyberforgeCopilot's `.agent.md`-only
scope (ADR-0013:43-46). These now cite ADR-0013. ADRs are append-only here;
the spec docs are refactored, which is what caused this rot.
- Two sites quoted the glob location-independence property, which no ADR owns.
The quote was already inline and carried the full rationale, so the citation
added a rot surface and no information — dropped, statement kept.
- sync-marketplace-mirror.sh's header attributed the mirror-not-a-profile fact
to CONTEXT.md; the parenthetical beside it already carries the evidence, so
the attribution is dropped rather than re-pointed.
- .pre-commit-config.yaml cited an AGENTS.md instruction that no longer exists;
generalised to "the documented instruction".
- LESSONS.md:29 misquoted AGENTS.md's current session-start line.
Also corrects a pre-existing misattribution at tests/test-vale-wrap.sh:454:
AGENTS.md has never named bash 3.2 as a repo target (`git log -S'3.2'` on it is
empty). LESSONS.md and the script headers do.
Impact: no behaviour change. test-check-vale-style-sync.sh and test-vale-wrap.sh
both pass (42 passed, 0 failed).
Two pre-existing failures, both red at HEAD before ADR-0020 work began,
both invisible in an ordinary local run.
RUN_TESTS_STRICT leaked from the environment into test-run-tests.sh's
fixture children. The meta-test is itself a suite the runner discovers,
so under the gate's own invocation the variable propagated outer runner
-> batch_run -> the fixture's copy of run-tests.sh, flipping it strict.
Case 10c (a deliberate control asserting a skip is tolerated WITHOUT
strict) then failed. Six further cases were silently running strict too
and asserting against the wrong stream — case 9 was matching the stderr
strict block rather than the stdout skip list it was written to check.
run_fake now spawns via 'env -u RUN_TESTS_STRICT', so fixture strictness
is a property of the case, never of how the file was launched. No
assertion weakened; run-tests.sh itself is untouched.
pretty-format-json --autofix was re-sorting apm's output on the way into
every commit. .claude/settings.json is apm-owned (ADR-0018/0019) and its
exclude list named fifteen generated manifests but not this file, so
since 2e395a4 it has been committed in a key order apm would never write
— permanent drift on a file with an empty git diff. Content was always
byte-identical; only JSON key order differed. The exclude ships in the
same commit as the corrected file because otherwise the hook re-breaks
it during staging.
apm.lock.yaml: generated_at churn, plus lint's exec_status corrected from
'deployed' to 'gated_pending_approval' — executables.allow grants only
kyberforge#1.5.0, so lint's hooks/bin are genuinely gated.
New coverage: an ambient RUN_TESTS_STRICT must not reach a fixture that
did not ask for it, and under --strict the skip report goes to stderr
only with the stdout list suppressed. Neither was pinned.
Skill name+description pairs are preloaded into every session, costing
~6,200 tokens across 39 skills before any skill is invoked. The authoring
rules mandated that growth: skill-author:104 and description-quality.md:21
both required padding, while skill-author:102 (the deflating rule) had no
FAIL condition behind it.
Gates (blocking, no baseline file):
- description 250 chars SUGGESTION / 400 FAIL, measured on the folded
YAML value
- body-only 600 words SUGGESTION / 900 FAIL, independent of the unchanged
whole-file 2770-word / 500-line spec backstop
- every boundary-clause routing target must resolve to a real skill or
agent; catches skill-improve, neuledge-context and gitea-labels
- agents take the description gates but deliberately no body gate; a test
pins that absence
Vale: DescriptionOpener widened to ^This\b, new CompositionNote rule
banning architecture notes from descriptions. 10 hits, 0 false positives.
Kyberforge's own four skills retrofitted: descriptions 3,364 -> 938 chars
(-72%), bodies 8,306 -> 2,487 words (-70%), all via the apm-workflow
dispatch pattern. Fixes the skill-improve dangling route and the
agent-author misroute to manual review.
Also fixes a pre-existing false positive where any line-initial 'read '
was flagged as interactive input, which had already caused two scripts to
be rewritten around it.
Refs: ADR-0020
apm approves a package's hooks and bin by an exact dictionary lookup on a
composed `name#version` key (apm_cli/security/executables.py,
is_package_approved). There is no wildcard and no version-less form, so
bumping plugins/kyberforge/apm.yml without editing root apm.yml's
`kyberforge#<version>` key errors nowhere: the entry stops matching, the
SessionStart hook stops deploying, and the install goes quietly stale.
ADR-0019 already named that as a live failure mode, mitigated only by a
comment in the executables block. This repo gates generated-content
drift, marketplace mirror drift and vale style drift deterministically,
and a silent-staleness failure is worse than any of them — a comment does
not survive the release that breaks it.
check-executables-allow-sync.sh parses the version out of the plugin
manifest and asserts the matching key exists in the root manifest. It
uses PyYAML where importable and falls back to a two-shape scan
otherwise, so a missing pip package cannot become the thing that blocks
every push; the test asserts both readers agree. 23 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
apm-audit-ci ran against root apm.yml alone, so it audited none of the six
plugin packages, and its description claimed a lockfile/policy/hidden-content
gate while delivering one vacuous check. It now loops all seven manifests, and
the description says only what runs. Proven load-bearing: a malformed dependency
in plugins/lint/apm.yml passed the old root-only entry at exit 0 and passed
apm pack --check-clean too, because that gate never parses plugin dependencies;
the loop catches it and names the file.
policy.fetch_failure_default: block was considered and rejected. apm's org-policy
discovery understands github.com and Azure DevOps; this repo's remote is
self-hosted Gitea, so no policy source is discoverable and the setting makes the
hook exit 1 on every push forever. Fail-closed is right when there is a control
to fail closed on -- a permanently red gate is one people learn to SKIP=, which
is worse than an accurate description.
agent-audit's validate.sh had never run against the four real .apm/agents files
it governs, only against synthetic fixtures. That is why an amended ADR-0016 and
a validator that still rejected the field it blessed could disagree unnoticed
until someone ran it by hand. check-apm-agents-valid.sh closes it, deriving the
expected set from git ls-files rather than a count, failing on zero discovered
files, and replaying validate.sh's own reason under each failing filename.
Also makes the pretty-format-json exclude consistently root-anchored: it mixed
(^|/) for five paths with ^ for one, so a nested fixture at
.../.claude-plugin/plugin.json was silently exempt from formatting.
Pre-push goes 12 repo-defined to 13, 14 total to 15; AGENTS.md's counts, hook
list and offline-skip note are updated to match. The new hook needs no network.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
Three gaps left by the previous round, all the same shape: a gate reporting
success having verified less than it appears to.
run_bats() hard-failed on a missing or non-executable runner but never checked
that the runner produced anything. An empty, executable run-bats.sh exits 0,
and the dispatcher printed a green summary with 166 bats tests silently absent.
It now requires an "N tests, M failures" line with a non-zero count.
run-tests.sh's skip listing is swallowed by pre-commit on a pass, so on a
machine without vale three suites exited 77 and the pre-push gate went green
having run 14 of 17. The hook now invokes it as --strict, where a skip fails
and the error names each suite and the reason it skipped. An ad-hoc local run
still skips gracefully -- at pre-push a skip means a documented dependency is
missing, which is a setup error, not a legitimate state. Deliberately not wired
to the vale downgrade's env var: one flag must not disarm two gates.
BATS_FILE_FLOOR is replaced by an expectation derived from git ls-files. A
floor of 8 against a real count of 10 let two files and eleven tests disappear
green, and the number needed an edit whenever a plugin was added. The derived
set needs no number, and catches an addition as well as a removal -- a .bats
file staged into the index and deleted from disk is now demanded back.
The vale opt-out announced its downgrade to nobody: pre-commit prints nothing
for a passing hook, so the summary line AGENTS.md tells the reader to check was
unreachable in exactly the situation it exists for. The hook is now verbose.
Also corrects the PROBES_CHECKED guard, whose commit message described a state
that cannot occur -- the .vale.ini loop errs first. Its two reachable triggers,
a gutted probe heredoc and a probe row naming a missing directory, had no test;
they do now, each asserting the guard is the sole cause.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
`check-plugin-content-sync` was the only local pre-push hook scoped with `files:`
instead of `always_run`, and its regex missed both compiled `plugin.json` paths —
exactly the manifests the script had been taught to diff. A commit tampering only
with one of those passed the entire pre-push suite at exit 0.
A regex fix alone could not close it: pre-commit drops deleted paths from the file
list, so removing a mirror directory skipped the hook too. The `hooks\.json`
alternative was also already dead, matching only the pre-move flat path. Dropping
`files:` for `always_run: true` closes all three and makes the hook uniform with its
three siblings; the check costs ~2-4s.
Also excludes `.github/plugin/marketplace.json` from `pretty-format-json`. This was
not hypothetical: `json.dumps(indent=2, sort_keys=True)` reproduces the manifests
committed at HEAD byte-for-byte, so `--autofix` had already rewritten compiled apm
output once. Without the exclusion it would do so again on the next commit,
silently breaking the byte-identical mirror invariant.
Landing first, ahead of the regenerated manifests, so that cannot happen.
Refs: #90
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
The pre-push hook ran a full `apm pack` + directory diff across every
plugin on every push (always_run: true), even for changes that could
never affect the sync (e.g. a README edit). Add a files: regex scoped
to what scripts/sync-plugin-content.sh actually consumes -- each
plugin's apm.yml, .mcp.json (read by reinject_mcp_servers), .apm/**
(the sync source), the flat mirror output dirs themselves (agents/,
skills/, commands/, instructions/, extensions/, hooks.json -- so a
direct edit to compiled output is still caught as drift), the
marketplace.json --all reads to build the plugin list, and the sync
script itself -- and drop always_run now that files: covers every
input path.
PR #95's review of the issue #90 apm-conversion work found several
defects in scripts/sync-plugin-content.sh and the gate wired to it:
- --check claimed never to mutate the plugin root, but apm pack still
wrote .claude-plugin/plugin.json and .github/plugin/plugin.json into
the real plugin_dir on first-time creation. --check now packs a
throwaway copy instead.
- check-plugin-content-sync hardcoded the six plugin directories
instead of deriving them the way check-manifests.sh already does.
Added an --all flag that parses .claude-plugin/marketplace.json, and
simplified the pre-commit hook to use it.
- A missing plugin_dir and one that legitimately has no .apm/ yet both
reported SKIP/success; a missing directory now FAILs.
- The dispatch loop backgrounded every plugin with no concurrency cap,
unlike the JOBS-bounded pattern this same PR added to
tests/run-bats.sh and tests/run-tests.sh. Added the same
bash-3.2-safe getconf + batched-wait cap here for consistency.
- Per-plugin scratch/log/status files were keyed only by basename, with
no collision guard across arguments; added a fail-fast check.
- sync_hooks_json()'s trailing-newline normalization was duplicated
between its --check and write branches; factored into one helper.
- tests/test-sync-plugin-content.sh set two competing `trap ... EXIT`
statements, so the first (cleaning up $FIXTURE) was silently
replaced by the second and its tmp dir leaked every run. Adopted the
track()/CLEANUP_DIRS pattern already used in
tests/test-check-release-needed.sh.
Separately: apm's Copilot-ecosystem plugin.json builder unconditionally
strips mcpServers, citing (in its own docstring) that the field is out
of schema for Copilot -- a claim this repo's own researched Copilot
plugin schema docs contradict. reinject_mcp_servers() narrowly restores
it from the plugin's .mcp.json on real syncs only, regenerating
plugins/bin/.github/plugin/plugin.json (the only plugin that currently
declares any MCP servers). Documented as an amendment to ADR-0017,
since it's a deliberate, narrow exception to that ADR's rejection of
patching apm's compiled output -- apm's premise for stripping
skills/agents/commands/hooks pointers is still accurate; its premise
for stripping mcpServers is not.
All 12 assertions in tests/test-sync-plugin-content.sh pass
individually, plus 5 new regression tests added for this round; the
full bats and shell-script suites are green; shellcheck is clean.
Refs: #95
ADR: 0017
apm.yml only declares claude and codex marketplace output profiles --
codex writes a differently-shaped file to .agents/plugins/marketplace.json,
so nothing regenerates the legacy Copilot CLI path at
.github/plugin/marketplace.json. It was hand-synced once during the #90
conversion and had drifted (missing the category field added when codex
output was enabled) with no gate to catch it.
scripts/sync-marketplace-mirror.sh keeps it byte-identical to the compiled
.claude-plugin/marketplace.json, wired as a pre-push check so it can't go
stale silently again.
Refs: #90
skill-frontmatter's files: pattern matched any SKILL.md, so it ran twice per
sync -- once on the .apm/ source and again on the compiled flat-mirror copy.
Scoped it to .apm/skills/ like its sibling hooks (skill-size-check,
vale-audit-prefilter-skill), which already only check the source.
sync-plugin-content.sh ran `apm pack` once per plugin, serially -- each
invocation is dominated by fixed CLI startup cost rather than per-plugin
work, so 6 plugins paid that cost 6 times over (~3.1s). Backgrounds the
per-plugin work instead, buffering each plugin's output so concurrent
DRIFT/FAIL messages can't interleave, then flushes in stable order after
`wait` (~1.3s, confirmed idempotent on a real sync).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
apm pack silently skips regenerating plugin.json when it already exists
("already exists; skipping plugin.json generation. Re-run with --force to
overwrite it."). sync-plugin-content.sh never passed --force, so
.claude-plugin/plugin.json and .github/plugin/plugin.json were written
once on first sync and never refreshed again -- a name/version/description
edit in a plugin's apm.yml would silently stop propagating, with no gate
catching it (check-manifests.sh only checks presence; check-plugin-content-
sync explicitly excludes plugin.json; apm-pack-check-clean is scoped to
marketplace.json only).
Pass --force on real-mode syncs only (--check must not mutate the plugin
root, so plugin.json staleness there stays a known, undetected gap until
the next real sync -- documented in the script's header comment).
Regenerating surfaced a second, unrelated bug: apm's writer and
pretty-format-json's --autofix disagree on JSON key order (semantic vs.
alphabetical), so every real sync would otherwise flip plugin.json's key
order right back after a commit re-alphabetized it. Excluded the four
apm-generated manifests (plugin.json x2, marketplace.json x2) from
pretty-format-json -- their exact formatting is dictated by apm's own
writer as compiled output, not this repo's generic JSON prettifier.
Verified idempotent: a second real sync after this fix produces zero
further diff.
Refs: #90
Validated the plugin-content-mirror fix (issue #90) against apm's own
packing/CI documentation and source: no apm-native mechanism replaces the
mirror script (apm's bundler treats .apm/ and root convention dirs as
mutually exclusive, by design), but the investigation surfaced a real,
separate gap -- this repo ran zero apm-native audit/check commands in CI,
relying entirely on custom scripts and Claude Code's own client-side
validator.
Add three pre-push hooks matching apm's documented producer CI pattern:
- apm marketplace check: validates every marketplace.packages[] entry
resolves, including live network reachability for remote refs -- a
blind spot check-manifests.sh explicitly skips (local sources only).
- apm audit --ci: apm's own lockfile/policy/hidden-content integrity gate.
- apm pack --check-versions --check-clean: closes issue #90's deferred
item 3 (a check-clean-equivalent gate) using apm's native flag instead
of bespoke drift logic, verifying .claude-plugin/marketplace.json still
matches what apm.yml + .apm/ would currently generate.
All three are network-tolerant and whole-repo in scope, so they belong at
pre-push alongside check-manifests/check-plugin-content-sync/
validate-plugins -- not pre-commit, which stays fast/offline/per-file.
Documented the packing/bundling/releasing/CI findings in
docs/research/docs/microsoft-apm/releasing.md (new) and extended
testing-and-validation.md with the apm-action wrapper and its documented
CI patterns, sourced from Context7 and cross-checked against the
installed apm-cli 0.28.0 package directly.
Refs: #90
Claude Code's (and Copilot's) native plugin installer has zero awareness of
.apm/ nesting -- it convention-scans only flat skills/, agents/, commands/,
hooks.json at each plugin's root. Confirmed via strings on the installed
claude binary and live installs of git@holocron/gitea@holocron/kyberforge@
holocron, all reporting Skills(0) Agents(0) Hooks(0) post ADR-0015's apm
conversion. Root cause (apm_cli/core/plugin_manifest.py): apm's plugin.json
compiler deliberately strips skills/agents/commands keys, assuming the host
already auto-discovers those convention directories -- it has no model of
.apm/ being host-visible at all. Separately, apm's own bundle exporter
(apm_cli/bundle/plugin_exporter.py, behind `apm pack --format plugin`)
implements the correct .apm/ -> flat mapping, but only ever targeted
build/<name>-<version>/, a path nothing in marketplace.json's source: points
at.
scripts/sync-plugin-content.sh wraps that bundle exporter and copies its
agents/, skills/, commands/, instructions/, extensions/, and merged
hooks.json back into each plugin's own root as a second tracked
compiled-output category -- same governance status as
.claude-plugin/plugin.json: generated from .apm/, never hand-edited. tests/
subdirectories are excluded from the mirror (dev fixtures, not host-visible
runtime content; several hardcode a relative repo-root walk-up sized for the
.apm/-nested depth, which breaks when duplicated one level shallower).
Applied for real across all 6 plugins and verified two ways: `claude plugin
validate --strict` passes on every real plugin directory, and a live
`claude --plugin-dir <path> -p "list skills/agents"` behavioral test
confirms content is now actually discovered.
Also, from the same issue #90 review round:
- scripts/check-manifests.sh pointed at each plugin's root-level plugin.json
(checking skills/hooks/mcpServers/agents pointer fields) -- that file was a
stale near-duplicate of .claude-plugin/plugin.json nothing else read or
wrote, now deleted across all 6 plugins. check-manifests.sh is rewritten to
validate .claude-plugin/plugin.json instead, and drops the pointer-field
checks entirely (nothing to check -- those fields are correctly absent by
design). Content-presence drift is now check-plugin-content-sync's job, a
new pre-push hook wired in .pre-commit-config.yaml.
docs/adr/0017 records the root cause and decision in full, including two
rejected alternatives (patching plugin.json's path fields directly -- apm's
compiler strips them on every run; pointing marketplace.json at apm pack's
build/ output -- a version-suffixed non-source directory nothing can install
from without an extra build step). ADR-0015 and CONTEXT.md are updated to
point at it.
Refs: #90
Why:
ADR-0015 established that Microsoft APM (apm.yml + .apm/) should replace
this repo's hand-authored plugin.json/marketplace.json model, with those
files becoming compiled output of `apm pack` instead of files edited by
hand via the (now-retired) plugin-author/marketplace-author skills.
Issue #90 was the deferred execution of that decision, gated on #88
(apm tooling) and #89 (apm-native agent-author/skill-author routing).
Implementation notes:
- All six plugins (bin, core, git, gitea, kyberforge, lint) now carry
apm.yml + .apm/{skills,agents,hooks} as their authoring source. Skills
moved with a plain git mv (content-identical across targets). Agents
were re-authored, not moved: per ADR-0016, .apm/agents/*.agent.md
compiles verbatim to both Claude and Copilot, so plugin-scope agents
now carry only name/description/model/source_keys -- no tools: field,
no Claude-only knobs (isolation, maxTurns, effort, memory,
permissionMode).
- Root apm.yml registers all 7 marketplace packages (6 local plus
mattpocock-skills as a remote entry) under versioning: per_package,
matching this repo's existing independent-plugin-versioning practice.
- .claude-plugin/marketplace.json and every plugin's plugin.json are now
apm-pack-compiled output, verified against the prior hand-maintained
content: same names/descriptions/versions/licenses/authors, only
cosmetic serialization differences (JSON key order, owner email vs.
url, Unicode escaping).
- plugin-author and marketplace-author are retired now that apm-based
authoring fully replaces their job; kyberforge bumped 1.3.1 -> 1.4.0
for that removal, and the root marketplace catalog bumped
0.3.1 -> 0.3.2 to match, per the version-bump convention now
documented in apm-workflow's reference docs instead of a dedicated
script (apm has no native version-bump automation).
- Fixed hardcoded pre-.apm/ path assumptions across
.pre-commit-config.yaml, .pre-commit-hooks.yaml,
scripts/check-scope-walkup-sync.sh, scripts/sync-vale-styles.sh,
scripts/check-vale-style-sync.sh, six plugins' root plugin.json
(stale skills/hooks/agents pointer fields that check-manifests.sh
validates), and several tests/*.bats and tests/*.sh fixtures --
including a bats REPO_ROOT relative-path depth bug (10 files, one
extra .apm/ directory level to walk up) and a vale probe-path
isolation regression introduced mid-fix.
- Corrected empirically-wrong assumptions surfaced this session in
apm-workflow/apm-install's own reference docs: `apm marketplace
package add` does not accept local paths (only owner/repo remote
shorthand -- local packages are registered by editing apm.yml's
marketplace.packages[] directly); `apm compile` is a consumer-side
AGENTS.md/CLAUDE.md generator, not the plugin.json producer, and
hard-fails on skill/agent-only packages without --clean; `apm plugin
init <name>` nests a stray subdirectory when run with a positional
name arg from inside a same-named directory; no native Copilot
marketplace output profile exists; .mcp.json is merged into the
compiled plugin.json content-aware and target-scoped, with no
dependencies.mcp entry needed for simple passthrough; pipx is the
correct pip fallback on externally-managed Python environments.
- Renamed agent-author's copilot.agent.md template asset to
copilot.agent.md.template so apm compile's recursive *.agent.md glob
stops misparsing the placeholder template as a real agent primitive.
Impact:
plugin.json and marketplace.json are compiled artifacts from here on --
editing them by hand is no longer the workflow; edit apm.yml/.apm/ and
run apm pack. CONTEXT.md's Plugin/Plugin marketplace glossary entries
reflect this. ADR-0001 is marked superseded, ADR-0006 moot, and
ADR-0010 updated for the new .apm/agents/ path (project/user scope
unaffected, per ADR-0016). Full local verification: claude plugin
validate --strict on all 6 plugins, apm audit --ci, apm marketplace
check, check-manifests.sh, and the full test suite (165/165 bats,
13/13 shell scripts) all pass clean.
Fixes: #90
Refs: #88, #89
ADR: 0015
ADR: 0016
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ub96PyaSRD9BHPktotj1pC
validate.sh's detect_scope() and validate-provenance.sh's
find_plugin_root() disagreed with new-agent.sh's already-correct,
documented walk-up semantics on three points, each causing validate.sh
to false-FAIL a legitimately-scaffolded project-scope agent pair:
- a marker-less directory walked up into $HOME (no .git/apm.yml of its
own) was classified as user scope instead of project scope
- the .git-boundary branch returned the walked-to .git location instead
of the conventional scope root, breaking any <root> that is a
subdirectory of a larger git-tracked tree (monorepo package dirs)
- the new conventional-root arithmetic introduced to fix the above two
cases had no guard against non-conventional/hand-placed file paths,
which could point it at the wrong ancestor
Also adds scripts/check-scope-walkup-sync.sh, a behavioral drift-guard
(per ADR-0014's no-cross-skill-path precedent) that cross-checks the
four independently hand-ported walk-up implementations (validate.sh,
validate-provenance.sh, new-agent.sh, new-skill.sh) against real
fixture scaffolds, wired into .pre-commit-config.yaml at pre-push so
future drift between the ports is caught automatically.
Verified via bash tests/run-tests.sh (13/13) and targeted before/after
reproduction of each bug this closes.
pre-commit prefixes only entry[0] with the hook-repo clone path
(cmd = (prefix.path(cmd[0]), *cmd[1:])), so the --config argument in
.pre-commit-hooks.yaml resolved against the *consuming* repo's root
and hard-failed every external run with E100. Two of the three hooks
ADR-0014 promises were unusable.
vale-wrap.sh now self-locates its config from BASH_SOURCE when no
--config is supplied; an explicit --config still wins in all three
argv forms and stays cwd-relative. Both manifests drop the argument
and are kept byte-identical: the local repo: local config resolved
--config correctly only because the consuming repo *was* this repo,
and that divergence is why three review rounds missed the defect.
Also in the wrapper:
- replace GNU-only `realpath -m` with a portable abspath helper; -m is
load-bearing (dest does not exist yet), so BSD realpath aborted the
script under set -e on macOS
- walk directory arguments instead of passing them through unflattened,
which reported a clean 0-error run for files that fail when named
explicitly
- read/write with errors='surrogateescape' so one non-UTF-8 .md under a
directory argument cannot abort the hook
New test-vale-hooks-consumer.sh builds the hook repo from the working
tree and points a file:// consumer at it, covering the manifest as a
hook repo for the first time.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58
.pre-commit-hooks.yaml now exposes hooks to external consumers pinning
rev: <tag>, but nothing enforced that a tag actually gets cut when the
files it references change — relying on memory is exactly what this
repo's governance rules say to avoid for a repeatable, deterministic
check.
scripts/check-release-needed.sh hard-fails at pre-push, but only when
PRE_COMMIT_REMOTE_BRANCH (set by pre-commit's hook-impl) is
refs/heads/main: it diffs .pre-commit-hooks.yaml's referenced paths
against the last tag reachable from HEAD, and fails if either no tag
exists yet or something changed since. It's a silent no-op on every
other branch — hard-failing on feature-branch pushes mid-review would
force a premature tag on a commit that might not survive a
squash-merge, the exact risk the repo: local (vs. pinned self-
reference) decision in ADR-0014 already avoids for this repo's own
dev-time gate.
Verified against the real git pre-push hook path (not just the script
in isolation): simulated stdin matching git's pre-push protocol through
.git/hooks/pre-push, confirmed it correctly fires and fails when
targeting main with no tag, and is silent otherwise.
ADR: 0014
Refs: #87
skill-audit/agent-audit's Step 1 resolved vale-wrap.sh/.vale.ini via
`git rev-parse --show-toplevel`, which returns whichever repo the skill
happens to run in. Inside ai-development that works; in any external repo
that installs kyberforge@holocron as a plugin, it resolves to that repo's
own root, which has no .vale.ini — the prefilter silently fell back to
full LLM judgment. ADR-0013 named this as a deliberately deferred gap.
Vale's config/styles/wrapper now ship inside the plugin itself: a
canonical copy in agent-audit/assets/vale/ (Kyberforge + KyberforgeCopilot,
the superset agent-audit needs) and a smaller duplicate in
skill-audit/assets/vale/ (Kyberforge only) — per the no-cross-skill-path
rule already established for plugin cache-installs. Both skills resolve
these relative to their own directory, same as scripts/validate.sh
already does.
A new root .pre-commit-hooks.yaml exposes both copies plus
skill-size-check so any external repo can enforce the same rules via
`repo: <this-repo-url>, rev: <tag>` in its own pre-commit config,
independent of Claude Code entirely — the same mechanism covers CI. This
repo's own pre-commit hook now consumes the identical plugin-bundled
copies via repo: local (not a third root copy, and not a pinned
self-reference, which would lint working-tree edits against the last
tagged release instead of the change being made). Split into
vale-audit-prefilter-skill/-agent hooks after confirming, by diffing the
full corpus against both old and new config before deleting the old
files, that one combined hook pointed at only one copy silently 0-file-
skips the other file type.
scripts/check-vale-style-sync.sh guards the two copies against drift,
wired at pre-push alongside check-manifests.
ADR: 0014
Round-3 review of PR #85 found the "enforcing" pre-commit hook enforced
nothing. Vale's exit code keys on error-level alerts alone: five of the
six rules were level: warning, so they exited 0, and pre-commit hides
output from a passing hook — the alerts were invisible and blocked
nothing. ADR-0013 rejected a report-only trial tier and then shipped one
by accident.
Flatten every rule to level: error. Vale's own exit code is then correct,
so the hook entry drops to a bare vale-wrap.sh call and the graded
error->FAIL / warning->SUGGESTION mapping disappears from both audit
skills: every alert is a FAIL, in the gate and the audit alike. No
ignorable tier, matching shellcheck, the test suite and
conventional-pre-commit.
Delete Kyberforge.VagueQualifier. Measured against the 41 skill/agent
files as they stood before the rule ever ran: 2 hits. One marginal
("very different" -> "fundamentally different"), one an unfixable false
positive — caveman/SKILL.md quotes "of course" as an example of filler,
a mention not a use — which forced the only Vale suppression comments in
the repo. Those four lines go with it; two of them were dead anyway,
suppressing a frontmatter-scoped rule on a body line. Held-out prose (273
files) fired 15 times, 9 inside out-of-scope research examples and the
rest one word in two idioms in a single doc. SentenceOpenerThereIs
survives: 22 held-out hits, both in-corpus hits clean rewrites, zero
suppressions.
Widen .vale.ini's globs to [**/SKILL.md], [**/agents/*.md] and
[**/*.agent.md]. The plugins/*/-prefixed globs scoped nothing — Vale's *
crosses /, so they already matched docs/research/examples/**/agents/*.md
and assets/templates/SKILL.md, the two paths CONTEXT.md claimed they
excluded. Scoping is and was the hook's files: regex. The old globs also
hid a silent false negative: a skill outside plugins/ matched no section,
so Vale reported 0 files and exited 0, which both audits read as clean.
They now treat a 0-file run as NOT RUN and fall back to full judgment.
Also:
- vale-wrap.sh resolves relative --config values and file arguments
against the caller's cwd, as vale does, instead of the repo root, which
hard-errored from a subdirectory and silently skipped flattening for
file args that did not resolve from the root. Absolute paths inside the
cwd are relativized so reports cite resolvable paths, not scratch ones.
- vale-run's exit-code model was documented backwards ("exits non-zero
whenever it finds an alert at or above MinAlertLevel") and would have
led anyone following it to build a gate that passes everything. Its
Markdown suppression syntax was MDX-only and does not suppress in .md;
corrected in the skill and its troubleshooting reference, with
backtick/fence exemption documented as the first resort.
- skill-size-check.sh fails only above 500 lines, agreeing with
skill-audit's validate.sh <= 500 pass.
- ADR-0013 and CONTEXT.md amended to match, recording why graded
severities cannot gate.
Verified: 9 test scripts / 15 vale-wrap cases pass; vale-audit-prefilter,
skill-size-check and shellcheck pass --all-files; check-manifests and
claude plugin validate --strict clean. New tests fail against the old
script (3 of them) and pass against the new one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58
Addresses PR #85's outstanding review items after grilling the open
questions against ADR-0013/CONTEXT.md/ADR-0010:
Blocking fixes:
- vale-wrap.sh: replace json.dumps() escaping (which silently defeated
Vale's frontmatter scope on any description containing a quote,
backslash, or non-ASCII char — ~58% of the corpus) with a single-quoted
YAML scalar, substituting a Unicode right single quote for embedded
apostrophes rather than '' doubling (Vale's frontmatter scanner isn't a
full YAML parser and silently truncates on '' too).
- vale-wrap.sh: fix a blank-line-inside-a-folded-description truncation
bug via indentation-based, blank-line-tolerant body capture; narrow
flattening to `>`-style scalars only (`|` already works unflattened).
- skill-audit/agent-audit Step 1: make the vale-wrap.sh invocation
cwd-independent via git rev-parse --show-toplevel, fixing a bug where
no single cwd satisfied all three Step 1 commands.
- styles/Kyberforge/VagueQualifier.yml: prune 17 tokens verified
false-positive-dominated on this repo's own voice via a real corpus
sweep (obvious, clearly, usually, several, simple, easy, completely,
simply, tiny, etc.), keep 13 with real or unattested noise. Revert the
28 prose "fixes" those tokens drove across 14 skill files back to their
original, correct wording, including a functional regression to
caveman/SKILL.md's own filler-word list (a mention, not a use) — now
guarded with vale-off comments against recurrence.
Gaps:
- --minAlertLevel=warning on the pre-commit hook and Step 1 invocation
so warning-level rules actually surface, without collapsing the
FAIL/SUGGESTION severity mapping skill-audit/agent-audit rely on.
- vale-wrap.sh: fix --config=<path> equals-form, absolute-path silent
no-op, and a zero-file-argument stdin hang.
- Route vale-run and lint-runner through a documented wrapper script
when a target repo has one, instead of unconditionally recommending
bare `vale`.
- Wire Kyberforge.VagueQualifier/SentenceOpenerThereIs into skill-audit/
agent-audit's dimension-mapping prose (Body discipline).
- Add plugins/lint/sources.md provenance for lint-runner (ADR-0010).
- Sync both marketplace.json lint-entry descriptions with plugin.json.
- Retune skill-size-check.sh's MAX_WORDS 5000->2900 (measured ~1.6-1.7
tokens/word on this repo's corpus, the old value gated at ~8,500
tokens against a stated 5,000 ceiling); fix the >/>= line-count
boundary and wc -l undercount on files with no trailing newline.
- Document the vale binary as a Setup prerequisite in AGENTS.md.
- Fix SentenceOpenerThereIs's dead regex alternative and add a real
sentence-start anchor/scope.
- Fix a stale docs/research/docs/vale/ index pointer in kyberforge's
docs README (moved to plugins/lint/ in e1a5403).
- Rewrite ADR-0013's Consequences section past-tense to describe what
actually landed, and record the styles-portability limitation
(repo-root placement stays intentional; deferred to a separate
session per this PR's review).
Test coverage: 9 new vale-wrap.sh fixtures (quotes, backslash/unicode,
blank-line paragraphs, --config= form, zero-arg/absolute-path handling,
literal-block no-regression) and boundary-pair tests for
skill-size-check.sh's line/word ceilings.
bash tests/run-tests.sh: 9 scripts + 125 bats assertions, all passing.
scripts/check-manifests.sh and claude plugin validate --strict: clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58
Deferred item from PR #85 review. Per ADR-0013: cherry-picks two low-noise
rules from trialing write-good/alex against the real corpus (VagueQualifier,
SentenceOpenerThereIs) into styles/Kyberforge rather than adopting either
package wholesale (both are tuned for blog prose and were noisy on this
repo's terse, imperative instruction files - see the ADR's rejected-rule
list). Adds a new skill-size-check pre-commit hook enforcing agentskills.io's
500-line/5,000-token SKILL.md ceiling, currently unenforced. Fixes the 28
resulting violations across 20 existing SKILL.md/agent files so the
enforcing pre-commit hook lands clean.
governance.md/CONTROLS.md were evaluated and excluded as rule sources -
they're org/CI-infrastructure controls, not prose patterns Vale can express.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QUDczvw1H3eEeMD29Q9Lbi
Vale's text.frontmatter.description scope silently stops matching once
the description is a YAML block scalar spanning 2+ physical lines —
the style used by most skills/agents in this repo. scripts/vale-wrap.sh
flattens the description to one line in a scratch copy (preserving the
repo-relative path and total line count) before invoking real vale, and
both audit skills plus the pre-commit hook now call it instead of vale
directly. Also tightens the pre-commit hook's file glob to single path
segments so it can't cross into docs/research examples or asset
templates the way the audit skills' scoped invocations already avoid.
Addresses PR #85 review feedback.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FxG5T8EJDgkABXxuneuFfn
Adds vale-audit-prefilter as a local pre-commit hook scoped to skill/agent
markdown files, matching the invocation pattern skill-audit/agent-audit
already use. Runs at commit-stage only since it's a fast deterministic
prefilter; push-stage already covers the full test suite and manifest checks.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FxG5T8EJDgkABXxuneuFfn
Add --autofix to pretty-format-json so the hook rewrites files in place
rather than requiring manual intervention. Also applies the resulting key
ordering fixes to settings.json files.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
## Why
Five gap areas were identified when auditing the config against tracked file extensions and the hooks reference:
- No TOML validator despite 2 `.toml` files tracked
- No Python AST check despite 10 `.py` files tracked
- No merge-conflict marker detection
- No PEM private key block detection (gitleaks covers high-entropy strings but not raw PEM)
- No meta-validation to catch hooks that match no files or useless exclude patterns
## Impact
All five new hooks pass on `--all-files` run.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P87CiC58Ru2PPWYTeXtjHT
## Why
The root marketplace.json is the index that ties all plugins together.
claude plugin validate --strict covers schema-level checks that
check-manifests.sh does not, so it warrants its own hook alongside
validate-plugins.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
## Why
`claude plugin validate --strict` catches structural issues in plugin
manifests that `check-manifests.sh` does not cover (e.g. schema
violations, unrecognised fields). Running it at pre-push ensures all
plugins in `plugins/*/` stay valid before reaching the remote.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
