Agent host mints a top-level MCP customization and never retires it, leaving two customizations for one server

[ulugbekna]

Follow-up from #330890, which fixed the user-visible symptom in the workbench. The underlying session state is still wrong, so this tracks the actual repair in the agent host.

The problem

McpCustomizationController._applyOne (src/vs/platform/agentHost/node/shared/mcpCustomizationController.ts) mints a top-level McpServerCustomization for any SDK-reported server whose declaring child cannot yet be resolved by name, and then never retires it once the child does become resolvable. The session ends up carrying two customizations for one server.

From a real user's AHP logs:

notion -> file:///.../agentPlugins/vscode-synced-customization-.../.mcp.json#mcp=notion
          state: stopped                 <- the declaration
notion -> mcp-top-level:copilotcli:<session>:notion
          state: ready, channel: mcp://  <- the live one

Replaying 887 customization snapshots from that session, 34 contained a duplicated server.

Why it sticks

Two independent latches, either of which is enough on its own:

1. The _live entry. _applyOne reads previous?.topLevelId and short-circuits when it is set:

   // 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;
   if (topLevelId === undefined) {
       // ...only here is _findPublishedMcpCustomization consulted
   }

   So the child-resolution attempt happens only on the first update for a server. Whether a child has since been published is never re-checked.

2. _findPublishedMcpCustomization. Even if the _live latch were cleared, this returns early on a published top-level 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);

The ordering that triggers it is routine, not exotic: the SDK reports a server before the plugin/directory scan has published the .mcp.json that declares it. That is the common case for the customization bundle VS Code syncs into the agent.

Why the comment is the bug

"Once promoted to a top-level entry, stay top-level for the session" reads as a requirement, but the stated justification — "flipping back to a child mid-stream would orphan the previously-published top-level id" — is a description of a problem that already has a solution in this same class. _remove emits ActionType.SessionCustomizationRemoved 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. The suggested behaviour: when _resolveChildId(serverName) starts answering for a server currently published as top-level, emit SessionCustomizationRemoved for the minted id, clear topLevelId on the _live entry, and switch to emitting SessionMcpServerStateChanged against the child from then on. _findPublishedMcpCustomization would need to stop short-circuiting on a top-level it minted itself, or the child lookup will never be reached.

Scope

#330890 makes the presentation path correct: getMcpServers drops a child that a top-level customization already speaks for, so the servers list renders one row per server. That is a workbench-side dedupe and it does not change session state.

Anything reading session state directly still sees both customizations. Two known consequences today:

• The duplicate is still mirrored into MCP diagnostics, which walk every customization by design (correctly — an id from either copy must resolve).
• Any future consumer of getCustomizations has to know to dedupe, or it inherits the bug. The workbench fix is deliberately narrow and does not generalize.

Fixing it here would let the workbench-side dedupe eventually be removed rather than becoming load-bearing.

Notes

• The duplicate is not always benign in the UI even after the dedupe: the list's ActiveSessionMcpServerMatcher.take() only matches when exactly one candidate answers a key, so extra copies in state can still prevent a local row from adopting its session twin in paths that do not go through the deduped accessor.
• Worth a 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.

[ulugbekna]

Duplicate of #330893, which was filed 16 seconds earlier from a parallel session on the same investigation. The two details unique to this issue — that _findPublishedMcpCustomization is a second, independent latch, and that _remove already provides the removal mechanism the code comment worries about — have been carried over there. Closing this one to keep the discussion in one place.