-
Notifications
You must be signed in to change notification settings - Fork 301
Add CLI and config file support #177
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
Add CLI and config file support #177
Conversation
I believe the CLI console should be integrated into the web console. Essentially, this involve merging another tool into the inspector, but it could also work well as a standalone tool. |
The CLI client is meant to exist as an alternative UI from the web client. It can't live within a web client by its very nature. It allows inspecting MCP servers via the command line rather than via a web interface. The goal is to facilitate programmatic access to MCP servers. For example running test scripts to validate an MCP server, or as a feedback loop when using coding assistant like Cursor, so the assistant can autonomously test MCP servers during development. |
Oh , perfect, i just saw you added the inspector-bin package. |
To my understanding the enhanced cli mode would support a subset of the UI-based testing features, correct? Do you think there should be some kind of feature breakdown table to make it clearer what the 'cli-mode' can be used to test, vs the UI mode, or should this be self-evident? Thinking through what would still require the UI:
Since the main purpose of this new cli mode seems to be streamlining testing MCP server functionality rather than simulating the expected user interactions, maybe this is a non issue, as long as that "when to use what mode" is clear to the user. |
Yes it's a one shot call to any MCP server, so some round-tripping functionalities are not supported at the moment, although they could, I think. Goal is to get the output of the MCP server on stdout so anyone or anything can read it and react accordingly. Personally I need it as part of automated testing workflows and using coding agents while developing MCP servers, where it's very useful to create a feedback loop between the agent and the output. I'm happy to add that table/explanation in the README if you would like me to. |
Hi @nbarraud, just wanted to let you know I'm not ignoring this one, since its a significant update I am pinging a few people directly for more eyes on it. :) |
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.
Overall this looks good to me, although so far I noticed a few things that are not cooperating on Windows. I am still tinkering with the script setup on my Windows machine, but I didn't want to leave you hanging and don't see anybody else weighing in yet. Will ping a couple others again to make sure there aren't any showstoppers here I am missing.
README.md
Outdated
``` | ||
|
||
> **Note for Windows users:** |
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.
Something to note once this is about to be merged - looks like some sections were accidentally deleted from Readme.
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.
I fixed that and added a table as you requested earlier.
bin/tests/cli-tests.sh
Outdated
@@ -0,0 +1,231 @@ | |||
#!/bin/bash |
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.
I got a little stuck on trying to run these tests myself on my Windows machine using Git Bash. Still investigating but it seems like the script hangs when connecting using npx plus it does not terminate even when I change the commands to use node and a local server file. Probably not specific to this script and more my own environment.
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.
I converted the tests to .js so it can be run on all systems (in theory)... Give it a try.
bin/package.json
Outdated
], | ||
"scripts": { | ||
"build": "tsc", | ||
"postbuild": "chmod +x build/index.js && cp build/index.js cli.js", |
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.
Lets make this Windows compatible using something like:
"postbuild": "node -e \"const fs=require('fs'); fs.copyFileSync('build/index.js', 'cli.js'); if(process.platform !== 'win32') require('child_process').execSync('chmod +x build/index.js')\""
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.
Done.
Thank you for reviewing @olaservo I will rebase to take into accounts later PRs and changes to the README. I don't have Windows at the moment so I will have to defer to you on that part. I can apply your suggested changes for Windows compat but won't be able to test it easily. |
Thank you for your comments @pulkitsharma07 Very busy on other projects this week but I will answer soon. |
383e8e3
to
4a0dab2
Compare
4a0dab2
to
5b22143
Compare
Hey @olaservo, just checking in! |
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.
Hi @nbarraud just catching up from being offline a little while. I approved this and we can get these enhancements merged in once you get a chance to resolve the latest conflicts. I will keep an eye out just in case the changes to resolve conflicts revoke the approval. Thanks!
Thank you @olaservo, I resolved the conflicts and the PR is ready for merging! I guess that revoked the approval... EDIT: I don't know if I'm supposed to increment the version number or if this is the responsibility of the maintainers. Let me know. |
New conflicts popped up over the weekend so I resolved them. @olaservo please approve, thank you! |
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.
Re-approving after conflict resolutions.
Maintainers bump the version separately when preparing a release. |
Thank you @olaservo, I think the workflow still needs approval? |
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.
LGTM! 👍
Tests run locally.
Motivation and Context
This PR adds two major features to the MCP Inspector:
npx @modelcontextprotocol/inspector node build/index.js --cli --method tools/call --tool-name mytool --tool-arg key=value --tool-arg another=value2
How Has This Been Tested?
The implementation has been tested with various MCP servers, including local and remote servers. Test scenarios include:
Shell-based tests have been added in
bin/tests/cli-tests.sh
to verify the functionality.Breaking Changes
No breaking changes. All existing functionality continues to work as before. The new features are opt-in through the
--cli
flag for CLI mode and the--config
/--server
flags for configuration file support.Types of changes
Checklist
Additional context
The implementation consists of two main components:
cli/
) that handles the programmatic interaction with MCP serversbin/
) to support configuration files and CLI modeTo support the more complex logic required for these features, the
bin/
directory has been converted into a TypeScript project.