7 Commits
Author SHA1 Message Date
Defame1297 9385c77ac7 Merge pull request 'feat(kyberforge): ADR-0020 context contract for skills and agents' (#103) from refactor/trim-skills-agents-context into main
Reviewed-on: https://git.dev.rkdr.net/Defame1297/holocron/pulls/103
Reviewed-by: Defame1297 <[email protected]>
2026-08-16 21:20:02 +00:00
Defame1297andClaude Opus 5 54d7bd80ba docs: rule host built-ins out of the routing target universe
Closes the second open design decision on PR #103. The `/compact` finding was
recorded as a false positive needing an allowlist or a suppression mechanism.
It is neither: the routing universe is the apm marketplace, so a target either
resolves to a skill or an agent or it does not resolve, and `/compact`,
`/clear` and `/init` are Claude Code slash commands with no counterpart in
Copilot CLI or Codex. `.apm/` source compiles for all three, so a
vendor-neutral description routing to one is a portability defect and the hard
FAIL is a true positive.

An allowlist was rejected for a concrete reason, not a stylistic one: it
answers a different question ("does this exist on some host?"), it cannot
answer that portably from a single source file, and it goes stale the next
time a host ships a command — reintroducing the same-commit-two-verdicts
failure ADR-0020 already closed for deployed trees.

Nothing is blocked today: zero of the 43 descriptions name a host built-in,
and an author who needs to mention one writes it un-slashed, which is not
route notation and carries no routing claim.

Recorded in ADR-0020 and in both author-facing contract references, so the
next agent reads the decision rather than "fixing" the gate.

ADR: 0020

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-16 20:45:19 +00:00
Defame1297andClaude Opus 5 75a13c82f6 fix(kyberforge): scope corroboration to a real sentence boundary
A prose-form routing target blocks a commit only when its own sentence names
another target that resolves. That makes the sentence splitter part of the
ADR-0020 contract rather than an implementation detail, and the naive
"period, space, capital" rule got it wrong in both directions:

- OVER-SPLIT: `e.g. "..."` is not a sentence end, but the quote looks like a
  start. The clause was cut in half and the corroborator stranded on the far
  side, so a genuinely dangling target silently demoted to SUGGESTION — a
  measurement taken and then discarded, the vacuous-green shape this gate
  exists to prevent. Seven such splits are live in the current corpus.
- UNDER-SPLIT: a sentence opening with a code span or a lowercase skill name
  was not seen as a start, so two sentences merged and a resolving target
  vouched for an unresolvable one it never stood beside — a hard FAIL with no
  escape hatch, which is the exact failure corroboration was added to prevent.

The splitter now excludes the five abbreviations that occur in routing prose
and admits a backtick or lowercase letter as a sentence opener. Applied
byte-identically to all three copies of the shared resolver.

Verified zero-delta against the corpus: 37 ERROR / 58 SUGGESTION / 2 dangling
before and after, findings byte-identical. The exposure this closes is to the
descriptions #99 is about to rewrite, not to the ones already measured — which
is why the deferral reason recorded on PR #103 ("can move the documented corpus
counts") does not hold and the fix lands here rather than after the retrofit.

Three regression tests, one per direction plus the backtick opener, each proven
non-vacuous by reverting the splitter alone and watching it go red.

Refs: #99
ADR: 0020

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-16 20:44:58 +00:00
Defame1297andClaude Opus 5 79c9089122 docs: make the resolution contract match what the gate actually does
Both AGENTS.md and ADR-0020 said deployed .claude/.agents trees are consulted "only
when no authoring root exists". That stopped being true in f7cc279: the walk-up
finds a root in any git repo, so the condition is now whether that root holds
plugins, not whether one was found at all. Left alone, the two documents describe a
resolver that no longer exists — and this repo's prose is load-bearing, since the
next agent reads it instead of the code.

Both now also record why a name-count delta is not an equivalent test, because it is
the obvious simplification and it is wrong: a single-plugin monorepo re-collects its
own package, adds no new name, and would pull the deployed trees back in.

ADR: 0020

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_015W3iwF9ncfRZddGBxsMCYi
2026-08-16 19:49:50 +00:00
Defame1297andClaude Opus 5 ede3f06689 fix(kyberforge): restore the authoring rules the ADR-0020 trim dropped
Diffing each retrofitted SKILL.md against its replacement references/ files found
rules that existed on main and now existed nowhere — relocated in intent, deleted in
fact. A trim that loses a rule is not progressive disclosure, it is data loss with a
smaller word count.

Three had no survivor. The least-privilege guidance for `tools` kept its mechanics
and lost the "restrict to what the agent needs" half, so the remaining text read as
encouragement to omit the field. The improve flow lost its regression check, so
nothing compared the closing audit against the pre-edit state and a PASS quietly
becoming a SUGGESTION went unnoticed — restored on both halves of the author pair,
since agent-author had dropped its equivalent too. And agent bodies lost "would the
agent get this wrong without it?", which mattered more than it looks: ADR-0020
deliberately sets no body word gate for agents, three of the four already sit
between 933 and 1,199 words, and the delegation check only fires on procedure a
skill already owns. That heuristic was the only brake left.

Two more were reachable only from the wrong scope. agent-author tells the reader to
load only the file for the resolved scope, but the mcp__ glob syntax for
disallowedTools and the five tools no subagent ever receives had both landed in
project-user-scope.md. disallowedTools is the ONLY permitted fence at plugin/APM
scope, so the scope that needs the syntax most could not reach it, and a plugin-scope
run could write a body telling the agent to ask the user a question.

Two documents were actively wrong rather than merely thin. agent-audit told auditors
that validate.sh resolves boundary targets for skills only; it runs at both scopes,
so the auditor was hand-resolving what the script had already decided and could
contradict it. And skill-audit routed to its script-troubleshooting reference
whenever validate.sh "fails" — but it exits 1 on ordinary content FAILs, the normal
outcome for the whole #99 population, so 1,302 words loaded on nearly every audit.
A context-budget regression inside the skill that enforces the context budget.

Finally, two illustrations taught the shape the gate ERRORs on, unfenced, while an
adjacent rubric called it a hard ERROR.

LESSONS.md records the reference-chain depth rule flipping from "one level deep" to
"two hops, never three". ADR-0020 is silent on it and the reversal rode entirely on
the diff; the looser rule is what mandatory dispatch requires.

Refs: #99
ADR: 0020

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_015W3iwF9ncfRZddGBxsMCYi
2026-08-16 19:49:38 +00:00
Defame1297andClaude Opus 5 b0d6d08239 test: pin the nine ADR-0020 gate defects that shipped untested
Every defect fixed in f7cc279 was reachable because nothing asserted against it.
The gate had 43 assertions and none of them covered a consumer repo, a non-string
description, an unclosed fence, or the two spec ceilings. Each case below fails
against the pre-fix code and passes against the current one; every one was proved
non-vacuous by mutating a scratch copy of the script and watching the test go red,
independently twice.

The two that mattered most had no fixture anywhere. A consumer repo WITH .git is
the shape the resolver exists to serve, and only the no-.git case had ever been
tested, which is exactly why the blocker was invisible. And ADR-0020 says the
walk-up runs in two passes specifically so a nested .git cannot beat a plugins/
root further up — no fixture had ever placed a .git inside a plugin.

test-adr0020-differential.sh loses _non_adr_hook_error(). It excluded MAX_LINES and
MAX_WORDS from the cross-script comparison on the untested assumption that awk and
splitlines() agree. They do not, and the divergence stayed invisible for exactly as
long as the exclusion stood. The ceilings are now compared like any other rule.

Two existing assertions were repairs, not additions. The skill-improve probe had
been fixed by this very branch, so its iteration permanently took an
assertion-free SKIP that still counted as a pass; both branches now fail loudly and
each names the other file's pin so the two stay in step. And the yaml-none fixture
emitted `---/---`, which never matched the frontmatter pattern at all — it passed on
the bare word "frontmatter", present in both messages, while never reaching the
branch it was named for. Needles throughout that file now name their branch.

Refs: #99
ADR: 0020

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_015W3iwF9ncfRZddGBxsMCYi
2026-08-16 19:49:15 +00:00
Defame1297andClaude Opus 5 f7cc27908c fix(kyberforge): close the vacuous-green and consumer-resolution defects
Review of the ADR-0020 gate found four ways it could exit 0 without measuring, and
one way it hard-failed a repo it had no business failing. On a gate shipping hot
with no baseline, a silent pass is the worst outcome available and a false block is
the second worst.

Consumer resolution was the blocker. _authoring_root() fell back to the nearest
.git, so it returned truthy in ANY git repo; _collect_authoring_root() then
contributed nothing and the deployed-tree branch was dead code in precisely the
consumer case it exists for. A consumer repo routing to an installed sibling got an
unblockable ERROR, and deleting .git "fixed" it. It now keys on which of the two
walk-up passes matched. A name-count delta was tried first and is wrong: a
single-plugin monorepo re-collects its own package and adds no new name, so the
delta reads zero and drags the deployed trees — including a global ~/.claude — back
into the universe. That reintroduces the install-dependence ADR-0020 forbids, one
layer down.

The three silent passes: an indented `---` inside a block scalar truncated the
frontmatter and reclassified the rest of the description as body; a non-string
description was str()-coerced, so `description: true` measured as the four-character
"True"; and an unterminated fence blanked the rest of the body, disabling the
ERROR-tier references/ check and the gotcha counts.

Two measurement defects came with them. The awk line/word counts discarded awk's
exit status, so an unreadable file passed both spec ceilings in total silence, and
awk NR/NF disagreed with the audit script's splitlines()/split() on Unicode
whitespace — the "fix one gate, get blocked by the other" bug, on the two axes the
differential test deliberately excluded. Both counts now run in the Python block
that already reads the file. A type error also no longer reports itself as a syntax
error.

Also: glob metacharacters in the checkout path silently disabled the resolver;
re.I was applied to some extraction patterns and not others; agent-audit missed
`tools:` written as a YAML block sequence, the shape Copilot files use; and a
nonexistent agent file raised a bare FileNotFoundError instead of a diagnostic.

The shared resolver block stays byte-identical across all three scripts. Corpus
output is unchanged — 26 description FAIL, 9 body FAIL, 2 dangling, 0 missing
references, 58 SUGGESTIONs — so no documented count moves.

Refs: #99
ADR: 0020

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_015W3iwF9ncfRZddGBxsMCYi
2026-08-16 19:48:56 +00:00
38 changed files with 1603 additions and 261 deletions

No files matched your search

+1 -1
View File
@@ -44,7 +44,7 @@ Fall back to raw shell only when no skill covers it.
- Install the `apm` CLI — four pre-push hooks shell out to it: `apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`, and `check-plugin-content-sync` (via `scripts/sync-plugin-content.sh`, which wraps `apm pack`). `apm-marketplace-check` and `apm-pack-check-clean` are bare `apm …` hook entries and `apm-audit-ci` is a `bash -c` loop calling `apm` once per package, so without it the push dies with an unhelpful "command not found". Use `apm-install`, or `curl -sSL https://aka.ms/apm-unix | sh`; verify with `apm --version`.
- Install `jq` — required by `scripts/check-manifests.sh` and `scripts/sync-plugin-content.sh`, both pre-push. These at least fail loudly (`Error: jq is required but not installed`).
- Install `python3` — required by `scripts/skill-size-check.sh`, the `skill-size-check` pre-commit hook. It measures the *folded* `description` value: most descriptions here are `>`-block scalars, so a regex over the raw lines measures indentation and newlines instead of the value. Missing it fails the hook with an install pointer rather than skipping the ADR-0020 checks, which would be a vacuous green. In practice it is already present — pre-commit is itself a Python application. **PyYAML is a hard requirement too**, not an optional accelerator: the hand-rolled fallback frontmatter reader has been removed, because a reader that mis-parses an unfamiliar scalar shape reports a clean pass on a file it never measured, which is the exact vacuous-green failure the `python3` check exists to avoid. `pip install pyyaml` if the hook reports it missing.
- That hook enforces **two independent gate families** over `plugins/*/.apm/skills/*/SKILL.md`, and neither replaced the other. The agentskills.io spec backstop is unchanged: 500 lines and 2,770 words, counted over the **whole file including frontmatter**. ADR-0020 adds a context budget measured differently — `description` 250 chars SUGGESTION / 400 FAIL (it is preloaded into every session whether the skill fires or not), **body-only** word count 600 SUGGESTION / 900 FAIL (everything after the frontmatter's closing `---`), a missing, valueless or `null` `description:` (a hard FAIL, not a skip — a gate that declines to measure the one preloaded field reports green), every boundary-clause routing target resolving to a real skill or agent, and every `references/<file>.md` a body names actually existing. Target resolution walks up **from the file being checked** to an authoring root — the nearest ancestor holding `plugins/*/.apm/{skills,agents}`, falling back to the nearest `.git`, in two passes so a nested `.git` cannot beat a real monorepo root. The universe is then every skill and agent under `<root>/plugins/*/`, plus the checked file's own apm package and whatever that package declares in its own `apm.yml` `dependencies.apm`; the **root** manifest's `dependencies:` block is not read, and no plugin here declares a cross-plugin apm dependency. Deployed `.claude/`/`.agents/` trees are consulted only when no authoring root exists — the consumer case. That matters because those trees are gitignored `apm install` output: resolution used to reach the four cross-plugin `gitea-*` → `git-*` targets through `.claude/skills/` alone, so the same commit measured 2 dangling targets on a developer machine and 6 on a fresh clone. It no longer does — verified by running the hook over a tree holding only `plugins/` and the root `apm.yml`, which reports findings identical to the working tree (26 description / 9 body / 2 dangling / 0 missing references / 58 SUGGESTIONs). Three further checks are SUGGESTION-only: a description with no boundary clause at all, a `## Gotchas` section with more than five entries, and a `## Gotchas` section over 25% of the body. A file can sit well inside one family and fail the other. The hook is `verbose: true` so the SUGGESTION tier is audible — pre-commit prints nothing at all for a passing hook, and a SUGGESTION deliberately does not fail. `skill-audit`'s `validate.sh` holds a second copy of the four ADR-0020 constants; `tests/test-skill-size-check.sh` asserts the copies agree.
- That hook enforces **two independent gate families** over `plugins/*/.apm/skills/*/SKILL.md`, and neither replaced the other. The agentskills.io spec backstop is unchanged: 500 lines and 2,770 words, counted over the **whole file including frontmatter**. ADR-0020 adds a context budget measured differently — `description` 250 chars SUGGESTION / 400 FAIL (it is preloaded into every session whether the skill fires or not), **body-only** word count 600 SUGGESTION / 900 FAIL (everything after the frontmatter's closing `---`), a missing, valueless or `null` `description:` (a hard FAIL, not a skip — a gate that declines to measure the one preloaded field reports green), every boundary-clause routing target resolving to a real skill or agent, and every `references/<file>.md` a body names actually existing. Target resolution walks up **from the file being checked** to an authoring root — the nearest ancestor holding `plugins/*/.apm/{skills,agents}`, falling back to the nearest `.git`, in two passes so a nested `.git` cannot beat a real monorepo root. The universe is then every skill and agent under `<root>/plugins/*/`, plus the checked file's own apm package and whatever that package declares in its own `apm.yml` `dependencies.apm`; the **root** manifest's `dependencies:` block is not read, and no plugin here declares a cross-plugin apm dependency. Deployed `.claude/`/`.agents/` trees are consulted only when the walk found no plugin monorepo root — whether it landed on a bare `.git` ancestor or on nothing at all (the consumer case). The gate keys on which of the two passes matched, not on whether the root contributed any new name: a single-plugin monorepo re-collects its own package and adds nothing, so a name-count test reads zero there and would drag the deployed trees back into the universe. That matters because those trees are gitignored `apm install` output: resolution used to reach the four cross-plugin `gitea-*` → `git-*` targets through `.claude/skills/` alone, so the same commit measured 2 dangling targets on a developer machine and 6 on a fresh clone. It no longer does — verified by running the hook over a tree holding only `plugins/` and the root `apm.yml`, which reports findings identical to the working tree (26 description / 9 body / 2 dangling / 0 missing references / 58 SUGGESTIONs). Three further checks are SUGGESTION-only: a description with no boundary clause at all, a `## Gotchas` section with more than five entries, and a `## Gotchas` section over 25% of the body. A file can sit well inside one family and fail the other. The hook is `verbose: true` so the SUGGESTION tier is audible — pre-commit prints nothing at all for a passing hook, and a SUGGESTION deliberately does not fail. `skill-audit`'s `validate.sh` holds a second copy of the four ADR-0020 constants; `tests/test-skill-size-check.sh` asserts the copies agree.
- **Those ADR-0020 gates ship hot, with no baseline file.** 26 of 39 descriptions and 9 of 39 bodies currently exceed their FAIL tier, so editing one of those skills *for any reason* means retrofitting it to the contract first — a one-line fix to `gitea-prs` cannot be committed until that skill complies. This is deliberate, and the retrofit is tracked as Gitea issue #99. Check where a skill stands before starting: `pre-commit run skill-size-check --all-files`.
- **A second gate ships hot alongside it, and `skill-size-check` will not warn you about it.** `Kyberforge.CompositionNote` — the ADR-0020 Vale rule banning composition and architecture prose from a description — currently fires **10 errors across four skills**: `gitea-issues`, `gitea-labels-milestones`, `gitea-prs` and `gitea-workflow`. Every Vale rule here is `level: error` with no ignorable tier, so touching any of those four means fixing its prose findings as well as its size findings. Scoping a retrofit off `skill-size-check` output alone will leave you blocked at the second gate. Check both: `pre-commit run --all-files`.
- Install the `vale` binary — required by the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks. Their `files:` patterns are `.apm/`-scoped: `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` and `^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$`. Only the authoring source triggers them — a `SKILL.md` in the generated mirror matches neither pattern, so prose findings surface only when you edit the file you are supposed to be editing. Without the binary the hooks fail with a bare "command not found" and no install pointer. `brew install vale` (macOS), `snap install vale` (Linux), `choco install vale` (Windows), or see https://vale.sh/docs/vale-cli/installation/. No `vale sync` needed — the `Kyberforge` styles are committed under `plugins/kyberforge/.apm/skills/{skill-audit,agent-audit}/assets/vale/styles/`, not downloaded packages (see ADR-0014).
+16
View File
@@ -245,3 +245,19 @@ noticed — and before describing any defect as pre-existing, run `git log -S` o
plugin-content and vale-style drift deterministically and has no equivalent gate asserting tool-owned
paths stay out of formatter scope — `.claude/settings.json` was the sixteenth exclude and nothing
prevents a seventeenth.
## 2026-08-16 — A rule reversed inside a retrofit leaves no trace unless someone writes it down
`skill-author/SKILL.md:204` on `main` said "Keep reference chains one level deep — a reference file
that references another reference file is rarely loaded correctly." The ADR-0020 retrofit replaced it
with "Two hops from `SKILL.md`, never three" in `references/create.md` and `references/retrofit.md`,
which permits exactly the chain the old rule banned. The looser rule is the right one and the
retrofit could not have shipped without it: dispatch pushes each flow into its own file, so the
shipped structure is `SKILL.md` → `improve.md` → `retrofit.md`, and a one-level ceiling would have
made the mandatory dispatch pattern illegal. But ADR-0020 says nothing about chain depth, so the
reversal was carried entirely by the diff — the new text asserts the new rule with no sign that a
contradicting rule ever existed, and a reader who remembers the old one has no way to tell whether it
was overturned or overlooked. Fix: when a change inverts a standing authoring rule rather than
tightening or restating it, record the inversion where the rule's rationale lives — the ADR if the
ADR is the reason, here otherwise. A rule that quietly flips is indistinguishable from a rule that
was forgotten, and the second reading is the one that gets it re-added later.
@@ -107,8 +107,12 @@ clause**, and a **boundary clause**. Capability enumeration, output-format detai
up. When an authoring root is found the universe is every skill and agent under
`<root>/plugins/*/`, plus the target's own apm package and the packages that package declares in
its own `apm.yml` `dependencies.apm`. Sibling plugins resolve against each other, which is what a
monorepo means. Deployed `.claude/`/`.agents/` trees are consulted **only** when no authoring root
exists — the consumer case, where there is no monorepo to read. What the resolver must never do is
monorepo means. Deployed `.claude/`/`.agents/` trees are consulted **only** when the walk found no
plugin monorepo root — whether it landed on a bare `.git` ancestor or on nothing at all. That is
the consumer case, where there is no monorepo to read. The condition is which of the two passes
matched, never a name-count delta: a single-plugin monorepo re-collects its own package and adds
no new name, so a delta test reads zero there and would pull the deployed trees back in. What the
resolver must never do is
derive the universe from its own location: a `${BASH_SOURCE}`-relative repo root leaked this repo's
39-skill universe into every consumer repo running the hook through pre-commit, so a consumer skill
routing to `skill-audit` resolved against a plugin it had never installed. Checked
@@ -125,6 +129,24 @@ clause**, and a **boundary clause**. Capability enumeration, output-format detai
dependency, and none needs to. Verified: a tree holding only `plugins/` and the root `apm.yml`,
with no `.claude/` or `.agents/` anywhere, now produces findings identical to the working tree —
26 description FAILs, 9 body FAILs, 2 dangling targets, 0 missing references, 58 SUGGESTIONs.
- **The universe is the apm marketplace, and nothing else.** A routing target resolves to a skill or
an agent, or it does not resolve. Host built-ins are deliberately outside it: `/compact`, `/clear`
and `/init` are Claude Code slash commands with no counterpart in Copilot CLI or Codex, so a
vendor-neutral `.apm/` description routing to one is a portability defect and the hard FAIL is a
true positive, not a false one. An allowlist of known built-ins was **rejected**: it answers a
different question ("does this exist on *some* host?"), it cannot answer that portably from a
single source file, and it goes stale the next time a host ships a command — reintroducing the
same-commit-two-verdicts failure the bullet above exists to close. An author who needs to mention
one writes it un-slashed (``the `compact` built-in``), which is not route notation and makes no
routing claim.
- **Blocking is scoped to a sentence, which makes sentence boundaries load-bearing.** A prose-form
target earns a hard error only when its own sentence names another target that *resolves*; route
notation (`/name`, `→ name`) is exempt and always blocks. So the splitter is part of the contract,
not a detail of it. `e.g. "…"` is not a sentence end, and a sentence opening with a code span or a
lowercase skill name is a start; getting either wrong moves targets between the two tiers in
opposite directions — a stranded corroborator silently demotes a real finding to SUGGESTION, and a
missed boundary lets one sentence vouch for a target it never stood beside, producing a hard FAIL
with no escape hatch.
- **The blanket pushiness rules are deleted.** `skill-author/SKILL.md:104` and
`description-quality.md:21` are replaced by a conditional: add an indirect trigger only where the
user's natural phrasing genuinely omits the domain word — true for the `gitea-*` family, false for
@@ -111,9 +111,12 @@ Flag as FAIL if:
available). `Kyberforge.VagueWording` catches the known filler; imprecision outside that list is
judgment.
- **A boundary clause naming a target that does not resolve** to a real skill directory or agent
file in the authoring source. No script checks this for an agent file — `validate.sh` resolves
boundary targets for skills only, so resolve the name yourself against `plugins/*/.apm/skills/`
and `plugins/*/.apm/agents/`.
file in the authoring source. `validate.sh` resolves this for agent files at both scopes and
reports each unresolved target itself — take its verdict rather than re-resolving the name by
hand, because a hand-walk over a different universe can contradict it. What is left to you is
semantic and the script cannot reach it: whether a target that *does* resolve is the right
sibling to exclude, and whether a clause naming no target at all ("examine the files manually")
should have named one.
- **`Use proactively` in a Copilot or vendor-neutral description.**
`KyberforgeCopilot.ProactivePhrase` catches it. The phrase steers the Claude Code runtime and
does nothing anywhere else, so in a `.agent.md` it is preloaded text that buys no behaviour.
@@ -222,17 +222,21 @@ def read_text(path):
# which is what a monorepo means,
# 2. the target's own apm package,
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
# case. They are `apm install` output, gitignored, and present only on a machine
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
# same commit measured 2 dangling targets on a developer machine and 6 on a
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
# root came from the plugins/ probe. They are `apm install` output, gitignored,
# and present only on a machine that has run it: four cross-plugin targets in
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
# developer machine and 6 on a fresh clone. A gate shipping hot with no baseline
# cannot give two answers.
#
# Deployed trees are used only when NO authoring root exists — the consumer
# case, where the file being checked lives in or beside a deployed tree and
# there is no monorepo to read.
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
# landed on a bare .git ancestor or on nothing at all. That is the consumer
# case: the file being checked lives in or beside a deployed tree, inside an
# ordinary git repo, with no monorepo to read. The two cases are told apart by
# which probe matched, never by how many names a root contributed; see
# known_targets().
def _is_fs_root(path):
@@ -241,11 +245,17 @@ def _is_fs_root(path):
def _collect_package(pkg_dir, names):
"""Add every skill/agent name a package directory exposes, any layout."""
# glob.escape() the DIRECTORY only. A checkout path containing `[`, `]`,
# `*` or `?` — a worktree named `feature[2]`, say — otherwise turns the
# whole pattern into a character class that matches nothing, and the
# resolver degrades to the "DID NOT RUN" INFO with rc=0 across every file
# in the tree. The wildcards in `sub` are the intended ones and stay raw.
safe_dir = glob.escape(pkg_dir)
for sub in ('.apm/skills/*/', 'skills/*/'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
names.add(os.path.basename(path.rstrip('/')).lower())
for sub in ('.apm/agents/*.md', 'agents/*.md'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
base = os.path.basename(path)
if base.endswith('.agent.md'):
base = base[:-len('.agent.md')]
@@ -277,28 +287,35 @@ def _apm_package_root(start_dir):
def _authoring_root(start_dir):
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
Returns (root, matched_plugins_probe). The flag reports WHICH probe
matched: True for the plugins/*/.apm/{skills,agents} glob, False for the
.git fallback and for no match at all. known_targets() needs that
distinction — only a real plugins/ root makes the deployed trees
redundant, and a name-count delta cannot tell the two apart.
Two passes, not one interleaved walk: a nested .git (a submodule, a
worktree of a sub-package) must not win over a real plugins/ root further
up. Both passes stop before the filesystem root for the same reason
_apm_package_root does.
"""
for probe in (
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git'))):
probes = (
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git')))
for index, probe in enumerate(probes):
current = os.path.abspath(start_dir)
for _ in range(12):
if _is_fs_root(current):
break
if probe(current):
return current
return current, index == 0
current = os.path.dirname(current)
return None
return None, False
def _collect_authoring_root(root, names):
"""Every plugin in the monorepo contributes its names."""
for pkg in glob.glob(os.path.join(root, 'plugins', '*')):
for pkg in glob.glob(os.path.join(glob.escape(root), 'plugins', '*')):
if os.path.isdir(pkg):
_collect_package(pkg, names)
@@ -362,7 +379,7 @@ def _declared_dependency_dirs(pkg_dir):
def _deployed_roots(start_dir):
""".claude/ and .agents/ trees above start_dir — what a host really sees.
Consulted ONLY when no authoring root exists; see the section header. The
Consulted ONLY when no plugin monorepo root was found; see the header. The
filesystem root is skipped for the same reason _apm_package_root skips it:
a stray /.claude/skills/ must not join every path's universe.
"""
@@ -401,10 +418,22 @@ def known_targets(start_dir):
for dep_dir in _declared_dependency_dirs(package):
_collect_package(dep_dir, names)
root = _authoring_root(start)
# A .git ancestor is an authoring root only if it actually holds plugins.
# _authoring_root() falls back to the nearest .git, so it is truthy in ANY
# git repo; without the distinction that fallback wins in every consumer
# checkout, _collect_authoring_root() contributes nothing, and the deployed
# branch below is dead code in the exact case it exists for. So condition
# on WHICH probe matched, which _authoring_root() reports directly. A
# name-count delta looks equivalent and is not: _collect_authoring_root()
# re-collects the checked file's own plugin, whose names the blocks above
# already added, so a one-plugin monorepo shows a delta of zero and would
# wrongly reach for the deployed trees — including the user's global
# ~/.claude/skills, making the verdict depend on what happens to be
# installed (ADR-0020 lines 118-127).
root, root_has_plugins = _authoring_root(start)
if root:
_collect_authoring_root(root, names)
else:
if not root_has_plugins:
for base in _deployed_roots(start):
_collect_package(base, names)
return names
@@ -510,16 +539,43 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
ROUTE_MARKED = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, MARKED_TARGET), re.I)
ROUTE_ANY = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, ANY_TARGET), re.I)
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
# re.I on ALL of them, uniformly. The patterns are built from the same
# lowercase NAME_* fragments, so half of them carrying the flag and half not
# meant `Skill-Audit` at the start of a boundary sentence was extracted by
# ROUTE_ANY but invisible to BACKTICK — contradicting normalize_target()'s own
# docstring, which exists precisely because extraction is case-insensitive and
# the universe is not.
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET, re.I)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET, re.I)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I)
ARROW_BOUNDARY = re.compile(r"\bnot\b[^.;]*?(?:->|→)\s*(%s)\b" % NAME_HYPH, re.I)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH, re.I)
# A boundary clause takes two shapes and BOTH count: the prose markers, and
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
BOUNDARY_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
BOUNDARY_ARROW = re.compile(r"\bnot\b[^.;]*?(?:->|→)", re.I)
SENTENCE_SPLIT = re.compile(u'(?<=[.!?])\\s+(?=[A-Z"“(])')
# Sentence boundaries decide the CORROBORATION scope above, so getting one wrong
# is not cosmetic — it moves a target between SUGGESTION and blocking ERROR. Two
# shapes common in these descriptions defeat the naive "period, space, capital"
# rule, in OPPOSITE directions:
# OVER-SPLIT. `e.g. "set up the manifest"` ends no sentence, but the quote
# looks like one starting. The clause is cut in half, the corroborating
# target lands on the far side of the cut, and a genuinely dangling target
# silently demotes to SUGGESTION — the gate takes a measurement and then
# throws it away, which is the vacuous-green shape this file exists to stop.
# UNDER-SPLIT. A real sentence opening with a code span or a lowercase skill
# name ("... Composes it. `gitea-prs` also uses it.") is not seen as a start
# at all, so two sentences merge and a resolving target vouches for an
# unresolvable one it never stood beside — a hard FAIL with no escape hatch,
# which is exactly the failure the corroboration rule was added to prevent.
# Both are closed here: the five abbreviations that actually occur in routing
# prose are excluded as sentence ends, and the opener class admits a backtick or
# a lowercase letter. Verified zero-delta on the current corpus (37 ERROR / 58
# SUGGESTION / 2 dangling before and after) — this protects the descriptions
# issue #99 is about to rewrite, not the ones already measured.
SENTENCE_SPLIT = re.compile(
u'(?<!\\be\\.g\\.)(?<!\\bi\\.e\\.)(?<!\\betc\\.)(?<!\\bvs\\.)(?<!\\bcf\\.)'
u'(?<=[.!?])\\s+(?=[A-Za-z`"“(])')
# The token that may follow a route target without turning it into a compound
# modifier: punctuation, end of sentence, a conjunction, a boundary word, or a
@@ -681,8 +737,16 @@ def unresolved_targets(description, known):
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
# be measured must never report green, so every caller of these two ERRORs on a
# miss instead of moving on.
#
# The CLOSING marker is anchored at column 0 — deliberately NOT `[ \t]*---`.
# YAML block-scalar content must be indented deeper than its key, so an
# indented `---` inside a folded description is CONTENT; letting it close the
# frontmatter truncated the description mid-value and silently reclassified the
# rest as body, which is a vacuous green in both directions at once. Leading
# whitespace is still tolerated on the OPENING marker, where no such content
# can exist.
FRONTMATTER_RE = re.compile(
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n[ \t]*---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
def strip_bom(text):
@@ -714,14 +778,26 @@ def description_value(fm_text):
try:
data = yaml.safe_load(fm_text)
except Exception as exc:
raise FrontmatterError(re.sub(r'\s+', ' ', str(exc)).strip())
# Every FrontmatterError message is a COMPLETE clause, never a detail a
# caller wraps in one. Callers used to prefix a hard-coded "frontmatter
# is not valid YAML (...)", which is true only of this branch: the two
# type failures below come from frontmatter that parsed fine, and
# telling their author the YAML is invalid sends them hunting for a
# syntax error that is not there — on a blocking gate with no baseline.
raise FrontmatterError('frontmatter is not valid YAML (%s)'
% re.sub(r'\s+', ' ', str(exc)).strip())
if not isinstance(data, dict):
raise FrontmatterError('frontmatter is not a YAML mapping')
value = data.get('description')
if value is None:
return ''
if not isinstance(value, str):
value = str(value)
# NOT str()-coerced. `description: true` became the 4-character "True"
# and sailed through the 400-character gate; a list or mapping was
# measured as its Python repr. Neither is a description a host can
# preload, so this is a parse failure, reported as one.
raise FrontmatterError(
'description is a %s, not a string' % type(value).__name__)
return re.sub(r'\s+', ' ', value).strip()
@@ -791,6 +867,18 @@ def mask_fenced(text):
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
and not stripped.strip()[len(marker):].strip()):
fence = None
# An UNCLOSED fence has no cost-free answer, only a choice of which way to
# be wrong. Masking to end-of-body blanks the rest of the body, silently
# disabling the ERROR-tier references/ check and the gotcha counts.
# Returning the raw text instead exposes the unclosed example's own
# content, so a fenced example naming a nonexistent references/ file
# becomes a hard ERROR it would not have been had the fence been closed —
# confirmed, not hypothetical. The loud-false-positive direction is the one
# chosen: this script's rule is that a file it cannot measure must never
# report green, and masking-onward is exactly that failure. Both outcomes
# need an already-malformed file, and the false positive costs one fence.
if fence is not None:
return text
return ''.join(out)
@@ -869,12 +957,15 @@ def get_frontmatter_keys(fm):
return keys
def agent_description(fm, local_fname):
"""The folded description VALUE, or None if the frontmatter is not YAML."""
"""The folded description VALUE, or None if it could not be read."""
try:
return description_value(fm)
except FrontmatterError as exc:
fail(f"frontmatter is not valid YAML ({exc}) — the ADR-0020 description and "
f"boundary-target gates could not run — {local_fname}")
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or
# a description of the wrong type. Do not prefix a diagnosis here; the
# last one named a syntax error for two failures that have none.
fail(f"{exc} — the ADR-0020 description and boundary-target gates could "
f"not run — {local_fname}")
return None
def check_description_budget(value, local_fname):
@@ -944,11 +1035,31 @@ def check_boundary(value, fpath, local_fname):
f"{local_fname}")
def extract_tools_list(fm):
"""Extract tool names from the tools frontmatter field (space or comma separated)."""
val = extract_field(fm, 'tools')
if not val:
"""Tool names from the `tools` field — inline scalar OR YAML block sequence.
Read off the PARSED mapping, never off extract_field(). That function's
capture is newline-bounded on purpose (`[^\\S\\r\\n]*(.+)`), so a `tools:`
written as a block sequence — the shape Copilot agent files use — captured
nothing at all and the subagent-unavailable-tool check silently stopped
firing on exactly the files it was written for. Both spellings are legal
YAML, so both are read here.
"""
try:
data = yaml.safe_load(fm)
except Exception:
# Not this function's failure to report: the frontmatter's validity is
# decided (and failed) by agent_description() on the same text.
return set()
return set(re.split(r'[\s,]+', val.strip()))
if not isinstance(data, dict):
return set()
val = data.get('tools')
if isinstance(val, list):
items = [str(item).strip() for item in val]
elif isinstance(val, str):
items = re.split(r'[\s,]+', val.strip())
else:
return set()
return {item for item in items if item}
def is_copilot_cloud_ide(fpath):
"""True if the file is a cloud/IDE Copilot agent (name is optional for these)."""
@@ -1063,6 +1174,15 @@ def check_apm_agent_file(fpath, allowlist, stem):
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
f"failure, not a skip — {local_fname}")
return
except OSError as exc:
# A path that cannot be opened gets a FAIL line naming it, not a bare
# FileNotFoundError traceback. scripts/check-apm-agents-valid.sh takes
# this path for an agent file deleted from the worktree but still
# tracked in the index — a real, expected state, and the caller needs to
# be told which file, not handed an interpreter stack.
fail(f"could not be read ({exc.strerror or exc}): {fpath}. Nothing could "
f"be measured, so this is a hard failure, not a skip — {local_fname}")
return
fm, body = parse_frontmatter(content)
if fm is None:
@@ -1168,6 +1288,13 @@ def check_file(fpath, file_provider):
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
f"failure, not a skip — {local_fname}")
return
except OSError as exc:
# Same reason as check_apm_agent_file's: a diagnostic naming the path
# beats a FileNotFoundError traceback. The counterpart is pre-checked at
# the bottom of this script, but agent_file itself never was.
fail(f"could not be read ({exc.strerror or exc}): {fpath}. Nothing could "
f"be measured, so this is a hard failure, not a skip — {local_fname}")
return
fm, body = parse_frontmatter(content)
if fm is None:
@@ -810,3 +810,136 @@ EOF
assert_success
refute_output --partial "hooks"
}
# ---------------------------------------------------------------------------
# tools: — both YAML spellings
# ---------------------------------------------------------------------------
# The subagent-unavailable-tool SUGGESTION is read off the `tools` field, and
# `tools` has two legal spellings: an inline scalar and a block sequence. The
# field used to be pulled out with a line regex whose capture is newline-bounded
# on purpose, so a block sequence captured NOTHING and the check silently
# stopped firing — on the shape Copilot agent files actually use, which is to say
# on the files it was written for. Both spellings are pinned, and they are pinned
# together: the inline case alone was green throughout.
# make_pair <root> <tools-frontmatter> — a project-scope CC + Copilot pair
# carrying the same `tools` value in both files. `tools` is on neither the
# claude-code-only nor the copilot-only list, so it is legal in both and the pair
# stays otherwise clean; the description carries a boundary clause so the only
# SUGGESTION that can fire is the one under test.
make_tools_pair() {
local root="$1" tools="$2"
mkdir -p "$root/.git" "$root/.claude/agents" "$root/.github/agents"
local f
for f in "$root/.claude/agents/my-agent.md" "$root/.github/agents/my-agent.agent.md"; do
{
echo "---"
echo "name: my-agent"
echo "description: A valid agent description. Do not use for anything else."
echo "$tools"
echo "---"
echo ""
echo "You are a test agent. When invoked, do the thing."
} > "$f"
done
}
@test "a subagent-unavailable tool in an INLINE tools scalar raises a SUGGESTION" {
make_tools_pair "$TMPDIR/inline" "tools: Read ExitPlanMode"
run bash "$SCRIPT" "$TMPDIR/inline/.claude/agents/my-agent.md"
assert_success
assert_output --partial "'ExitPlanMode' is listed in tools but is never available to subagents"
}
@test "a subagent-unavailable tool in a BLOCK SEQUENCE tools field raises the same SUGGESTION" {
make_tools_pair "$TMPDIR/block" "$(printf 'tools:\n - Read\n - ExitPlanMode')"
run bash "$SCRIPT" "$TMPDIR/block/.claude/agents/my-agent.md"
assert_success
assert_output --partial "'ExitPlanMode' is listed in tools but is never available to subagents"
}
@test "a tools list with no subagent-unavailable tool stays silent in both spellings" {
# The control. Without it both cases above are satisfied by a check that
# fires on every tools field it can see, which would be the opposite defect.
make_tools_pair "$TMPDIR/inline-clean" "tools: Read Edit"
run bash "$SCRIPT" "$TMPDIR/inline-clean/.claude/agents/my-agent.md"
assert_success
refute_output --partial "never available to subagents"
make_tools_pair "$TMPDIR/block-clean" "$(printf 'tools:\n - Read\n - Edit')"
run bash "$SCRIPT" "$TMPDIR/block-clean/.claude/agents/my-agent.md"
assert_success
refute_output --partial "never available to subagents"
}
# ---------------------------------------------------------------------------
# A file that cannot be read
# ---------------------------------------------------------------------------
# scripts/check-apm-agents-valid.sh derives its expected agent-file set from
# `git ls-files`, so it hands this script paths that are tracked but absent from
# the worktree — a real and expected state, not a corner case. That used to exit
# 1 with a bare FileNotFoundError traceback and no FAIL line at all: non-zero, so
# the gate blocked, but with an interpreter stack instead of a diagnostic naming
# the file. Both scope paths are covered because they are separate call sites
# (check_apm_agent_file and check_file) and each needed its own handler.
#
# `is-a-dir.agent.md` is a DIRECTORY rather than a chmod 000 file on purpose:
# these tests run as root in CI, where mode bits do not deny anything and a
# permissions fixture would be silently readable and prove nothing.
@test "a nonexistent plugin/APM-scope agent file gets a FAIL naming the path, not a traceback" {
local root="$TMPDIR/pkg"
mkdir -p "$root/.apm/agents"
cat > "$root/apm.yml" <<EOF
name: test-package
version: 0.1.0
type: skill
EOF
run bash "$SCRIPT" "$root/.apm/agents/absent.agent.md"
assert_failure
assert_output --partial "FAIL"
assert_output --partial "could not be read"
assert_output --partial "absent.agent.md"
refute_output --partial "Traceback"
refute_output --partial "FileNotFoundError"
}
@test "an unreadable plugin/APM-scope agent file gets a FAIL naming the path, not a traceback" {
local root="$TMPDIR/pkg-dir"
mkdir -p "$root/.apm/agents/is-a-dir.agent.md"
cat > "$root/apm.yml" <<EOF
name: test-package
version: 0.1.0
type: skill
EOF
run bash "$SCRIPT" "$root/.apm/agents/is-a-dir.agent.md"
assert_failure
assert_output --partial "FAIL"
assert_output --partial "could not be read"
assert_output --partial "is-a-dir.agent.md"
refute_output --partial "Traceback"
refute_output --partial "IsADirectoryError"
}
@test "a nonexistent project-scope agent file gets a FAIL naming the path, not a traceback" {
# The counterpart is pre-checked before either file is opened, so this
# exercises the OTHER call site: the counterpart exists, the named file does
# not, and check_file is what has to report it.
local root="$TMPDIR/proj-missing"
mkdir -p "$root/.git" "$root/.claude/agents" "$root/.github/agents"
cat > "$root/.github/agents/my-agent.agent.md" <<EOF
---
name: my-agent
description: A valid agent description. Do not use for anything else.
---
You are a test agent. When invoked, do the thing.
EOF
run bash "$SCRIPT" "$root/.claude/agents/my-agent.md"
assert_failure
assert_output --partial "FAIL"
assert_output --partial "could not be read"
assert_output --partial "my-agent.md"
refute_output --partial "Traceback"
refute_output --partial "FileNotFoundError"
}
@@ -53,6 +53,8 @@ Gates `agent-audit` enforces at every scope:
- **Body** — no word gate, and a delegation check in its place: name the skill to invoke rather than restating what it does.
- **Invocation** — decide whether the agent is model-delegated or reached only by name. Only Copilot's cloud/IDE format expresses that in frontmatter (`disable-model-invocation`, `user-invocable`).
At every scope, five tools reach no subagent whatever `tools` says — `AskUserQuestion`, `EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup`, `WaitForMcpServers`. Never write a body that has the agent ask the user a question or enter plan mode; it describes a turn the runtime cannot give it.
## Step 4 — Validate and close
Invoke `agent-audit` on each file written and resolve every FAIL before reporting done. It checks the field allowlist, name-to-stem match, leftover placeholders and template comments, the description budget and the Copilot body limit — do not hand-check those.
@@ -25,12 +25,13 @@ description: FILL IN: Use when <trigger>. <One capability clause.> Not <thing> -
<!-- tools: Read, Bash, Grep
Optional. Allowlist of tool names: a comma-separated string or a YAML list.
Omit to inherit all tools from parent.
Restrict it to what the agent actually needs. Omit only when it needs them
all — omitting inherits every tool from the parent.
Use Agent(type1,type2) to restrict which subagent types this agent can spawn.
Omit Agent entirely to prevent this agent from spawning subagents.
Never available to subagents regardless of tools field:
AskUserQuestion, EnterPlanMode, ExitPlanMode, ScheduleWakeup, WaitForMcpServers
Exception: ExitPlanMode IS available when parent session runs in permissionMode: plan -->
Listing any of them is a finding: agent-audit enforces the flat rule. -->
<!-- model: sonnet
Optional. Aliases: sonnet, opus, haiku, fable. Or full model ID.
@@ -70,6 +70,12 @@ that package declares in `apm.yml` under `dependencies.apm`. A sibling plugin in
therefore resolves; a skill in an unrelated repo does not. A target outside that universe sends the
router nowhere. Verify it before writing it — do not invent a plausible sibling.
That universe is the apm marketplace and stops there. A **host built-in is not a routing target**:
`/compact`, `/clear` and `/init` are Claude Code slash commands with no counterpart in Copilot CLI
or Codex, and `.apm/` source compiles for all three, so routing to one is a portability defect. The
gate is right to fail it and there is no allowlist. If a built-in genuinely needs mentioning, write
it un-slashed — ``the `compact` built-in`` — which makes no routing claim and is not checked.
**Length.** 250 characters SUGGESTION, 400 characters FAIL, counting the frontmatter value only
with YAML folding resolved. Treat 250 as the target: the SUGGESTION tier is what moves the corpus
average, the FAIL tier only stops outliers.
@@ -57,6 +57,11 @@ procedure a skill it can invoke already owns is an `agent-audit` FAIL. When a si
missing procedure, check first whether an installed skill owns it and name that skill instead of
transcribing it. See `references/contract.md`.
The delegation check is not a length brake — it fires only on procedure an invocable skill already
owns, and says nothing about original prose. That brake is judgment, and it is the only one left:
for every sentence you add, ask "would the agent get this wrong without it?" and delete it if the
answer is no.
**Explain the why.** Reasoning-based instructions outperform rigid directives. A rule written in
all caps (ALWAYS/NEVER) is usually better reframed as why the behaviour matters, so the agent can
apply judgment at the edges.
@@ -73,4 +78,10 @@ that was already there.
If the edit adds or removes research-sourced content, update `source_keys` in the edited file and
the matching `sources.md` entry — the create flow's Step 3 has the rules.
**Check for regressions before handing back.** `SKILL.md` Step 4 tells you to resolve every FAIL,
which says nothing about a check that passed *before* these edits and no longer does. Compare the
closing `agent-audit` against the agent's pre-edit state — a PASS that has become a SUGGESTION, or
a SUGGESTION that has become a FAIL, is damage this flow caused and is in scope for it. Only the
improve flow can make that comparison; the create flow has no prior state to compare against.
Then return to `SKILL.md` Step 4.
@@ -37,6 +37,11 @@ The rule is about a field's *shape*, not a fixed roster:
Claude Code honours it for plugin subagents; the three fields plugin agents do silently ignore
are `hooks`, `mcpServers` and `permissionMode`, and this is not one of them. Copilot's handling
of the key is unconfirmed, which ADR-0016 accepts as a stated risk.
Its syntax is the same at every scope, and this is the one scope that cannot reach it anywhere
else: MCP tools are denied as `mcp__<server>`, `mcp__<server>__*` or `mcp__*`; both a YAML list
and a delimited string are accepted, and this repo writes the comma-separated string form
(`disallowedTools: Edit, Write, NotebookEdit`) — match it.
- The Claude-only knobs (`isolation`, `maxTurns`, `effort`, `memory`, `permissionMode`, `skills`,
`color`, `initialPrompt`, `background`, `hooks`, `mcpServers`) have no Copilot equivalent and
are never written to this file at all. "Silently ignored at plugin scope" is the wrong framing:
@@ -25,7 +25,10 @@ duplicate silently.
**`description`** — write it against `references/contract.md`. It is the primary signal for
autonomous delegation.
**`tools`** — an allowlist; omit it to inherit every tool from the parent. Use `Agent(type1,type2)`
**`tools`** — an allowlist. Write it, and restrict it to the tools the agent actually needs;
omitting it inherits every tool from the parent, which is the right value only when the agent
genuinely needs all of them. Least privilege is the default, not the exception. Use
`Agent(type1,type2)`
to restrict which subagent types this agent may spawn, and omit `Agent` entirely to stop it
spawning any. Five tools reach no subagent whatever this field says — `AskUserQuestion`,
`EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup` and `WaitForMcpServers` — so listing one buys
@@ -35,7 +35,7 @@ scripts/vale-wrap.sh <skill-dir>/SKILL.md
`validate.sh` findings become the `### Structure` dimension — its FAILs and its SUGGESTIONs both.
If any of the three fails, cannot run, or reports something needing interpretation, read `references/validation-scripts.md` — it carries the manual fallback and the misleading exit codes.
If any of the three cannot run, or exits non-zero for a reason other than findings, read `references/validation-scripts.md` — it carries the manual fallback and the misleading exit codes. Ordinary content FAILs are the expected outcome here and need no fallback.
`validate-provenance.sh` prints nothing on success. Its FAIL and INFO findings become a separate `### Provenance` dimension, and it emits Why and Fix itself — surface those verbatim.
@@ -28,8 +28,10 @@ Include content the agent lacks:
- The specific tools or sequences to use — not the full range of options
- One default per decision point with one escape hatch
Move to `references/`, behind an explicit "If X, read `references/file.md`" trigger — the literal
conditional form, never a generic pointer:
Move to `references/`, behind an explicit "If X, read `references/<file>.md`" trigger — the literal
conditional form, never a generic pointer. Write the real filename in the skill under audit; the
angle brackets are a placeholder here, and a literal `references/file.md` in a body is an ERROR
from the ADR-0020 gate because no such file exists on disk. Move:
- Lookup tables and spec restatements
- Output schemas, templates and example blocks
@@ -32,8 +32,16 @@ read after the mistake.
inner fence as `` \`\`\` ``. An unescaped inner fence terminates the outer block and the remaining
instructions render as prose.
**Conditional references** state a specific trigger: "If the API returns a non-200 status, read
`references/api-errors.md`." The generic form — pointing at the directory and hoping — defeats
**Conditional references** state a specific trigger, naming a file that exists in the skill's own
`references/` directory:
```text
If the API returns a non-200 status, read `references/api-errors.md`.
```
That block is fenced because the filename in it is illustrative — an unfenced `references/` pointer
in a `SKILL.md` body must resolve on disk or the ADR-0020 gate reports a hard ERROR. The generic
form — pointing at the directory and hoping — defeats
progressive disclosure, because the agent either loads everything or loads nothing.
`Kyberforge.PaddingPhrase` catches the common generic phrasing deterministically; other malformed
forms are judgment.
@@ -148,17 +148,21 @@ def read_text(path):
# which is what a monorepo means,
# 2. the target's own apm package,
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
# case. They are `apm install` output, gitignored, and present only on a machine
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
# same commit measured 2 dangling targets on a developer machine and 6 on a
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
# root came from the plugins/ probe. They are `apm install` output, gitignored,
# and present only on a machine that has run it: four cross-plugin targets in
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
# developer machine and 6 on a fresh clone. A gate shipping hot with no baseline
# cannot give two answers.
#
# Deployed trees are used only when NO authoring root exists — the consumer
# case, where the file being checked lives in or beside a deployed tree and
# there is no monorepo to read.
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
# landed on a bare .git ancestor or on nothing at all. That is the consumer
# case: the file being checked lives in or beside a deployed tree, inside an
# ordinary git repo, with no monorepo to read. The two cases are told apart by
# which probe matched, never by how many names a root contributed; see
# known_targets().
def _is_fs_root(path):
@@ -167,11 +171,17 @@ def _is_fs_root(path):
def _collect_package(pkg_dir, names):
"""Add every skill/agent name a package directory exposes, any layout."""
# glob.escape() the DIRECTORY only. A checkout path containing `[`, `]`,
# `*` or `?` — a worktree named `feature[2]`, say — otherwise turns the
# whole pattern into a character class that matches nothing, and the
# resolver degrades to the "DID NOT RUN" INFO with rc=0 across every file
# in the tree. The wildcards in `sub` are the intended ones and stay raw.
safe_dir = glob.escape(pkg_dir)
for sub in ('.apm/skills/*/', 'skills/*/'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
names.add(os.path.basename(path.rstrip('/')).lower())
for sub in ('.apm/agents/*.md', 'agents/*.md'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
base = os.path.basename(path)
if base.endswith('.agent.md'):
base = base[:-len('.agent.md')]
@@ -203,28 +213,35 @@ def _apm_package_root(start_dir):
def _authoring_root(start_dir):
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
Returns (root, matched_plugins_probe). The flag reports WHICH probe
matched: True for the plugins/*/.apm/{skills,agents} glob, False for the
.git fallback and for no match at all. known_targets() needs that
distinction — only a real plugins/ root makes the deployed trees
redundant, and a name-count delta cannot tell the two apart.
Two passes, not one interleaved walk: a nested .git (a submodule, a
worktree of a sub-package) must not win over a real plugins/ root further
up. Both passes stop before the filesystem root for the same reason
_apm_package_root does.
"""
for probe in (
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git'))):
probes = (
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git')))
for index, probe in enumerate(probes):
current = os.path.abspath(start_dir)
for _ in range(12):
if _is_fs_root(current):
break
if probe(current):
return current
return current, index == 0
current = os.path.dirname(current)
return None
return None, False
def _collect_authoring_root(root, names):
"""Every plugin in the monorepo contributes its names."""
for pkg in glob.glob(os.path.join(root, 'plugins', '*')):
for pkg in glob.glob(os.path.join(glob.escape(root), 'plugins', '*')):
if os.path.isdir(pkg):
_collect_package(pkg, names)
@@ -288,7 +305,7 @@ def _declared_dependency_dirs(pkg_dir):
def _deployed_roots(start_dir):
""".claude/ and .agents/ trees above start_dir — what a host really sees.
Consulted ONLY when no authoring root exists; see the section header. The
Consulted ONLY when no plugin monorepo root was found; see the header. The
filesystem root is skipped for the same reason _apm_package_root skips it:
a stray /.claude/skills/ must not join every path's universe.
"""
@@ -327,10 +344,22 @@ def known_targets(start_dir):
for dep_dir in _declared_dependency_dirs(package):
_collect_package(dep_dir, names)
root = _authoring_root(start)
# A .git ancestor is an authoring root only if it actually holds plugins.
# _authoring_root() falls back to the nearest .git, so it is truthy in ANY
# git repo; without the distinction that fallback wins in every consumer
# checkout, _collect_authoring_root() contributes nothing, and the deployed
# branch below is dead code in the exact case it exists for. So condition
# on WHICH probe matched, which _authoring_root() reports directly. A
# name-count delta looks equivalent and is not: _collect_authoring_root()
# re-collects the checked file's own plugin, whose names the blocks above
# already added, so a one-plugin monorepo shows a delta of zero and would
# wrongly reach for the deployed trees — including the user's global
# ~/.claude/skills, making the verdict depend on what happens to be
# installed (ADR-0020 lines 118-127).
root, root_has_plugins = _authoring_root(start)
if root:
_collect_authoring_root(root, names)
else:
if not root_has_plugins:
for base in _deployed_roots(start):
_collect_package(base, names)
return names
@@ -436,16 +465,43 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
ROUTE_MARKED = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, MARKED_TARGET), re.I)
ROUTE_ANY = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, ANY_TARGET), re.I)
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
# re.I on ALL of them, uniformly. The patterns are built from the same
# lowercase NAME_* fragments, so half of them carrying the flag and half not
# meant `Skill-Audit` at the start of a boundary sentence was extracted by
# ROUTE_ANY but invisible to BACKTICK — contradicting normalize_target()'s own
# docstring, which exists precisely because extraction is case-insensitive and
# the universe is not.
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET, re.I)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET, re.I)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I)
ARROW_BOUNDARY = re.compile(r"\bnot\b[^.;]*?(?:->|→)\s*(%s)\b" % NAME_HYPH, re.I)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH, re.I)
# A boundary clause takes two shapes and BOTH count: the prose markers, and
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
BOUNDARY_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
BOUNDARY_ARROW = re.compile(r"\bnot\b[^.;]*?(?:->|→)", re.I)
SENTENCE_SPLIT = re.compile(u'(?<=[.!?])\\s+(?=[A-Z"“(])')
# Sentence boundaries decide the CORROBORATION scope above, so getting one wrong
# is not cosmetic — it moves a target between SUGGESTION and blocking ERROR. Two
# shapes common in these descriptions defeat the naive "period, space, capital"
# rule, in OPPOSITE directions:
# OVER-SPLIT. `e.g. "set up the manifest"` ends no sentence, but the quote
# looks like one starting. The clause is cut in half, the corroborating
# target lands on the far side of the cut, and a genuinely dangling target
# silently demotes to SUGGESTION — the gate takes a measurement and then
# throws it away, which is the vacuous-green shape this file exists to stop.
# UNDER-SPLIT. A real sentence opening with a code span or a lowercase skill
# name ("... Composes it. `gitea-prs` also uses it.") is not seen as a start
# at all, so two sentences merge and a resolving target vouches for an
# unresolvable one it never stood beside — a hard FAIL with no escape hatch,
# which is exactly the failure the corroboration rule was added to prevent.
# Both are closed here: the five abbreviations that actually occur in routing
# prose are excluded as sentence ends, and the opener class admits a backtick or
# a lowercase letter. Verified zero-delta on the current corpus (37 ERROR / 58
# SUGGESTION / 2 dangling before and after) — this protects the descriptions
# issue #99 is about to rewrite, not the ones already measured.
SENTENCE_SPLIT = re.compile(
u'(?<!\\be\\.g\\.)(?<!\\bi\\.e\\.)(?<!\\betc\\.)(?<!\\bvs\\.)(?<!\\bcf\\.)'
u'(?<=[.!?])\\s+(?=[A-Za-z`"“(])')
# The token that may follow a route target without turning it into a compound
# modifier: punctuation, end of sentence, a conjunction, a boundary word, or a
@@ -607,8 +663,16 @@ def unresolved_targets(description, known):
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
# be measured must never report green, so every caller of these two ERRORs on a
# miss instead of moving on.
#
# The CLOSING marker is anchored at column 0 — deliberately NOT `[ \t]*---`.
# YAML block-scalar content must be indented deeper than its key, so an
# indented `---` inside a folded description is CONTENT; letting it close the
# frontmatter truncated the description mid-value and silently reclassified the
# rest as body, which is a vacuous green in both directions at once. Leading
# whitespace is still tolerated on the OPENING marker, where no such content
# can exist.
FRONTMATTER_RE = re.compile(
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n[ \t]*---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
def strip_bom(text):
@@ -640,14 +704,26 @@ def description_value(fm_text):
try:
data = yaml.safe_load(fm_text)
except Exception as exc:
raise FrontmatterError(re.sub(r'\s+', ' ', str(exc)).strip())
# Every FrontmatterError message is a COMPLETE clause, never a detail a
# caller wraps in one. Callers used to prefix a hard-coded "frontmatter
# is not valid YAML (...)", which is true only of this branch: the two
# type failures below come from frontmatter that parsed fine, and
# telling their author the YAML is invalid sends them hunting for a
# syntax error that is not there — on a blocking gate with no baseline.
raise FrontmatterError('frontmatter is not valid YAML (%s)'
% re.sub(r'\s+', ' ', str(exc)).strip())
if not isinstance(data, dict):
raise FrontmatterError('frontmatter is not a YAML mapping')
value = data.get('description')
if value is None:
return ''
if not isinstance(value, str):
value = str(value)
# NOT str()-coerced. `description: true` became the 4-character "True"
# and sailed through the 400-character gate; a list or mapping was
# measured as its Python repr. Neither is a description a host can
# preload, so this is a parse failure, reported as one.
raise FrontmatterError(
'description is a %s, not a string' % type(value).__name__)
return re.sub(r'\s+', ' ', value).strip()
@@ -717,6 +793,18 @@ def mask_fenced(text):
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
and not stripped.strip()[len(marker):].strip()):
fence = None
# An UNCLOSED fence has no cost-free answer, only a choice of which way to
# be wrong. Masking to end-of-body blanks the rest of the body, silently
# disabling the ERROR-tier references/ check and the gotcha counts.
# Returning the raw text instead exposes the unclosed example's own
# content, so a fenced example naming a nonexistent references/ file
# becomes a hard ERROR it would not have been had the fence been closed —
# confirmed, not hypothetical. The loud-false-positive direction is the one
# chosen: this script's rule is that a file it cannot measure must never
# report green, and masking-onward is exactly that failure. Both outcomes
# need an already-malformed file, and the false positive costs one fence.
if fence is not None:
return text
return ''.join(out)
@@ -802,8 +890,11 @@ name = name_m.group(1).strip('"\'') if name_m else ""
try:
desc = description_value(fm)
except FrontmatterError as exc:
fail(f"frontmatter is not valid YAML ({exc}). Nothing downstream can be "
f"measured, so this is a hard failure, not a skip")
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or a
# description of the wrong type. Do not prefix a diagnosis here; the last
# one named a syntax error for two failures that have none.
fail(f"{exc}. Nothing downstream can be measured, so this is a hard "
f"failure, not a skip")
print("One or more checks failed.")
sys.exit(1)
@@ -57,6 +57,12 @@ therefore resolves; a skill in an unrelated repo does not. A boundary clause nam
outside that universe sends the router nowhere and fails the audit. Check the target exists before
writing it — do not invent a plausible sibling name.
That universe is the apm marketplace and stops there. A **host built-in is not a routing target**:
`/compact`, `/clear` and `/init` are Claude Code slash commands with no counterpart in Copilot CLI
or Codex, and `.apm/` source compiles for all three, so routing to one is a portability defect. The
gate is right to fail it and there is no allowlist. If a built-in genuinely needs mentioning, write
it un-slashed — ``the `compact` built-in`` — which makes no routing claim and is not checked.
**Length.** 250 characters SUGGESTION, 400 characters FAIL, counting the frontmatter value only
with YAML folding resolved. The agentskills.io 1,024-character spec limit is unchanged and sits
above both. The SUGGESTION tier is the one that moves the average; treat 250 as the target and 400
@@ -85,4 +85,10 @@ improvise the cuts — four dry runs invented six to ten different answers to th
If a signal points to a script or reference file, edit that file directly rather than adding a
workaround in SKILL.md.
**Check for regressions before handing back.** `SKILL.md` Step 4 tells you to resolve every FAIL,
which says nothing about a check that passed *before* these edits and no longer does. Compare the
closing audit against the skill's pre-edit state — a PASS that has become a SUGGESTION, or a
SUGGESTION that has become a FAIL, is damage this flow caused and is in scope for it. Only the
improve flow can make that comparison; the create flow has no prior state to compare against.
Then return to `SKILL.md` Step 4.
@@ -111,9 +111,12 @@ Flag as FAIL if:
available). `Kyberforge.VagueWording` catches the known filler; imprecision outside that list is
judgment.
- **A boundary clause naming a target that does not resolve** to a real skill directory or agent
file in the authoring source. No script checks this for an agent file — `validate.sh` resolves
boundary targets for skills only, so resolve the name yourself against `plugins/*/.apm/skills/`
and `plugins/*/.apm/agents/`.
file in the authoring source. `validate.sh` resolves this for agent files at both scopes and
reports each unresolved target itself — take its verdict rather than re-resolving the name by
hand, because a hand-walk over a different universe can contradict it. What is left to you is
semantic and the script cannot reach it: whether a target that *does* resolve is the right
sibling to exclude, and whether a clause naming no target at all ("examine the files manually")
should have named one.
- **`Use proactively` in a Copilot or vendor-neutral description.**
`KyberforgeCopilot.ProactivePhrase` catches it. The phrase steers the Claude Code runtime and
does nothing anywhere else, so in a `.agent.md` it is preloaded text that buys no behaviour.
@@ -222,17 +222,21 @@ def read_text(path):
# which is what a monorepo means,
# 2. the target's own apm package,
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
# case. They are `apm install` output, gitignored, and present only on a machine
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
# same commit measured 2 dangling targets on a developer machine and 6 on a
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
# root came from the plugins/ probe. They are `apm install` output, gitignored,
# and present only on a machine that has run it: four cross-plugin targets in
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
# developer machine and 6 on a fresh clone. A gate shipping hot with no baseline
# cannot give two answers.
#
# Deployed trees are used only when NO authoring root exists — the consumer
# case, where the file being checked lives in or beside a deployed tree and
# there is no monorepo to read.
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
# landed on a bare .git ancestor or on nothing at all. That is the consumer
# case: the file being checked lives in or beside a deployed tree, inside an
# ordinary git repo, with no monorepo to read. The two cases are told apart by
# which probe matched, never by how many names a root contributed; see
# known_targets().
def _is_fs_root(path):
@@ -241,11 +245,17 @@ def _is_fs_root(path):
def _collect_package(pkg_dir, names):
"""Add every skill/agent name a package directory exposes, any layout."""
# glob.escape() the DIRECTORY only. A checkout path containing `[`, `]`,
# `*` or `?` — a worktree named `feature[2]`, say — otherwise turns the
# whole pattern into a character class that matches nothing, and the
# resolver degrades to the "DID NOT RUN" INFO with rc=0 across every file
# in the tree. The wildcards in `sub` are the intended ones and stay raw.
safe_dir = glob.escape(pkg_dir)
for sub in ('.apm/skills/*/', 'skills/*/'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
names.add(os.path.basename(path.rstrip('/')).lower())
for sub in ('.apm/agents/*.md', 'agents/*.md'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
base = os.path.basename(path)
if base.endswith('.agent.md'):
base = base[:-len('.agent.md')]
@@ -277,28 +287,35 @@ def _apm_package_root(start_dir):
def _authoring_root(start_dir):
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
Returns (root, matched_plugins_probe). The flag reports WHICH probe
matched: True for the plugins/*/.apm/{skills,agents} glob, False for the
.git fallback and for no match at all. known_targets() needs that
distinction — only a real plugins/ root makes the deployed trees
redundant, and a name-count delta cannot tell the two apart.
Two passes, not one interleaved walk: a nested .git (a submodule, a
worktree of a sub-package) must not win over a real plugins/ root further
up. Both passes stop before the filesystem root for the same reason
_apm_package_root does.
"""
for probe in (
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git'))):
probes = (
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git')))
for index, probe in enumerate(probes):
current = os.path.abspath(start_dir)
for _ in range(12):
if _is_fs_root(current):
break
if probe(current):
return current
return current, index == 0
current = os.path.dirname(current)
return None
return None, False
def _collect_authoring_root(root, names):
"""Every plugin in the monorepo contributes its names."""
for pkg in glob.glob(os.path.join(root, 'plugins', '*')):
for pkg in glob.glob(os.path.join(glob.escape(root), 'plugins', '*')):
if os.path.isdir(pkg):
_collect_package(pkg, names)
@@ -362,7 +379,7 @@ def _declared_dependency_dirs(pkg_dir):
def _deployed_roots(start_dir):
""".claude/ and .agents/ trees above start_dir — what a host really sees.
Consulted ONLY when no authoring root exists; see the section header. The
Consulted ONLY when no plugin monorepo root was found; see the header. The
filesystem root is skipped for the same reason _apm_package_root skips it:
a stray /.claude/skills/ must not join every path's universe.
"""
@@ -401,10 +418,22 @@ def known_targets(start_dir):
for dep_dir in _declared_dependency_dirs(package):
_collect_package(dep_dir, names)
root = _authoring_root(start)
# A .git ancestor is an authoring root only if it actually holds plugins.
# _authoring_root() falls back to the nearest .git, so it is truthy in ANY
# git repo; without the distinction that fallback wins in every consumer
# checkout, _collect_authoring_root() contributes nothing, and the deployed
# branch below is dead code in the exact case it exists for. So condition
# on WHICH probe matched, which _authoring_root() reports directly. A
# name-count delta looks equivalent and is not: _collect_authoring_root()
# re-collects the checked file's own plugin, whose names the blocks above
# already added, so a one-plugin monorepo shows a delta of zero and would
# wrongly reach for the deployed trees — including the user's global
# ~/.claude/skills, making the verdict depend on what happens to be
# installed (ADR-0020 lines 118-127).
root, root_has_plugins = _authoring_root(start)
if root:
_collect_authoring_root(root, names)
else:
if not root_has_plugins:
for base in _deployed_roots(start):
_collect_package(base, names)
return names
@@ -510,16 +539,43 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
ROUTE_MARKED = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, MARKED_TARGET), re.I)
ROUTE_ANY = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, ANY_TARGET), re.I)
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
# re.I on ALL of them, uniformly. The patterns are built from the same
# lowercase NAME_* fragments, so half of them carrying the flag and half not
# meant `Skill-Audit` at the start of a boundary sentence was extracted by
# ROUTE_ANY but invisible to BACKTICK — contradicting normalize_target()'s own
# docstring, which exists precisely because extraction is case-insensitive and
# the universe is not.
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET, re.I)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET, re.I)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I)
ARROW_BOUNDARY = re.compile(r"\bnot\b[^.;]*?(?:->|→)\s*(%s)\b" % NAME_HYPH, re.I)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH, re.I)
# A boundary clause takes two shapes and BOTH count: the prose markers, and
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
BOUNDARY_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
BOUNDARY_ARROW = re.compile(r"\bnot\b[^.;]*?(?:->|→)", re.I)
SENTENCE_SPLIT = re.compile(u'(?<=[.!?])\\s+(?=[A-Z"“(])')
# Sentence boundaries decide the CORROBORATION scope above, so getting one wrong
# is not cosmetic — it moves a target between SUGGESTION and blocking ERROR. Two
# shapes common in these descriptions defeat the naive "period, space, capital"
# rule, in OPPOSITE directions:
# OVER-SPLIT. `e.g. "set up the manifest"` ends no sentence, but the quote
# looks like one starting. The clause is cut in half, the corroborating
# target lands on the far side of the cut, and a genuinely dangling target
# silently demotes to SUGGESTION — the gate takes a measurement and then
# throws it away, which is the vacuous-green shape this file exists to stop.
# UNDER-SPLIT. A real sentence opening with a code span or a lowercase skill
# name ("... Composes it. `gitea-prs` also uses it.") is not seen as a start
# at all, so two sentences merge and a resolving target vouches for an
# unresolvable one it never stood beside — a hard FAIL with no escape hatch,
# which is exactly the failure the corroboration rule was added to prevent.
# Both are closed here: the five abbreviations that actually occur in routing
# prose are excluded as sentence ends, and the opener class admits a backtick or
# a lowercase letter. Verified zero-delta on the current corpus (37 ERROR / 58
# SUGGESTION / 2 dangling before and after) — this protects the descriptions
# issue #99 is about to rewrite, not the ones already measured.
SENTENCE_SPLIT = re.compile(
u'(?<!\\be\\.g\\.)(?<!\\bi\\.e\\.)(?<!\\betc\\.)(?<!\\bvs\\.)(?<!\\bcf\\.)'
u'(?<=[.!?])\\s+(?=[A-Za-z`"“(])')
# The token that may follow a route target without turning it into a compound
# modifier: punctuation, end of sentence, a conjunction, a boundary word, or a
@@ -681,8 +737,16 @@ def unresolved_targets(description, known):
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
# be measured must never report green, so every caller of these two ERRORs on a
# miss instead of moving on.
#
# The CLOSING marker is anchored at column 0 — deliberately NOT `[ \t]*---`.
# YAML block-scalar content must be indented deeper than its key, so an
# indented `---` inside a folded description is CONTENT; letting it close the
# frontmatter truncated the description mid-value and silently reclassified the
# rest as body, which is a vacuous green in both directions at once. Leading
# whitespace is still tolerated on the OPENING marker, where no such content
# can exist.
FRONTMATTER_RE = re.compile(
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n[ \t]*---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
def strip_bom(text):
@@ -714,14 +778,26 @@ def description_value(fm_text):
try:
data = yaml.safe_load(fm_text)
except Exception as exc:
raise FrontmatterError(re.sub(r'\s+', ' ', str(exc)).strip())
# Every FrontmatterError message is a COMPLETE clause, never a detail a
# caller wraps in one. Callers used to prefix a hard-coded "frontmatter
# is not valid YAML (...)", which is true only of this branch: the two
# type failures below come from frontmatter that parsed fine, and
# telling their author the YAML is invalid sends them hunting for a
# syntax error that is not there — on a blocking gate with no baseline.
raise FrontmatterError('frontmatter is not valid YAML (%s)'
% re.sub(r'\s+', ' ', str(exc)).strip())
if not isinstance(data, dict):
raise FrontmatterError('frontmatter is not a YAML mapping')
value = data.get('description')
if value is None:
return ''
if not isinstance(value, str):
value = str(value)
# NOT str()-coerced. `description: true` became the 4-character "True"
# and sailed through the 400-character gate; a list or mapping was
# measured as its Python repr. Neither is a description a host can
# preload, so this is a parse failure, reported as one.
raise FrontmatterError(
'description is a %s, not a string' % type(value).__name__)
return re.sub(r'\s+', ' ', value).strip()
@@ -791,6 +867,18 @@ def mask_fenced(text):
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
and not stripped.strip()[len(marker):].strip()):
fence = None
# An UNCLOSED fence has no cost-free answer, only a choice of which way to
# be wrong. Masking to end-of-body blanks the rest of the body, silently
# disabling the ERROR-tier references/ check and the gotcha counts.
# Returning the raw text instead exposes the unclosed example's own
# content, so a fenced example naming a nonexistent references/ file
# becomes a hard ERROR it would not have been had the fence been closed —
# confirmed, not hypothetical. The loud-false-positive direction is the one
# chosen: this script's rule is that a file it cannot measure must never
# report green, and masking-onward is exactly that failure. Both outcomes
# need an already-malformed file, and the false positive costs one fence.
if fence is not None:
return text
return ''.join(out)
@@ -869,12 +957,15 @@ def get_frontmatter_keys(fm):
return keys
def agent_description(fm, local_fname):
"""The folded description VALUE, or None if the frontmatter is not YAML."""
"""The folded description VALUE, or None if it could not be read."""
try:
return description_value(fm)
except FrontmatterError as exc:
fail(f"frontmatter is not valid YAML ({exc}) — the ADR-0020 description and "
f"boundary-target gates could not run — {local_fname}")
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or
# a description of the wrong type. Do not prefix a diagnosis here; the
# last one named a syntax error for two failures that have none.
fail(f"{exc} — the ADR-0020 description and boundary-target gates could "
f"not run — {local_fname}")
return None
def check_description_budget(value, local_fname):
@@ -944,11 +1035,31 @@ def check_boundary(value, fpath, local_fname):
f"{local_fname}")
def extract_tools_list(fm):
"""Extract tool names from the tools frontmatter field (space or comma separated)."""
val = extract_field(fm, 'tools')
if not val:
"""Tool names from the `tools` field — inline scalar OR YAML block sequence.
Read off the PARSED mapping, never off extract_field(). That function's
capture is newline-bounded on purpose (`[^\\S\\r\\n]*(.+)`), so a `tools:`
written as a block sequence — the shape Copilot agent files use — captured
nothing at all and the subagent-unavailable-tool check silently stopped
firing on exactly the files it was written for. Both spellings are legal
YAML, so both are read here.
"""
try:
data = yaml.safe_load(fm)
except Exception:
# Not this function's failure to report: the frontmatter's validity is
# decided (and failed) by agent_description() on the same text.
return set()
return set(re.split(r'[\s,]+', val.strip()))
if not isinstance(data, dict):
return set()
val = data.get('tools')
if isinstance(val, list):
items = [str(item).strip() for item in val]
elif isinstance(val, str):
items = re.split(r'[\s,]+', val.strip())
else:
return set()
return {item for item in items if item}
def is_copilot_cloud_ide(fpath):
"""True if the file is a cloud/IDE Copilot agent (name is optional for these)."""
@@ -1063,6 +1174,15 @@ def check_apm_agent_file(fpath, allowlist, stem):
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
f"failure, not a skip — {local_fname}")
return
except OSError as exc:
# A path that cannot be opened gets a FAIL line naming it, not a bare
# FileNotFoundError traceback. scripts/check-apm-agents-valid.sh takes
# this path for an agent file deleted from the worktree but still
# tracked in the index — a real, expected state, and the caller needs to
# be told which file, not handed an interpreter stack.
fail(f"could not be read ({exc.strerror or exc}): {fpath}. Nothing could "
f"be measured, so this is a hard failure, not a skip — {local_fname}")
return
fm, body = parse_frontmatter(content)
if fm is None:
@@ -1168,6 +1288,13 @@ def check_file(fpath, file_provider):
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
f"failure, not a skip — {local_fname}")
return
except OSError as exc:
# Same reason as check_apm_agent_file's: a diagnostic naming the path
# beats a FileNotFoundError traceback. The counterpart is pre-checked at
# the bottom of this script, but agent_file itself never was.
fail(f"could not be read ({exc.strerror or exc}): {fpath}. Nothing could "
f"be measured, so this is a hard failure, not a skip — {local_fname}")
return
fm, body = parse_frontmatter(content)
if fm is None:
@@ -53,6 +53,8 @@ Gates `agent-audit` enforces at every scope:
- **Body** — no word gate, and a delegation check in its place: name the skill to invoke rather than restating what it does.
- **Invocation** — decide whether the agent is model-delegated or reached only by name. Only Copilot's cloud/IDE format expresses that in frontmatter (`disable-model-invocation`, `user-invocable`).
At every scope, five tools reach no subagent whatever `tools` says — `AskUserQuestion`, `EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup`, `WaitForMcpServers`. Never write a body that has the agent ask the user a question or enter plan mode; it describes a turn the runtime cannot give it.
## Step 4 — Validate and close
Invoke `agent-audit` on each file written and resolve every FAIL before reporting done. It checks the field allowlist, name-to-stem match, leftover placeholders and template comments, the description budget and the Copilot body limit — do not hand-check those.
@@ -25,12 +25,13 @@ description: FILL IN: Use when <trigger>. <One capability clause.> Not <thing> -
<!-- tools: Read, Bash, Grep
Optional. Allowlist of tool names: a comma-separated string or a YAML list.
Omit to inherit all tools from parent.
Restrict it to what the agent actually needs. Omit only when it needs them
all — omitting inherits every tool from the parent.
Use Agent(type1,type2) to restrict which subagent types this agent can spawn.
Omit Agent entirely to prevent this agent from spawning subagents.
Never available to subagents regardless of tools field:
AskUserQuestion, EnterPlanMode, ExitPlanMode, ScheduleWakeup, WaitForMcpServers
Exception: ExitPlanMode IS available when parent session runs in permissionMode: plan -->
Listing any of them is a finding: agent-audit enforces the flat rule. -->
<!-- model: sonnet
Optional. Aliases: sonnet, opus, haiku, fable. Or full model ID.
@@ -70,6 +70,12 @@ that package declares in `apm.yml` under `dependencies.apm`. A sibling plugin in
therefore resolves; a skill in an unrelated repo does not. A target outside that universe sends the
router nowhere. Verify it before writing it — do not invent a plausible sibling.
That universe is the apm marketplace and stops there. A **host built-in is not a routing target**:
`/compact`, `/clear` and `/init` are Claude Code slash commands with no counterpart in Copilot CLI
or Codex, and `.apm/` source compiles for all three, so routing to one is a portability defect. The
gate is right to fail it and there is no allowlist. If a built-in genuinely needs mentioning, write
it un-slashed — ``the `compact` built-in`` — which makes no routing claim and is not checked.
**Length.** 250 characters SUGGESTION, 400 characters FAIL, counting the frontmatter value only
with YAML folding resolved. Treat 250 as the target: the SUGGESTION tier is what moves the corpus
average, the FAIL tier only stops outliers.
@@ -57,6 +57,11 @@ procedure a skill it can invoke already owns is an `agent-audit` FAIL. When a si
missing procedure, check first whether an installed skill owns it and name that skill instead of
transcribing it. See `references/contract.md`.
The delegation check is not a length brake — it fires only on procedure an invocable skill already
owns, and says nothing about original prose. That brake is judgment, and it is the only one left:
for every sentence you add, ask "would the agent get this wrong without it?" and delete it if the
answer is no.
**Explain the why.** Reasoning-based instructions outperform rigid directives. A rule written in
all caps (ALWAYS/NEVER) is usually better reframed as why the behaviour matters, so the agent can
apply judgment at the edges.
@@ -73,4 +78,10 @@ that was already there.
If the edit adds or removes research-sourced content, update `source_keys` in the edited file and
the matching `sources.md` entry — the create flow's Step 3 has the rules.
**Check for regressions before handing back.** `SKILL.md` Step 4 tells you to resolve every FAIL,
which says nothing about a check that passed *before* these edits and no longer does. Compare the
closing `agent-audit` against the agent's pre-edit state — a PASS that has become a SUGGESTION, or
a SUGGESTION that has become a FAIL, is damage this flow caused and is in scope for it. Only the
improve flow can make that comparison; the create flow has no prior state to compare against.
Then return to `SKILL.md` Step 4.
@@ -37,6 +37,11 @@ The rule is about a field's *shape*, not a fixed roster:
Claude Code honours it for plugin subagents; the three fields plugin agents do silently ignore
are `hooks`, `mcpServers` and `permissionMode`, and this is not one of them. Copilot's handling
of the key is unconfirmed, which ADR-0016 accepts as a stated risk.
Its syntax is the same at every scope, and this is the one scope that cannot reach it anywhere
else: MCP tools are denied as `mcp__<server>`, `mcp__<server>__*` or `mcp__*`; both a YAML list
and a delimited string are accepted, and this repo writes the comma-separated string form
(`disallowedTools: Edit, Write, NotebookEdit`) — match it.
- The Claude-only knobs (`isolation`, `maxTurns`, `effort`, `memory`, `permissionMode`, `skills`,
`color`, `initialPrompt`, `background`, `hooks`, `mcpServers`) have no Copilot equivalent and
are never written to this file at all. "Silently ignored at plugin scope" is the wrong framing:
@@ -25,7 +25,10 @@ duplicate silently.
**`description`** — write it against `references/contract.md`. It is the primary signal for
autonomous delegation.
**`tools`** — an allowlist; omit it to inherit every tool from the parent. Use `Agent(type1,type2)`
**`tools`** — an allowlist. Write it, and restrict it to the tools the agent actually needs;
omitting it inherits every tool from the parent, which is the right value only when the agent
genuinely needs all of them. Least privilege is the default, not the exception. Use
`Agent(type1,type2)`
to restrict which subagent types this agent may spawn, and omit `Agent` entirely to stop it
spawning any. Five tools reach no subagent whatever this field says — `AskUserQuestion`,
`EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup` and `WaitForMcpServers` — so listing one buys
@@ -35,7 +35,7 @@ scripts/vale-wrap.sh <skill-dir>/SKILL.md
`validate.sh` findings become the `### Structure` dimension — its FAILs and its SUGGESTIONs both.
If any of the three fails, cannot run, or reports something needing interpretation, read `references/validation-scripts.md` — it carries the manual fallback and the misleading exit codes.
If any of the three cannot run, or exits non-zero for a reason other than findings, read `references/validation-scripts.md` — it carries the manual fallback and the misleading exit codes. Ordinary content FAILs are the expected outcome here and need no fallback.
`validate-provenance.sh` prints nothing on success. Its FAIL and INFO findings become a separate `### Provenance` dimension, and it emits Why and Fix itself — surface those verbatim.
@@ -28,8 +28,10 @@ Include content the agent lacks:
- The specific tools or sequences to use — not the full range of options
- One default per decision point with one escape hatch
Move to `references/`, behind an explicit "If X, read `references/file.md`" trigger — the literal
conditional form, never a generic pointer:
Move to `references/`, behind an explicit "If X, read `references/<file>.md`" trigger — the literal
conditional form, never a generic pointer. Write the real filename in the skill under audit; the
angle brackets are a placeholder here, and a literal `references/file.md` in a body is an ERROR
from the ADR-0020 gate because no such file exists on disk. Move:
- Lookup tables and spec restatements
- Output schemas, templates and example blocks
@@ -32,8 +32,16 @@ read after the mistake.
inner fence as `` \`\`\` ``. An unescaped inner fence terminates the outer block and the remaining
instructions render as prose.
**Conditional references** state a specific trigger: "If the API returns a non-200 status, read
`references/api-errors.md`." The generic form — pointing at the directory and hoping — defeats
**Conditional references** state a specific trigger, naming a file that exists in the skill's own
`references/` directory:
```text
If the API returns a non-200 status, read `references/api-errors.md`.
```
That block is fenced because the filename in it is illustrative — an unfenced `references/` pointer
in a `SKILL.md` body must resolve on disk or the ADR-0020 gate reports a hard ERROR. The generic
form — pointing at the directory and hoping — defeats
progressive disclosure, because the agent either loads everything or loads nothing.
`Kyberforge.PaddingPhrase` catches the common generic phrasing deterministically; other malformed
forms are judgment.
@@ -148,17 +148,21 @@ def read_text(path):
# which is what a monorepo means,
# 2. the target's own apm package,
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
# case. They are `apm install` output, gitignored, and present only on a machine
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
# same commit measured 2 dangling targets on a developer machine and 6 on a
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
# root came from the plugins/ probe. They are `apm install` output, gitignored,
# and present only on a machine that has run it: four cross-plugin targets in
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
# developer machine and 6 on a fresh clone. A gate shipping hot with no baseline
# cannot give two answers.
#
# Deployed trees are used only when NO authoring root exists — the consumer
# case, where the file being checked lives in or beside a deployed tree and
# there is no monorepo to read.
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
# landed on a bare .git ancestor or on nothing at all. That is the consumer
# case: the file being checked lives in or beside a deployed tree, inside an
# ordinary git repo, with no monorepo to read. The two cases are told apart by
# which probe matched, never by how many names a root contributed; see
# known_targets().
def _is_fs_root(path):
@@ -167,11 +171,17 @@ def _is_fs_root(path):
def _collect_package(pkg_dir, names):
"""Add every skill/agent name a package directory exposes, any layout."""
# glob.escape() the DIRECTORY only. A checkout path containing `[`, `]`,
# `*` or `?` — a worktree named `feature[2]`, say — otherwise turns the
# whole pattern into a character class that matches nothing, and the
# resolver degrades to the "DID NOT RUN" INFO with rc=0 across every file
# in the tree. The wildcards in `sub` are the intended ones and stay raw.
safe_dir = glob.escape(pkg_dir)
for sub in ('.apm/skills/*/', 'skills/*/'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
names.add(os.path.basename(path.rstrip('/')).lower())
for sub in ('.apm/agents/*.md', 'agents/*.md'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
base = os.path.basename(path)
if base.endswith('.agent.md'):
base = base[:-len('.agent.md')]
@@ -203,28 +213,35 @@ def _apm_package_root(start_dir):
def _authoring_root(start_dir):
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
Returns (root, matched_plugins_probe). The flag reports WHICH probe
matched: True for the plugins/*/.apm/{skills,agents} glob, False for the
.git fallback and for no match at all. known_targets() needs that
distinction — only a real plugins/ root makes the deployed trees
redundant, and a name-count delta cannot tell the two apart.
Two passes, not one interleaved walk: a nested .git (a submodule, a
worktree of a sub-package) must not win over a real plugins/ root further
up. Both passes stop before the filesystem root for the same reason
_apm_package_root does.
"""
for probe in (
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git'))):
probes = (
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git')))
for index, probe in enumerate(probes):
current = os.path.abspath(start_dir)
for _ in range(12):
if _is_fs_root(current):
break
if probe(current):
return current
return current, index == 0
current = os.path.dirname(current)
return None
return None, False
def _collect_authoring_root(root, names):
"""Every plugin in the monorepo contributes its names."""
for pkg in glob.glob(os.path.join(root, 'plugins', '*')):
for pkg in glob.glob(os.path.join(glob.escape(root), 'plugins', '*')):
if os.path.isdir(pkg):
_collect_package(pkg, names)
@@ -288,7 +305,7 @@ def _declared_dependency_dirs(pkg_dir):
def _deployed_roots(start_dir):
""".claude/ and .agents/ trees above start_dir — what a host really sees.
Consulted ONLY when no authoring root exists; see the section header. The
Consulted ONLY when no plugin monorepo root was found; see the header. The
filesystem root is skipped for the same reason _apm_package_root skips it:
a stray /.claude/skills/ must not join every path's universe.
"""
@@ -327,10 +344,22 @@ def known_targets(start_dir):
for dep_dir in _declared_dependency_dirs(package):
_collect_package(dep_dir, names)
root = _authoring_root(start)
# A .git ancestor is an authoring root only if it actually holds plugins.
# _authoring_root() falls back to the nearest .git, so it is truthy in ANY
# git repo; without the distinction that fallback wins in every consumer
# checkout, _collect_authoring_root() contributes nothing, and the deployed
# branch below is dead code in the exact case it exists for. So condition
# on WHICH probe matched, which _authoring_root() reports directly. A
# name-count delta looks equivalent and is not: _collect_authoring_root()
# re-collects the checked file's own plugin, whose names the blocks above
# already added, so a one-plugin monorepo shows a delta of zero and would
# wrongly reach for the deployed trees — including the user's global
# ~/.claude/skills, making the verdict depend on what happens to be
# installed (ADR-0020 lines 118-127).
root, root_has_plugins = _authoring_root(start)
if root:
_collect_authoring_root(root, names)
else:
if not root_has_plugins:
for base in _deployed_roots(start):
_collect_package(base, names)
return names
@@ -436,16 +465,43 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
ROUTE_MARKED = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, MARKED_TARGET), re.I)
ROUTE_ANY = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, ANY_TARGET), re.I)
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
# re.I on ALL of them, uniformly. The patterns are built from the same
# lowercase NAME_* fragments, so half of them carrying the flag and half not
# meant `Skill-Audit` at the start of a boundary sentence was extracted by
# ROUTE_ANY but invisible to BACKTICK — contradicting normalize_target()'s own
# docstring, which exists precisely because extraction is case-insensitive and
# the universe is not.
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET, re.I)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET, re.I)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I)
ARROW_BOUNDARY = re.compile(r"\bnot\b[^.;]*?(?:->|→)\s*(%s)\b" % NAME_HYPH, re.I)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH, re.I)
# A boundary clause takes two shapes and BOTH count: the prose markers, and
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
BOUNDARY_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
BOUNDARY_ARROW = re.compile(r"\bnot\b[^.;]*?(?:->|→)", re.I)
SENTENCE_SPLIT = re.compile(u'(?<=[.!?])\\s+(?=[A-Z"“(])')
# Sentence boundaries decide the CORROBORATION scope above, so getting one wrong
# is not cosmetic — it moves a target between SUGGESTION and blocking ERROR. Two
# shapes common in these descriptions defeat the naive "period, space, capital"
# rule, in OPPOSITE directions:
# OVER-SPLIT. `e.g. "set up the manifest"` ends no sentence, but the quote
# looks like one starting. The clause is cut in half, the corroborating
# target lands on the far side of the cut, and a genuinely dangling target
# silently demotes to SUGGESTION — the gate takes a measurement and then
# throws it away, which is the vacuous-green shape this file exists to stop.
# UNDER-SPLIT. A real sentence opening with a code span or a lowercase skill
# name ("... Composes it. `gitea-prs` also uses it.") is not seen as a start
# at all, so two sentences merge and a resolving target vouches for an
# unresolvable one it never stood beside — a hard FAIL with no escape hatch,
# which is exactly the failure the corroboration rule was added to prevent.
# Both are closed here: the five abbreviations that actually occur in routing
# prose are excluded as sentence ends, and the opener class admits a backtick or
# a lowercase letter. Verified zero-delta on the current corpus (37 ERROR / 58
# SUGGESTION / 2 dangling before and after) — this protects the descriptions
# issue #99 is about to rewrite, not the ones already measured.
SENTENCE_SPLIT = re.compile(
u'(?<!\\be\\.g\\.)(?<!\\bi\\.e\\.)(?<!\\betc\\.)(?<!\\bvs\\.)(?<!\\bcf\\.)'
u'(?<=[.!?])\\s+(?=[A-Za-z`"“(])')
# The token that may follow a route target without turning it into a compound
# modifier: punctuation, end of sentence, a conjunction, a boundary word, or a
@@ -607,8 +663,16 @@ def unresolved_targets(description, known):
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
# be measured must never report green, so every caller of these two ERRORs on a
# miss instead of moving on.
#
# The CLOSING marker is anchored at column 0 — deliberately NOT `[ \t]*---`.
# YAML block-scalar content must be indented deeper than its key, so an
# indented `---` inside a folded description is CONTENT; letting it close the
# frontmatter truncated the description mid-value and silently reclassified the
# rest as body, which is a vacuous green in both directions at once. Leading
# whitespace is still tolerated on the OPENING marker, where no such content
# can exist.
FRONTMATTER_RE = re.compile(
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n[ \t]*---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
def strip_bom(text):
@@ -640,14 +704,26 @@ def description_value(fm_text):
try:
data = yaml.safe_load(fm_text)
except Exception as exc:
raise FrontmatterError(re.sub(r'\s+', ' ', str(exc)).strip())
# Every FrontmatterError message is a COMPLETE clause, never a detail a
# caller wraps in one. Callers used to prefix a hard-coded "frontmatter
# is not valid YAML (...)", which is true only of this branch: the two
# type failures below come from frontmatter that parsed fine, and
# telling their author the YAML is invalid sends them hunting for a
# syntax error that is not there — on a blocking gate with no baseline.
raise FrontmatterError('frontmatter is not valid YAML (%s)'
% re.sub(r'\s+', ' ', str(exc)).strip())
if not isinstance(data, dict):
raise FrontmatterError('frontmatter is not a YAML mapping')
value = data.get('description')
if value is None:
return ''
if not isinstance(value, str):
value = str(value)
# NOT str()-coerced. `description: true` became the 4-character "True"
# and sailed through the 400-character gate; a list or mapping was
# measured as its Python repr. Neither is a description a host can
# preload, so this is a parse failure, reported as one.
raise FrontmatterError(
'description is a %s, not a string' % type(value).__name__)
return re.sub(r'\s+', ' ', value).strip()
@@ -717,6 +793,18 @@ def mask_fenced(text):
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
and not stripped.strip()[len(marker):].strip()):
fence = None
# An UNCLOSED fence has no cost-free answer, only a choice of which way to
# be wrong. Masking to end-of-body blanks the rest of the body, silently
# disabling the ERROR-tier references/ check and the gotcha counts.
# Returning the raw text instead exposes the unclosed example's own
# content, so a fenced example naming a nonexistent references/ file
# becomes a hard ERROR it would not have been had the fence been closed —
# confirmed, not hypothetical. The loud-false-positive direction is the one
# chosen: this script's rule is that a file it cannot measure must never
# report green, and masking-onward is exactly that failure. Both outcomes
# need an already-malformed file, and the false positive costs one fence.
if fence is not None:
return text
return ''.join(out)
@@ -802,8 +890,11 @@ name = name_m.group(1).strip('"\'') if name_m else ""
try:
desc = description_value(fm)
except FrontmatterError as exc:
fail(f"frontmatter is not valid YAML ({exc}). Nothing downstream can be "
f"measured, so this is a hard failure, not a skip")
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or a
# description of the wrong type. Do not prefix a diagnosis here; the last
# one named a syntax error for two failures that have none.
fail(f"{exc}. Nothing downstream can be measured, so this is a hard "
f"failure, not a skip")
print("One or more checks failed.")
sys.exit(1)
@@ -57,6 +57,12 @@ therefore resolves; a skill in an unrelated repo does not. A boundary clause nam
outside that universe sends the router nowhere and fails the audit. Check the target exists before
writing it — do not invent a plausible sibling name.
That universe is the apm marketplace and stops there. A **host built-in is not a routing target**:
`/compact`, `/clear` and `/init` are Claude Code slash commands with no counterpart in Copilot CLI
or Codex, and `.apm/` source compiles for all three, so routing to one is a portability defect. The
gate is right to fail it and there is no allowlist. If a built-in genuinely needs mentioning, write
it un-slashed — ``the `compact` built-in`` — which makes no routing claim and is not checked.
**Length.** 250 characters SUGGESTION, 400 characters FAIL, counting the frontmatter value only
with YAML folding resolved. The agentskills.io 1,024-character spec limit is unchanged and sits
above both. The SUGGESTION tier is the one that moves the average; treat 250 as the target and 400
@@ -85,4 +85,10 @@ improvise the cuts — four dry runs invented six to ten different answers to th
If a signal points to a script or reference file, edit that file directly rather than adding a
workaround in SKILL.md.
**Check for regressions before handing back.** `SKILL.md` Step 4 tells you to resolve every FAIL,
which says nothing about a check that passed *before* these edits and no longer does. Compare the
closing audit against the skill's pre-edit state — a PASS that has become a SUGGESTION, or a
SUGGESTION that has become a FAIL, is damage this flow caused and is in scope for it. Only the
improve flow can make that comparison; the create flow has no prior state to compare against.
Then return to `SKILL.md` Step 4.
+163 -56
View File
@@ -30,8 +30,11 @@ set -euo pipefail
# SKILL.md could pass its own audit and still be blocked by the commit hook.
# The ADR-0020 ceilings are inclusive the same way.
#
# Token counts aren't computed exactly here — word count (`wc -w`) is used as
# a proxy. Measured over this repo's 39 in-scope SKILL.md files, characters per
# Token counts aren't computed exactly here — a whitespace word count is used
# as a proxy (Python's str.split(), the same primitive
# skill-audit/scripts/validate.sh applies to these two constants; `wc -w`
# disagrees with it on Unicode separators, which is why the awk pass that used
# to live in the loop below is gone). Measured over this repo's 39 in-scope SKILL.md files, characters per
# word runs min 5.97 / median 6.79 / mean 6.77 / max 7.22. At the standard
# ~4-characters-per-token English approximation that is 1.49 / 1.70 / 1.69 /
# 1.81 tokens per word.
@@ -105,23 +108,19 @@ for f in "$@"; do
continue
fi
# Single awk pass computes both line count and word count, avoiding a
# second read of the file. NR counts the final line even without a
# trailing newline, matching Python's splitlines() semantics (used by
# skill-audit/scripts/validate.sh for its own line count) — `wc -l`
# undercounts by 1 in that case. Word count uses awk's default
# whitespace-splitting NF, matching `wc -w` semantics.
read -r lines words <<< "$(awk '{w += NF} END{print NR, w+0}' "$f")"
if (( lines > MAX_LINES )); then
echo "ERROR: $f has $lines lines, exceeding the $MAX_LINES-line ceiling (agentskills.io skill-authoring.md)" >&2
FAIL=1
fi
if (( words > MAX_WORDS )); then
echo "ERROR: $f has $words words (proxy for tokens), exceeding the $MAX_WORDS-word ceiling (~5,000 tokens, agentskills.io skill-authoring.md)" >&2
FAIL=1
fi
# The MAX_LINES / MAX_WORDS ceilings are NOT measured here. They used to be,
# in a single awk pass, and that pass was wrong twice over:
# * `read -r lines words <<< "$(awk ...)"` discarded awk's exit status, so a
# file awk could not read yielded empty variables, bash arithmetic read
# them as 0, and both ceilings passed in total silence — the one outcome
# this script forbids itself.
# * awk's NR/NF do not agree with the Python splitlines()/split() that
# skill-audit/scripts/validate.sh uses for the SAME two constants.
# splitlines() also breaks on \x0b \x0c \x1c \x1d \x1e \x85 U+2028 U+2029
# and split() on every Unicode space, so a body padded with U+2028 read as
# 6 lines here and 606 lines there — hook green, audit FAIL.
# One implementation now owns both: the Python block below already reads every
# file (with a real diagnostic on failure), so it counts there.
done
if ! command -v python3 > /dev/null 2>&1; then
@@ -140,7 +139,7 @@ fi
if ! python3 -u - \
"$DESC_SUGGEST_CHARS" "$DESC_MAX_CHARS" \
"$BODY_SUGGEST_WORDS" "$BODY_MAX_WORDS" "$MAX_WORDS" "$@" <<'PYTHON'
"$BODY_SUGGEST_WORDS" "$BODY_MAX_WORDS" "$MAX_WORDS" "$MAX_LINES" "$@" <<'PYTHON'
import glob
import os
import re
@@ -153,7 +152,8 @@ DESC_MAX_CHARS = int(sys.argv[2])
BODY_SUGGEST_WORDS = int(sys.argv[3])
BODY_MAX_WORDS = int(sys.argv[4])
MAX_WORDS = int(sys.argv[5])
files = sys.argv[6:]
MAX_LINES = int(sys.argv[6])
files = sys.argv[7:]
failed = False
@@ -232,17 +232,21 @@ def read_text(path):
# which is what a monorepo means,
# 2. the target's own apm package,
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
# case. They are `apm install` output, gitignored, and present only on a machine
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
# same commit measured 2 dangling targets on a developer machine and 6 on a
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
# root came from the plugins/ probe. They are `apm install` output, gitignored,
# and present only on a machine that has run it: four cross-plugin targets in
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
# developer machine and 6 on a fresh clone. A gate shipping hot with no baseline
# cannot give two answers.
#
# Deployed trees are used only when NO authoring root exists — the consumer
# case, where the file being checked lives in or beside a deployed tree and
# there is no monorepo to read.
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
# landed on a bare .git ancestor or on nothing at all. That is the consumer
# case: the file being checked lives in or beside a deployed tree, inside an
# ordinary git repo, with no monorepo to read. The two cases are told apart by
# which probe matched, never by how many names a root contributed; see
# known_targets().
def _is_fs_root(path):
@@ -251,11 +255,17 @@ def _is_fs_root(path):
def _collect_package(pkg_dir, names):
"""Add every skill/agent name a package directory exposes, any layout."""
# glob.escape() the DIRECTORY only. A checkout path containing `[`, `]`,
# `*` or `?` — a worktree named `feature[2]`, say — otherwise turns the
# whole pattern into a character class that matches nothing, and the
# resolver degrades to the "DID NOT RUN" INFO with rc=0 across every file
# in the tree. The wildcards in `sub` are the intended ones and stay raw.
safe_dir = glob.escape(pkg_dir)
for sub in ('.apm/skills/*/', 'skills/*/'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
names.add(os.path.basename(path.rstrip('/')).lower())
for sub in ('.apm/agents/*.md', 'agents/*.md'):
for path in glob.glob(os.path.join(pkg_dir, sub)):
for path in glob.glob(os.path.join(safe_dir, sub)):
base = os.path.basename(path)
if base.endswith('.agent.md'):
base = base[:-len('.agent.md')]
@@ -287,28 +297,35 @@ def _apm_package_root(start_dir):
def _authoring_root(start_dir):
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
Returns (root, matched_plugins_probe). The flag reports WHICH probe
matched: True for the plugins/*/.apm/{skills,agents} glob, False for the
.git fallback and for no match at all. known_targets() needs that
distinction — only a real plugins/ root makes the deployed trees
redundant, and a name-count delta cannot tell the two apart.
Two passes, not one interleaved walk: a nested .git (a submodule, a
worktree of a sub-package) must not win over a real plugins/ root further
up. Both passes stop before the filesystem root for the same reason
_apm_package_root does.
"""
for probe in (
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git'))):
probes = (
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
lambda d: os.path.exists(os.path.join(d, '.git')))
for index, probe in enumerate(probes):
current = os.path.abspath(start_dir)
for _ in range(12):
if _is_fs_root(current):
break
if probe(current):
return current
return current, index == 0
current = os.path.dirname(current)
return None
return None, False
def _collect_authoring_root(root, names):
"""Every plugin in the monorepo contributes its names."""
for pkg in glob.glob(os.path.join(root, 'plugins', '*')):
for pkg in glob.glob(os.path.join(glob.escape(root), 'plugins', '*')):
if os.path.isdir(pkg):
_collect_package(pkg, names)
@@ -372,7 +389,7 @@ def _declared_dependency_dirs(pkg_dir):
def _deployed_roots(start_dir):
""".claude/ and .agents/ trees above start_dir — what a host really sees.
Consulted ONLY when no authoring root exists; see the section header. The
Consulted ONLY when no plugin monorepo root was found; see the header. The
filesystem root is skipped for the same reason _apm_package_root skips it:
a stray /.claude/skills/ must not join every path's universe.
"""
@@ -411,10 +428,22 @@ def known_targets(start_dir):
for dep_dir in _declared_dependency_dirs(package):
_collect_package(dep_dir, names)
root = _authoring_root(start)
# A .git ancestor is an authoring root only if it actually holds plugins.
# _authoring_root() falls back to the nearest .git, so it is truthy in ANY
# git repo; without the distinction that fallback wins in every consumer
# checkout, _collect_authoring_root() contributes nothing, and the deployed
# branch below is dead code in the exact case it exists for. So condition
# on WHICH probe matched, which _authoring_root() reports directly. A
# name-count delta looks equivalent and is not: _collect_authoring_root()
# re-collects the checked file's own plugin, whose names the blocks above
# already added, so a one-plugin monorepo shows a delta of zero and would
# wrongly reach for the deployed trees — including the user's global
# ~/.claude/skills, making the verdict depend on what happens to be
# installed (ADR-0020 lines 118-127).
root, root_has_plugins = _authoring_root(start)
if root:
_collect_authoring_root(root, names)
else:
if not root_has_plugins:
for base in _deployed_roots(start):
_collect_package(base, names)
return names
@@ -520,16 +549,43 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
ROUTE_MARKED = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, MARKED_TARGET), re.I)
ROUTE_ANY = re.compile(r"\b%s\s+(?:the\s+|an?\s+)?%s" % (ROUTE_VERB, ANY_TARGET), re.I)
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
# re.I on ALL of them, uniformly. The patterns are built from the same
# lowercase NAME_* fragments, so half of them carrying the flag and half not
# meant `Skill-Audit` at the start of a boundary sentence was extracted by
# ROUTE_ANY but invisible to BACKTICK — contradicting normalize_target()'s own
# docstring, which exists precisely because extraction is case-insensitive and
# the universe is not.
CONT_MARKED = re.compile(r"\s*(?:or|and|/|,)\s*%s" % MARKED_TARGET, re.I)
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET, re.I)
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET, re.I)
ARROW_BOUNDARY = re.compile(r"\bnot\b[^.;]*?(?:->|→)\s*(%s)\b" % NAME_HYPH, re.I)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH)
BACKTICK = re.compile(r"`(%s)`" % NAME_HYPH, re.I)
# A boundary clause takes two shapes and BOTH count: the prose markers, and
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
BOUNDARY_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
BOUNDARY_ARROW = re.compile(r"\bnot\b[^.;]*?(?:->|→)", re.I)
SENTENCE_SPLIT = re.compile(u'(?<=[.!?])\\s+(?=[A-Z"“(])')
# Sentence boundaries decide the CORROBORATION scope above, so getting one wrong
# is not cosmetic — it moves a target between SUGGESTION and blocking ERROR. Two
# shapes common in these descriptions defeat the naive "period, space, capital"
# rule, in OPPOSITE directions:
# OVER-SPLIT. `e.g. "set up the manifest"` ends no sentence, but the quote
# looks like one starting. The clause is cut in half, the corroborating
# target lands on the far side of the cut, and a genuinely dangling target
# silently demotes to SUGGESTION — the gate takes a measurement and then
# throws it away, which is the vacuous-green shape this file exists to stop.
# UNDER-SPLIT. A real sentence opening with a code span or a lowercase skill
# name ("... Composes it. `gitea-prs` also uses it.") is not seen as a start
# at all, so two sentences merge and a resolving target vouches for an
# unresolvable one it never stood beside — a hard FAIL with no escape hatch,
# which is exactly the failure the corroboration rule was added to prevent.
# Both are closed here: the five abbreviations that actually occur in routing
# prose are excluded as sentence ends, and the opener class admits a backtick or
# a lowercase letter. Verified zero-delta on the current corpus (37 ERROR / 58
# SUGGESTION / 2 dangling before and after) — this protects the descriptions
# issue #99 is about to rewrite, not the ones already measured.
SENTENCE_SPLIT = re.compile(
u'(?<!\\be\\.g\\.)(?<!\\bi\\.e\\.)(?<!\\betc\\.)(?<!\\bvs\\.)(?<!\\bcf\\.)'
u'(?<=[.!?])\\s+(?=[A-Za-z`"“(])')
# The token that may follow a route target without turning it into a compound
# modifier: punctuation, end of sentence, a conjunction, a boundary word, or a
@@ -691,8 +747,16 @@ def unresolved_targets(description, known):
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
# be measured must never report green, so every caller of these two ERRORs on a
# miss instead of moving on.
#
# The CLOSING marker is anchored at column 0 — deliberately NOT `[ \t]*---`.
# YAML block-scalar content must be indented deeper than its key, so an
# indented `---` inside a folded description is CONTENT; letting it close the
# frontmatter truncated the description mid-value and silently reclassified the
# rest as body, which is a vacuous green in both directions at once. Leading
# whitespace is still tolerated on the OPENING marker, where no such content
# can exist.
FRONTMATTER_RE = re.compile(
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n[ \t]*---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
r'^[ \t\r\n]*---[ \t]*\r?\n(.*?)\r?\n---[ \t]*(?:\r?\n|\Z)', re.DOTALL)
def strip_bom(text):
@@ -724,14 +788,26 @@ def description_value(fm_text):
try:
data = yaml.safe_load(fm_text)
except Exception as exc:
raise FrontmatterError(re.sub(r'\s+', ' ', str(exc)).strip())
# Every FrontmatterError message is a COMPLETE clause, never a detail a
# caller wraps in one. Callers used to prefix a hard-coded "frontmatter
# is not valid YAML (...)", which is true only of this branch: the two
# type failures below come from frontmatter that parsed fine, and
# telling their author the YAML is invalid sends them hunting for a
# syntax error that is not there — on a blocking gate with no baseline.
raise FrontmatterError('frontmatter is not valid YAML (%s)'
% re.sub(r'\s+', ' ', str(exc)).strip())
if not isinstance(data, dict):
raise FrontmatterError('frontmatter is not a YAML mapping')
value = data.get('description')
if value is None:
return ''
if not isinstance(value, str):
value = str(value)
# NOT str()-coerced. `description: true` became the 4-character "True"
# and sailed through the 400-character gate; a list or mapping was
# measured as its Python repr. Neither is a description a host can
# preload, so this is a parse failure, reported as one.
raise FrontmatterError(
'description is a %s, not a string' % type(value).__name__)
return re.sub(r'\s+', ' ', value).strip()
@@ -801,6 +877,18 @@ def mask_fenced(text):
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
and not stripped.strip()[len(marker):].strip()):
fence = None
# An UNCLOSED fence has no cost-free answer, only a choice of which way to
# be wrong. Masking to end-of-body blanks the rest of the body, silently
# disabling the ERROR-tier references/ check and the gotcha counts.
# Returning the raw text instead exposes the unclosed example's own
# content, so a fenced example naming a nonexistent references/ file
# becomes a hard ERROR it would not have been had the fence been closed —
# confirmed, not hypothetical. The loud-false-positive direction is the one
# chosen: this script's rule is that a file it cannot measure must never
# report green, and masking-onward is exactly that failure. Both outcomes
# need an already-malformed file, and the false positive costs one fence.
if fence is not None:
return text
return ''.join(out)
@@ -868,12 +956,28 @@ for path in files:
"silence." % (path, why))
continue
try:
content = strip_bom(read_text(path))
raw = read_text(path)
except EncodingError as exc:
error("%s: %s. None of the ADR-0020 gates could run on this file."
% (path, exc))
error("%s: %s. Neither the spec line/word ceilings nor any of the "
"ADR-0020 gates could run on this file." % (path, exc))
continue
# SPEC CONFORMANCE (family 1). Whole file, frontmatter included, counted
# with the SAME primitives skill-audit/scripts/validate.sh uses for these
# two constants — see the note in the bash loop above for what the previous
# awk pass got wrong.
lines = len(raw.splitlines())
words = len(raw.split())
if lines > MAX_LINES:
error("%s has %d lines, exceeding the %d-line ceiling "
"(agentskills.io skill-authoring.md)" % (path, lines, MAX_LINES))
if words > MAX_WORDS:
error("%s has %d words (proxy for tokens), exceeding the %d-word ceiling "
"(~5,000 tokens, agentskills.io skill-authoring.md)"
% (path, words, MAX_WORDS))
content = strip_bom(raw)
fm_match = FRONTMATTER_RE.match(content)
if not fm_match:
error("%s: no parseable YAML frontmatter block. Expected a `---` line, "
@@ -887,8 +991,11 @@ for path in files:
try:
desc = description_value(fm_match.group(1))
except FrontmatterError as exc:
error("%s: frontmatter is not valid YAML (%s). None of the ADR-0020 "
"gates could run on this file." % (path, exc))
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or
# a description of the wrong type. Do not prefix a diagnosis here; the
# last one named a syntax error for two failures that have none.
error("%s: %s. None of the ADR-0020 gates could run on this file."
% (path, exc))
continue
body = content[fm_match.end():]
+70
View File
@@ -332,6 +332,76 @@ EOF
)"
expect "a fenced references/example-file.md does not ERROR" "$F_REF_FENCED" silent
echo ""
echo "--- an UNTERMINATED fence does not blank the rest of the body ---"
# The fenced-block exemptions above all rest on mask_fenced(), and an unclosed
# fence used to run to EOF: everything after it was blanked, so the ERROR-tier
# references/ check and both Gotchas counts silently stopped seeing any of it.
# That is the worst shape a masking bug can take — a stray ``` line, which is a
# typo an author makes while writing the very examples the masking exists for,
# turned the rest of the file invisible and the gate green. Masking may narrow
# what a check reads; it may never delete content from every check at once.
#
# Both suppressed checks are asserted, because they are separate call sites and
# a fix that restored only one would leave the other silent.
F_FENCE_REF="$(make_skill fence-unclosed-ref "$CLEAN_DESC" <<EOF
Here is how it is invoked:
\`\`\`bash
some-command --all
If the caller needs the long form, read references/behind-the-fence.md first.
EOF
)"
expect "an absent references/ pointer after an unclosed fence still ERRORs" \
"$F_FENCE_REF" errors "points at references/behind-the-fence.md"
F_FENCE_GOTCHAS="$(make_skill fence-unclosed-gotchas "$CLEAN_DESC" <<EOF
Here is how it is invoked:
\`\`\`bash
some-command --all
## Common gotchas
- first trap here
- second trap here
- third trap here
- fourth trap here
- fifth trap here
- sixth trap here
- seventh trap here
## Notes
$(filler 200)
EOF
)"
expect "a Gotchas section after an unclosed fence is still counted" \
"$F_FENCE_GOTCHAS" suggests "Gotchas section has 7 entries"
# The control. Closing the fence must still mask, or the fix above would have
# been "stop masking", which re-breaks every false-positive case in this file.
F_FENCE_CLOSED="$(make_skill fence-closed-ref "$CLEAN_DESC" <<EOF
Here is how it is invoked:
\`\`\`bash
some-command --all
\`\`\`
Dispatch tables look like this:
\`\`\`markdown
If X, read references/behind-the-fence.md.
\`\`\`
EOF
)"
expect "control: the same pointer inside a CLOSED fence is still masked" \
"$F_FENCE_CLOSED" silent
echo ""
echo "--- a references/ pointer in a same-line removal context is history, not dispatch ---"
# Narrow on purpose: a live dispatch table never describes its own target as
+111 -24
View File
@@ -15,14 +15,20 @@
# ready to ship and the commit hook then rejects it, or worse, the reverse. So the
# comparison here is over VERDICTS on files, not over source text.
#
# Scope: the ADR-0020 axes the two scripts share — description length and tier,
# body word count and tier, dangling routing targets, missing references/
# pointers, the two Gotchas suggestions, the missing-boundary-clause suggestion,
# a declined resolution, and an empty description. The two scripts legitimately
# differ elsewhere (validate.sh also checks name/directory agreement, script
# executability and the 1024-char spec backstop; the hook checks whole-file lines
# and words), and those lines are ignored rather than being forced into a shared
# shape they were never meant to have.
# Scope: every axis the two scripts share. The ADR-0020 ones — description
# length and tier, body word count and tier, dangling routing targets, missing
# references/ pointers, the two Gotchas suggestions, the missing-boundary-clause
# suggestion, a declined resolution, an empty description — plus the two
# agentskills.io spec ceilings, MAX_LINES and MAX_WORDS.
#
# Those last two were EXCLUDED from this comparison until a real divergence
# shipped behind the exclusion. The header used to say "the hook checks
# whole-file lines and words" as if the auditor did not; it does, from its own
# copy of the same two constants, and the two implementations disagreed on
# Unicode whitespace for as long as nobody compared them. An axis both scripts
# measure is in scope by definition — the only lines still ignored are the ones
# a single script owns outright (validate.sh's name/directory agreement, script
# executability and 1024-char description backstop).
#
# Run over the real 39-skill corpus AND over purpose-built fixtures that sit ON
# each boundary. The corpus alone is not enough — it happens not to contain a
@@ -145,6 +151,48 @@ make_fx gotchas-fraction "$CLEAN" 0
python3 -c "print(' '.join(['word'] * 70))"
} >> "$FX/gotchas-fraction/SKILL.md"
# The agentskills.io spec ceilings, measured over Unicode whitespace.
#
# These two are in the comparison at all because they used to be excluded from
# it — `_non_adr_hook_error()` waved a spec-ceiling exit through as "not a
# disagreement", and that exclusion is exactly why the divergence below stayed
# invisible. The hook counted lines and words in a single awk pass (NR / NF)
# while skill-audit counted them with Python's splitlines() / split(). The two
# primitives do not agree: splitlines() also breaks on U+2028, U+2029, \x0b,
# \x0c, \x1c-\x1e and \x85, and split() breaks on every Unicode space. Same
# constants, same file, different verdict — hook green, audit FAIL, which is the
# precise failure mode ("passes its own audit, blocked by the commit hook",
# inverted) this whole suite exists to catch.
#
# One fixture per primitive, each sitting just past its ceiling on the Python
# measurement and nowhere near it on the awk one.
python3 - "$FX" <<'PY'
import os
import sys
fx = sys.argv[1]
# Spelled as escapes, never as literals. An invisible separator pasted into a
# source file is unreviewable and one editor round-trip away from becoming an
# ordinary space, which would silently turn both fixtures into nothing.
SEP_LINE = '\u2028' # LINE SEPARATOR: splitlines() breaks on it, awk's NR does not
SEP_WORD = '\u00a0' # NO-BREAK SPACE: split() breaks on it, awk's NF does not
head = ('---\nname: %s\n'
'description: Use when doing the thing. Do not use for anything else.\n'
'---\n\n')
# 600 U+2028-separated segments: 605 lines to splitlines(), 6 to awk's NR.
# Word count stays far below the 2,770 ceiling, so this fixture isolates lines.
cases = {
'spec-lines-u2028': SEP_LINE.join(['word'] * 600),
# 2,800 U+00A0-separated words: 2,816 words to split(), 17 to awk's NF.
'spec-words-u00a0': SEP_WORD.join(['word'] * 2800),
}
for name, body in cases.items():
d = os.path.join(fx, name)
os.makedirs(d, exist_ok=True)
with open(os.path.join(d, 'SKILL.md'), 'w', encoding='utf-8') as fh:
fh.write(head % name + body + '\n')
PY
# Empty description — the shape that used to exit 0 in silence.
mkdir -p "$FX/empty-desc"
printf -- '---\nname: empty-desc\ndescription:\nmodel: sonnet\n---\n\nDo the thing.\n' \
@@ -219,6 +267,18 @@ RULES = (
('NO_BOUNDARY_CLAUSE', re.compile(r'(description has no boundary clause)')),
('RESOLUTION_DECLINED', re.compile(r'(boundary-target resolution DID NOT RUN)')),
('DESC_EMPTY', re.compile(r'(description field is missing or empty)')),
# The agentskills.io spec ceilings. These were EXCLUDED from the comparison
# until the awk/Python divergence shipped, on the reasoning that "the hook
# checks whole-file lines and words" and the auditor did not. It does — with
# the same two constants — so the exclusion was never a scope decision, only
# an untested assumption, and it hid a real disagreement. Both scripts spell
# the finding differently, so the patterns match either wording and capture
# only the MEASUREMENT:
# hook: "... has 605 lines, exceeding the 500-line ceiling ..."
# audit: "SKILL.md line count 605 — exceeds 500-line limit"
('SPEC_LINES', re.compile(r'(?:has|line count) (\d+)(?: lines,)? (?:exceeding|—)')),
('SPEC_WORDS', re.compile(
r'(?:has|word count) (\d+)(?: words \(proxy for tokens\),)? (?:exceeding|—)')),
)
@@ -227,9 +287,15 @@ def verdict(output):
Lines that match no rule are dropped rather than compared: the two scripts
legitimately check different things outside ADR-0020 (name/directory
agreement, script executability, the 1024-char spec backstop, whole-file
line and word ceilings), and forcing those into the comparison would report
a difference that is not a disagreement.
agreement, script executability, the 1024-char spec backstop), and forcing
those into the comparison would report a difference that is not a
disagreement.
The whole-file line and word ceilings are NOT in that list. They were
excluded once, on the untested assumption that awk and splitlines() agree;
they do not, and the divergence was invisible for exactly as long as the
exclusion stood. SPEC_LINES/SPEC_WORDS are compared like any other rule —
see the file header. Do not re-add an exclusion for them.
"""
found = set()
for raw in output.splitlines():
@@ -278,9 +344,15 @@ def compare(label, skill_dir):
problems.append('the hook reported an ADR-0020 ERROR but exited 0')
if audit_err and audit_rc == 0:
problems.append('skill-audit reported an ADR-0020 FAIL but exited 0')
if not hook_err and hook_rc != 0 and not _non_adr_hook_error(hook_out):
problems.append('the hook exited %d with no ADR-0020 ERROR and no spec-ceiling ERROR'
% hook_rc)
# No escape hatch here any more. There used to be one — a
# `_non_adr_hook_error()` helper that waved through a non-zero hook exit
# explained by MAX_LINES / MAX_WORDS, on the grounds that those two were
# outside the comparison. They are inside it now (see SPEC_LINES /
# SPEC_WORDS in RULES), so every ERROR the hook can raise is a token this
# comparison holds both scripts to.
if not hook_err and hook_rc != 0:
problems.append('the hook exited %d with no compared ERROR at all — it has an '
'ERROR source this comparison does not know about' % hook_rc)
if problems:
bad('%s: %s' % (label, '; '.join(problems)))
@@ -289,16 +361,6 @@ def compare(label, skill_dir):
return False
def _non_adr_hook_error(output):
"""True if the hook failed on a spec ceiling rather than an ADR-0020 gate.
MAX_LINES / MAX_WORDS are the hook's other ERROR sources and are outside
this comparison, so a non-zero exit explained by one of them is not a
disagreement.
"""
return bool(re.search(r'ERROR: .*(-line ceiling|-word ceiling \(~5,000 tokens)', output))
# --- The real corpus -------------------------------------------------------
corpus = sorted(glob.glob(os.path.join(repo_root, 'plugins', '*', '.apm', 'skills', '*')))
corpus = [d for d in corpus if os.path.isfile(os.path.join(d, 'SKILL.md'))]
@@ -359,6 +421,31 @@ else:
ok('every one of the %d compared axes was exercised by at least one fixture'
% len(expected_tokens))
# --- The Unicode-whitespace fixtures, named and asserted directly -----------
# The two comparisons above would catch this divergence, but only as "fixture
# spec-lines-u2028 disagreed" — one line among 65. Spelled out here so the
# failure names the primitive, and so the ceiling is asserted to FIRE in both
# scripts rather than merely to be reported the same way by both.
print("")
print("--- both scripts break the spec ceilings on the same Unicode whitespace ---")
for name, token, expected in (('spec-lines-u2028', 'SPEC_LINES', '605'),
('spec-words-u00a0', 'SPEC_WORDS', '2816')):
skill_dir = os.path.join(fixture_dir, name)
_, h_out = run(['bash', hook, os.path.join(skill_dir, 'SKILL.md')])
_, a_out = run(['bash', validate, skill_dir])
want = ('ERROR', token, expected)
missing = [who for who, v in (('the hook', verdict(h_out)),
('skill-audit', verdict(a_out)))
if want not in v]
if missing:
bad('%s: %s did not report %s=%s. The two scripts must count with the '
'same primitive — Python splitlines()/split(), not awk NR/NF, which '
'does not break on this character' % (name, ' and '.join(missing),
token, expected))
else:
ok('%s: both scripts measure %s=%s and raise the ceiling ERROR'
% (name, token, expected))
print("")
print("Results: %d passed, %d failed" % (passes, failures))
sys.exit(1 if failures else 0)
+162 -8
View File
@@ -19,7 +19,22 @@
# NEXT key. The value then looked present (so "missing or empty" never
# fired) and was empty once folded (so every ADR-0020 gate early-returned).
# An agent file with one exited 0 with zero output through a BLOCKING
# pre-push gate. All five spellings of "no value" are pinned here.
# pre-push gate. All five spellings of "no value" are pinned here, plus the
# three shapes where the value is present but is not TEXT — a list, a
# mapping, a bool. Those used to be `str()`-coerced and then measured as a
# Python repr, so `description: true` was the four-character "True" and
# passed the 400-character gate.
#
# 3. THE INDENTED CLOSING MARKER. The mirror image of (1): content the pattern
# was too LOOSE to reject. `\r?\n[ \t]*---` matched an indented `---` inside
# a `>`-folded description, truncating the frontmatter mid-value — the
# description gate then measured a fragment and the body gate measured the
# discarded description text.
#
# Every needle names the specific branch or measurement the case is about. A
# needle loose enough to match two branches is how the yaml-none fixture spent
# its life asserting the wrong one: it emitted `---\n---\n`, which never matched
# the frontmatter pattern at all, and passed on the bare word "frontmatter".
#
# Both fixtures carry an over-ceiling description AND an over-ceiling body on
# purpose: asserting a non-zero exit alone would be satisfied by the "cannot
@@ -81,7 +96,48 @@ elif kind == 'yaml-list':
elif kind == 'yaml-string':
fm_lines = ['just a bare scalar, not a mapping']
elif kind == 'yaml-none':
# A comment-only block, NOT an empty one. `---\n---\n` does not match
# FRONTMATTER_RE at all (the pattern needs a `\n` between the markers), so
# it lands on the "no parseable frontmatter" branch and never reaches the
# `data is None` -> "not a YAML mapping" branch this fixture is named for.
# It passed anyway because the needle used to be the bare word
# "frontmatter", which both messages contain. A comment is real frontmatter
# text that yaml.safe_load() returns None for, which is the branch.
fm_lines = ['# nothing but a comment']
elif kind == 'yaml-empty-block':
# The shape the fixture above USED to have, kept as its own case so the
# "no parseable frontmatter block" branch is covered on purpose rather than
# by accident.
fm_lines = []
elif kind == 'desc-folded-indented':
# A `>`-folded description whose CONTENT contains an indented `---` line.
# YAML block-scalar content must be indented deeper than its key, so this is
# a value, not a document marker — but the closing pattern used to be
# `\r?\n[ \t]*---`, which matched it, truncated the frontmatter mid-value
# and silently reclassified the rest of the description as body. Both halves
# of that are vacuous greens: the description gate measured a fragment, and
# the body gate measured description text.
#
# The value is padded to exactly desc_chars AFTER folding, and the boundary
# clause naming a target sits in the part the truncation used to discard.
head = 'Use when doing the thing. '
tail = ' Do not use for improvements — use no-such-folded-target instead.'
span = int(desc_chars) - len(head) - len(tail) - len(' --- ')
if span < 2:
raise SystemExit('desc_chars too small for the folded fixture')
fm_lines = [
'name: ' + name,
'description: >',
' ' + head + 'x' * (span // 2),
' ---',
' ' + 'x' * (span - span // 2) + tail,
]
elif kind == 'desc-list':
fm_lines = ['name: ' + name, 'description:', ' - one', ' - two']
elif kind == 'desc-mapping':
fm_lines = ['name: ' + name, 'description:', ' text: a description']
elif kind == 'desc-bool':
fm_lines = ['name: ' + name, 'description: true']
elif kind == 'yaml-malformed':
fm_lines = ['name: ' + name, 'description: "unterminated', 'tabs:\t- a']
elif kind == 'desc-no-value':
@@ -222,24 +278,58 @@ probe_all "trailing whitespace after either --- marker does not hide the finding
probe_all "CRLF line endings do not hide the findings" \
crlf "description is $DESC_CHARS char" "@skills:body is $BODY_WORDS words"
# ---------------------------------------------------------------------------
# 1a-bis. An indented `---` inside a block scalar is CONTENT, not a marker
# ---------------------------------------------------------------------------
# The mirror image of the four shapes above. Those were markers the pattern was
# too strict to accept; this is content the pattern was too loose to reject. The
# closing marker used to be `\r?\n[ \t]*---`, so an indented `---` inside a
# `>`-folded description ended the frontmatter early: the description gate then
# measured a truncated fragment (under every ceiling, so silent) and the body
# gate measured the discarded description text as body. Measured on the fixture
# below, the old code exited 0 with nothing but a spurious "no boundary clause"
# SUGGESTION — the clause is in the half it threw away.
#
# The needle is the full-value length, so a script that merely rejected the file
# would not satisfy it.
echo ""
echo "--- an indented --- inside a >-folded description is content, not the end of the frontmatter ---"
build_subjects desc-folded-indented
probe_all "a folded description containing an indented '---' is measured whole" \
desc-folded-indented "description is $DESC_CHARS char"
# ---------------------------------------------------------------------------
# 1b. Unparseable frontmatter is a hard ERROR, never a quiet skip
# ---------------------------------------------------------------------------
echo ""
echo "--- genuinely unparseable frontmatter exits non-zero with a message, rather than passing quietly ---"
for kind in no-close yaml-list yaml-string yaml-none yaml-malformed; do
# Each needle names the BRANCH the fixture is supposed to reach, not the word
# "frontmatter" — which every one of these messages contains, and which is why
# the yaml-none fixture below passed for years while landing on the wrong branch
# entirely.
for kind in no-close yaml-list yaml-string yaml-none yaml-empty-block yaml-malformed; do
build_subjects "$kind"
done
probe_all "frontmatter with no closing --- is reported, not skipped" \
no-close "frontmatter"
no-close "parseable YAML frontmatter block"
probe_all "frontmatter that parses to a LIST is reported, not skipped" \
yaml-list "frontmatter"
yaml-list "frontmatter is not a YAML mapping"
probe_all "frontmatter that parses to a STRING is reported, not skipped" \
yaml-string "frontmatter"
probe_all "frontmatter that parses to None (empty block) is reported, not skipped" \
yaml-none "frontmatter"
yaml-string "frontmatter is not a YAML mapping"
probe_all "frontmatter that parses to None (a comment-only block) is reported, not skipped" \
yaml-none "frontmatter is not a YAML mapping"
probe_all "a completely empty '---/---' block is reported, not skipped" \
yaml-empty-block "parseable YAML frontmatter block"
# Two needles, both naming the SYNTAX branch specifically. "frontmatter is not
# valid YAML" is now exclusive to it — the wrong-typed-description failures reach
# the same wrapper and no longer borrow that phrase (see 2c below) — and the
# scanner context proves the parser's own diagnostic survives the wrapper rather
# than being replaced by a generic one. Do not needle the tail of PyYAML's
# message: an earlier attempt used "could not find expected", which PyYAML 6.0.3
# does not emit for this fixture at all, so the case failed on the assertion
# rather than on the behaviour.
probe_all "malformed YAML in the frontmatter is reported, not skipped" \
yaml-malformed "frontmatter"
yaml-malformed "frontmatter is not valid YAML" "while scanning a quoted scalar"
# ---------------------------------------------------------------------------
# 2. A valueless description is a hard FAIL in all three scripts
@@ -265,6 +355,70 @@ probe_all "'description: \"\"' FAILs" \
probe_all "'description: >' with nothing folded under it FAILs" \
desc-empty-fold "description field is missing or empty"
# ---------------------------------------------------------------------------
# 2b. A description that is not a STRING is a parse failure, not a measurement
# ---------------------------------------------------------------------------
# The other half of the same family, and the reason it belongs beside the five
# above: all eight shapes are "the description is not a description", and seven
# of them used to be handled while this one was silently coerced. A non-string
# value went through `str()` and was then measured as a Python repr —
# `description: true` became the four-character "True" and sailed through the
# 400-character gate, a list became "['one', 'two']", a mapping its dict repr.
# None of those is text a host can preload, so measuring one is a green verdict
# on a file that was never measured.
echo ""
echo "--- a description that is a list, a mapping or a bool hard-FAILs in all three scripts ---"
for kind in desc-list desc-mapping desc-bool; do
build_subjects "$kind"
done
probe_all "a LIST description FAILs rather than being measured as its repr" \
desc-list "description is a list, not a string"
probe_all "a MAPPING description FAILs rather than being measured as its repr" \
desc-mapping "description is a dict, not a string"
probe_all "a BOOL description FAILs rather than being measured as the 4-char 'True'" \
desc-bool "description is a bool, not a string"
# ---------------------------------------------------------------------------
# 2c. The FAILURE CLASS reported has to be the one that happened
# ---------------------------------------------------------------------------
# The three fixtures above reach the same wrapper as a genuine YAML syntax
# error, and that wrapper used to prefix a hard-coded "frontmatter is not valid
# YAML (...)" onto all of them. For a non-string description that is false: the
# block parses, only the field's TYPE is wrong. On a blocking gate with no
# baseline it sent the author hunting for a syntax error that is not there. The
# assertion runs in both directions, because fixing it by dropping the phrase
# everywhere would trade one wrong diagnosis for another.
echo ""
echo "--- 'not valid YAML' is said for a syntax error and NOT for a wrong-typed description ---"
YAML_CLASS_PROBLEMS=""
for spec in "yaml-malformed|yes" "desc-list|no" "desc-mapping|no" "desc-bool|no"; do
kind="${spec%%|*}"
want="${spec#*|}"
build_subjects "$kind"
for target in \
"hook|$HOOK|$TMPDIR_T/$kind/skill/my-skill/SKILL.md" \
"skill-audit|$SKILL_VALIDATE|$TMPDIR_T/$kind/skill/my-skill" \
"agent-audit|$AGENT_VALIDATE|$TMPDIR_T/$kind/agent/.apm/agents/my-agent.agent.md"
do
who="${target%%|*}"; rest="${target#*|}"
script="${rest%%|*}"; arg="${rest#*|}"
set +e
out="$(bash "$script" "$arg" 2>&1)"
set -e
if [[ "$want" == yes && "$out" != *"frontmatter is not valid YAML"* ]]; then
YAML_CLASS_PROBLEMS="$YAML_CLASS_PROBLEMS [$who did not call $kind a YAML syntax error: $out]"
fi
if [[ "$want" == no && "$out" == *"not valid YAML"* ]]; then
YAML_CLASS_PROBLEMS="$YAML_CLASS_PROBLEMS [$who called $kind invalid YAML, but the frontmatter parsed: $out]"
fi
done
done
if [[ -z "$YAML_CLASS_PROBLEMS" ]]; then
pass "a type error is reported as a type error and a syntax error as a syntax error"
else
fail "wrong failure class reported —$YAML_CLASS_PROBLEMS"
fi
# The specific regression, spelled out: the valueless-description agent file must
# not merely fail — it must not be SILENT. Zero output on a blocking gate is what
# made this un-diagnosable, so the output is asserted non-empty independently.
+201
View File
@@ -13,6 +13,14 @@
# identical with and without a deployed tree — on a synthetic fixture AND on
# the real 39-skill corpus.
#
# Three further ways the universe can be built out of the wrong directory,
# each of which shipped: a `.git` at the CONSUMER root (the fallback is
# truthy in any git repo, which made the deployed-tree branch dead code), a
# `.git` INSIDE a plugin (the walk-up is two passes precisely so this cannot
# capture the root), and glob metacharacters in the checkout path (which
# turned the directory name into a character class matching nothing, and the
# resolver into a no-op that still reported green).
#
# 2. THE BARE-TARGET GRAMMAR RULE. A hyphenated token used as a compound
# MODIFIER ("pre-commit hooks", "pull-request template") is prose, not a
# route; a terminal one is a real target. Getting this wrong in either
@@ -138,6 +146,175 @@ else
fail "the consumer path did not resolve through the deployed tree (exit $CONSUMER_RC): ${CONSUMER_OUT:-<empty>}"
fi
# ---------------------------------------------------------------------------
# 1a-bis. The consumer case with the one thing every real consumer has: .git
# ---------------------------------------------------------------------------
# The fixture immediately above has no .git, and that is precisely why it could
# never catch this. _authoring_root() falls back to the nearest .git ancestor, so
# it returns truthy in ANY git repo — a consumer checkout included. The branch
# that reads the deployed trees was guarded by `else`, so in every consumer
# checkout the fallback won, _collect_authoring_root() contributed nothing
# (there is no plugins/ directory to collect), and _deployed_roots() was dead
# code in exactly the case it exists for.
#
# The pair below is the whole test: the SAME tree, once with .git and once
# without. Old behaviour was rc=1 with .git and rc=0 without; a test covering
# only the no-.git shape reports green on both.
#
# `deployed-only-agent` lives ONLY in .agents/agents/, so it can be reached
# through no route but _deployed_roots(). `sibling-skill` sits in .claude/skills/
# beside the subject, which the sibling-collection block above reaches on its own
# — it is the corroborator that makes the dangling target BLOCKING rather than a
# SUGGESTION, so the old failure shows up in the exit code and not only in prose.
echo ""
echo "--- a consumer checkout resolves through its deployed trees even though it is a git repo ---"
build_consumer() {
local root="$1"
mkdir -p "$root/.agents/agents"
write_skill "$root/.claude/skills/sibling-skill" sibling-skill \
"Use when doing the other thing. Do not use for anything else."
write_skill "$root/.claude/skills/my-skill" my-skill \
"Use when doing the thing. Do not use for the other thing — use sibling-skill or deployed-only-agent instead."
: > "$root/.agents/agents/deployed-only-agent.agent.md"
}
build_consumer "$TMPDIR_T/consumer-git"
mkdir -p "$TMPDIR_T/consumer-git/.git"
build_consumer "$TMPDIR_T/consumer-nogit"
# consumer_case <label> <root>
consumer_case() {
local label="$1" root="$2" out status=0
set +e
out="$(bash "$HOOK" "$root/.claude/skills/my-skill/SKILL.md" 2>&1)"
status=$?
set -e
if [[ $status -eq 0 && "$out" != *"routes to"* && "$out" != *"DID NOT RUN"* ]]; then
pass "$label"
else
fail "$label (exit $status, output: ${out:-<empty>})"
fi
}
consumer_case "an agent in .agents/agents/ resolves in a consumer checkout that HAS a .git directory" \
"$TMPDIR_T/consumer-git"
consumer_case "control: the same tree without .git resolves too (the shape that always passed)" \
"$TMPDIR_T/consumer-nogit"
# ---------------------------------------------------------------------------
# 1a-ter. A monorepo with ONE plugin is still a monorepo
# ---------------------------------------------------------------------------
# The first attempt at the fix above conditioned the deployed branch on whether
# the authoring root had CONTRIBUTED a name — `if len(names) == before:`. That
# reads as "the .git fallback collected nothing, so fall through", and it is
# wrong: _collect_authoring_root() re-collects the subject's OWN plugin, whose
# names the sibling and package blocks have already added. With two plugins
# (fixture 1) the cross-plugin name makes the delta non-zero and the guard stays
# shut. With ONE plugin the delta is zero, the guard fires in a genuine
# monorepo, and _deployed_roots() walks up to ten levels — reaching the user's
# global ~/.claude/skills. That is install-dependence again, in the shape
# ADR-0020 lines 118-127 exist to forbid.
#
# So the predicate is which PROBE matched, not how many names arrived. The
# assertion is the same shape as fixture 1 — identical verdict either way — but
# on a single-plugin tree, which fixture 1 cannot express.
echo ""
echo "--- a SINGLE-plugin monorepo does not fall through to the deployed trees ---"
build_single() {
local root="$1"
write_skill "$root/plugins/only-plugin/.apm/skills/my-skill" my-skill \
"Use when doing the thing. Do not use for the other thing — use /deployed-only-skill instead."
}
build_single "$TMPDIR_T/single-no-claude"
build_single "$TMPDIR_T/single-with-claude"
write_skill "$TMPDIR_T/single-with-claude/.claude/skills/deployed-only-skill" deployed-only-skill \
"Use when doing the other thing. Do not use for anything else."
run_single() {
local root="$1" out
set +e
out="$(bash "$HOOK" "$root/plugins/only-plugin/.apm/skills/my-skill/SKILL.md" 2>&1)"
set -e
printf '%s\n' "$out" | sed "s#$root#<ROOT>#g"
}
SINGLE_NO_OUT="$(run_single "$TMPDIR_T/single-no-claude")"
SINGLE_WITH_OUT="$(run_single "$TMPDIR_T/single-with-claude")"
if [[ "$SINGLE_NO_OUT" == "$SINGLE_WITH_OUT" ]]; then
pass "a single-plugin monorepo gets the same verdict with and without a deployed .claude/ tree"
else
fail "the deployed tree changed the verdict in a single-plugin monorepo — without: [$SINGLE_NO_OUT] with: [$SINGLE_WITH_OUT]"
fi
# Identical-but-wrong guard, as in fixture 1: the deployed-only name must DANGLE,
# not resolve. Written as `/deployed-only-skill` so it blocks on its own without
# needing a second target in the sentence to corroborate it.
if [[ "$SINGLE_WITH_OUT" == *"routes to 'deployed-only-skill'"* ]]; then
pass "the deployed-only target dangles in a single-plugin monorepo (~/.claude/skills is not in the universe)"
else
fail "the deployed-only target resolved — the single-plugin tree fell through to _deployed_roots(): $SINGLE_WITH_OUT"
fi
# ---------------------------------------------------------------------------
# 1c. A nested .git inside a plugin must not beat the monorepo root
# ---------------------------------------------------------------------------
# ADR-0020 records the walk-up as TWO passes — plugins/*/.apm/{skills,agents}
# first, .git only afterwards — specifically so a .git inside a plugin (a
# submodule, or a sub-package with its own worktree) cannot capture the root.
# Nothing anywhere placed a .git inside a plugin, so the second pass was
# structural claim only. Collapsing the two probes into one interleaved walk
# passes every other fixture in this repo and fails here.
echo ""
echo "--- a .git INSIDE a plugin does not shadow the monorepo root above it ---"
NESTED="$TMPDIR_T/nested-git"
write_skill "$NESTED/plugins/other-plugin/.apm/skills/cross-plugin-skill" cross-plugin-skill \
"Use when doing the other thing. Do not use for anything else."
write_skill "$NESTED/plugins/subject-plugin/.apm/skills/sibling-skill" sibling-skill \
"Use when doing the other thing. Do not use for anything else."
write_skill "$NESTED/plugins/subject-plugin/.apm/skills/my-skill" my-skill \
"Use when doing the thing. Do not use for the other thing — use sibling-skill or cross-plugin-skill instead."
# The trap: a git checkout one level BELOW the monorepo root and above the skill.
mkdir -p "$NESTED/plugins/subject-plugin/.git"
set +e
NESTED_OUT="$(bash "$HOOK" "$NESTED/plugins/subject-plugin/.apm/skills/my-skill/SKILL.md" 2>&1)"
NESTED_RC=$?
set -e
# The sibling-plugin name is the discriminator: it is reachable ONLY from the
# monorepo root. If the nested .git won, subject-plugin would be the root, its
# plugins/ glob would collect nothing, and cross-plugin-skill would dangle —
# corroborated by sibling-skill in the same sentence, so it would BLOCK.
if [[ $NESTED_RC -eq 0 && "$NESTED_OUT" != *"routes to"* && "$NESTED_OUT" != *"DID NOT RUN"* ]]; then
pass "a sibling-plugin target still resolves with a .git directory inside the subject's own plugin"
else
fail "the nested .git captured the authoring root (exit $NESTED_RC): ${NESTED_OUT:-<empty>}"
fi
# ---------------------------------------------------------------------------
# 1d. Glob metacharacters in the checkout path
# ---------------------------------------------------------------------------
# The universe is built with glob.glob() against paths that begin with the
# checkout directory. A `[`, `]`, `*` or `?` anywhere in that prefix — a worktree
# named `feature[2]`, a CI workspace named `build[1]` — turned the literal
# directory name into a character class that matched nothing. The resolver then
# found no universe at all and degraded to the "DID NOT RUN" INFO with rc=0:
# every routing target in the tree silently unchecked, on a gate that reports
# green. Same monorepo as above, one directory renamed.
echo ""
echo "--- glob metacharacters in the checkout path do not silently disable the resolver ---"
GLOBDIR="$TMPDIR_T/gl[1]?x/mono"
write_skill "$GLOBDIR/plugins/other-plugin/.apm/skills/cross-plugin-skill" cross-plugin-skill \
"Use when doing the other thing. Do not use for anything else."
write_skill "$GLOBDIR/plugins/subject-plugin/.apm/skills/sibling-skill" sibling-skill \
"Use when doing the other thing. Do not use for anything else."
write_skill "$GLOBDIR/plugins/subject-plugin/.apm/skills/my-skill" my-skill \
"Use when doing the thing. Do not use for the other thing — use sibling-skill or cross-plugin-skill instead."
set +e
GLOB_OUT="$(bash "$HOOK" "$GLOBDIR/plugins/subject-plugin/.apm/skills/my-skill/SKILL.md" 2>&1)"
GLOB_RC=$?
set -e
if [[ $GLOB_RC -eq 0 && "$GLOB_OUT" != *"DID NOT RUN"* && "$GLOB_OUT" != *"routes to"* ]]; then
pass "a monorepo under a directory named 'gl[1]?x' resolves exactly like any other"
else
fail "glob metacharacters in the path changed the verdict (exit $GLOB_RC): ${GLOB_OUT:-<empty>}"
fi
# ---------------------------------------------------------------------------
# 1b. Machine independence — the real corpus
# ---------------------------------------------------------------------------
@@ -387,6 +564,30 @@ grammar_case tp-backticked errors "routes to 'no-such-backticked-skill'" \
grammar_case tp-bare-terminal errors "routes to 'no-such-bare-skill'" \
"Use when doing the thing. Do not use for improvements — use sibling-skill or no-such-bare-skill instead."
echo ""
echo "--- corroboration is scoped to a REAL sentence, not to whatever the splitter says ---"
# Corroboration decides SUGGESTION vs blocking ERROR, so a mis-placed sentence
# boundary moves a target between the two tiers. The naive "period, space,
# capital" rule got this wrong in both directions, and both were live:
#
# OVER-SPLIT. `e.g. "..."` is not a sentence end, but the quote looks like a
# start. The clause was cut in half and the corroborator stranded on the far
# side, so a target that DOES sit beside a resolving sibling silently demoted
# to SUGGESTION — a measurement taken and then discarded.
grammar_case abbrev-split errors "routes to 'no-such-abbrev-skill'" \
"Use when doing the thing. Do not use for improvements — use sibling-skill first, e.g. \"run the audit\", then use no-such-abbrev-skill instead."
#
# UNDER-SPLIT. A sentence opening with a lowercase word or a code span was not
# seen as a start at all, so two sentences merged and a resolving target in the
# FIRST vouched for an unresolvable one in the SECOND that it never stood
# beside — a hard FAIL with no escape hatch, which is the exact failure
# corroboration was added to prevent. The target must still be REPORTED; only
# the power to block is withdrawn.
grammar_case lowercase-start suggests "routes to 'no-such-lower-skill'" \
"Use when doing the thing. Use sibling-skill for the main case. do not use for improvements — use no-such-lower-skill instead."
grammar_case backtick-start suggests "routes to 'no-such-tick-skill'" \
"Use when doing the thing. Use sibling-skill for the main case. \`no-such-tick-skill\` is not for this — do not use it instead."
# And the confirming half of the grammar rule: a compound-modifier target is
# CONFIRM-ONLY, not ignored. When the name does exist it still counts as a route
# — the rule suppresses the ERROR, it does not delete the target.
+20 -10
View File
@@ -535,19 +535,31 @@ expect_gate "a fixture with no authoring root reports DID NOT RUN and exits 0" \
"Unchecked target(s): some-other-skill"
echo ""
echo "--- the three live dangling routing targets are caught (issue #100) ---"
# ADR-0020 records four broken routing targets and splits fixing them into its
# own issue. Three are detectable from the description text alone; this asserts
# the gate actually sees them rather than the check being vacuous in the corpus
# it was written against.
echo "--- the live dangling routing targets are caught (issue #100) ---"
# ADR-0020 records the broken routing targets and splits fixing them into its own
# issue. This asserts the gate actually sees them rather than the check being
# vacuous in the corpus it was written against.
#
# There used to be a third probe here, for `skill-improve` in skill-audit's
# description. It was already stale: that target was fixed, so the iteration
# permanently took a `pass "SKIP: ..."` branch — an assertion-free result counted
# in the totals, which is worse than no probe at all because it makes the suite
# look one test stronger than it is. It also contradicted
# tests/test-adr0020-targets.sh, which pins the live dangling set as EXACTLY
# {gitea-labels, neuledge-context}; that file is the authority on the set, this
# one only checks the two are individually detected.
#
# Both SKIP branches are gone with it, for the same reason. A probe whose fixture
# has been retrofitted is not "still passing" — it is a pin that needs updating,
# here and in the exact-set assertion in test-adr0020-targets.sh, and it should
# say so out loud rather than quietly agreeing with whatever it finds.
for probe in \
"plugins/bin/.apm/skills/research/SKILL.md:neuledge-context" \
"plugins/kyberforge/.apm/skills/skill-audit/SKILL.md:skill-improve" \
"plugins/gitea/.apm/skills/gitea-issues/SKILL.md:gitea-labels"; do
probe_file="$REPO_ROOT/${probe%%:*}"
probe_name="${probe##*:}"
if [[ ! -f "$probe_file" ]]; then
pass "SKIP: ${probe%%:*} no longer exists (retrofitted)"
fail "the probe fixture ${probe%%:*} no longer exists — this pin has become vacuous; update it and EXPECTED_DANGLING in tests/test-adr0020-targets.sh together"
continue
fi
# Captured, not piped: the script exits non-zero on these files and
@@ -558,10 +570,8 @@ for probe in \
set -e
if [[ "$probe_out" == *"routes to '$probe_name'"* ]]; then
pass "detects the dangling '$probe_name' target in ${probe%%:*}"
elif ! grep -q "$probe_name" "$probe_file"; then
pass "SKIP: '$probe_name' no longer appears in ${probe%%:*} (fixed by issue #100)"
else
fail "did not detect the dangling '$probe_name' target in ${probe%%:*}"
fail "did not detect the dangling '$probe_name' target in ${probe%%:*}. If issue #100 retrofitted it, drop this probe and update EXPECTED_DANGLING in tests/test-adr0020-targets.sh; if a false-positive fix took a true positive with it, that is the regression this asserts."
fi
done