Merge origin/main into feat/cursor-cli

Merges latest main branch changes including:
- MCP server support and configuration
- Pipeline configuration system
- Prompt customization settings
- GitHub issue comments in validation
- Auth middleware improvements
- Various UI/UX improvements

All Cursor CLI features preserved:
- Multi-provider support (Claude + Cursor)
- Model override capabilities
- Phase model configuration
- Provider tabs in settings

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

apps/server/tests/unit/lib/auth.test.ts

@@ -1,5 +1,7 @@
 import { describe, it, expect, beforeEach, vi } from 'vitest';
 import { createMockExpressContext } from '../../utils/mocks.js';
+import fs from 'fs';
+import path from 'path';
 
 /**
  * Note: auth.ts reads AUTOMAKER_API_KEY at module load time.
@@ -8,26 +10,13 @@ import { createMockExpressContext } from '../../utils/mocks.js';
 describe('auth.ts', () => {
   beforeEach(() => {
     vi.resetModules();
+    delete process.env.AUTOMAKER_API_KEY;
+    delete process.env.AUTOMAKER_HIDE_API_KEY;
+    delete process.env.NODE_ENV;
   });
 
-  describe('authMiddleware - no API key', () => {
-    it('should call next() when no API key is set', async () => {
-      delete process.env.AUTOMAKER_API_KEY;
-
-      const { authMiddleware } = await import('@/lib/auth.js');
-      const { req, res, next } = createMockExpressContext();
-
-      authMiddleware(req, res, next);
-
-      expect(next).toHaveBeenCalled();
-      expect(res.status).not.toHaveBeenCalled();
-    });
-  });
-
-  describe('authMiddleware - with API key', () => {
-    it('should reject request without API key header', async () => {
-      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
-
+  describe('authMiddleware', () => {
+    it('should reject request without any authentication', async () => {
       const { authMiddleware } = await import('@/lib/auth.js');
       const { req, res, next } = createMockExpressContext();
 
@@ -36,7 +25,7 @@ describe('auth.ts', () => {
       expect(res.status).toHaveBeenCalledWith(401);
       expect(res.json).toHaveBeenCalledWith({
         success: false,
-        error: 'Authentication required. Provide X-API-Key header.',
+        error: 'Authentication required.',
       });
       expect(next).not.toHaveBeenCalled();
     });
@@ -70,46 +59,340 @@ describe('auth.ts', () => {
       expect(next).toHaveBeenCalled();
       expect(res.status).not.toHaveBeenCalled();
     });
+
+    it('should authenticate with session token in header', async () => {
+      const { authMiddleware, createSession } = await import('@/lib/auth.js');
+      const token = await createSession();
+      const { req, res, next } = createMockExpressContext();
+      req.headers['x-session-token'] = token;
+
+      authMiddleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+    });
+
+    it('should reject invalid session token in header', async () => {
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContext();
+      req.headers['x-session-token'] = 'invalid-token';
+
+      authMiddleware(req, res, next);
+
+      expect(res.status).toHaveBeenCalledWith(403);
+      expect(res.json).toHaveBeenCalledWith({
+        success: false,
+        error: 'Invalid or expired session token.',
+      });
+      expect(next).not.toHaveBeenCalled();
+    });
+
+    it('should authenticate with API key in query parameter', async () => {
+      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
+
+      const { authMiddleware } = await import('@/lib/auth.js');
+      const { req, res, next } = createMockExpressContext();
+      req.query.apiKey = 'test-secret-key';
+
+      authMiddleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+    });
+
+    it('should authenticate with session cookie', async () => {
+      const { authMiddleware, createSession, getSessionCookieName } = await import('@/lib/auth.js');
+      const token = await createSession();
+      const cookieName = getSessionCookieName();
+      const { req, res, next } = createMockExpressContext();
+      req.cookies = { [cookieName]: token };
+
+      authMiddleware(req, res, next);
+
+      expect(next).toHaveBeenCalled();
+      expect(res.status).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('createSession', () => {
+    it('should create a new session and return token', async () => {
+      const { createSession } = await import('@/lib/auth.js');
+      const token = await createSession();
+
+      expect(token).toBeDefined();
+      expect(typeof token).toBe('string');
+      expect(token.length).toBeGreaterThan(0);
+    });
+
+    it('should create unique tokens for each session', async () => {
+      const { createSession } = await import('@/lib/auth.js');
+      const token1 = await createSession();
+      const token2 = await createSession();
+
+      expect(token1).not.toBe(token2);
+    });
+  });
+
+  describe('validateSession', () => {
+    it('should validate a valid session token', async () => {
+      const { createSession, validateSession } = await import('@/lib/auth.js');
+      const token = await createSession();
+
+      expect(validateSession(token)).toBe(true);
+    });
+
+    it('should reject invalid session token', async () => {
+      const { validateSession } = await import('@/lib/auth.js');
+
+      expect(validateSession('invalid-token')).toBe(false);
+    });
+
+    it('should reject expired session token', async () => {
+      vi.useFakeTimers();
+      const { createSession, validateSession } = await import('@/lib/auth.js');
+      const token = await createSession();
+
+      // Advance time past session expiration (30 days)
+      vi.advanceTimersByTime(31 * 24 * 60 * 60 * 1000);
+
+      expect(validateSession(token)).toBe(false);
+      vi.useRealTimers();
+    });
+  });
+
+  describe('invalidateSession', () => {
+    it('should invalidate a session token', async () => {
+      const { createSession, validateSession, invalidateSession } = await import('@/lib/auth.js');
+      const token = await createSession();
+
+      expect(validateSession(token)).toBe(true);
+      await invalidateSession(token);
+      expect(validateSession(token)).toBe(false);
+    });
+  });
+
+  describe('createWsConnectionToken', () => {
+    it('should create a WebSocket connection token', async () => {
+      const { createWsConnectionToken } = await import('@/lib/auth.js');
+      const token = createWsConnectionToken();
+
+      expect(token).toBeDefined();
+      expect(typeof token).toBe('string');
+      expect(token.length).toBeGreaterThan(0);
+    });
+
+    it('should create unique tokens', async () => {
+      const { createWsConnectionToken } = await import('@/lib/auth.js');
+      const token1 = createWsConnectionToken();
+      const token2 = createWsConnectionToken();
+
+      expect(token1).not.toBe(token2);
+    });
+  });
+
+  describe('validateWsConnectionToken', () => {
+    it('should validate a valid WebSocket token', async () => {
+      const { createWsConnectionToken, validateWsConnectionToken } = await import('@/lib/auth.js');
+      const token = createWsConnectionToken();
+
+      expect(validateWsConnectionToken(token)).toBe(true);
+    });
+
+    it('should reject invalid WebSocket token', async () => {
+      const { validateWsConnectionToken } = await import('@/lib/auth.js');
+
+      expect(validateWsConnectionToken('invalid-token')).toBe(false);
+    });
+
+    it('should reject expired WebSocket token', async () => {
+      vi.useFakeTimers();
+      const { createWsConnectionToken, validateWsConnectionToken } = await import('@/lib/auth.js');
+      const token = createWsConnectionToken();
+
+      // Advance time past token expiration (5 minutes)
+      vi.advanceTimersByTime(6 * 60 * 1000);
+
+      expect(validateWsConnectionToken(token)).toBe(false);
+      vi.useRealTimers();
+    });
+
+    it('should invalidate token after first use (single-use)', async () => {
+      const { createWsConnectionToken, validateWsConnectionToken } = await import('@/lib/auth.js');
+      const token = createWsConnectionToken();
+
+      expect(validateWsConnectionToken(token)).toBe(true);
+      // Token should be deleted after first use
+      expect(validateWsConnectionToken(token)).toBe(false);
+    });
+  });
+
+  describe('validateApiKey', () => {
+    it('should validate correct API key', async () => {
+      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
+
+      const { validateApiKey } = await import('@/lib/auth.js');
+
+      expect(validateApiKey('test-secret-key')).toBe(true);
+    });
+
+    it('should reject incorrect API key', async () => {
+      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
+
+      const { validateApiKey } = await import('@/lib/auth.js');
+
+      expect(validateApiKey('wrong-key')).toBe(false);
+    });
+
+    it('should reject empty string', async () => {
+      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
+
+      const { validateApiKey } = await import('@/lib/auth.js');
+
+      expect(validateApiKey('')).toBe(false);
+    });
+
+    it('should reject null/undefined', async () => {
+      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
+
+      const { validateApiKey } = await import('@/lib/auth.js');
+
+      expect(validateApiKey(null as any)).toBe(false);
+      expect(validateApiKey(undefined as any)).toBe(false);
+    });
+
+    it('should use timing-safe comparison for different lengths', async () => {
+      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
+
+      const { validateApiKey } = await import('@/lib/auth.js');
+
+      // Key with different length should be rejected without timing leak
+      expect(validateApiKey('short')).toBe(false);
+      expect(validateApiKey('very-long-key-that-does-not-match')).toBe(false);
+    });
+  });
+
+  describe('getSessionCookieOptions', () => {
+    it('should return cookie options with httpOnly true', async () => {
+      const { getSessionCookieOptions } = await import('@/lib/auth.js');
+      const options = getSessionCookieOptions();
+
+      expect(options.httpOnly).toBe(true);
+      expect(options.sameSite).toBe('strict');
+      expect(options.path).toBe('/');
+      expect(options.maxAge).toBeGreaterThan(0);
+    });
+
+    it('should set secure to true in production', async () => {
+      process.env.NODE_ENV = 'production';
+
+      const { getSessionCookieOptions } = await import('@/lib/auth.js');
+      const options = getSessionCookieOptions();
+
+      expect(options.secure).toBe(true);
+    });
+
+    it('should set secure to false in non-production', async () => {
+      process.env.NODE_ENV = 'development';
+
+      const { getSessionCookieOptions } = await import('@/lib/auth.js');
+      const options = getSessionCookieOptions();
+
+      expect(options.secure).toBe(false);
+    });
+  });
+
+  describe('getSessionCookieName', () => {
+    it('should return the session cookie name', async () => {
+      const { getSessionCookieName } = await import('@/lib/auth.js');
+      const name = getSessionCookieName();
+
+      expect(name).toBe('automaker_session');
+    });
+  });
+
+  describe('isRequestAuthenticated', () => {
+    it('should return true for authenticated request with API key', async () => {
+      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
+
+      const { isRequestAuthenticated } = await import('@/lib/auth.js');
+      const { req } = createMockExpressContext();
+      req.headers['x-api-key'] = 'test-secret-key';
+
+      expect(isRequestAuthenticated(req)).toBe(true);
+    });
+
+    it('should return false for unauthenticated request', async () => {
+      const { isRequestAuthenticated } = await import('@/lib/auth.js');
+      const { req } = createMockExpressContext();
+
+      expect(isRequestAuthenticated(req)).toBe(false);
+    });
+
+    it('should return true for authenticated request with session token', async () => {
+      const { isRequestAuthenticated, createSession } = await import('@/lib/auth.js');
+      const token = await createSession();
+      const { req } = createMockExpressContext();
+      req.headers['x-session-token'] = token;
+
+      expect(isRequestAuthenticated(req)).toBe(true);
+    });
+  });
+
+  describe('checkRawAuthentication', () => {
+    it('should return true for valid API key in headers', async () => {
+      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
+
+      const { checkRawAuthentication } = await import('@/lib/auth.js');
+
+      expect(checkRawAuthentication({ 'x-api-key': 'test-secret-key' }, {}, {})).toBe(true);
+    });
+
+    it('should return true for valid session token in headers', async () => {
+      const { checkRawAuthentication, createSession } = await import('@/lib/auth.js');
+      const token = await createSession();
+
+      expect(checkRawAuthentication({ 'x-session-token': token }, {}, {})).toBe(true);
+    });
+
+    it('should return true for valid API key in query', async () => {
+      process.env.AUTOMAKER_API_KEY = 'test-secret-key';
+
+      const { checkRawAuthentication } = await import('@/lib/auth.js');
+
+      expect(checkRawAuthentication({}, { apiKey: 'test-secret-key' }, {})).toBe(true);
+    });
+
+    it('should return true for valid session cookie', async () => {
+      const { checkRawAuthentication, createSession, getSessionCookieName } =
+        await import('@/lib/auth.js');
+      const token = await createSession();
+      const cookieName = getSessionCookieName();
+
+      expect(checkRawAuthentication({}, {}, { [cookieName]: token })).toBe(true);
+    });
+
+    it('should return false for invalid credentials', async () => {
+      const { checkRawAuthentication } = await import('@/lib/auth.js');
+
+      expect(checkRawAuthentication({}, {}, {})).toBe(false);
+    });
   });
 
   describe('isAuthEnabled', () => {
-    it('should return false when no API key is set', async () => {
-      delete process.env.AUTOMAKER_API_KEY;
-
-      const { isAuthEnabled } = await import('@/lib/auth.js');
-      expect(isAuthEnabled()).toBe(false);
-    });
-
-    it('should return true when API key is set', async () => {
-      process.env.AUTOMAKER_API_KEY = 'test-key';
-
+    it('should always return true (auth is always required)', async () => {
       const { isAuthEnabled } = await import('@/lib/auth.js');
       expect(isAuthEnabled()).toBe(true);
     });
   });
 
   describe('getAuthStatus', () => {
-    it('should return disabled status when no API key', async () => {
-      delete process.env.AUTOMAKER_API_KEY;
-
-      const { getAuthStatus } = await import('@/lib/auth.js');
-      const status = getAuthStatus();
-
-      expect(status).toEqual({
-        enabled: false,
-        method: 'none',
-      });
-    });
-
-    it('should return enabled status when API key is set', async () => {
-      process.env.AUTOMAKER_API_KEY = 'test-key';
-
+    it('should return enabled status with api_key_or_session method', async () => {
       const { getAuthStatus } = await import('@/lib/auth.js');
       const status = getAuthStatus();
 
       expect(status).toEqual({
         enabled: true,
-        method: 'api_key',
+        method: 'api_key_or_session',
       });
     });
   });

apps/server/tests/unit/lib/sdk-options.test.ts

@@ -179,7 +179,7 @@ describe('sdk-options.ts', () => {
     it('should create options with chat settings', async () => {
       const { createChatOptions, TOOL_PRESETS, MAX_TURNS } = await import('@/lib/sdk-options.js');
 
-      const options = createChatOptions({ cwd: '/test/path' });
+      const options = createChatOptions({ cwd: '/test/path', enableSandboxMode: true });
 
       expect(options.cwd).toBe('/test/path');
       expect(options.maxTurns).toBe(MAX_TURNS.standard);
@@ -212,6 +212,27 @@ describe('sdk-options.ts', () => {
 
       expect(options.model).toBe('claude-sonnet-4-20250514');
     });
+
+    it('should not set sandbox when enableSandboxMode is false', async () => {
+      const { createChatOptions } = await import('@/lib/sdk-options.js');
+
+      const options = createChatOptions({
+        cwd: '/test/path',
+        enableSandboxMode: false,
+      });
+
+      expect(options.sandbox).toBeUndefined();
+    });
+
+    it('should not set sandbox when enableSandboxMode is not provided', async () => {
+      const { createChatOptions } = await import('@/lib/sdk-options.js');
+
+      const options = createChatOptions({
+        cwd: '/test/path',
+      });
+
+      expect(options.sandbox).toBeUndefined();
+    });
   });
 
   describe('createAutoModeOptions', () => {
@@ -219,7 +240,7 @@ describe('sdk-options.ts', () => {
       const { createAutoModeOptions, TOOL_PRESETS, MAX_TURNS } =
         await import('@/lib/sdk-options.js');
 
-      const options = createAutoModeOptions({ cwd: '/test/path' });
+      const options = createAutoModeOptions({ cwd: '/test/path', enableSandboxMode: true });
 
       expect(options.cwd).toBe('/test/path');
       expect(options.maxTurns).toBe(MAX_TURNS.maximum);
@@ -252,6 +273,27 @@ describe('sdk-options.ts', () => {
 
       expect(options.abortController).toBe(abortController);
     });
+
+    it('should not set sandbox when enableSandboxMode is false', async () => {
+      const { createAutoModeOptions } = await import('@/lib/sdk-options.js');
+
+      const options = createAutoModeOptions({
+        cwd: '/test/path',
+        enableSandboxMode: false,
+      });
+
+      expect(options.sandbox).toBeUndefined();
+    });
+
+    it('should not set sandbox when enableSandboxMode is not provided', async () => {
+      const { createAutoModeOptions } = await import('@/lib/sdk-options.js');
+
+      const options = createAutoModeOptions({
+        cwd: '/test/path',
+      });
+
+      expect(options.sandbox).toBeUndefined();
+    });
   });
 
   describe('createCustomOptions', () => {

apps/server/tests/unit/lib/settings-helpers.test.ts

@@ -0,0 +1,378 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { getMCPServersFromSettings, getMCPPermissionSettings } from '@/lib/settings-helpers.js';
+import type { SettingsService } from '@/services/settings-service.js';
+
+// Mock the logger
+vi.mock('@automaker/utils', async () => {
+  const actual = await vi.importActual('@automaker/utils');
+  const mockLogger = {
+    info: vi.fn(),
+    error: vi.fn(),
+    warn: vi.fn(),
+    debug: vi.fn(),
+  };
+  return {
+    ...actual,
+    createLogger: () => mockLogger,
+  };
+});
+
+describe('settings-helpers.ts', () => {
+  describe('getMCPServersFromSettings', () => {
+    beforeEach(() => {
+      vi.clearAllMocks();
+    });
+
+    it('should return empty object when settingsService is null', async () => {
+      const result = await getMCPServersFromSettings(null);
+      expect(result).toEqual({});
+    });
+
+    it('should return empty object when settingsService is undefined', async () => {
+      const result = await getMCPServersFromSettings(undefined);
+      expect(result).toEqual({});
+    });
+
+    it('should return empty object when no MCP servers configured', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({ mcpServers: [] }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result).toEqual({});
+    });
+
+    it('should return empty object when mcpServers is undefined', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({}),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result).toEqual({});
+    });
+
+    it('should convert enabled stdio server to SDK format', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            {
+              id: '1',
+              name: 'test-server',
+              type: 'stdio',
+              command: 'node',
+              args: ['server.js'],
+              env: { NODE_ENV: 'test' },
+              enabled: true,
+            },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result).toEqual({
+        'test-server': {
+          type: 'stdio',
+          command: 'node',
+          args: ['server.js'],
+          env: { NODE_ENV: 'test' },
+        },
+      });
+    });
+
+    it('should convert enabled SSE server to SDK format', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            {
+              id: '1',
+              name: 'sse-server',
+              type: 'sse',
+              url: 'http://localhost:3000/sse',
+              headers: { Authorization: 'Bearer token' },
+              enabled: true,
+            },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result).toEqual({
+        'sse-server': {
+          type: 'sse',
+          url: 'http://localhost:3000/sse',
+          headers: { Authorization: 'Bearer token' },
+        },
+      });
+    });
+
+    it('should convert enabled HTTP server to SDK format', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            {
+              id: '1',
+              name: 'http-server',
+              type: 'http',
+              url: 'http://localhost:3000/api',
+              headers: { 'X-API-Key': 'secret' },
+              enabled: true,
+            },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result).toEqual({
+        'http-server': {
+          type: 'http',
+          url: 'http://localhost:3000/api',
+          headers: { 'X-API-Key': 'secret' },
+        },
+      });
+    });
+
+    it('should filter out disabled servers', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            {
+              id: '1',
+              name: 'enabled-server',
+              type: 'stdio',
+              command: 'node',
+              enabled: true,
+            },
+            {
+              id: '2',
+              name: 'disabled-server',
+              type: 'stdio',
+              command: 'python',
+              enabled: false,
+            },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(Object.keys(result)).toHaveLength(1);
+      expect(result['enabled-server']).toBeDefined();
+      expect(result['disabled-server']).toBeUndefined();
+    });
+
+    it('should treat servers without enabled field as enabled', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            {
+              id: '1',
+              name: 'implicit-enabled',
+              type: 'stdio',
+              command: 'node',
+              // enabled field not set
+            },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result['implicit-enabled']).toBeDefined();
+    });
+
+    it('should handle multiple enabled servers', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            { id: '1', name: 'server1', type: 'stdio', command: 'node', enabled: true },
+            { id: '2', name: 'server2', type: 'stdio', command: 'python', enabled: true },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(Object.keys(result)).toHaveLength(2);
+      expect(result['server1']).toBeDefined();
+      expect(result['server2']).toBeDefined();
+    });
+
+    it('should return empty object and log error on exception', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockRejectedValue(new Error('Settings error')),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService, '[Test]');
+      expect(result).toEqual({});
+      // Logger will be called with error, but we don't need to assert it
+    });
+
+    it('should throw error for SSE server without URL', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            {
+              id: '1',
+              name: 'bad-sse',
+              type: 'sse',
+              enabled: true,
+              // url missing
+            },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      // The error is caught and logged, returns empty
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result).toEqual({});
+    });
+
+    it('should throw error for HTTP server without URL', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            {
+              id: '1',
+              name: 'bad-http',
+              type: 'http',
+              enabled: true,
+              // url missing
+            },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result).toEqual({});
+    });
+
+    it('should throw error for stdio server without command', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            {
+              id: '1',
+              name: 'bad-stdio',
+              type: 'stdio',
+              enabled: true,
+              // command missing
+            },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result).toEqual({});
+    });
+
+    it('should default to stdio type when type is not specified', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpServers: [
+            {
+              id: '1',
+              name: 'no-type',
+              command: 'node',
+              enabled: true,
+              // type not specified, should default to stdio
+            },
+          ],
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPServersFromSettings(mockSettingsService);
+      expect(result['no-type']).toEqual({
+        type: 'stdio',
+        command: 'node',
+        args: undefined,
+        env: undefined,
+      });
+    });
+  });
+
+  describe('getMCPPermissionSettings', () => {
+    beforeEach(() => {
+      vi.clearAllMocks();
+    });
+
+    it('should return defaults when settingsService is null', async () => {
+      const result = await getMCPPermissionSettings(null);
+      expect(result).toEqual({
+        mcpAutoApproveTools: true,
+        mcpUnrestrictedTools: true,
+      });
+    });
+
+    it('should return defaults when settingsService is undefined', async () => {
+      const result = await getMCPPermissionSettings(undefined);
+      expect(result).toEqual({
+        mcpAutoApproveTools: true,
+        mcpUnrestrictedTools: true,
+      });
+    });
+
+    it('should return settings from service', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpAutoApproveTools: false,
+          mcpUnrestrictedTools: false,
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPPermissionSettings(mockSettingsService);
+      expect(result).toEqual({
+        mcpAutoApproveTools: false,
+        mcpUnrestrictedTools: false,
+      });
+    });
+
+    it('should default to true when settings are undefined', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({}),
+      } as unknown as SettingsService;
+
+      const result = await getMCPPermissionSettings(mockSettingsService);
+      expect(result).toEqual({
+        mcpAutoApproveTools: true,
+        mcpUnrestrictedTools: true,
+      });
+    });
+
+    it('should handle mixed settings', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpAutoApproveTools: true,
+          mcpUnrestrictedTools: false,
+        }),
+      } as unknown as SettingsService;
+
+      const result = await getMCPPermissionSettings(mockSettingsService);
+      expect(result).toEqual({
+        mcpAutoApproveTools: true,
+        mcpUnrestrictedTools: false,
+      });
+    });
+
+    it('should return defaults and log error on exception', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockRejectedValue(new Error('Settings error')),
+      } as unknown as SettingsService;
+
+      const result = await getMCPPermissionSettings(mockSettingsService, '[Test]');
+      expect(result).toEqual({
+        mcpAutoApproveTools: true,
+        mcpUnrestrictedTools: true,
+      });
+      // Logger will be called with error, but we don't need to assert it
+    });
+
+    it('should use custom log prefix', async () => {
+      const mockSettingsService = {
+        getGlobalSettings: vi.fn().mockResolvedValue({
+          mcpAutoApproveTools: true,
+          mcpUnrestrictedTools: true,
+        }),
+      } as unknown as SettingsService;
+
+      await getMCPPermissionSettings(mockSettingsService, '[CustomPrefix]');
+      // Logger will be called with custom prefix, but we don't need to assert it
+    });
+  });
+});