From 420d0e12e2a78fb5cf52621f6689b6b05ed87581 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 20 Jul 2026 13:27:17 -0400 Subject: [PATCH 1/3] feat(ci): register cross-plugin skill leaf-name collisions Eight leaf names are carried by more than one plugin -- setup by 33, audit by 6, and check/clean/diagnose/plan/workflow/write by 2 each. Every one is separately invocable, so none of this is a correctness problem and none of it is a rename mandate. What namespacing does not cover is the listing. The picker labels a row by the leaf name and keeps : as a hidden alias, so two colliding skills read identically and are told apart only by the () prefix on the description. That cost is invisible from inside any one plugin, and the grammar's own collision rule governs siblings within a namespace, not across plugins -- so a new collision appears silently and nothing forces a decision. Mirrors the check-cross-plugin-source-drift idiom: discover mode lists the collisions, --check fails on an unregistered one, and a stale guard drops entries that no longer collide so the registry cannot rot into pre-authorization for a future name. The registry seeds with all eight, each carrying the grounds it is accepted on rather than a bare name. Closes #720 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_011ogVV7z2Heg9ojJ88nqNxh --- .github/workflows/ci.yml | 15 +++ scripts/check-skill-leaf-names.sh | 123 +++++++++++++++++++++++++ scripts/check-skill-leaf-names.test.sh | 123 +++++++++++++++++++++++++ scripts/skill-leaf-name-registry.txt | 53 +++++++++++ 4 files changed, 314 insertions(+) create mode 100755 scripts/check-skill-leaf-names.sh create mode 100755 scripts/check-skill-leaf-names.test.sh create mode 100644 scripts/skill-leaf-name-registry.txt diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcd906cdb..6134ae965 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -282,6 +282,20 @@ jobs: - name: Run cross-plugin-source-drift tests run: bash scripts/check-cross-plugin-source-drift.test.sh + skill-leaf-name-gate: + runs-on: ubuntu-24.04 + timeout-minutes: 15 + steps: + - name: Check out + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + # Self-test first, so a broken detector cannot mask a regression. + - name: Run skill-leaf-name tests + run: bash scripts/check-skill-leaf-names.test.sh + - name: Check for unregistered cross-plugin skill leaf-name collisions + run: scripts/check-skill-leaf-names.sh --check + silent-skip-gate: runs-on: ubuntu-24.04 timeout-minutes: 15 @@ -577,6 +591,7 @@ jobs: - hook-utils-sync - standards-contract-sync - cross-plugin-source-drift + - skill-leaf-name-gate - silent-skip-gate - orphaned-fixture-gate - changelog-parity-gate diff --git a/scripts/check-skill-leaf-names.sh b/scripts/check-skill-leaf-names.sh new file mode 100755 index 000000000..d4e95babd --- /dev/null +++ b/scripts/check-skill-leaf-names.sh @@ -0,0 +1,123 @@ +#!/usr/bin/env bash +# Discover and check cross-plugin skill leaf-name collisions: a skill directory +# name (the leaf of `/:`) carried by 2+ plugins. +# +# scripts/check-skill-leaf-names.sh discover: list every leaf name +# owned by 2+ plugins, with its +# owners and registration state +# scripts/check-skill-leaf-names.sh --check fail on an UNREGISTERED +# collision, or a registry entry +# that no longer collides +# +# Namespacing already guarantees every one of these is separately invocable -- +# `/disk-hygiene:clean` and `/repo-hygiene:clean` can never resolve to each +# other, and the philosophy's Naming section is explicit that a built-in never +# forces a plugin skill's name. So a collision is not a correctness bug and this +# script is not a rename mandate. +# +# What it catches is the thing namespacing does NOT cover: the slash-command +# picker labels a row by the LEAF name and keeps `:` as a hidden +# alias, so two colliding skills read identically in the listing and are told +# apart only by the `()` prefix the description carries. That cost +# is invisible from inside any one plugin -- nothing in a single skill's own +# review surfaces it -- and the grammar's own collision rule +# (docs/PLUGIN-PHILOSOPHY.md, Naming) governs siblings WITHIN one namespace, not +# across plugins. +# +# Registering a leaf name in skill-leaf-name-registry.txt records the grounds it +# was accepted on. An unregistered collision is a decision waiting to be made, +# not yet a violation of anything. +set -euo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/.." + +registry="scripts/skill-leaf-name-registry.txt" + +# leaf name -> space-separated owning plugin names +declare -A leaf_owners + +for plugin_dir in plugins/*/; do + plugin="${plugin_dir%/}" + plugin="${plugin##*/}" + skills_dir="${plugin_dir}skills" + [[ -d "$skills_dir" ]] || continue + for skill_dir in "$skills_dir"/*/; do + [[ -f "${skill_dir}SKILL.md" ]] || continue + leaf="${skill_dir%/}" + leaf="${leaf##*/}" + leaf_owners["$leaf"]+="${plugin} " + done +done + +# Collisions only: a leaf name owned by 2+ plugins. +declare -A collisions +for leaf in "${!leaf_owners[@]}"; do + # shellcheck disable=SC2206 # plugin names are kebab-case; word-splitting is the intent + owners=(${leaf_owners[$leaf]}) + ((${#owners[@]} >= 2)) || continue + collisions["$leaf"]="${leaf_owners[$leaf]}" +done + +declare -A registered +if [[ -f "$registry" ]]; then + while IFS= read -r line; do + line="${line%%#*}" + line="${line#"${line%%[![:space:]]*}"}" + line="${line%"${line##*[![:space:]]}"}" + [[ -z "$line" ]] && continue + registered["$line"]=1 + done <"$registry" +fi + +mode="${1:-discover}" +case "$mode" in +discover | --check) ;; +*) + echo "usage: $(basename "$0") [--check]" >&2 + exit 2 + ;; +esac + +if [[ "$mode" == "discover" ]]; then + if ((${#collisions[@]} == 0)); then + echo "No cross-plugin skill leaf-name collisions." + exit 0 + fi + for leaf in $(printf '%s\n' "${!collisions[@]}" | sort); do + # shellcheck disable=SC2206 + owners=(${collisions[$leaf]}) + state="UNREGISTERED" + [[ -n "${registered[$leaf]:-}" ]] && state="registered" + printf '%-14s %-14s %d plugins: %s\n' \ + "$leaf" "$state" "${#owners[@]}" "$(printf '%s ' "${owners[@]}" | sed 's/ $//')" + done + exit 0 +fi + +failed=0 + +for leaf in $(printf '%s\n' "${!collisions[@]}" | sort); do + [[ -n "${registered[$leaf]:-}" ]] && continue + # shellcheck disable=SC2206 + owners=(${collisions[$leaf]}) + printf 'FAIL: skill leaf name %s is now carried by %d plugins (%s) and is not registered.\n' \ + "$leaf" "${#owners[@]}" "$(printf '%s ' "${owners[@]}" | sed 's/ $//')" >&2 + printf ' These are separately invocable, but the picker labels both rows %s.\n' "$leaf" >&2 + printf ' Rename one, or add %s to %s with the grounds it is accepted on.\n' "$leaf" "$registry" >&2 + failed=1 +done + +# Stale guard: a registry entry that no longer collides has outlived its reason +# and would otherwise silently pre-authorize a future collision on that name. +for leaf in $(printf '%s\n' "${!registered[@]}" | sort); do + [[ -n "${collisions[$leaf]:-}" ]] && continue + printf 'FAIL: %s lists %s, but it is no longer carried by 2+ plugins. Drop the entry.\n' \ + "$registry" "$leaf" >&2 + failed=1 +done + +if ((failed)); then + exit 1 +fi + +printf 'All %d cross-plugin skill leaf-name collisions are registered.\n' "${#collisions[@]}" diff --git a/scripts/check-skill-leaf-names.test.sh b/scripts/check-skill-leaf-names.test.sh new file mode 100755 index 000000000..5caf7da45 --- /dev/null +++ b/scripts/check-skill-leaf-names.test.sh @@ -0,0 +1,123 @@ +#!/usr/bin/env bash +# Black-box contract test for check-skill-leaf-names.sh. +# +# Self-contained and cwd-independent: builds a throwaway plugins/ tree with +# fixture skills, runs the checker against it, and asserts on exit code + +# output. Mutates only its own mktemp dir. The SUT resolves its paths relative +# to its own location, so the fixture tree carries a copy of it. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SUT_SRC="$SCRIPT_DIR/check-skill-leaf-names.sh" + +fails=0 +pass() { printf 'ok - %s\n' "$1"; } +fail() { + printf 'FAIL - %s\n' "$1" >&2 + fails=$((fails + 1)) +} + +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +mkdir -p "$TMP/scripts" +cp "$SUT_SRC" "$TMP/scripts/check-skill-leaf-names.sh" +SUT="$TMP/scripts/check-skill-leaf-names.sh" +REGISTRY="$TMP/scripts/skill-leaf-name-registry.txt" + +make_skill() { + # plugin, skill + mkdir -p "$TMP/plugins/$1/skills/$2" + printf -- '---\nname: %s\ndescription: "fixture"\n---\n' "$2" >"$TMP/plugins/$1/skills/$2/SKILL.md" +} + +run() { bash "$SUT" "$@" 2>&1; } + +# A directory without SKILL.md must not count as a skill. +mkdir -p "$TMP/plugins/alpha/skills/not-a-skill" + +make_skill alpha solo +make_skill alpha shared +make_skill beta shared +make_skill gamma shared + +# 1. discover lists a collision with all its owners. +out="$(run)" +if grep -q 'shared' <<<"$out" && grep -q '3 plugins' <<<"$out"; then + pass "discover lists a collision with its owner count" +else + fail "discover should list shared across 3 plugins: $out" +fi + +# 2. A leaf name owned by only one plugin is not a collision. +if ! grep -q 'solo' <<<"$out"; then + pass "single-owner leaf name is not reported" +else + fail "solo should not be reported as a collision: $out" +fi + +# 3. A directory without SKILL.md is not counted as a skill. +if ! grep -q 'not-a-skill' <<<"$out"; then + pass "directory without SKILL.md is not counted" +else + fail "not-a-skill should be ignored: $out" +fi + +# 4. --check fails on an unregistered collision. +: >"$REGISTRY" +out="$(run --check)" +rc=$? +if [[ $rc -eq 1 ]] && grep -q 'is not registered' <<<"$out"; then + pass "unregistered collision fails --check" +else + fail "unregistered collision should fail (rc=$rc): $out" +fi + +# 5. --check passes once the collision is registered, and comments/blank lines +# in the registry are ignored. +printf '# a comment\n\n shared # trailing comment\n' >"$REGISTRY" +out="$(run --check)" +rc=$? +if [[ $rc -eq 0 ]] && grep -q 'are registered' <<<"$out"; then + pass "registered collision passes --check (comments and whitespace ignored)" +else + fail "registered collision should pass (rc=$rc): $out" +fi + +# 6. Stale guard: a registry entry that no longer collides fails. +printf 'shared\nvanished\n' >"$REGISTRY" +out="$(run --check)" +rc=$? +if [[ $rc -eq 1 ]] && grep -q 'no longer carried by 2+ plugins' <<<"$out"; then + pass "stale registry entry fails --check" +else + fail "stale entry should fail (rc=$rc): $out" +fi + +# 7. A NEW collision on an unregistered name fails even when others are +# registered — the regression-critical path this gate exists for. +printf 'shared\n' >"$REGISTRY" +make_skill alpha newdupe +make_skill beta newdupe +out="$(run --check)" +rc=$? +if [[ $rc -eq 1 ]] && grep -q 'newdupe' <<<"$out"; then + pass "a newly introduced collision fails while existing ones stay registered" +else + fail "new collision should fail (rc=$rc): $out" +fi + +# 8. Unknown mode is a usage error, not a silent pass. +bash "$SUT" --bogus >/dev/null 2>&1 +rc=$? +if [[ $rc -eq 2 ]]; then + pass "unknown mode exits 2" +else + fail "unknown mode should exit 2 (rc=$rc)" +fi + +if [[ $fails -ne 0 ]]; then + printf '%d assertion(s) failed\n' "$fails" >&2 + exit 1 +fi +printf 'all assertions passed\n' diff --git a/scripts/skill-leaf-name-registry.txt b/scripts/skill-leaf-name-registry.txt new file mode 100644 index 000000000..f8731612f --- /dev/null +++ b/scripts/skill-leaf-name-registry.txt @@ -0,0 +1,53 @@ +# Skill leaf names deliberately carried by more than one plugin. Read by +# check-skill-leaf-names.sh --check: a collision not listed here fails as +# "unregistered" (a new shared leaf name needing a decision); an entry listed +# here that no longer collides fails as stale and must be dropped. +# +# Every one of these is separately invocable -- namespacing guarantees that, and +# the philosophy's Naming section is explicit that a name is never degraded to +# dodge a collision. What a shared leaf name costs is legibility: the picker +# labels the row by this name, so two entries read identically and are told +# apart only by the `()` prefix on the description. That is the +# cost each entry below is accepted at, and the reason each description's first +# clause has to name its object. +# +# One leaf name per line, with the grounds it is accepted on. + +# Contract-mandated. docs/PLUGIN-PHILOSOPHY.md (Setup) fixes the name for every +# plugin that ships consumer configuration; uniformity IS the contract. Not +# consolidatable: ${CLAUDE_PLUGIN_ROOT} resolves per containing plugin and there +# is no cross-plugin skill inheritance, so each plugin owns its own. +setup + +# Fixed verb meaning: read-only findings report (PLUGIN-PHILOSOPHY.md, Naming +# verb table). The namespace supplies the object in every case -- +# claude-config, claude-memory, codebase-health, machine-health, mcp-tools, +# repo-fleet-hygiene. Qualifying the verb per plugin would say the same thing +# twice; a topic qualifier is reserved for siblings WITHIN one namespace +# (audit-noise beside audit-encapsulation). +audit + +# Fixed verb meaning: deterministic pass/fail gate. skill-quality checks a +# skill, toolchain checks a build. +check + +# Fixed verb meaning: mutates the target. disk-hygiene cleans an arbitrary +# directory tree, repo-hygiene cleans a repository's caches and git metadata. +clean + +# D19, kept on merit. songwriting diagnoses a song, testing diagnoses a failing +# suite -- unrelated domains, and the namespace disambiguates. +diagnose + +# D19, kept on merit. planning:plan is the root-echo core action of its domain; +# testing:plan is a test-strategy plan. The bare token still belongs to the +# built-in plan-mode toggle, which neither shadows. +plan + +# D19, kept on merit. session-flow:workflow routes a development session, +# songwriting:workflow routes a writing session. +workflow + +# Fixed verb meaning: produces an artifact. bug-report writes a report, testing +# writes tests. +write From 93241eeaec7f5b0c95233b5985c5a6988afe839a Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 20 Jul 2026 13:38:01 -0400 Subject: [PATCH 2/3] fix(ci): register the owner set, not just the colliding leaf name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Registering a bare name meant the first registration silently pre-authorized every later owner: once `audit` was accepted for six plugins, a seventh could add one and --check stayed green, while the entry's rationale still enumerated the original six. The gate exists to force a conscious decision, and that was the one case it waved through. Entries now carry their owner set and --check fails when it changes, so a new owner is argued on its own grounds. `*` covers a name whose owner set is fixed by contract — `setup`, where re-arguing each new plugin would be ceremony with no decision in it. Two bugs surfaced while verifying, both now covered by tests: unquoted array splitting pathname-expanded the literal `*` against the cwd, and only the discovered side was sorted, so a hand-ordered registry entry failed a comparison documented as set-vs-set. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_011ogVV7z2Heg9ojJ88nqNxh --- scripts/check-skill-leaf-names.sh | 82 ++++++++++++++++++++++---- scripts/check-skill-leaf-names.test.sh | 49 ++++++++++++++- scripts/skill-leaf-name-registry.txt | 54 ++++++++++------- 3 files changed, 149 insertions(+), 36 deletions(-) diff --git a/scripts/check-skill-leaf-names.sh b/scripts/check-skill-leaf-names.sh index d4e95babd..553a786b9 100755 --- a/scripts/check-skill-leaf-names.sh +++ b/scripts/check-skill-leaf-names.sh @@ -6,8 +6,9 @@ # owned by 2+ plugins, with its # owners and registration state # scripts/check-skill-leaf-names.sh --check fail on an UNREGISTERED -# collision, or a registry entry -# that no longer collides +# collision, on a registered one +# whose OWNER SET changed, or on +# an entry that no longer collides # # Namespacing already guarantees every one of these is separately invocable -- # `/disk-hygiene:clean` and `/repo-hygiene:clean` can never resolve to each @@ -25,8 +26,12 @@ # across plugins. # # Registering a leaf name in skill-leaf-name-registry.txt records the grounds it -# was accepted on. An unregistered collision is a decision waiting to be made, -# not yet a violation of anything. +# was accepted on, together with the owner set those grounds were argued over. +# A new plugin joining an already-registered collision has to be argued on its +# own merits, so the owner set is part of the entry rather than the bare name -- +# otherwise the first registration would silently pre-authorize every later one. +# An unregistered collision is a decision waiting to be made, not yet a +# violation of anything. set -euo pipefail cd "$(dirname "${BASH_SOURCE[0]}")/.." @@ -58,6 +63,8 @@ for leaf in "${!leaf_owners[@]}"; do collisions["$leaf"]="${leaf_owners[$leaf]}" done +# leaf name -> accepted owner set: a sorted comma-separated plugin list, or `*` +# for a name whose owner set is open by contract (see the registry header). declare -A registered if [[ -f "$registry" ]]; then while IFS= read -r line; do @@ -65,10 +72,18 @@ if [[ -f "$registry" ]]; then line="${line#"${line%%[![:space:]]*}"}" line="${line%"${line##*[![:space:]]}"}" [[ -z "$line" ]] && continue - registered["$line"]=1 + # `read`, not array splitting: the owner field may be a literal `*`, which + # unquoted word splitting would pathname-expand against the cwd. + read -r leaf_key owner_field _ <<<"$line" + registered["$leaf_key"]="${owner_field:-}" done <"$registry" fi +# Sorted comma-separated owner set, so the comparison is order-independent. +owner_set() { + printf '%s\n' "$@" | sort | paste -sd, - +} + mode="${1:-discover}" case "$mode" in discover | --check) ;; @@ -86,8 +101,21 @@ if [[ "$mode" == "discover" ]]; then for leaf in $(printf '%s\n' "${!collisions[@]}" | sort); do # shellcheck disable=SC2206 owners=(${collisions[$leaf]}) + # shellcheck disable=SC2310 # owner_set only sorts strings; nothing inside can fail + actual="$(owner_set "${owners[@]}")" state="UNREGISTERED" - [[ -n "${registered[$leaf]:-}" ]] && state="registered" + if [[ -n "${registered[$leaf]+set}" ]]; then + accepted="${registered[$leaf]}" + state="OWNERS-CHANGED" + if [[ "$accepted" == "*" ]]; then + state="registered(*)" + elif [[ -n "$accepted" ]]; then + IFS=',' read -ra accepted_owners <<<"$accepted" + # shellcheck disable=SC2310 # owner_set only sorts strings; nothing inside can fail + accepted="$(owner_set "${accepted_owners[@]}")" + [[ "$accepted" == "$actual" ]] && state="registered" + fi + fi printf '%-14s %-14s %d plugins: %s\n' \ "$leaf" "$state" "${#owners[@]}" "$(printf '%s ' "${owners[@]}" | sed 's/ $//')" done @@ -97,14 +125,44 @@ fi failed=0 for leaf in $(printf '%s\n' "${!collisions[@]}" | sort); do - [[ -n "${registered[$leaf]:-}" ]] && continue # shellcheck disable=SC2206 owners=(${collisions[$leaf]}) - printf 'FAIL: skill leaf name %s is now carried by %d plugins (%s) and is not registered.\n' \ - "$leaf" "${#owners[@]}" "$(printf '%s ' "${owners[@]}" | sed 's/ $//')" >&2 - printf ' These are separately invocable, but the picker labels both rows %s.\n' "$leaf" >&2 - printf ' Rename one, or add %s to %s with the grounds it is accepted on.\n' "$leaf" "$registry" >&2 - failed=1 + # shellcheck disable=SC2310 # owner_set only sorts strings; nothing inside can fail + actual="$(owner_set "${owners[@]}")" + + if [[ -z "${registered[$leaf]+set}" ]]; then + printf 'FAIL: skill leaf name %s is now carried by %d plugins (%s) and is not registered.\n' \ + "$leaf" "${#owners[@]}" "${actual//,/ }" >&2 + printf ' These are separately invocable, but the picker labels every row %s.\n' "$leaf" >&2 + printf ' Rename one, or add "%s %s" to %s with the grounds it is accepted on.\n' \ + "$leaf" "$actual" "$registry" >&2 + failed=1 + continue + fi + + accepted="${registered[$leaf]}" + # An open owner set is accepted by contract; only the name is registered. + [[ "$accepted" == "*" ]] && continue + + # Normalize the registry side too, so the comparison is genuinely set-vs-set + # and a hand-edited entry does not fail on ordering alone. + if [[ -n "$accepted" ]]; then + IFS=',' read -ra accepted_owners <<<"$accepted" + # shellcheck disable=SC2310 # owner_set only sorts strings; nothing inside can fail + accepted="$(owner_set "${accepted_owners[@]}")" + fi + + if [[ -z "$accepted" ]]; then + printf 'FAIL: %s registers %s without an owner set. Record it as "%s %s".\n' \ + "$registry" "$leaf" "$leaf" "$actual" >&2 + failed=1 + elif [[ "$accepted" != "$actual" ]]; then + printf 'FAIL: skill leaf name %s is registered for %s but is now carried by %s.\n' \ + "$leaf" "${accepted//,/ }" "${actual//,/ }" >&2 + printf ' A new owner joins an accepted collision on its own grounds, not the old ones.\n' >&2 + printf ' Update the entry to "%s %s" and revisit the rationale above it.\n' "$leaf" "$actual" >&2 + failed=1 + fi done # Stale guard: a registry entry that no longer collides has outlived its reason diff --git a/scripts/check-skill-leaf-names.test.sh b/scripts/check-skill-leaf-names.test.sh index 5caf7da45..270bf1d79 100755 --- a/scripts/check-skill-leaf-names.test.sh +++ b/scripts/check-skill-leaf-names.test.sh @@ -73,9 +73,9 @@ else fail "unregistered collision should fail (rc=$rc): $out" fi -# 5. --check passes once the collision is registered, and comments/blank lines -# in the registry are ignored. -printf '# a comment\n\n shared # trailing comment\n' >"$REGISTRY" +# 5. --check passes once the collision is registered with its owner set, and +# comments/blank lines in the registry are ignored. +printf '# a comment\n\n shared alpha,beta,gamma # trailing comment\n' >"$REGISTRY" out="$(run --check)" rc=$? if [[ $rc -eq 0 ]] && grep -q 'are registered' <<<"$out"; then @@ -84,6 +84,49 @@ else fail "registered collision should pass (rc=$rc): $out" fi +# 5b. Owner order in the registry does not matter — sets are compared, not lists. +printf 'shared gamma,alpha,beta\n' >"$REGISTRY" +out="$(run --check)" +rc=$? +if [[ $rc -eq 0 ]]; then + pass "owner set comparison is order-independent" +else + fail "reordered owner set should pass (rc=$rc): $out" +fi + +# 5c. A registered name whose owner set GREW fails — the first registration must +# not pre-authorize later owners joining on the original grounds. +printf 'shared alpha,beta\n' >"$REGISTRY" +out="$(run --check)" +rc=$? +if [[ $rc -eq 1 ]] && grep -q 'is registered for' <<<"$out"; then + pass "a new owner joining a registered collision fails" +else + fail "grown owner set should fail (rc=$rc): $out" +fi + +# 5d. A bare name with no owner field is rejected, so the old format cannot +# silently keep passing as an open registration. +printf 'shared\n' >"$REGISTRY" +out="$(run --check)" +rc=$? +if [[ $rc -eq 1 ]] && grep -q 'without an owner set' <<<"$out"; then + pass "registration without an owner set is rejected" +else + fail "bare-name entry should fail (rc=$rc): $out" +fi + +# 5e. `*` accepts any owner set — and must reach the comparison as a literal +# rather than being pathname-expanded against the cwd. +printf 'shared *\n' >"$REGISTRY" +out="$(run --check)" +rc=$? +if [[ $rc -eq 0 ]] && grep -q 'are registered' <<<"$out"; then + pass "wildcard owner set accepts any owners (literal, not glob-expanded)" +else + fail "wildcard entry should pass (rc=$rc): $out" +fi + # 6. Stale guard: a registry entry that no longer collides fails. printf 'shared\nvanished\n' >"$REGISTRY" out="$(run --check)" diff --git a/scripts/skill-leaf-name-registry.txt b/scripts/skill-leaf-name-registry.txt index f8731612f..756f2901f 100644 --- a/scripts/skill-leaf-name-registry.txt +++ b/scripts/skill-leaf-name-registry.txt @@ -1,53 +1,65 @@ # Skill leaf names deliberately carried by more than one plugin. Read by -# check-skill-leaf-names.sh --check: a collision not listed here fails as -# "unregistered" (a new shared leaf name needing a decision); an entry listed -# here that no longer collides fails as stale and must be dropped. +# check-skill-leaf-names.sh --check. +# +# Format, one entry per line: +# +# ,,... accepted for exactly this owner set +# * owner set is open by contract +# +# The owner set is part of the entry, not decoration. A new plugin joining an +# already-accepted collision changes the collision being accepted, and has to be +# argued on its own merits -- registering the bare name would silently +# pre-authorize every later owner. Owners are sorted; the check compares sets, +# not order. `*` is reserved for a name whose owner set is fixed by a contract +# that already decided the question, where re-arguing each new owner would be +# ceremony with no decision in it. +# +# --check fails three ways: an unregistered collision, a registered one whose +# owner set has changed, and an entry that no longer collides at all (which has +# outlived its reason and would otherwise pre-authorize a future name). # # Every one of these is separately invocable -- namespacing guarantees that, and # the philosophy's Naming section is explicit that a name is never degraded to # dodge a collision. What a shared leaf name costs is legibility: the picker -# labels the row by this name, so two entries read identically and are told +# labels the row by this name, so the entries read identically and are told # apart only by the `()` prefix on the description. That is the # cost each entry below is accepted at, and the reason each description's first # clause has to name its object. -# -# One leaf name per line, with the grounds it is accepted on. -# Contract-mandated. docs/PLUGIN-PHILOSOPHY.md (Setup) fixes the name for every -# plugin that ships consumer configuration; uniformity IS the contract. Not +# Contract-mandated, open owner set. docs/PLUGIN-PHILOSOPHY.md (Setup) fixes the +# name for every plugin that ships consumer configuration; uniformity IS the +# contract, so a new plugin adopting it is conformance, not a new decision. Not # consolidatable: ${CLAUDE_PLUGIN_ROOT} resolves per containing plugin and there # is no cross-plugin skill inheritance, so each plugin owns its own. -setup +setup * # Fixed verb meaning: read-only findings report (PLUGIN-PHILOSOPHY.md, Naming -# verb table). The namespace supplies the object in every case -- -# claude-config, claude-memory, codebase-health, machine-health, mcp-tools, -# repo-fleet-hygiene. Qualifying the verb per plugin would say the same thing -# twice; a topic qualifier is reserved for siblings WITHIN one namespace -# (audit-noise beside audit-encapsulation). -audit +# verb table). The namespace supplies the object in every case. Qualifying the +# verb per plugin would say the same thing twice; a topic qualifier is reserved +# for siblings WITHIN one namespace (audit-noise beside audit-encapsulation). +audit claude-config,claude-memory,codebase-health,machine-health,mcp-tools,repo-fleet-hygiene # Fixed verb meaning: deterministic pass/fail gate. skill-quality checks a # skill, toolchain checks a build. -check +check skill-quality,toolchain # Fixed verb meaning: mutates the target. disk-hygiene cleans an arbitrary # directory tree, repo-hygiene cleans a repository's caches and git metadata. -clean +clean disk-hygiene,repo-hygiene # D19, kept on merit. songwriting diagnoses a song, testing diagnoses a failing # suite -- unrelated domains, and the namespace disambiguates. -diagnose +diagnose songwriting,testing # D19, kept on merit. planning:plan is the root-echo core action of its domain; # testing:plan is a test-strategy plan. The bare token still belongs to the # built-in plan-mode toggle, which neither shadows. -plan +plan planning,testing # D19, kept on merit. session-flow:workflow routes a development session, # songwriting:workflow routes a writing session. -workflow +workflow session-flow,songwriting # Fixed verb meaning: produces an artifact. bug-report writes a report, testing # writes tests. -write +write bug-report,testing From 59ee8db20f2d747d465f8bb626484ded1d834114 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 20 Jul 2026 13:46:10 -0400 Subject: [PATCH 3/3] fix(ci): survive the zero-collision end state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under `set -u` an associative array that was declared but never assigned is unbound, so `${#collisions[@]}` and `${!collisions[@]}` aborted with "collisions: unbound variable" the moment the last cross-plugin collision was removed. Reproduced with a single unique fixture skill and an empty registry. That is exactly the state the stale-entry guard exists to shepherd the repo into, so it was the one outcome the gate could not express — a clean tree read as a crash. Tracks a plain counter and an indexed leaf list alongside the map, and guards the remaining expansions. Both zero-collision paths are now covered by tests. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_011ogVV7z2Heg9ojJ88nqNxh --- scripts/check-skill-leaf-names.sh | 22 ++++++++++++++++----- scripts/check-skill-leaf-names.test.sh | 27 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/scripts/check-skill-leaf-names.sh b/scripts/check-skill-leaf-names.sh index 553a786b9..72847a901 100755 --- a/scripts/check-skill-leaf-names.sh +++ b/scripts/check-skill-leaf-names.sh @@ -55,17 +55,28 @@ for plugin_dir in plugins/*/; do done # Collisions only: a leaf name owned by 2+ plugins. +# +# `collision_leaves` and `collision_count` shadow the associative array on +# purpose: under `set -u` an associative array that was declared but never +# assigned is UNBOUND, so `${#collisions[@]}` and `${!collisions[@]}` abort the +# script in the zero-collision case -- which is exactly the state the stale-entry +# guard is meant to shepherd the repo into. declare -A collisions +collision_leaves=() +collision_count=0 for leaf in "${!leaf_owners[@]}"; do # shellcheck disable=SC2206 # plugin names are kebab-case; word-splitting is the intent owners=(${leaf_owners[$leaf]}) ((${#owners[@]} >= 2)) || continue collisions["$leaf"]="${leaf_owners[$leaf]}" + collision_leaves+=("$leaf") + collision_count=$((collision_count + 1)) done # leaf name -> accepted owner set: a sorted comma-separated plugin list, or `*` # for a name whose owner set is open by contract (see the registry header). declare -A registered +registered_leaves=() if [[ -f "$registry" ]]; then while IFS= read -r line; do line="${line%%#*}" @@ -75,6 +86,7 @@ if [[ -f "$registry" ]]; then # `read`, not array splitting: the owner field may be a literal `*`, which # unquoted word splitting would pathname-expand against the cwd. read -r leaf_key owner_field _ <<<"$line" + [[ -n "${registered[$leaf_key]+set}" ]] || registered_leaves+=("$leaf_key") registered["$leaf_key"]="${owner_field:-}" done <"$registry" fi @@ -94,11 +106,11 @@ discover | --check) ;; esac if [[ "$mode" == "discover" ]]; then - if ((${#collisions[@]} == 0)); then + if ((collision_count == 0)); then echo "No cross-plugin skill leaf-name collisions." exit 0 fi - for leaf in $(printf '%s\n' "${!collisions[@]}" | sort); do + for leaf in $(printf '%s\n' ${collision_leaves[@]+"${collision_leaves[@]}"} | sort); do # shellcheck disable=SC2206 owners=(${collisions[$leaf]}) # shellcheck disable=SC2310 # owner_set only sorts strings; nothing inside can fail @@ -124,7 +136,7 @@ fi failed=0 -for leaf in $(printf '%s\n' "${!collisions[@]}" | sort); do +for leaf in $(printf '%s\n' ${collision_leaves[@]+"${collision_leaves[@]}"} | sort); do # shellcheck disable=SC2206 owners=(${collisions[$leaf]}) # shellcheck disable=SC2310 # owner_set only sorts strings; nothing inside can fail @@ -167,7 +179,7 @@ done # Stale guard: a registry entry that no longer collides has outlived its reason # and would otherwise silently pre-authorize a future collision on that name. -for leaf in $(printf '%s\n' "${!registered[@]}" | sort); do +for leaf in $(printf '%s\n' ${registered_leaves[@]+"${registered_leaves[@]}"} | sort); do [[ -n "${collisions[$leaf]:-}" ]] && continue printf 'FAIL: %s lists %s, but it is no longer carried by 2+ plugins. Drop the entry.\n' \ "$registry" "$leaf" >&2 @@ -178,4 +190,4 @@ if ((failed)); then exit 1 fi -printf 'All %d cross-plugin skill leaf-name collisions are registered.\n' "${#collisions[@]}" +printf 'All %d cross-plugin skill leaf-name collisions are registered.\n' "$collision_count" diff --git a/scripts/check-skill-leaf-names.test.sh b/scripts/check-skill-leaf-names.test.sh index 270bf1d79..3d4f5bbf8 100755 --- a/scripts/check-skill-leaf-names.test.sh +++ b/scripts/check-skill-leaf-names.test.sh @@ -150,6 +150,33 @@ else fail "new collision should fail (rc=$rc): $out" fi +# 7b. The zero-collision end state succeeds. Under `set -u` an associative array +# declared but never assigned is unbound, so a bare ${#a[@]}/${!a[@]} aborts +# here — and this is precisely the state the stale-entry guard exists to +# shepherd the repo into, so it must not be the one state that crashes. +CLEAN="$(mktemp -d)" +mkdir -p "$CLEAN/scripts" "$CLEAN/plugins/solo/skills/only" +cp "$SUT_SRC" "$CLEAN/scripts/check-skill-leaf-names.sh" +printf -- '---\nname: only\ndescription: "fixture"\n---\n' >"$CLEAN/plugins/solo/skills/only/SKILL.md" +: >"$CLEAN/scripts/skill-leaf-name-registry.txt" + +out="$(bash "$CLEAN/scripts/check-skill-leaf-names.sh" --check 2>&1)" +rc=$? +if [[ $rc -eq 0 ]] && ! grep -q 'unbound variable' <<<"$out"; then + pass "zero collisions with an empty registry passes --check" +else + fail "zero-collision state should pass (rc=$rc): $out" +fi + +out="$(bash "$CLEAN/scripts/check-skill-leaf-names.sh" 2>&1)" +rc=$? +if [[ $rc -eq 0 ]] && grep -q 'No cross-plugin skill leaf-name collisions' <<<"$out"; then + pass "zero collisions reports cleanly in discover mode" +else + fail "zero-collision discover should report cleanly (rc=$rc): $out" +fi +rm -rf "$CLEAN" + # 8. Unknown mode is a usage error, not a silent pass. bash "$SUT" --bogus >/dev/null 2>&1 rc=$?