Improve MCP settings auth flow, remote config support, and diagnostics UX (#953)

* feat: improve MCP settings auth workflow * fix: complete MCP settings auth flow * fix: harden MCP settings auth flow * fix: add MCP settings refresh control * fix: stabilize MCP authorization and status handling * fix: clarify MCP advanced remote options toggle * fix: improve MCP import and diagnostics * feat: improve MCP settings panel visual hierarchy and UX * fix: expose MCP auth actions in connected state * fix: remove MCP import snippet helper text * fix: address MCP review feedback * fix: correct MCP page transport layout after rebase
2026-04-21 20:46:51 +03:00
parent b1a96c7b36
commit d73edc672e
16 changed files with 2554 additions and 131 deletions
@@ -20,6 +20,29 @@ export const registerConfigEntityRoutes = (app, dependencies) => {
    deleteMcpConfig,
  } = dependencies;

+  const completeMcpMutation = async (res, action, name, applyChange) => {
+    applyChange();
+
+    try {
+      await refreshOpenCodeAfterConfigChange(`mcp ${action}`);
+      return res.json({
+        success: true,
+        requiresReload: true,
+        message: `MCP server "${name}" ${action}d. Reloading interface…`,
+        reloadDelayMs: clientReloadDelayMs,
+      });
+    } catch (error) {
+      console.error(`[API:MCP ${action}] Reload failed after config write:`, error);
+      return res.json({
+        success: true,
+        requiresReload: false,
+        reloadFailed: true,
+        message: `MCP server "${name}" ${action}d, but OpenCode reload failed.`,
+        warning: error.message || 'OpenCode reload failed after the MCP configuration changed',
+      });
+    }
+  };
+
  app.get('/api/config/agents/:name', async (req, res) => {
    try {
      const agentName = req.params.name;
@@ -187,14 +210,8 @@ export const registerConfigEntityRoutes = (app, dependencies) => {
      }
      console.log(`[API:POST /api/config/mcp] Creating MCP server: ${name}`);

-      createMcpConfig(name, config, directory, scope);
-      await refreshOpenCodeAfterConfigChange('mcp creation', { mcpName: name });
-
-      res.json({
-        success: true,
-        requiresReload: true,
-        message: `MCP server "${name}" created. Reloading interface…`,
-        reloadDelayMs: clientReloadDelayMs,
+      await completeMcpMutation(res, 'create', name, () => {
+        createMcpConfig(name, config, directory, scope);
      });
    } catch (error) {
      console.error('[API:POST /api/config/mcp/:name] Failed:', error);
@@ -212,17 +229,14 @@ export const registerConfigEntityRoutes = (app, dependencies) => {
      }
      console.log(`[API:PATCH /api/config/mcp] Updating MCP server: ${name}`);

-      updateMcpConfig(name, updates, directory);
-      await refreshOpenCodeAfterConfigChange('mcp update');
-
-      res.json({
-        success: true,
-        requiresReload: true,
-        message: `MCP server "${name}" updated. Reloading interface…`,
-        reloadDelayMs: clientReloadDelayMs,
+      await completeMcpMutation(res, 'update', name, () => {
+        updateMcpConfig(name, updates, directory);
      });
    } catch (error) {
      console.error('[API:PATCH /api/config/mcp/:name] Failed:', error);
+      if (error?.message === `MCP server "${req.params.name}" not found`) {
+        return res.status(404).json({ error: error.message });
+      }
      res.status(500).json({ error: error.message || 'Failed to update MCP server' });
    }
  });
@@ -236,14 +250,8 @@ export const registerConfigEntityRoutes = (app, dependencies) => {
      }
      console.log(`[API:DELETE /api/config/mcp] Deleting MCP server: ${name}`);

-      deleteMcpConfig(name, directory);
-      await refreshOpenCodeAfterConfigChange('mcp deletion');
-
-      res.json({
-        success: true,
-        requiresReload: true,
-        message: `MCP server "${name}" deleted. Reloading interface…`,
-        reloadDelayMs: clientReloadDelayMs,
+      await completeMcpMutation(res, 'delete', name, () => {
+        deleteMcpConfig(name, directory);
      });
    } catch (error) {
      console.error('[API:DELETE /api/config/mcp/:name] Failed:', error);
@@ -118,6 +118,11 @@ function createMcpConfig(name, mcpConfig, workingDirectory, scope) {
 function updateMcpConfig(name, updates, workingDirectory) {
  const layers = readConfigLayers(workingDirectory);
  const source = getJsonEntrySource(layers, 'mcp', name);
+
+  if (!source.exists) {
+    throw new Error(`MCP server "${name}" not found`);
+  }
+
  const targetPath = source.path || CONFIG_FILE;
  const config = source.config || (fs.existsSync(targetPath) ? readConfigFile(targetPath) : {});

@@ -125,7 +130,7 @@ function updateMcpConfig(name, updates, workingDirectory) {
    config.mcp = {};
  }

-  const existing = config.mcp[name] ?? {};
+  const existing = config.mcp[name];
  const { name: _ignoredName, ...updateData } = updates;

  config.mcp[name] = buildMcpEntry({ ...existing, ...updateData });
@@ -161,7 +166,12 @@ function deleteMcpConfig(name, workingDirectory) {
 * Build a clean MCP entry object, omitting undefined/null values
 */
 function buildMcpEntry(data) {
-  const entry = {};
+  const entry = (data && typeof data === 'object' && !Array.isArray(data))
+    ? { ...data }
+    : {};
+
+  delete entry.name;
+  delete entry.scope;

  // type is required
  entry.type = data.type === 'remote' ? 'remote' : 'local';
@@ -170,11 +180,69 @@ function buildMcpEntry(data) {
    // command must be a non-empty array of strings
    if (Array.isArray(data.command) && data.command.length > 0) {
      entry.command = data.command.map(String);
+    } else {
+      delete entry.command;
    }
+
+    delete entry.url;
+    delete entry.headers;
+    delete entry.oauth;
+    delete entry.timeout;
  } else {
    // remote: url required
    if (data.url && typeof data.url === 'string') {
      entry.url = data.url.trim();
+    } else {
+      delete entry.url;
+    }
+
+    delete entry.command;
+
+    if (data.headers && typeof data.headers === 'object' && !Array.isArray(data.headers)) {
+      const cleaned = {};
+      for (const [k, v] of Object.entries(data.headers)) {
+        if (k && v !== undefined && v !== null) {
+          cleaned[k] = String(v);
+        }
+      }
+      if (Object.keys(cleaned).length > 0) {
+        entry.headers = cleaned;
+      } else {
+        delete entry.headers;
+      }
+    } else if (data.headers === undefined) {
+      delete entry.headers;
+    }
+
+    if (data.oauth === false) {
+      entry.oauth = false;
+    } else if (data.oauth && typeof data.oauth === 'object' && !Array.isArray(data.oauth)) {
+      const oauth = {};
+      if (typeof data.oauth.clientId === 'string' && data.oauth.clientId.trim()) {
+        oauth.clientId = data.oauth.clientId.trim();
+      }
+      if (typeof data.oauth.clientSecret === 'string' && data.oauth.clientSecret.trim()) {
+        oauth.clientSecret = data.oauth.clientSecret.trim();
+      }
+      if (typeof data.oauth.scope === 'string' && data.oauth.scope.trim()) {
+        oauth.scope = data.oauth.scope.trim();
+      }
+      if (typeof data.oauth.redirectUri === 'string' && data.oauth.redirectUri.trim()) {
+        oauth.redirectUri = data.oauth.redirectUri.trim();
+      }
+      if (Object.keys(oauth).length > 0) {
+        entry.oauth = oauth;
+      } else {
+        delete entry.oauth;
+      }
+    } else if (data.oauth === undefined) {
+      delete entry.oauth;
+    }
+
+    if (typeof data.timeout === 'number' && Number.isFinite(data.timeout) && data.timeout > 0) {
+      entry.timeout = data.timeout;
+    } else if (data.timeout === undefined || data.timeout === null || data.timeout === '') {
+      delete entry.timeout;
    }
  }

@@ -188,7 +256,11 @@ function buildMcpEntry(data) {
    }
    if (Object.keys(cleaned).length > 0) {
      entry.environment = cleaned;
+    } else {
+      delete entry.environment;
    }
+  } else if (data.environment === undefined) {
+    delete entry.environment;
  }

  // enabled defaults to true
@@ -18,6 +18,8 @@ export const registerOpenCodeRoutes = (app, dependencies) => {
  } = dependencies;

  let authLibrary = null;
+  const pendingMcpAuthContextByState = new Map();
+  const PENDING_MCP_AUTH_TTL_MS = 30 * 60 * 1000;
  const getAuthLibrary = async () => {
    if (!authLibrary) {
      authLibrary = await import('./auth.js');
@@ -25,6 +27,24 @@ export const registerOpenCodeRoutes = (app, dependencies) => {
    return authLibrary;
  };

+  const normalizePendingString = (value) => {
+    if (typeof value !== 'string') {
+      return null;
+    }
+
+    const trimmed = value.trim();
+    return trimmed || null;
+  };
+
+  const pruneExpiredPendingMcpAuthContexts = () => {
+    const now = Date.now();
+    for (const [state, entry] of pendingMcpAuthContextByState.entries()) {
+      if (!entry || typeof entry.expiresAt !== 'number' || entry.expiresAt <= now) {
+        pendingMcpAuthContextByState.delete(state);
+      }
+    }
+  };
+
  app.get('/api/config/settings', async (_req, res) => {
    try {
      const settings = await readSettingsFromDiskMigrated();
@@ -59,6 +79,76 @@ export const registerOpenCodeRoutes = (app, dependencies) => {
    }
  });

+  app.post('/api/mcp/auth/pending', async (req, res) => {
+    try {
+      pruneExpiredPendingMcpAuthContexts();
+
+      const state = normalizePendingString(req.body?.state);
+      if (!state) {
+        return res.json({ success: true, context: null });
+      }
+
+      const name = normalizePendingString(req.body?.name);
+      if (!name) {
+        return res.status(400).json({ error: 'MCP server name is required' });
+      }
+
+      const entry = {
+        name,
+        directory: normalizePendingString(req.body?.directory),
+        expiresAt: Date.now() + PENDING_MCP_AUTH_TTL_MS,
+      };
+      pendingMcpAuthContextByState.set(state, entry);
+
+      return res.json({
+        success: true,
+        context: {
+          name: entry.name,
+          directory: entry.directory,
+        },
+      });
+    } catch (error) {
+      console.error('Failed to store pending MCP auth context:', error);
+      return res.status(500).json({ error: error.message || 'Failed to store pending MCP auth context' });
+    }
+  });
+
+  app.get('/api/mcp/auth/pending', async (req, res) => {
+    try {
+      pruneExpiredPendingMcpAuthContexts();
+
+      const state = normalizePendingString(Array.isArray(req.query?.state) ? req.query.state[0] : req.query?.state);
+      if (!state) {
+        return res.json(null);
+      }
+
+      const pendingMcpAuthContext = pendingMcpAuthContextByState.get(state) ?? null;
+      if (!pendingMcpAuthContext) {
+        return res.status(404).json({ error: 'No pending MCP auth context' });
+      }
+
+      return res.json(pendingMcpAuthContext);
+    } catch (error) {
+      console.error('Failed to read pending MCP auth context:', error);
+      return res.status(500).json({ error: error.message || 'Failed to read pending MCP auth context' });
+    }
+  });
+
+  app.delete('/api/mcp/auth/pending', async (req, res) => {
+    try {
+      const state = normalizePendingString(Array.isArray(req.query?.state) ? req.query.state[0] : req.query?.state);
+      if (!state) {
+        return res.json({ success: true });
+      }
+
+      pendingMcpAuthContextByState.delete(state);
+      return res.json({ success: true });
+    } catch (error) {
+      console.error('Failed to clear pending MCP auth context:', error);
+      return res.status(500).json({ error: error.message || 'Failed to clear pending MCP auth context' });
+    }
+  });
+
  app.get('/api/provider/:providerId/source', async (req, res) => {
    try {
      const { providerId } = req.params;