Skip to content

fix: bound MCP extra to supported major version - #339

Open
koriyoshi2041 wants to merge 1 commit into
NVIDIA:mainfrom
koriyoshi2041:rios/fix-mcp-v2-compat
Open

fix: bound MCP extra to supported major version#339
koriyoshi2041 wants to merge 1 commit into
NVIDIA:mainfrom
koriyoshi2041:rios/fix-mcp-v2-compat

Conversation

@koriyoshi2041

Copy link
Copy Markdown
Contributor

Problem

Fresh skillspector[mcp] installs resolve MCP 2.x, which removed mcp.server.fastmcp. The install succeeds, but the server fails at initialization and incorrectly reports that the optional dependency is missing. Fixes #333.

Fix

  • constrain the MCP extra to the supported 1.x line
  • regenerate the lockfile
  • distinguish an absent mcp package from an installed package missing the expected FastMCP module
  • cover the incompatible-install diagnostic

Test

  • uv run pytest -q tests/unit/test_mcp_server.py (10 passed, including stdio initialize/list-tools)
  • uv run ruff check src/skillspector/mcp_server.py tests/unit/test_mcp_server.py
  • uv run ruff format --check src/skillspector/mcp_server.py tests/unit/test_mcp_server.py
  • git diff --check

Risk

Low. This deliberately holds the optional server integration on MCP 1.x until SkillSpector migrates to the 2.x API; users that force an incompatible install now receive an accurate error instead of a missing-extra message.

rng1995
rng1995 previously approved these changes Aug 5, 2026

@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]

Approved. Bounding the optional MCP dependency below v2 keeps the currently supported FastMCP import contract explicit, and the improved import error distinguishes an absent package from an incompatible installed layout. The focused MCP suite passed (10 tests), as did Ruff lint and format checks.

Signed-off-by: kigland <shuaizhicheng336@gmail.com>
@koriyoshi2041

Copy link
Copy Markdown
Contributor Author

Rebased onto current main at 7fc09a3. Upstream now already carries the MCP <2.0.0 dependency bound, so the remaining delta is limited to distinguishing an incompatible installed MCP layout from a missing optional dependency, with its regression test.

Focused MCP tests pass (10/10); Ruff lint/format, lock check, and diff check also pass.

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