Skip to content

Commit 28da517

Browse files
committed
feat(mcp): S6a — Manage MCP servers, and take back a G1 approval
Adds `levelcode.ai.manageMcp` on the pickModel QuickPick pattern, linked from the foot of `/mcp`. Rows come from the same `mcpOverview()` S5 already uses, so the two views cannot disagree. Three things it adds beyond looking: **Add / remove a server** without hand-writing JSON — the last place MCP forced people into a settings file. Writes the Global tier only, never Workspace: `mcp.servers` is `application`-scoped precisely so a repo cannot introduce a server that starts without consent (G1), and offering the workspace tier would quietly undo that. **Revoke G1 trust**, per server or workspace-wide. This is the missing half of trust-on-first-use — approving was write-once with no way back short of editing `workspaceState` by hand, which makes the consent prompt harder to say yes to than it should be. Revoking does not kill a running server, and the wording says so. **Names a stale approval.** `summarizeMcp` reports "never approved" and "approved, then the repo changed the command" both as `trusted:false`. That is right for starting the server and wrong for explaining it: the second case is exactly the G1 attack — get something benign approved, then swap the command. The row now reads `command changed — needs approval` and offers to forget the dead entry. `parseArgv()` in mcpConfig.js turns the typed argument line into argv. Not a whitespace split: `-y @modelcontextprotocol/server-filesystem "/Users/me/My Documents"` is close to the most common MCP server there is, and splitting it turns one argument into three. It is a splitter, not a shell — no expansion, no globbing — matching `shell:false` at the spawn site. The Add wizard has no `env` step on purpose. The servers that need one need an API token, and prompting for it would put a live credential in plaintext settings.json while looking like the recommended way to do it. `connect()` inherits `process.env`, so exporting the variable keeps the secret out of any file; the success notification offers "Open settings JSON" for anyone who wants to do it differently. docs/MCP.md §9 documents the GitHub MCP recipe, verified end-to-end through mcpClient today: 41 tools, all names legal under D4, a real PR read back, write access confirmed. Also records that no GitHub tool sets `destructiveHint`, so `"*": "allow"` would let autopilot merge a pull request. Tests: 9 new in test/mcpManage.test.js (extracted from extension.js the way ctxSegments does with chat.html) + 4 in mcpConfig.test.js. Each verified non-vacuous by bypassing the fix and confirming failure.
1 parent 1ae25d6 commit 28da517

7 files changed

Lines changed: 509 additions & 5 deletions

File tree

docs/MCP.md

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,29 @@ servers, mirroring the project-rules chip (`agent.js:493`).
194194
the bar would stop summing to `used`. Hidden entirely when no server contributed one. Every tool
195195
schema rides every turn, so this is the standing cost a chatty server imposes, and it was invisible.
196196

197-
**S6 — later.** Streamable HTTP transport + the `2026-07-28` revision; resources/prompts; a
198-
"Manage MCP servers…" QuickPick on the `pickModel` pattern (`extension.js:1165-1228`).
197+
**S6a — "Manage MCP servers…". DONE.** `levelcode.ai.manageMcp`, on the `pickModel` QuickPick pattern,
198+
linked from the foot of `/mcp`. Rows come from the same `mcpOverview()` as S5, so the two views can
199+
never disagree. Three things it adds beyond looking:
200+
- **Add / remove a server** without hand-writing JSON — the last place MCP forced people into a settings
201+
file. It writes the **Global** tier only, never Workspace: `mcp.servers` is `application`-scoped
202+
precisely so a repo cannot introduce a server that starts without consent (G1), and a UI that offered
203+
the workspace tier would quietly undo that. The arguments box takes a **command line**, split by the
204+
quote-aware `parseArgv()` — a whitespace split would break
205+
`-y @modelcontextprotocol/server-filesystem "/Users/me/My Documents"`, which is close to the single
206+
most common MCP server there is. It is a splitter, **not a shell**: no expansion, no globbing, matching
207+
`shell:false` at the spawn site.
208+
- **Revoke G1 trust**, per server or workspace-wide — the missing half of trust-on-first-use. Approving
209+
was write-once with no way back short of editing `workspaceState` by hand, which makes the consent
210+
prompt harder to say yes to than it should be. Revoking does not kill a running server; the wording
211+
says "will ask before starting again" rather than implying otherwise.
212+
- **Names a stale approval.** `summarizeMcp` reports "never approved" and "approved, then the repo
213+
changed the command" both as `trusted:false`. Correct for *starting* the server, wrong for
214+
*explaining* it: the second case is exactly the G1 attack — get something benign approved, then swap
215+
the command. The row says `command changed — needs approval`, and the detail view offers to forget the
216+
dead approval. (`mcpTrustIsStale` / `mcpServerItem`, tested in `test/mcpManage.test.js`.)
217+
218+
**S6b — later.** Streamable HTTP transport + the `2026-07-28` revision; resources/prompts.
219+
Until HTTP lands, a hosted server is reachable only through a stdio bridge — see §9.
199220

200221
---
201222

@@ -227,3 +248,50 @@ servers, mirroring the project-rules chip (`agent.js:493`).
227248

228249
Remote/HTTP servers and OAuth · sampling (a server driving our model) · resources & prompts ·
229250
MCP "apps"/UI extensions · auto-discovery or an in-editor server marketplace.
251+
252+
---
253+
254+
## 9. Recipe: the GitHub MCP server
255+
256+
The most-asked-for server, and the one that shows where D2 (stdio only) actually bites. **Verified
257+
end-to-end on 2026-07-28** against `ghcr.io/github/github-mcp-server` through `mcpClient.connect`:
258+
handshake 122 ms, **41 tools**, all names legal under D4, a real PR read back, and write access
259+
confirmed. Settings, user tier:
260+
261+
```jsonc
262+
"levelcode.ai.mcp.servers": {
263+
"github": {
264+
"command": "docker",
265+
"args": ["run", "-i", "--rm",
266+
"-e", "GITHUB_PERSONAL_ACCESS_TOKEN",
267+
"-e", "GITHUB_TOOLSETS",
268+
"ghcr.io/github/github-mcp-server"],
269+
"env": { "GITHUB_TOOLSETS": "pull_requests,repos,issues,context" }
270+
}
271+
}
272+
```
273+
274+
Four things worth knowing, each of which is a design constraint rather than a detail:
275+
276+
- **GitHub's hosted server (`api.githubcopilot.com/mcp/`) is Streamable HTTP, which we do not speak
277+
(D2).** The container above is the same server over stdio. A bridge like `mcp-remote` also works and
278+
is the only way to get the OAuth flow, at the cost of a second process in the chain.
279+
- **`-e NAME` with no `=value` is deliberate.** `connect()` spawns with `Object.assign({}, process.env,
280+
server.env)` (`mcpClient.js:52`), so Docker inherits the token from the editor's environment and the
281+
credential never lands in `settings.json`. Putting the value in the `env` block works too — it is also
282+
a plaintext secret in a synced settings file. This is why the S6a Add wizard has no `env` step.
283+
- **`GITHUB_TOOLSETS` is a context-budget decision, not a preference.** The full server advertises far
284+
more tools, every schema rides every turn, and the S5 context meter will show exactly what that costs.
285+
- **Everything is `ask` by default, including `merge_pull_request`** — no GitHub tool sets
286+
`destructiveHint`, so nothing is force-asked by G2's hard rule, which means the allow-list *can* grant
287+
any of them. `"levelcode.ai.mcp.toolPolicy": { "github__pull_request_read": "allow" }` is the sane
288+
shape: allow-list the reads, keep the writes on the card. `"*": "allow"` would let autopilot merge.
289+
290+
| Ask | Tool |
291+
| --- | --- |
292+
| read a PR — diff, files, reviews, comments, checks | `github__pull_request_read` (`method` enum, 9 values) |
293+
| create a PR | `github__create_pull_request` |
294+
| update a PR — title, body, base, reviewers, draft | `github__update_pull_request` |
295+
| **close** a PR | `github__update_pull_request` with `state: "closed"` |
296+
| review a PR | `github__pull_request_review_write`, `github__add_comment_to_pending_review` |
297+
| merge a PR | `github__merge_pull_request` |

extensions/levelcode-ai/extension.js

Lines changed: 223 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const { loadSkills, skillsMenu, getSkillBody } = require('./skills');
2727
const { openCustomize } = require('./customize');
2828
const { importFromVscode } = require('./importVscode');
2929
const { reapMcp, listActive, getServer } = require('./mcpClient');
30-
const { userScopedSetting, isNamespacedToolName, safeCopy, loadServerConfig, summarizeMcp } = require('./mcpConfig');
30+
const { userScopedSetting, isNamespacedToolName, safeCopy, loadServerConfig, summarizeMcp, parseArgv } = require('./mcpConfig');
3131

3232
const SECRET_KEY = 'levelcode.ai.anthropicKey'; // legacy Anthropic key location (kept for back-compat)
3333
const FILE_EXCLUDES = '{**/node_modules/**,**/.git/**,**/out/**,**/dist/**,**/.vscode-test/**,**/*.map}';
@@ -1350,6 +1350,226 @@ async function pickCloudModel() {
13501350
}
13511351
}
13521352

1353+
// ---- "Manage MCP servers…" (docs/MCP.md S6) --------------------------------
1354+
// The last place MCP forced people into raw JSON, and the only UI for revoking a repo server's G1
1355+
// launch trust — until now that decision was write-once, with no way back short of clearing
1356+
// workspaceState by hand.
1357+
1358+
/**
1359+
* Is this server's G1 approval STALE — approved once, but the command line has changed since?
1360+
*
1361+
* summarizeMcp collapses "never approved" and "approved, then the repo edited the command" into one
1362+
* `trusted:false`, which is right for starting the server and wrong for explaining it. The second case
1363+
* is the exact attack G1 exists to stop: get a benign entry approved, then swap the command. It
1364+
* deserves to be named rather than shown as a server that was simply never set up.
1365+
*/
1366+
function mcpTrustIsStale(s, store) {
1367+
return s.trusted === false && Object.prototype.hasOwnProperty.call(store || {}, s.name);
1368+
}
1369+
1370+
/** One row per configured server, on the same shape summarizeMcp already produces for `/mcp`. */
1371+
function mcpServerItem(s, store) {
1372+
let state, icon;
1373+
if (s.running) { state = 'running'; icon = '$(pass-filled)'; }
1374+
else if (mcpTrustIsStale(s, store)) { state = 'command changed — needs approval'; icon = '$(warning)'; }
1375+
// A repo server waiting on the consent card is not broken — saying only "not started" would send
1376+
// the user debugging a gate that is working.
1377+
else if (s.trusted === false) { state = 'needs approval'; icon = '$(shield)'; }
1378+
else { state = 'not started'; icon = '$(circle-outline)'; }
1379+
1380+
const bits = [ s.source === 'settings' ? 'your settings' : (s.origin || 'workspace'), state ];
1381+
if (s.running) {
1382+
bits.push(s.tools + ' tool' + (s.tools === 1 ? '' : 's'));
1383+
if (s.allowed != null) { bits.push(s.allowed + ' allow-listed'); }
1384+
}
1385+
return { label: icon + ' ' + s.name, description: bits.join(' · '), detail: s.commandLine, _server: s };
1386+
}
1387+
1388+
async function manageMcpServers() {
1389+
const view = mcpOverview();
1390+
const store = mcpLaunchTrust();
1391+
const items = [];
1392+
1393+
if (view.servers.length) {
1394+
items.push({ label: 'Configured servers', kind: vscode.QuickPickItemKind.Separator });
1395+
for (const s of view.servers) { items.push(mcpServerItem(s, store)); }
1396+
}
1397+
items.push({ label: view.servers.length ? 'Actions' : 'No servers configured', kind: vscode.QuickPickItemKind.Separator });
1398+
items.push({ label: '$(add) Add a server…', description: 'Write a new entry to your settings', _action: 'add' });
1399+
items.push({ label: '$(edit) Open settings JSON', description: 'levelcode.ai.mcp.servers', _action: 'settings' });
1400+
if (Object.keys(store).length) {
1401+
items.push({ label: '$(discard) Revoke all workspace trust…', description: 'Repo-defined servers will ask again before starting', _action: 'revokeAll' });
1402+
}
1403+
// Config problems belong here rather than only in `/mcp`: a server the user typed and cannot find is
1404+
// most often a rejected entry, and this is where they came to look for it.
1405+
if (view.problems.length) {
1406+
items.push({ label: 'Problems', kind: vscode.QuickPickItemKind.Separator });
1407+
for (const p of view.problems) {
1408+
items.push({ label: '$(warning) ' + p.message, description: p.level, _action: 'settings' });
1409+
}
1410+
}
1411+
1412+
const pick = await vscode.window.showQuickPick(items, {
1413+
placeHolder: view.configured
1414+
? view.configured + ' configured · ' + view.running + ' running'
1415+
: 'No MCP servers configured yet',
1416+
matchOnDescription: true,
1417+
matchOnDetail: true
1418+
});
1419+
if (!pick) { return; }
1420+
1421+
if (pick._action === 'add') { return mcpAddServer(); }
1422+
if (pick._action === 'settings') { return openMcpSettings(); }
1423+
if (pick._action === 'revokeAll') { return mcpRevokeTrust(null, Object.keys(store).length); }
1424+
if (pick._server) { return mcpServerActions(pick._server, store); }
1425+
}
1426+
1427+
/** Second level: what you can do to one server. */
1428+
async function mcpServerActions(s, store) {
1429+
const items = [ { label: '$(clippy) Copy command line', description: s.commandLine, _action: 'copy' } ];
1430+
1431+
if (s.source === 'settings') {
1432+
items.push({ label: '$(edit) Edit in settings JSON', _action: 'settings' });
1433+
items.push({ label: '$(trash) Remove from settings', _action: 'remove' });
1434+
} else if (mcpTrustIsStale(s, store)) {
1435+
items.push({
1436+
label: '$(warning) This server\'s command line changed since you approved it',
1437+
description: 'Approve the new one on the consent card, or forget the old approval',
1438+
_action: 'noop'
1439+
});
1440+
items.push({ label: '$(discard) Forget the old approval', _action: 'revoke' });
1441+
} else if (s.trusted) {
1442+
items.push({ label: '$(discard) Revoke trust for this workspace', description: 'It will ask again before starting', _action: 'revoke' });
1443+
} else {
1444+
items.push({ label: '$(shield) Not yet approved', description: 'The consent card appears when the agent first needs it', _action: 'noop' });
1445+
}
1446+
1447+
const pick = await vscode.window.showQuickPick(items, { placeHolder: s.name + ' · ' + (s.source === 'settings' ? 'your settings' : s.origin) });
1448+
if (!pick || pick._action === 'noop') { return; }
1449+
if (pick._action === 'copy') {
1450+
try { await vscode.env.clipboard.writeText(s.commandLine || ''); } catch (e) { dbg('mcp.manage.copyFailed', { error: String((e && e.message) || e) }); }
1451+
return;
1452+
}
1453+
if (pick._action === 'settings') { return openMcpSettings(); }
1454+
if (pick._action === 'revoke') { return mcpRevokeTrust(s.name); }
1455+
if (pick._action === 'remove') { return mcpRemoveServer(s.name); }
1456+
}
1457+
1458+
function openMcpSettings() {
1459+
return vscode.commands.executeCommand('workbench.action.openSettingsJson', { revealSetting: { key: 'levelcode.ai.mcp.servers' } });
1460+
}
1461+
1462+
/**
1463+
* Add a server to the USER settings tier.
1464+
*
1465+
* Global on purpose, and not offered for the workspace tier: `levelcode.ai.mcp.servers` is declared
1466+
* `application` scope precisely so a repo cannot introduce a server that starts without consent
1467+
* (docs/MCP.md G1). A UI that wrote it anywhere else would quietly undo that.
1468+
*/
1469+
async function mcpAddServer() {
1470+
const cfg = aiConfig();
1471+
const existing = safeCopy(userScopedSetting(cfg.inspect('mcp.servers'), {}) || {});
1472+
1473+
const name = (await vscode.window.showInputBox({
1474+
title: 'Add an MCP server — name',
1475+
prompt: 'Short id, used to namespace its tools as name__tool',
1476+
placeHolder: 'github',
1477+
ignoreFocusOut: true,
1478+
validateInput: (v) => {
1479+
const t = String(v || '').trim();
1480+
if (!t) { return 'A name is required.'; }
1481+
if (Object.prototype.hasOwnProperty.call(existing, t)) { return 'A server called "' + t + '" already exists.'; }
1482+
// Not a hard rule — namespaceToolName sanitizes anyway — but a name that survives verbatim
1483+
// makes the tool names in the transcript readable.
1484+
if (!/^[A-Za-z0-9_-]+$/.test(t)) { return 'Use letters, digits, dashes or underscores so the tool names stay readable.'; }
1485+
return null;
1486+
}
1487+
}) || '').trim();
1488+
if (!name) { return; }
1489+
1490+
const command = (await vscode.window.showInputBox({
1491+
title: 'Add an MCP server — command',
1492+
prompt: 'The executable to run (it is spawned directly, not through a shell)',
1493+
placeHolder: 'npx',
1494+
ignoreFocusOut: true,
1495+
validateInput: (v) => (String(v || '').trim() ? null : 'A command is required.')
1496+
}) || '').trim();
1497+
if (!command) { return; }
1498+
1499+
const argsLine = await vscode.window.showInputBox({
1500+
title: 'Add an MCP server — arguments',
1501+
prompt: 'Space-separated. Quote anything containing spaces, e.g. "/Users/me/My Documents"',
1502+
placeHolder: '-y @modelcontextprotocol/server-filesystem /path/to/dir',
1503+
ignoreFocusOut: true
1504+
});
1505+
if (argsLine === undefined) { return; } // escaped — an empty string is a valid "no arguments"
1506+
1507+
const entry = { command: command };
1508+
const args = parseArgv(argsLine);
1509+
if (args.length) { entry.args = args; }
1510+
1511+
existing[name] = entry;
1512+
try {
1513+
await cfg.update('mcp.servers', existing, vscode.ConfigurationTarget.Global);
1514+
dbg('mcp.manage.added', { name: name });
1515+
} catch (e) {
1516+
vscode.window.showErrorMessage('Could not save the MCP server: ' + String((e && e.message) || e));
1517+
return;
1518+
}
1519+
1520+
// The wizard deliberately does not ask for `env`: the servers that need one need an API TOKEN, and a
1521+
// prompt for it would end with a live credential in plaintext settings.json — worse, it would look
1522+
// like the recommended way to do it. mcpClient spawns with process.env inherited, so exporting the
1523+
// variable and launching the editor from that shell keeps the secret out of any file. Anyone who
1524+
// wants it in settings anyway can put it there; this just points at the file instead of pretending
1525+
// the wizard covered everything.
1526+
const next = await vscode.window.showInformationMessage(
1527+
'Added MCP server "' + name + '". It starts on the next agent run.',
1528+
'Open settings JSON');
1529+
if (next === 'Open settings JSON') { return openMcpSettings(); }
1530+
}
1531+
1532+
async function mcpRemoveServer(name) {
1533+
const ok = await vscode.window.showWarningMessage(
1534+
'Remove the MCP server "' + name + '" from your settings?', { modal: true }, 'Remove');
1535+
if (ok !== 'Remove') { return; }
1536+
1537+
const cfg = aiConfig();
1538+
const existing = safeCopy(userScopedSetting(cfg.inspect('mcp.servers'), {}) || {});
1539+
delete existing[name];
1540+
try {
1541+
await cfg.update('mcp.servers', existing, vscode.ConfigurationTarget.Global);
1542+
dbg('mcp.manage.removed', { name: name });
1543+
} catch (e) {
1544+
vscode.window.showErrorMessage('Could not remove the MCP server: ' + String((e && e.message) || e));
1545+
}
1546+
}
1547+
1548+
/**
1549+
* Forget a G1 launch approval — for one server, or all of them in this workspace.
1550+
*
1551+
* The missing half of trust-on-first-use: approving was permanent with no way back, which makes the
1552+
* prompt harder to say yes to than it should be. Revoking does not stop a server already running in
1553+
* this session; it means the next run asks again, so the message says exactly that rather than
1554+
* implying the process was killed.
1555+
*/
1556+
async function mcpRevokeTrust(name, count) {
1557+
const target = name ? ('trust for "' + name + '"') : ('trust for all ' + count + ' repo-defined server(s)');
1558+
const ok = await vscode.window.showWarningMessage(
1559+
'Revoke ' + target + ' in this workspace? They will ask for approval again before starting.',
1560+
{ modal: true }, 'Revoke');
1561+
if (ok !== 'Revoke') { return; }
1562+
1563+
// Re-read rather than reuse the snapshot the menu was built from: a modal was just open, and
1564+
// approving a different server in the meantime must not be silently rolled back by this write.
1565+
let next;
1566+
if (name) { next = mcpLaunchTrust(); delete next[name]; } else { next = {}; }
1567+
await saveMcpLaunchTrust(next);
1568+
dbg('mcp.manage.trustRevoked', { server: name || '*' });
1569+
vscode.window.showInformationMessage(
1570+
name ? '"' + name + '" will ask before starting again.' : 'Repo-defined servers will ask before starting again.');
1571+
}
1572+
13531573
async function pickModel() {
13541574
const cfg = aiConfig();
13551575
// Gateway mode + signed in → a plan-scoped LevelCode Cloud menu (free engine + flagship, the latter
@@ -1521,6 +1741,7 @@ class ChatViewProvider {
15211741
}
15221742
case 'setKey': await promptForKey(); break;
15231743
case 'pickModel': await pickModel(); break;
1744+
case 'manageMcp': await manageMcpServers(); break;
15241745
case 'openSettings': vscode.commands.executeCommand('workbench.action.openSettings', '@ext:levelcode.levelcode-ai'); break;
15251746
}
15261747
});
@@ -1795,6 +2016,7 @@ function activate(context) {
17952016
vscode.commands.registerCommand('levelcode.import.vscode', () => importFromVscode(context)),
17962017
vscode.commands.registerCommand('levelcode.ai.newChat', newChat),
17972018
vscode.commands.registerCommand('levelcode.ai.pickModel', pickModel),
2019+
vscode.commands.registerCommand('levelcode.ai.manageMcp', manageMcpServers),
17982020
vscode.commands.registerCommand('levelcode.ai.addSelection', addSelection),
17992021
vscode.commands.registerCommand('levelcode.ai.addFileContext', addContext),
18002022
vscode.commands.registerCommand('levelcode.ai.setApiKey', () => promptForKey()),

0 commit comments

Comments
 (0)