mcp: initial work #243221
mcp: initial work #243221
Conversation
This is some basic work to get MCP hooked up. It's enough to discover and establish connections to MCP servers on the machine with very very basic commands to manage them:  Refs #242864 The McpRegistry registers both collections of servers (from various config files) and 'delegates', which is currently _only_ the node extension host but is pretty generic and so could point at other processes in the future. SSE could even be served from the renderer when we aren't on a remote. It wraps into IMcpServerConnection's, which do some basic connection management and expose the McpServerRequestHandler which speaks JSON-RPC to the MCP server. This is wrapped into the IMcpServer which is the complete, stateful representation of the server. It does stuff like caching discovered tools such that they can be viewed and controlled even when the MCP server isn't running. The IMcpService is the 'public' entry for other VS Code features. Its API is very simple right now, exposing an observable of the available servers, which should be easy for chat to consume. Still need to get some good tests going, add proper and more discovery, and excercise currently-untested API. SSE is also a stub right now.
- register mcp server tools as tools - add `IMcpTool#id` - make sure `callOn` awaits result before disposing fyi @connor4312
- There's an mcpDiscoveryRegistry that allows components to register in how they discover MCP servers. - Config discovery is one of these. Figured out all the bits for a standalone file config shebang. Duplication there that could be cleaned up but it works. - The others are remote and local filesystem discovery. I ended up making another message channel for the main process/remote server to get a couple environment variables we need since I didn't see anything generic for this already(?) 
…zed` after init-response has been received
There was a problem hiding this comment.
Curious, is there more things planned for the channel in the future that warrant it to be its own thing?
I suggest to use ProxyChannel.fromService, you don't have to implement the channel yourself.
//cc @sandy081 for config changes
Not right now. But I didn't see that we had other ways to get environment variables outside of the common ones from IEnvironmentService/IRemoteEnvironment. I'd be very happy to do away with the extra channel if there's something I missed. Or maybe we put APPDATA and the XDG_CONFIG_HOME directory in these two interfaces, although it's quite possible we'll need more values later. What do you think? |
|
actually, I suggest keeping the channel for now regardless since I might need to do more on-machine stuff for other configs over the next few days. But I'll move to ProxyChannel.fromService. |
…vers being around
|
|
||
| for (const pick of picks) { | ||
| if (pick.type === 'item' && !seen.has(pick.tool)) { | ||
| pick.tool.updateEnablement(false); |
There was a problem hiding this comment.
I suppose in the future the enablement state could be per-chat view, wouldn't worry about it for now...
There was a problem hiding this comment.
Yeah, I was thinking about this and wanna do that but ATM we don't have a way to express this. Tools only have the global when-clause and doing something session-specific would require new API which enables certain tools only for certain sessions. Like sending along tools with the request like ChatRequest#toolReferences but without the reference treatment, e.g without the rendering
| async prepareToolInvocation(parameters, token) { | ||
| return { | ||
| confirmationMessages: { | ||
| title: localize('aaa', "Run tool from {0}", server.definition.label), |
There was a problem hiding this comment.
We should figure out how to have a nicer looking confirmation message, and auto-run at some point of course. You might want to fill in the localization ids at least
There was a problem hiding this comment.
Any problem with 3P extensions being able to call these? I suppose not
There was a problem hiding this comment.
You might want to fill in the localization ids at least
I think nls ids aren't surfaces anywhere, afaik it's all array indicies and not even the translators see them. We only use them as dedupe-key (which could be inferred from the value and extra-args). That we still write them is debt and we could do what we have done for the API - just strings w/o ids
There was a problem hiding this comment.
We should figure out how to have a nicer looking confirmation message, and auto-run at some point of course.
Enlighten me. Is there more/better API for this? Ideally something that works for all tool calls and not special-cases for MCP-tools vs normal tools
There was a problem hiding this comment.
No, I'm just thinking more along the lines of something like a "display name" for these tools, but maybe that doesn't exist in MCP?
Probably this confirmation should include the arguments too?
addressed, future changes can be in future PRs!
|
Awesome |
This adds initial support for MCP discovery and connections in VS Code.
.vscode/mcp.jsonMCP: List Serversthat allows listing and manually starting/stopping serversGranular changes can be found in commits within this PR. Closes #242864, more issues will come under the
chat-mcptag for future work.