Skip to content

add default MCP server URL on startup #288

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rinormaloku
Copy link

@rinormaloku rinormaloku commented Apr 9, 2025

Tiny change to pass the default MCP Server through the CLI, e.g.:

MCP_SERVER_URL=localhost:3030 npx @modelcontextprotocol/inspector

Makes the docs for MCP servers with SSE support a little cleaner.

@rinormaloku rinormaloku changed the title add default MCP server URL add default MCP server URL on startup Apr 9, 2025
Copy link
Contributor

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

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

Looks good (I haven't tested yet) but should be documented in the configuration section of the README.

@cliffhall cliffhall added enhancement New feature or request waiting on submitter Waiting for the submitter to provide more info labels Apr 11, 2025
@rinormaloku
Copy link
Author

@cliffhall updated the readme. Feel free to reword

@cliffhall
Copy link
Contributor

cliffhall commented Apr 18, 2025

@cliffhall updated the readme. Feel free to reword

Looks good, I'm testing now. Needs an npm run prettier-fix to pass CI.

@rinormaloku rinormaloku requested a review from cliffhall April 18, 2025 20:02
Copy link
Contributor

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

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

@rinormaloku This is working now. Since it isn't published yet, I tested with:

Based on your example:

MCP_SERVER_URL=http://localhost:3001 node client/bin/start.js

This doesn't work because that's not enough, it needs to be the full SSE endpoint.

MCP_SERVER_URL=http://localhost:3001/sse node client/bin/start.js
Screenshot 2025-04-18 at 5 27 15 PM

I added a note to change the README example, or better, keep it, and make it a base url, adding the endpoint in the UI.

@@ -60,6 +60,7 @@ The MCP Inspector supports the following configuration settings. To change them,
| `MCP_REQUEST_TIMEOUT_RESET_ON_PROGRESS` | Reset timeout on progress notifications | true |
| `MCP_REQUEST_MAX_TOTAL_TIMEOUT` | Maximum total timeout for requests sent to the MCP server (ms) (Use with progress notifications) | 60000 |
| `MCP_PROXY_FULL_ADDRESS` | Set this if you are running the MCP Inspector Proxy on a non-default address. Example: http://10.1.1.22:5577 | "" |
| `MCP_SERVER_URL` | Set this to configure the default MCP Server to connect to on startup. Example: http://localhost:8080 | "" |
Copy link
Contributor

Choose a reason for hiding this comment

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

  • It would be nice if this could be MCP_SERVER_BASE_URL, and keep the example http://localhost:8080.
  • You would need to add /sse to it in the UI before passing it to the sidebar.
  • Otherwise, this needs to be the full url to an endpoint, not just the server and port.
  • Note soon the /sse part will need to become /mcp.

Copy link
Author

Choose a reason for hiding this comment

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

What about just changing the example? It's likely that people want to change the endpoint as well for example my.domain.com/prefix/sse

Copy link
Contributor

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

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

Just a few notes.

@@ -244,6 +244,10 @@ const App = () => {
if (data.defaultArgs) {
setArgs(data.defaultArgs);
}
if (data.defaultMcpServerUrl) {
setSseUrl(data.defaultMcpServerUrl);
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is to be the full endpoint and not just the location and port of the server, we should look at the path and see if it is /sse or /mcp and set the transport type accordingly.

@@ -24,6 +24,7 @@ const defaultEnvironment = {
...getDefaultEnvironment(),
...(process.env.MCP_ENV_VARS ? JSON.parse(process.env.MCP_ENV_VARS) : {}),
};
const defaultMcpServerUrl = process.env.MCP_SERVER_URL;
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably this environment variable should be MCP_SERVER_ENDPOINT to avoid any confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request waiting on submitter Waiting for the submitter to provide more info
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants