Skip to content

Conversation

dolan-openai
Copy link
Contributor

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

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
Copy link

github-actions bot commented Sep 4, 2025

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

@dolan-openai
Copy link
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
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
Copy link
Contributor

@jif-oai jif-oai left a comment

Choose a reason for hiding this comment

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

Few nits

@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