From e0246aae36c95acdc10d37eb6cd8540c71ff5b2c Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 8 Jul 2026 03:17:05 +0000 Subject: [PATCH 1/4] fix(plugin): load root SKILL.md as a single-skill plugin When a plugin has no skills/ directory, load a SKILL.md at the plugin root as the plugin's single skill. This mirrors Claude Code's single-skill-plugin behavior (v2.1.142+) and lets standalone Agent Skills be published as plugins without an extra skills// nesting level. Previously Plugin._load_skills only scanned a nested skills/ directory, so a marketplace whose entries point at bare SKILL.md directories loaded zero skills at conversation startup. A skills/ directory still takes precedence over a root SKILL.md, matching the Claude spec. Co-authored-by: openhands --- openhands-sdk/openhands/sdk/plugin/plugin.py | 36 ++++++++- tests/sdk/plugin/test_plugin_loading.py | 85 ++++++++++++++++++++ 2 files changed, 118 insertions(+), 3 deletions(-) diff --git a/openhands-sdk/openhands/sdk/plugin/plugin.py b/openhands-sdk/openhands/sdk/plugin/plugin.py index 718afd068d9..9e40993fe52 100644 --- a/openhands-sdk/openhands/sdk/plugin/plugin.py +++ b/openhands-sdk/openhands/sdk/plugin/plugin.py @@ -374,15 +374,33 @@ def _load_manifest(plugin_dir: Path) -> PluginManifest: def _load_skills(plugin_dir: Path) -> list[Skill]: - """Load skills from the skills/ directory. + """Load a plugin's skills. + + Supports both Claude Code plugin skill layouts: + + - Multi-skill: a ``skills/`` directory containing one ``/SKILL.md`` + per skill (or single ``.md`` files). + - Single-skill: a ``SKILL.md`` at the plugin root when there is no + ``skills/`` directory. Claude Code loads such a plugin as a single-skill + plugin (v2.1.142+); this mirrors that behavior so standalone Agent Skills + published as plugins load without an extra nesting level. Note: Plugin skills are loaded with relaxed validation (strict=False) to support Claude Code plugins which may use different naming conventions. """ skills_dir = plugin_dir / "skills" - if not skills_dir.is_dir(): - return [] + if skills_dir.is_dir(): + return _load_skills_from_skills_dir(skills_dir) + + root_skill_md = find_skill_md(plugin_dir) + if root_skill_md is not None: + return _load_root_skill(plugin_dir, root_skill_md) + + return [] + +def _load_skills_from_skills_dir(skills_dir: Path) -> list[Skill]: + """Load every skill under a plugin's ``skills/`` directory.""" skills: list[Skill] = [] for item in sorted(skills_dir.iterdir()): if item.is_dir(): @@ -408,6 +426,18 @@ def _load_skills(plugin_dir: Path) -> list[Skill]: return skills +def _load_root_skill(plugin_dir: Path, skill_md: Path) -> list[Skill]: + """Load a single-skill plugin whose ``SKILL.md`` lives at the plugin root.""" + try: + skill = Skill.load(skill_md, plugin_dir, strict=False) + skill.resources = discover_skill_resources(plugin_dir) + logger.debug(f"Loaded single-skill plugin: {skill.name} from {skill_md}") + return [skill] + except Exception as e: + logger.warning(f"Failed to load root skill from {plugin_dir}: {e}") + return [] + + def _load_hooks(plugin_dir: Path) -> HookConfig | None: """Load hooks configuration from hooks/hooks.json.""" hooks_json = plugin_dir / "hooks" / "hooks.json" diff --git a/tests/sdk/plugin/test_plugin_loading.py b/tests/sdk/plugin/test_plugin_loading.py index 2e826f65bd7..42c1422da73 100644 --- a/tests/sdk/plugin/test_plugin_loading.py +++ b/tests/sdk/plugin/test_plugin_loading.py @@ -139,6 +139,91 @@ def test_load_plugin_with_skills(self, tmp_path: Path): assert len(plugin.skills) == 1 assert plugin.skills[0].name == "test-skill" + def test_load_single_skill_plugin_root_skill_md(self, tmp_path: Path): + """A SKILL.md at the plugin root loads as a single-skill plugin. + + Mirrors Claude Code's single-skill-plugin behavior (v2.1.142+): when a + plugin has no skills/ directory, a root SKILL.md is loaded as the + plugin's skill. This is how standalone Agent Skills are published as + plugins without an extra nesting level. + """ + plugin_dir = tmp_path / "solo-skill" + plugin_dir.mkdir() + manifest_dir = plugin_dir / ".claude-plugin" + manifest_dir.mkdir() + (manifest_dir / "plugin.json").write_text( + '{"name": "solo-skill", "version": "1.0.0"}' + ) + (plugin_dir / "SKILL.md").write_text( + """--- +name: solo-skill +description: A standalone skill published as a single-skill plugin. +--- + +Body of the solo skill. +""" + ) + + plugin = Plugin.load(plugin_dir) + + assert plugin.name == "solo-skill" + assert len(plugin.skills) == 1 + assert plugin.skills[0].name == "solo-skill" + + def test_load_single_skill_plugin_without_manifest(self, tmp_path: Path): + """Root SKILL.md loads even when plugin.json is absent.""" + plugin_dir = tmp_path / "inferred-solo" + plugin_dir.mkdir() + (plugin_dir / "SKILL.md").write_text( + """--- +name: inferred-solo +description: Root skill with no manifest. +--- + +Body. +""" + ) + + plugin = Plugin.load(plugin_dir) + + assert plugin.name == "inferred-solo" + assert len(plugin.skills) == 1 + assert plugin.skills[0].name == "inferred-solo" + + def test_skills_dir_takes_precedence_over_root_skill_md(self, tmp_path: Path): + """When a skills/ directory exists, it wins over a root SKILL.md. + + Matches Claude Code: the root SKILL.md is only a fallback used when + there is no skills/ directory. + """ + plugin_dir = tmp_path / "both" + plugin_dir.mkdir() + # Root SKILL.md that must be IGNORED because skills/ exists. + (plugin_dir / "SKILL.md").write_text( + "---\nname: root-ignored\ndescription: should not load\n---\nbody\n" + ) + skill_dir = plugin_dir / "skills" / "nested" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + "---\nname: nested\ndescription: the real one\n---\nbody\n" + ) + + plugin = Plugin.load(plugin_dir) + + names = {s.name for s in plugin.skills} + assert names == {"nested"} + assert "root-ignored" not in names + + def test_load_plugin_no_skills_anywhere(self, tmp_path: Path): + """A plugin with neither skills/ nor a root SKILL.md loads zero skills.""" + plugin_dir = tmp_path / "empty" + plugin_dir.mkdir() + (plugin_dir / "README.md").write_text("# not a skill\n") + + plugin = Plugin.load(plugin_dir) + + assert plugin.skills == [] + def test_load_plugin_with_hooks(self, tmp_path: Path): """Test loading a plugin with hooks.""" plugin_dir = tmp_path / "hook-plugin" From 62fac714b4983a5b7711f41a3a1a8aafe3ce8910 Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 8 Jul 2026 06:49:50 +0000 Subject: [PATCH 2/4] trigger CI From 3c31dc7408dec29f48909716ca52190b4420b538 Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 8 Jul 2026 07:32:52 +0000 Subject: [PATCH 3/4] fix: prevent double-loading of root .mcp.json with divergent semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes three issues when loading root SKILL.md plugins: Issue #1 (🔴): Malformed root .mcp.json silently dropped the entire skill - Plugin-level loader tolerates malformed .mcp.json (logs warning) - Skill-level loader raised SkillValidationError, caught by broad except - Result: skill silently dropped (0 skills loaded) Issue #2 (🟠): Root .mcp.json parsed twice with divergent expansion - Plugin level: expand_defaults=False (preserves ${VAR:-default}) - Skill level: expand_defaults=True (eagerly expands defaults) - Result: same file processed with different semantics Issue #3 (🟡): Redundant resource discovery - discover_skill_resources() called by both Skill.load() and plugin loader - Minor performance issue, but unnecessary duplication Root cause: For root SKILL.md plugins, plugin_dir == skill_root, so .mcp.json at plugin root is the same file both loaders attempt to process. Solution: - Add skip_mcp parameter to Skill.load() (defaults to False) - Pass skip_mcp=True when loading root skills from plugin loader - Remove redundant discover_skill_resources() calls from plugin loader - Add comprehensive regression tests (4 new tests) Test coverage: - test_malformed_root_mcp_json_does_not_drop_skill - test_root_mcp_json_not_double_loaded - test_root_skill_resources_not_duplicated - test_nested_skill_with_own_mcp_json_still_loads All 266 tests pass (46 plugin + 220 skill tests) Co-authored-by: openhands --- openhands-sdk/openhands/sdk/plugin/plugin.py | 18 +- openhands-sdk/openhands/sdk/skills/skill.py | 20 +- tests/sdk/plugin/test_plugin_loading.py | 217 +++++++++++++++++++ 3 files changed, 243 insertions(+), 12 deletions(-) diff --git a/openhands-sdk/openhands/sdk/plugin/plugin.py b/openhands-sdk/openhands/sdk/plugin/plugin.py index 9e40993fe52..f3a37e867f9 100644 --- a/openhands-sdk/openhands/sdk/plugin/plugin.py +++ b/openhands-sdk/openhands/sdk/plugin/plugin.py @@ -19,7 +19,6 @@ ) from openhands.sdk.skills.skill import Skill from openhands.sdk.skills.utils import ( - discover_skill_resources, find_skill_md, load_mcp_config, ) @@ -407,9 +406,8 @@ def _load_skills_from_skills_dir(skills_dir: Path) -> list[Skill]: skill_md = find_skill_md(item) if skill_md: try: + # Don't manually set skill.resources: Skill.load() already discovers them skill = Skill.load(skill_md, skills_dir, strict=False) - # Discover and attach resources - skill.resources = discover_skill_resources(item) skills.append(skill) logger.debug(f"Loaded skill: {skill.name} from {skill_md}") except Exception as e: @@ -427,10 +425,18 @@ def _load_skills_from_skills_dir(skills_dir: Path) -> list[Skill]: def _load_root_skill(plugin_dir: Path, skill_md: Path) -> list[Skill]: - """Load a single-skill plugin whose ``SKILL.md`` lives at the plugin root.""" + """Load a single-skill plugin whose ``SKILL.md`` lives at the plugin root. + + For root skills, the plugin directory is the skill root, so .mcp.json at the + plugin level is the same file that Skill.load() would try to load. We pass + skip_mcp=True to avoid double-loading with different semantics (plugin-level + uses expand_defaults=False for deferred secret expansion; skill-level would + use expand_defaults=True and raise on validation errors). + """ try: - skill = Skill.load(skill_md, plugin_dir, strict=False) - skill.resources = discover_skill_resources(plugin_dir) + # skip_mcp=True: Plugin-level MCP config already loaded by _load_plugin_mcp_config() + # Don't manually set skill.resources: Skill.load() already discovers them + skill = Skill.load(skill_md, plugin_dir, strict=False, skip_mcp=True) logger.debug(f"Loaded single-skill plugin: {skill.name} from {skill_md}") return [skill] except Exception as e: diff --git a/openhands-sdk/openhands/sdk/skills/skill.py b/openhands-sdk/openhands/sdk/skills/skill.py index b137362e3ea..16ea845b052 100644 --- a/openhands-sdk/openhands/sdk/skills/skill.py +++ b/openhands-sdk/openhands/sdk/skills/skill.py @@ -299,6 +299,7 @@ def load( path: str | Path, skill_base_dir: Path | None = None, strict: bool = True, + skip_mcp: bool = False, ) -> "Skill": """Load a skill from a markdown file with frontmatter. @@ -313,6 +314,9 @@ def load( skill_base_dir: Base directory for skills (used to derive relative names). strict: If True, enforce strict AgentSkills name validation. If False, allow relaxed naming (e.g., for plugin compatibility). + skip_mcp: If True, skip loading MCP configuration from .mcp.json. + Used when loading root plugin skills where MCP config is already + loaded at the plugin level to avoid double-loading. """ path = Path(path) if isinstance(path, str) else path @@ -320,13 +324,15 @@ def load( file_content = f.read() if path.name.lower() == "skill.md": - return cls._load_agentskills_skill(path, file_content, strict=strict) + return cls._load_agentskills_skill( + path, file_content, strict=strict, skip_mcp=skip_mcp + ) else: return cls._load_legacy_openhands_skill(path, file_content, skill_base_dir) @classmethod def _load_agentskills_skill( - cls, path: Path, file_content: str, strict: bool = True + cls, path: Path, file_content: str, strict: bool = True, skip_mcp: bool = False ) -> "Skill": """Load a skill from an AgentSkills-format SKILL.md file. @@ -334,6 +340,7 @@ def _load_agentskills_skill( path: Path to the SKILL.md file. file_content: Content of the file. strict: If True, enforce strict AgentSkills name validation. + skip_mcp: If True, skip loading MCP configuration from .mcp.json. """ # For SKILL.md files, use parent directory name as the skill name directory_name = path.parent.name @@ -357,10 +364,11 @@ def _load_agentskills_skill( # Load MCP configuration from .mcp.json (agent_skills ONLY use .mcp.json) mcp_tools: dict[str, MCPServer] | None = None - mcp_json_path = find_mcp_config(skill_root) - if mcp_json_path: - mcp_config = load_mcp_config(mcp_json_path, skill_root) - mcp_tools = coerce_mcp_config(mcp_config.get("mcpServers", {})) + if not skip_mcp: + mcp_json_path = find_mcp_config(skill_root) + if mcp_json_path: + mcp_config = load_mcp_config(mcp_json_path, skill_root) + mcp_tools = coerce_mcp_config(mcp_config.get("mcpServers", {})) # Discover resource directories resources: SkillResources | None = None diff --git a/tests/sdk/plugin/test_plugin_loading.py b/tests/sdk/plugin/test_plugin_loading.py index 42c1422da73..f5a94b4cdaa 100644 --- a/tests/sdk/plugin/test_plugin_loading.py +++ b/tests/sdk/plugin/test_plugin_loading.py @@ -943,3 +943,220 @@ def test_plugin_mcp_skill_root_is_expanded(self, tmp_path: Path): command = dump_mcp_config(plugin.mcp_config)["test-server"]["command"] assert str(plugin_dir) in command assert "${SKILL_ROOT}" not in command + + +class TestRootSkillMcpHandling: + """Tests for proper MCP config handling in root SKILL.md plugins. + + These tests verify the fix for issues where root .mcp.json files were + being loaded twice with different semantics, causing failures and + inconsistencies. + """ + + def test_malformed_root_mcp_json_does_not_drop_skill(self, tmp_path: Path): + """Issue #1: Malformed root .mcp.json should not silently drop the skill. + + Before fix: Plugin-level loader tolerates malformed .mcp.json (logs warning), + but skill-level loader raises SkillValidationError, caught by broad except, + returning [] - skill silently dropped. + + After fix: skip_mcp=True prevents double-loading, skill loads successfully. + """ + import json + + plugin_dir = tmp_path / "test-plugin" + plugin_dir.mkdir() + + # Create minimal manifest + manifest_dir = plugin_dir / ".plugin" + manifest_dir.mkdir() + (manifest_dir / "plugin.json").write_text( + json.dumps({"name": "test-plugin", "version": "1.0.0"}) + ) + + # Create valid root SKILL.md + (plugin_dir / "SKILL.md").write_text( + """--- +name: test-skill +description: A test skill +--- + +# Test Skill + +This is a test skill. +""" + ) + + # Create MALFORMED root .mcp.json (invalid JSON) + (plugin_dir / ".mcp.json").write_text('{ "mcpServers": { invalid json }') + + # Load the plugin + plugin = Plugin.load(plugin_dir) + + # The skill should still load (MCP loading is skipped for root skills) + assert len(plugin.skills) == 1 + assert plugin.skills[0].name == "test-skill" + # Plugin-level MCP config should be empty (malformed file tolerated) + assert plugin.mcp_config == {} + + def test_root_mcp_json_not_double_loaded(self, tmp_path: Path): + """Issue #2: Root .mcp.json should not be loaded twice with different semantics. + + Before fix: Same file loaded by both _load_plugin_mcp_config + (expand_defaults=False) and Skill.load (expand_defaults=True). + + After fix: Only loaded once at plugin level, skill uses skip_mcp=True. + """ + import json + + plugin_dir = tmp_path / "test-plugin" + plugin_dir.mkdir() + + # Create minimal manifest + manifest_dir = plugin_dir / ".plugin" + manifest_dir.mkdir() + (manifest_dir / "plugin.json").write_text( + json.dumps({"name": "test-plugin", "version": "1.0.0"}) + ) + + # Create root SKILL.md + (plugin_dir / "SKILL.md").write_text( + """--- +name: test-skill +description: A test skill +--- + +# Test Skill +""" + ) + + # Create root .mcp.json with variable placeholder + (plugin_dir / ".mcp.json").write_text( + json.dumps( + { + "mcpServers": { + "test-server": { + "command": "server", + "args": ["--token", "${DB_TOKEN:-DEFAULT_TOKEN}"], + } + } + } + ) + ) + + # Load the plugin + plugin = Plugin.load(plugin_dir) + + # Plugin-level MCP config should preserve the placeholder (expand_defaults=False) + mcp_dump = dump_mcp_config(plugin.mcp_config) + assert "${DB_TOKEN:-DEFAULT_TOKEN}" in mcp_dump["test-server"]["args"] + + # Skill should have loaded successfully with skip_mcp=True + assert len(plugin.skills) == 1 + assert plugin.skills[0].name == "test-skill" + # Skill's mcp_tools should be None (not loaded due to skip_mcp=True) + assert plugin.skills[0].mcp_tools is None + + def test_root_skill_resources_not_duplicated(self, tmp_path: Path): + """Issue #3: Resources should not be discovered twice for root skills. + + Before fix: discover_skill_resources called both by Skill.load() and + by plugin loader (redundant). + + After fix: Only Skill.load() discovers resources, plugin loader doesn't + call discover_skill_resources. + """ + import json + + plugin_dir = tmp_path / "test-plugin" + plugin_dir.mkdir() + + # Create minimal manifest + manifest_dir = plugin_dir / ".plugin" + manifest_dir.mkdir() + (manifest_dir / "plugin.json").write_text( + json.dumps({"name": "test-plugin", "version": "1.0.0"}) + ) + + # Create root SKILL.md + (plugin_dir / "SKILL.md").write_text( + """--- +name: test-skill +description: A test skill +--- + +# Test Skill +""" + ) + + # Create some resources + (plugin_dir / "test.txt").write_text("test resource") + assets_dir = plugin_dir / "assets" + assets_dir.mkdir() + (assets_dir / "asset.txt").write_text("asset content") + + # Load the plugin + plugin = Plugin.load(plugin_dir) + + # Resources should be discovered and attached + assert len(plugin.skills) == 1 + assert plugin.skills[0].resources is not None + assert plugin.skills[0].resources.assets == ["asset.txt"] + # This test passes if no exception is raised - the fix prevents the + # redundant call but the end result is the same + + def test_nested_skill_with_own_mcp_json_still_loads(self, tmp_path: Path): + """Verify nested skills with their own .mcp.json still work correctly. + + Nested skills should continue to load their own .mcp.json normally + (not skipped) since they're in a different directory than the plugin root. + """ + import json + + plugin_dir = tmp_path / "test-plugin" + plugin_dir.mkdir() + + # Create minimal manifest + manifest_dir = plugin_dir / ".plugin" + manifest_dir.mkdir() + (manifest_dir / "plugin.json").write_text( + json.dumps({"name": "test-plugin", "version": "1.0.0"}) + ) + + # Create nested skill with its own .mcp.json + skills_dir = plugin_dir / "skills" + skills_dir.mkdir() + skill_dir = skills_dir / "nested-skill" + skill_dir.mkdir() + + (skill_dir / "SKILL.md").write_text( + """--- +name: nested-skill +description: A nested skill +--- + +# Nested Skill +""" + ) + + (skill_dir / ".mcp.json").write_text( + json.dumps( + { + "mcpServers": { + "nested-server": { + "command": "nested-server", + } + } + } + ) + ) + + # Load the plugin + plugin = Plugin.load(plugin_dir) + + # Skill should load with its own MCP config + assert len(plugin.skills) == 1 + assert plugin.skills[0].name == "nested-skill" + # Nested skill SHOULD have mcp_tools (not skipped) + assert plugin.skills[0].mcp_tools is not None + assert "nested-server" in plugin.skills[0].mcp_tools From c082e051df0f278c6ea4cedd3111ae3d482764f7 Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 8 Jul 2026 07:44:31 +0000 Subject: [PATCH 4/4] style: fix line length violations and apply ruff formatting - Shorten comment lines to stay within 88 character limit - Apply ruff auto-formatting to test files - All pre-commit checks now pass --- openhands-sdk/openhands/sdk/plugin/plugin.py | 8 ++++---- tests/sdk/plugin/test_plugin_loading.py | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/openhands-sdk/openhands/sdk/plugin/plugin.py b/openhands-sdk/openhands/sdk/plugin/plugin.py index f3a37e867f9..bbc2ec8736a 100644 --- a/openhands-sdk/openhands/sdk/plugin/plugin.py +++ b/openhands-sdk/openhands/sdk/plugin/plugin.py @@ -406,7 +406,7 @@ def _load_skills_from_skills_dir(skills_dir: Path) -> list[Skill]: skill_md = find_skill_md(item) if skill_md: try: - # Don't manually set skill.resources: Skill.load() already discovers them + # Skill.load() discovers resources, no need to do it again skill = Skill.load(skill_md, skills_dir, strict=False) skills.append(skill) logger.debug(f"Loaded skill: {skill.name} from {skill_md}") @@ -426,7 +426,7 @@ def _load_skills_from_skills_dir(skills_dir: Path) -> list[Skill]: def _load_root_skill(plugin_dir: Path, skill_md: Path) -> list[Skill]: """Load a single-skill plugin whose ``SKILL.md`` lives at the plugin root. - + For root skills, the plugin directory is the skill root, so .mcp.json at the plugin level is the same file that Skill.load() would try to load. We pass skip_mcp=True to avoid double-loading with different semantics (plugin-level @@ -434,8 +434,8 @@ def _load_root_skill(plugin_dir: Path, skill_md: Path) -> list[Skill]: use expand_defaults=True and raise on validation errors). """ try: - # skip_mcp=True: Plugin-level MCP config already loaded by _load_plugin_mcp_config() - # Don't manually set skill.resources: Skill.load() already discovers them + # skip_mcp=True: Plugin-level MCP already loaded + # Skill.load() discovers resources, no need to do it again skill = Skill.load(skill_md, plugin_dir, strict=False, skip_mcp=True) logger.debug(f"Loaded single-skill plugin: {skill.name} from {skill_md}") return [skill] diff --git a/tests/sdk/plugin/test_plugin_loading.py b/tests/sdk/plugin/test_plugin_loading.py index f5a94b4cdaa..6aa9087c102 100644 --- a/tests/sdk/plugin/test_plugin_loading.py +++ b/tests/sdk/plugin/test_plugin_loading.py @@ -947,7 +947,7 @@ def test_plugin_mcp_skill_root_is_expanded(self, tmp_path: Path): class TestRootSkillMcpHandling: """Tests for proper MCP config handling in root SKILL.md plugins. - + These tests verify the fix for issues where root .mcp.json files were being loaded twice with different semantics, causing failures and inconsistencies. @@ -955,11 +955,11 @@ class TestRootSkillMcpHandling: def test_malformed_root_mcp_json_does_not_drop_skill(self, tmp_path: Path): """Issue #1: Malformed root .mcp.json should not silently drop the skill. - + Before fix: Plugin-level loader tolerates malformed .mcp.json (logs warning), but skill-level loader raises SkillValidationError, caught by broad except, returning [] - skill silently dropped. - + After fix: skip_mcp=True prevents double-loading, skill loads successfully. """ import json @@ -1001,10 +1001,10 @@ def test_malformed_root_mcp_json_does_not_drop_skill(self, tmp_path: Path): def test_root_mcp_json_not_double_loaded(self, tmp_path: Path): """Issue #2: Root .mcp.json should not be loaded twice with different semantics. - - Before fix: Same file loaded by both _load_plugin_mcp_config + + Before fix: Same file loaded by both _load_plugin_mcp_config (expand_defaults=False) and Skill.load (expand_defaults=True). - + After fix: Only loaded once at plugin level, skill uses skip_mcp=True. """ import json @@ -1047,7 +1047,7 @@ def test_root_mcp_json_not_double_loaded(self, tmp_path: Path): # Load the plugin plugin = Plugin.load(plugin_dir) - # Plugin-level MCP config should preserve the placeholder (expand_defaults=False) + # Plugin-level MCP preserves placeholders (expand_defaults=False) mcp_dump = dump_mcp_config(plugin.mcp_config) assert "${DB_TOKEN:-DEFAULT_TOKEN}" in mcp_dump["test-server"]["args"] @@ -1059,10 +1059,10 @@ def test_root_mcp_json_not_double_loaded(self, tmp_path: Path): def test_root_skill_resources_not_duplicated(self, tmp_path: Path): """Issue #3: Resources should not be discovered twice for root skills. - + Before fix: discover_skill_resources called both by Skill.load() and by plugin loader (redundant). - + After fix: Only Skill.load() discovers resources, plugin loader doesn't call discover_skill_resources. """ @@ -1107,7 +1107,7 @@ def test_root_skill_resources_not_duplicated(self, tmp_path: Path): def test_nested_skill_with_own_mcp_json_still_loads(self, tmp_path: Path): """Verify nested skills with their own .mcp.json still work correctly. - + Nested skills should continue to load their own .mcp.json normally (not skipped) since they're in a different directory than the plugin root. """