Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/PLUGIN-PHILOSOPHY.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,14 @@ For project configuration, use neutral repository-relative paths anchored at
when the contract requires containment, and document precedence. Do not add an environment variable
merely to create a second configuration channel.

Apply the same anchoring rule to bundled assets: one skill citing another skill's supporting file
writes the full `${CLAUDE_PLUGIN_ROOT}/skills/<other-skill>/<path>` form, optionally paired with a
relative markdown link target for browsing on GitHub — for example
``[`${CLAUDE_PLUGIN_ROOT}/skills/audit/context/suppression.md`](../audit/context/suppression.md)``.
A bare `context/…`-style path is reserved for a skill's OWN supporting files; it resolves against
the citing skill's directory, so a cross-skill citation written that way points at a file that is
not there.

## Setup is explicit and repeatable

A plugin requires a `setup` skill iff it has (a) a consumer-project configuration surface, (b) an
Expand Down
2 changes: 1 addition & 1 deletion plugins/skill-quality/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
"name": "skill-quality",
"version": "0.15.0",
"version": "0.15.1",
"description": "Skill-authoring QA tooling: a static contract checker that runs twenty-two deterministic checks over a Claude Code skill (frontmatter, per-skill listing-entry cap, trigger-keyword preservation, line caps, broken internal refs, markdownlint, gotchas surface, evals presence, precompute opportunity, injection shell-declaration, fresh-eyes declaration conformance), a shared skill-listing budget reporter across a set of skills, and a bundled evals.json schema plus a deterministic eval-quality lint (duplicate case identities, missing fixtures, empty or vague grading criteria, set-coverage warnings). Runs against any repo's skills directory via the convention-resolution ladder — no baked layout.",
"author": {
"name": "Melodic Software",
Expand Down
18 changes: 18 additions & 0 deletions plugins/skill-quality/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,24 @@
All notable changes to the `skill-quality` plugin are documented here. Format follows
[Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.

## [0.15.1]

### Fixed

- **A cross-skill citation reported as a broken internal ref pointed at the wrong directory.**
Check 5 resolves every bare `context/…`-shaped path against the CITING skill's own directory, so
a skill citing a sibling skill's supporting file failed with "no such file under the skill dir"
while the file plainly existed one directory over. The message sent the author looking for the
file where it could never be. When the unresolved path does resolve under a sibling skill of the
same skills root, the finding now also names that sibling and the citation form that works —
`${CLAUDE_PLUGIN_ROOT}/skills/<sibling>/<path>` in a plugin-shaped root, `../<sibling>/<path>`
outside one, where that variable is undefined. Still a FAIL: the bare form really does resolve
against the citing skill, so it is wrong regardless of where the file lives. The sibling hit is
evidence, not proof — this check deliberately extracts prose and inline-code refs, so a generic
path can collide with an unrelated same-named sibling file — so the original hand-verify wording
is kept and the suggestion is phrased conditionally. A path no sibling hosts is unchanged.
Extraction is unchanged too: prose and inline-code refs are still in scope, deliberately.

## [0.15.0]

### Changed
Expand Down
5 changes: 4 additions & 1 deletion plugins/skill-quality/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ phrase, which quietly degrades a skill's auto-invocation. Check 3 compares the t
truncates that entry) — a different, narrower limit from the shared budget below.
- Trigger-keyword preservation vs `HEAD` (skipped for a new, uncommitted skill).
- `SKILL.md` under 500 lines (hard) / 200 lines (soft, advisory).
- Backtick- and link-cited skill-internal supporting files resolve.
- Backtick- and link-cited skill-internal supporting files resolve — when a path that misses instead
resolves under a sibling skill, the finding names that sibling and the
`${CLAUDE_PLUGIN_ROOT}/skills/<sibling>/...` cross-skill form, while keeping the hand-verify
caveat (the sibling hit is evidence, not proof: paths can collide).
- `markdownlint-cli2` clean (advisory-skips when `npx` is absent).
- `scripts/*.test.sh` pass where present.
- Vendored `vendor/` byte-identical vs `HEAD`; stale-tracking metadata keys preserved; sync age.
Expand Down
58 changes: 55 additions & 3 deletions plugins/skill-quality/scripts/check-skill.sh
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@
# sibling did not carry at the base ref — WARNs, since the marketplace
# listing still routes it; lost phrases and coincidental overlap FAIL)
# 4. SKILL.md < 500 lines (hard cap)
# 5. Backtick-cited skill-internal supporting files resolve
# 5. Backtick-cited skill-internal supporting files resolve (a path that misses
# here but resolves under a SIBLING skill also names that sibling and the
# `${CLAUDE_PLUGIN_ROOT}/skills/<sibling>/…` cross-skill form, without
# dropping the hand-verify caveat — the hit may be a name collision)
# 6. markdownlint clean (markdownlint-cli2; WARN-skip if npx absent)
# 7. scripts/*.test.sh pass where present
# 8. vendor/ byte-identical vs HEAD, unless paired with an upstream-version
Expand Down Expand Up @@ -197,6 +200,15 @@ if [[ ! -f "$SKILL_MD" ]]; then
exit 1
fi

# Is this skill bundled in a PLUGIN, or a loose skill under some skills root?
# A plugin manifest two levels up (<plugin>/skills/<skill>/) is the marker.
# Several checks branch on it — check 1 (a declared `name` also registers a bare
# alias there) and check 5 (a cross-skill citation anchors at
# ${CLAUDE_PLUGIN_ROOT}, which is undefined outside a plugin) — so the layout
# convention is asserted in ONE place rather than restated per call site.
IS_PLUGIN_SKILL=0
[[ -f "$SKILL_DIR/../../.claude-plugin/plugin.json" ]] && IS_PLUGIN_SKILL=1

# --- Check 1: frontmatter parses; description present; declared name matches --

FRONTMATTER="$(skill_frontmatter::extract <"$SKILL_MD")"
Expand Down Expand Up @@ -229,7 +241,7 @@ else
err "frontmatter name '$CUR_NAME' is not kebab-case ([a-z0-9] and hyphens, per the Agent Skills spec)"
elif [[ -n "$CUR_NAME" && "$CUR_NAME" != "$SKILL_NAME" ]]; then
err "frontmatter name '$CUR_NAME' does not match skill directory '$SKILL_NAME'"
elif [[ -n "$CUR_NAME" && -f "$SKILL_DIR/../../.claude-plugin/plugin.json" ]]; then
elif [[ -n "$CUR_NAME" && "$IS_PLUGIN_SKILL" == 1 ]]; then
# In a PLUGIN skill a matching `name` is not inert: a declared name also
# answers to the bare `/<name>` unless another command owns that token
# (https://code.claude.com/docs/en/skills#how-a-skill-gets-its-command-name),
Expand Down Expand Up @@ -363,7 +375,47 @@ while IFS= read -r ref; do
fi
if [[ ! -e "$SKILL_DIR/$ref" ]]; then
ref_line="$(grep -nF "$ref" "$SKILL_MD" 2>/dev/null | head -1 | cut -d: -f1)"
err "broken skill-internal ref: $ref (no such file under the skill dir; cited at SKILL.md:${ref_line:-?} — hand-verify the line before fixing, may be an illustrative example)"
# A path that misses here but DOES resolve under a SIBLING skill of the same
# skills root is most often a cross-skill citation written in the
# skill-internal form — not a missing file. Every bare path in this check
# resolves against the CITING skill's dir, so the bare form is wrong either
# way and still FAILs; what changes is the message. The default wording
# sends the author looking for the file under their own skill, where it will
# never be — naming the host sibling and the anchored form that works is the
# difference between a dead end and a one-line fix.
#
# The sibling hit is EVIDENCE, not proof: this check deliberately extracts
# prose and inline-code refs, so a generic path (`scripts/run.sh`) can
# collide with an unrelated same-named sibling file. The wording therefore
# stays conditional and keeps the hand-verify instruction the default
# message carries — a coincidental name match and an illustrative example
# are both still live readings. Glob order is sorted, so a path present
# under more than one sibling names the first deterministically.
REF_HOST=""
for other_md in "$SKILLS_ROOT"/*/SKILL.md; do
[[ -f "$other_md" ]] || continue
[[ "$other_md" == "$SKILL_MD" ]] && continue
if [[ -e "${other_md%/SKILL.md}/$ref" ]]; then
REF_HOST="${other_md%/SKILL.md}"
REF_HOST="${REF_HOST##*/}"
break
Comment thread
kyle-sexton marked this conversation as resolved.
fi
done
if [[ -z "$REF_HOST" ]]; then
err "broken skill-internal ref: $ref (no such file under the skill dir; cited at SKILL.md:${ref_line:-?} — hand-verify the line before fixing, may be an illustrative example)"
else
# Plugin-shaped root: bundled plugin assets are anchored at the plugin
# root, so that is the form to name. Outside a plugin
# ${CLAUDE_PLUGIN_ROOT} is undefined, so name the layout-free
# sibling-relative form rather than advertise a variable that resolves to
# nothing.
if [[ "$IS_PLUGIN_SKILL" == 1 ]]; then
ref_fix="\${CLAUDE_PLUGIN_ROOT}/skills/$REF_HOST/$ref"
else
ref_fix="../$REF_HOST/$ref"
fi
err "broken skill-internal ref: $ref (no such file under the skill dir; cited at SKILL.md:${ref_line:-?} — hand-verify the line before fixing, may be an illustrative example). A file with that path DOES exist under sibling skill '$REF_HOST': if that is the file meant, this is a cross-skill citation, and a bare path always resolves against the CITING skill's dir — write it as $ref_fix. If the names merely collide, the ref is unrelated to that sibling."
fi
fi
done < <(
{
Expand Down
111 changes: 111 additions & 0 deletions plugins/skill-quality/scripts/check-skill.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2285,6 +2285,117 @@ else
fail "redundancy warning should be plugin-scoped"
fi

# 36a. Check 5: a bare path that misses under the citing skill but resolves under
# a SIBLING skill is most likely a cross-skill citation, not a missing file.
# It still FAILs (the bare form really does resolve against the citing
# skill), but in a plugin-shaped skills root the message must name the host
# sibling and the `${CLAUDE_PLUGIN_ROOT}` form — the default wording alone
# sends the author hunting under their own skill, where the file will never
# be. The hand-verify caveat stays: the hit is evidence, not proof.
mkdir -p "$PLUGIN_SKILLS/xref-host/context" "$PLUGIN_SKILLS/xref-citer"
printf 'Sibling-owned supporting file.\n' >"$PLUGIN_SKILLS/xref-host/context/suppression.md"
printf '%s' '---
description: "Hosts a supporting file. Use when: '"'"'checking cross-skill citations'"'"'."
---

## Purpose

Fixture sibling that owns context/suppression.md.

## Gotchas

None known.
' >"$PLUGIN_SKILLS/xref-host/SKILL.md"
printf '%s' '---
description: "Cites a sibling file. Use when: '"'"'checking cross-skill citations'"'"'."
---

## Purpose

Cites the sibling-owned `context/suppression.md` in the skill-internal form.

## Gotchas

None known.
' >"$PLUGIN_SKILLS/xref-citer/SKILL.md"
out="$( (cd "$TMP" &&
CHECK_SKILL_SKILLS_ROOT="$PLUGIN_SKILLS" CHECK_SKILL_SKIP_MARKDOWNLINT=1 \
bash "$SUT" xref-citer) 2>&1)"
rc=$?
if [[ $rc -eq 1 ]] &&
grep -q 'broken skill-internal ref: context/suppression.md' <<<"$out" &&
grep -q "sibling skill 'xref-host'" <<<"$out" &&
grep -qF '${CLAUDE_PLUGIN_ROOT}/skills/xref-host/context/suppression.md' <<<"$out" &&
grep -q 'hand-verify the line' <<<"$out"; then
pass "a sibling-hosted ref names the sibling and the plugin-root form"
else
fail "sibling-hosted ref should name the sibling and the plugin-root form (rc=$rc): $out"
fi

# 36b. Same detection outside a plugin: `${CLAUDE_PLUGIN_ROOT}` is undefined
# there, so the message must name the layout-free sibling-relative form
# instead of advertising a variable the consumer does not have.
mkdir -p "$SKILLS/plain-host/reference"
printf 'Sibling-owned supporting file.\n' >"$SKILLS/plain-host/reference/scaling.md"
make_skill plain-host '---
description: "Hosts a supporting file. Use when: '"'"'checking non-plugin cross-skill citations'"'"'."
---

## Purpose

Fixture sibling that owns reference/scaling.md.

## Gotchas

None known.
'
make_skill plain-citer '---
description: "Cites a sibling file. Use when: '"'"'checking non-plugin cross-skill citations'"'"'."
---

## Purpose

Cites the sibling-owned `reference/scaling.md` in the skill-internal form.

## Gotchas

None known.
'
out="$(run plain-citer 2>&1)"
rc=$?
if [[ $rc -eq 1 ]] &&
grep -q "sibling skill 'plain-host'" <<<"$out" &&
grep -q '\.\./plain-host/reference/scaling\.md' <<<"$out" &&
! grep -q 'skills/plain-host/reference/scaling\.md' <<<"$out"; then
pass "outside a plugin the sibling message names the relative form, not the plugin root"
else
fail "non-plugin sibling message should name ../plain-host/... (rc=$rc): $out"
fi

# 36c. False-positive guard: a ref no sibling hosts is still a plain broken
# internal ref, with the original hand-verify wording intact.
make_skill orphan-citer '---
description: "Cites a file nobody owns. Use when: '"'"'checking the broken-ref message'"'"'."
---

## Purpose

Cites `context/nowhere-at-all.md`, which no skill in this root owns.

## Gotchas

None known.
'
out="$(run orphan-citer 2>&1)"
rc=$?
if [[ $rc -eq 1 ]] &&
grep -q 'broken skill-internal ref: context/nowhere-at-all.md' <<<"$out" &&
! grep -q 'sibling skill' <<<"$out"; then
pass "a ref no sibling hosts keeps the plain broken-internal-ref message"
else
fail "unhosted ref should keep the broken-internal-ref message (rc=$rc): $out"
fi

if [[ $fails -ne 0 ]]; then
printf '%d assertion(s) failed\n' "$fails" >&2
exit 1
Expand Down