Skip to content

MCP servers UI: state you can read, controls you can reach - #330943

Open
Ulugbek Abdullaev (ulugbekna) wants to merge 13 commits into
mainfrom
ulugbekna/mcp-ui-rework-on-new-enablement
Open

MCP servers UI: state you can read, controls you can reach#330943
Ulugbek Abdullaev (ulugbekna) wants to merge 13 commits into
mainfrom
ulugbekna/mcp-ui-rework-on-new-enablement

Conversation

@ulugbekna

@ulugbekna Ulugbek Abdullaev (ulugbekna) commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Supersedes #330207.

Stacked on #330890base branch is ulugbekna/mcp-standalone-fixes, so this PR shows only its own delta. It auto-retargets to main when that merges.

Why this exists

#330207 redesigned this tab and was then overtaken by upstream: main replaced the agent-host enablement layer it was built on. A rebase produced 13 conflicts that were architectural rather than textual, so it was aborted. This PR ports that PR's presentation work onto upstream's model and drops its enablement machinery entirely.

Upstream's model wins on every axis, so none of it is reintroduced here:

#330207 main today, used here
get/setMcpServerEnablement, keyed by server name setCustomizationEnablement(...), keyed by customization id
resolveMcpDisabledState, formatMcpStatusWithScope, getStatusScopeNote, McpEnablementScope getCustomizationDisabledLabel + CustomizationDisabledReason — which also knows Disabled (Plugin)
IAgentHostDurableEnablement deleted
mcpServerIdentity.ts already inlined in mcpListWidget.ts

What changes

Status became legible. getMcpStatusPresentation returned no icon for Stopped, and the render path bailed out on a missing icon — so the most common row in the list drew zero pixels, and the rows that did draw asked you to infer meaning from a colour. Status is now a word beside the name with a colour dot, shown only when it adds something: Failed, Sign-in needed, Starting, scope-qualified Disabled (…). Running/Idle are suppressed (MCP servers start lazily; liveness is noise), and a bare Disabled is suppressed on a row whose switch already says it. The accessible name deliberately keeps the plain Disabled — a screen reader reads the row label without the switch beside it.

Every row that can be turned off has a switch. Previously right-click only. Failing rows get a labelled Show Output button instead of a coloured icon that was both the only sign of failure and the only way to read why.

Rows say what a server offers. Tool counts read from cache state, so they survive an idle server. Line two is origin · transport · (error or description).

Four sections by origin — User, Workspace, Extensions & Plugins, Built-in — with agent-contributed servers folded into Built-in, naming their product on the row.

The detail pane is worth opening: full tool list with descriptions, plus the command/address. Env-var and header values are never rendered, only names — they routinely carry tokens.

The switch's write semantics

The decisive decision is enablement[0], so the switch replaces that kind — this is what actually moves the effective state, and it answers a deliberate workspace or session choice where it was made instead of silently promoting it. With nothing decided yet it writes Global: an unqualified control means the whole durable answer.

Deliberately not IAgentHostMcpServer.setEnabled, which only writes the session layer. Reads and writes are live, because the render guard cannot cover every decision in an enablement array and writing back a captured one would resurrect a scope the user had since changed.

Local enablement is keyed by the workbench server id — the key EnableMcpServerGloballyAction already uses — not the matched runtime definition.id. That match is deliberately conservative and declines whenever two servers share a name; a row must not lose its switch, or misreport its status, over an ambiguity in a lookup it never needed.

Two deliberate exceptions (please read as intent, not oversight)

  • A gallery result gets no switch — nothing is installed to turn on.
  • A server disabled by its owning plugin gets no switch. Writing its own enablement cannot free it, so it keeps the word Disabled (Plugin) and the context menu's Enable {plugin} rather than a control that would appear to do nothing.

Both were confirmed with the requester before implementation.

Known limitation

withCustomizationEnablement replaces one kind at a time, so a server held off at both Global and Session cannot be turned "on everywhere" in a single gesture. Writing the deciding kind always moves the effective state, but a broader false survives and re-asserts later (e.g. in a new session). This matches the context menu's own per-scope semantics — silently deleting a broader durable choice would be worse. Documented in AI_CUSTOMIZATIONS.md.

Note for the #330890 reviewer

This widens IMcpStatusRenderInput (adds switchChecked, errorMessage, toolCount, toolsFromCache, transport, description, origin, impliedOrigin; removes statusIconId along with the icon it described).

That is a consequence of the design, not an oversight: line two draws on transport and errorMessage, which are only known once the row's observables have been read, so it genuinely cannot be built in renderElement any more. The failure mode is silent — a field left out doesn't throw, it freezes that text on screen until something else happens to move — so the mapped-type test was left to drive every addition. It caught me twice.

The guard is also now fed the live agent-host server rather than the entry's snapshot. logOutputChannelId and the scoped id are pure derivations of the session and customization id, so the signature still only moves when the server is genuinely republished — and it fixes an output action that could stay bound to a dead channel.

This PR also changes the Plugins tab

Not obvious from the title, so stating it plainly. pluginListWidget.ts reuses this tab's row CSS — // Installed Plugin Renderer (reuses .mcp-server-item CSS) — and both of its renderers add .mcp-server-item. Two shared rules changed:

.mcp-server-item.disabled — was a blanket opacity: 0.5 on the whole row. It now dims only the name and description, so a disabled row's status and error text stay readable (they are the reason you are looking at the row). Both plugin renderers set .disabled, so the "Disabled Locally" group on the Plugins tab dims slightly differently. Expect PluginsTab / PluginsTabScrolled baselines to move a little.

.mcp-server-status — was a fixed 24×24 box with justify-content: center and font-size: var(--vscode-codiconFontSize), i.e. sized for a codicon glyph. But PluginRemoteItemRenderer writes text into it ("Loading", "Loaded", "Warning", "Error", "Disabled"), so remote-plugin status has been clipping into a 24px box. My text-shaped rule fixes that.

That second one is a fix, not a side-effect — but no baseline will show it, because no fixture exercises the remote-plugin renderer. Flagging it so "shared rule changed, nothing moved" doesn't read as suspicious. It is visible only against a remote agent host.

Reactivity

getMcpServers is a plain read, so a derived over it never re-runs. ActiveSessionMcpServerReader folds it into the reactive graph via observableSignalFromEvent(onDidChangeCustomizations). Aria labels are IObservable<string> because a row's status changes without the list being spliced.

Session servers are claimed against the unfiltered lists and filtered afterwards — take is consuming, so a row hidden by the search box would otherwise release its claim. The sidebar badge is computed the same way; it previously grew while you typed.

The detail pane does not re-derive the join: the matcher is consuming and order-dependent, so a fresh match there can adopt a server the list already gave to another row. The row's claim travels with the selection.

Validation

  • tsc --project src/tsconfig.json --noEmit clean — and each of the 10 commits typechecks standalone (verified in an isolated worktree)
  • 39 mcpListWidget + 4 enablement + 8 agent-host MCP tests passing
  • eslint + stylelint clean
  • 82 component fixtures rendered and visually reviewed; adds a scrolled active-session fixture, since Built-in is where agent servers land and no screenshot could reach it
  • The fixtures gained a working enablement model — the stub they had rendered every server as off, and failed silently because the read happens inside an autorun

`text-transform: capitalize` on the shared group header re-cased strings that
were already cased correctly, and got them wrong: "Included Based on Context"
rendered as "Included Based On Context" and "Loaded on Demand" as "Loaded On
Demand", against this repo's own rule that short prepositions stay lowercase,
and "Built-in" rendered as "Built-In".

Per-word capitalization is also not a transform that survives translation, so
the rule was wrong for every locale rather than just awkward in English.

The header is shared, so this reaches every customizations tab -- Agents,
Skills, Instructions, Hooks, Prompts, Plugins and MCP Servers. Every group
label on those tabs is already a correctly cased localized string, so the only
rendered differences are the three the transform was getting wrong.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`updateStatus` began by clearing the row's action disposables and emptying its
actions node, then rebuilt them. It runs from an autorun over the server's
connection state, and an erroring server re-runs it about twice a second while
producing byte-identical content: measured in a real Code OSS build, 9-10
rebuilds per 5 seconds, every one of them a no-op.

A DOM node replaced between mousedown and mouseup never receives the click, so
the inline `Show Output` button did nothing on precisely the rows that needed
it -- the failing ones, which are the only rows that offer it at all.

The row's actions are now rebuilt only when something about them changed.
`getMcpStatusRenderSignature` reduces them to a comparable value covering both
what they render and what they act on; leaving anything out would drop an
update that matters, so it is a pure exported function with a test that fails
to compile if a field is added without being covered.

The list re-splices on every customizations change, so `renderElement` would
otherwise undo this by clearing the actions itself. It now keys on the row's
content identity rather than the entry object, which is recreated on every
refresh and therefore says nothing about whether this is the same row.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A session can carry two customizations for a single MCP server. The agent host
publishes the declaration as a child of whatever declared it -- a plugin, or
the .mcp.json VS Code syncs into the agent -- and separately mints a top-level
customization for any server the SDK reports before that child can be resolved
by name. `McpCustomizationController._applyOne` never retires the minted entry
once the child becomes resolvable ("Once promoted to a top-level entry, stay
top-level for the session"), so both remain in state:

    notion -> file:///.../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

Every consumer of getMcpServers saw both, so the servers list rendered the same
server twice with contradictory status. It was worse than a repeat: the list's
matcher only matches when exactly one candidate answers a key, so with two
copies the server's local row could not adopt either, and both fell through as
extra rows.

getMcpServers now drops a child that a top-level customization already speaks
for. The top-level copy wins because it is the one the host treats as live: it
carries the running state and channel, and its id is what the host resolves for
lifecycle and enablement. Position in the tree is the signal, not the shape of
the minted id, which is the host's own business -- and not the absence of an
owning plugin either, since a directory-declared child has none.

Nothing else is collapsed. Two plugins that each declare a server named
`search` stay two rows, because they are two servers. Only the presentation
path dedupes; log, diagnostics and id lookups still walk every customization,
so an id from either copy continues to resolve.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
UI that needs to name the agent cannot derive it from the descriptor's
label: that string is localized and carries a disambiguating suffix, so
stripping the suffix would break in translation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A contribution turned off for this workspace should come back on for this
workspace, and one turned off everywhere should come back on everywhere.
Promoting or demoting the scope behind the user's back rewrites a choice
they made deliberately, and a plain on/off control shows the scope only
while the row is off, so they would not even see it happen.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Enablement used to be reachable only through a right-click menu, which
meant the most common thing someone wants to do with a server was also
the least discoverable. This gives a row one control in one place, so the
eye learns a single target while scanning a long list.

The accessible name is the subject rather than the act: role=switch
announces on/off from aria-checked, so an action phrase would read
"Disable Redis, switch, on" -- a label arguing with the state beside it.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There were three paths deciding what a row's status was: two that shared
updateKnownServerStatus and a third that computed the same answer inline.
They agreed only by coincidence, and this is the same file whose history
says copies of a status rule had already drifted apart.

The accessible name now comes from that one resolver too, as an
observable. It has to be one: a row's status changes without the list
being spliced, so a plain string is computed once and then describes the
past. getMcpServers is a plain read rather than an observable, so
agent-host state is folded into the same reactive graph through a signal
over onDidChangeCustomizations -- without it a derived over it would
never re-run.

A session row whose server momentarily leaves the service now keeps
showing what it last knew instead of blanking, because both paths share
the same fallback.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Status was a bare coloured icon in the far-right actions slot, and
getMcpStatusPresentation returned no icon for Stopped while the render
path bailed out on a missing one -- so the most common state in the list
drew zero pixels, and the states that did draw asked the user to infer a
meaning from a colour.

It is now a word beside the name, where the eye already is when reading
which server a row is, with a dot alongside so the distinction is never
carried by colour alone. Running and Idle are left unsaid: MCP servers
start lazily, so whether one holds a process right now is an
implementation detail that flickers and that nobody acts on.

Failing rows get an explicit 'Show Output' button rather than a coloured
icon that was both the only sign a row had failed and the only way to
read why.

Wording follows: 'Idle' rather than 'Stopped' for a resting state,
'Failed' rather than 'Error' because a row reports what happened rather
than being an error itself, and 'Sign-in needed' in the same words as the
button beside it.

A locally disabled row now reports its scope through the same
getCustomizationDisabledLabel path the agent-host rows use, so
'Disabled (Workspace)' cannot be worded two ways.

statusIconId leaves IMcpStatusRenderInput with the icon it described.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Turning a server off was reachable only through a right-click menu, so
the most common thing someone wants to do with a server was also the
least discoverable. Every row that has an enablement to write now carries
one switch, in the same place, so it is a single predictable target.

The switch writes the layer that already decides the row, falling back to
Global when nothing decides it yet. Replacing the decisive kind is what
actually moves the effective state, and it answers a deliberate workspace
or session choice where it was made rather than silently promoting it. An
unqualified control means the whole durable answer, so a first 'off' is
off everywhere; narrower scopes stay explicit, in the context menu.
Deliberately not IAgentHostMcpServer.setEnabled, which only ever writes
the session layer.

Rows held off by two layers align both, because a switch that leaves a
server visibly off after being turned on is a broken switch. Both sides
read live rather than through values captured at render: the render guard
cannot cover every decision in an enablement array, and writing back a
stale one would resurrect a scope the user had since changed.

Two rows get no switch. A gallery result has nothing installed to turn
on. And a server held off by the plugin that owns it cannot be freed by
writing its own enablement at all, so it keeps the word
'Disabled (Plugin)' and the context menu's 'Enable {plugin}' instead of a
control that would appear to do nothing.

Enablement is keyed by the workbench server's own id -- the key
EnableMcpServerGloballyAction already uses -- rather than the matched
runtime server's definition id. The runtime match is deliberately
conservative and declines whenever two servers answer to one name, and a
row must not lose its switch, or misreport its status, because of an
ambiguity in a lookup it never needed.

The row no longer prints a bare 'Disabled' beside an off switch, which
was the same fact twice. A qualified reason still shows, because it says
where the choice lives and the switch cannot express that. The accessible
name deliberately keeps saying it: a screen reader reads the row label
without the switch beside it.

The fixtures gain a working enablement model. The stub they had returned
nothing, which rendered every server as off -- and failed silently, since
the read happens inside an autorun.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A row said a server's name and its description. It did not say how many
tools the server provides -- the whole reason to install one -- and when
a server failed it said so only through a coloured icon, leaving the
reason readable exclusively by opening the output channel.

Tool counts are read from the cache state, so they survive a stopped
server: what a server offers is exactly what someone deciding whether to
turn it back on is looking for. A first refresh in flight is not treated
as a known empty result, and a count that came from the cache reads
quieter and says so on hover.

Line two becomes transport, then either the failure or the description.
A failure replaces the description and drops the transport with it: when
something is broken that is the only thing on the line worth the user's
attention, and truncating the error to make room for 'Local' helps
nobody.

The error hover is anchored to the line the error is printed on rather
than the row. Built-in rows already register a provenance hover on the
container, and the hover service keys delayed hovers by element, so two
registrations on one element would overwrite each other.

This widens IMcpStatusRenderInput. Line two genuinely cannot be built in
renderElement any more -- transport and the error are only known once the
row's observables have been read -- so it is drawn inside the guard, and
every value it draws from has to be covered. The failure mode is silent:
a field left out does not throw, it freezes that text on screen until
something else happens to move. The mapped type in the test is what makes
that a compile error rather than a bug report.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Five sections became four. Extension and Plugin were two headings for one
thing as a user experiences it -- software they installed -- so they now
share a section, and the distinction survives on the row, where the
header cannot say it. Ordering leads with the user's own settings rather
than the workspace, then moves outwards to the product's.

Servers only the agent knows about join Built-in rather than getting a
section of their own. They arrive because you are using that agent, which
is the same reason VS Code's own servers are there; the row names which
product it was, using the agent's own name where the harness reports one.

Line two gains the origin, but only where the header does not already say
it, so a row stops repeating what it sits under. A row with nothing else
to say falls back to naming its origin anyway, because a gap under the
name reads worse than repetition.

Session servers are claimed against the unfiltered lists and filtered
afterwards. Claiming is consuming, so claiming against filtered lists let
a row hidden by the search box release its claim for another row to pick
up. Claiming first keeps the join a property of the configuration rather
than of what happens to be on screen.

The sidebar badge counts what the user has rather than what the current
search matched. It was derived from the filtered arrays, so typing in the
search box silently rewrote the tab's badge -- and the consumed matcher
could not answer the question either, since the narrower the query the
more servers it reported as session-only.

Adds a scrolled active-session fixture: Built-in is where the agent's own
servers land, and it was the one section no screenshot could reach.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The pane showed a name, a scope, and the same description already on the
row, so opening a server told you nothing you had not just read. It now
answers the two questions a row has no room for: what the server can do,
and what it actually runs.

Tools are listed in full, with their descriptions, because that is the
whole reason to install a server and was otherwise visible only as a
count. A server that has never run says so rather than showing an empty
list that reads as "this server offers nothing", and one that is merely
off is told to turn on instead.

Configuration shows the command or address. Environment variables and
HTTP headers are reduced to their names: knowing a server reads
GITHUB_TOKEN is the useful part, and printing the token itself into a
pane that can be screen-shared is not a trade worth making. It is read
from the installed configuration rather than the resolved launch, so it
is what the user wrote and is available whether or not the server has
ever started.

Configuration sits above the tool list even though tools matter more,
because it is short and fixed-height while the tool list is unbounded.
The other order pushes it below the fold for any server with more than a
handful of tools.

The pane no longer re-derives which agent-host server a row is showing.
The matcher is consuming and order-dependent, so matching again here
starts from zero claims and can adopt a server the list had already given
to a different row -- the exact disagreement this is meant to prevent.
The row's claim now travels with the selection instead.

Status is resolved through the same helpers the row uses. Unlike a row
this pane has no switch, so it does say "Disabled"; Running and Idle stay
unsaid, since whether a lazily-started server holds a process right now
is not why anyone opened it.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Documents the four sections and the question they answer, the status
suppression rules and why the accessible name deliberately differs from
the row, the switch's write rule and the two rows that get no switch, the
claim-before-filter invariant behind both the row join and the sidebar
badge, and the secret-elision rule in the detail pane.

Also records the one thing upstream's enablement model cannot express:
withCustomizationEnablement replaces one kind at a time, so a server held
off at two scopes cannot be turned on everywhere in a single gesture.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
if (topLevelNames.size === 0) {
return entries;
}
return entries.filter(entry => entry.isTopLevel || !topLevelNames.has(entry.server.name));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI Review: Do not hide every same-name child

With two plugins declaring search and a top-level search, this filter removes both plugin children. At most one can be the live entry's twin; the other server, provenance, and row actions disappear. Deduplicate using stable identity/provenance, or only collapse when exactly one child matches.

// claims, so with two rows answering to one name this pane would happily adopt the
// server the list had already given to a different row -- the exact disagreement this
// is supposed to prevent.
this.renderStatus(server, runtime, this.activeSessionServer, reader);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI Review: Refresh the active-session server snapshot

getMcpServers() creates fresh wrapper snapshots, but customization changes rerender the object captured by setInput. Authentication, failure, recovery, and enablement changes can therefore remain stale while the pane is open. Keep the selected claim's stable ID/session resource and reacquire that exact wrapper on change rather than rerunning the consuming matcher.

}

private renderStatus(server: IWorkbenchMcpServer, runtime: IMcpServer | undefined, sessionServer: AgentHostMcpServer | undefined, reader: IReader): void {
const enablement = runtime?.enablement.read(reader);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI Review: Resolve detail enablement independently of runtime matching

A missing or ambiguous runtime match makes this pane ignore durable disablement that the list still reads by server.id. Successful workspace-disabled matches are also labeled Global at line 239. Read mcpService.enablementModel by the workbench server ID and derive the actual deciding scope, matching the list's row-state logic.

if (config?.type === McpServerType.LOCAL) {
facts.push({
label: localize('mcpDetailCommand', "Command"),
value: [config.command, ...(config.args ?? [])].join(' '),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI Review: Keep argument and URL credentials hidden by default

Environment and header values are hidden for screen-sharing safety, but command arguments are rendered verbatim here and remote URLs are rendered verbatim below. Tokens commonly appear as argument values, URL userinfo, or signed query parameters. Keep these values hidden or sanitized by default and offer an explicit reveal action rather than relying on secret detection.

// than an empty list, which reads as "this server offers nothing" -- but only a server
// that is actually off should be told to turn on, or the pane asks the user to flip a
// switch they can see is already flipped.
const isOff = enablement !== undefined && isContributionDisabled(enablement);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI Review: Include session enablement in the tools prompt

When the local runtime is enabled but the active session disables this server, the status says Disabled while this check remains false and says tools will appear once it runs. Base this message on the same effective active-session presentation used by renderStatus so the pane asks the user to enable the server.

// A workspace write is dropped when the session has no working directory, so fall back to the
// layer that can still be written rather than issuing one that silently vanishes.
if (decision.kind === CustomizationEnablementKind.Workspace && agentHostCustomizations.getWorkingDirectories(sessionResource).length === 0) {
return CustomizationEnablementKind.Global;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI Review: Fallback write leaves the server disabled

Replacing Global here preserves the existing Workspace false entry, which remains first and decisive. The switch sends Global: true but the server stays disabled and the control snaps back. Use a writable higher-precedence Session decision or otherwise replace the stale Workspace decision.

templateData.description.style.display = 'none';
return;
}
parts.push({ text: templateData.context.impliedOrigin, isContext: true });

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI Review: Reserve height for the fallback metadata line

This makes descriptionless rows two-line rows, but McpServerItemDelegate still returns the 36px single-line height unless the original description exists. Use the 44px height whenever origin or implied-origin metadata can render, or for all non-gallery server rows now that each has a metadata line.

@@ -1119,7 +1810,11 @@ export class McpListWidget extends Disposable {
const server = e.element.server;
const isGallery = e.element.marketplace || !server.local;
if (isGallery || server.description) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI Review: Open details for descriptionless installed servers

description no longer indicates whether details have content. A hand-authored local server commonly has no description, yet the pane renders its command/address and full tool names and descriptions, which the row does not expose. Fire selection for every installed server and align the has-detail affordance.

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.

3 participants