From e1fa573bf632c9cd6fdade4cbae971a246347f94 Mon Sep 17 00:00:00 2001 From: agupta-cruiser Date: Wed, 9 Sep 2026 02:34:50 +0530 Subject: [PATCH] Fixed the package analytics endpoint generation by mcp, included v2 in it --- cloudsmith_cli/cli/tests/commands/test_mcp.py | 62 +++++++++++++++++++ cloudsmith_cli/core/mcp/server.py | 28 ++++++++- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/cloudsmith_cli/cli/tests/commands/test_mcp.py b/cloudsmith_cli/cli/tests/commands/test_mcp.py index 72a96f5b..82c45e1c 100644 --- a/cloudsmith_cli/cli/tests/commands/test_mcp.py +++ b/cloudsmith_cli/cli/tests/commands/test_mcp.py @@ -375,6 +375,68 @@ def test_server_respects_tool_filtering(self): assert "packages_list" not in server.tools +class TestMCPServerVersionPrefix: + """The version prefix (/v1, /v2) is declared out-of-band from the path + keys (servers[].url for v2, basePath for v1), so tool URLs must prepend + it or v2 requests hit the wrong path and 404. + """ + + def _server(self): + import cloudsmith_api + + api_config = cloudsmith_api.Configuration() + api_config.host = "https://api.cloudsmith.io" + api_config.api_key = {"X-Api-Key": "test-key"} + return DynamicMCPServer(api_config=api_config, force_all_tools=True) + + def test_v2_spec_base_url_includes_version_from_servers(self): + server = self._server() + server.spec = {"servers": [{"url": "https://api.cloudsmith.io/v2/"}]} + assert server._spec_base_url() == "https://api.cloudsmith.io/v2" + + def test_v1_spec_base_url_falls_back_to_basepath(self): + server = self._server() + server.spec = {"basePath": "/"} + assert server._spec_base_url() == "https://api.cloudsmith.io" + + def test_spec_base_url_keeps_configured_host_not_spec_host(self): + """A custom --api-host must win; only the version path is borrowed.""" + import cloudsmith_api + + api_config = cloudsmith_api.Configuration() + api_config.host = "https://api.eu.cloudsmith.example" + api_config.api_key = {"X-Api-Key": "test-key"} + server = DynamicMCPServer(api_config=api_config, force_all_tools=True) + server.spec = {"servers": [{"url": "https://api.cloudsmith.io/v2/"}]} + assert server._spec_base_url() == "https://api.eu.cloudsmith.example/v2" + + def test_v2_tools_are_generated_under_v2_path(self): + import asyncio + + server = self._server() + server.spec = { + "servers": [{"url": "https://api.cloudsmith.io/v2/"}], + "paths": { + "/analytics/logs/package/{workspace}/": { + "get": { + "operationId": "analytics_logs_package_list", + "summary": "List package log events", + } + } + }, + } + asyncio.run( + server._generate_tools_from_spec( # pylint: disable=protected-access + server._spec_base_url() # pylint: disable=protected-access + ) + ) + tool = server.tools["analytics_logs_package_list"] + assert ( + tool.base_url + tool.path + == "https://api.cloudsmith.io/v2/analytics/logs/package/{workspace}/" + ) + + SERVER_CONFIG = {"command": "cloudsmith", "args": ["mcp", "start"]} diff --git a/cloudsmith_cli/core/mcp/server.py b/cloudsmith_cli/core/mcp/server.py index be64b10f..a4b780da 100644 --- a/cloudsmith_cli/core/mcp/server.py +++ b/cloudsmith_cli/core/mcp/server.py @@ -212,7 +212,26 @@ async def load_openapi_spec(self): response = await http_client.get(spec_url) response.raise_for_status() self.spec = response.json() - await self._generate_tools_from_spec() + await self._generate_tools_from_spec(self._spec_base_url()) + + def _spec_base_url(self) -> str: + """Base URL for the currently-loaded spec, including its version path. + + The OpenAPI spec declares its version prefix out-of-band from the + path keys: OpenAPI 3 (v2) puts it in ``servers[].url`` (e.g. + ``https://api.cloudsmith.io/v2/``) while Swagger 2 (v1) uses + ``basePath``. The path keys themselves are version-relative, so we + must prepend that prefix or v2 requests hit ``/analytics/...`` instead + of ``/v2/analytics/...`` and 404. We keep the configured host (so a + custom ``--api-host`` still wins) and only borrow the version path. + """ + version_path = "" + servers = self.spec.get("servers") + if servers and servers[0].get("url"): + version_path = parse.urlsplit(servers[0]["url"]).path + else: + version_path = self.spec.get("basePath", "") or "" + return f"{self.api_base_url.rstrip('/')}{version_path}".rstrip("/") def _get_tool_groups(self, tool_name: str) -> list[str]: """ @@ -286,12 +305,15 @@ def _is_tool_allowed(self, tool_name: str) -> bool: # Otherwise disable all categories in the default list return not any(group in DEFAULT_DISABLED_CATEGORIES for group in tool_groups) - async def _generate_tools_from_spec(self): + async def _generate_tools_from_spec(self, base_url: str | None = None): """Generate MCP tools from OpenAPI specification""" if not self.spec: raise ValueError("OpenAPI spec not loaded") + if base_url is None: + base_url = self.api_base_url + # Parse paths and generate tools for path, path_item in self.spec.get("paths", {}).items(): for method, operation in path_item.items(): @@ -303,7 +325,7 @@ async def _generate_tools_from_spec(self): path, operation, path_parameters, - self.api_base_url, + base_url, ) if tool and self._is_tool_allowed(tool.name): self.tools[tool.name] = tool