-
Notifications
You must be signed in to change notification settings - Fork 13
fix(mcp): use configured timeout instead of hardcoded 15s #15
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.
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
fix(mcp): use configured timeout instead of hardcoded 15s #15
Conversation
…tool-calling-protocol/dev MCP resolve defs refs
…tool-calling-protocol/dev v1.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 2 files
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/mcp/src/mcp_communication_protocol.ts">
<violation number="1" location="packages/mcp/src/mcp_communication_protocol.ts:178">
P2: The timeout expression `serverConfig.timeout ?? 30` is duplicated. Consider using `${timeoutMs / 1000}s` in the error message instead, which references the already-calculated value and ensures consistency.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
The `timeout` field was defined in `McpHttpServerSchema` but never used. `_withSession()` had a hardcoded 15000ms timeout which caused issues with slow MCP servers (e.g., LLM providers via OpenRouter). Changes: - Add `timeout` field to `McpStdioServerSchema` for consistency - Use `serverConfig.timeout` in `_withSession()` instead of hardcoded 15s - Default timeout increased from 15s to 30s to match schema default Fixes #20 in code-mode repo
c966bcb to
9a3a7a5
Compare
|
Thank you for the PR, makes a lot of sense. If you want to get added to our contributors page https://www.utcp.io/about/about-us please tell me your name and any links you want added. Feel free to also join our discord server if youre not already there. |
@utcp/mcp@1.1.0+ includes the fix for configurable MCP operation timeout (universal-tool-calling-protocol/typescript-utcp#15). Without this bump, code-mode-mcp stays pinned to 1.0.x which has a hardcoded 15s timeout that breaks slow MCP tools (e.g., LLM providers via OpenRouter that need 30-60s+). Users can now set `timeout` in their UTCP config to override the default.
|
@h3xxit We also need to merge this PR universal-tool-calling-protocol/code-mode#23 which bumps the deps so this fix will work in codemode. Feel free to include Provi + my github in the about page. Thx :) |
Summary
The
timeoutfield was defined inMcpHttpServerSchemabut never actually used in the implementation._withSession()had a hardcoded 15000ms (15s) timeout which caused issues with slow MCP servers (e.g., LLM providers like PAL via OpenRouter that need 30-60s+ for responses).Changes
timeoutfield toMcpStdioServerSchemafor consistency with HTTP serversserverConfig.timeoutin_withSession()instead of hardcoded 15sBefore
After
Usage
Users can now configure timeout per-server in their
.utcp_config.json:{ "manual_call_templates": [{ "name": "pal", "call_template_type": "mcp", "config": { "mcpServers": { "pal": { "transport": "stdio", "command": "...", "timeout": 120 } } } }] }Related Issues
Fixes universal-tool-calling-protocol/code-mode#20
Summary by cubic
Use the configured MCP timeout instead of a hardcoded 15s to avoid premature failures with slow MCP servers. Default is 30s; set per-server timeouts in .utcp_config.json (fixes code-mode #20).
Written for commit 9a3a7a5. Summary will update automatically on new commits.