From fcc2d6f2606de90b76e0b0000a4ed63925285d48 Mon Sep 17 00:00:00 2001 From: Nikita Fishbakh Date: Wed, 2 Sep 2026 12:21:46 +0200 Subject: [PATCH 1/4] CM-71972: Collect Claude Code skills in the Guardrails session sweep 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) --- .../cli/apps/ai_guardrails/ides/__init__.py | 16 +++ .../apps/ai_guardrails/ides/_skill_utils.py | 90 +++++++++++++++ cycode/cli/apps/ai_guardrails/ides/base.py | 14 +++ .../apps/ai_guardrails/ides/claude_code.py | 19 +++- .../ai_guardrails/session_start_command.py | 15 ++- cycode/cyclient/ai_security_manager_client.py | 2 + .../ai_guardrails/ides/test_claude_code.py | 63 +++++++++++ .../ai_guardrails/ides/test_contract.py | 10 ++ .../ai_guardrails/ides/test_skill_utils.py | 94 ++++++++++++++++ .../test_session_start_command.py | 105 ++++++++++++++++++ 10 files changed, 423 insertions(+), 5 deletions(-) create mode 100644 cycode/cli/apps/ai_guardrails/ides/_skill_utils.py create mode 100644 tests/cli/commands/ai_guardrails/ides/test_skill_utils.py diff --git a/cycode/cli/apps/ai_guardrails/ides/__init__.py b/cycode/cli/apps/ai_guardrails/ides/__init__.py index 396074da..bc48448a 100644 --- a/cycode/cli/apps/ai_guardrails/ides/__init__.py +++ b/cycode/cli/apps/ai_guardrails/ides/__init__.py @@ -54,6 +54,22 @@ def collect_all_session_contexts() -> tuple[dict[str, dict], dict]: return config_files_by_ide, plugins +def collect_all_skills() -> list[dict]: + """Sweep every registered IDE's user-scope skills, regardless of which IDE triggered the hook. + + Returns ``[{"path", "content"}]`` deduplicated by path and sorted, so two IDEs sharing a skills + directory report it once and the session-context digest stays stable across registry order. + Skills a plugin ships are not here - those ride on their plugin entry, which carries the + marketplace provenance. + """ + skills_by_path: dict[str, dict] = {} + for ide in IDES.values(): + for skill in ide.get_skills(): + skills_by_path.setdefault(skill['path'], skill) + + return [skills_by_path[path] for path in sorted(skills_by_path)] + + def resolve_ides(name: str) -> list[IDE]: """Resolve an ``--ide`` argument to one or all IDE instances. diff --git a/cycode/cli/apps/ai_guardrails/ides/_skill_utils.py b/cycode/cli/apps/ai_guardrails/ides/_skill_utils.py new file mode 100644 index 00000000..5254d38e --- /dev/null +++ b/cycode/cli/apps/ai_guardrails/ides/_skill_utils.py @@ -0,0 +1,90 @@ +"""Shared skill-collection helpers for IDE integrations. + +A skill is a directory holding a ``SKILL.md``: ``//SKILL.md``. +The same layout is used for user-scope skills (``~/.claude/skills/``) and for the skills a +plugin ships (``/skills/``), so one walker serves both. + +Unlike an MCP config - a small JSON file at a known path - a ``SKILL.md`` body is unbounded +prose, and the number of installed skills is unbounded too. Both are capped here rather than +downstream: the whole session-context report is one request, so an oversized skill would cost +the device its MCP inventory as well. +""" + +from pathlib import Path +from typing import Optional + +from cycode.logger import get_logger + +logger = get_logger('AI Guardrails Skills') + +SKILL_FILE_NAME = 'SKILL.md' + +# A skill is instructions, not data. Anything larger is not a skill we can usefully inventory, +# and sending it would push the one-request report toward the API's body limit. +MAX_SKILL_FILE_BYTES = 256 * 1024 + +# Per skills root, not per device: a developer with more installed skills than this in one place +# is an outlier we would rather truncate than let define the payload size. +MAX_SKILLS_PER_ROOT = 200 + + +def _read_skill_file(skill_file: Path) -> Optional[dict]: + """Read one ``SKILL.md`` into the session-context file shape, or None if unusable.""" + try: + size = skill_file.stat().st_size + except OSError as e: + logger.debug('Failed to stat skill file, %s', {'path': str(skill_file)}, exc_info=e) + return None + + if size > MAX_SKILL_FILE_BYTES: + logger.debug( + 'Skill file exceeds the size cap; skipping, %s', + {'path': str(skill_file), 'size': size, 'cap': MAX_SKILL_FILE_BYTES}, + ) + return None + + try: + content = skill_file.read_text(encoding='utf-8') + except Exception as e: + logger.debug('Failed to read skill file, %s', {'path': str(skill_file)}, exc_info=e) + return None + + if not content.strip(): + return None + + return {'path': str(skill_file), 'content': content} + + +def walk_skill_dirs(skills_root: Path) -> list[dict]: + """Collect every ``//SKILL.md`` as ``{"path", "content"}``. + + Exactly one directory level is scanned. A skill directory may hold nested references and + scripts, but its ``SKILL.md`` always sits at the top of it, so there is nothing to recurse + into - which is also what keeps this bounded without a depth cap. + + Results are sorted by path: the session-context report is deduplicated by hashing the whole + payload, so an unstable order would re-send an unchanged inventory. + """ + if not skills_root.is_dir(): + return [] + + try: + skill_dirs = sorted(d for d in skills_root.iterdir() if d.is_dir()) + except OSError as e: + logger.debug('Failed to list skills root, %s', {'path': str(skills_root)}, exc_info=e) + return [] + + skills: list[dict] = [] + for skill_dir in skill_dirs: + if len(skills) >= MAX_SKILLS_PER_ROOT: + logger.debug( + 'Skills root exceeds the count cap; truncating, %s', + {'path': str(skills_root), 'cap': MAX_SKILLS_PER_ROOT}, + ) + break + + skill = _read_skill_file(skill_dir / SKILL_FILE_NAME) + if skill: + skills.append(skill) + + return skills diff --git a/cycode/cli/apps/ai_guardrails/ides/base.py b/cycode/cli/apps/ai_guardrails/ides/base.py index 38b16def..331ffe41 100644 --- a/cycode/cli/apps/ai_guardrails/ides/base.py +++ b/cycode/cli/apps/ai_guardrails/ides/base.py @@ -189,3 +189,17 @@ def get_session_context(self) -> tuple[Optional[dict], dict]: Override to surface MCP/plugin inventory. """ return None, {} + + def get_skills(self) -> list[dict]: + """Return the IDE's user-scope skills as ``[{"path", "content"}]``. + + A skill is a ``SKILL.md`` under a per-skill directory. Raw content is returned rather + than parsed frontmatter: the backend owns parsing, because the device connectors that + read these files off endpoints can only ever return raw content. + + Kept separate from ``get_session_context`` rather than folded into its + ``global_config_file`` slot, which is normalized to an MCP server map. + + Default: ``[]`` (the IDE has no skill system). Override to surface skills. + """ + return [] diff --git a/cycode/cli/apps/ai_guardrails/ides/claude_code.py b/cycode/cli/apps/ai_guardrails/ides/claude_code.py index ba2d04fa..7a9ea7b9 100644 --- a/cycode/cli/apps/ai_guardrails/ides/claude_code.py +++ b/cycode/cli/apps/ai_guardrails/ides/claude_code.py @@ -13,6 +13,7 @@ resolve_cached_plugin_dir, walk_enabled_plugins, ) +from cycode.cli.apps.ai_guardrails.ides._skill_utils import walk_skill_dirs from cycode.cli.apps.ai_guardrails.ides.base import IDE, DecisionAction, HookDecision from cycode.cli.apps.ai_guardrails.scan.payload import AIHookPayload from cycode.cli.apps.ai_guardrails.scan.types import AiHookEventType @@ -33,6 +34,10 @@ _CLAUDE_CONFIG_PATH = Path.home() / '.claude.json' _CLAUDE_SETTINGS_PATH = Path.home() / '.claude' / 'settings.json' +_CLAUDE_SKILLS_DIR = Path.home() / '.claude' / 'skills' + +# Claude hardcodes a plugin's skills under this subdirectory, the same way it hardcodes ".mcp.json". +_PLUGIN_SKILLS_SUBDIR = 'skills' _SCAN_COMMAND = f'{CYCODE_SCAN_PROMPT_COMMAND} --ide claude-code' _SESSION_START_COMMAND = f'{CYCODE_SESSION_START_COMMAND} --ide claude-code' @@ -187,10 +192,11 @@ def _resolve_marketplace_path(marketplace: dict) -> Optional[Path]: def _read_claude_plugin(plugin_dir: Path) -> tuple[dict, dict]: - """Read one Claude Code plugin's manifest + MCP servers. + """Read one Claude Code plugin's manifest, MCP servers and skills. Claude hardcodes the MCP file at ``/.mcp.json`` and always - wraps it as ``{"mcpServers": {...}}``. + wraps it as ``{"mcpServers": {...}}``, and a plugin's skills at + ``/skills//SKILL.md``. """ manifest = load_plugin_json(plugin_dir / '.claude-plugin' / 'plugin.json') or {} entry: dict = {} @@ -198,6 +204,12 @@ def _read_claude_plugin(plugin_dir: Path) -> tuple[dict, dict]: if field in manifest: entry[field] = manifest[field] + # Attached to the plugin entry rather than the top-level skills list so the backend keeps the + # plugin provenance (marketplace, plugin, version) that a marketplace-installed skill has. + skill_files = walk_skill_dirs(plugin_dir / _PLUGIN_SKILLS_SUBDIR) + if skill_files: + entry['skill_files'] = skill_files + mcp_config_path = plugin_dir / '.mcp.json' mcp_config = load_plugin_json(mcp_config_path) or {} servers: dict = mcp_config.get('mcpServers') or {} @@ -387,3 +399,6 @@ def get_session_context(self) -> tuple[Optional[dict], dict]: enriched_plugins = resolve_plugins(settings) if settings else {} return global_config_file, enriched_plugins + + def get_skills(self) -> list[dict]: + return walk_skill_dirs(_CLAUDE_SKILLS_DIR) diff --git a/cycode/cli/apps/ai_guardrails/session_start_command.py b/cycode/cli/apps/ai_guardrails/session_start_command.py index bebd421d..79304aa1 100644 --- a/cycode/cli/apps/ai_guardrails/session_start_command.py +++ b/cycode/cli/apps/ai_guardrails/session_start_command.py @@ -9,7 +9,12 @@ import typer -from cycode.cli.apps.ai_guardrails.ides import DEFAULT_IDE_NAME, collect_all_session_contexts, get_ide +from cycode.cli.apps.ai_guardrails.ides import ( + DEFAULT_IDE_NAME, + collect_all_session_contexts, + collect_all_skills, + get_ide, +) from cycode.cli.apps.ai_guardrails.scan.utils import read_stdin_text, safe_json_parse from cycode.cli.apps.auth.auth_common import get_authorization_info from cycode.cli.apps.auth.auth_manager import AuthManager @@ -75,8 +80,9 @@ def _report_session_context( ) -> None: """Report the device + cross-IDE session context to the AI security manager. Never raises. - The device context is always reported. MCP configs are collected from every registered IDE, - not just the triggering one. Unchanged payloads are skipped via a hash cache until the TTL expires. + The device context is always reported. MCP configs and skills are collected from every + registered IDE, not just the triggering one. Unchanged payloads are skipped via a hash cache + until the TTL expires. """ try: config_files_by_ide, enabled_plugins = collect_all_session_contexts() @@ -89,6 +95,9 @@ def _report_session_context( # Sorted by path so the digest is stable regardless of IDE registry order. 'config_files': sorted(config_files_by_ide.values(), key=lambda f: f['path']), 'enabled_plugins': enabled_plugins, + # Already deduplicated and sorted by path, for the same digest-stability reason. + # Editing a skill body changes the digest and so re-reports the device's inventory. + 'skill_files': collect_all_skills(), 'user_email': user_email, } diff --git a/cycode/cyclient/ai_security_manager_client.py b/cycode/cyclient/ai_security_manager_client.py index 62f5618b..0639f841 100644 --- a/cycode/cyclient/ai_security_manager_client.py +++ b/cycode/cyclient/ai_security_manager_client.py @@ -101,6 +101,7 @@ def report_session_context( last_login_user: Optional[str] = None, config_files: Optional[list[dict]] = None, enabled_plugins: Optional[dict] = None, + skill_files: Optional[list[dict]] = None, user_email: Optional[str] = None, ) -> bool: """Report session context to the backend. Returns whether the report was accepted.""" @@ -113,6 +114,7 @@ def report_session_context( 'user_email': user_email, 'config_files': config_files, 'enabled_plugins': enabled_plugins, + 'skill_files': skill_files, } try: diff --git a/tests/cli/commands/ai_guardrails/ides/test_claude_code.py b/tests/cli/commands/ai_guardrails/ides/test_claude_code.py index 743bb372..595452c0 100644 --- a/tests/cli/commands/ai_guardrails/ides/test_claude_code.py +++ b/tests/cli/commands/ai_guardrails/ides/test_claude_code.py @@ -386,3 +386,66 @@ def test_email_from_config_missing_oauth_account() -> None: def test_email_from_config_missing_email_address() -> None: assert _email_from_config({'oauthAccount': {'someOtherField': 'value'}}) is None + + +# skills + + +def test_get_skills_reads_user_scope_skills(fs: FakeFilesystem) -> None: + body = '---\nname: dummy-skill\ndescription: Dummy.\n---\n\nDo it.\n' + skill_file = Path.home() / '.claude' / 'skills' / 'dummy-skill' / 'SKILL.md' + fs.create_file(skill_file, contents=body) + + skills = ClaudeCode().get_skills() + + assert skills == [{'path': str(skill_file), 'content': body}] + + +def test_get_skills_no_skills_dir_returns_empty(fs: FakeFilesystem) -> None: + fs.create_file(Path.home() / '.claude' / 'settings.json', contents='{}') + + assert ClaudeCode().get_skills() == [] + + +def test_read_claude_plugin_collects_plugin_skills(fs: FakeFilesystem) -> None: + """Plugin skills ride on the plugin entry, which is what carries marketplace provenance.""" + plugin_dir = Path('/dummy/marketplace/dummy-plugin') + fs.create_file( + plugin_dir / '.claude-plugin' / 'plugin.json', + contents=json.dumps({'name': 'dummy-plugin', 'version': '2.0.0'}), + ) + skill_file = plugin_dir / 'skills' / 'plugin-skill' / 'SKILL.md' + fs.create_file(skill_file, contents='---\nname: plugin-skill\n---\n\nPlugin body.\n') + + entry, servers = _read_claude_plugin(plugin_dir) + + assert servers == {} + assert entry['version'] == '2.0.0' + assert [s['path'] for s in entry['skill_files']] == [str(skill_file)] + + +def test_read_claude_plugin_without_skills_omits_key(fs: FakeFilesystem) -> None: + plugin_dir = Path('/dummy/marketplace/dummy-plugin') + fs.create_file( + plugin_dir / '.claude-plugin' / 'plugin.json', + contents=json.dumps({'name': 'dummy-plugin'}), + ) + + entry, _ = _read_claude_plugin(plugin_dir) + + assert 'skill_files' not in entry + + +def test_resolve_plugins_surfaces_plugin_skills(fs: FakeFilesystem) -> None: + plugin_dir = Path.home() / '.claude' / 'plugins' / 'cache' / 'dummy-marketplace' / 'dummy-plugin' / '1.0.0' + fs.create_file( + plugin_dir / '.claude-plugin' / 'plugin.json', + contents=json.dumps({'name': 'dummy-plugin', 'version': '1.0.0'}), + ) + skill_file = plugin_dir / 'skills' / 'cached-skill' / 'SKILL.md' + fs.create_file(skill_file, contents='---\nname: cached-skill\n---\nBody.\n') + + plugins = resolve_plugins({'enabledPlugins': {'dummy-plugin@dummy-marketplace': True}}) + + entry = plugins['dummy-plugin@dummy-marketplace'] + assert [Path(s['path']).parent.name for s in entry['skill_files']] == ['cached-skill'] diff --git a/tests/cli/commands/ai_guardrails/ides/test_contract.py b/tests/cli/commands/ai_guardrails/ides/test_contract.py index fb0ca2df..05596e76 100644 --- a/tests/cli/commands/ai_guardrails/ides/test_contract.py +++ b/tests/cli/commands/ai_guardrails/ides/test_contract.py @@ -134,6 +134,16 @@ def test_get_session_context_returns_pair(ide: IDE) -> None: assert isinstance(plugins, dict) +def test_get_skills_returns_path_content_dicts(ide: IDE) -> None: + """Skills must be a list of ``{"path", "content"}`` dicts - empty for IDEs without skills.""" + skills = ide.get_skills() + assert isinstance(skills, list) + for skill in skills: + assert isinstance(skill, dict) + assert isinstance(skill.get('path'), str) + assert isinstance(skill.get('content'), str) + + # HookDecision helpers diff --git a/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py b/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py new file mode 100644 index 00000000..251ce1a5 --- /dev/null +++ b/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py @@ -0,0 +1,94 @@ +"""Skill collection helper tests.""" + +from pathlib import Path + +from pyfakefs.fake_filesystem import FakeFilesystem + +from cycode.cli.apps.ai_guardrails.ides._skill_utils import ( + MAX_SKILL_FILE_BYTES, + MAX_SKILLS_PER_ROOT, + walk_skill_dirs, +) + +_BODY = '---\nname: dummy-skill\ndescription: Dummy.\n---\n\nDo the dummy thing.\n' + + +def test_walk_skill_dirs_missing_root_returns_empty() -> None: + assert walk_skill_dirs(Path('/dummy/does-not-exist')) == [] + + +def test_walk_skill_dirs_collects_path_and_content(fs: FakeFilesystem) -> None: + root = Path('/dummy/skills') + fs.create_file(root / 'dummy-skill' / 'SKILL.md', contents=_BODY) + + skills = walk_skill_dirs(root) + + assert skills == [{'path': str(root / 'dummy-skill' / 'SKILL.md'), 'content': _BODY}] + + +def test_walk_skill_dirs_sorted_by_path(fs: FakeFilesystem) -> None: + """The session-context digest hashes the whole payload, so ordering has to be stable.""" + root = Path('/dummy/skills') + for name in ('charlie', 'alpha', 'bravo'): + fs.create_file(root / name / 'SKILL.md', contents=_BODY) + + paths = [skill['path'] for skill in walk_skill_dirs(root)] + + assert paths == sorted(paths) + assert [Path(p).parent.name for p in paths] == ['alpha', 'bravo', 'charlie'] + + +def test_walk_skill_dirs_ignores_dirs_without_skill_file(fs: FakeFilesystem) -> None: + root = Path('/dummy/skills') + fs.create_file(root / 'real-skill' / 'SKILL.md', contents=_BODY) + fs.create_file(root / 'not-a-skill' / 'README.md', contents='nothing here') + + skills = walk_skill_dirs(root) + + assert [Path(s['path']).parent.name for s in skills] == ['real-skill'] + + +def test_walk_skill_dirs_ignores_loose_files_in_root(fs: FakeFilesystem) -> None: + """Only one directory level is scanned: a skill is always a directory holding a SKILL.md.""" + root = Path('/dummy/skills') + fs.create_file(root / 'SKILL.md', contents=_BODY) + + assert walk_skill_dirs(root) == [] + + +def test_walk_skill_dirs_does_not_recurse(fs: FakeFilesystem) -> None: + """A skill directory may hold nested references; its SKILL.md sits at the top of it.""" + root = Path('/dummy/skills') + fs.create_file(root / 'dummy-skill' / 'SKILL.md', contents=_BODY) + fs.create_file(root / 'dummy-skill' / 'references' / 'SKILL.md', contents=_BODY) + + skills = walk_skill_dirs(root) + + assert len(skills) == 1 + assert skills[0]['path'] == str(root / 'dummy-skill' / 'SKILL.md') + + +def test_walk_skill_dirs_skips_empty_skill_file(fs: FakeFilesystem) -> None: + root = Path('/dummy/skills') + fs.create_file(root / 'blank-skill' / 'SKILL.md', contents=' \n\n') + + assert walk_skill_dirs(root) == [] + + +def test_walk_skill_dirs_skips_oversized_skill_file(fs: FakeFilesystem) -> None: + """An oversized body would cost the device its MCP inventory too - one request carries both.""" + root = Path('/dummy/skills') + fs.create_file(root / 'huge-skill' / 'SKILL.md', contents='x' * (MAX_SKILL_FILE_BYTES + 1)) + fs.create_file(root / 'small-skill' / 'SKILL.md', contents=_BODY) + + skills = walk_skill_dirs(root) + + assert [Path(s['path']).parent.name for s in skills] == ['small-skill'] + + +def test_walk_skill_dirs_truncates_at_count_cap(fs: FakeFilesystem) -> None: + root = Path('/dummy/skills') + for index in range(MAX_SKILLS_PER_ROOT + 5): + fs.create_file(root / f'skill-{index:04d}' / 'SKILL.md', contents=_BODY) + + assert len(walk_skill_dirs(root)) == MAX_SKILLS_PER_ROOT diff --git a/tests/cli/commands/ai_guardrails/test_session_start_command.py b/tests/cli/commands/ai_guardrails/test_session_start_command.py index eaec8531..596fc53b 100644 --- a/tests/cli/commands/ai_guardrails/test_session_start_command.py +++ b/tests/cli/commands/ai_guardrails/test_session_start_command.py @@ -31,6 +31,17 @@ def _isolated_session_context_cache(tmp_path: Path, monkeypatch: pytest.MonkeyPa monkeypatch.setattr(_session_start_mod, '_session_context_cache_path', lambda: tmp_path / '.session-context-cache') +@pytest.fixture(autouse=True) +def _no_local_skills(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the skills sweep away from the developer's real ~/.claude/skills. + + Unlike the MCP collectors, which these tests stub per IDE, the skills sweep walks the + filesystem directly - so without this every assertion would depend on whoever ran it. + A test that cares about skills overrides ``collect_all_skills`` itself. + """ + monkeypatch.setattr(_session_start_mod, 'collect_all_skills', lambda: []) + + # Auth tests @@ -248,6 +259,7 @@ def test_reports_cross_ide_session_context( last_login_user=ANY, config_files=[claude_file, cursor_file], enabled_plugins=plugins, + skill_files=[], user_email=None, ) @@ -282,6 +294,7 @@ def test_no_mcp_anywhere_still_reports_device( last_login_user=ANY, config_files=[], enabled_plugins={}, + skill_files=[], user_email=None, ) @@ -363,6 +376,7 @@ def test_claude_code_reports_config_files_and_plugin_metadata( 'mcp_config_file': json.dumps(plugin_mcp), } }, + skill_files=[], user_email=None, ) @@ -421,6 +435,7 @@ def test_cursor_trigger_sweeps_other_ides( last_login_user=ANY, config_files=[claude_file, cursor_file], enabled_plugins={}, + skill_files=[], user_email=None, ) @@ -590,3 +605,93 @@ def test_unauthenticated_skips_session_init( session_start_command(mock_ctx, ide='claude-code') mock_get_client.assert_not_called() + + +# Skills reporting + + +@patch.object(_claude_mod, 'load_claude_config', return_value={}) +@patch.object(_session_start_mod, 'collect_all_session_contexts') +@patch.object(_session_start_mod, 'get_ai_security_manager_client') +@patch.object(_session_start_mod, 'get_authorization_info') +def test_reports_skill_files( + mock_get_auth: MagicMock, + mock_get_client: MagicMock, + mock_collect: MagicMock, + mock_load_config: MagicMock, + mock_ctx: MagicMock, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """User-scope skills ride alongside the MCP inventory in the same report.""" + mock_get_auth.return_value = MagicMock(tenant_id='tenant-1') + mock_ai_client = MagicMock() + mock_get_client.return_value = mock_ai_client + mock_collect.return_value = ({}, {}) + skill_path = '/home/u/.claude/skills/dummy-skill/SKILL.md' + skills = [{'path': skill_path, 'content': '---\nname: dummy-skill\n---\nBody.\n'}] + monkeypatch.setattr(_session_start_mod, 'collect_all_skills', lambda: skills) + + payload = {'session_id': 'session-123'} + + with patch('sys.stdin', new=StringIO(json.dumps(payload))): + session_start_command(mock_ctx, ide='claude-code') + + mock_ai_client.report_session_context.assert_called_once_with( + hostname=ANY, + platform_name=ANY, + os_version=ANY, + serial_number=ANY, + last_login_user=ANY, + config_files=[], + enabled_plugins={}, + skill_files=skills, + user_email=None, + ) + + +@patch.object(_claude_mod, 'load_claude_config', return_value={}) +@patch.object(_session_start_mod, 'collect_all_session_contexts') +@patch.object(_session_start_mod, 'get_ai_security_manager_client') +@patch.object(_session_start_mod, 'get_authorization_info') +def test_editing_a_skill_re_reports( + mock_get_auth: MagicMock, + mock_get_client: MagicMock, + mock_collect: MagicMock, + mock_load_config: MagicMock, + mock_ctx: MagicMock, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Skill bodies are part of the dedup digest, so an edit sends a fresh report.""" + mock_get_auth.return_value = MagicMock(tenant_id='tenant-1') + mock_ai_client = MagicMock() + mock_get_client.return_value = mock_ai_client + mock_collect.return_value = ({}, {}) + path = '/home/u/.claude/skills/dummy-skill/SKILL.md' + payload = json.dumps({'session_id': 'session-123'}) + + monkeypatch.setattr(_session_start_mod, 'collect_all_skills', lambda: [{'path': path, 'content': 'first'}]) + with patch('sys.stdin', new=StringIO(payload)): + session_start_command(mock_ctx, ide='claude-code') + + # Unchanged inventory is deduplicated away. + with patch('sys.stdin', new=StringIO(payload)): + session_start_command(mock_ctx, ide='claude-code') + assert mock_ai_client.report_session_context.call_count == 1 + + monkeypatch.setattr(_session_start_mod, 'collect_all_skills', lambda: [{'path': path, 'content': 'edited'}]) + with patch('sys.stdin', new=StringIO(payload)): + session_start_command(mock_ctx, ide='claude-code') + assert mock_ai_client.report_session_context.call_count == 2 + + +def test_collect_all_skills_dedupes_and_sorts_by_path() -> None: + """Two IDEs sharing a skills dir report it once; order is stable for the digest.""" + from cycode.cli.apps.ai_guardrails.ides import collect_all_skills + + charlie = {'path': '/dummy/charlie/SKILL.md', 'content': 'c'} + alpha = {'path': '/dummy/alpha/SKILL.md', 'content': 'a'} + first = MagicMock(get_skills=MagicMock(return_value=[charlie, alpha])) + second = MagicMock(get_skills=MagicMock(return_value=[alpha])) + + with patch.dict('cycode.cli.apps.ai_guardrails.ides.IDES', {'first': first, 'second': second}, clear=True): + assert collect_all_skills() == [alpha, charlie] From 705985b852a5a09de0dfe7538bc44b487e9924ca Mon Sep 17 00:00:00 2001 From: Nikita Fishbakh Date: Wed, 2 Sep 2026 13:10:08 +0200 Subject: [PATCH 2/4] CM-71972: Collect plugin skills for every IDE, not just Claude Code A plugin's skills live at /skills//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) --- .../cli/apps/ai_guardrails/ides/_skill_utils.py | 13 +++++++++++++ .../cli/apps/ai_guardrails/ides/claude_code.py | 7 ++----- cycode/cli/apps/ai_guardrails/ides/codex.py | 7 +++++++ cycode/cli/apps/ai_guardrails/ides/copilot.py | 7 +++++++ .../commands/ai_guardrails/ides/test_codex.py | 15 +++++++++++++++ .../commands/ai_guardrails/ides/test_copilot.py | 16 ++++++++++++++++ 6 files changed, 60 insertions(+), 5 deletions(-) diff --git a/cycode/cli/apps/ai_guardrails/ides/_skill_utils.py b/cycode/cli/apps/ai_guardrails/ides/_skill_utils.py index 5254d38e..711681d4 100644 --- a/cycode/cli/apps/ai_guardrails/ides/_skill_utils.py +++ b/cycode/cli/apps/ai_guardrails/ides/_skill_utils.py @@ -19,6 +19,10 @@ SKILL_FILE_NAME = 'SKILL.md' +# Where a plugin keeps its skills, relative to the plugin directory. A property of the plugin format +# rather than of any one IDE, so Claude Code, Codex and Copilot plugins all use it. +PLUGIN_SKILLS_SUBDIR = 'skills' + # A skill is instructions, not data. Anything larger is not a skill we can usefully inventory, # and sending it would push the one-request report toward the API's body limit. MAX_SKILL_FILE_BYTES = 256 * 1024 @@ -88,3 +92,12 @@ def walk_skill_dirs(skills_root: Path) -> list[dict]: skills.append(skill) return skills + + +def walk_plugin_skills(plugin_dir: Path) -> list[dict]: + """Collect the skills a plugin ships, from ``/skills//SKILL.md``. + + Shared by every IDE with a plugin system: the layout belongs to the plugin format, so a plugin + shipping skills is inventoried whichever IDE loaded it. + """ + return walk_skill_dirs(plugin_dir / PLUGIN_SKILLS_SUBDIR) diff --git a/cycode/cli/apps/ai_guardrails/ides/claude_code.py b/cycode/cli/apps/ai_guardrails/ides/claude_code.py index 7a9ea7b9..80ed090e 100644 --- a/cycode/cli/apps/ai_guardrails/ides/claude_code.py +++ b/cycode/cli/apps/ai_guardrails/ides/claude_code.py @@ -13,7 +13,7 @@ resolve_cached_plugin_dir, walk_enabled_plugins, ) -from cycode.cli.apps.ai_guardrails.ides._skill_utils import walk_skill_dirs +from cycode.cli.apps.ai_guardrails.ides._skill_utils import walk_plugin_skills, walk_skill_dirs from cycode.cli.apps.ai_guardrails.ides.base import IDE, DecisionAction, HookDecision from cycode.cli.apps.ai_guardrails.scan.payload import AIHookPayload from cycode.cli.apps.ai_guardrails.scan.types import AiHookEventType @@ -36,9 +36,6 @@ _CLAUDE_SETTINGS_PATH = Path.home() / '.claude' / 'settings.json' _CLAUDE_SKILLS_DIR = Path.home() / '.claude' / 'skills' -# Claude hardcodes a plugin's skills under this subdirectory, the same way it hardcodes ".mcp.json". -_PLUGIN_SKILLS_SUBDIR = 'skills' - _SCAN_COMMAND = f'{CYCODE_SCAN_PROMPT_COMMAND} --ide claude-code' _SESSION_START_COMMAND = f'{CYCODE_SESSION_START_COMMAND} --ide claude-code' @@ -206,7 +203,7 @@ def _read_claude_plugin(plugin_dir: Path) -> tuple[dict, dict]: # Attached to the plugin entry rather than the top-level skills list so the backend keeps the # plugin provenance (marketplace, plugin, version) that a marketplace-installed skill has. - skill_files = walk_skill_dirs(plugin_dir / _PLUGIN_SKILLS_SUBDIR) + skill_files = walk_plugin_skills(plugin_dir) if skill_files: entry['skill_files'] = skill_files diff --git a/cycode/cli/apps/ai_guardrails/ides/codex.py b/cycode/cli/apps/ai_guardrails/ides/codex.py index bdcc5889..8826c1a2 100644 --- a/cycode/cli/apps/ai_guardrails/ides/codex.py +++ b/cycode/cli/apps/ai_guardrails/ides/codex.py @@ -20,6 +20,7 @@ resolve_cached_plugin_dir, walk_enabled_plugins, ) +from cycode.cli.apps.ai_guardrails.ides._skill_utils import walk_plugin_skills from cycode.cli.apps.ai_guardrails.ides.base import IDE, DecisionAction, HookDecision from cycode.cli.apps.ai_guardrails.scan.payload import AIHookPayload from cycode.cli.apps.ai_guardrails.scan.types import AiHookEventType @@ -121,6 +122,12 @@ def _read_codex_plugin(plugin_dir: Path) -> tuple[dict, dict]: if field in manifest: entry[field] = manifest[field] + # Same plugin-format layout as every other IDE's plugins, so a plugin shipping skills is + # inventoried whichever IDE loaded it. + skill_files = walk_plugin_skills(plugin_dir) + if skill_files: + entry['skill_files'] = skill_files + mcp_ref = manifest.get('mcpServers') if not mcp_ref: return entry, {} diff --git a/cycode/cli/apps/ai_guardrails/ides/copilot.py b/cycode/cli/apps/ai_guardrails/ides/copilot.py index 99e6cbac..7b8371db 100644 --- a/cycode/cli/apps/ai_guardrails/ides/copilot.py +++ b/cycode/cli/apps/ai_guardrails/ides/copilot.py @@ -37,6 +37,7 @@ load_plugin_json, walk_enabled_plugins, ) +from cycode.cli.apps.ai_guardrails.ides._skill_utils import walk_plugin_skills from cycode.cli.apps.ai_guardrails.ides.base import IDE, DecisionAction, HookDecision from cycode.cli.apps.ai_guardrails.scan.payload import AIHookPayload from cycode.cli.apps.ai_guardrails.scan.types import AiHookEventType @@ -178,6 +179,12 @@ def _read_copilot_plugin(plugin_dir: Path) -> tuple[dict, dict]: if field in manifest: entry[field] = manifest[field] + # Same plugin-format layout as every other IDE's plugins, so a plugin shipping skills is + # inventoried whichever IDE loaded it. + skill_files = walk_plugin_skills(plugin_dir) + if skill_files: + entry['skill_files'] = skill_files + mcp_ref = manifest.get('mcpServers') mcp_config_path = plugin_dir / mcp_ref if isinstance(mcp_ref, str) else plugin_dir / '.mcp.json' mcp_doc = load_plugin_json(mcp_config_path) or {} diff --git a/tests/cli/commands/ai_guardrails/ides/test_codex.py b/tests/cli/commands/ai_guardrails/ides/test_codex.py index fc0e8849..c7d0e981 100644 --- a/tests/cli/commands/ai_guardrails/ides/test_codex.py +++ b/tests/cli/commands/ai_guardrails/ides/test_codex.py @@ -377,3 +377,18 @@ def test_read_codex_plugin_no_mcp_config_file_when_no_manifest(tmp_path: Path) - assert 'mcp_config_file' not in entry assert servers == {} + + +def test_read_codex_plugin_collects_plugin_skills(fs: FakeFilesystem) -> None: + """Plugin skills are a property of the plugin format, so Codex plugins carry them too.""" + plugin_dir = Path('/dummy/codex-marketplace/dummy-plugin') + fs.create_file( + plugin_dir / '.codex-plugin' / 'plugin.json', + contents=json.dumps({'name': 'dummy-plugin', 'version': '3.0.0'}), + ) + skill_file = plugin_dir / 'skills' / 'codex-skill' / 'SKILL.md' + fs.create_file(skill_file, contents='---\nname: codex-skill\n---\nBody.\n') + + entry, _ = _read_codex_plugin(plugin_dir) + + assert [s['path'] for s in entry['skill_files']] == [str(skill_file)] diff --git a/tests/cli/commands/ai_guardrails/ides/test_copilot.py b/tests/cli/commands/ai_guardrails/ides/test_copilot.py index d11e81ab..8b863f36 100644 --- a/tests/cli/commands/ai_guardrails/ides/test_copilot.py +++ b/tests/cli/commands/ai_guardrails/ides/test_copilot.py @@ -15,6 +15,7 @@ from cycode.cli.apps.ai_guardrails.ides.base import HookDecision from cycode.cli.apps.ai_guardrails.ides.copilot import ( Copilot, + _read_copilot_plugin, _vscode_mcp_config_path, split_mcp_tool_name, ) @@ -499,3 +500,18 @@ def test_parse_mcp_payload_matches_plugin_server_via_normalized_name(fs: FakeFil assert unified.mcp_server_name == 'dummy-tracker' assert unified.mcp_tool_name == 'fetch_api' + + +def test_read_copilot_plugin_collects_plugin_skills(fs: FakeFilesystem) -> None: + """Plugin skills are a property of the plugin format, so Copilot plugins carry them too.""" + plugin_dir = Path('/dummy/copilot-marketplace/dummy-plugin') + fs.create_file( + plugin_dir / 'plugin.json', + contents=json.dumps({'name': 'dummy-plugin', 'version': '4.0.0'}), + ) + skill_file = plugin_dir / 'skills' / 'copilot-skill' / 'SKILL.md' + fs.create_file(skill_file, contents='---\nname: copilot-skill\n---\nBody.\n') + + entry, _ = _read_copilot_plugin(plugin_dir) + + assert [s['path'] for s in entry['skill_files']] == [str(skill_file)] From 5f646b05b4f92e51fe87ec0899c47f32319be79e Mon Sep 17 00:00:00 2001 From: Nikita Fishbakh Date: Wed, 2 Sep 2026 14:18:07 +0200 Subject: [PATCH 3/4] CM-71972: Resolve the skills directory at call time, not at import 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) --- .../cli/apps/ai_guardrails/ides/claude_code.py | 12 ++++++++++-- .../ai_guardrails/ides/test_skill_utils.py | 17 +++++++++++++++++ .../ai_guardrails/test_session_start_command.py | 2 +- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/cycode/cli/apps/ai_guardrails/ides/claude_code.py b/cycode/cli/apps/ai_guardrails/ides/claude_code.py index 80ed090e..74fed9f4 100644 --- a/cycode/cli/apps/ai_guardrails/ides/claude_code.py +++ b/cycode/cli/apps/ai_guardrails/ides/claude_code.py @@ -34,7 +34,6 @@ _CLAUDE_CONFIG_PATH = Path.home() / '.claude.json' _CLAUDE_SETTINGS_PATH = Path.home() / '.claude' / 'settings.json' -_CLAUDE_SKILLS_DIR = Path.home() / '.claude' / 'skills' _SCAN_COMMAND = f'{CYCODE_SCAN_PROMPT_COMMAND} --ide claude-code' _SESSION_START_COMMAND = f'{CYCODE_SESSION_START_COMMAND} --ide claude-code' @@ -171,6 +170,15 @@ def load_claude_settings(settings_path: Optional[Path] = None) -> Optional[dict] return None +def _claude_skills_dir() -> Path: + """Claude Code's user-scope skills: ``~/.claude/skills//SKILL.md``. + + A function, not a module constant: resolving ``Path.home()`` at import time pins the directory to + whatever home the process started with, which a test filesystem then cannot redirect. + """ + return Path.home() / '.claude' / 'skills' + + def _plugins_cache_dir() -> Path: """Claude Code's local plugin content cache: ``~/.claude/plugins/cache////``.""" return Path.home() / '.claude' / 'plugins' / 'cache' @@ -398,4 +406,4 @@ def get_session_context(self) -> tuple[Optional[dict], dict]: return global_config_file, enriched_plugins def get_skills(self) -> list[dict]: - return walk_skill_dirs(_CLAUDE_SKILLS_DIR) + return walk_skill_dirs(_claude_skills_dir()) diff --git a/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py b/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py index 251ce1a5..82372f3c 100644 --- a/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py +++ b/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py @@ -2,6 +2,7 @@ from pathlib import Path +import pytest from pyfakefs.fake_filesystem import FakeFilesystem from cycode.cli.apps.ai_guardrails.ides._skill_utils import ( @@ -92,3 +93,19 @@ def test_walk_skill_dirs_truncates_at_count_cap(fs: FakeFilesystem) -> None: fs.create_file(root / f'skill-{index:04d}' / 'SKILL.md', contents=_BODY) assert len(walk_skill_dirs(root)) == MAX_SKILLS_PER_ROOT + + +def test_walk_skill_dirs_follows_the_current_home(fs: FakeFilesystem, monkeypatch: pytest.MonkeyPatch) -> None: + """Regression: the skills directory must resolve when called, not when the module is imported. + + It was a module-level constant, so Path.home() was captured at import and a redirected home could + never be seen - which passed locally and failed on CI runners whose home differed. + """ + from cycode.cli.apps.ai_guardrails.ides.claude_code import ClaudeCode, _claude_skills_dir + + relocated = Path('/relocated-home') + fs.create_file(relocated / '.claude' / 'skills' / 'moved-skill' / 'SKILL.md', contents=_BODY) + monkeypatch.setattr(Path, 'home', staticmethod(lambda: relocated)) + + assert _claude_skills_dir() == relocated / '.claude' / 'skills' + assert [Path(s['path']).parent.name for s in ClaudeCode().get_skills()] == ['moved-skill'] diff --git a/tests/cli/commands/ai_guardrails/test_session_start_command.py b/tests/cli/commands/ai_guardrails/test_session_start_command.py index 596fc53b..21e420a2 100644 --- a/tests/cli/commands/ai_guardrails/test_session_start_command.py +++ b/tests/cli/commands/ai_guardrails/test_session_start_command.py @@ -39,7 +39,7 @@ def _no_local_skills(monkeypatch: pytest.MonkeyPatch) -> None: filesystem directly - so without this every assertion would depend on whoever ran it. A test that cares about skills overrides ``collect_all_skills`` itself. """ - monkeypatch.setattr(_session_start_mod, 'collect_all_skills', lambda: []) + monkeypatch.setattr(_session_start_mod, 'collect_all_skills', list) # Auth tests From d9c1d444134372f0283bb4a23874d4f2278a0984 Mon Sep 17 00:00:00 2001 From: Nikita Fishbakh Date: Wed, 2 Sep 2026 14:25:47 +0200 Subject: [PATCH 4/4] CM-71972: Patch home with a plain function in the regression test 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) --- tests/cli/commands/ai_guardrails/ides/test_skill_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py b/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py index 82372f3c..09160582 100644 --- a/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py +++ b/tests/cli/commands/ai_guardrails/ides/test_skill_utils.py @@ -105,7 +105,9 @@ def test_walk_skill_dirs_follows_the_current_home(fs: FakeFilesystem, monkeypatc relocated = Path('/relocated-home') fs.create_file(relocated / '.claude' / 'skills' / 'moved-skill' / 'SKILL.md', contents=_BODY) - monkeypatch.setattr(Path, 'home', staticmethod(lambda: relocated)) + # A plain function, not staticmethod/classmethod: those are not reliably callable when set as a + # class attribute across Python versions, which is what broke this test on 3.9 and 3.13. + monkeypatch.setattr(Path, 'home', lambda: relocated) assert _claude_skills_dir() == relocated / '.claude' / 'skills' assert [Path(s['path']).parent.name for s in ClaudeCode().get_skills()] == ['moved-skill']