Skip to content

fix(mcp): rewrite skill-shipped MCP command paths for the Claude target #1465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danielmeppiel merged 1 commit into from
May 26, 2026
Merged

fix(mcp): rewrite skill-shipped MCP command paths for the Claude target #1465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/apm_cli/adapters/client/claude.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,28 @@ class ClaudeClientAdapter(CopilotClientAdapter):
# See #1152 supply-chain analysis.
_supports_runtime_env_substitution: bool = False

# Skill packages declare self-defined MCP launchers using the cross-tool
# converged path ``.agents/skills/<name>/...`` (see
# https://microsoft.github.io/apm/reference/targets-matrix/#skills-convergence).
# Claude is the only target that does NOT converge to ``.agents/skills/``
# -- it keeps ``.claude/skills/`` (see install/skill_path_migration.py:55).
# Rewrite the command prefix so ``.mcp.json`` points at where the launcher
# actually lives after deploy.
_CONVERGED_SKILL_PREFIX = ".agents/skills/"
_CLAUDE_SKILL_PREFIX = ".claude/skills/"

@classmethod
def _rewrite_self_defined_skill_command(cls, command: str) -> str:
if isinstance(command, str) and command.startswith(cls._CONVERGED_SKILL_PREFIX):
return cls._CLAUDE_SKILL_PREFIX + command[len(cls._CONVERGED_SKILL_PREFIX) :]

@srid srid May 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is ruff format's style (Black-style, E203) for slices whose index is a complex expression — and the project's configured formatter, per pyproject.toml. Both ruff check src/apm_cli/adapters/client/claude.py and ruff format --check pass; removing the space would in fact be re-added by ruff format.

It's also the dominant convention in src/apm_cli/ — a quick grep:

src/apm_cli/bundle/plugin_exporter.py:72:        normalized = normalized[len("skills/") :].lstrip("/")
src/apm_cli/utils/normalization.py:41:        return content[len(_BOM) :]
src/apm_cli/core/conflict_detector.py:115:                server_name = raw_key[len("mcp_servers.") :]
src/apm_cli/commands/policy.py:54:            return source[len(prefix) :]
src/apm_cli/bundle/lockfile_enrichment.py:153:                    mapped = dst_prefix + f[len(src_prefix) :]
src/apm_cli/core/command_logger.py:693:            reason = reason[len(prefix) :]
src/apm_cli/bundle/local_bundle.py:281:        return value[len("sha256:") :].strip().lower()
src/apm_cli/commands/marketplace/check.py:60:                            tag_name = tag_name[len("refs/tags/") :]
src/apm_cli/core/token_manager.py:242:                    token = line[len("password=") :]

Leaving as-is.

return command

def _format_server_config(self, server_info, env_overrides=None, runtime_vars=None):
config = super()._format_server_config(server_info, env_overrides, runtime_vars)
if isinstance(config, dict) and "command" in config:
config["command"] = self._rewrite_self_defined_skill_command(config["command"])
return config

@staticmethod
def _normalize_mcp_entry_for_claude_code(entry: dict) -> dict:
"""Normalize a server entry to Claude Code's on-disk shape.
Expand Down
42 changes: 42 additions & 0 deletions tests/unit/test_claude_mcp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,48 @@ def test_update_config_tolerates_malformed_project_mcp_json(self):
data = json.loads(self.mcp_path.read_text(encoding="utf-8"))
self.assertIn("srv", data["mcpServers"])

def test_self_defined_skill_command_rewritten_to_claude_skills(self):
"""Skill-shipped MCP launchers declare their command using the
cross-tool ``.agents/skills/<name>/...`` convention. Claude does not
converge to ``.agents/skills/`` (see install/skill_path_migration.py),
so the prefix must be rewritten to ``.claude/skills/`` -- otherwise
``.mcp.json`` points at a path that does not exist on disk.
"""
with patch.object(self.adapter, "registry_client") as mock_registry:
mock_registry.find_server_by_reference.return_value = {
"name": "chrome-devtools",
"_raw_stdio": {
"command": ".agents/skills/nix-chrome-devtools-mcp/bin/serve",
"args": [],
"env": {},
},
}
ok = self.adapter.configure_mcp_server("chrome-devtools")

self.assertTrue(ok)
data = json.loads(self.mcp_path.read_text(encoding="utf-8"))
self.assertEqual(
data["mcpServers"]["chrome-devtools"]["command"],
".claude/skills/nix-chrome-devtools-mcp/bin/serve",
)

def test_self_defined_non_skill_command_left_unchanged(self):
"""Only the ``.agents/skills/<name>/...`` prefix is rewritten;
bare binaries (``npx``, ``node``), absolute paths, and unrelated
relative paths must pass through verbatim.
"""
for raw_command in (
"npx",
"/usr/local/bin/mcp-server",
"./scripts/run-mcp.sh",
".agents/other/path",
):
with self.subTest(command=raw_command):
self.assertEqual(
self.adapter._rewrite_self_defined_skill_command(raw_command),
raw_command,
)

def test_configure_self_defined_stdio_preserves_env(self):
with patch.object(self.adapter, "registry_client") as mock_registry:
mock_registry.find_server_by_reference.return_value = {
Expand Down
Loading