fix(gates): body-level routing targets are checked by nothing #124

Closed
opened 2026-08-31 08:07:10 +00:00 by Claude · 1 comment
Collaborator

Found during the #99 ADR-0020 retrofit and the follow-up audit pass on refactor/adr0020-skill-retrofit. The two live instances are fixed on that branch; the gate is not, and the gate is the actual ask.

The claim that is narrower than it reads

AGENTS.md says:

No routing target dangles, and tests/test-adr0020-targets.sh pins that set as empty, so a new boundary clause naming a non-existent skill fails the suite rather than joining a backlog.

True, and scoped to descriptions only. Every function in scripts/skill-size-check.sh that participates in target resolution takes the description string as its input — has_boundary_clause(description), boundary_targets(description), _arrow_targets(description), unresolved_targets(description, known), boundary_clause_status(description). The body is measured for word count and for unqualified references/… pointers; it is never scanned for skill names or slash commands.

tests/test-adr0020-targets.sh's EXPECTED_DANGLING="" assertion inherits that scope. It pins "no description names a target that does not resolve".

So: a skill name or slash command named in a body is validated by nothing. That is the larger surface — a description is one to three sentences, a body is up to 900 words of procedure, and the body is where an agent is actually told what to run next.

Both live instances were found by reading, not by any check

1. bin/write-docs/SKILL.md routed twice to to-prd, a skill deleted under the great refactoring milestone (its scope line: "delete stale bin skills (to-issues, to-prd)") and present nowhere in the tree:

- User wants a PRD, decision doc, or architecture proposal → `to-prd` or `grill-me`
- Requested output is a PRD, decision doc, or architecture proposal → redirect to `to-prd`, `grill-me`, or `grill-with-docs`

Note the arrow form. Had either line sat in the description, BOUNDARY_ARROW would have caught it and the commit would have been blocked. In the body it was invisible.

2. bin/triage/SKILL.md told the agent to run /setup-matt-pocock-skills, an unremoved upstream artifact with no counterpart in this repo:

These are canonical role names — the actual label strings used in the issue tracker may differ. The mapping should have been provided to you - run `/setup-matt-pocock-skills` if not.

This one had teeth: the role-to-label mapping step was left with no resolution path at all. An agent hitting the "if not" branch had nowhere to go.

Both were fixed in commit 03abcff. The fix requested here is the gate, not those two edits.

Design notes for whoever implements it

The description-side resolver is reusable — known_targets() walks up from the target file to an authoring root and builds the resolution universe without consulting the deployed .claude/ / .agents/ trees, which is what makes the verdict machine-independent. Point it at body text and the resolution half is done.

The hard half is extraction, and skill-size-check.sh's own comments are the best available record of why: a hyphenated token used as a compound modifier ("pre-commit hooks") is prose, not a route; terminal position alone is not evidence either ("run pre-commit instead", "see commit-msg"). Body prose is far denser in this hazard than a description is — a procedure step naming a file, a CLI verb, or a config key looks exactly like a route. Starting narrow is probably right: slash-command tokens (/name) and arrow forms (-> name, → name) only, both of which are unambiguous notation and both of which would have caught the two instances above.

bin/improve-codebase-architecture/SKILL.md linked twice into a sibling skill's directory:

(see [CONTEXT-FORMAT.md](../grill-with-docs/CONTEXT-FORMAT.md))
See [ADR-FORMAT.md](../grill-with-docs/ADR-FORMAT.md).

That resolves in this monorepo and breaks on a cache install, where each skill deploys standalone — the constraint skill-audit/references/file-structure.md states under "Cross-plugin path references". Also fixed in 03abcff, rewritten to the sanctioned possessive form. Also caught by reading rather than by a gate, and worth folding into the same check: a body-level ../ chain leaving the skill root is mechanically detectable and needs no grammar heuristic at all. That half could ship first.

Files

  • scripts/skill-size-check.sh — the resolver and every call site
  • tests/test-adr0020-targets.sh — EXPECTED_DANGLING and the machine-independence fixtures
  • AGENTS.md — the "No routing target dangles" line, which should say description once the scope is either widened or documented
  • plugins/kyberforge/.apm/skills/skill-audit/references/file-structure.md — "Cross-plugin path references"
  • #99 — the retrofit during which this surfaced
  • #100 — the description-level version of exactly this, already fixed and closed
  • #107, #110 — other holes in the same boundary-target resolver
Found during the #99 ADR-0020 retrofit and the follow-up audit pass on `refactor/adr0020-skill-retrofit`. The two live instances are fixed on that branch; the gate is not, and the gate is the actual ask. ## The claim that is narrower than it reads `AGENTS.md` says: > **No routing target dangles**, and `tests/test-adr0020-targets.sh` pins that set as empty, so a new boundary clause naming a non-existent skill fails the suite rather than joining a backlog. True, and scoped to **descriptions only**. Every function in `scripts/skill-size-check.sh` that participates in target resolution takes the description string as its input — `has_boundary_clause(description)`, `boundary_targets(description)`, `_arrow_targets(description)`, `unresolved_targets(description, known)`, `boundary_clause_status(description)`. The body is measured for word count and for unqualified `references/…` pointers; it is never scanned for skill names or slash commands. `tests/test-adr0020-targets.sh`'s `EXPECTED_DANGLING=""` assertion inherits that scope. It pins "no *description* names a target that does not resolve". So: **a skill name or slash command named in a body is validated by nothing.** That is the larger surface — a description is one to three sentences, a body is up to 900 words of procedure, and the body is where an agent is actually told what to run next. ## Both live instances were found by reading, not by any check **1. `bin/write-docs/SKILL.md` routed twice to `to-prd`**, a skill deleted under the great refactoring milestone (its scope line: *"delete stale bin skills (to-issues, to-prd)"*) and present nowhere in the tree: ``` - User wants a PRD, decision doc, or architecture proposal → `to-prd` or `grill-me` - Requested output is a PRD, decision doc, or architecture proposal → redirect to `to-prd`, `grill-me`, or `grill-with-docs` ``` Note the arrow form. Had either line sat in the description, `BOUNDARY_ARROW` would have caught it and the commit would have been blocked. In the body it was invisible. **2. `bin/triage/SKILL.md` told the agent to run `/setup-matt-pocock-skills`**, an unremoved upstream artifact with no counterpart in this repo: ``` These are canonical role names — the actual label strings used in the issue tracker may differ. The mapping should have been provided to you - run `/setup-matt-pocock-skills` if not. ``` This one had teeth: the role-to-label mapping step was left with no resolution path at all. An agent hitting the "if not" branch had nowhere to go. Both were fixed in commit `03abcff`. **The fix requested here is the gate, not those two edits.** ## Design notes for whoever implements it The description-side resolver is reusable — `known_targets()` walks up from the target file to an authoring root and builds the resolution universe without consulting the deployed `.claude/` / `.agents/` trees, which is what makes the verdict machine-independent. Point it at body text and the resolution half is done. The hard half is extraction, and `skill-size-check.sh`'s own comments are the best available record of why: a hyphenated token used as a compound modifier ("pre-commit hooks") is prose, not a route; terminal position alone is not evidence either ("run `pre-commit` instead", "see `commit-msg`"). Body prose is *far* denser in this hazard than a description is — a procedure step naming a file, a CLI verb, or a config key looks exactly like a route. Starting narrow is probably right: slash-command tokens (`/name`) and arrow forms (`-> name`, `→ name`) only, both of which are unambiguous notation and both of which would have caught the two instances above. ## The related-but-distinct gap in the same commit `bin/improve-codebase-architecture/SKILL.md` linked twice into a *sibling skill's* directory: ``` (see [CONTEXT-FORMAT.md](../grill-with-docs/CONTEXT-FORMAT.md)) See [ADR-FORMAT.md](../grill-with-docs/ADR-FORMAT.md). ``` That resolves in this monorepo and breaks on a cache install, where each skill deploys standalone — the constraint `skill-audit/references/file-structure.md` states under "Cross-plugin path references". Also fixed in `03abcff`, rewritten to the sanctioned possessive form. Also caught by reading rather than by a gate, and worth folding into the same check: a body-level `../` chain leaving the skill root is mechanically detectable and needs no grammar heuristic at all. That half could ship first. ## Files - `scripts/skill-size-check.sh` — the resolver and every call site - `tests/test-adr0020-targets.sh` — `EXPECTED_DANGLING` and the machine-independence fixtures - `AGENTS.md` — the "No routing target dangles" line, which should say *description* once the scope is either widened or documented - `plugins/kyberforge/.apm/skills/skill-audit/references/file-structure.md` — "Cross-plugin path references" ## Related - #99 — the retrofit during which this surfaced - #100 — the description-level version of exactly this, already fixed and closed - #107, #110 — other holes in the same boundary-target resolver
Claude added this to the Skills & Agents milestone 2026-08-31 08:07:10 +00:00
Claude added the Kind/Bug
Reviewed
Confirmed
1
Priority
Medium
3
labels 2026-08-31 08:07:10 +00:00
Author
Collaborator

Automated note from the PR #135 review follow-up. This issue's own conclusion, state and labels are untouched — this comment only records what PR #135's branch renamed or removed, so the issue stays actionable.

What this issue quotes that no longer exists

  • The AGENTS.md sentence quoted here — "No routing target dangles, and tests/test-adr0020-targets.sh pins that set as empty" — was removed by this branch's AGENTS.md slimdown. Searching AGENTS.md for that wording will find nothing.
  • The test it names, tests/test-adr0020-targets.sh, does still exist and still runs. Only the AGENTS.md sentence describing it is gone, so the guarantee is still enforced even though the prose pointer is not.
  • This issue also cites skill-audit/references/file-structure.md, the same stale path as #121. It is now factory-audit/references/skill-file-structure.md — skill-audit was merged into factory-audit (ADR-0025, commit 620f20b), and the filename changed as well as the directory.

Nothing else in this issue was re-validated.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2

Automated note from the PR #135 review follow-up. This issue's own conclusion, state and labels are untouched — this comment only records what PR #135's branch renamed or removed, so the issue stays actionable. **What this issue quotes that no longer exists** - The AGENTS.md sentence quoted here — "**No routing target dangles**, and tests/test-adr0020-targets.sh pins that set as empty" — was removed by this branch's AGENTS.md slimdown. Searching AGENTS.md for that wording will find nothing. - The test it names, `tests/test-adr0020-targets.sh`, **does still exist and still runs**. Only the AGENTS.md sentence describing it is gone, so the guarantee is still enforced even though the prose pointer is not. - This issue also cites `skill-audit/references/file-structure.md`, the same stale path as #121. It is now `factory-audit/references/skill-file-structure.md` — `skill-audit` was merged into `factory-audit` (ADR-0025, commit `620f20b`), and the filename changed as well as the directory. Nothing else in this issue was re-validated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwD8Egs5r4ndqeFLmhusX2
Sign in to join this conversation.