CM-71972: Collect Claude Code skills in the Guardrails session sweep - #538
Merged
Altruistus merged 6 commits intoSep 7, 2026
Merged
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
3 times, most recently
from
September 3, 2026 09:25
433bbfb to
d9c1d44
Compare
Ilanlido
reviewed
Sep 5, 2026
Altruistus
force-pushed
the
CM-71972-be-implement-skills-gathering-using-cli
branch
from
September 7, 2026 11:01
7cd0014 to
0cd36d2
Compare
Ilanlido
approved these changes
Sep 7, 2026
Altruistus
enabled auto-merge (squash)
September 7, 2026 13:06
Altruistus
force-pushed
the
CM-71972-be-implement-skills-gathering-using-cli
branch
from
September 7, 2026 13:09
0cd36d2 to
39f9653
Compare
Altruistus
disabled auto-merge
September 7, 2026 13:12
omer-roth
approved these changes
Sep 7, 2026
Altruistus
enabled auto-merge (squash)
September 7, 2026 15:16
Altruistus
force-pushed
the
CM-71972-be-implement-skills-gathering-using-cli
branch
from
September 7, 2026 15:16
39f9653 to
224159f
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>
Codex, Copilot and Cursor each get get_skills() reading their own skills directory, honoring $CODEX_HOME and $COPILOT_HOME the way the rest of those integrations do. collect_all_skills already sweeps every registered IDE and dedupes by path, so nothing else changes and an IDE with no skills directory simply contributes none. The session-start tests now isolate the home directory instead of stubbing collect_all_skills, so the two skill tests exercise the real sweep against files they create rather than asserting against a mock. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An underscore prefix marks a fixture as side-effect-only, and ruff's PT019 asks for usefixtures in that case. This one returns the scratch home the skill tests write into, so the value is used and the name should say so. 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 7, 2026 15:17
224159f to
1e1b7ca
Compare
Altruistus
deleted the
CM-71972-be-implement-skills-gathering-using-cli
branch
September 7, 2026 15:18
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