add default MCP server URL on startup#288
add default MCP server URL on startup#288rinormaloku wants to merge 4 commits intomodelcontextprotocol:mainfrom
Conversation
There was a problem hiding this comment.
Looks good (I haven't tested yet) but should be documented in the configuration section of the README.
|
@cliffhall updated the readme. Feel free to reword |
Looks good, I'm testing now. Needs an |
cliffhall
left a comment
There was a problem hiding this comment.
@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.jsThis 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
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 | "" | |
There was a problem hiding this comment.
- It would be nice if this could be
MCP_SERVER_BASE_URL, and keep the examplehttp://localhost:8080. - You would need to add
/sseto 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
/ssepart will need to become/mcp.
There was a problem hiding this comment.
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
| setArgs(data.defaultArgs); | ||
| } | ||
| if (data.defaultMcpServerUrl) { | ||
| setSseUrl(data.defaultMcpServerUrl); |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
Probably this environment variable should be MCP_SERVER_ENDPOINT to avoid any confusion.
|
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! |
Tiny change to pass the default MCP Server through the CLI, e.g.:
Makes the docs for MCP servers with SSE support a little cleaner.