Repository navigation
Conversation
When an MCP server declares a tools capability but then sends a tools/list reply with a mismatched JSON-RPC id, the @modelcontextprotocol/sdk drops the bad response (correct per JSON-RPC 2.0) and then waits the full MCP_DEFAULT_TIMEOUT_MSEC (10 minutes) for a matching reply before sending notifications/cancelled. During that window gemini-cli shows no spinner, no warning, and no partial tool list — the user sees a frozen terminal on every startup. The cap of 10 minutes is appropriate for ongoing tool calls but wrong for the initial one-shot discovery probe. Add MCP_DISCOVERY_TIMEOUT_MSEC = 10s and use it for: - connectAndDiscover() initial listTools - McpClient.discoverTools() default Active tool calls continue to use MCP_DEFAULT_TIMEOUT_MSEC. Per-server serverConfig.timeout still wins over both defaults. Closes google-gemini#28355
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a startup performance issue where the CLI would hang for up to 10 minutes when an MCP server provided an invalid response during initial tool discovery. By implementing a shorter, dedicated timeout for the discovery phase, the system now fails fast on misbehaving servers, allowing the CLI to remain responsive while maintaining the existing long-duration timeout for active tool calls. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
GitHub Actions
Waiting
|
📊 PR Size: size/M
|
![gemini-code-assist[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a shorter 10-second discovery timeout (MCP_DISCOVERY_TIMEOUT_MSEC) for initial MCP tool and prompt discovery to prevent misbehaving servers from hanging CLI startup, along with corresponding unit tests. The review feedback correctly identifies a bug in the timeout fallback logic within McpClient: using the nullish coalescing operator on the entire options object causes the default timeout to be completely discarded when options is a truthy object that lacks a timeout property (such as when only a signal is passed). The reviewer suggests spreading the options to ensure the default timeout is correctly applied.
| ...(options ?? { | ||
| timeout: this.serverConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC, | ||
| timeout: | ||
| this.serverConfig.timeout ?? MCP_DISCOVERY_TIMEOUT_MSEC, | ||
| }), |
There was a problem hiding this comment.
The current implementation uses a nullish coalescing operator on the entire options object (options ?? { timeout: ... }). When refreshTools() calls discoverTools() with { signal: abortController.signal }, options is truthy, so the default timeout is completely omitted. This means the timeout is not applied during post-connection refresh. Using object spreading with the default timeout defined first ensures that the default is always applied unless explicitly overridden by options.timeout. Additionally, we should rely on the schema as the single source of truth for the configuration default of this.serverConfig.timeout, avoiding the redundant nullish coalescing operator with MCP_DISCOVERY_TIMEOUT_MSEC.
timeout: this.serverConfig.timeout,
...options,References
- Rely on the schema as the single source of truth for configuration defaults, avoiding redundant nullish coalescing operators.
gemini-cli
Bot
added
priority/p1
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
|
This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
This branch is waiting to be deployed
1 participant
Closes #28355
What
When an MCP server declares a
toolscapability but then sends atools/listreply with a mismatched JSON-RPC id, the@modelcontextprotocol/sdkdrops the bad response (correct per JSON-RPC 2.0) and then waits the fullMCP_DEFAULT_TIMEOUT_MSEC(10 minutes) for a matching reply before sendingnotifications/cancelled.During that window gemini-cli shows no spinner, no warning, and no partial tool list. The user sees a frozen terminal on every startup, regardless of how many other servers are healthy.
The 10-minute cap is appropriate for active tool calls but wrong for the initial one-shot discovery probe.
Fix
Add a new constant
MCP_DISCOVERY_TIMEOUT_MSEC = 10 * 1000(10 seconds) and use it for the initial discovery path:connectAndDiscover()— the entrypoint that gemini-cli calls at startupMcpClient.discoverTools()— the instance method used for post-connection refreshActive tool calls continue to use
MCP_DEFAULT_TIMEOUT_MSEC. An explicitserverConfig.timeoutin the user's MCP config still wins over both defaults.A misbehaving server now fails discovery in 10 seconds with the existing error path (
Error discovering tools from <server>emitted viaemitMcpDiagnostic) instead of silently blocking startup for the full default window.Tests
Two new tests in
packages/core/src/tools/mcp-client.test.tsunderdescribe('connectAndDiscover discovery timeout (#28355)'):uses a short discovery timeout by default, not the 10m active-call default— asserts thatconnectAndDiscovercallsmcpClient.listToolswithtimeout: MCP_DISCOVERY_TIMEOUT_MSECwhen noserverConfig.timeoutis set, and that the constant is under one minute.honors an explicit serverConfig.timeout over the discovery default— asserts that a user-configured timeout still wins.Existing tests for the refresh timeout continue to pass unchanged.