Skip to content

fix(mcp): support MCP 2.x timeouts 🤖🤖🤖 - #314

Merged
furgalep merged 2 commits into
NVIDIA-NeMo:mainfrom
Vi-Sri:svenkatanara/issue-262-mcp2-timeout
Sep 10, 2026
Merged

fix(mcp): support MCP 2.x timeouts 🤖🤖🤖#314
furgalep merged 2 commits into
NVIDIA-NeMo:mainfrom
Vi-Sri:svenkatanara/issue-262-mcp2-timeout

Conversation

@Vi-Sri

@Vi-Sri Vi-Sri commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Adapts ClientSession read timeouts for the MCP 1.x and 2.x contracts.
  • Accepts both Tool.inputSchema and Tool.input_schema during tool discovery.
  • Handles the three-value MCP 1.x and two-value MCP 2.x streamable HTTP transport tuples.
  • Keeps the bundled real MCP server compatible with the MCP 2.x MCPServer rename.
  • Adds regression coverage using a real mcp.types.Tool and both transport tuple shapes.

Related issues

Closes #262

Validation

  • MCP 1.28.1: tests/test_mcp passed, 107 tests.
  • MCP 2.1.1: tests/test_mcp passed, 107 tests, including real stdio and streamable HTTP invocation.
  • MCP 2.2.0: tests/test_mcp passed, 107 tests, including real stdio and streamable HTTP invocation.
  • uv run ruff check .
  • uv run ruff format --check .
  • uv run python scripts/check_license_headers.py
  • Focused Pyright passes for client.py and the bundled server; tool.py still reports existing errors on untouched lines.

Checklist

  • Code follows the project style (uv run ruff check . and uv run ruff format --check . pass)
  • Tests added/updated and passing (uv run pytest)
  • Docs updated if behavior or public APIs changed (not applicable; no public API change)
  • New source files carry an SPDX license header (not applicable; no new source files)

🤖🤖🤖

Preserve MCP 1.x compatibility by adapting the timeout type at the SDK boundary.

Fixes NVIDIA-NeMo#262

Signed-off-by: Srinivas Venkatanarayanan <nikola.tesla195@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 04a12c58-7bf3-40f9-b5ba-ece683a73bd3

📥 Commits

Reviewing files that changed from the base of the PR and between fbbbfb1 and fb63ec9.

📒 Files selected for processing (2)
  • src/nooa/mcp/client.py
  • tests/test_mcp/test_client.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The MCP client detects the installed SDK major version and adapts session read timeouts for MCP 1.x and 2.x. SSE, stdio, and streamable HTTP transports use the adapted value. Tests cover both timeout contracts.

Changes

MCP timeout compatibility

Layer / File(s) Summary
Version-aware timeout adaptation
src/nooa/mcp/client.py, tests/test_mcp/test_client.py
The client detects the MCP SDK version and returns either the original timedelta or floating-point seconds. Parameterized tests cover both contracts.
Transport session integration
src/nooa/mcp/client.py, tests/test_mcp/test_client.py
SSE, stdio, and streamable HTTP sessions receive the adapted timeout. Existing assertions validate the converted value.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to fb63e

MCP clients now pass read timeouts in the representation required by MCP 1.x or 2.x across supported transports, preventing MCP 2.x session initialization failures. No current merge-blocking risk remains.

Suggested reviewers: furgalep

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: MCP 2.x timeout support. The emojis add minor noise but do not make the title unclear or unrelated.
Linked Issues check ✅ Passed The changes address issue #262 by converting the MCP read timeout to numeric seconds for MCP 2.x while preserving timedelta values for MCP 1.x. The compatibility logic covers stdio, SSE, and streamabl…
Out of Scope Changes check ✅ Passed The source changes and tests are directly related to MCP timeout compatibility described in issue #262. No unrelated code changes are identified.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Normalize SDK tool fields and streamable HTTP transport tuples, and keep the real test server compatible across MCP 1.x and 2.x.

Refs NVIDIA-NeMo#262

Signed-off-by: Srinivas Venkatanarayanan <nikola.tesla195@gmail.com>
@furgalep
furgalep merged commit 96a52bc into NVIDIA-NeMo:main Sep 10, 2026
8 checks passed
@furgalep

Copy link
Copy Markdown
Collaborator

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nooa[mcp] breaks on mcp>=2.0: timedelta read_timeout passed to ClientSession, TypeError at initialize

2 participants