Skip to content

fix(mcp): pin mcp<2.0.0 and report fastmcp removal accurately - #355

Open
erensh27 wants to merge 1 commit into
NVIDIA:mainfrom
erensh27:fix/pin-mcp-under-2
Open

fix(mcp): pin mcp<2.0.0 and report fastmcp removal accurately#355
erensh27 wants to merge 1 commit into
NVIDIA:mainfrom
erensh27:fix/pin-mcp-under-2

Conversation

@erensh27

@erensh27 erensh27 commented Aug 9, 2026

Copy link
Copy Markdown

Fixes #333.

Two fixes:

  1. pyproject.toml — cap the mcp extra at mcp<2.0.0: mcp 2.0.0 (released 2026-07-28) removed mcp.server.fastmcp, so fresh skillspector[mcp] installs could resolve mcp 2.x and then fail at server init.
  2. src/skillspector/mcp_server.py — on import failure, first check whether mcp itself is importable. If it is, the problem is the missing mcp.server.fastmcp submodule (mcp>=2.0.0), so the error now says so and points at pip install 'skillspector[mcp]' for a compatible pin, instead of the misleading "optional 'mcp' dependency is missing" message.

Reproduction from the issue now yields a truthful error under mcp 2.x, and pip install 'skillspector[mcp]' no longer resolves to mcp 2.x at all.

Comment thread src/skillspector/mcp_server.py Outdated
from mcp.server.fastmcp import FastMCP
except ModuleNotFoundError as exc:
try:
import mcp # noqa: F401

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking: this catches every ModuleNotFoundError raised while importing FastMCP, including a missing transitive dependency inside an installed MCP 1.x package. Importing top-level mcp then succeeds and the code falsely reports that MCP 2.x removed FastMCP. Distinguish exc.name == "mcp", exc.name == "mcp.server.fastmcp", and unrelated missing modules, preserving the original error for the last case, and add tests for both incompatible-MCP and transitive-import failures.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Automated SkillSpector Review]

Requesting changes. The new error branch misdiagnoses missing transitive imports inside installed MCP 1.x as the MCP 2.x FastMCP removal, and the dependency change leaves uv.lock stale (uv lock --check fails). This branch also conflicts with main and substantially overlaps already-approved PR #339, which includes the lock and a focused regression.

mcp 2.0.0 (2026-07-28) removed mcp.server.fastmcp, so an install that
resolved mcp 2.x fails at server init with the misleading 'requires the
optional mcp dependency' error even though mcp is present.

- Distinguish a missing 'mcp' package (missing optional extra) from an
  installed mcp package that lacks 'mcp.server.fastmcp' (incompatible
  mcp>=2.0.0), and re-raise the original error for unrelated missing
  modules such as a missing transitive dependency.
- Add tests covering all three failure modes.
@erensh27
erensh27 force-pushed the fix/pin-mcp-under-2 branch from 47f3abf to 7a055a4 Compare August 11, 2026 09:54
@erensh27

Copy link
Copy Markdown
Author

All review comments addressed:

  • Blocking: the error branch now distinguishes exc.name == "mcp" (missing extra), exc.name == "mcp.server.fastmcp" (installed but incompatible, mcp>=2.0.0), and unrelated missing modules — the original error is preserved for the last case via raise exc.
  • Tests: added coverage for all three failure modes (missing extra, incompatible install, transitive-import failure) in tests/unit/test_mcp_server.py.
  • Conflict/stale lock: rebased on main. main already carries the mcp<2.0.0 pin and matching uv.lock (merged via release snapshots), so this PR no longer touches pyproject.toml/uv.lockuv lock --check passes and the diff is now only the diagnostic fix + tests.

Verified: pytest -m "not integration and not provider" tests/ → 1939 passed; ruff check/format clean. Ready for re-review.

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

Labels

None yet

Projects

None yet

2 participants