feat(mcp): per-call approval card — S4a (lifts S3's allow-list-only wall)

S3 REFUSED any MCP tool the user hadn't hand-added to levelcode.ai.mcp.toolPolicy.
That was the safe-but-unusable placeholder; S4 replaces it with an actual prompt.

Now a tool that isn't allow-listed shows a card — server · tool · the real
arguments — with Skip / Allow once / Always allow. "Always allow" writes the tool
to the allow-list (Global tier, matching the setting's application scope), so
future runs skip the prompt: the allow-list is still the ONE thing that grants
'allow' (G3). Autopilot does not relax any of this — an MCP tool is third-party
code.

Security properties, all verified end-to-end against the S2 fixture server (five
paths: allow-listed runs silently; un-listed prompts→runs on Allow; un-listed
prompts→NOT run on Skip; destructive prompts even when allow-listed and offers no
"always"; no-webview refuses):

- A destructive tool ALWAYS prompts (classifyMcpTool tightens on it) and the card
  hides "Always allow" — offering it would be a button that does nothing, since a
  destructive tool can never be allow-listed. canAllowAlways is derived from the
  same annotation the classifier reads, so they can't disagree.
- The arguments are shown IN FULL on the card (only length-capped). That is the
  decision — the user owns the credentials and needs to see the repo it will
  touch, the row it will delete. The card is ephemeral UI, never the transcript;
  the debug-log redaction (G4) is unchanged.
- No webview to ask through (headless / tests) → falls back to S3's refusal
  rather than running third-party code with no way to say no.

Pieces:
- mcpConfig.describeMcpCall (pure): server/tool from the route with a
  namespaced-name fallback, bounded+pretty args (never throws on circular input),
  destructive/canAllowAlways. 3 new tests, mutation-checked (making a destructive
  tool "always-allowable" fails).
- agent.js router: refuse → prompt via ctx.approve({kind:'mcp',…}).
- extension.js: mcpAllowAlways writes the allow-list (user-scoped read, Global
  write, rejects a non-namespaced name).
- chat.html: the kind:'mcp' card. VERIFIED VISUALLY — screenshotted both the
  normal and destructive variants: args wrap, buttons are distinct (two sharing
  `.approve` was a selector bug, caught and fixed), destructive shows the amber
  warning and no "Always allow".

Also: the MCP chips used icon:'plug', which isn't a registered codicon, so
addAgentLine rendered the literal word "plug" in the rail. Swapped to 'sparkle'
(the 🔌 emoji still marks it MCP). The identical latent issue on the auto-preview
'globe' chip (extension.js:701) is #35's, left for a follow-up. And
explainMcpRefusal's wording ("this build has no prompt") was made accurate — it's
now only the non-interactive fallback.

Deferred to S4b: the G1 trust-on-first-use LAUNCH gate for workspace-file
(.levelcode/mcp.json) servers — they're still read-and-listed but never started.
This PR is the per-call gate; the launch gate is its own reviewable slice.

Verified: 24 suites, 0 failures (mcpConfig 44 cases).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: ndemianc

extensions/levelcode-ai/agent.js

@@ -17,7 +17,7 @@ const providers = require('./providers/index');
 const { formatVerifyFeedback, verifyOutcome, looksUnrunnable, sniffPort, sniffPreviewUrl, looksReady } = require('./verify');
 const { classifyCommand, dangerLabel } = require('./commandSafety');
 const { loadProjectRules } = require('./projectRules');
-const { loadServerConfig, buildAgentTools, toolCountsByServer, classifyMcpTool, explainMcpRefusal } = require('./mcpConfig');
+const { loadServerConfig, buildAgentTools, toolCountsByServer, classifyMcpTool, explainMcpRefusal, describeMcpCall } = require('./mcpConfig');
 const { connectAll, getServer } = require('./mcpClient');
 
 const SYSTEM_BASE = [
@@ -484,19 +484,30 @@ async function runTool(tu, ctx) {
 		const route = ctx.mcpRoutes && ctx.mcpRoutes.get(tu.name);
 		if (route) {
 			const verdict = classifyMcpTool(tu.name, ctx.mcp && ctx.mcp.toolPolicy, route.annotations);
-			if (verdict.approve !== 'allow') {
-				// S3 deliberately ships no approval CARD (S4 owns it), so anything the user has not
-				// explicitly allow-listed is REFUSED rather than run — the alternative would be silently
-				// executing third-party code on the user's behalf with no way to say no. The explanation
-				// lives in mcpConfig beside the classifier so it can't drift from it (PR #31 review): a
-				// destructive tool is refused for a reason the allow-list cannot fix, and must not be
-				// described as allow-listable.
-				ctx.post({ type: 'agentTool', icon: 'shield', text: '🔌 mcp · refused ' + tu.name + ' — ' + verdict.reason });
-				return explainMcpRefusal(tu.name, verdict);
-			}
 			const server = getServer(route.server);
 			if (!server || !server.alive) { return 'ERROR: the MCP server "' + route.server + '" is not running.'; }
-			ctx.post({ type: 'agentTool', icon: 'plug', text: '🔌 ' + route.server + ' · ' + route.tool });
+			// S4: a call the user hasn't allow-listed is now PROMPTED, not refused. Autopilot does not relax
+			// this (G3) — an MCP tool is third-party code — and a server-marked-destructive tool prompts even
+			// when allow-listed (classifyMcpTool tightens on it). Only 'allow' skips the card.
+			if (verdict.approve !== 'allow') {
+				if (typeof ctx.approve !== 'function') {
+					// No webview to ask through (headless / a test harness) — fall back to S3's safe refusal
+					// rather than run third-party code with no way to say no.
+					ctx.post({ type: 'agentTool', icon: 'shield', text: '🔌 mcp · refused ' + tu.name + ' — ' + verdict.reason });
+					return explainMcpRefusal(tu.name, verdict);
+				}
+				const call = describeMcpCall(tu.name, input, route);
+				const approved = await ctx.approve({
+					kind: 'mcp', name: tu.name, server: call.server, tool: call.tool,
+					args: call.argsText, destructive: call.destructive, canAllowAlways: call.canAllowAlways
+				});
+				if (!approved) {
+					ctx.post({ type: 'agentTool', icon: 'shield', text: '🔌 mcp · skipped ' + tu.name });
+					return 'User declined to run the MCP tool "' + tu.name + '". Do NOT retry it in this run — '
+						+ 'continue without it, or tell the user what you needed it for.';
+				}
+			}
+			ctx.post({ type: 'agentTool', icon: 'sparkle', text: '🔌 ' + route.server + ' · ' + route.tool });
 			return await server.call(route.tool, input);   // never throws — failures come back as `ERROR: …`
 		}
 		return 'ERROR: unknown tool ' + tu.name;
@@ -586,7 +597,7 @@ async function setupMcp(ctx, wsFolders, dbg) {
 		const perServer = toolCountsByServer(built.routes);
 		const summary = handles.map((h) => h.name + ' (' + (perServer.get(h.name) || 0) + ')').join(', ');
 		dbg('mcp.ready', { servers: handles.map((h) => h.name), tools: built.tools.length, allowed });
-		ctx.post({ type: 'agentTool', icon: 'plug', text: '🔌 mcp · ' + summary + ' · ' + allowed + '/' + built.tools.length + ' allow-listed' });
+		ctx.post({ type: 'agentTool', icon: 'sparkle', text: '🔌 mcp · ' + summary + ' · ' + allowed + '/' + built.tools.length + ' allow-listed' });
 		return built;
 	} catch (e) {
 		dbg('mcp.failed', { error: (e && e.message) || String(e) });

extensions/levelcode-ai/extension.js

@@ -769,6 +769,28 @@ async function restoreCheckpoint(turnId) {
 const pendingApprovals = new Map();
 let approvalSeq = 0;
 
+/**
+ * Persist an MCP tool to the allow-list (the ONLY thing that grants 'allow' — G3). Writes to the USER
+ * (Global) tier, matching the application scope the setting is declared with, so a repo can never flip
+ * it. Reads the current value the same user-scoped way it is read at run start. Idempotent, and refuses
+ * a tool name that is not a namespaced server__tool to avoid writing junk from a malformed message.
+ */
+async function mcpAllowAlways(name) {
+	if (typeof name !== 'string' || !/^[A-Za-z0-9_-]+__[A-Za-z0-9_-]+$/.test(name)) {
+		dbg('mcp.allow.reject', { name }); return;
+	}
+	try {
+		const cfg = aiConfig();
+		const cur = userScopedSetting(cfg.inspect('mcp.toolPolicy'), {}) || {};
+		if (cur[name] === 'allow') { return; }
+		await cfg.update('mcp.toolPolicy', Object.assign({}, cur, { [name]: 'allow' }), vscode.ConfigurationTarget.Global);
+		post({ type: 'agentTool', icon: 'check', text: '🔌 mcp · always allow ' + name });
+		dbg('mcp.allow.persisted', { name });
+	} catch (e) {
+		dbg('mcp.allow.failed', { name, error: String((e && e.message) || e) });
+	}
+}
+
 /** Ask the webview to approve an action; resolves true/false. */
 function requestApproval(req) {
 	const id = String(++approvalSeq);
@@ -1358,7 +1380,13 @@ class ChatViewProvider {
 				case 'send': await handleSend(msg.text); break;
 				case 'stop': dbg('stop.clicked', { running: commandStops.size }); for (const [, stop] of commandStops) { try { stop(); } catch (e) { /* gone */ } } if (abort) { abort.abort(); } clearApprovals(); clearQuestions(); break;
 				case 'stopCommand': { dbg('stopCommand', { id: msg.id }); const s = commandStops.get(msg.id); if (s) { try { s(); } catch (e) { /* gone */ } } break; }
-				case 'approvalResponse': resolveApproval(msg.id, msg.approved); break;
+				case 'approvalResponse':
+					// "Always allow" on an MCP card persists the tool to the allow-list BEFORE resolving, so a
+					// future run skips the prompt. It only ever adds an ALLOW (never a broadening default), and
+					// the webview offers it only for non-destructive tools — mcpAllowAlways re-checks anyway.
+					if (msg.approved && msg.remember && msg.mcpName) { await mcpAllowAlways(msg.mcpName); }
+					resolveApproval(msg.id, msg.approved);
+					break;
 				case 'questionsResponse': resolveQuestions(msg.id, msg.answers, msg.notes); break;
 				case 'accountSignIn': await accountSignIn(msg.provider, msg.create); break;
 				case 'accountSignOut': await accountSignOut(); break;

extensions/levelcode-ai/mcpConfig.js

@@ -400,17 +400,59 @@ function classifyMcpTool(name, policy, annotations) {
  * @returns {string}
  */
 function explainMcpRefusal(name, verdict) {
+	// Reached only when there is NO interactive approval to fall back on (a headless run, a test harness).
+	// With a webview present, S4 shows the per-call card instead of this message.
 	const head = 'ERROR: the MCP tool "' + name + '" is not approved to run (' + verdict.reason + '). ';
 	const fix = verdict.policyCanAllow
-		? 'This build has no per-call approval prompt, so the only way to permit it is for the USER to add '
+		? 'No interactive approval is available here, so the only way to permit it is for the USER to add '
 			+ '"' + name + '": "allow" to the "levelcode.ai.mcp.toolPolicy" setting. '
-		: 'Such tools always require per-call approval — which this build does not yet provide — so it '
-			+ 'CANNOT be enabled through the allow-list. ';
+		: 'Such tools always require per-call approval and CANNOT be enabled through the allow-list, so '
+			+ 'there is no way to run it in this non-interactive context. ';
 	return head + fix + 'Do NOT retry it in this run — continue without it, or tell the user what you needed it for.';
 }
 
+// A tool call's arguments can be large, and the approval card must not be blown open by one. See
+// describeMcpCall — the card is capped, the full args still reach the server if approved.
+const MAX_ARG_CHARS = 2000;
+
+/** Pretty, bounded JSON for the args shown on the approval card. Never throws (circular/huge input). */
+function previewArgs(args) {
+	if (args == null) { return ''; }
+	let text;
+	try { text = JSON.stringify(args, null, 2); }
+	catch { try { text = String(args); } catch { text = '[unserializable arguments]'; } }
+	if (text == null) { return ''; }
+	return text.length > MAX_ARG_CHARS ? text.slice(0, MAX_ARG_CHARS - 1) + '…' : text;
+}
+
+/**
+ * Shape an MCP call for the approval card (S4) — exactly what the user reads before deciding.
+ *
+ * Unlike the debug log (G4), the arguments are shown in FULL here, only length-capped. That is not an
+ * oversight: the card is ephemeral UI shown to the person who owns the credentials, and seeing the real
+ * arguments — the repo it will touch, the id it will delete — IS the decision. Redacting them would make
+ * the prompt meaningless. Nothing here is persisted; the card is not the transcript.
+ *
+ * `canAllowAlways` is false for a destructive tool: a server-marked-destructive tool can never be moved
+ * to the allow-list (classifyMcpTool tightens on it), so the card must not offer a button that would do
+ * nothing. Derived from the same annotation the classifier reads, so the two cannot disagree.
+ *
+ * @param {string} name  namespaced tool name (server__tool)
+ * @param {*} args  the arguments the model produced for this call
+ * @param {{server?:string, tool?:string, annotations?:object}} [route]
+ * @returns {{server:string, tool:string, argsText:string, destructive:boolean, canAllowAlways:boolean}}
+ */
+function describeMcpCall(name, args, route) {
+	const r = route || {};
+	const fallback = String(name == null ? '' : name).split(NAME_SEPARATOR);
+	const server = typeof r.server === 'string' && r.server ? r.server : (fallback[0] || String(name));
+	const tool = typeof r.tool === 'string' && r.tool ? r.tool : (fallback.slice(1).join(NAME_SEPARATOR) || String(name));
+	const destructive = !!(r.annotations && r.annotations.destructiveHint === true);
+	return { server, tool, argsText: previewArgs(args), destructive, canAllowAlways: !destructive };
+}
+
 module.exports = {
 	loadServerConfig, userScopedSetting, namespaceToolName, assignToolNames, buildAgentTools,
-	toolCountsByServer, classifyMcpTool, explainMcpRefusal,
-	BUILTIN_TOOL_NAMES, MAX_TOOL_NAME, MAX_TOOL_DESC, MAX_SERVERS, MAX_TOOLS_PER_SERVER, WORKSPACE_CONFIG_PATH
+	toolCountsByServer, classifyMcpTool, explainMcpRefusal, describeMcpCall,
+	BUILTIN_TOOL_NAMES, MAX_TOOL_NAME, MAX_TOOL_DESC, MAX_ARG_CHARS, MAX_SERVERS, MAX_TOOLS_PER_SERVER, WORKSPACE_CONFIG_PATH
 };

extensions/levelcode-ai/media/chat.html

@@ -380,6 +380,9 @@
   .tl-ask .asksub { margin-top: 3px; font-size: 11.5px; line-height: 1.5; color: var(--muted); white-space: pre-wrap; word-break: break-word; }
   /* recessed well for the command; capped so a long one can't push the buttons off-screen */
   .tl-ask .askcode { margin-top: 10px; border-radius: 8px; background: var(--vscode-textCodeBlock-background, rgba(127,127,127,.14)); max-height: 40vh; overflow: auto; }
+  /* MCP call arguments — wrap rather than force horizontal scroll, and keep them monospace + tidy. */
+  .tl-ask .mcpargs { margin: 0; padding: 9px 12px; white-space: pre-wrap; word-break: break-word; font-family: var(--vscode-editor-font-family, ui-monospace, monospace); font-size: 12px; line-height: 1.5; }
+  .tl-ask .askbtns .mcp-always { margin-left: auto; }
   .tl-ask .askbtns { display: flex; justify-content: space-between; align-items: center; gap: 8px; margin-top: 11px; }
   .tl-ask .askbtns button { cursor: pointer; border: 1px solid transparent; border-radius: 7px; padding: 5px 14px; font-size: 12px; font-weight: 500;
     display: inline-flex; align-items: baseline; gap: 6px; }   /* transparent border keeps both buttons the same height */
@@ -1871,7 +1874,59 @@
   // I being asked", which is exactly what it is for. On decision it collapses to a one-line verdict;
   // the live run card follows with the output.
   let pendingApproval = null;   // { done } while a decision is awaited — Enter approves, Esc skips
+  // MCP tool call (S4) — its own card: server · tool · arguments, so the user sees exactly what a
+  // third-party tool is about to do. Args are shown in full (capped host-side): that IS the decision.
+  function addMcpApproval(m){
+    clearEmpty(); clearStatus(); agentBubble = null;
+    closeGroup();
+    const card = document.createElement('div'); card.className = 'tl tl-cmd tl-ask asking';
+    const dangerLine = m.destructive
+      ? '<div class="askdanger">' + codicon('warning') + ' The server marks this tool <b>destructive</b>. It will always ask, even if allow-listed.</div>'
+      : '';
+    const argsWell = (m.args && String(m.args).trim())
+      ? '<div class="askcode"><pre class="cmdsrc mcpargs">' + esc(m.args) + '</pre></div>'
+      : '<div class="asksub" style="opacity:.7">No arguments.</div>';
+    // "Always allow" only when the tool CAN be allow-listed — a destructive tool can't, so we don't
+    // offer a button that would silently do nothing (mirrors describeMcpCall.canAllowAlways).
+    // "Always allow" reuses the secondary (skip-style) look but is selected by its OWN class — two
+    // buttons sharing `.approve` would make querySelector('.approve') ambiguous.
+    const always = m.canAllowAlways
+      ? '<button class="skip mcp-always" title="Run it now and add it to your allow-list so future runs don’t ask">Always allow</button>'
+      : '';
+    card.innerHTML =
+      '<div class="tl-rail"><span class="tl-node">' + codicon('shield') + '</span></div>'
+      + '<div class="tl-body"><div class="askcard">'
+        + '<div class="asktitle">Run an MCP tool?</div>'
+        + '<div class="asksub"><b>' + esc(m.server || '') + '</b> · ' + esc(m.tool || '') + ' — a third-party tool, not one of LevelCode’s own.</div>'
+        + dangerLine
+        + argsWell
+        + '<div class="askbtns">'
+          + '<button class="skip" title="Don’t run it (esc)">Skip <kbd>esc</kbd></button>'
+          + always
+          + '<button class="approve" title="Run it once (⏎)">Allow once <kbd>⏎</kbd></button>'
+        + '</div>'
+      + '</div></div>';
+    log.appendChild(card); scrollIfStuck();
+    const done = (approved, remember) => {
+      pendingApproval = null;
+      vscode.postMessage({ type: 'approvalResponse', id: m.id, approved, remember: !!remember, mcpName: m.name });
+      card.classList.remove('asking');
+      if (!approved) { card.classList.add('skipped'); }
+      const verb = !approved ? 'Skipped' : (remember ? 'Always allowed' : 'Approved');
+      card.querySelector('.tl-body').innerHTML =
+        '<div class="cmdhead"><span class="cmdverb">' + verb + '</span>'
+        + '<span class="cmdchips"><code>' + esc(m.server || '') + '</code><code>' + esc(m.tool || '') + '</code></span>'
+        + '<span class="cmdstate ' + (approved ? 'ok' : 'bad') + '">' + codicon(approved ? 'check-circle' : 'circle-slash') + '</span></div>';
+      forceStick();
+    };
+    card.querySelector('.approve').onclick = () => done(true, false);   // Allow once (primary)
+    card.querySelector('.skip:not(.mcp-always)').onclick = () => done(false, false);
+    const alt = card.querySelector('.mcp-always'); if (alt) { alt.onclick = () => done(true, true); }
+    pendingApproval = { done };   // Enter = Allow once, Esc = Skip
+  }
+
   function addApproval(m){
+    if (m.kind === 'mcp') { return addMcpApproval(m); }
     clearEmpty(); clearStatus(); agentBubble = null;
     closeGroup();   // a blocking gate never hides inside a collapsed group (D4)
     const card = document.createElement('div'); card.className = 'tl tl-cmd tl-ask asking';

extensions/levelcode-ai/test/mcpConfig.test.js

@@ -504,4 +504,43 @@ test('SOURCE: the mcp modules contain no raw control bytes', () => {
 	}
 });
 
+// ---- 6. describeMcpCall: the approval card's content (S4) -------------------------------------
+
+test('CARD: server/tool come from the route, with a namespaced-name fallback', () => {
+	const d = M.describeMcpCall('github__create_issue', { title: 'x' }, { server: 'github', tool: 'create_issue' });
+	assert.strictEqual(d.server, 'github');
+	assert.strictEqual(d.tool, 'create_issue');
+	// No route → split the namespaced name on the separator rather than showing a blank card.
+	const f = M.describeMcpCall('github__create_issue', {}, undefined);
+	assert.strictEqual(f.server, 'github');
+	assert.strictEqual(f.tool, 'create_issue');
+});
+
+test('CARD: a destructive tool cannot be "always allowed"', () => {
+	// The card must not offer a button that does nothing — a destructive tool can never be allow-listed
+	// (classifyMcpTool tightens on it), so canAllowAlways is derived from the SAME annotation.
+	const d = M.describeMcpCall('gh__nuke', {}, { server: 'gh', tool: 'nuke', annotations: { destructiveHint: true } });
+	assert.strictEqual(d.destructive, true);
+	assert.strictEqual(d.canAllowAlways, false);
+	const ok = M.describeMcpCall('gh__list', {}, { server: 'gh', tool: 'list' });
+	assert.strictEqual(ok.destructive, false);
+	assert.strictEqual(ok.canAllowAlways, true);
+});
+
+test('CARD: arguments are shown in full but bounded, and never throw', () => {
+	const d = M.describeMcpCall('s__t', { path: '/etc/passwd', n: 3 }, { server: 's', tool: 't' });
+	assert.ok(d.argsText.includes('/etc/passwd') && d.argsText.includes('"n": 3'), 'the user must SEE the real args');
+
+	// Over the cap → truncated with an ellipsis, not dropped and not unbounded.
+	const big = M.describeMcpCall('s__t', { blob: 'z'.repeat(5000) }, { server: 's', tool: 't' });
+	assert.ok(big.argsText.length <= M.MAX_ARG_CHARS, 'args preview must obey MAX_ARG_CHARS');
+	assert.ok(big.argsText.endsWith('…'));
+
+	// Circular / weird input must not throw inside the card renderer's data prep.
+	const circ = {}; circ.self = circ;
+	assert.doesNotThrow(() => M.describeMcpCall('s__t', circ, { server: 's', tool: 't' }));
+	assert.strictEqual(M.describeMcpCall('s__t', null, { server: 's', tool: 't' }).argsText, '');
+	assert.strictEqual(M.describeMcpCall('s__t', undefined, { server: 's', tool: 't' }).argsText, '');
+});
+
 console.log('\nmcpConfig.js: ' + n + ' tests passed.');