CM-71972: Collect Claude Code skills in the Guardrails session sweep - #538
Open
Altruistus wants to merge 4 commits into
Open
CM-71972: Collect Claude Code skills in the Guardrails session sweep#538Altruistus wants to merge 4 commits into
Altruistus wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Altruistus
force-pushed
the
CM-71972-be-implement-skills-gathering-using-cli
branch
2 times, most recently
from
September 3, 2026 08:52
c189654 to
433bbfb
Compare
Skills installed from a marketplace land in ~/.claude/skills and are never committed, so the inventory has never seen them. The session-start hook already sweeps every IDE for MCP configs; this adds skills to the same sweep and the same report. Skills get their own collector rather than riding in config_files[]: the backend parses every entry there as an MCP server map, and a skill is a directory of Markdown, not a normalizable JSON config. Plugin skills are the exception - they hang off the plugin entry, because that is what carries the marketplace, plugin and version a marketplace-installed skill came from, and _read_claude_plugin already holds the resolved directory. Raw SKILL.md content is sent rather than parsed frontmatter. Parsing belongs to the backend, which is also the only option for the device connectors that will read these files off endpoints and can return nothing but raw content. Two caps, neither of which the MCP collectors needed: a SKILL.md body is unbounded prose and the number of installed skills is unbounded too. One request carries the whole session context, so an oversized skill would otherwise cost the device its MCP inventory as well. The session-context tests grew an autouse fixture pinning the sweep to empty. The MCP collectors are stubbed per IDE, but the skills sweep walks the filesystem, so without it every assertion in that file would depend on whoever ran it - which is exactly what the first run did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A plugin's skills live at <plugin_dir>/skills/<name>/SKILL.md, which is a property of the plugin format rather than of any one IDE - Codex and Copilot plugins use the same layout, and all three readers already hold the resolved plugin_dir. So the scan moved into _skill_utils and all three call it: a plugin shipping skills is now inventoried whichever IDE loaded it. User-scope collection stays Claude Code only, because that is the only tool with a SKILL.md convention of its own today - the SCM side agrees, it classifies .claude/skills/*/SKILL.md as category Skill while Cursor's .mdc rules and CLAUDE.md are category Rule. The hook is generic: IDE.get_skills defaults to [] and any IDE opts in by overriding it, so adding one is two lines once its skills directory is confirmed rather than guessed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI caught a real bug. The skills directory was a module-level constant, so Path.home() was evaluated when the module was imported - pinning it to whatever home the process started with. A test filesystem that redirects home could then never be seen, which is why the collection test passed on my machine and on some runners and failed on others: it depended on whether the fake home happened to match the real one. Every other home-relative directory in this module is already a function for exactly this reason, so this follows _plugins_cache_dir. The regression test patches home and asserts the directory follows; it fails against the old constant, so the bug cannot come back silently. Also fixes the lint failure: a lambda returning an empty list is just list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
staticmethod and classmethod objects are not reliably callable when set as an attribute on pyfakefs's fake Path - it does not unwrap the descriptor the same way on every Python version, which is why the test passed on 3.11, 3.12 and 3.14 and failed on 3.9, 3.10 and 3.13. A plain function has no descriptor to unwrap and is correct in plain pathlib too, verified on both interpreters available here. My local venv is 3.14, which is exactly why the first attempt looked fine locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Altruistus
force-pushed
the
CM-71972-be-implement-skills-gathering-using-cli
branch
from
September 3, 2026 09:25
433bbfb to
d9c1d44
Compare
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
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.
TL;DR
A developer installs a skill from a marketplace into
~/.claude/skills/, never commits it, and Cycode's Skills inventory shows nothing. The session-start hook already sweeps every IDE for MCP configs; this adds skills to the same sweep and the same report, so an installed skill becomes visible without anyone having to commit it.Why
Skills inventory has only ever seen a
SKILL.mdthat reached a repository. The instructions and scripts a marketplace skill carries have been invisible — worst of all for people with no SCM footprint at all. The backend can already inventory a reported skill; it just needs the CLI to report one.Reviewer notes
config_files[]. The backend parses every entry there as an MCP server map, and a skill is a directory of Markdown, not a normalizable JSON config.skills/layout belongs to the plugin format, not to any one IDE.SKILL.mdbody is unbounded prose and the number of installed skills is unbounded too. One request carries the whole session context, so an oversized skill would otherwise cost the device its MCP inventory as well.SKILL.mdconvention of its own today — Cursor's.mdcrules andCLAUDE.mdare rule files, a different category. The hook is generic (IDE.get_skillsdefaults to empty), so adding another IDE is two lines once its skills directory is confirmed rather than guessed.~/.claude/skills, so their assertions depended on whose machine ran them. An autouse fixture now pins the sweep.<repo>/.claude/skills/) are out of scope: there is no repo-scope collection in this app at all, which is structural plumbing rather than a missing path.CM-71972