Skip to content

fix: detect install method when generating .mcp.json#77

Merged
tirth8205 merged 1 commit intotirth8205:mainfrom
AR6420:fix/mcp-json-uvx-fallback
Mar 31, 2026
Merged

fix: detect install method when generating .mcp.json#77
tirth8205 merged 1 commit intotirth8205:mainfrom
AR6420:fix/mcp-json-uvx-fallback

Conversation

@AR6420
Copy link
Copy Markdown
Contributor

@AR6420 AR6420 commented Mar 28, 2026

Fixes #75

Problem

The install command hardcodes "uvx" as the command in the generated .mcp.json.
Users who install via pip don't have uvx on PATH, so the MCP server fails to connect in Claude Code.

Solution

Added a shutil.which("uvx") check when generating the MCP config.
If uvx is available, the existing behavior is preserved.
If not, it falls back to using "command": "code-review-graph" directly, which works regardless of install method.

Testing

  • Existing tests pass
  • Manually verified: code-review-graph serve works with the fallback config

The install command hardcodes uvx as the MCP server command, which fails
for pip-installed users who don't have uvx on PATH. Now checks for uvx
availability via shutil.which() and falls back to the direct
code-review-graph command.

Fixes tirth8205#75

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@JiwaniZakir JiwaniZakir left a comment

Choose a reason for hiding this comment

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

The detection logic in _build_server_entry uses shutil.which("uvx") at call time, which is a reasonable approach, but it's worth noting this check runs on the machine generating the .mcp.json, not on the machine that will ultimately execute it. If a developer generates the config on a system where uvx happens to be installed but deploys it to an environment where it isn't (or vice versa), the generated entry will be wrong. Consider documenting this assumption or accepting an explicit install_method parameter so callers can override the detected value.

Additionally, the entry: dict[str, Any] type annotation is repeated in both branches of the if/else, which is redundant — declaring it once after the block (or using a single assignment with a ternary-style approach) would be cleaner and avoid the Pylance/mypy warning about re-declaring a variable in the same scope.

It's also worth adding a test that mocks shutil.which to return None and verifies the fallback "code-review-graph" command is used, since that path currently has no coverage implied by the diff.

@AR6420
Copy link
Copy Markdown
Contributor Author

AR6420 commented Mar 31, 2026

Quick clarification on the cross-machine point.
"code-review-graph install" is a Local dev tool setup command. It writes .mcp.json on the same machine where Claude Code reads it. There's no generate-here-deploy-there workflow for config files.

Claude Code needs this file locally to connect to the MCP server. So the shutil.which check at install time is the right time to check, because that's the machine that'll use it.

Also worth noting: the current code hardcodes uvx unconditionally so the "wrong machine" concern applies even more to main than to this PR. Good catch on the type annotation and test coverage though, I'll clean those up later.

@tirth8205 tirth8205 merged commit f965536 into tirth8205:main Mar 31, 2026
1 check passed
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.

install command hardcodes uvx in .mcp.json, breaks MCP for pip-installed users

3 participants