feat: improve MCP server installation #461
Conversation
| ### Option 1: Global Installation (Recommended) | ||
| ```bash | ||
| npm install -g @devcycle/cli | ||
| # This installs both 'dvc' CLI and 'dvc-mcp' server | ||
| ``` |
There was a problem hiding this comment.
don't we have a brew package that we also maintain?
There was a problem hiding this comment.
not sure, I'll add a ticket for looking at that later.
There was a problem hiding this comment.
| 'Please verify your DEVCYCLE_CLIENT_ID and DEVCYCLE_CLIENT_SECRET are correct, ' + | ||
| 'or run "dvc login sso" to authenticate with SSO.', | ||
| ) | ||
| } else { |
There was a problem hiding this comment.
you don't need an else here - you're throwing
There was a problem hiding this comment.
yea, reads cleaner this way thou.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the DevCycle MCP server by improving cross-platform installation, adding version/help flags, and enriching error handling with actionable suggestions.
- Introduced detailed, contextual error messages in authentication utilities and tool handlers
- Implemented
--version/-vand--help/-hflags with dynamic version reporting - Updated documentation with step-by-step installation, verification, and troubleshooting
Reviewed Changes
Copilot reviewed 7 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mcp/utils/auth.ts | Enhanced authentication errors with env var vs. SSO guidance |
| src/mcp/server.ts | Centralized tool error handling, added categorization & suggestions |
| src/mcp/index.ts | Added CLI flags, dynamic version lookup, and improved startup errors |
| src/api/zodClient.ts | Refined VariableValidationEntity schema types |
| package.json | Switched dvc-mcp bin entry to use cross-platform wrapper |
| oclif.manifest.json | Bumped manifest version |
| README.md | Added MCP installation guide, updated TOC and troubleshooting |
Comments suppressed due to low confidence (3)
README.md:25
- The TOC entry links to a section that doesn't exist, causing a broken link. Update or remove this entry to match actual headings.
* [This installs both 'dvc' CLI and 'dvc-mcp' server](#this-installs-both-dvc-cli-and-dvc-mcp-server)
README.md:204
- [nitpick] This heading duplicates the earlier 'Option 2: Project-Specific Installation' section and may confuse readers. Consider renaming or consolidating to avoid duplication.
### For Project-Specific Installation
src/mcp/server.ts:87
- The new error handling methods (handleToolError, categorizeError, getErrorSuggestions) currently lack unit tests. Consider adding tests to validate categorization logic and suggestion outputs.
private handleToolError(error: unknown, toolName: string) {
| case lowerMessage.includes('project') && | ||
| lowerMessage.includes('not found'): | ||
| return 'PROJECT_ERROR' | ||
|
|
There was a problem hiding this comment.
The PROJECT_ERROR case comes after the more general RESOURCE_NOT_FOUND case, so errors with 'not found' are always classified as RESOURCE_NOT_FOUND. Consider moving the PROJECT_ERROR case before RESOURCE_NOT_FOUND to correctly capture project-specific 'not found' errors.
| case lowerMessage.includes('project') && | |
| lowerMessage.includes('not found'): | |
| return 'PROJECT_ERROR' |
| (error.message.includes('authentication') || | ||
| error.message.includes('project') || | ||
| error.message.includes('DEVCYCLE_')) |
There was a problem hiding this comment.
The includes checks on error.message are case-sensitive, so messages starting with an uppercase 'Authentication' won't match. Normalize error.message to lowercase before matching (e.g., const msg = error.message.toLowerCase()).
| (error.message.includes('authentication') || | |
| error.message.includes('project') || | |
| error.message.includes('DEVCYCLE_')) | |
| (() => { | |
| const msg = error.message.toLowerCase(); | |
| return ( | |
| msg.includes('authentication') || | |
| msg.includes('project') || | |
| msg.includes('devcycle_') | |
| ); | |
| })() |
| 'No authentication found. Please set DEVCYCLE_CLIENT_ID and DEVCYCLE_CLIENT_SECRET environment variables, ' + | ||
| 'or run "dvc login sso" in the CLI first.', | ||
| ) | ||
| const hasEnvVars = |
There was a problem hiding this comment.
[nitpick] The variable hasEnvVars is assigned a string or undefined, relying on truthiness. Consider explicitly converting to boolean for clarity, e.g., const hasEnvVars = Boolean(process.env.DEVCYCLE_CLIENT_ID && process.env.DEVCYCLE_CLIENT_SECRET).
* feat: improve MCP server installation * fix: update yarn.lock * fix: error handling * fix: switch to path.resolve() * chore: refactor and cleanup error handling code * chore: address PR comments
* feat: improve MCP server installation * fix: update yarn.lock * fix: error handling * fix: switch to path.resolve() * chore: refactor and cleanup error handling code * chore: address PR comments
* feat: improve MCP server installation * fix: update yarn.lock * fix: error handling * fix: switch to path.resolve() * chore: refactor and cleanup error handling code * chore: address PR comments
Improve MCP Server Installation and User Experience
This PR enhances the DevCycle MCP server installation and usability with several key improvements:
🔧 Cross-Platform Installation Support
bin/mcp,bin/mcp.cmd) for better Windows/Unix compatibility📚 Enhanced Documentation & Setup
✅ Quick Verification Support
--version/-vand--help/-hflags to MCP server🚨 Improved Error Handling
Impact
Makes MCP server installation more reliable across platforms and significantly improves the developer experience when encountering issues. Users now get clear, actionable guidance instead of cryptic error messages.