-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(mcp-adapters): respect tool timeouts #8241
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
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.
Pull Request Overview
This PR adds support for respecting per-tool timeouts (and abort signals) by forwarding LangChain's RunnableConfig
into MCP SDK calls.
- Extend
_callTool
to extracttimeout
andsignal
fromRunnableConfig
and pass them toclient.callTool
- Update
loadMcpTools
to wrap tool functions so they accept and forwardconfig
- Add unit and integration tests for timeout behavior, plus documentation in README
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
libs/langchain-mcp-adapters/src/tools.ts | Implemented timeout/signal forwarding in _callTool and updated loader wraps |
libs/langchain-mcp-adapters/tests/tools.test.ts | Adjusted mock expectations to include the new config parameters |
libs/langchain-mcp-adapters/tests/fixtures/dummy-http-server.ts | Added sleep_tool to simulate delays |
libs/langchain-mcp-adapters/tests/client.int.test.ts | Added integration tests for respecting tool timeouts |
libs/langchain-mcp-adapters/README.md | Documented timeout configuration for MCP tools |
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.
Pull Request Overview
This PR adds support for respecting RunnableConfig
timeouts and abort signals when calling MCP tools, updates the tool loader to forward these settings, augments tests to cover timeout behavior, and documents the new configuration options.
- Forward
timeout
andsignal
fromRunnableConfig
into the MCP SDK call in_callTool
- Update
loadMcpTools
to accept and pass along the optionalconfig
parameter - Add integration tests for timeout success and failure scenarios and extend the dummy server fixture
- Document the new timeout configuration section in
README.md
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
libs/langchain-mcp-adapters/src/tools.ts | Extract and pass timeout /signal from RunnableConfig into client.callTool |
libs/langchain-mcp-adapters/tests/tools.test.ts | Adjust unit tests to expect the additional parameters on callTool |
libs/langchain-mcp-adapters/tests/fixtures/dummy-http-server.ts | Add a sleep_tool endpoint to simulate delay for timeout tests |
libs/langchain-mcp-adapters/tests/client.int.test.ts | Increase expected tool counts and add integration tests for timeout behavior |
libs/langchain-mcp-adapters/README.md | Introduce "Tool Timeout Configuration" section with examples and parameter table |
Comments suppressed due to low confidence (2)
libs/langchain-mcp-adapters/src/tools.ts:182
- Wrapping all non-ToolException errors in a
ToolException
can obscure the originalTimeoutError
type, making it harder for callers to catch timeouts specifically. Consider detecting and rethrowingTimeoutError
(orAbortError
) unmodified before the generic wrap.
throw new ToolException(`Error calling tool ${toolName}: ${String(error)}`);
libs/langchain-mcp-adapters/tests/tools.test.ts:125
- There’s no unit test verifying that an abort
signal
fromRunnableConfig
is passed through toclient.callTool
. Adding a test that supplies anAbortSignal
and asserts it appears in the third argument would improve coverage for cancellation behavior.
expect(mockClient.callTool).toHaveBeenCalledWith(
73dad70
to
4e0325b
Compare
fixes #8193