Skip to content

CM-71972: Collect Claude Code skills in the Guardrails session sweep - #538

Open
Altruistus wants to merge 4 commits into
mainfrom
CM-71972-be-implement-skills-gathering-using-cli
Open

CM-71972: Collect Claude Code skills in the Guardrails session sweep#538
Altruistus wants to merge 4 commits into
mainfrom
CM-71972-be-implement-skills-gathering-using-cli

Conversation

@Altruistus

@Altruistus Altruistus commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

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.md that 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

  • 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 and hang off the plugin entry, because that is what carries the marketplace, plugin and version a marketplace-installed skill came from. The plugin readers already hold the resolved directory, so the scan is shared and all three IDEs with a plugin system get it — the skills/ layout belongs to the plugin format, not to any one IDE.
  • Raw content is sent, not parsed frontmatter. Parsing belongs to the backend, which is also the only option for the device connectors that read these files off endpoints and can return nothing but file bytes.
  • Two caps that the MCP collectors did not need. 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.
  • User-scope collection is Claude Code only, deliberately. It is the only tool with a SKILL.md convention of its own today — Cursor's .mdc rules and CLAUDE.md are rule files, a different category. The hook is generic (IDE.get_skills defaults to empty), so adding another IDE is two lines once its skills directory is confirmed rather than guessed.
  • A test-isolation bug surfaced on the first run and is fixed here. The existing session-context tests read the developer's real ~/.claude/skills, so their assertions depended on whose machine ran them. An autouse fixture now pins the sweep.
  • Project-scope skills (<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

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@omer-roth
omer-roth requested a review from Ilanlido September 2, 2026 12:52
@Altruistus
Altruistus force-pushed the CM-71972-be-implement-skills-gathering-using-cli branch 2 times, most recently from c189654 to 433bbfb Compare September 3, 2026 08:52
Altruistus and others added 4 commits September 3, 2026 11:25
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
Altruistus force-pushed the CM-71972-be-implement-skills-gathering-using-cli branch from 433bbfb to d9c1d44 Compare September 3, 2026 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant