Support server.json being returned by assisted MCP install #259634
Conversation
joelverhagen
changed the title
| } | ||
|
|
||
| protected toScannedMcpServerAndInputs(manifest: IMcpServerManifest, packageType?: PackageType): { config: IMcpServerConfiguration; inputs?: IMcpServerVariable[] } { | ||
| static toScannedMcpServerAndInputs(manifest: IMcpServerManifest, packageType?: PackageType): { config: IMcpServerConfiguration; inputs?: IMcpServerVariable[] } { |
There was a problem hiding this comment.
@joelverhagen wondering why this is made public and static? This is an internal implementation of handling gallery mcp server installations. Now it is being reused for other installation flows which will make difficult to do any changes/enhancements for gallery installation flows.
There was a problem hiding this comment.
Our general guideline is to use expose APIs(methods) on services instead of such static functions.
There was a problem hiding this comment.
Hey @sandy081, apologies that I didn't follow that guideline. I was looking for a way to perform the mapping from server.json to VS Code mcp.json and this seemed like the simplest way to get it done. It sounds like what you're saying is that this metho should be made public but not static, and some implementation of AbstractMcpResourceManagementService is injected via DI instead? Or am I misunderstanding?
5 participants
This works around the lack of #259100 and is progress on #257679. There will be a separate PR on the chat extension to support server.json in NuGet packages.
It enhances the
github.copilot.chat.mcp.setup.flowcommand to have an additionaltypeproperty. When this type property is set to"server.json", the result is treated as an MCP Registry server.json and mapped to the VS Code configuration format. If thetypeproperty is falsey set or set to"vscode", the existing codepath is used.Additionally, I updated the
toScannedMcpServerAndInputsto be a public static method so it can be called after thegithub.copilot.chat.mcp.setup.flowcommand returns.I noticed the resulting
argsandenvplaceholder values had the format{input:foo}, instead of format${input:foo}. Based on https://code.visualstudio.com/docs/reference/variables-reference#_input-variables, it seems like this was a bug in the original implementation, but I may be wrong. @sandy081 - could you confirm?Verified with:
@azure/mcp(old flow, no server.json)mcp-server-fetch(old flow, no server.json)mcp/node-code-sandbox(old flow, no server.json)NuGet.Mcp.Server(new flow),Knacode.SampleMcpServer(new flow, this one has an input)