-
Notifications
You must be signed in to change notification settings - Fork 120
[Feature] Support for project-level MCP configuration files (.mcp.json) #292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… & added support for claude code's object style mcp configuration.
… title heading for /help command.
|
@will-lamerton Please review, approve & merge this PR when you get the time. Minor enhancements & improvements in this one |
… environment values Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
Hey @Avtrkrb, thanks for putting this together! I like the direction of adding I've been thinking about the overall config setup though, and I'm wondering if we're overcomplicating things a bit. Let me share my thinking: The current situation:
What this PR adds:
I'm a bit worried this creates confusion for users - "where do I put my MCP config?" becomes a complicated question. I'm wondering if we need to simplify this instead?
That would be it. Two files, clear separation, no precedence confusion. The object format from Claude Code would be the only supported format in .mcp.json: This gives us cross-tool compatibility (which I really want) without the complexity of checking 6 different locations with merge logic. If MCP servers are found in agents.config.json, we show a deprecation warning pointing users to move their config to .mcp.json. Something like: "MCP servers in agents.config.json is deprecated. Please move your MCP configuration to .mcp.json for cross-tool compatibility." This keeps things backward compatible while nudging users toward the simpler setup. For the local overrides use case, users can just gitignore their .mcp.json or use environment variables for secrets - we don't necessarily need a separate .mcp.local.json. What do you think? Happy to chat more about this if you want to discuss the tradeoffs. The core work you've done here on format parsing and the new loader is solid - I'm mainly suggesting we scope down the number of supported locations and how we handle the existing use of |
Hey @will-lamerton I agree with you. I'll be honest, this was something that was on my mind since the very start of my journey with Nanocoder & would be up for an overhaul of configuration files if you agree to what I'm proposing. Here's my thoughts:
This is just a starting proposal, free to iterate on this further ! |
@Avtrkrb - awesome, glad we’re on the same page about simplifying! I personally think we can go even simpler: Just 2 locations, same filename:
Just 1 format:
My reasoning:
So the mental model becomes super simple:
The global fallback at ~/.config/nanocoder/.mcp.json is there for users who want default MCP servers across all projects, but project-level .mcp.json wins when present. What do you think? I know it’s more opinionated, but I think simpler is better here - fewer decisions for users to make. Everything else you stated in regards to agents.config.json I agree with as this is Nanocoder specific stuff. |
Sounds like a plan ! Ideally I'd like to keep all mcp servers from both the |
Let's do it :) |
… environment values Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Updates to
|
The test-coverage job was failing in CI but passing locally due to: 1. Missing environment variables for proper terminal rendering 2. Detached HEAD state breaking git diff commands in file-snapshot.ts Changes: - Add CI, NODE_ENV, FORCE_COLOR, TERM environment variables - Add set-safe-directory: false to checkout step This ensures the Ink CLI tests have proper color support and git state for file snapshot operations. Related to PR #292
The CLI integration tests require dist/cli.js to exist. This was causing test failures in CI (but not locally where dist/ exists). Added build step after dependency installation in test-coverage job. Related to PR #292
I think we might need to provide support for additional shell like zsh, fish, powershell etc. I remember this was failing sometime last week & I had fixed it as well. Not sure how it has resurfaced |
Fixed :) |
That's awesome ! Please review it & let me know if anything needs to change |
will-lamerton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Avtrkrb - looking great :)
A few things I spotted:
- Platform-specific path in deprecation warning
The deprecation warning hardcodes the Linux path:
logError('Please migrate to ~/.config/nanocoder/.mcp.json');
But the actual config path varies by platform:
Linux: ~/.config/nanocoder/.mcp.json
macOS: ~/Library/Preferences/nanocoder/.mcp.json
Windows: %APPDATA%\nanocoder.mcp.json
Windows and macOS users would see a path that doesn't exist on their system. Should be an easy fix (I think):
import {getConfigPath} from '@/config/paths';
import {join} from 'path';
function showAgentsConfigDeprecationWarning() {
const configPath = join(getConfigPath(), '.mcp.json');
logError('Warning: MCP servers in agents.config.json are deprecated.');
logError(`Please migrate to ${configPath}`);
}
Same thing for the array format warning if it mentions a path anywhere.
-
Separate /setup-mcp command?
Now that MCP config lives in its own file, should we have a dedicated /setup-mcp command? This would let users manage providers and MCP servers separately, which feels cleaner than bundling everything in /setup-config. Just a thought and curious to hear your thoughts as I might be wrong. -
Bug: Can't remove all MCP servers
I noticed if I add one MCP server globally, then try to remove it and save, I can't get back to having 0 MCPs. The wizard seems to require at least one entry. Users should be able to have an empty MCP config if they want. -
Missing test coverage
A few test cases that would be good to add (unless I have missed them):
- Verify the deprecation warning is actually shown when loading from agents.config.json
- Verify the deprecation warning is shown for array format
- Test the platform-specific path in the warning message (once fixed)
- Test that empty .mcp.json files are handled gracefully
Let me know thoughts :D
…tory instead of the hardcoded Linux config directory.
|
Hey @will-lamerton I've pushed a fix for the hardcoded Linux config path, forgot about it while I was investigating the CI issue.
This sounds logical. I'll take a look into this. I was also thinking having a feature where users can add mcp servers while in non interactive mode, something like
I think you are testing an edge case here & the wizard might not currently support this. Will look into it.
These are good coverage, will add them in. I did manually test an empty .mcp.json from the project directory & it worked, will add this to the ava tests as well. |
Sounds good :) if you like the idea of a /setup-mcp wizard then I think we should add this for this PR? I agree the non-interactive mode addition can be in a future one though! Awesome on the rest of it! Will keep tabs on this :D |
Yes I'll be adding the /setup-mcp as part of this PR. |
|
@will-lamerton I've made the necessary changes & added the new |
Today's Changes Summary1. Removed MCP Functionality from
|
I think this is a good idea... makes sense and is clearer for the user :) |
Let me know and I can work on this if you need 😎 |
Works ! I think apart from this change there is nothing else to be done in this PR |
I agree! Looks great :D I'll work on this now quick |
|
@will-lamerton I think when this PR gets merged, we need to bump the version to 1.30.0 due to some of the breaking changes that are introduced. The older |
I agree, but, I was going to do just version 1.21.0 - any specific reason to go to 1.30.0? :) |
Sorry, it was a typo on my end. I meant 1.21.0, but got distracted by another number on my screen & typed 30 instead of 21. 😅 I originally meant could we do 1.21.0 instead of 1.20.5... |
|
@Avtrkrb - pretty happy to merge if you are :) |
Let's go for it !👍🏻 |
|
Hey @Avtrkrb - resolved conflicts, can you test one more time? Everything passes this end |
Thanks for your help in resolving the conflicts, I did another round of test on my end, everything looks good. I think we're good to go ! 👍🏻 |
[Feature] Support for project-level MCP configuration files (.mcp.json)
Description
This PR implements hierarchical configuration loading for MCP (Model Context Protocol) servers, enabling team collaboration through project-level configuration files. The implementation adds support for Claude Code's object-based MCP server format while maintaining full backward compatibility with existing configurations. Additionally, this PR adds CLI options for version and help information.
Key Features Added:
Project-Level Configuration Support:
.mcp.json,mcp.json,.nanocoder/mcp.json,.claude/mcp.json, and.nanocoder/mcp.local.jsonfilesClaude Code Format Compatibility:
{ "serverName": { ...serverConfig } }[ {...serverConfig} ]Hierarchical Configuration Loading:
Enhanced MCP Command Display:
/mcpcommand now shows configuration source levels (e.g., [project], [global])Security Enhancements:
.gitignoregeneration for sensitive config filesCLI Version and Help Options:
--version/-vflag to display version information--help/-hflag to show usage information and available optionsNew Quit Command and UI Improvements:
/quitcommand as an alternative to/exitfor exiting the application/helpcommand display to show "Help" instead of "/help" as the title/mcpcommand display to show "Model Context Protocol Servers" instead of "/mcp" as the title/mcpcommand UI with improved color scheme using theme's tool colorType of Change
Testing
Automated Tests
.spec.tsfilespnpm test:allcompletes successfully)Manual Testing
.mcp.jsonfiles--versionand--helpflags work correctly/quitcommand works as expected/helpcommand title display is updated correctly/mcpcommand UI improvements are visibleFiles Changed
source/config/mcp-config-loader.ts- New hierarchical configuration loading systemsource/config/mcp-config-loader.spec.ts- Comprehensive tests for new functionalitysource/config/validation.ts- Security validation for project configssource/config/validation.spec.ts- Tests for security validationsource/config/index.ts- Updated to use hierarchical loadingsource/commands/mcp.tsx- Enhanced to show configuration source levelssource/hooks/useAppInitialization.tsx- Updated MCP initializationsource/client-factory.ts- Updated provider loadingsource/cli.tsx- Added CLI version and help functionalitysource/cli.spec.ts- Added tests for CLI version/help flagssource/cli-integration.spec.ts- Added integration tests for CLI functionalitysource/commands/exit.ts- Added quit command functionalitysource/commands/help.tsx- Updated help command display titlesource/commands/mcp.tsx- Enhanced MCP command UI with improved color schemesource/hooks/useAppInitialization.tsx- Registered the new quit commandREADME.md- Updated documentation with new configuration options and CLI usagedocs/mcp-configuration.md- Detailed documentation for new featuresChecklist
Backwards Compatibility
This implementation maintains full backward compatibility:
agents.config.jsonfiles continue to work unchanged/quitcommand is an optional addition alongside existing/exitcommand/helpand/mcpcommands are visual enhancements onlySecurity Considerations
$API_KEY) are recommended over hardcoded values.gitignoresuggestions