Skip to content

feat(mcp): per-server startup timeout - #3182

Merged
bolinfest merged 9 commits into
mainfrom
dolan-2025-09-04-tool-timeout
Sep 8, 2025
Merged

feat(mcp): per-server startup timeout#3182
bolinfest merged 9 commits into
mainfrom
dolan-2025-09-04-tool-timeout

Conversation

@dolan-openai

@dolan-openai dolan-openai commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

Seeing timeouts on certain, slow mcp server starting up when codex is invoked. Before this change, the timeout was a hard-coded 10s. Need the ability to define arbitrary timeouts on a per-server basis.

Summary of changes

  • Add startup_timeout_ms to McpServerConfig with 10s default when unset
  • Use per-server timeout for initialize and tools/list
  • Introduce ManagedClient to store client and timeout; rename LIST_TOOLS_TIMEOUT to DEFAULT_STARTUP_TIMEOUT
  • Update docs to document startup_timeout_ms with example and options table

- Add timeout_ms to McpServerConfig with 10s default when unset
- Use per-server timeout for initialize and tools/list; aggregate startup/list errors with captured output and drop failing servers from registry
- Introduce ManagedClient to store client and timeout; rename LIST_TOOLS_TIMEOUT to DEFAULT_MCP_SERVER_TIMEOUT
- mcp-client: pipe and retain last 20 lines of STDOUT/STDERR; add output_snippet API for diagnostics
- Update docs to document timeout_ms with example and options table
@github-actions

github-actions Bot commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Comment thread codex-rs/mcp-client/src/mcp_client.rs Outdated
@dolan-openai

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Sep 4, 2025
@dolan-openai
dolan-openai marked this pull request as ready for review September 4, 2025 19:49
@bolinfest
bolinfest requested a review from jif-oai September 4, 2025 22:21
@bolinfest

Copy link
Copy Markdown
Collaborator

I'm going to let @jif-oai take a first crack at this, but two things off the top of my head:

  • Should this be two PRs? One for the timeout and one for the output?
  • Should the option be named startup_timeout_ms and should it be specific to startup as we may want to have different timeouts for other behaviors in the future?

@dolan-openai
dolan-openai force-pushed the dolan-2025-09-04-tool-timeout branch from fee27a0 to 05f282e Compare September 5, 2025 20:54
@dolan-openai
dolan-openai force-pushed the dolan-2025-09-04-tool-timeout branch from 05f282e to 917f55c Compare September 5, 2025 20:55
@dolan-openai dolan-openai changed the title feat(mcp): per-server timeout and capture server stdout/stderr feat(mcp): per-server startup timeout Sep 5, 2025

@jif-oai jif-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few nits

Comment thread codex-rs/core/src/config_types.rs Outdated
Comment thread codex-rs/core/src/config_types.rs Outdated
Comment thread codex-rs/core/src/mcp_connection_manager.rs Outdated
Comment thread codex-rs/core/src/mcp_connection_manager.rs
Comment thread docs/config.md Outdated
Comment thread docs/config.md Outdated
Comment thread docs/config.md Outdated
@bolinfest
bolinfest merged commit 6efb52e into main Sep 8, 2025
18 checks passed
@bolinfest
bolinfest deleted the dolan-2025-09-04-tool-timeout branch September 8, 2025 15:12
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants