From 3611a900a5f4ca12131cecd37b71f4d4d4014393 Mon Sep 17 00:00:00 2001 From: Erez Hadad Date: Thu, 11 Jun 2026 19:58:54 +0300 Subject: [PATCH] Changed to UUID store. Code migrated to use UUID where appropriate, including tests. All tests passing except one related to llm-client. --- pyproject.toml | 2 +- .../skillberry_agent_lib/pyproject.toml | 2 +- .../skillberry_agent_lib/skillberry_store.py | 91 ++++++++++++++++--- .../test_vmcp_server_manager.py | 48 ++++++---- .../vmcp_server_manager.py | 17 +++- 5 files changed, 121 insertions(+), 39 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3b31018..467f580 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ dependencies = [ "Flask>=3.1.0", "ipython>=8.12.3", "skillberry-agent-lib", - "skillberry-store-sdk @ git+https://github.com/skillberry-ai/skillberry-store.git@0.1.0-alpha#subdirectory=client/python/skillberry_store_sdk", + "skillberry-store-sdk @ git+https://github.com/skillberry-ai/skillberry-store.git#subdirectory=client/python/skillberry_store_sdk", "toml-cli", "llm-switchboard[litellm]>=0.1.0" ] diff --git a/shared/python/skillberry_agent_lib/pyproject.toml b/shared/python/skillberry_agent_lib/pyproject.toml index 9028cd3..0d91db1 100644 --- a/shared/python/skillberry_agent_lib/pyproject.toml +++ b/shared/python/skillberry_agent_lib/pyproject.toml @@ -17,7 +17,7 @@ langchain = ">= 0.3.25" langchain-core = ">= 0.3.59" langgraph = ">= 0.4.3" langchain-mcp-adapters = ">= 0.2.1" -skillberry-store-sdk = { git = "https://github.com/skillberry-ai/skillberry-store.git", tag = "0.1.0-alpha", subdirectory = "client/python/skillberry_store_sdk" } +skillberry-store-sdk = { git = "https://github.com/skillberry-ai/skillberry-store.git", subdirectory = "client/python/skillberry_store_sdk" } [tool.poetry.dev-dependencies] pytest = ">= 7.2.1" diff --git a/shared/python/skillberry_agent_lib/skillberry_agent_lib/skillberry_store.py b/shared/python/skillberry_agent_lib/skillberry_agent_lib/skillberry_store.py index 1f5d196..a7263e3 100644 --- a/shared/python/skillberry_agent_lib/skillberry_agent_lib/skillberry_store.py +++ b/shared/python/skillberry_agent_lib/skillberry_agent_lib/skillberry_store.py @@ -73,7 +73,8 @@ def search_skills( similarity_threshold (float): Similarity threshold for the search Returns: - list: List of matching skills with name and similarity score + list: List of matching skills with filename, uuid, and similarity score + Each result contains: {'filename': str, 'uuid': str, 'similarity_score': float} Raises: Exception: Any failure occurred during execution @@ -110,7 +111,7 @@ def get_skill(self, skill_name: str): logger.info(f"get_skill called for skill: {skill_name}") try: - skill_data = self.skills_api.get_skill_skills_name_get(name=skill_name) + skill_data = self.skills_api.get_skill_skills_uuid_or_name_get(uuid_or_name=skill_name) logger.info(f"get_skill returned skill with UUID: {skill_data.get('uuid')}, name: {skill_data.get('name')}") logger.debug(f"Full skill data: {skill_data}") return skill_data @@ -118,6 +119,31 @@ def get_skill(self, skill_name: str): logger.error(f"Error retrieving skill '{skill_name}': {e}") raise Exception(f"Error retrieving skill: {str(e)}") + def get_skill_by_uuid(self, skill_uuid: str): + """ + Retrieve the skill with the given UUID. + + Parameters: + skill_uuid (str): The UUID of the skill + + Returns: + dict: The skill object with full details + + Raises: + Exception: Any failure occurred during execution + + """ + logger.info(f"get_skill_by_uuid called for UUID: {skill_uuid}") + + try: + skill_data = self.skills_api.get_skill_skills_uuid_or_name_get(uuid_or_name=skill_uuid) + logger.info(f"get_skill_by_uuid returned skill: {skill_data.get('name')}") + logger.debug(f"Full skill data: {skill_data}") + return skill_data + except ApiException as e: + logger.error(f"Error retrieving skill by UUID '{skill_uuid}': {e}") + raise Exception(f"Error retrieving skill: {str(e)}") + def find_skill_uuid_by_search(self, search_term: str): """ Find a skill UUID by searching for a skill matching the search term. @@ -139,18 +165,13 @@ def find_skill_uuid_by_search(self, search_term: str): logger.warning(f"No skills found matching search term: '{search_term}'") return None - # Get the first matching skill name + # Extract UUID directly from search result first_match = search_results[0] + skill_uuid = first_match.get("uuid") skill_name = first_match.get("filename") similarity_score = first_match.get("similarity_score", 0.0) - logger.info(f"Found matching skill: '{skill_name}' with similarity score: {similarity_score}") - - # Get the full skill details to retrieve UUID - skill_data = self.get_skill(skill_name) - skill_uuid = skill_data.get("uuid") - - logger.info(f"Retrieved skill UUID: {skill_uuid} for skill: '{skill_name}'") + logger.info(f"Found skill '{skill_name}' (UUID: {skill_uuid}) with similarity: {similarity_score}") return skill_uuid except Exception as e: @@ -214,7 +235,7 @@ def add_vmcp_server(self, name: str, description: str, skill_uuid: Optional[str] raise Exception(f"Error creating vmcp server: {str(e)}") def get_vmcp_server_details(self, name: str): - """Get detailed information about a virtual MCP server. + """Get detailed information about a virtual MCP server by name. Retrieves comprehensive details about the specified virtual MCP server, including its configuration, port, and available tools. @@ -231,14 +252,35 @@ def get_vmcp_server_details(self, name: str): logger.info(f"get_vmcp_server_details called for: {name}") try: - result = self.vmcp_api.get_vmcp_server_vmcp_servers_name_get(name=name) + result = self.vmcp_api.get_vmcp_server_vmcp_servers_uuid_or_name_get(uuid_or_name=name) return result except ApiException as e: logger.error(f"Error retrieving vmcp server '{name}': {e}") raise Exception(f"Error retrieving vmcp server: {str(e)}") + def get_vmcp_server_by_uuid(self, vmcp_uuid: str): + """Get detailed information about a virtual MCP server by UUID. + + Args: + vmcp_uuid: The UUID of the virtual MCP server. + + Returns: + dict: Detailed information about the virtual MCP server. + + Raises: + Exception: Any failure occurred during execution. + """ + logger.info(f"get_vmcp_server_by_uuid called for UUID: {vmcp_uuid}") + + try: + result = self.vmcp_api.get_vmcp_server_vmcp_servers_uuid_or_name_get(uuid_or_name=vmcp_uuid) + return result + except ApiException as e: + logger.error(f"Error retrieving vmcp server by UUID '{vmcp_uuid}': {e}") + raise Exception(f"Error retrieving vmcp server: {str(e)}") + def remove_vmcp_server(self, name: str): - """Remove a virtual MCP server + """Remove a virtual MCP server by name. Args: name: The name of the virtual MCP server to remove. @@ -253,12 +295,33 @@ def remove_vmcp_server(self, name: str): logger.info(f"remove_vmcp_server called for: {name}") try: - result = self.vmcp_api.delete_vmcp_server_vmcp_servers_name_delete(name=name) + result = self.vmcp_api.delete_vmcp_server_vmcp_servers_uuid_or_name_delete(uuid_or_name=name) return result except ApiException as e: logger.error(f"Error removing vmcp server '{name}': {e}") raise Exception(f"Error removing vmcp server: {str(e)}") + def remove_vmcp_server_by_uuid(self, vmcp_uuid: str): + """Remove a virtual MCP server by UUID. + + Args: + vmcp_uuid: The UUID of the virtual MCP server to remove. + + Returns: + dict: Success message + + Raises: + Exception: Any failure occurred during execution. + """ + logger.info(f"remove_vmcp_server_by_uuid called for UUID: {vmcp_uuid}") + + try: + result = self.vmcp_api.delete_vmcp_server_vmcp_servers_uuid_or_name_delete(uuid_or_name=vmcp_uuid) + return result + except ApiException as e: + logger.error(f"Error removing vmcp server by UUID '{vmcp_uuid}': {e}") + raise Exception(f"Error removing vmcp server: {str(e)}") + def get_mcp_tools(self, port: int, server_name: str = "skillberry-tools", tool_interceptors: Optional[List[Any]] = None) -> List[Any]: """Get tools from an MCP server via SSE transport. diff --git a/shared/python/skillberry_agent_lib/skillberry_agent_lib/test_vmcp_server_manager.py b/shared/python/skillberry_agent_lib/skillberry_agent_lib/test_vmcp_server_manager.py index 4d61b71..0f37f44 100644 --- a/shared/python/skillberry_agent_lib/skillberry_agent_lib/test_vmcp_server_manager.py +++ b/shared/python/skillberry_agent_lib/skillberry_agent_lib/test_vmcp_server_manager.py @@ -41,34 +41,36 @@ def test_create_new_server_success(self, mock_api): mock_api.get_vmcp_server_details.side_effect = [ Exception("Not found"), # First call - server doesn't exist { # Second call - after creation + "uuid": "a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d", "name": "vmcp-server-test-env", "description": "VMCP Server for env_id: test-env", "port": 8001, - "skill_uuid": "test-uuid-123", + "skill_uuid": "550e8400-e29b-41d4-a716-446655440000", "runtime": {"tools": ["tool1", "tool2"]} } ] mock_api.add_vmcp_server.return_value = { "name": "vmcp-server-test-env", - "uuid": "server-uuid-456", + "uuid": "a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d", "port": 8001 } # Create server context = {"env_id": "test-env"} - result = get_or_create_vmcp_server(context, skill_uuid="test-uuid-123") + result = get_or_create_vmcp_server(context, skill_uuid="550e8400-e29b-41d4-a716-446655440000") - # Verify result + # Verify result includes UUID + self.assertEqual(result["uuid"], "a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d") self.assertEqual(result["name"], "vmcp-server-test-env") self.assertEqual(result["port"], 8001) - self.assertEqual(result["skill_uuid"], "test-uuid-123") + self.assertEqual(result["skill_uuid"], "550e8400-e29b-41d4-a716-446655440000") self.assertEqual(result["env_id"], "test-env") # Verify API calls mock_api.add_vmcp_server.assert_called_once_with( name="vmcp-server-test-env", description="VMCP Server for env_id: test-env", - skill_uuid="test-uuid-123", + skill_uuid="550e8400-e29b-41d4-a716-446655440000", skillberry_context=context ) @@ -79,18 +81,22 @@ def test_reuse_existing_server(self, mock_api): mock_api.get_vmcp_server_details.side_effect = [ Exception("Not found"), { + "uuid": "b2c3d4e5-f6a7-4b8c-9d0e-1f2a3b4c5d6e", "name": "vmcp-server-test-env", "description": "VMCP Server for env_id: test-env", "port": 8001, - "skill_uuid": "test-uuid-123", + "skill_uuid": "550e8400-e29b-41d4-a716-446655440000", "runtime": {"tools": ["tool1"]} } ] - mock_api.add_vmcp_server.return_value = {"name": "vmcp-server-test-env"} + mock_api.add_vmcp_server.return_value = { + "name": "vmcp-server-test-env", + "uuid": "b2c3d4e5-f6a7-4b8c-9d0e-1f2a3b4c5d6e" + } # Create server first time context = {"env_id": "test-env"} - result1 = get_or_create_vmcp_server(context, skill_uuid="test-uuid-123") + result1 = get_or_create_vmcp_server(context, skill_uuid="550e8400-e29b-41d4-a716-446655440000") # Reset mock to verify no new calls mock_api.reset_mock() @@ -110,16 +116,17 @@ def test_server_already_exists_in_tools_service(self, mock_api): """Test when server already exists in Skillberry Tools Service.""" # Setup mock - server exists on first check mock_api.get_vmcp_server_details.return_value = { + "uuid": "c3d4e5f6-a7b8-4c9d-0e1f-2a3b4c5d6e7f", "name": "vmcp-server-test-env", "description": "Existing server", "port": 8001, - "skill_uuid": "test-uuid-123", + "skill_uuid": "550e8400-e29b-41d4-a716-446655440000", "runtime": {"tools": ["tool1", "tool2"]} } # Create server context = {"env_id": "test-env"} - result = get_or_create_vmcp_server(context, skill_uuid="test-uuid-123") + result = get_or_create_vmcp_server(context, skill_uuid="550e8400-e29b-41d4-a716-446655440000") # Verify result self.assertEqual(result["name"], "vmcp-server-test-env") @@ -149,7 +156,7 @@ def test_creation_failure_cleans_up_placeholder(self, mock_api): # Attempt to create server (should fail) with self.assertRaises(ValueError) as cm: - get_or_create_vmcp_server(context, skill_uuid="test-uuid") + get_or_create_vmcp_server(context, skill_uuid="550e8400-e29b-41d4-a716-446655440000") self.assertIn("VMCP server creation failed", str(cm.exception)) @@ -161,10 +168,11 @@ def test_skill_uuid_mismatch_warning(self, mock_api): """Test warning when existing server has different skill_uuid.""" # Setup mock - server exists with different skill_uuid mock_api.get_vmcp_server_details.return_value = { + "uuid": "d4e5f6a7-b8c9-4d0e-1f2a-3b4c5d6e7f8a", "name": "vmcp-server-test-env", "description": "Existing server", "port": 8001, - "skill_uuid": "different-uuid", + "skill_uuid": "d4e5f6a7-b8c9-4d0e-1f2a-3b4c5d6e7f8a", "runtime": {"tools": []} } @@ -172,13 +180,13 @@ def test_skill_uuid_mismatch_warning(self, mock_api): # Create server with different skill_uuid with self.assertLogs(level='WARNING') as log: - result = get_or_create_vmcp_server(context, skill_uuid="requested-uuid") + result = get_or_create_vmcp_server(context, skill_uuid="550e8400-e29b-41d4-a716-446655440000") # Verify warning was logged self.assertTrue(any("skill mismatch detected" in msg for msg in log.output)) # Verify existing server was reused - self.assertEqual(result["skill_uuid"], "different-uuid") + self.assertEqual(result["skill_uuid"], "d4e5f6a7-b8c9-4d0e-1f2a-3b4c5d6e7f8a") class TestGetOrCreateVmcpServerThreadSafety(unittest.TestCase): @@ -203,18 +211,22 @@ def test_concurrent_creation_same_env_id(self, mock_api): mock_api.get_vmcp_server_details.side_effect = [ Exception("Not found"), # First check { # After creation + "uuid": "e5f6a7b8-c9d0-4e1f-2a3b-4c5d6e7f8a9b", "name": "vmcp-server-test-env", "port": 8001, - "skill_uuid": "test-uuid", + "skill_uuid": "550e8400-e29b-41d4-a716-446655440000", "runtime": {"tools": []} } ] - mock_api.add_vmcp_server.return_value = {"name": "vmcp-server-test-env"} + mock_api.add_vmcp_server.return_value = { + "name": "vmcp-server-test-env", + "uuid": "e5f6a7b8-c9d0-4e1f-2a3b-4c5d6e7f8a9b" + } context = {"env_id": "test-env"} # First thread creates the server - result1 = get_or_create_vmcp_server(context, skill_uuid="test-uuid") + result1 = get_or_create_vmcp_server(context, skill_uuid="550e8400-e29b-41d4-a716-446655440000") self.assertIsNotNone(result1) self.assertEqual(result1["name"], "vmcp-server-test-env") diff --git a/shared/python/skillberry_agent_lib/skillberry_agent_lib/vmcp_server_manager.py b/shared/python/skillberry_agent_lib/skillberry_agent_lib/vmcp_server_manager.py index a746005..2f23dd3 100644 --- a/shared/python/skillberry_agent_lib/skillberry_agent_lib/vmcp_server_manager.py +++ b/shared/python/skillberry_agent_lib/skillberry_agent_lib/vmcp_server_manager.py @@ -139,6 +139,7 @@ def get_or_create_vmcp_server( # Step 4: Extract necessary fields for VirtualMcpServer vmcp_data = { + "uuid": vmcp_server_info.get("uuid"), # Store UUID for future operations "name": vmcp_server_info.get("name") or server_name, "description": vmcp_server_info.get("description") or f"VMCP Server for {env_id}", "port": vmcp_server_info.get("port"), @@ -201,22 +202,28 @@ def remove_vmcp_server(skillberry_context: Dict) -> bool: env_id = skillberry_context["env_id"] server_name = f"vmcp-server-{env_id}" registry_removed = False + vmcp_uuid = None - # Remove from local registry + # Remove from local registry and get UUID if available with _registry_lock: if env_id in _vmcp_server_registry: + vmcp_uuid = _vmcp_server_registry[env_id].get('uuid') del _vmcp_server_registry[env_id] logging.info(f"Removed VMCP server for env_id '{env_id}' from local registry") registry_removed = True else: logging.warning(f"No VMCP server found in local registry for env_id '{env_id}'") - # Remove from Skillberry Tools Service + # Remove from Skillberry Tools Service - prefer UUID-based removal try: - skillberry_store.remove_vmcp_server(name=server_name) - logging.info(f"Removed VMCP server '{server_name}' from Skillberry Tools Service") + if vmcp_uuid: + skillberry_store.remove_vmcp_server_by_uuid(vmcp_uuid) + logging.info(f"Removed VMCP server by UUID '{vmcp_uuid}' from Skillberry Tools Service") + else: + skillberry_store.remove_vmcp_server(name=server_name) + logging.info(f"Removed VMCP server by name '{server_name}' from Skillberry Tools Service") except Exception as e: - logging.warning(f"Failed to remove VMCP server '{server_name}' from Tools Service: {e}") + logging.warning(f"Failed to remove VMCP server from Tools Service: {e}") return registry_removed