microsoft / microsoft/vscode

agentHost: a minted top-level MCP customization is never retired, so one server stays published twice

Open
#330,893 1 comment 0 reactions 1 assignee Assigned to @ulugbekna View on GitHub
Dominant language
TypeScript
Stars
193k
Forks
42.4k
PR merge metrics
PR metrics pending

Description

## Summary

An agent-host session can end up publishing **two customizations for a single MCP server**, and both persist for the life of the session. The workbench now de-duplicates this for display (#330890), but the underlying session state still carries both entries, so anything reading that state directly still sees one server twice.

## Root cause

`McpCustomizationController` publishes a server's declaration as a **child** of whatever declared it (a plugin, or the generated `.mcp.json` that VS Code syncs into the agent). Separately, `_applyOne` mints a **top-level** customization for any server the SDK reports before that child can be resolved by name:

https://github.com/microsoft/vscode/blob/main/src/vs/platform/agentHost/node/shared/mcpCustomizationController.ts

```ts
// Once promoted to a top-level entry, stay top-level for the
// session — flipping back to a child mid-stream would orphan the
// previously-published top-level id.
let topLevelId = previous?.topLevelId;
```

That comment is the bug rather than a requirement. Once `resolveChildId(serverName)` starts answering, the minted top-level entry is redundant, but it is never retired — so the session state holds both copies from then on.

The two are not equivalent, which is what makes it user-visible:

```
notion -> file:///.../agentPlugins/vscode-synced-customization-.../.mcp.json#mcp=notion
state: stopped <- the declaration; never leaves stopped
notion -> mcp-top-level:copilotcli::notion
state: ready, channel: mcp:// <- the live one the host keeps current
```

## Evidence

From a real user session's exported AHP logs: replaying **887 customization snapshots**, **34** contained at least one duplicated server name. The pairs always had the shape above — a `stopped` child with no channel alongside a live top-level entry.

## Impact

Before #330890 this rendered as two rows in the MCP Servers list with contradictory status. It was worse than a visual repeat: `ActiveSessionMcpServerMatcher.take()` only matches when *exactly one* candidate answers a key, so with two copies the user's local row could not adopt either, and both fell through as additional rows.

#330890 fixes the presentation path only (`getMcpServers` de-duplicates; log, diagnostics and id lookups deliberately still walk every customization so an id from either copy resolves). The session state itself is unchanged.

## Suggested fix

**There are two independent latches here, and clearing only the first will look like a fix while changing nothing.** (Credit to the follow-up analysis in the closed duplicate #330892 for catching the second; verified against `main` while writing this.)

**Latch 1 — the live entry.** `_applyOne` reuses `previous?.topLevelId` and never clears it.

**Latch 2 — the resolver.** `_findPublishedMcpCustomization` returns as soon as it finds a published *top-level* customization with that name, and never reaches `findMcpChildId`:

```ts
const topLevel = customizations.find(c => c.type === CustomizationType.McpServer && c.name === serverName);
if (topLevel?.type === CustomizationType.McpServer) {
return { topLevelId: topLevel.id };
}
const childId = findMcpChildId(customizations, serverName);
```

Since `_resolveChildId` is just `_findPublishedMcpCustomization(serverName)?.childId`, it returns `undefined` for the rest of the session once a top-level entry exists — *even after the child is published*. So a fix that only clears the live entry will call `_resolveChildId`, get `undefined`, and re-mint or reuse the same top-level id. Both latches have to be released together.

**The removal mechanism already exists**, so the orphaning concern in the comment is addressable without new protocol surface: `_remove` already emits `SessionCustomizationRemoved` for exactly `entry.topLevelId`. The fix is to retire the minted entry the same way once the declaring child becomes resolvable, then point the live entry at the child id.

**Suggested regression test** (controller level): report a server via `applyOne` *before* its child is published, then publish the child, then assert exactly one `McpServerCustomization` exists for that name.

## Notes

- Discovered while investigating duplicate MCP server rows reported in the Agents window.
- Workbench-side mitigation: #330890.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.