From 16004426a2cfd0980f5373ec54abe7cfd2330e14 Mon Sep 17 00:00:00 2001 From: Tejas Goyal <83608316+tejas-goyal@users.noreply.github.com> Date: Wed, 24 Sep 2025 08:43:54 -0400 Subject: [PATCH] feat: Add configurable timeouts for MCP tool invocations (Good first issues: #10684) (#11029) Co-authored-by: Tejas Goyal --- config.template.toml | 41 ++++++ docs/usage/mcp.mdx | 46 +++++++ frontend/public/mockServiceWorker.js | 2 +- .../settings/mcp-settings/mcp-server-form.tsx | 55 +++++++- .../mcp-settings/mcp-server-list-item.tsx | 1 + .../settings/mcp-settings/mcp-server-list.tsx | 1 + .../src/hooks/mutation/use-add-mcp-server.ts | 2 + .../hooks/mutation/use-update-mcp-server.ts | 2 + frontend/src/i18n/declaration.ts | 3 + frontend/src/i18n/translation.json | 48 +++++++ frontend/src/routes/mcp-settings.tsx | 2 + frontend/src/types/settings.ts | 1 + openhands/core/config/mcp_config.py | 20 +++ openhands/mcp/client.py | 30 ++++- openhands/mcp/utils.py | 22 ++++ .../core/config/test_shttp_timeout_config.py | 122 ++++++++++++++++++ 16 files changed, 391 insertions(+), 7 deletions(-) create mode 100644 tests/unit/core/config/test_shttp_timeout_config.py diff --git a/config.template.toml b/config.template.toml index 542a3c2e71..e7b7836dcd 100644 --- a/config.template.toml +++ b/config.template.toml @@ -489,6 +489,47 @@ type = "noop" # Run the runtime sandbox container in privileged mode for use with docker-in-docker #privileged = false +#################################### MCP ##################################### +# Configuration for Model Context Protocol (MCP) servers +# MCP allows OpenHands to communicate with external tool servers +############################################################################## +[mcp] +# SSE servers - Server-Sent Events transport (legacy) +#sse_servers = [ +# # Basic SSE server with just a URL +# "http://localhost:8080/mcp/sse", +# +# # SSE server with authentication +# {url = "https://api.example.com/mcp/sse", api_key = "your-api-key"} +#] + +# SHTTP servers - Streamable HTTP transport (recommended) +#shttp_servers = [ +# # Basic SHTTP server with default 60s timeout +# "https://api.example.com/mcp/shttp", +# +# # SHTTP server with custom timeout for long-running tools +# { +# url = "https://api.example.com/mcp/shttp", +# api_key = "your-api-key", +# timeout = 180 # 3 minutes for processing-heavy tools (1-3600 seconds) +# } +#] + +# Stdio servers - Direct process communication (development only) +#stdio_servers = [ +# # Basic stdio server +# {name = "filesystem", command = "npx", args = ["@modelcontextprotocol/server-filesystem", "/"]}, +# +# # Stdio server with environment variables +# { +# name = "fetch", +# command = "uvx", +# args = ["mcp-server-fetch"], +# env = {DEBUG = "true"} +# } +#] + #################################### Model Routing ############################ # Configuration for experimental model routing feature # Enables intelligent switching between different LLM models for specific purposes diff --git a/docs/usage/mcp.mdx b/docs/usage/mcp.mdx index 78593fed42..f41539d1f6 100644 --- a/docs/usage/mcp.mdx +++ b/docs/usage/mcp.mdx @@ -67,6 +67,19 @@ sse_servers = [ # External MCP service with authentication {url="https://api.example.com/mcp/sse", api_key="your-api-key"} ] + +# SHTTP Servers - Modern streamable HTTP transport (recommended) +shttp_servers = [ + # Basic SHTTP server with default 60s timeout + "https://api.example.com/mcp/shttp", + + # Server with custom timeout for heavy operations + { + url = "https://files.example.com/mcp/shttp", + api_key = "your-api-key", + timeout = 1800 # 30 minutes for large file processing + } +] ``` @@ -118,6 +131,17 @@ SHTTP (Streamable HTTP) servers are configured using either a string URL or an o - Type: `str` - Description: API key for authentication +- `timeout` (optional) + - Type: `int` + - Default: `60` + - Range: `1-3600` seconds (1 hour maximum) + - Description: Timeout in seconds for tool execution. This prevents tool calls from hanging indefinitely. + - **Use Cases:** + - **Short timeout (1-30s)**: For lightweight operations like status checks or simple queries + - **Medium timeout (30-300s)**: For standard processing tasks like data analysis or API calls + - **Long timeout (300-3600s)**: For heavy operations like file processing, complex calculations, or batch operations + - **Note**: This timeout only applies to individual tool calls, not server connection establishment. + ### Stdio Servers **Note**: While stdio servers are supported, we recommend using MCP proxies (see above) for better reliability and performance. @@ -192,5 +216,27 @@ SHTTP is the modern HTTP-based transport protocol that provides enhanced feature SHTTP is the recommended transport for HTTP-based MCP servers as it provides better reliability and features compared to the legacy SSE transport. +#### SHTTP Timeout Best Practices + +When configuring SHTTP timeouts, consider these guidelines: + +**Timeout Selection:** +- **Database queries**: 30-60 seconds +- **File operations**: 60-300 seconds (depending on file size) +- **Web scraping**: 60-120 seconds +- **Complex calculations**: 300-1800 seconds +- **Batch processing**: 1800-3600 seconds (maximum) + +**Error Handling:** +When a tool call exceeds the configured timeout: +- The operation is cancelled with an `asyncio.TimeoutError` +- The agent receives a timeout error message +- The server connection remains active for subsequent requests + +**Monitoring:** +- Set timeouts based on your tool's actual performance characteristics +- Monitor timeout occurrences to optimize timeout values +- Consider implementing server-side timeout handling for graceful degradation + ### Standard Input/Output (stdio) Stdio transport enables communication through standard input and output streams, making it ideal for local integrations and command-line tools. This transport is used for locally executed MCP servers that run as separate processes. diff --git a/frontend/public/mockServiceWorker.js b/frontend/public/mockServiceWorker.js index 723b0714cd..7e23102e0b 100644 --- a/frontend/public/mockServiceWorker.js +++ b/frontend/public/mockServiceWorker.js @@ -7,7 +7,7 @@ * - Please do NOT modify this file. */ -const PACKAGE_VERSION = '2.10.5' +const PACKAGE_VERSION = '2.11.1' const INTEGRITY_CHECKSUM = 'f5825c521429caf22a4dd13b66e243af' const IS_MOCKED_RESPONSE = Symbol('isMockedResponse') const activeClientIds = new Set() diff --git a/frontend/src/components/features/settings/mcp-settings/mcp-server-form.tsx b/frontend/src/components/features/settings/mcp-settings/mcp-server-form.tsx index 09ab498fc1..2e6367896a 100644 --- a/frontend/src/components/features/settings/mcp-settings/mcp-server-form.tsx +++ b/frontend/src/components/features/settings/mcp-settings/mcp-server-form.tsx @@ -15,6 +15,7 @@ interface MCPServerConfig { name?: string; url?: string; api_key?: string; + timeout?: number; command?: string; args?: string[]; env?: Record; @@ -120,6 +121,22 @@ export function MCPServerForm({ return null; }; + const validateTimeout = (timeoutStr: string): string | null => { + if (!timeoutStr.trim()) return null; // Optional field + + const timeout = parseInt(timeoutStr.trim(), 10); + if (Number.isNaN(timeout)) { + return t(I18nKey.SETTINGS$MCP_ERROR_TIMEOUT_INVALID_NUMBER); + } + if (timeout <= 0) { + return t(I18nKey.SETTINGS$MCP_ERROR_TIMEOUT_POSITIVE); + } + if (timeout > 3600) { + return t(I18nKey.SETTINGS$MCP_ERROR_TIMEOUT_MAX_EXCEEDED); + } + return null; + }; + const validateStdioServer = (formData: FormData): string | null => { const name = formData.get("name")?.toString().trim() || ""; const command = formData.get("command")?.toString().trim() || ""; @@ -148,6 +165,14 @@ export function MCPServerForm({ if (urlError) return urlError; const urlDupError = validateUrlUniqueness(url); if (urlDupError) return urlDupError; + + // Validate timeout for SHTTP servers only + if (serverType === "shttp") { + const timeoutStr = formData.get("timeout")?.toString() || ""; + const timeoutError = validateTimeout(timeoutStr); + if (timeoutError) return timeoutError; + } + return null; } @@ -203,12 +228,23 @@ export function MCPServerForm({ if (serverType === "sse" || serverType === "shttp") { const url = formData.get("url")?.toString().trim(); const apiKey = formData.get("api_key")?.toString().trim(); + const timeoutStr = formData.get("timeout")?.toString().trim(); - onSubmit({ + const serverConfig: MCPServerConfig = { ...baseConfig, url: url!, ...(apiKey && { api_key: apiKey }), - }); + }; + + // Only add timeout for SHTTP servers + if (serverType === "shttp" && timeoutStr) { + const timeoutValue = parseInt(timeoutStr, 10); + if (!Number.isNaN(timeoutValue)) { + serverConfig.timeout = timeoutValue; + } + } + + onSubmit(serverConfig); } else if (serverType === "stdio") { const name = formData.get("name")?.toString().trim(); const command = formData.get("command")?.toString().trim(); @@ -283,6 +319,21 @@ export function MCPServerForm({ defaultValue={server?.api_key || ""} placeholder={t(I18nKey.SETTINGS$MCP_API_KEY_PLACEHOLDER)} /> + + {serverType === "shttp" && ( + + )} )} diff --git a/frontend/src/components/features/settings/mcp-settings/mcp-server-list-item.tsx b/frontend/src/components/features/settings/mcp-settings/mcp-server-list-item.tsx index 509ea1c853..d86e835129 100644 --- a/frontend/src/components/features/settings/mcp-settings/mcp-server-list-item.tsx +++ b/frontend/src/components/features/settings/mcp-settings/mcp-server-list-item.tsx @@ -8,6 +8,7 @@ interface MCPServerConfig { name?: string; url?: string; api_key?: string; + timeout?: number; command?: string; args?: string[]; env?: Record; diff --git a/frontend/src/components/features/settings/mcp-settings/mcp-server-list.tsx b/frontend/src/components/features/settings/mcp-settings/mcp-server-list.tsx index 93018ca0f5..85ce5747d4 100644 --- a/frontend/src/components/features/settings/mcp-settings/mcp-server-list.tsx +++ b/frontend/src/components/features/settings/mcp-settings/mcp-server-list.tsx @@ -8,6 +8,7 @@ interface MCPServerConfig { name?: string; url?: string; api_key?: string; + timeout?: number; command?: string; args?: string[]; env?: Record; diff --git a/frontend/src/hooks/mutation/use-add-mcp-server.ts b/frontend/src/hooks/mutation/use-add-mcp-server.ts index 4c722cdd37..92725cbe58 100644 --- a/frontend/src/hooks/mutation/use-add-mcp-server.ts +++ b/frontend/src/hooks/mutation/use-add-mcp-server.ts @@ -10,6 +10,7 @@ interface MCPServerConfig { name?: string; url?: string; api_key?: string; + timeout?: number; command?: string; args?: string[]; env?: Record; @@ -49,6 +50,7 @@ export function useAddMcpServer() { const shttpServer: MCPSHTTPServer = { url: server.url!, ...(server.api_key && { api_key: server.api_key }), + ...(server.timeout !== undefined && { timeout: server.timeout }), }; newConfig.shttp_servers.push(shttpServer); } diff --git a/frontend/src/hooks/mutation/use-update-mcp-server.ts b/frontend/src/hooks/mutation/use-update-mcp-server.ts index b727597baa..83ef5dfcf3 100644 --- a/frontend/src/hooks/mutation/use-update-mcp-server.ts +++ b/frontend/src/hooks/mutation/use-update-mcp-server.ts @@ -10,6 +10,7 @@ interface MCPServerConfig { name?: string; url?: string; api_key?: string; + timeout?: number; command?: string; args?: string[]; env?: Record; @@ -51,6 +52,7 @@ export function useUpdateMcpServer() { const shttpServer: MCPSHTTPServer = { url: server.url!, ...(server.api_key && { api_key: server.api_key }), + ...(server.timeout !== undefined && { timeout: server.timeout }), }; newConfig.shttp_servers[index] = shttpServer; } diff --git a/frontend/src/i18n/declaration.ts b/frontend/src/i18n/declaration.ts index bbbd083662..92bd91dc12 100644 --- a/frontend/src/i18n/declaration.ts +++ b/frontend/src/i18n/declaration.ts @@ -806,6 +806,9 @@ export enum I18nKey { SETTINGS$MCP_ERROR_COMMAND_NO_SPACES = "SETTINGS$MCP_ERROR_COMMAND_NO_SPACES", SETTINGS$MCP_ERROR_URL_DUPLICATE = "SETTINGS$MCP_ERROR_URL_DUPLICATE", SETTINGS$MCP_ERROR_ENV_INVALID_FORMAT = "SETTINGS$MCP_ERROR_ENV_INVALID_FORMAT", + SETTINGS$MCP_ERROR_TIMEOUT_INVALID_NUMBER = "SETTINGS$MCP_ERROR_TIMEOUT_INVALID_NUMBER", + SETTINGS$MCP_ERROR_TIMEOUT_POSITIVE = "SETTINGS$MCP_ERROR_TIMEOUT_POSITIVE", + SETTINGS$MCP_ERROR_TIMEOUT_MAX_EXCEEDED = "SETTINGS$MCP_ERROR_TIMEOUT_MAX_EXCEEDED", SETTINGS$MCP_SERVER_TYPE = "SETTINGS$MCP_SERVER_TYPE", SETTINGS$MCP_API_KEY_PLACEHOLDER = "SETTINGS$MCP_API_KEY_PLACEHOLDER", SETTINGS$MCP_COMMAND_ARGUMENTS = "SETTINGS$MCP_COMMAND_ARGUMENTS", diff --git a/frontend/src/i18n/translation.json b/frontend/src/i18n/translation.json index 094ee088f2..972aef07e7 100644 --- a/frontend/src/i18n/translation.json +++ b/frontend/src/i18n/translation.json @@ -12895,6 +12895,54 @@ "de": "Environment variables must follow KEY=value format", "uk": "Environment variables must follow KEY=value format" }, + "SETTINGS$MCP_ERROR_TIMEOUT_INVALID_NUMBER": { + "en": "Timeout must be a valid number", + "ja": "タイムアウトは有効な数値である必要があります", + "zh-CN": "超时必须是有效数字", + "zh-TW": "超時必須是有效數字", + "ko-KR": "타임아웃은 유효한 숫자여야 합니다", + "no": "Timeout må være et gyldig tall", + "it": "Il timeout deve essere un numero valido", + "pt": "O timeout deve ser um número válido", + "es": "El timeout debe ser un número válido", + "ar": "يجب أن يكون المهلة الزمنية رقمًا صالحًا", + "fr": "Le timeout doit être un nombre valide", + "tr": "Zaman aşımı geçerli bir sayı olmalıdır", + "de": "Timeout muss eine gültige Zahl sein", + "uk": "Таймаут повинен бути дійсним числом" + }, + "SETTINGS$MCP_ERROR_TIMEOUT_POSITIVE": { + "en": "Timeout must be positive", + "ja": "タイムアウトは正の値である必要があります", + "zh-CN": "超时必须为正数", + "zh-TW": "超時必須為正數", + "ko-KR": "타임아웃은 양수여야 합니다", + "no": "Timeout må være positiv", + "it": "Il timeout deve essere positivo", + "pt": "O timeout deve ser positivo", + "es": "El timeout debe ser positivo", + "ar": "يجب أن تكون المهلة الزمنية موجبة", + "fr": "Le timeout doit être positif", + "tr": "Zaman aşımı pozitif olmalıdır", + "de": "Timeout muss positiv sein", + "uk": "Таймаут повинен бути позитивним" + }, + "SETTINGS$MCP_ERROR_TIMEOUT_MAX_EXCEEDED": { + "en": "Timeout cannot exceed 3600 seconds (1 hour)", + "ja": "タイムアウトは3600秒（1時間）を超えることはできません", + "zh-CN": "超时不能超过3600秒（1小时）", + "zh-TW": "超時不能超過3600秒（1小時）", + "ko-KR": "타임아웃은 3600초(1시간)을 초과할 수 없습니다", + "no": "Timeout kan ikke overstige 3600 sekunder (1 time)", + "it": "Il timeout non può superare 3600 secondi (1 ora)", + "pt": "O timeout não pode exceder 3600 segundos (1 hora)", + "es": "El timeout no puede exceder 3600 segundos (1 hora)", + "ar": "لا يمكن أن تتجاوز المهلة الزمنية 3600 ثانية (ساعة واحدة)", + "fr": "Le timeout ne peut pas dépasser 3600 secondes (1 heure)", + "tr": "Zaman aşımı 3600 saniyeyi (1 saat) aşamaz", + "de": "Timeout kann 3600 Sekunden (1 Stunde) nicht überschreiten", + "uk": "Таймаут не може перевищувати 3600 секунд (1 година)" + }, "SETTINGS$MCP_SERVER_TYPE": { "en": "Server Type", "ja": "サーバータイプ", diff --git a/frontend/src/routes/mcp-settings.tsx b/frontend/src/routes/mcp-settings.tsx index 47e372609c..dce469cc99 100644 --- a/frontend/src/routes/mcp-settings.tsx +++ b/frontend/src/routes/mcp-settings.tsx @@ -20,6 +20,7 @@ interface MCPServerConfig { name?: string; url?: string; api_key?: string; + timeout?: number; command?: string; args?: string[]; env?: Record; @@ -67,6 +68,7 @@ function MCPSettingsScreen() { type: "shttp" as const, url: typeof server === "string" ? server : server.url, api_key: typeof server === "object" ? server.api_key : undefined, + timeout: typeof server === "object" ? server.timeout : undefined, })), ]; diff --git a/frontend/src/types/settings.ts b/frontend/src/types/settings.ts index c82187389f..1d9a542705 100644 --- a/frontend/src/types/settings.ts +++ b/frontend/src/types/settings.ts @@ -27,6 +27,7 @@ export type MCPStdioServer = { export type MCPSHTTPServer = { url: string; api_key?: string; + timeout?: number; }; export type MCPConfig = { diff --git a/openhands/core/config/mcp_config.py b/openhands/core/config/mcp_config.py index d2f7328909..dd25318d37 100644 --- a/openhands/core/config/mcp_config.py +++ b/openhands/core/config/mcp_config.py @@ -189,8 +189,17 @@ class MCPStdioServerConfig(BaseModel): class MCPSHTTPServerConfig(BaseModel): + """Configuration for a MCP server that uses SHTTP. + + Attributes: + url: The server URL + api_key: Optional API key for authentication + timeout: Timeout in seconds for tool calls (default: 60s) + """ + url: str api_key: str | None = None + timeout: int | None = 60 @field_validator('url', mode='before') @classmethod @@ -198,6 +207,17 @@ class MCPSHTTPServerConfig(BaseModel): """Validate URL format for MCP servers.""" return _validate_mcp_url(v) + @field_validator('timeout') + @classmethod + def validate_timeout(cls, v: int | None) -> int | None: + """Validate timeout value for MCP tool calls.""" + if v is not None: + if v <= 0: + raise ValueError('Timeout must be positive') + if v > 3600: # 1 hour max + raise ValueError('Timeout cannot exceed 3600 seconds') + return v + class MCPConfig(BaseModel): """Configuration for MCP (Message Control Protocol) settings. diff --git a/openhands/mcp/client.py b/openhands/mcp/client.py index 197ed3c390..5fc19a9062 100644 --- a/openhands/mcp/client.py +++ b/openhands/mcp/client.py @@ -1,3 +1,4 @@ +import asyncio from typing import Optional from fastmcp import Client @@ -29,6 +30,7 @@ class MCPClient(BaseModel): description: str = 'MCP client tools for server interaction' tools: list[MCPClientTool] = Field(default_factory=list) tool_map: dict[str, MCPClientTool] = Field(default_factory=dict) + server_timeout: Optional[float] = None # Timeout from server config for tool calls async def _initialize_and_list_tools(self) -> None: """Initialize session and populate tool map.""" @@ -60,7 +62,7 @@ class MCPClient(BaseModel): conversation_id: str | None = None, timeout: float = 30.0, ): - """Connect to MCP server using SHTTP or SSE transport""" + """Connect to MCP server using SHTTP or SSE transport.""" server_url = server.url api_key = server.api_key @@ -123,7 +125,7 @@ class MCPClient(BaseModel): raise async def connect_stdio(self, server: MCPStdioServerConfig, timeout: float = 30.0): - """Connect to MCP server using stdio transport""" + """Connect to MCP server using stdio transport.""" try: transport = StdioTransport( command=server.command, args=server.args or [], env=server.env @@ -145,7 +147,20 @@ class MCPClient(BaseModel): raise async def call_tool(self, tool_name: str, args: dict) -> CallToolResult: - """Call a tool on the MCP server.""" + """Call a tool on the MCP server with timeout from server configuration. + + Args: + tool_name: Name of the tool to call + args: Arguments to pass to the tool + + Returns: + CallToolResult from the MCP server + + Raises: + asyncio.TimeoutError: If the tool call times out + ValueError: If the tool is not found + RuntimeError: If the client session is not available + """ if tool_name not in self.tool_map: raise ValueError(f'Tool {tool_name} not found.') # The MCPClientTool is primarily for metadata; use the session to call the actual tool. @@ -153,4 +168,11 @@ class MCPClient(BaseModel): raise RuntimeError('Client session is not available.') async with self.client: - return await self.client.call_tool_mcp(name=tool_name, arguments=args) + # Use server timeout if configured + if self.server_timeout is not None: + return await asyncio.wait_for( + self.client.call_tool_mcp(name=tool_name, arguments=args), + timeout=self.server_timeout, + ) + else: + return await self.client.call_tool_mcp(name=tool_name, arguments=args) diff --git a/openhands/mcp/utils.py b/openhands/mcp/utils.py index 83e88cfa2c..cf6a8fca1d 100644 --- a/openhands/mcp/utils.py +++ b/openhands/mcp/utils.py @@ -1,3 +1,4 @@ +import asyncio import json import shutil from typing import TYPE_CHECKING @@ -128,6 +129,11 @@ async def create_mcp_clients( ) client = MCPClient() + # Set server timeout for SHTTP servers + if isinstance(server, MCPSHTTPServerConfig) and server.timeout is not None: + client.server_timeout = float(server.timeout) + logger.debug(f'Set SHTTP server timeout to {server.timeout}s') + try: await client.connect_http(server, conversation_id=conversation_id) @@ -253,6 +259,22 @@ async def call_tool_mcp(mcp_clients: list[MCPClient], action: MCPAction) -> Obse name=action.name, arguments=action.arguments, ) + except asyncio.TimeoutError: + # Handle timeout errors specifically + timeout_val = getattr(matching_client, 'server_timeout', 'unknown') + logger.error(f'MCP tool {action.name} timed out after {timeout_val}s') + error_content = json.dumps( + { + 'isError': True, + 'error': f'Tool "{action.name}" timed out after {timeout_val} seconds', + 'content': [], + } + ) + return MCPObservation( + content=error_content, + name=action.name, + arguments=action.arguments, + ) except McpError as e: # Handle MCP errors by returning an error observation instead of raising logger.error(f'MCP error when calling tool {action.name}: {e}') diff --git a/tests/unit/core/config/test_shttp_timeout_config.py b/tests/unit/core/config/test_shttp_timeout_config.py new file mode 100644 index 0000000000..6de919419f --- /dev/null +++ b/tests/unit/core/config/test_shttp_timeout_config.py @@ -0,0 +1,122 @@ +"""Test MCP SHTTP server timeout configuration.""" + +import pytest +from pydantic import ValidationError + +from openhands.core.config.mcp_config import MCPSHTTPServerConfig + + +class TestMCPSHTTPServerConfig: + """Test SHTTP server configuration with timeout field.""" + + def test_shttp_config_with_timeout(self): + """Test SHTTP config accepts timeout parameter.""" + config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=90) + assert config.timeout == 90 + assert config.url == 'https://api.example.com/mcp' + assert config.api_key is None + + def test_shttp_config_with_api_key_and_timeout(self): + """Test SHTTP config with both API key and timeout.""" + config = MCPSHTTPServerConfig( + url='https://api.example.com/mcp', api_key='test-key-123', timeout=120 + ) + assert config.timeout == 120 + assert config.api_key == 'test-key-123' + + def test_shttp_config_default_timeout(self): + """Test SHTTP config uses default timeout when not specified.""" + config = MCPSHTTPServerConfig(url='https://api.example.com/mcp') + assert config.timeout == 60 # Default value + + def test_shttp_config_none_timeout(self): + """Test SHTTP config accepts None timeout.""" + config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=None) + assert config.timeout is None + + def test_timeout_validation_positive_values(self): + """Test timeout validation accepts positive values.""" + # Test various valid timeout values + valid_timeouts = [1, 5, 30, 60, 120, 300, 600, 1800, 3600] + + for timeout in valid_timeouts: + config = MCPSHTTPServerConfig( + url='https://api.example.com/mcp', timeout=timeout + ) + assert config.timeout == timeout + + def test_timeout_validation_zero_raises_error(self): + """Test timeout validation rejects zero timeout.""" + with pytest.raises(ValidationError) as exc_info: + MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=0) + assert 'Timeout must be positive' in str(exc_info.value) + + def test_timeout_validation_negative_raises_error(self): + """Test timeout validation rejects negative timeout.""" + with pytest.raises(ValidationError) as exc_info: + MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=-10) + assert 'Timeout must be positive' in str(exc_info.value) + + def test_timeout_validation_max_limit(self): + """Test timeout validation enforces maximum limit.""" + # Test exactly at the limit + config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=3600) + assert config.timeout == 3600 + + # Test exceeding the limit + with pytest.raises(ValidationError) as exc_info: + MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=3601) + assert 'Timeout cannot exceed 3600 seconds' in str(exc_info.value) + + def test_timeout_validation_way_over_limit(self): + """Test timeout validation rejects very large values.""" + with pytest.raises(ValidationError) as exc_info: + MCPSHTTPServerConfig( + url='https://api.example.com/mcp', + timeout=86400, # 24 hours + ) + assert 'Timeout cannot exceed 3600 seconds' in str(exc_info.value) + + def test_url_validation_still_works(self): + """Test that existing URL validation still works with timeout field.""" + # Valid URL should work + config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=30) + assert config.url == 'https://api.example.com/mcp' + + # Invalid URL should fail + with pytest.raises(ValidationError): + MCPSHTTPServerConfig(url='not-a-url', timeout=30) + + def test_backward_compatibility_no_timeout(self): + """Test backward compatibility - config works without timeout field.""" + # Should work exactly like before the timeout field was added + config = MCPSHTTPServerConfig(url='https://api.example.com/mcp') + assert config.url == 'https://api.example.com/mcp' + assert config.api_key is None + assert config.timeout == 60 # Default + + def test_model_dump_includes_timeout(self): + """Test that model serialization includes timeout field.""" + config = MCPSHTTPServerConfig( + url='https://api.example.com/mcp', api_key='test-key', timeout=90 + ) + + data = config.model_dump() + expected = { + 'url': 'https://api.example.com/mcp', + 'api_key': 'test-key', + 'timeout': 90, + } + assert data == expected + + def test_model_dump_with_none_timeout(self): + """Test model serialization with None timeout.""" + config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=None) + + data = config.model_dump() + expected = { + 'url': 'https://api.example.com/mcp', + 'api_key': None, + 'timeout': None, + } + assert data == expected