MCP server management: working Edit button, correct scope handling, Windows spawn quoting, CLI-scope servers

[leonidasbarkas98-cpu]

Stacked on top of #224 — please review/merge that one first; only the last 12 commits (from 2fe4259 onwards) are new here. Everything below 2fe4259 in the diff belongs to that PR, whose escaping helpers this branch builds on.

Fixes for the MCP server panel, where several paths were broken outright rather than merely rough:

• MCP configuration hardening. Three independent items. Windows spawn quoting (upstream "Username with spaces causes MCP error" #125): without a custom executable the Claude process is spawned with shell: true, where Node joins the argv array into one command line without quoting, and cmd.exe then splits any argument containing a space — an --mcp-config path under a profile like C:\Users\Vincent K. Bae\... becomes two arguments and the CLI reports "MCP config file not found". A quoteWinShellArgs helper (its own vscode-free module, so it is unit-testable under plain mocha) quotes space-containing args only when a shell is actually used; for paths without spaces the command line is byte-identical. CLI local-scope servers (upstream MCP Servers configured in Claude Code, don't seem to be available when running Chat #100): servers configured through the CLI (~/.claude.json → projects[cwd].mcpServers) were used but never listed, so the panel claimed "none configured". They are merged in read-only — matched case-insensitively with normalized slashes, since the CLI stores project keys with forward slashes and inconsistent drive casing while uri.fsPath uses backslashes — rendered with a "via CLI" badge and no edit/delete, because that scope has no config path of its own and must never be written by the extension. Plus the display escaping for server name/command/args/url, now that this surfaces external ~/.claude.json data. (upstream The VS Code extension Claude Code Chat throws a runtime error on startup due to an invalid, hard-coded MCP configuration path. #106 and upstream Error with MCP server: Storage path not available #140 were checked and are already fixed in 2.2.0; nothing here touches them.)
• The Edit button never worked. editMCPServer dereferenced getElementById('addServerBtn') as its first statement, and no element in the emitted HTML carries that id — every click threw a TypeError and the form never appeared. With that guarded, the rest of the function becomes reachable, which exposes the next problem: it never set the scope select either, so editing a server while the select still showed a different scope wrote the update into the other config file and left the original behind as a duplicate. The select is now set from the server's own scope and disabled while editing (changing it would be a move between two config files, which is implemented nowhere and only ever produced duplicates); installMarketplaceServer clears the lock again, as it already did for the name field. Two robustness fixes in the same loop: the per-render config map is Object.create(null) so a server named toString or constructor cannot resolve through the prototype chain past the "not found" guard, and the server type goes through String() — a non-string type in a .mcp.json threw mid-loop and swallowed every remaining server plus the buttons after the list.
• The scope was still wrong for extension-scope servers. The scope select had no <option> for extension, so the field the edit path locks to the server's own scope read back as '', and the host dispatch turned that empty value into project — writing a second copy into the workspace's .mcp.json while the original stayed in the extension config. Saving now takes the scope from the edited server's own config rather than from the display-only select; a disabled extension option makes the locked field readable; and the add/hide form paths share one resetAddServerFormFields() helper, so "Add manually" after an abandoned edit no longer inherits the previous server's name, scope or fields.
• Editing one server no longer leaks into the next. editMCPServer() only ever populated the args/env fields conditionally and never cleared them first, so editing server A (stdio, with args and env) and then server B (with none of its own) without Cancel or Save in between left A's values in the form — "Update Server" for B would have written A's args and env into B's config. It now calls the same full reset before populating, and hides the server list behind the edit form, matching the add path. Only reachable at all once the null-guard above landed, which is why it had gone unnoticed.
• An unknown MCP scope resolves to no path instead of the extension's own config. The scope-to-path helper ended in a catch-all returning the extension's own config path for anything that was not local, global or project, so an unknown or empty scope wrote silently into a file the caller never asked for. It now returns undefined, which the existing guards in the save/delete paths already turn into a proper error. The decision moved into a pure, vscode-free src/mcp-config-path.ts taking home dir, workspace folder and extension storage path as arguments: as a private method of the vscode-dependent provider class it was untestable and could have been reverted without a single test going red. 13 unit tests behind npm run test:mcp-config-path.
• Marketplace detail header overlapped its own title. The header is a flex row without flex-wrap, and the name box used flex: 1 — that is, flex-basis: 0%. Flexbox decides where to wrap from each item's hypothetical main size, so the name counted as zero width no matter how long it was: the row never wrapped, the name got whatever the icon and the ~208px select+install block left over, and since the name rule sets neither white-space nor overflow, the text spilled out under the action buttons, which paint later. flex-wrap on the row plus flex: 1 1 auto on the name box lets the name enter that calculation at its real content width, so a narrow panel wraps the actions onto their own line; wide panels are unchanged. Pre-existing since the upstream import.
• Duplicate slash-command notice removed. executeSlashCommand called addMessage('user', 'Executing /...', 'assistant') — arguments in the wrong order for addMessage(content, type, timestamp, rawText). 'user' became the content, the whole sentence became the type and landed as class tokens on the bubble, and since that type matched none of user/claude/error the bubble got no header at all: no icon, no copy button, no collapse, no timestamp, and it displayed the literal word "user". Repairing the call would have produced two identical notices, because the host already posts the same text back as terminalOpened; the broken one just never looked like a duplicate. So the client-side call is gone instead, after confirming the host covers every case it covered (the host path returns early only for /compact, which the client branch skipped too, and nothing between that and the post can bail out). The notice now arrives one round trip later — the only behavioural difference. A sweep of the ~31 addMessage call sites found no other with swapped or unsupported arguments.
• Test harness that hid the null dereference. The fake DOM handed out an element for any id asked of it, which is exactly why the addServerBtn crash above stayed green for so long. It now returns null for ids that do not exist in the real emitted HTML, parsed from that output rather than hand-maintained; removing the guard turns six tests red on that single cause. A sweep of every id used by the edit/show/hide form paths against the 189 real ones found no further mismatch.

The remaining commits are small follow-ups: the "via CLI" badge had no CSS rule and no hover rule for its tooltip attribute, so it rendered as unstyled text; and comments carrying stale cross-references, a remaining non-English word and mis-attributed internal issue numbers were corrected. Internal tracker references are written as fork-issue-NN so they do not auto-link to unrelated issues on this repository; genuine upstream references are kept as such.

This is one of several PRs from our fork submitted together as part of a broader, thematically grouped series (permission/session handling, settings/UI, security hardening, rendering, etc. split across separate branches/PRs). This is the MCP branch of that series and it stacks on the security-hardening branch, since the escaping the MCP panel relies on lives there.

tsc --noEmit clean, no new dependencies; full unit run at this tip is 179 passing, 0 failing.

🤖 Generated with Claude Code