chore: add env mcp config check & fix lint python

Author: ducphamle2

frontend/src/i18n/declaration.ts

@@ -301,6 +301,7 @@ export enum I18nKey {
   STATUS$ERROR_LLM_SERVICE_UNAVAILABLE = "STATUS$ERROR_LLM_SERVICE_UNAVAILABLE",
   STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR = "STATUS$ERROR_LLM_INTERNAL_SERVER_ERROR",
   STATUS$ERROR_LLM_OUT_OF_CREDITS = "STATUS$ERROR_LLM_OUT_OF_CREDITS",
+  STATUS$ERROR_LLM_CONTENT_POLICY_VIOLATION = "STATUS$ERROR_LLM_CONTENT_POLICY_VIOLATION",
   STATUS$ERROR_RUNTIME_DISCONNECTED = "STATUS$ERROR_RUNTIME_DISCONNECTED",
   STATUS$LLM_RETRY = "STATUS$LLM_RETRY",
   AGENT_ERROR$BAD_ACTION = "AGENT_ERROR$BAD_ACTION",

openhands/agenthub/codeact_agent/codeact_agent.py

@@ -1,8 +1,6 @@
 import os
 from collections import deque
 
-from litellm import ChatCompletionToolParam
-
 import openhands.agenthub.codeact_agent.function_calling as codeact_function_calling
 from openhands.controller.agent import Agent
 from openhands.controller.state.state import State
@@ -139,13 +137,13 @@ def step(self, state: State) -> Action:
             'messages': self.llm.format_messages_for_llm(messages),
         }
         params['tools'] = self.tools
-        
-        
+
         if self.mcp_tools:
             # Only add tools with unique names
             existing_names = {tool['function']['name'] for tool in params['tools']}
             unique_mcp_tools = [
-                tool for tool in self.mcp_tools 
+                tool
+                for tool in self.mcp_tools
                 if tool['function']['name'] not in existing_names
             ]
             params['tools'] += unique_mcp_tools

openhands/core/config/app_config.py

@@ -139,8 +139,3 @@ def model_post_init(self, __context):
         super().model_post_init(__context)
         if not AppConfig.defaults_dict:  # Only set defaults_dict if it's empty
             AppConfig.defaults_dict = model_defaults_to_dict(self)
-        # Validate MCP configurations
-        if self.mcp.sse.mcp_servers:
-            self.mcp.sse.validate_servers()
-        if self.mcp.stdio.commands:
-            self.mcp.stdio.validate_stdio()

openhands/core/config/mcp_config.py

@@ -37,16 +37,17 @@ class MCPStdioConfig(BaseModel):
     Attributes:
         commands: List of commands to run.
         args: List of arguments for each command.
+        envs: List of environment variable tuples for each command.
     """
 
     commands: List[str] = Field(default_factory=list)
     args: List[List[str]] = Field(default_factory=list)
     envs: List[List[tuple[str, str]]] = Field(default_factory=list)
-    
+
     model_config = {'extra': 'forbid'}
 
     def validate_stdio(self) -> None:
-        """Validate that commands and args are properly configured."""
+        """Validate that commands, args, and envs are properly configured."""
 
         # Check if number of commands matches number of args lists
         if len(self.commands) != len(self.args):
@@ -55,6 +56,26 @@ def validate_stdio(self) -> None:
                 f'number of args lists ({len(self.args)})'
             )
 
+        # Check if number of commands matches number of envs lists
+        if len(self.commands) != len(self.envs):
+            raise ValueError(
+                f'Number of commands ({len(self.commands)}) does not match '
+                f'number of envs lists ({len(self.envs)})'
+            )
+
+        # Validate each environment variable tuple
+        for i, env_list in enumerate(self.envs):
+            for j, env_tuple in enumerate(env_list):
+                if not isinstance(env_tuple, tuple) or len(env_tuple) != 2:
+                    raise ValueError(
+                        f'Environment variable at index {j} for command {i} must be a tuple of (key, value)'
+                    )
+                key, value = env_tuple
+                if not isinstance(key, str) or not isinstance(value, str):
+                    raise ValueError(
+                        f'Environment variable key and value at index {j} for command {i} must be strings'
+                    )
+
 
 class MCPConfig(BaseModel):
     """Configuration for MCP (Message Control Protocol) settings.

openhands/core/setup.py

@@ -1,7 +1,7 @@
 import hashlib
 import os
 import uuid
-from typing import Callable, List, Tuple, Type
+from typing import Callable, Tuple, Type
 
 from pydantic import SecretStr
 

openhands/integrations/github/github_service.py

@@ -100,7 +100,6 @@ async def get_user(self) -> User:
             email=response.get('email'),
         )
 
-
     async def _fetch_paginated_repos(
         self, url: str, params: dict, max_repos: int, extract_key: str | None = None
     ) -> list[dict]:

openhands/integrations/gitlab/gitlab_service.py

@@ -1,6 +1,5 @@
 import os
 from typing import Any
-from urllib.parse import quote_plus
 
 import httpx
 from pydantic import SecretStr

openhands/io/json.py

@@ -36,14 +36,14 @@ def dumps(obj, **kwargs):
     """Serialize an object to str format"""
     if not kwargs:
         return _json_encoder.encode(obj)
-    
+
     # Create a copy of the kwargs to avoid modifying the original
     encoder_kwargs = kwargs.copy()
-    
+
     # If cls is specified, use it; otherwise use our custom encoder
     if 'cls' not in encoder_kwargs:
         encoder_kwargs['cls'] = OpenHandsJSONEncoder
-    
+
     return json.dumps(obj, **encoder_kwargs)
 
 

openhands/mcp/__init__.py

@@ -1,3 +1,13 @@
-from openhands.mcp.mcp import MCPClient, convert_mcp_clients_to_tools, create_mcp_clients, BaseTool
+from openhands.mcp.mcp import (
+    BaseTool,
+    MCPClient,
+    convert_mcp_clients_to_tools,
+    create_mcp_clients,
+)
 
-__all__ = ['MCPClient', 'convert_mcp_clients_to_tools', 'create_mcp_clients', 'BaseTool']
+__all__ = [
+    'MCPClient',
+    'convert_mcp_clients_to_tools',
+    'create_mcp_clients',
+    'BaseTool',
+]

openhands/mcp/mcp.py

@@ -7,7 +7,6 @@
 from mcp.client.stdio import stdio_client
 from mcp.types import CallToolResult, TextContent, Tool
 from pydantic import BaseModel, Field
-from termcolor import colored
 
 from openhands.core.config.mcp_config import MCPConfig
 from openhands.core.logger import openhands_logger as logger
@@ -76,7 +75,7 @@ class Config:
 
     async def connect_sse(self, server_url: str, timeout: float = 30.0) -> None:
         """Connect to an MCP server using SSE transport.
-        
+
         Args:
             server_url: The URL of the SSE server to connect to.
             timeout: Connection timeout in seconds. Default is 30 seconds.
@@ -89,15 +88,19 @@ async def connect_sse(self, server_url: str, timeout: float = 30.0) -> None:
         try:
             import asyncio
             from asyncio import TimeoutError
-            
+
             # Create a task for the connection
-            connection_task = asyncio.create_task(self._connect_sse_internal(server_url))
-            
+            connection_task = asyncio.create_task(
+                self._connect_sse_internal(server_url)
+            )
+
             # Wait for the connection with timeout
             try:
                 await asyncio.wait_for(connection_task, timeout=timeout)
             except TimeoutError:
-                logger.error(f'Connection to {server_url} timed out after {timeout} seconds')
+                logger.error(
+                    f'Connection to {server_url} timed out after {timeout} seconds'
+                )
                 # Cancel the connection task
                 connection_task.cancel()
                 try:
@@ -110,7 +113,7 @@ async def connect_sse(self, server_url: str, timeout: float = 30.0) -> None:
     async def _connect_sse_internal(self, server_url: str) -> None:
         """Internal method to establish SSE connection."""
         streams_context = sse_client(
-            url=server_url, 
+            url=server_url,
         )
         streams = await self.exit_stack.enter_async_context(streams_context)
         self.session = await self.exit_stack.enter_async_context(
@@ -119,9 +122,15 @@ async def _connect_sse_internal(self, server_url: str) -> None:
 
         await self._initialize_and_list_tools()
 
-    async def connect_stdio(self, command: str, args: List[str], envs: List[tuple[str, str]], timeout: float = 30.0) -> None:
+    async def connect_stdio(
+        self,
+        command: str,
+        args: List[str],
+        envs: List[tuple[str, str]],
+        timeout: float = 30.0,
+    ) -> None:
         """Connect to an MCP server using stdio transport.
-        
+
         Args:
             command: The command to execute.
             args: The arguments to pass to the command.
@@ -136,15 +145,19 @@ async def connect_stdio(self, command: str, args: List[str], envs: List[tuple[st
         try:
             import asyncio
             from asyncio import TimeoutError
-            
+
             # Create a task for the connection
-            connection_task = asyncio.create_task(self._connect_stdio_internal(command, args, envs))
-            
+            connection_task = asyncio.create_task(
+                self._connect_stdio_internal(command, args, envs)
+            )
+
             # Wait for the connection with timeout
             try:
                 await asyncio.wait_for(connection_task, timeout=timeout)
             except TimeoutError:
-                logger.error(f'Connection to {command} timed out after {timeout} seconds')
+                logger.error(
+                    f'Connection to {command} timed out after {timeout} seconds'
+                )
                 # Cancel the connection task
                 connection_task.cancel()
                 try:
@@ -154,7 +167,9 @@ async def connect_stdio(self, command: str, args: List[str], envs: List[tuple[st
         except Exception as e:
             logger.error(f'Error connecting to {command}: {str(e)}')
 
-    async def _connect_stdio_internal(self, command: str, args: List[str], envs: List[tuple[str, str]]) -> None:
+    async def _connect_stdio_internal(
+        self, command: str, args: List[str], envs: List[tuple[str, str]]
+    ) -> None:
         """Internal method to establish stdio connection."""
         envs_dict: dict[str, str] = {}
         for env in envs:
@@ -249,7 +264,10 @@ def convert_mcp_clients_to_tools(mcp_clients: list[MCPClient] | None) -> list[di
 
 
 async def create_mcp_clients(
-    sse_mcp_server: List[str], commands: List[str], args: List[List[str]], envs: List[List[tuple[str, str]]]
+    sse_mcp_server: List[str],
+    commands: List[str],
+    args: List[List[str]],
+    envs: List[List[tuple[str, str]]],
 ) -> List[MCPClient]:
     mcp_clients: List[MCPClient] = []
     # Initialize SSE connections
@@ -271,11 +289,15 @@ async def create_mcp_clients(
                 try:
                     await client.disconnect()
                 except Exception as disconnect_error:
-                    logger.error(f'Error during disconnect after failed connection: {str(disconnect_error)}')
+                    logger.error(
+                        f'Error during disconnect after failed connection: {str(disconnect_error)}'
+                    )
 
     # Initialize stdio connections
     if commands:
-        for i, (command, command_args, command_envs) in enumerate(zip(commands, args, envs)):
+        for i, (command, command_args, command_envs) in enumerate(
+            zip(commands, args, envs)
+        ):
             logger.info(
                 f'Initializing MCP agent for {command} with stdio connection...'
             )
@@ -292,39 +314,45 @@ async def create_mcp_clients(
                 try:
                     await client.disconnect()
                 except Exception as disconnect_error:
-                    logger.error(f'Error during disconnect after failed connection: {str(disconnect_error)}')
+                    logger.error(
+                        f'Error during disconnect after failed connection: {str(disconnect_error)}'
+                    )
 
     return mcp_clients
 
+
 async def fetch_mcp_tools_from_config(mcp_config: MCPConfig) -> list[dict]:
     """
     Retrieves the list of MCP tools from the MCP clients.
-    
+
     Returns:
         A list of tool dictionaries. Returns an empty list if no connections could be established.
     """
     mcp_clients = []
     mcp_tools = []
-    
+
     try:
         mcp_clients = await create_mcp_clients(
-            mcp_config.sse.mcp_servers, mcp_config.stdio.commands, mcp_config.stdio.args, mcp_config.stdio.envs
+            mcp_config.sse.mcp_servers,
+            mcp_config.stdio.commands,
+            mcp_config.stdio.args,
+            mcp_config.stdio.envs,
         )
-        
+
         if not mcp_clients:
-            logger.warning("No MCP clients were successfully connected")
+            logger.warning('No MCP clients were successfully connected')
             return []
-            
+
         mcp_tools = convert_mcp_clients_to_tools(mcp_clients)
     except Exception as e:
-        logger.error(f"Error fetching MCP tools: {str(e)}")
+        logger.error(f'Error fetching MCP tools: {str(e)}')
         return []
     finally:
         # Always disconnect clients to clean up resources
         for mcp_client in mcp_clients:
             try:
                 await mcp_client.disconnect()
             except Exception as disconnect_error:
-                logger.error(f"Error disconnecting MCP client: {str(disconnect_error)}")
-    
+                logger.error(f'Error disconnecting MCP client: {str(disconnect_error)}')
+
     return mcp_tools

openhands/runtime/action_execution_server.py

@@ -74,7 +74,9 @@
 class ActionRequest(BaseModel):
     action: dict
     sse_mcp_config: Optional[list[str]] = None
-    stdio_mcp_config: Optional[tuple[list[str], list[list[str]], list[list[tuple[str, str]]]]] = None
+    stdio_mcp_config: Optional[
+        tuple[list[str], list[list[str]], list[list[tuple[str, str]]]]
+    ] = None
 
 
 ROOT_GID = 0
@@ -190,7 +192,9 @@ def __init__(
         )
         self.memory_monitor.start_monitoring()
         self.sse_mcp_servers: list[str] = []
-        self.stdio_mcp_config: tuple[list[str], list[list[str]], list[list[tuple[str, str]]]] = ([], [], [])
+        self.stdio_mcp_config: tuple[
+            list[str], list[list[str]], list[list[tuple[str, str]]]
+        ] = ([], [], [])
 
     @property
     def initial_cwd(self):

openhands/server/routes/git.py

@@ -1,7 +1,7 @@
 from fastapi import APIRouter, Depends, status
 from fastapi.responses import JSONResponse
 from pydantic import SecretStr
-from openhands.server.shared import server_config
+
 from openhands.integrations.github.github_service import GithubServiceImpl
 from openhands.integrations.provider import (
     PROVIDER_TOKEN_TYPE,
@@ -16,7 +16,7 @@
     User,
 )
 from openhands.server.auth import get_access_token, get_provider_tokens
-from openhands.server.types import AppMode
+from openhands.server.shared import server_config
 
 app = APIRouter(prefix='/api/user')
 
@@ -33,7 +33,9 @@ async def get_user_repositories(
         )
 
         try:
-            repos: list[Repository] = await client.get_repositories(sort, server_config.app_mode)
+            repos: list[Repository] = await client.get_repositories(
+                sort, server_config.app_mode
+            )
             return repos
 
         except AuthenticationError as e:

openhands/server/routes/manage_conversations.py

@@ -112,7 +112,9 @@ async def _create_new_conversation(
             title=conversation_title,
             user_id=user_id,
             github_user_id=None,
-            selected_repository=selected_repository.full_name if selected_repository else selected_repository,
+            selected_repository=selected_repository.full_name
+            if selected_repository
+            else selected_repository,
             selected_branch=selected_branch,
         )
     )