diff --git a/extensions/levelcode-ai/agent.js b/extensions/levelcode-ai/agent.js index f57fd33..d38febe 100644 --- a/extensions/levelcode-ai/agent.js +++ b/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) }); diff --git a/extensions/levelcode-ai/extension.js b/extensions/levelcode-ai/extension.js index 08fc1c6..07ebb3e 100644 --- a/extensions/levelcode-ai/extension.js +++ b/extensions/levelcode-ai/extension.js @@ -745,6 +745,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); @@ -1340,7 +1362,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; diff --git a/extensions/levelcode-ai/mcpConfig.js b/extensions/levelcode-ai/mcpConfig.js index b7dc942..2503718 100644 --- a/extensions/levelcode-ai/mcpConfig.js +++ b/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 }; diff --git a/extensions/levelcode-ai/media/chat.html b/extensions/levelcode-ai/media/chat.html index ed6a948..8ab6111 100644 --- a/extensions/levelcode-ai/media/chat.html +++ b/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 */ @@ -1771,7 +1774,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 + ? '
' + codicon('warning') + ' The server marks this tool destructive. It will always ask, even if allow-listed.
' + : ''; + const argsWell = (m.args && String(m.args).trim()) + ? '
' + esc(m.args) + '
' + : '
No arguments.
'; + // "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 + ? '' + : ''; + card.innerHTML = + '
' + codicon('shield') + '
' + + '
' + + '
Run an MCP tool?
' + + '
' + esc(m.server || '') + ' Β· ' + esc(m.tool || '') + ' β€” a third-party tool, not one of LevelCode’s own.
' + + dangerLine + + argsWell + + '
' + + '' + + always + + '' + + '
' + + '
'; + 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 = + '
' + verb + '' + + '' + esc(m.server || '') + '' + esc(m.tool || '') + '' + + '' + codicon(approved ? 'check-circle' : 'circle-slash') + '
'; + 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'; diff --git a/extensions/levelcode-ai/test/mcpConfig.test.js b/extensions/levelcode-ai/test/mcpConfig.test.js index 55e5736..f7de219 100644 --- a/extensions/levelcode-ai/test/mcpConfig.test.js +++ b/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.');