Files

Defame1297 620f20b0fd refactor(kyberforge)!: merge skill-audit and agent-audit into factory-audit

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

2026-09-16 09:13:57 +00:00

11 KiB

Raw Blame History

source_keys

agentskills-spec

agentskills-using-scripts

Validation Scripts Reference

Read this when a Step 1 script fails, cannot run, or reports something that needs interpreting — including validate-provenance.sh exiting 0 having printed something, which is INFO findings, not a clean run. Its silent exit 0 is the only outcome that needs nothing here.

Report the gap, do not guess

If a script cannot run at all — Bash denied, python3 unavailable, PyYAML not importable, vale not installed — say so as an INFO finding naming the script and the missing dependency, then fall back to the manual checks below. An INFO never changes PASS/FAIL. Silently omitting the dimension a script would have covered reports a clean audit that checked less than it claims to have checked, and the Step 4 coverage line then names a dimension nothing actually examined.

Manual structural fallback

validate.sh needs python3 and PyYAML, and refuses to start without either — the description value has to be measured after YAML folding is resolved, so skipping these gates would be a vacuous pass rather than a partial one. The two are checked separately, so the message already names the right one — report it verbatim rather than diagnosing further:

Error: python3 is required but was not found on PATH.
Error: PyYAML is required but is not importable by python3.

Without them — or with Bash denied, or on a permission error — work this list by hand and file the results under ### Structure exactly as the script's output would have been:

name present, 1–64 characters, kebab-case (lowercase letters, digits and hyphens; no leading, trailing or doubled hyphen), and matching the skill's directory name exactly.
description present and non-empty; no unfilled FILL IN: placeholder in it. An absent or empty description is a FAIL, never a silent skip — it is the one field preloaded into every session, so a skill without one can never be routed to.
Description length, measured on the folded YAML value with newlines collapsed to single spaces — not on the raw block scalar, which counts indentation. 250 characters SUGGESTION, 400 FAIL (house), 1,024 FAIL (agentskills.io spec).
Body length, counting everything after the frontmatter's closing ---. 600 words SUGGESTION, 900 FAIL (house).
Whole-file ceilings, counting the file including frontmatter: 500 lines FAIL, 2,770 words FAIL (agentskills.io spec). These are a different measurement from the two above — report them as separate findings, never merged.
A boundary clause is present — either the prose form (do not / instead / rather than / not for) or the compressed Not <thing> -> <name> arrow. SUGGESTION, not FAIL: the absence is deterministic, but whether this skill warrants one is the auditor's call.
Boundary targets resolve — FAIL on a name that resolves to nothing. See the section below; resolving these by hand is the one item on this list with a procedure of its own.
Every references/<file>.md named in the body exists on disk — FAIL, not a suggestion. A dispatch table or "read X" trigger naming a missing file sends the agent nowhere. Ignore mentions inside fenced code blocks, and ignore a mention whose own line says the file is gone (removed, deleted, renamed, superseded, replaced, obsolete, deprecated, former, gone, no longer, used to) — that is a historical note, not a dispatch entry.
Gotchas discipline, both SUGGESTION. Locate the section by a heading that is Gotchas (## Common Gotchas counts; ## Gotcha handling and ## Why gotchas matter do not), running to the next heading at the same level or shallower. More than five top-level entries is one suggestion; a section over 25% of the body word count is a second, independent one. Count entries at column 0 only — an indented child bullet is not an entry — and ignore fenced code blocks for both.
No unfilled FILL IN: placeholder anywhere in the body.
Every file in scripts/ carries the executable bit and contains no interactive prompt — no bare read, no select, nothing that blocks on a TTY.

Resolving boundary targets by hand

Targets are read from both boundary forms. The compressed Not <thing> -> <name> arrow and the prose form are each parsed and target-checked, so a typo in prose phrasing fails exactly as an arrow typo does — do not check only the names after an arrow.

Build the universe by walking up from the SKILL.md under audit, never from the validator's own location. The nearest ancestor holding plugins/*/.apm/skills/ or plugins/*/.apm/agents/ is the authoring root, falling back to the nearest ancestor holding .git. When one is found the universe is every skill and agent under <root>/plugins/*/, plus the skill's own apm package, plus the packages that package declares in its apm.yml under dependencies.apm. Deployed .claude/ and .agents/ trees are consulted only when no authoring root exists — they are gitignored apm install output, and reading them would make a fresh clone and a developer machine disagree.

Three ways to read the result wrong:

A hyphenated name used attributively is not a dangling target. "Use pre-commit hooks instead of ad-hoc scripts" reads as a route to pre-commit on wording alone. What separates a route from prose is grammar: a route target is terminal — followed by punctuation, a conjunction, or a boundary word — whereas a compound modifier is followed by the noun it modifies. A name followed by an ordinary noun still confirms a route when it exists, but never raises a FAIL on its own.
A SUGGESTION-tier unresolved target is not a FAIL you may promote. Terminal position alone is not evidence of a route: "run pre-commit instead", "see commit-msg" and "use the clean-up instead" are all terminal and all prose. A prose-form target earns a FAIL only when its own sentence names another target that does resolve; otherwise the script reports it and moves on, and so should you. Route notation — /name and -> name — is exempt and always FAILs, and it is the fix to recommend when the author did mean a route.
INFO boundary-target resolution DID NOT RUN is not a pass. The script prints it, and exits 0, when no universe could be determined for that path — the usual cause being a skill copy audited outside its package. Report it as an INFO naming the unchecked targets and re-run against the real directory; filing it as clean signs off targets nothing verified.

Script-specific failures

validate-provenance.sh printed nothing and exited 0. That is a pass, not a skip — it exits 0 silently when the skill has no source_keys and no references/sources.md, and nothing to validate is not a finding. Check the exit code before you believe the silence: a target that is not a directory, a directory holding no SKILL.md, a missing or extra argument, and an absent python3 all exit 2 with a message on stderr. Exit 2 means the script never ran — report it as an unaudited dimension, never as a pass and never as a finding. Exit 1 is findings.
A check-9 INFO — '<field>' changed for '<slug>' since <ref> — means go read, not just relay. Check 9 diffs the current references/sources.md against a base ref and flags a slug whose Description or Contributing files text differs. It is structurally incapable of telling you whether the new wording is still true — it only detects that the text changed — so when this INFO fires, open that slug's own entry: the document named in its Research doc: field, and the files its Contributing files list names. Read whichever the changed field is a claim about — a Description-only change often leaves the file list untouched, so "open the Contributing files" is where to look, not proof that they are what moved. Confirm by reading whether the (possibly strengthened) claim genuinely holds. This is the one provenance finding this script cannot verify for you: every other check here is a structural fact you can relay as-is, but check 9's job is only to tell you where to spend that reading effort, not to replace it. Acknowledging the INFO without opening those files is not auditing it. Its companion — '<field>' removed for '<slug>' since <ref> — is the same obligation in the other direction: a claim withdrawn rather than rewritten. No other check here requires the field, so confirm the removal was deliberate.
The check-9 base ref defaults to git merge-base HEAD origin/main, and there are two ways to override it. --base-ref=<ref> on the command line, or the VALIDATE_PROVENANCE_BASE_REF environment variable; the flag wins when both are given, including when it is given empty (--base-ref=), which selects the default resolution and ignores the environment. Reach for one on a fork, a long-lived branch, or a mirror whose remote is not called origin — and when a review asks what changed since a specific commit rather than since the branch point.
A single check-9 INFO naming a whole-check skip is an unaudited dimension, not a finding about the skill. There are three: "no repo root above the skill directory", "no base ref could be resolved", and "<path> is not tracked at <ref>". The third is the one to read carefully — it fires when the base ref resolved but git show <ref>:<path> did not, which covers both a genuinely new sources.md (nothing to flag) and a path git does not know under that name: a renamed skill directory, or an installed, gitignored copy such as a deployed .claude/skills/ tree. Auditing the deployed copy silently checks nothing; re-run against the authoring path under plugins/*/.apm/skills/.
vale reports 0 files. Treat the pass as NOT RUN, not as clean, and fall back to full Step 3 judgment for the dimensions it would have covered. The bundled Kyberforge style is scoped by glob in assets/vale/.vale.ini; a file outside those globs is silently not linted.
E100 Runtime error ... does not exist (exit 2) from vale-wrap.sh. An explicit relative --config was passed. Pass none: the wrapper locates its own assets/vale/.vale.ini from its own path, so a resolved script path plus an unresolved config path produces exactly this. Do not read this exit code as vale being unavailable — that misreading sends the audit down the fallback path while vale was installed and working the whole time.
The vale binary is genuinely absent (command not found). Report one INFO naming it, then fall back to full Step 3 judgment for the description, body-discipline and patterns dimensions — the prefilter's whole coverage. Judge those by rubric rather than dropping them.
A path argument that does not exist is a hard error in vale-wrap.sh, deliberately: bare vale would fall back to reading stdin and print a clean-looking 0 errors ... in stdin, which the 0 files guard above does not catch.

11 KiB Raw Blame History Unescape Escape