Skip to content

add default MCP server URL on startup#288

Closed
rinormaloku wants to merge 4 commits intomodelcontextprotocol:mainfrom
rinormaloku:set-default-mcp-server-url
Closed

add default MCP server URL on startup#288
rinormaloku wants to merge 4 commits intomodelcontextprotocol:mainfrom
rinormaloku:set-default-mcp-server-url

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
Member

@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 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
Member

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
Member

@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.

| `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
Member

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
Member

@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.

setArgs(data.defaultArgs);
}
if (data.defaultMcpServerUrl) {
setSseUrl(data.defaultMcpServerUrl);
Copy link
Member

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.

...getDefaultEnvironment(),
...(process.env.MCP_ENV_VARS ? JSON.parse(process.env.MCP_ENV_VARS) : {}),
};
const defaultMcpServerUrl = process.env.MCP_SERVER_URL;
Copy link
Member

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.

@olaservo
Copy link
Member

Hi @rinormaloku ! I'm going to close this since it hasn't been updated in a while, but please feel free to re-open if you'd still like to come back to it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature 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.

3 participants