fix(list-skills): discover every installed skill, drop the PyYAML import - #1187
Merged
Conversation
`list-skills` resolved its skill set from `Path(__file__)`, so under a per-family marketplace plugin install it listed only the family that happens to ship it — 5 skills out of 74. It also derived each skill's family from the name prefix, against Golden rule 8, which invented families like `write/` and `optimize/` for skills declaring `utilities` and split `repo-health` and `contributor-growth` across headings. Discovery is now installation-aware and repository-first: the agent target directories an install writes into, the framework's own `skills/` when the repository is the framework checkout, and the sibling plugins in the marketplace cache when the script runs from one. Rows de-duplicate by the name a person would type, so relays collapse while a skill reachable from two install methods keeps both. Grouping reads the `family:` frontmatter key; a skill declaring none lands in `other`. The script was also the only one under `skills/` importing a third-party module, against the stdlib-only contract in `skills/pyproject.toml`, and crashed with `ModuleNotFoundError: No module named 'yaml'` on any interpreter without PyYAML — including every plugin install. Frontmatter is now parsed with the standard library, as `collect_status.py` and the validator already do, and PEP 723 inline metadata states the contract so `uv run --script` and a bare `python3` behave identically. Generated-by: Claude Code (Opus 5)
The previous commit rewrote Step 1 to use the `<framework>` placeholder. Models resolve that to an absolute path, which is machine-specific and cannot be pinned in an eval fixture, so step-1-command cases 1-4 failed: expected `<framework>/skills/…`, got `/Users/…/skills/…`. Step 1 goes back to the literal repository-relative `.claude/skills/magpie-list-skills/scripts/list_skills.py`, with an explicit instruction not to expand it, and the four fixtures return to their upstream values — they never needed to change. The marketplace exception moves below the primary command and keeps its own case, which passed throughout. Generated-by: Claude Code (Opus 5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
list-skillsresolved its skill set fromPath(__file__), so under a per-family marketplace plugin install it listed only the family that ships it — 5 skills out of 74. Discovery is now installation-aware and repository-first.write/andoptimize/for skills declaringutilities, and splittingrepo-healthandcontributor-growthacross headings. Grouping now reads thefamily:frontmatter key.skills/importing a third-party module, against the stdlib-only contract inskills/pyproject.toml, and crashed withModuleNotFoundError: No module named 'yaml'on any interpreter without PyYAML — including every plugin install. Frontmatter is now parsed with the standard library, and PEP 723 inline metadata states the contract.Discovery
Three sources, unioned and de-duplicated by the name a person would type:
.agents/skills/+ relays (pinned snapshot)/magpie-<skill>skills/(framework checkout)/magpie-<skill>/<plugin>:<skill>Relays collapse to one row; a skill reachable from two install methods keeps both, because they are two different things to type. Root comes from
git rev-parse --show-toplevel, with--rootto override. A skill declaring nofamily:lands inother.Type of change
skills/list-skills/) — eval fixtures updated belowTest plan
prek run --all-filespassesruff check/ruff format/mypy/pytestpass for theskillsworkspace memberskills/list-skills/tests/test_list_skills.py— 17 cases, including the marketplace regression, relay collapse, declared-vs-inferred family, and "a project with its own unrelatedskills/dir is not the framework checkout"list-skills; 8/8 passcase-5-marketplace-installpins the one install where the documented command does not applyRFC-AI-0004 compliance
skills/setup/agents.md; no vendor is special-casedNotes for reviewers
Two judgement calls worth a look:
dependencies = [], notdependencies = ["pyyaml"]. Declaring PyYAML would have fixed the crash foruv runcallers only and leftskills/pyproject.toml's stdlib-only guarantee broken for a barepython3. The stdlib parser matches whatcollect_status.pyandskill-and-tool-validatoralready do.<framework>/skills/list-skills/…; models expand that placeholder to an absolute path, which cannot be pinned in a fixture, and cases 1–4 failed. Second commit reverts those four fixtures to their upstream values — they never needed to change.A sweep found no other script with either bug: nothing else under
skills/derives its working set from__file__or infers family from a prefix.Generated-by: Claude Code (Opus 5)