fix: remove parsing from mcp configs (#56)

Author: joshsny

src/steps/add-mcp-server-to-clients/MCPClient.ts

@@ -1,7 +1,101 @@
+import * as fs from 'fs';
+import * as path from 'path';
+import { getDefaultServerConfig } from './defaults';
+import { merge } from 'lodash';
+
 export abstract class MCPClient {
   name: string;
+  abstract getConfigPath(): Promise<string>;
   abstract isServerInstalled(): Promise<boolean>;
-  abstract addServer(apiKey: string): Promise<void>;
-  abstract removeServer(): Promise<void>;
-  abstract isClientSupported(): boolean;
+  abstract addServer(apiKey: string): Promise<{ success: boolean }>;
+  abstract removeServer(): Promise<{ success: boolean }>;
+  abstract isClientSupported(): Promise<boolean>;
+}
+
+export abstract class DefaultMCPClient extends MCPClient {
+  name = 'Default';
+
+  constructor() {
+    super();
+  }
+  async isServerInstalled(): Promise<boolean> {
+    try {
+      const configPath = await this.getConfigPath();
+
+      if (!fs.existsSync(configPath)) {
+        return false;
+      }
+
+      const configContent = await fs.promises.readFile(configPath, 'utf8');
+      const config = JSON.parse(configContent);
+
+      return 'mcpServers' in config && 'posthog' in config.mcpServers;
+    } catch {
+      return false;
+    }
+  }
+
+  async addServer(apiKey: string): Promise<{ success: boolean }> {
+    try {
+      const configPath = await this.getConfigPath();
+      const configDir = path.dirname(configPath);
+
+      await fs.promises.mkdir(configDir, { recursive: true });
+
+      const newServerConfig = {
+        mcpServers: {
+          posthog: getDefaultServerConfig(apiKey),
+        },
+      };
+
+      let existingConfig = {};
+
+      if (fs.existsSync(configPath)) {
+        const existingContent = await fs.promises.readFile(configPath, 'utf8');
+        existingConfig = JSON.parse(existingContent);
+      }
+
+      const mergedConfig = merge({}, existingConfig, newServerConfig);
+
+      await fs.promises.writeFile(
+        configPath,
+        JSON.stringify(mergedConfig, null, 2),
+        'utf8',
+      );
+
+      return { success: true };
+    } catch {
+      //
+    }
+    return { success: false };
+  }
+
+  async removeServer(): Promise<{ success: boolean }> {
+    try {
+      const configPath = await this.getConfigPath();
+
+      if (!fs.existsSync(configPath)) {
+        return { success: false };
+      }
+
+      const configContent = await fs.promises.readFile(configPath, 'utf8');
+      const config = JSON.parse(configContent);
+
+      if ('mcpServers' in config && 'posthog' in config.mcpServers) {
+        delete config.mcpServers.posthog;
+
+        await fs.promises.writeFile(
+          configPath,
+          JSON.stringify(config, null, 2),
+          'utf8',
+        );
+
+        return { success: true };
+      }
+    } catch {
+      //
+    }
+
+    return { success: false };
+  }
 }

src/steps/add-mcp-server-to-clients/clients/__tests__/claude.test.ts

@@ -1,3 +1,4 @@
+// We use the ClaudeMCPClient as a reference to test the DefaultMCPClient
 import * as fs from 'fs';
 import * as path from 'path';
 import * as os from 'os';
@@ -68,47 +69,47 @@ describe('ClaudeMCPClient', () => {
   });
 
   describe('isClientSupported', () => {
-    it('should return true for macOS', () => {
+    it('should return true for macOS', async () => {
       Object.defineProperty(process, 'platform', {
         value: 'darwin',
         writable: true,
       });
-      expect(client.isClientSupported()).toBe(true);
+      await expect(client.isClientSupported()).resolves.toBe(true);
     });
 
-    it('should return true for Windows', () => {
+    it('should return true for Windows', async () => {
       Object.defineProperty(process, 'platform', {
         value: 'win32',
         writable: true,
       });
-      expect(client.isClientSupported()).toBe(true);
+      await expect(client.isClientSupported()).resolves.toBe(true);
     });
 
-    it('should return false for Linux', () => {
+    it('should return false for Linux', async () => {
       Object.defineProperty(process, 'platform', {
         value: 'linux',
         writable: true,
       });
-      expect(client.isClientSupported()).toBe(false);
+      await expect(client.isClientSupported()).resolves.toBe(false);
     });
 
-    it('should return false for other platforms', () => {
+    it('should return false for other platforms', async () => {
       Object.defineProperty(process, 'platform', {
         value: 'freebsd',
         writable: true,
       });
-      expect(client.isClientSupported()).toBe(false);
+      await expect(client.isClientSupported()).resolves.toBe(false);
     });
   });
 
   describe('getConfigPath', () => {
-    it('should return correct path for macOS', () => {
+    it('should return correct path for macOS', async () => {
       Object.defineProperty(process, 'platform', {
         value: 'darwin',
         writable: true,
       });
 
-      const configPath = (client as any).getConfigPath();
+      const configPath = await client.getConfigPath();
       expect(configPath).toBe(
         path.join(
           mockHomeDir,
@@ -120,7 +121,7 @@ describe('ClaudeMCPClient', () => {
       );
     });
 
-    it('should return correct path for Windows', () => {
+    it('should return correct path for Windows', async () => {
       Object.defineProperty(process, 'platform', {
         value: 'win32',
         writable: true,
@@ -129,19 +130,19 @@ describe('ClaudeMCPClient', () => {
       const mockAppData = 'C:\\Users\\Test\\AppData\\Roaming';
       process.env.APPDATA = mockAppData;
 
-      const configPath = (client as any).getConfigPath();
+      const configPath = await client.getConfigPath();
       expect(configPath).toBe(
         path.join(mockAppData, 'Claude', 'claude_desktop_config.json'),
       );
     });
 
-    it('should throw error for unsupported platform', () => {
+    it('should throw error for unsupported platform', async () => {
       Object.defineProperty(process, 'platform', {
         value: 'linux',
         writable: true,
       });
 
-      expect(() => (client as any).getConfigPath()).toThrow(
+      await expect(client.getConfigPath()).rejects.toThrow(
         'Unsupported platform: linux',
       );
     });
@@ -231,6 +232,7 @@ describe('ClaudeMCPClient', () => {
       expect(mkdirMock).toHaveBeenCalledWith(expectedConfigDir, {
         recursive: true,
       });
+
       expect(writeFileMock).toHaveBeenCalledWith(
         expectedConfigPath,
         JSON.stringify(
@@ -277,16 +279,35 @@ describe('ClaudeMCPClient', () => {
       );
     });
 
-    it('should create new config when existing config is invalid', async () => {
+    it('should not overwrite existing config when it is invalid', async () => {
       existsSyncMock.mockReturnValue(true);
-      readFileMock.mockResolvedValue('invalid json');
+      readFileMock.mockResolvedValue(
+        JSON.stringify({
+          invalidKey: {
+            existingServer: {
+              command: 'existing',
+              args: [],
+              env: {},
+            },
+          },
+          x: 'y',
+        }),
+      );
 
       await client.addServer(mockApiKey);
 
       expect(writeFileMock).toHaveBeenCalledWith(
         expect.any(String),
         JSON.stringify(
           {
+            invalidKey: {
+              existingServer: {
+                command: 'existing',
+                args: [],
+                env: {},
+              },
+            },
+            x: 'y',
             mcpServers: {
               posthog: mockServerConfig,
             },