You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2026/09/09 10:30:15 UTC

[PR] fix(list-skills): discover every installed skill, drop the PyYAML import [magpie]

potiuk opened a new pull request, #1187:
URL: https://github.com/apache/magpie/pull/1187

   ## Summary
   
   - `list-skills` resolved its skill set from `Path(__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.
   - It derived each skill's family from the name prefix, against Golden rule 8 — inventing families like `write/` and `optimize/` for skills declaring `utilities`, and splitting `repo-health` and `contributor-growth` across headings. Grouping now reads the `family:` frontmatter key.
   - It was the only script 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, and PEP 723 inline metadata states the contract.
   
   ## Discovery
   
   Three sources, unioned and de-duplicated by the name a person would type:
   
   | Source | Invocation shown |
   |---|---|
   | `.agents/skills/` + relays (pinned snapshot) | `/magpie-<skill>` |
   | the repo's own `skills/` (framework checkout) | `/magpie-<skill>` |
   | sibling plugins in the marketplace cache | `/<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 `--root` to override. A skill declaring no `family:` lands in `other`.
   
   ## Type of change
   
   - [x] Skill change (`skills/list-skills/`) — eval fixtures updated below
   - [x] CI / dev loop (spec + eval fixtures)
   
   ## Test plan
   
   - [x] `prek run --all-files` passes
   - [x] `ruff check` / `ruff format` / `mypy` / `pytest` pass for the `skills` workspace member
   - [x] New `skills/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 unrelated `skills/` dir is not the framework checkout"
   - [x] Eval suite run for `list-skills`; 8/8 pass
   - [x] New eval case `case-5-marketplace-install` pins the one install where the documented command does not apply
   - [x] Measured end to end: **74** in the framework checkout, **10** in an adopter, **47** from a marketplace install against a real plugin cache (was 5)
   
   ## RFC-AI-0004 compliance
   
   - [x] **Sandbox** — read-only; no new host access
   - [x] **Vendor neutrality** — the agent-target list mirrors the registry in `skills/setup/agents.md`; no vendor is special-cased
   
   ## Notes for reviewers
   
   Two judgement calls worth a look:
   
   1. **PEP 723 with `dependencies = []`, not `dependencies = ["pyyaml"]`.** Declaring PyYAML would have fixed the crash for `uv run` callers only and left `skills/pyproject.toml`'s stdlib-only guarantee broken for a bare `python3`. The stdlib parser matches what `collect_status.py` and `skill-and-tool-validator` already do.
   2. **Step 1's command stays literal.** I first rewrote it to `<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)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@magpie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(list-skills): discover every installed skill, drop the PyYAML import [magpie]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #1187:
URL: https://github.com/apache/magpie/pull/1187


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@magpie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org