Repository navigation
mcp: read migrated local servers from workspace configuration - #340034
Vritant Bhardwaj (vritant24) merged 3 commits into
Conversation
Use the existing Copilot configuration adapter when parsing workspace-root MCP files and cover wrapped/flat migration round trips. Fixes microsoft#339955. AI-assisted implementation and validation using OpenAI Codex.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused parser change correctly reuses the existing adapter and is covered by appropriate round-trip tests.
Review effort: Balanced
Findings: None
What changed in this PR
Enables migrated local MCP servers to be read from workspace-root .mcp.json files.
Changes:
- Uses the Copilot configuration adapter for workspace-root MCP parsing.
- Adds wrapped and flat configuration coverage.
- Verifies multi-root migration round trips.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Vritant Bhardwaj (vritant24)
left a comment
There was a problem hiding this comment.
Thanks for the fix and the migration round-trip coverage
One additional path needs the same treatment: RootMcpDiscovery._scan() still calls normalizeMcpServerConfiguration directly when reading the workspace-root file. That means migrated servers can reappear in the workbench but still be skipped by agent-session discovery.
Please switch that reader to fromCopilotMcpServerConfiguration too, with a regression test
Use the Copilot configuration adapter for workspace-root MCP discovery, as requested in review. Add wrapped and flat regression coverage with legacy, remote and invalid-entry controls. Validation: both regressions fail before the production fix; 84 checks pass in the official Node runner across discovery, Copilot configuration, workspace configuration and plugin parsers. Official client transpilation, changed-file hygiene/ESLint, and git diff --check pass. Independent AI review found no issues. Full native/Electron setup is unavailable locally because X11/xkbfile development libraries are missing. Locked JavaScript dependencies were installed with lifecycle scripts disabled; no dependency or configuration files changed. Browser/Electron, full repository typechecking, and OS matrices were not run locally. AI-assisted implementation, validation and review using OpenAI Codex.
Vritant Bhardwaj (vritant24)
left a comment
There was a problem hiding this comment.
Can we drop the sessionMcpDiscovery.ts change (and its test)? The agent host shouldn't accept "type": "local" in workspace .mcp.json.
.mcp.json is a defined format: stdio servers are "stdio" (or have no type and a command), and remote servers are "http"/"sse". Accepting "local" here would make a non-standard file look valid in VS Code and could behave differently from what the runtime does.
The VS Code-side parser in mcpWorkspaceConfiguration.ts is handled in #340181. Is that change enough to fix #339955 for you?
Drop the `parseWorkspaceRootMcpConfiguration` change that accepted the Copilot `"local"` type, along with its tests. #340034 makes the same change, so this PR now only covers Copilot CLI user configuration discovery and keeping Copilot-home servers from being forwarded. Refs #339955, #340161 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the agent-host reader change and its two added tests as requested in review. Preserve the original workspace parser fix and wrapped/flat multi-root migration coverage; PR microsoft#340181 handles Copilot CLI user configuration discovery separately. The exact two-file revision passed 134 checks in the official focused Node suites, client transpilation, changed-file hygiene/ESLint and git diff --check. No full local Electron or platform-matrix validation was run. AI-assisted implementation, validation and review using OpenAI Codex.
…ng it (#340181) <!-- Thank you for submitting a Pull Request. Please: * Read our Pull Request guidelines: https://github.com/microsoft/vscode/wiki/How-to-Contribute#pull-requests * Associate an issue with the Pull Request. * Ensure that the code is up-to-date with the `main` branch. * Include a description of the proposed changes and how to test them. --> Part of #340161. Together with #340034, fixes #339955. Servers migrated from user `mcp.json` to `~/.copilot/mcp-config.json` stopped showing up in local chat. This PR makes the local harness discover that file by default, without forwarding its servers to other agents or hosts. The workspace `.mcp.json` side (accepting `"type": "local"`) is handled in #340034. ## Changes **`~/.copilot/mcp-config.json` is discovered by default** Migration moves user-level servers into the Copilot CLI user config, but the `copilot` source of `chat.mcp.discovery.enabled` was off by default. - Added `defaultDiscoverySourceEnablement`, with only `copilot: true`. Claude Desktop, Cursor and Windsurf stay off. - Added `isDiscoverySourceEnabled`, used by `_isDiscoveryEnabled`. A source the user hasn't set falls back to its default. An explicit `false` and the legacy boolean form are still respected. - The configuration service deep-merges a partial user value (e.g. `{ "claude-desktop": true }`) with the default object, so `copilot` stays on in that case too. - Updated the setting description. **Copilot-home servers are never forwarded to other agents or hosts** - The same-machine Copilot CLI session still reads the file itself (`RuntimeDiscovered`). - In every other agent-host session (Claude, Codex, or Copilot on another remote authority) they are `NotDelivered`, with a new `CopilotHomeNotForwarded` reason and message. - This keeps literal `env` and header credentials in `mcp-config.json` from being copied to another provider or machine without approval, now that the source is on by default. ## Notes for reviewers - **Duplicates:** with the default `chat.mcp.collisionBehavior: disable`, a server with the same name in user `mcp.json` (order 200) takes priority, and the `mcp-config.json` copy (order 400) is disabled. - **Servers appearing for existing CLI users:** people with servers in `~/.copilot/mcp-config.json` will now see them in local chat. This includes the remote machine's file in remote windows. These servers are trusted only after the user approves them, so expect a trust prompt. - **Lost forwarding:** users who had opted in to the `copilot` source lose forwarding of these servers to Claude/Codex sessions and other remote hosts. - **Explicit opt-outs:** users who set `"copilot": false`, or the legacy `false`, will still not see their migrated user servers. ## How to test 1. With `chat.mcp.discovery.enabled` unset, add a server to `~/.copilot/mcp-config.json`. It should appear as "GitHub Copilot CLI". 2. Set `chat.mcp.discovery.enabled` to `{ "copilot": false }` or `false`. The server should disappear. 3. Open a Claude or Codex agent-host session. The Copilot-home server should show as unsupported with the "not shared with other agents or remote hosts" explanation. ## Validation - `npm run typecheck-client`: passed - `npm run build-fast -- --client-only`: passed - `./scripts/test.sh --reporter dot --run src/vs/platform/mcp/test/common/mcpWorkspaceConfiguration.test.ts --run src/vs/platform/mcp/test/common/mcpResourceScannerService.test.ts --run src/vs/workbench/contrib/mcp/test/common/workspaceDotMcpDiscovery.test.ts --run src/vs/workbench/contrib/mcp/test/common/nativeMcpDiscoveryAdapters.test.ts --run src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostMcpServerSupport.test.ts --run src/vs/workbench/contrib/chat/test/browser/agentSessions/resolveCustomizationRefs.test.ts --run src/vs/workbench/contrib/chat/test/browser/aiCustomization/mcpServerCustomizationMigration.test.ts`: 181 passing - `npm run eslint -- <changed files>`: clean

Use the existing Copilot configuration adapter when parsing workspace-root MCP files and cover wrapped/flat migration round trips.
Fixes #339955.
AI-assisted implementation and validation using OpenAI Codex.