Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
79c9089122 | ||
|
|
ede3f06689 | ||
|
|
b0d6d08239 | ||
|
|
f7cc27908c |
No files matched your search
@@ -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 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 `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.
|
- 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`.
|
- **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`.
|
- **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).
|
- 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
@@ -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
|
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
|
paths stay out of formatter scope — `.claude/settings.json` was the sixteenth exclude and nothing
|
||||||
prevents a seventeenth.
|
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
|
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
|
`<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
|
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
|
monorepo means. Deployed `.claude/`/`.agents/` trees are consulted **only** when the walk found no
|
||||||
exists — the consumer case, where there is no monorepo to read. What the resolver must never do is
|
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
|
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
|
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
|
routing to `skill-audit` resolved against a plugin it had never installed. Checked
|
||||||
|
|||||||
@@ -111,9 +111,12 @@ Flag as FAIL if:
|
|||||||
available). `Kyberforge.VagueWording` catches the known filler; imprecision outside that list is
|
available). `Kyberforge.VagueWording` catches the known filler; imprecision outside that list is
|
||||||
judgment.
|
judgment.
|
||||||
- **A boundary clause naming a target that does not resolve** to a real skill directory or agent
|
- **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
|
file in the authoring source. `validate.sh` resolves this for agent files at both scopes and
|
||||||
boundary targets for skills only, so resolve the name yourself against `plugins/*/.apm/skills/`
|
reports each unresolved target itself — take its verdict rather than re-resolving the name by
|
||||||
and `plugins/*/.apm/agents/`.
|
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.**
|
- **`Use proactively` in a Copilot or vendor-neutral description.**
|
||||||
`KyberforgeCopilot.ProactivePhrase` catches it. The phrase steers the Claude Code runtime and
|
`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.
|
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,
|
# which is what a monorepo means,
|
||||||
# 2. the target's own apm package,
|
# 2. the target's own apm package,
|
||||||
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
||||||
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
|
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
|
||||||
# case. They are `apm install` output, gitignored, and present only on a machine
|
# root came from the plugins/ probe. They are `apm install` output, gitignored,
|
||||||
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
|
# and present only on a machine that has run it: four cross-plugin targets in
|
||||||
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
|
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
|
||||||
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
|
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
|
||||||
# same commit measured 2 dangling targets on a developer machine and 6 on a
|
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
|
||||||
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
|
# 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
|
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
|
||||||
# case, where the file being checked lives in or beside a deployed tree and
|
# landed on a bare .git ancestor or on nothing at all. That is the consumer
|
||||||
# there is no monorepo to read.
|
# 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):
|
def _is_fs_root(path):
|
||||||
@@ -241,11 +245,17 @@ def _is_fs_root(path):
|
|||||||
|
|
||||||
def _collect_package(pkg_dir, names):
|
def _collect_package(pkg_dir, names):
|
||||||
"""Add every skill/agent name a package directory exposes, any layout."""
|
"""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 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())
|
names.add(os.path.basename(path.rstrip('/')).lower())
|
||||||
for sub in ('.apm/agents/*.md', 'agents/*.md'):
|
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)
|
base = os.path.basename(path)
|
||||||
if base.endswith('.agent.md'):
|
if base.endswith('.agent.md'):
|
||||||
base = base[:-len('.agent.md')]
|
base = base[:-len('.agent.md')]
|
||||||
@@ -277,28 +287,35 @@ def _apm_package_root(start_dir):
|
|||||||
def _authoring_root(start_dir):
|
def _authoring_root(start_dir):
|
||||||
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
|
"""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
|
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
|
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
|
up. Both passes stop before the filesystem root for the same reason
|
||||||
_apm_package_root does.
|
_apm_package_root does.
|
||||||
"""
|
"""
|
||||||
for probe in (
|
probes = (
|
||||||
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
|
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
|
||||||
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
|
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
|
||||||
lambda d: os.path.exists(os.path.join(d, '.git'))):
|
lambda d: os.path.exists(os.path.join(d, '.git')))
|
||||||
|
for index, probe in enumerate(probes):
|
||||||
current = os.path.abspath(start_dir)
|
current = os.path.abspath(start_dir)
|
||||||
for _ in range(12):
|
for _ in range(12):
|
||||||
if _is_fs_root(current):
|
if _is_fs_root(current):
|
||||||
break
|
break
|
||||||
if probe(current):
|
if probe(current):
|
||||||
return current
|
return current, index == 0
|
||||||
current = os.path.dirname(current)
|
current = os.path.dirname(current)
|
||||||
return None
|
return None, False
|
||||||
|
|
||||||
|
|
||||||
def _collect_authoring_root(root, names):
|
def _collect_authoring_root(root, names):
|
||||||
"""Every plugin in the monorepo contributes its 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):
|
if os.path.isdir(pkg):
|
||||||
_collect_package(pkg, names)
|
_collect_package(pkg, names)
|
||||||
|
|
||||||
@@ -362,7 +379,7 @@ def _declared_dependency_dirs(pkg_dir):
|
|||||||
def _deployed_roots(start_dir):
|
def _deployed_roots(start_dir):
|
||||||
""".claude/ and .agents/ trees above start_dir — what a host really sees.
|
""".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:
|
filesystem root is skipped for the same reason _apm_package_root skips it:
|
||||||
a stray /.claude/skills/ must not join every path's universe.
|
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):
|
for dep_dir in _declared_dependency_dirs(package):
|
||||||
_collect_package(dep_dir, names)
|
_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:
|
if root:
|
||||||
_collect_authoring_root(root, names)
|
_collect_authoring_root(root, names)
|
||||||
else:
|
if not root_has_plugins:
|
||||||
for base in _deployed_roots(start):
|
for base in _deployed_roots(start):
|
||||||
_collect_package(base, names)
|
_collect_package(base, names)
|
||||||
return names
|
return names
|
||||||
@@ -510,11 +539,17 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
|
|||||||
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
|
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_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)
|
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)
|
# re.I on ALL of them, uniformly. The patterns are built from the same
|
||||||
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
|
# lowercase NAME_* fragments, so half of them carrying the flag and half not
|
||||||
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
|
# 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)
|
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
|
# A boundary clause takes two shapes and BOTH count: the prose markers, and
|
||||||
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
|
# 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_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
|
||||||
@@ -681,8 +716,16 @@ def unresolved_targets(description, known):
|
|||||||
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
|
# 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
|
# be measured must never report green, so every caller of these two ERRORs on a
|
||||||
# miss instead of moving on.
|
# 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(
|
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):
|
def strip_bom(text):
|
||||||
@@ -714,14 +757,26 @@ def description_value(fm_text):
|
|||||||
try:
|
try:
|
||||||
data = yaml.safe_load(fm_text)
|
data = yaml.safe_load(fm_text)
|
||||||
except Exception as exc:
|
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):
|
if not isinstance(data, dict):
|
||||||
raise FrontmatterError('frontmatter is not a YAML mapping')
|
raise FrontmatterError('frontmatter is not a YAML mapping')
|
||||||
value = data.get('description')
|
value = data.get('description')
|
||||||
if value is None:
|
if value is None:
|
||||||
return ''
|
return ''
|
||||||
if not isinstance(value, str):
|
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()
|
return re.sub(r'\s+', ' ', value).strip()
|
||||||
|
|
||||||
|
|
||||||
@@ -791,6 +846,18 @@ def mask_fenced(text):
|
|||||||
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
||||||
and not stripped.strip()[len(marker):].strip()):
|
and not stripped.strip()[len(marker):].strip()):
|
||||||
fence = None
|
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)
|
return ''.join(out)
|
||||||
|
|
||||||
|
|
||||||
@@ -869,12 +936,15 @@ def get_frontmatter_keys(fm):
|
|||||||
return keys
|
return keys
|
||||||
|
|
||||||
def agent_description(fm, local_fname):
|
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:
|
try:
|
||||||
return description_value(fm)
|
return description_value(fm)
|
||||||
except FrontmatterError as exc:
|
except FrontmatterError as exc:
|
||||||
fail(f"frontmatter is not valid YAML ({exc}) — the ADR-0020 description and "
|
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or
|
||||||
f"boundary-target gates could not run — {local_fname}")
|
# 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
|
return None
|
||||||
|
|
||||||
def check_description_budget(value, local_fname):
|
def check_description_budget(value, local_fname):
|
||||||
@@ -944,11 +1014,31 @@ def check_boundary(value, fpath, local_fname):
|
|||||||
f"{local_fname}")
|
f"{local_fname}")
|
||||||
|
|
||||||
def extract_tools_list(fm):
|
def extract_tools_list(fm):
|
||||||
"""Extract tool names from the tools frontmatter field (space or comma separated)."""
|
"""Tool names from the `tools` field — inline scalar OR YAML block sequence.
|
||||||
val = extract_field(fm, 'tools')
|
|
||||||
if not val:
|
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()
|
||||||
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):
|
def is_copilot_cloud_ide(fpath):
|
||||||
"""True if the file is a cloud/IDE Copilot agent (name is optional for these)."""
|
"""True if the file is a cloud/IDE Copilot agent (name is optional for these)."""
|
||||||
@@ -1063,6 +1153,15 @@ def check_apm_agent_file(fpath, allowlist, stem):
|
|||||||
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
|
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
|
||||||
f"failure, not a skip — {local_fname}")
|
f"failure, not a skip — {local_fname}")
|
||||||
return
|
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)
|
fm, body = parse_frontmatter(content)
|
||||||
if fm is None:
|
if fm is None:
|
||||||
@@ -1168,6 +1267,13 @@ def check_file(fpath, file_provider):
|
|||||||
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
|
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
|
||||||
f"failure, not a skip — {local_fname}")
|
f"failure, not a skip — {local_fname}")
|
||||||
return
|
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)
|
fm, body = parse_frontmatter(content)
|
||||||
if fm is None:
|
if fm is None:
|
||||||
|
|||||||
@@ -810,3 +810,136 @@ EOF
|
|||||||
assert_success
|
assert_success
|
||||||
refute_output --partial "hooks"
|
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.
|
- **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`).
|
- **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
|
## 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.
|
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
|
<!-- tools: Read, Bash, Grep
|
||||||
Optional. Allowlist of tool names: a comma-separated string or a YAML list.
|
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.
|
Use Agent(type1,type2) to restrict which subagent types this agent can spawn.
|
||||||
Omit Agent entirely to prevent this agent from spawning subagents.
|
Omit Agent entirely to prevent this agent from spawning subagents.
|
||||||
Never available to subagents regardless of tools field:
|
Never available to subagents regardless of tools field:
|
||||||
AskUserQuestion, EnterPlanMode, ExitPlanMode, ScheduleWakeup, WaitForMcpServers
|
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
|
<!-- model: sonnet
|
||||||
Optional. Aliases: sonnet, opus, haiku, fable. Or full model ID.
|
Optional. Aliases: sonnet, opus, haiku, fable. Or full model ID.
|
||||||
|
|||||||
@@ -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
|
missing procedure, check first whether an installed skill owns it and name that skill instead of
|
||||||
transcribing it. See `references/contract.md`.
|
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
|
**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
|
all caps (ALWAYS/NEVER) is usually better reframed as why the behaviour matters, so the agent can
|
||||||
apply judgment at the edges.
|
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
|
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.
|
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.
|
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
|
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
|
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.
|
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`,
|
- The Claude-only knobs (`isolation`, `maxTurns`, `effort`, `memory`, `permissionMode`, `skills`,
|
||||||
`color`, `initialPrompt`, `background`, `hooks`, `mcpServers`) have no Copilot equivalent and
|
`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:
|
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
|
**`description`** — write it against `references/contract.md`. It is the primary signal for
|
||||||
autonomous delegation.
|
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
|
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`,
|
spawning any. Five tools reach no subagent whatever this field says — `AskUserQuestion`,
|
||||||
`EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup` and `WaitForMcpServers` — so listing one buys
|
`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.
|
`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.
|
`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
|
- The specific tools or sequences to use — not the full range of options
|
||||||
- One default per decision point with one escape hatch
|
- One default per decision point with one escape hatch
|
||||||
|
|
||||||
Move to `references/`, behind an explicit "If X, read `references/file.md`" trigger — the literal
|
Move to `references/`, behind an explicit "If X, read `references/<file>.md`" trigger — the literal
|
||||||
conditional form, never a generic pointer:
|
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
|
- Lookup tables and spec restatements
|
||||||
- Output schemas, templates and example blocks
|
- 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
|
inner fence as `` \`\`\` ``. An unescaped inner fence terminates the outer block and the remaining
|
||||||
instructions render as prose.
|
instructions render as prose.
|
||||||
|
|
||||||
**Conditional references** state a specific trigger: "If the API returns a non-200 status, read
|
**Conditional references** state a specific trigger, naming a file that exists in the skill's own
|
||||||
`references/api-errors.md`." The generic form — pointing at the directory and hoping — defeats
|
`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.
|
progressive disclosure, because the agent either loads everything or loads nothing.
|
||||||
`Kyberforge.PaddingPhrase` catches the common generic phrasing deterministically; other malformed
|
`Kyberforge.PaddingPhrase` catches the common generic phrasing deterministically; other malformed
|
||||||
forms are judgment.
|
forms are judgment.
|
||||||
|
|||||||
@@ -148,17 +148,21 @@ def read_text(path):
|
|||||||
# which is what a monorepo means,
|
# which is what a monorepo means,
|
||||||
# 2. the target's own apm package,
|
# 2. the target's own apm package,
|
||||||
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
||||||
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
|
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
|
||||||
# case. They are `apm install` output, gitignored, and present only on a machine
|
# root came from the plugins/ probe. They are `apm install` output, gitignored,
|
||||||
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
|
# and present only on a machine that has run it: four cross-plugin targets in
|
||||||
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
|
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
|
||||||
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
|
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
|
||||||
# same commit measured 2 dangling targets on a developer machine and 6 on a
|
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
|
||||||
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
|
# 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
|
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
|
||||||
# case, where the file being checked lives in or beside a deployed tree and
|
# landed on a bare .git ancestor or on nothing at all. That is the consumer
|
||||||
# there is no monorepo to read.
|
# 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):
|
def _is_fs_root(path):
|
||||||
@@ -167,11 +171,17 @@ def _is_fs_root(path):
|
|||||||
|
|
||||||
def _collect_package(pkg_dir, names):
|
def _collect_package(pkg_dir, names):
|
||||||
"""Add every skill/agent name a package directory exposes, any layout."""
|
"""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 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())
|
names.add(os.path.basename(path.rstrip('/')).lower())
|
||||||
for sub in ('.apm/agents/*.md', 'agents/*.md'):
|
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)
|
base = os.path.basename(path)
|
||||||
if base.endswith('.agent.md'):
|
if base.endswith('.agent.md'):
|
||||||
base = base[:-len('.agent.md')]
|
base = base[:-len('.agent.md')]
|
||||||
@@ -203,28 +213,35 @@ def _apm_package_root(start_dir):
|
|||||||
def _authoring_root(start_dir):
|
def _authoring_root(start_dir):
|
||||||
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
|
"""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
|
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
|
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
|
up. Both passes stop before the filesystem root for the same reason
|
||||||
_apm_package_root does.
|
_apm_package_root does.
|
||||||
"""
|
"""
|
||||||
for probe in (
|
probes = (
|
||||||
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
|
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
|
||||||
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
|
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
|
||||||
lambda d: os.path.exists(os.path.join(d, '.git'))):
|
lambda d: os.path.exists(os.path.join(d, '.git')))
|
||||||
|
for index, probe in enumerate(probes):
|
||||||
current = os.path.abspath(start_dir)
|
current = os.path.abspath(start_dir)
|
||||||
for _ in range(12):
|
for _ in range(12):
|
||||||
if _is_fs_root(current):
|
if _is_fs_root(current):
|
||||||
break
|
break
|
||||||
if probe(current):
|
if probe(current):
|
||||||
return current
|
return current, index == 0
|
||||||
current = os.path.dirname(current)
|
current = os.path.dirname(current)
|
||||||
return None
|
return None, False
|
||||||
|
|
||||||
|
|
||||||
def _collect_authoring_root(root, names):
|
def _collect_authoring_root(root, names):
|
||||||
"""Every plugin in the monorepo contributes its 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):
|
if os.path.isdir(pkg):
|
||||||
_collect_package(pkg, names)
|
_collect_package(pkg, names)
|
||||||
|
|
||||||
@@ -288,7 +305,7 @@ def _declared_dependency_dirs(pkg_dir):
|
|||||||
def _deployed_roots(start_dir):
|
def _deployed_roots(start_dir):
|
||||||
""".claude/ and .agents/ trees above start_dir — what a host really sees.
|
""".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:
|
filesystem root is skipped for the same reason _apm_package_root skips it:
|
||||||
a stray /.claude/skills/ must not join every path's universe.
|
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):
|
for dep_dir in _declared_dependency_dirs(package):
|
||||||
_collect_package(dep_dir, names)
|
_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:
|
if root:
|
||||||
_collect_authoring_root(root, names)
|
_collect_authoring_root(root, names)
|
||||||
else:
|
if not root_has_plugins:
|
||||||
for base in _deployed_roots(start):
|
for base in _deployed_roots(start):
|
||||||
_collect_package(base, names)
|
_collect_package(base, names)
|
||||||
return names
|
return names
|
||||||
@@ -436,11 +465,17 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
|
|||||||
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
|
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_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)
|
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)
|
# re.I on ALL of them, uniformly. The patterns are built from the same
|
||||||
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
|
# lowercase NAME_* fragments, so half of them carrying the flag and half not
|
||||||
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
|
# 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)
|
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
|
# A boundary clause takes two shapes and BOTH count: the prose markers, and
|
||||||
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
|
# 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_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
|
||||||
@@ -607,8 +642,16 @@ def unresolved_targets(description, known):
|
|||||||
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
|
# 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
|
# be measured must never report green, so every caller of these two ERRORs on a
|
||||||
# miss instead of moving on.
|
# 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(
|
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):
|
def strip_bom(text):
|
||||||
@@ -640,14 +683,26 @@ def description_value(fm_text):
|
|||||||
try:
|
try:
|
||||||
data = yaml.safe_load(fm_text)
|
data = yaml.safe_load(fm_text)
|
||||||
except Exception as exc:
|
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):
|
if not isinstance(data, dict):
|
||||||
raise FrontmatterError('frontmatter is not a YAML mapping')
|
raise FrontmatterError('frontmatter is not a YAML mapping')
|
||||||
value = data.get('description')
|
value = data.get('description')
|
||||||
if value is None:
|
if value is None:
|
||||||
return ''
|
return ''
|
||||||
if not isinstance(value, str):
|
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()
|
return re.sub(r'\s+', ' ', value).strip()
|
||||||
|
|
||||||
|
|
||||||
@@ -717,6 +772,18 @@ def mask_fenced(text):
|
|||||||
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
||||||
and not stripped.strip()[len(marker):].strip()):
|
and not stripped.strip()[len(marker):].strip()):
|
||||||
fence = None
|
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)
|
return ''.join(out)
|
||||||
|
|
||||||
|
|
||||||
@@ -802,8 +869,11 @@ name = name_m.group(1).strip('"\'') if name_m else ""
|
|||||||
try:
|
try:
|
||||||
desc = description_value(fm)
|
desc = description_value(fm)
|
||||||
except FrontmatterError as exc:
|
except FrontmatterError as exc:
|
||||||
fail(f"frontmatter is not valid YAML ({exc}). Nothing downstream can be "
|
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or a
|
||||||
f"measured, so this is a hard failure, not a skip")
|
# 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.")
|
print("One or more checks failed.")
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
|
|
||||||
|
|||||||
@@ -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
|
If a signal points to a script or reference file, edit that file directly rather than adding a
|
||||||
workaround in SKILL.md.
|
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.
|
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
|
available). `Kyberforge.VagueWording` catches the known filler; imprecision outside that list is
|
||||||
judgment.
|
judgment.
|
||||||
- **A boundary clause naming a target that does not resolve** to a real skill directory or agent
|
- **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
|
file in the authoring source. `validate.sh` resolves this for agent files at both scopes and
|
||||||
boundary targets for skills only, so resolve the name yourself against `plugins/*/.apm/skills/`
|
reports each unresolved target itself — take its verdict rather than re-resolving the name by
|
||||||
and `plugins/*/.apm/agents/`.
|
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.**
|
- **`Use proactively` in a Copilot or vendor-neutral description.**
|
||||||
`KyberforgeCopilot.ProactivePhrase` catches it. The phrase steers the Claude Code runtime and
|
`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.
|
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,
|
# which is what a monorepo means,
|
||||||
# 2. the target's own apm package,
|
# 2. the target's own apm package,
|
||||||
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
||||||
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
|
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
|
||||||
# case. They are `apm install` output, gitignored, and present only on a machine
|
# root came from the plugins/ probe. They are `apm install` output, gitignored,
|
||||||
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
|
# and present only on a machine that has run it: four cross-plugin targets in
|
||||||
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
|
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
|
||||||
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
|
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
|
||||||
# same commit measured 2 dangling targets on a developer machine and 6 on a
|
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
|
||||||
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
|
# 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
|
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
|
||||||
# case, where the file being checked lives in or beside a deployed tree and
|
# landed on a bare .git ancestor or on nothing at all. That is the consumer
|
||||||
# there is no monorepo to read.
|
# 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):
|
def _is_fs_root(path):
|
||||||
@@ -241,11 +245,17 @@ def _is_fs_root(path):
|
|||||||
|
|
||||||
def _collect_package(pkg_dir, names):
|
def _collect_package(pkg_dir, names):
|
||||||
"""Add every skill/agent name a package directory exposes, any layout."""
|
"""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 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())
|
names.add(os.path.basename(path.rstrip('/')).lower())
|
||||||
for sub in ('.apm/agents/*.md', 'agents/*.md'):
|
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)
|
base = os.path.basename(path)
|
||||||
if base.endswith('.agent.md'):
|
if base.endswith('.agent.md'):
|
||||||
base = base[:-len('.agent.md')]
|
base = base[:-len('.agent.md')]
|
||||||
@@ -277,28 +287,35 @@ def _apm_package_root(start_dir):
|
|||||||
def _authoring_root(start_dir):
|
def _authoring_root(start_dir):
|
||||||
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
|
"""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
|
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
|
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
|
up. Both passes stop before the filesystem root for the same reason
|
||||||
_apm_package_root does.
|
_apm_package_root does.
|
||||||
"""
|
"""
|
||||||
for probe in (
|
probes = (
|
||||||
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
|
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
|
||||||
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
|
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
|
||||||
lambda d: os.path.exists(os.path.join(d, '.git'))):
|
lambda d: os.path.exists(os.path.join(d, '.git')))
|
||||||
|
for index, probe in enumerate(probes):
|
||||||
current = os.path.abspath(start_dir)
|
current = os.path.abspath(start_dir)
|
||||||
for _ in range(12):
|
for _ in range(12):
|
||||||
if _is_fs_root(current):
|
if _is_fs_root(current):
|
||||||
break
|
break
|
||||||
if probe(current):
|
if probe(current):
|
||||||
return current
|
return current, index == 0
|
||||||
current = os.path.dirname(current)
|
current = os.path.dirname(current)
|
||||||
return None
|
return None, False
|
||||||
|
|
||||||
|
|
||||||
def _collect_authoring_root(root, names):
|
def _collect_authoring_root(root, names):
|
||||||
"""Every plugin in the monorepo contributes its 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):
|
if os.path.isdir(pkg):
|
||||||
_collect_package(pkg, names)
|
_collect_package(pkg, names)
|
||||||
|
|
||||||
@@ -362,7 +379,7 @@ def _declared_dependency_dirs(pkg_dir):
|
|||||||
def _deployed_roots(start_dir):
|
def _deployed_roots(start_dir):
|
||||||
""".claude/ and .agents/ trees above start_dir — what a host really sees.
|
""".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:
|
filesystem root is skipped for the same reason _apm_package_root skips it:
|
||||||
a stray /.claude/skills/ must not join every path's universe.
|
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):
|
for dep_dir in _declared_dependency_dirs(package):
|
||||||
_collect_package(dep_dir, names)
|
_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:
|
if root:
|
||||||
_collect_authoring_root(root, names)
|
_collect_authoring_root(root, names)
|
||||||
else:
|
if not root_has_plugins:
|
||||||
for base in _deployed_roots(start):
|
for base in _deployed_roots(start):
|
||||||
_collect_package(base, names)
|
_collect_package(base, names)
|
||||||
return names
|
return names
|
||||||
@@ -510,11 +539,17 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
|
|||||||
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
|
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_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)
|
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)
|
# re.I on ALL of them, uniformly. The patterns are built from the same
|
||||||
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
|
# lowercase NAME_* fragments, so half of them carrying the flag and half not
|
||||||
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
|
# 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)
|
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
|
# A boundary clause takes two shapes and BOTH count: the prose markers, and
|
||||||
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
|
# 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_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
|
||||||
@@ -681,8 +716,16 @@ def unresolved_targets(description, known):
|
|||||||
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
|
# 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
|
# be measured must never report green, so every caller of these two ERRORs on a
|
||||||
# miss instead of moving on.
|
# 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(
|
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):
|
def strip_bom(text):
|
||||||
@@ -714,14 +757,26 @@ def description_value(fm_text):
|
|||||||
try:
|
try:
|
||||||
data = yaml.safe_load(fm_text)
|
data = yaml.safe_load(fm_text)
|
||||||
except Exception as exc:
|
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):
|
if not isinstance(data, dict):
|
||||||
raise FrontmatterError('frontmatter is not a YAML mapping')
|
raise FrontmatterError('frontmatter is not a YAML mapping')
|
||||||
value = data.get('description')
|
value = data.get('description')
|
||||||
if value is None:
|
if value is None:
|
||||||
return ''
|
return ''
|
||||||
if not isinstance(value, str):
|
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()
|
return re.sub(r'\s+', ' ', value).strip()
|
||||||
|
|
||||||
|
|
||||||
@@ -791,6 +846,18 @@ def mask_fenced(text):
|
|||||||
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
||||||
and not stripped.strip()[len(marker):].strip()):
|
and not stripped.strip()[len(marker):].strip()):
|
||||||
fence = None
|
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)
|
return ''.join(out)
|
||||||
|
|
||||||
|
|
||||||
@@ -869,12 +936,15 @@ def get_frontmatter_keys(fm):
|
|||||||
return keys
|
return keys
|
||||||
|
|
||||||
def agent_description(fm, local_fname):
|
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:
|
try:
|
||||||
return description_value(fm)
|
return description_value(fm)
|
||||||
except FrontmatterError as exc:
|
except FrontmatterError as exc:
|
||||||
fail(f"frontmatter is not valid YAML ({exc}) — the ADR-0020 description and "
|
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or
|
||||||
f"boundary-target gates could not run — {local_fname}")
|
# 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
|
return None
|
||||||
|
|
||||||
def check_description_budget(value, local_fname):
|
def check_description_budget(value, local_fname):
|
||||||
@@ -944,11 +1014,31 @@ def check_boundary(value, fpath, local_fname):
|
|||||||
f"{local_fname}")
|
f"{local_fname}")
|
||||||
|
|
||||||
def extract_tools_list(fm):
|
def extract_tools_list(fm):
|
||||||
"""Extract tool names from the tools frontmatter field (space or comma separated)."""
|
"""Tool names from the `tools` field — inline scalar OR YAML block sequence.
|
||||||
val = extract_field(fm, 'tools')
|
|
||||||
if not val:
|
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()
|
||||||
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):
|
def is_copilot_cloud_ide(fpath):
|
||||||
"""True if the file is a cloud/IDE Copilot agent (name is optional for these)."""
|
"""True if the file is a cloud/IDE Copilot agent (name is optional for these)."""
|
||||||
@@ -1063,6 +1153,15 @@ def check_apm_agent_file(fpath, allowlist, stem):
|
|||||||
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
|
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
|
||||||
f"failure, not a skip — {local_fname}")
|
f"failure, not a skip — {local_fname}")
|
||||||
return
|
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)
|
fm, body = parse_frontmatter(content)
|
||||||
if fm is None:
|
if fm is None:
|
||||||
@@ -1168,6 +1267,13 @@ def check_file(fpath, file_provider):
|
|||||||
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
|
fail(f"file is {exc}. Nothing could be measured, so this is a hard "
|
||||||
f"failure, not a skip — {local_fname}")
|
f"failure, not a skip — {local_fname}")
|
||||||
return
|
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)
|
fm, body = parse_frontmatter(content)
|
||||||
if fm is None:
|
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.
|
- **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`).
|
- **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
|
## 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.
|
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
|
<!-- tools: Read, Bash, Grep
|
||||||
Optional. Allowlist of tool names: a comma-separated string or a YAML list.
|
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.
|
Use Agent(type1,type2) to restrict which subagent types this agent can spawn.
|
||||||
Omit Agent entirely to prevent this agent from spawning subagents.
|
Omit Agent entirely to prevent this agent from spawning subagents.
|
||||||
Never available to subagents regardless of tools field:
|
Never available to subagents regardless of tools field:
|
||||||
AskUserQuestion, EnterPlanMode, ExitPlanMode, ScheduleWakeup, WaitForMcpServers
|
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
|
<!-- model: sonnet
|
||||||
Optional. Aliases: sonnet, opus, haiku, fable. Or full model ID.
|
Optional. Aliases: sonnet, opus, haiku, fable. Or full model ID.
|
||||||
|
|||||||
@@ -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
|
missing procedure, check first whether an installed skill owns it and name that skill instead of
|
||||||
transcribing it. See `references/contract.md`.
|
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
|
**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
|
all caps (ALWAYS/NEVER) is usually better reframed as why the behaviour matters, so the agent can
|
||||||
apply judgment at the edges.
|
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
|
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.
|
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.
|
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
|
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
|
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.
|
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`,
|
- The Claude-only knobs (`isolation`, `maxTurns`, `effort`, `memory`, `permissionMode`, `skills`,
|
||||||
`color`, `initialPrompt`, `background`, `hooks`, `mcpServers`) have no Copilot equivalent and
|
`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:
|
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
|
**`description`** — write it against `references/contract.md`. It is the primary signal for
|
||||||
autonomous delegation.
|
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
|
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`,
|
spawning any. Five tools reach no subagent whatever this field says — `AskUserQuestion`,
|
||||||
`EnterPlanMode`, `ExitPlanMode`, `ScheduleWakeup` and `WaitForMcpServers` — so listing one buys
|
`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.
|
`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.
|
`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
|
- The specific tools or sequences to use — not the full range of options
|
||||||
- One default per decision point with one escape hatch
|
- One default per decision point with one escape hatch
|
||||||
|
|
||||||
Move to `references/`, behind an explicit "If X, read `references/file.md`" trigger — the literal
|
Move to `references/`, behind an explicit "If X, read `references/<file>.md`" trigger — the literal
|
||||||
conditional form, never a generic pointer:
|
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
|
- Lookup tables and spec restatements
|
||||||
- Output schemas, templates and example blocks
|
- 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
|
inner fence as `` \`\`\` ``. An unescaped inner fence terminates the outer block and the remaining
|
||||||
instructions render as prose.
|
instructions render as prose.
|
||||||
|
|
||||||
**Conditional references** state a specific trigger: "If the API returns a non-200 status, read
|
**Conditional references** state a specific trigger, naming a file that exists in the skill's own
|
||||||
`references/api-errors.md`." The generic form — pointing at the directory and hoping — defeats
|
`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.
|
progressive disclosure, because the agent either loads everything or loads nothing.
|
||||||
`Kyberforge.PaddingPhrase` catches the common generic phrasing deterministically; other malformed
|
`Kyberforge.PaddingPhrase` catches the common generic phrasing deterministically; other malformed
|
||||||
forms are judgment.
|
forms are judgment.
|
||||||
|
|||||||
@@ -148,17 +148,21 @@ def read_text(path):
|
|||||||
# which is what a monorepo means,
|
# which is what a monorepo means,
|
||||||
# 2. the target's own apm package,
|
# 2. the target's own apm package,
|
||||||
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
||||||
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
|
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
|
||||||
# case. They are `apm install` output, gitignored, and present only on a machine
|
# root came from the plugins/ probe. They are `apm install` output, gitignored,
|
||||||
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
|
# and present only on a machine that has run it: four cross-plugin targets in
|
||||||
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
|
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
|
||||||
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
|
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
|
||||||
# same commit measured 2 dangling targets on a developer machine and 6 on a
|
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
|
||||||
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
|
# 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
|
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
|
||||||
# case, where the file being checked lives in or beside a deployed tree and
|
# landed on a bare .git ancestor or on nothing at all. That is the consumer
|
||||||
# there is no monorepo to read.
|
# 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):
|
def _is_fs_root(path):
|
||||||
@@ -167,11 +171,17 @@ def _is_fs_root(path):
|
|||||||
|
|
||||||
def _collect_package(pkg_dir, names):
|
def _collect_package(pkg_dir, names):
|
||||||
"""Add every skill/agent name a package directory exposes, any layout."""
|
"""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 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())
|
names.add(os.path.basename(path.rstrip('/')).lower())
|
||||||
for sub in ('.apm/agents/*.md', 'agents/*.md'):
|
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)
|
base = os.path.basename(path)
|
||||||
if base.endswith('.agent.md'):
|
if base.endswith('.agent.md'):
|
||||||
base = base[:-len('.agent.md')]
|
base = base[:-len('.agent.md')]
|
||||||
@@ -203,28 +213,35 @@ def _apm_package_root(start_dir):
|
|||||||
def _authoring_root(start_dir):
|
def _authoring_root(start_dir):
|
||||||
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
|
"""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
|
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
|
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
|
up. Both passes stop before the filesystem root for the same reason
|
||||||
_apm_package_root does.
|
_apm_package_root does.
|
||||||
"""
|
"""
|
||||||
for probe in (
|
probes = (
|
||||||
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
|
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
|
||||||
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
|
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
|
||||||
lambda d: os.path.exists(os.path.join(d, '.git'))):
|
lambda d: os.path.exists(os.path.join(d, '.git')))
|
||||||
|
for index, probe in enumerate(probes):
|
||||||
current = os.path.abspath(start_dir)
|
current = os.path.abspath(start_dir)
|
||||||
for _ in range(12):
|
for _ in range(12):
|
||||||
if _is_fs_root(current):
|
if _is_fs_root(current):
|
||||||
break
|
break
|
||||||
if probe(current):
|
if probe(current):
|
||||||
return current
|
return current, index == 0
|
||||||
current = os.path.dirname(current)
|
current = os.path.dirname(current)
|
||||||
return None
|
return None, False
|
||||||
|
|
||||||
|
|
||||||
def _collect_authoring_root(root, names):
|
def _collect_authoring_root(root, names):
|
||||||
"""Every plugin in the monorepo contributes its 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):
|
if os.path.isdir(pkg):
|
||||||
_collect_package(pkg, names)
|
_collect_package(pkg, names)
|
||||||
|
|
||||||
@@ -288,7 +305,7 @@ def _declared_dependency_dirs(pkg_dir):
|
|||||||
def _deployed_roots(start_dir):
|
def _deployed_roots(start_dir):
|
||||||
""".claude/ and .agents/ trees above start_dir — what a host really sees.
|
""".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:
|
filesystem root is skipped for the same reason _apm_package_root skips it:
|
||||||
a stray /.claude/skills/ must not join every path's universe.
|
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):
|
for dep_dir in _declared_dependency_dirs(package):
|
||||||
_collect_package(dep_dir, names)
|
_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:
|
if root:
|
||||||
_collect_authoring_root(root, names)
|
_collect_authoring_root(root, names)
|
||||||
else:
|
if not root_has_plugins:
|
||||||
for base in _deployed_roots(start):
|
for base in _deployed_roots(start):
|
||||||
_collect_package(base, names)
|
_collect_package(base, names)
|
||||||
return names
|
return names
|
||||||
@@ -436,11 +465,17 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
|
|||||||
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
|
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_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)
|
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)
|
# re.I on ALL of them, uniformly. The patterns are built from the same
|
||||||
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
|
# lowercase NAME_* fragments, so half of them carrying the flag and half not
|
||||||
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
|
# 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)
|
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
|
# A boundary clause takes two shapes and BOTH count: the prose markers, and
|
||||||
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
|
# 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_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
|
||||||
@@ -607,8 +642,16 @@ def unresolved_targets(description, known):
|
|||||||
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
|
# 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
|
# be measured must never report green, so every caller of these two ERRORs on a
|
||||||
# miss instead of moving on.
|
# 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(
|
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):
|
def strip_bom(text):
|
||||||
@@ -640,14 +683,26 @@ def description_value(fm_text):
|
|||||||
try:
|
try:
|
||||||
data = yaml.safe_load(fm_text)
|
data = yaml.safe_load(fm_text)
|
||||||
except Exception as exc:
|
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):
|
if not isinstance(data, dict):
|
||||||
raise FrontmatterError('frontmatter is not a YAML mapping')
|
raise FrontmatterError('frontmatter is not a YAML mapping')
|
||||||
value = data.get('description')
|
value = data.get('description')
|
||||||
if value is None:
|
if value is None:
|
||||||
return ''
|
return ''
|
||||||
if not isinstance(value, str):
|
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()
|
return re.sub(r'\s+', ' ', value).strip()
|
||||||
|
|
||||||
|
|
||||||
@@ -717,6 +772,18 @@ def mask_fenced(text):
|
|||||||
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
||||||
and not stripped.strip()[len(marker):].strip()):
|
and not stripped.strip()[len(marker):].strip()):
|
||||||
fence = None
|
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)
|
return ''.join(out)
|
||||||
|
|
||||||
|
|
||||||
@@ -802,8 +869,11 @@ name = name_m.group(1).strip('"\'') if name_m else ""
|
|||||||
try:
|
try:
|
||||||
desc = description_value(fm)
|
desc = description_value(fm)
|
||||||
except FrontmatterError as exc:
|
except FrontmatterError as exc:
|
||||||
fail(f"frontmatter is not valid YAML ({exc}). Nothing downstream can be "
|
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or a
|
||||||
f"measured, so this is a hard failure, not a skip")
|
# 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.")
|
print("One or more checks failed.")
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
|
|
||||||
|
|||||||
@@ -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
|
If a signal points to a script or reference file, edit that file directly rather than adding a
|
||||||
workaround in SKILL.md.
|
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.
|
Then return to `SKILL.md` Step 4.
|
||||||
+141
-55
@@ -30,8 +30,11 @@ set -euo pipefail
|
|||||||
# SKILL.md could pass its own audit and still be blocked by the commit hook.
|
# SKILL.md could pass its own audit and still be blocked by the commit hook.
|
||||||
# The ADR-0020 ceilings are inclusive the same way.
|
# The ADR-0020 ceilings are inclusive the same way.
|
||||||
#
|
#
|
||||||
# Token counts aren't computed exactly here — word count (`wc -w`) is used as
|
# Token counts aren't computed exactly here — a whitespace word count is used
|
||||||
# a proxy. Measured over this repo's 39 in-scope SKILL.md files, characters per
|
# 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
|
# 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 /
|
# ~4-characters-per-token English approximation that is 1.49 / 1.70 / 1.69 /
|
||||||
# 1.81 tokens per word.
|
# 1.81 tokens per word.
|
||||||
@@ -105,23 +108,19 @@ for f in "$@"; do
|
|||||||
continue
|
continue
|
||||||
fi
|
fi
|
||||||
|
|
||||||
# Single awk pass computes both line count and word count, avoiding a
|
# The MAX_LINES / MAX_WORDS ceilings are NOT measured here. They used to be,
|
||||||
# second read of the file. NR counts the final line even without a
|
# in a single awk pass, and that pass was wrong twice over:
|
||||||
# trailing newline, matching Python's splitlines() semantics (used by
|
# * `read -r lines words <<< "$(awk ...)"` discarded awk's exit status, so a
|
||||||
# skill-audit/scripts/validate.sh for its own line count) — `wc -l`
|
# file awk could not read yielded empty variables, bash arithmetic read
|
||||||
# undercounts by 1 in that case. Word count uses awk's default
|
# them as 0, and both ceilings passed in total silence — the one outcome
|
||||||
# whitespace-splitting NF, matching `wc -w` semantics.
|
# this script forbids itself.
|
||||||
read -r lines words <<< "$(awk '{w += NF} END{print NR, w+0}' "$f")"
|
# * awk's NR/NF do not agree with the Python splitlines()/split() that
|
||||||
|
# skill-audit/scripts/validate.sh uses for the SAME two constants.
|
||||||
if (( lines > MAX_LINES )); then
|
# splitlines() also breaks on \x0b \x0c \x1c \x1d \x1e \x85 U+2028 U+2029
|
||||||
echo "ERROR: $f has $lines lines, exceeding the $MAX_LINES-line ceiling (agentskills.io skill-authoring.md)" >&2
|
# and split() on every Unicode space, so a body padded with U+2028 read as
|
||||||
FAIL=1
|
# 6 lines here and 606 lines there — hook green, audit FAIL.
|
||||||
fi
|
# One implementation now owns both: the Python block below already reads every
|
||||||
|
# file (with a real diagnostic on failure), so it counts there.
|
||||||
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
|
|
||||||
done
|
done
|
||||||
|
|
||||||
if ! command -v python3 > /dev/null 2>&1; then
|
if ! command -v python3 > /dev/null 2>&1; then
|
||||||
@@ -140,7 +139,7 @@ fi
|
|||||||
|
|
||||||
if ! python3 -u - \
|
if ! python3 -u - \
|
||||||
"$DESC_SUGGEST_CHARS" "$DESC_MAX_CHARS" \
|
"$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 glob
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
@@ -153,7 +152,8 @@ DESC_MAX_CHARS = int(sys.argv[2])
|
|||||||
BODY_SUGGEST_WORDS = int(sys.argv[3])
|
BODY_SUGGEST_WORDS = int(sys.argv[3])
|
||||||
BODY_MAX_WORDS = int(sys.argv[4])
|
BODY_MAX_WORDS = int(sys.argv[4])
|
||||||
MAX_WORDS = int(sys.argv[5])
|
MAX_WORDS = int(sys.argv[5])
|
||||||
files = sys.argv[6:]
|
MAX_LINES = int(sys.argv[6])
|
||||||
|
files = sys.argv[7:]
|
||||||
|
|
||||||
failed = False
|
failed = False
|
||||||
|
|
||||||
@@ -232,17 +232,21 @@ def read_text(path):
|
|||||||
# which is what a monorepo means,
|
# which is what a monorepo means,
|
||||||
# 2. the target's own apm package,
|
# 2. the target's own apm package,
|
||||||
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
# 3. the packages that package DECLARES in apm.yml dependencies.apm.
|
||||||
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted in that
|
# Deployed .claude/ and .agents/ trees are deliberately NOT consulted when the
|
||||||
# case. They are `apm install` output, gitignored, and present only on a machine
|
# root came from the plugins/ probe. They are `apm install` output, gitignored,
|
||||||
# that has run it: four cross-plugin targets in this repo (gitea-branches ->
|
# and present only on a machine that has run it: four cross-plugin targets in
|
||||||
# git-branches, gitea-branches -> git-history, gitea-issues -> git-branches,
|
# this repo (gitea-branches -> git-branches, gitea-branches -> git-history,
|
||||||
# gitea-workflow -> git-workflow) resolved through .claude/skills/ alone, so the
|
# gitea-issues -> git-branches, gitea-workflow -> git-workflow) resolved through
|
||||||
# same commit measured 2 dangling targets on a developer machine and 6 on a
|
# .claude/skills/ alone, so the same commit measured 2 dangling targets on a
|
||||||
# fresh clone. A gate shipping hot with no baseline cannot give two answers.
|
# 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
|
# Deployed trees ARE used when no plugin monorepo was found — whether the walk
|
||||||
# case, where the file being checked lives in or beside a deployed tree and
|
# landed on a bare .git ancestor or on nothing at all. That is the consumer
|
||||||
# there is no monorepo to read.
|
# 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):
|
def _is_fs_root(path):
|
||||||
@@ -251,11 +255,17 @@ def _is_fs_root(path):
|
|||||||
|
|
||||||
def _collect_package(pkg_dir, names):
|
def _collect_package(pkg_dir, names):
|
||||||
"""Add every skill/agent name a package directory exposes, any layout."""
|
"""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 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())
|
names.add(os.path.basename(path.rstrip('/')).lower())
|
||||||
for sub in ('.apm/agents/*.md', 'agents/*.md'):
|
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)
|
base = os.path.basename(path)
|
||||||
if base.endswith('.agent.md'):
|
if base.endswith('.agent.md'):
|
||||||
base = base[:-len('.agent.md')]
|
base = base[:-len('.agent.md')]
|
||||||
@@ -287,28 +297,35 @@ def _apm_package_root(start_dir):
|
|||||||
def _authoring_root(start_dir):
|
def _authoring_root(start_dir):
|
||||||
"""Nearest ancestor that is a plugin monorepo, else the nearest .git tree.
|
"""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
|
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
|
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
|
up. Both passes stop before the filesystem root for the same reason
|
||||||
_apm_package_root does.
|
_apm_package_root does.
|
||||||
"""
|
"""
|
||||||
for probe in (
|
probes = (
|
||||||
lambda d: bool(glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'skills'))
|
lambda d: bool(glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'skills'))
|
||||||
or glob.glob(os.path.join(d, 'plugins', '*', '.apm', 'agents'))),
|
or glob.glob(os.path.join(glob.escape(d), 'plugins', '*', '.apm', 'agents'))),
|
||||||
lambda d: os.path.exists(os.path.join(d, '.git'))):
|
lambda d: os.path.exists(os.path.join(d, '.git')))
|
||||||
|
for index, probe in enumerate(probes):
|
||||||
current = os.path.abspath(start_dir)
|
current = os.path.abspath(start_dir)
|
||||||
for _ in range(12):
|
for _ in range(12):
|
||||||
if _is_fs_root(current):
|
if _is_fs_root(current):
|
||||||
break
|
break
|
||||||
if probe(current):
|
if probe(current):
|
||||||
return current
|
return current, index == 0
|
||||||
current = os.path.dirname(current)
|
current = os.path.dirname(current)
|
||||||
return None
|
return None, False
|
||||||
|
|
||||||
|
|
||||||
def _collect_authoring_root(root, names):
|
def _collect_authoring_root(root, names):
|
||||||
"""Every plugin in the monorepo contributes its 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):
|
if os.path.isdir(pkg):
|
||||||
_collect_package(pkg, names)
|
_collect_package(pkg, names)
|
||||||
|
|
||||||
@@ -372,7 +389,7 @@ def _declared_dependency_dirs(pkg_dir):
|
|||||||
def _deployed_roots(start_dir):
|
def _deployed_roots(start_dir):
|
||||||
""".claude/ and .agents/ trees above start_dir — what a host really sees.
|
""".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:
|
filesystem root is skipped for the same reason _apm_package_root skips it:
|
||||||
a stray /.claude/skills/ must not join every path's universe.
|
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):
|
for dep_dir in _declared_dependency_dirs(package):
|
||||||
_collect_package(dep_dir, names)
|
_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:
|
if root:
|
||||||
_collect_authoring_root(root, names)
|
_collect_authoring_root(root, names)
|
||||||
else:
|
if not root_has_plugins:
|
||||||
for base in _deployed_roots(start):
|
for base in _deployed_roots(start):
|
||||||
_collect_package(base, names)
|
_collect_package(base, names)
|
||||||
return names
|
return names
|
||||||
@@ -520,11 +549,17 @@ MARKED_TARGET = r"(?:`/?(%s)`|(?<![\w./*-])/(%s)\b)" % (NAME_ANY, NAME_ANY)
|
|||||||
ANY_TARGET = r"(?:%s|(%s)\b)" % (MARKED_TARGET, NAME_HYPH)
|
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_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)
|
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)
|
# re.I on ALL of them, uniformly. The patterns are built from the same
|
||||||
CONT_ANY = re.compile(r"\s*(?:or|and|/|,)\s*%s" % ANY_TARGET)
|
# lowercase NAME_* fragments, so half of them carrying the flag and half not
|
||||||
ARROW_MARKED = re.compile(r"(?:->|→)\s*%s" % MARKED_TARGET)
|
# 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)
|
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
|
# A boundary clause takes two shapes and BOTH count: the prose markers, and
|
||||||
# ADR-0020's compressed arrow form `Not <thing> -> <name>`.
|
# 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_MARKER = re.compile(r"\b(?:do\s+not|instead|rather\s+than|not\s+for)\b", re.I)
|
||||||
@@ -691,8 +726,16 @@ def unresolved_targets(description, known):
|
|||||||
# description with a 1,000-word body exited 0 behind a BOM). A file that cannot
|
# 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
|
# be measured must never report green, so every caller of these two ERRORs on a
|
||||||
# miss instead of moving on.
|
# 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(
|
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):
|
def strip_bom(text):
|
||||||
@@ -724,14 +767,26 @@ def description_value(fm_text):
|
|||||||
try:
|
try:
|
||||||
data = yaml.safe_load(fm_text)
|
data = yaml.safe_load(fm_text)
|
||||||
except Exception as exc:
|
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):
|
if not isinstance(data, dict):
|
||||||
raise FrontmatterError('frontmatter is not a YAML mapping')
|
raise FrontmatterError('frontmatter is not a YAML mapping')
|
||||||
value = data.get('description')
|
value = data.get('description')
|
||||||
if value is None:
|
if value is None:
|
||||||
return ''
|
return ''
|
||||||
if not isinstance(value, str):
|
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()
|
return re.sub(r'\s+', ' ', value).strip()
|
||||||
|
|
||||||
|
|
||||||
@@ -801,6 +856,18 @@ def mask_fenced(text):
|
|||||||
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
if (marker and marker[0] == fence[0] and len(marker) >= len(fence)
|
||||||
and not stripped.strip()[len(marker):].strip()):
|
and not stripped.strip()[len(marker):].strip()):
|
||||||
fence = None
|
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)
|
return ''.join(out)
|
||||||
|
|
||||||
|
|
||||||
@@ -868,12 +935,28 @@ for path in files:
|
|||||||
"silence." % (path, why))
|
"silence." % (path, why))
|
||||||
continue
|
continue
|
||||||
try:
|
try:
|
||||||
content = strip_bom(read_text(path))
|
raw = read_text(path)
|
||||||
except EncodingError as exc:
|
except EncodingError as exc:
|
||||||
error("%s: %s. None of the ADR-0020 gates could run on this file."
|
error("%s: %s. Neither the spec line/word ceilings nor any of the "
|
||||||
% (path, exc))
|
"ADR-0020 gates could run on this file." % (path, exc))
|
||||||
continue
|
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)
|
fm_match = FRONTMATTER_RE.match(content)
|
||||||
if not fm_match:
|
if not fm_match:
|
||||||
error("%s: no parseable YAML frontmatter block. Expected a `---` line, "
|
error("%s: no parseable YAML frontmatter block. Expected a `---` line, "
|
||||||
@@ -887,8 +970,11 @@ for path in files:
|
|||||||
try:
|
try:
|
||||||
desc = description_value(fm_match.group(1))
|
desc = description_value(fm_match.group(1))
|
||||||
except FrontmatterError as exc:
|
except FrontmatterError as exc:
|
||||||
error("%s: frontmatter is not valid YAML (%s). None of the ADR-0020 "
|
# `exc` carries the whole clause — invalid YAML, a non-mapping block, or
|
||||||
"gates could run on this file." % (path, exc))
|
# 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
|
continue
|
||||||
|
|
||||||
body = content[fm_match.end():]
|
body = content[fm_match.end():]
|
||||||
|
|||||||
@@ -332,6 +332,76 @@ EOF
|
|||||||
)"
|
)"
|
||||||
expect "a fenced references/example-file.md does not ERROR" "$F_REF_FENCED" silent
|
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 ""
|
||||||
echo "--- a references/ pointer in a same-line removal context is history, not dispatch ---"
|
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
|
# Narrow on purpose: a live dispatch table never describes its own target as
|
||||||
|
|||||||
@@ -15,14 +15,20 @@
|
|||||||
# ready to ship and the commit hook then rejects it, or worse, the reverse. So the
|
# 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.
|
# comparison here is over VERDICTS on files, not over source text.
|
||||||
#
|
#
|
||||||
# Scope: the ADR-0020 axes the two scripts share — description length and tier,
|
# Scope: every axis the two scripts share. The ADR-0020 ones — description
|
||||||
# body word count and tier, dangling routing targets, missing references/
|
# length and tier, body word count and tier, dangling routing targets, missing
|
||||||
# pointers, the two Gotchas suggestions, the missing-boundary-clause suggestion,
|
# references/ pointers, the two Gotchas suggestions, the missing-boundary-clause
|
||||||
# a declined resolution, and an empty description. The two scripts legitimately
|
# suggestion, a declined resolution, an empty description — plus the two
|
||||||
# differ elsewhere (validate.sh also checks name/directory agreement, script
|
# agentskills.io spec ceilings, MAX_LINES and MAX_WORDS.
|
||||||
# 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
|
# Those last two were EXCLUDED from this comparison until a real divergence
|
||||||
# shape they were never meant to have.
|
# 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
|
# 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
|
# 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))"
|
python3 -c "print(' '.join(['word'] * 70))"
|
||||||
} >> "$FX/gotchas-fraction/SKILL.md"
|
} >> "$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.
|
# Empty description — the shape that used to exit 0 in silence.
|
||||||
mkdir -p "$FX/empty-desc"
|
mkdir -p "$FX/empty-desc"
|
||||||
printf -- '---\nname: empty-desc\ndescription:\nmodel: sonnet\n---\n\nDo the thing.\n' \
|
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)')),
|
('NO_BOUNDARY_CLAUSE', re.compile(r'(description has no boundary clause)')),
|
||||||
('RESOLUTION_DECLINED', re.compile(r'(boundary-target resolution DID NOT RUN)')),
|
('RESOLUTION_DECLINED', re.compile(r'(boundary-target resolution DID NOT RUN)')),
|
||||||
('DESC_EMPTY', re.compile(r'(description field is missing or empty)')),
|
('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
|
Lines that match no rule are dropped rather than compared: the two scripts
|
||||||
legitimately check different things outside ADR-0020 (name/directory
|
legitimately check different things outside ADR-0020 (name/directory
|
||||||
agreement, script executability, the 1024-char spec backstop, whole-file
|
agreement, script executability, the 1024-char spec backstop), and forcing
|
||||||
line and word ceilings), and forcing those into the comparison would report
|
those into the comparison would report a difference that is not a
|
||||||
a difference that is not a disagreement.
|
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()
|
found = set()
|
||||||
for raw in output.splitlines():
|
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')
|
problems.append('the hook reported an ADR-0020 ERROR but exited 0')
|
||||||
if audit_err and audit_rc == 0:
|
if audit_err and audit_rc == 0:
|
||||||
problems.append('skill-audit reported an ADR-0020 FAIL but exited 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):
|
# No escape hatch here any more. There used to be one — a
|
||||||
problems.append('the hook exited %d with no ADR-0020 ERROR and no spec-ceiling ERROR'
|
# `_non_adr_hook_error()` helper that waved through a non-zero hook exit
|
||||||
% hook_rc)
|
# 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:
|
if problems:
|
||||||
bad('%s: %s' % (label, '; '.join(problems)))
|
bad('%s: %s' % (label, '; '.join(problems)))
|
||||||
@@ -289,16 +361,6 @@ def compare(label, skill_dir):
|
|||||||
return False
|
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 -------------------------------------------------------
|
# --- The real corpus -------------------------------------------------------
|
||||||
corpus = sorted(glob.glob(os.path.join(repo_root, 'plugins', '*', '.apm', 'skills', '*')))
|
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'))]
|
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'
|
ok('every one of the %d compared axes was exercised by at least one fixture'
|
||||||
% len(expected_tokens))
|
% 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("")
|
||||||
print("Results: %d passed, %d failed" % (passes, failures))
|
print("Results: %d passed, %d failed" % (passes, failures))
|
||||||
sys.exit(1 if failures else 0)
|
sys.exit(1 if failures else 0)
|
||||||
|
|||||||
@@ -19,7 +19,22 @@
|
|||||||
# NEXT key. The value then looked present (so "missing or empty" never
|
# 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).
|
# 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
|
# 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
|
# 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
|
# 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':
|
elif kind == 'yaml-string':
|
||||||
fm_lines = ['just a bare scalar, not a mapping']
|
fm_lines = ['just a bare scalar, not a mapping']
|
||||||
elif kind == 'yaml-none':
|
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 = []
|
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':
|
elif kind == 'yaml-malformed':
|
||||||
fm_lines = ['name: ' + name, 'description: "unterminated', 'tabs:\t- a']
|
fm_lines = ['name: ' + name, 'description: "unterminated', 'tabs:\t- a']
|
||||||
elif kind == 'desc-no-value':
|
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" \
|
probe_all "CRLF line endings do not hide the findings" \
|
||||||
crlf "description is $DESC_CHARS char" "@skills:body is $BODY_WORDS words"
|
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
|
# 1b. Unparseable frontmatter is a hard ERROR, never a quiet skip
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
echo ""
|
echo ""
|
||||||
echo "--- genuinely unparseable frontmatter exits non-zero with a message, rather than passing quietly ---"
|
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"
|
build_subjects "$kind"
|
||||||
done
|
done
|
||||||
probe_all "frontmatter with no closing --- is reported, not skipped" \
|
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" \
|
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" \
|
probe_all "frontmatter that parses to a STRING is reported, not skipped" \
|
||||||
yaml-string "frontmatter"
|
yaml-string "frontmatter is not a YAML mapping"
|
||||||
probe_all "frontmatter that parses to None (empty block) is reported, not skipped" \
|
probe_all "frontmatter that parses to None (a comment-only block) is reported, not skipped" \
|
||||||
yaml-none "frontmatter"
|
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" \
|
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
|
# 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" \
|
probe_all "'description: >' with nothing folded under it FAILs" \
|
||||||
desc-empty-fold "description field is missing or empty"
|
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
|
# 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
|
# 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.
|
# made this un-diagnosable, so the output is asserted non-empty independently.
|
||||||
|
|||||||
@@ -13,6 +13,14 @@
|
|||||||
# identical with and without a deployed tree — on a synthetic fixture AND on
|
# identical with and without a deployed tree — on a synthetic fixture AND on
|
||||||
# the real 39-skill corpus.
|
# 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
|
# 2. THE BARE-TARGET GRAMMAR RULE. A hyphenated token used as a compound
|
||||||
# MODIFIER ("pre-commit hooks", "pull-request template") is prose, not a
|
# MODIFIER ("pre-commit hooks", "pull-request template") is prose, not a
|
||||||
# route; a terminal one is a real target. Getting this wrong in either
|
# 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>}"
|
fail "the consumer path did not resolve through the deployed tree (exit $CONSUMER_RC): ${CONSUMER_OUT:-<empty>}"
|
||||||
fi
|
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
|
# 1b. Machine independence — the real corpus
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -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"
|
"Unchecked target(s): some-other-skill"
|
||||||
|
|
||||||
echo ""
|
echo ""
|
||||||
echo "--- the three live dangling routing targets are caught (issue #100) ---"
|
echo "--- the live dangling routing targets are caught (issue #100) ---"
|
||||||
# ADR-0020 records four broken routing targets and splits fixing them into its
|
# ADR-0020 records the broken routing targets and splits fixing them into its own
|
||||||
# own issue. Three are detectable from the description text alone; this asserts
|
# issue. This asserts the gate actually sees them rather than the check being
|
||||||
# the gate actually sees them rather than the check being vacuous in the corpus
|
# vacuous in the corpus it was written against.
|
||||||
# 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 \
|
for probe in \
|
||||||
"plugins/bin/.apm/skills/research/SKILL.md:neuledge-context" \
|
"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
|
"plugins/gitea/.apm/skills/gitea-issues/SKILL.md:gitea-labels"; do
|
||||||
probe_file="$REPO_ROOT/${probe%%:*}"
|
probe_file="$REPO_ROOT/${probe%%:*}"
|
||||||
probe_name="${probe##*:}"
|
probe_name="${probe##*:}"
|
||||||
if [[ ! -f "$probe_file" ]]; then
|
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
|
continue
|
||||||
fi
|
fi
|
||||||
# Captured, not piped: the script exits non-zero on these files and
|
# Captured, not piped: the script exits non-zero on these files and
|
||||||
@@ -558,10 +570,8 @@ for probe in \
|
|||||||
set -e
|
set -e
|
||||||
if [[ "$probe_out" == *"routes to '$probe_name'"* ]]; then
|
if [[ "$probe_out" == *"routes to '$probe_name'"* ]]; then
|
||||||
pass "detects the dangling '$probe_name' target in ${probe%%:*}"
|
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
|
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
|
fi
|
||||||
done
|
done
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user