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).
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-vale-style-sync.sh's glob-coverage probe silently self-disabled when
vale was absent from PATH, exiting 0 on the one-character glob typo it exists
to catch. pre-commit swallows a passing hook's output, so the pre-push hook
reported Passed. The script already hard-fails on a bad REPO_ROOT for exactly
this reason -- "a clean exit 0 here would read as 'checked, in sync' when
nothing ran at all" -- and six of its assertions are vale invocations. Absence
now fails; the opt-out is an env var that must be set deliberately, and it
downgrades the run to text-level assertions while saying so in the summary.
Neither script had a floor on its rewritten .apm/ paths, so relocating .apm/
made both exit 0 -- and this PR's whole change to them was a path rewrite,
the exact edit that failure mode survives. A third gap the directory check
could not see: relocating only assets/vale/ left both audit skill directories
in place while every probe continued past its missing .vale.ini, skipping the
whole table with FAIL=0. A zero-probe run is now an error.
Both test suites encoded the vacuous pass as a passing case. Those cases are
now scoped to "no plugins/kyberforge at all" and assert the fixture really
lacks it, with new counterparts covering the drift shape and new positive
cases requiring each script to report a non-zero inspected-target count.
Also removes the HOOK_REGEX_CACHE memoization: every call site was a command
substitution, so the writes happened in a subshell and the lookup always
missed. Measured at 14ms of an ~870ms run, all of which is the six vale
invocations. Deleted rather than repaired -- 35 lines claiming a benefit they
never delivered is worse than no cache -- with a comment recording why.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
Issue #97 item 1 reports the unreadable-.vale.ini guard as untested. It
was worse: it was dead. `[[ -r ]]` is access(2), which asks whether the
permission bits would allow a read -- and for uid 0 that is yes even on
a mode-000 file. This hook runs at pre-push and the dev environment is
root, so the guard could never fire where it exists to fire. That is
why no uid-independent test for it existed; there was nothing to test.
Readability is now decided by actually reading (`cat`), which is
uid-independent and strictly stronger, catching EISDIR and EIO that
access(2) reports on neither. `cat`, not a `< "$ini"` redirect: opening
a directory for reading succeeds, only the read fails. The missing
branch moves to `-e`, so a directory sitting where the file belongs is
reported as unreadable rather than sending the reader hunting for a
deleted file.
The new case asserts the MESSAGE, not the exit code. With the guard
removed the script still exits 1 -- the greps hit the unreadable path
and blame a missing StylesPath on a file that has one. An exit-code-only
test would have been green with the guard deleted.
Also stops paying for vale in cases that only assert .vale.ini text:
21 of 28 script runs now mask it via the PATH_NO_VALE mechanism case 12
already builds, cutting the suite's bottleneck ~3.5x (issue #97 item 5).
The helper falls back to an unmasked run rather than skipping, so a
machine where masking is unavailable loses speed, never coverage.
That masking is a coverage gain, not only a speedup. With vale on PATH,
cases 8 and 9 could not detect deletion of the assertions they were
written to catch: a dropped StylesPath also breaks the glob probe, so
the script exited 1 for the wrong reason and both cases went green.
Verified against the pre-change files -- the same mutation was caught by
one incidental assertion before, and by three after.
Refs #97
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
Four repo gates reported success in states they exist to reject.
`check-vale-style-sync.sh` passed while a Kyberforge lint rule was silenced. The
check matched a blocklist of severity values, but Vale's semantic is an allowlist:
anything that is not exactly YES/error/warning/suggestion disables the rule. So
`= false`, `= 0`, `= garbage`, an empty value and — worst — a lowercase `= yes` all
killed enforcement while reading as "enabled" to a human. Inverted to an allowlist.
Two sibling holes: dropping `KyberforgeCopilot` from `BasedOnStyles` unloaded the
Copilot-only check silently, and narrowing a section glob to a location made Vale
lint zero files, which is the "0 files, hook Passed" failure the script's own
comment says it exists to catch.
`sync-marketplace-mirror.sh --check` failed open when its source was missing, while
its sibling correctly errored in the same state.
`check-scope-walkup-sync.sh` wrote to hardcoded `/tmp/fN.out` paths and read one
back, making it non-reentrant — a concurrent instance can flip a verdict, and this
branch made the test runner concurrent. Now per-run `mktemp -d`.
`check-manifests.sh` had no disk-to-marketplace pass, so a plugin directory absent
from `marketplace.json` passed every gate while the `validate-plugins` hook globbed
it. The "listed" match is restricted to remote-source entry names; matching any
entry name let a genuine orphan through on a name coincidence.
`run-bats.sh` reported an empty TAP stream as `0 tests, 0 failures`, exit 0 — a
total harness failure reading as a pass.
The test-side changes are the larger half, because the guards were the real problem.
`test-sync-marketplace-mirror.sh` could overwrite the live tracked mirror under an
inherited GIT_DIR, which is precisely the git-hook context it runs in. The bash-3.2
scan hand-maintained its file list, omitting the new shared runner, and had no rule
for `wait -n` or `nproc` — the two hazards the previous review round found live. It
now derives 43 files across three globs with per-glob floors. Several assertions
were decoration: the concurrency checks caught the reentrancy defect 0 times in 10,
the leak fix was green either way, and two manifest fixtures passed with the code
they claimed to cover deleted. Every assertion now has a revert it provably fails
against.
Refs: #90
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X7GvKuJfy2WrdBmUttV4DT
hook_file_regexes() unioned the `files:` regex from .pre-commit-hooks.yaml
and .pre-commit-config.yaml before checking whether a probe path is in
scope of a kyberforge vale-audit-prefilter hook. That union let a probe
matching only the old, looser .pre-commit-hooks.yaml pattern pass even
after .pre-commit-config.yaml's copy of the same hook had been narrowed
(e.g. to require a `.agent.md` suffix) -- silently masking exactly the
kind of hook-rescoping drift this check exists to catch. Per ADR-0014
the two manifests are meant to exercise the same resolution path an
external consumer's hook would, so this divergence is real drift, not
noise.
hook_file_regexes() now takes the manifest path explicitly and caches
per (skill, manifest) pair instead of per skill, so each manifest's
regex set can be inspected on its own. The probe-validation loop
computes in_hooks/in_config independently via a new matches_any_regex()
helper. Probes carry a new third heredoc field, `shared` or
`hooks-only`: `shared` probes (a file shape genuinely covered by both
manifests, e.g. plugins/demo/.apm/agents/demo.agent.md) must agree
between the two or the check now fails with a drift error;
`hooks-only` probes (a Copilot .agent.md living outside this repo's
own plugins/.apm/ layout, and the legacy bare-`.md`-under-agents/ shape
kept only to exercise a distinct .vale.ini glob section in isolation)
are exempt, since .pre-commit-hooks.yaml is deliberately broader there
by design. The original "matches no regex in either manifest"
staleness check is unchanged.
Added case 11b to tests/test-check-vale-style-sync.sh: narrows a
fixture's local config regex further while leaving
.pre-commit-hooks.yaml untouched, and asserts the check now flags it.
Confirmed red against the pre-fix script before applying the fix.
Refs: #95
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
A bad or stale REPO_ROOT argument fell through to the "neither copy
present" no-op guard and exited 0 — the exact "clean result can mean
nothing was checked" anti-pattern this PR spent multiple review rounds
eliminating elsewhere. That guard exists for a repo that legitimately has
no kyberforge plugin installed, not for a typo'd path.
Only the documented manual-invocation mode was affected: the shipped
pre-push hook always calls this script with zero args, which resolves via
`git rev-parse --show-toplevel` and is always valid inside a repo.
Added a regression test asserting a nonexistent REPO_ROOT exits non-zero.
Refs: #85
The external-consumer test asserted a combined alert count (>=2) across both
shipped Vale hooks, but the SKILL.md fixture alone raises two alerts — so one
working hook satisfied the threshold. Retargeting agent-audit's glob to match
nothing left the suite reporting "3 passed" under the message "both hooks
flatten and flag". The Skipped guard does not catch this: the hook still
matches the file, Vale lints nothing, reports 0 errors in 1 file and exits 0,
which pre-commit renders as Passed. An assertion aggregating over N subjects
proves nothing about any individual subject.
Each hook now runs individually and its alerts are attributed to the nearest
preceding path header, so an alert is checked by path rather than by presence
in the combined blob. The two fixtures carry distinct VagueWording tokens, so
one hook's alert cannot be credited to another.
Nothing in the repo read either .vale.ini — the sync check diffed only
vale-wrap.sh and styles/Kyberforge, so a one-line glob typo silently disabled
the prefilter for a whole file type. That was the enabling half of the same
defect. The check now asserts the shared lines both copies must carry
(StylesPath, a section naming Kyberforge as a whole word) without flagging
their intentional divergence, and probes each glob section by asking Vale
itself to lint a representative path. Regex-to-glob comparison was rejected as
it means reimplementing doublestar semantics in bash; a file-count dry-run was
rejected because a section whose glob matches but whose BasedOnStyles lost
Kyberforge reports "1 file" with no alerts and would pass it.
Every new assertion is bound to a failing case in both directions: breaking the
artifact fails the suite, and neutering the assertion fails exactly one case.
That reverse sweep exposed two assertions bound to no failing case at all, one
masked by a stronger check running first.
Refs: #85
The guard used `||`, so exactly one of the two audit skill directories
missing also exited 0, where the intended silent no-op is both absent.
A renamed skill-audit reported green instead of flagging that a
canonical style copy had lost its counterpart.
One-present now exits 1 naming the missing side and the remedy.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58
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
