mcp: read migrated local servers from workspace configuration by Tabisharaza · Pull Request #340034 · microsoft/vscode · GitHub
Skip to content

mcp: read migrated local servers from workspace configuration - #340034

Merged
Vritant Bhardwaj (vritant24) merged 3 commits into
microsoft:mainfrom
Tabisharaza:fix/workspace-mcp-local-readback
Oct 8, 2026
Merged

Vritant Bhardwaj (vritant24) merged 3 commits into
microsoft:mainfrom
Tabisharaza:fix/workspace-mcp-local-readback

Conversation

@Tabisharaza

Copy link
Copy Markdown
Contributor

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.

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.
Copilot AI balanced review requested due to automatic review settings October 6, 2026 12:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
File Description
src/​vs/​platform/​mcp/​common/​mcpWorkspaceConfiguration.ts Parses Copilot-style local servers.
src/​vs/​platform/​mcp/​test/​common/​mcpWorkspaceConfiguration.test.ts Tests both supported document shapes.
src/​vs/​workbench/​contrib/​chat/​test/​browser/​aiCustomization/​mcpServerCustomizationMigration.test.ts Tests migrated configurations across workspace roots.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Tabisharaza

Copy link
Copy Markdown
Contributor Author

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Vritant Bhardwaj (vritant24) added a commit that referenced this pull request Oct 8, 2026
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.
Vritant Bhardwaj (vritant24) added a commit that referenced this pull request Oct 8, 2026
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP migration writes type: "local" to workspace .mcp.json, which VS Code can't read, so the servers disappear

5 participants