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

[ulugbekna]

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

// 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:<session>: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:

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: Three standalone MCP/customizations fixes: group-header casing, unclickable Show Output, duplicated server rows #330890.

[ulugbekna]

Adding two implementation details from #330892 (filed concurrently, closing that one as a duplicate of this) so they are not lost.

There are two independent latches, not one. Clearing previous.topLevelId is necessary but not sufficient — _findPublishedMcpCustomization also short-circuits on an already-published top-level and never reaches the child lookup:

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);

So a fix that only addresses the _live entry will still never resolve the child, because the entry it minted earlier is itself found first. Both need to change.

The removal mechanism already exists. The comments concern about orphaning the previously-published top-level id is addressable with code already in this class — _remove emits exactly the right action for exactly this id:

if (entry.topLevelId !== undefined) {
    this._options.emit({ type: ActionType.SessionCustomizationRemoved, id: entry.topLevelId });
    return;
}

So the minted entry can be retired rather than orphaned; it does not need a new protocol action.

Suggested regression test. At the controller level: report a server via applyOne before its declaring child is published, then publish the child, and assert the session ends with exactly one McpServerCustomization for that name.