Feature custom themes logic (#2639)

Co-authored-by: Jacob Richman <[email protected]>

2 files changed, 189 insertions, 13 deletions

diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts
index 698ba745..b99e8b79 100644
--- a/packages/cli/src/config/settings.test.ts
+++ b/packages/cli/src/config/settings.test.ts
@@ -95,7 +95,10 @@ describe('Settings Loading and Merging', () => {
       expect(settings.system.settings).toEqual({});
       expect(settings.user.settings).toEqual({});
       expect(settings.workspace.settings).toEqual({});
-      expect(settings.merged).toEqual({});
+      expect(settings.merged).toEqual({
+        customThemes: {},
+        mcpServers: {},
+      });
       expect(settings.errors.length).toBe(0);
     });
 
@@ -124,7 +127,11 @@ describe('Settings Loading and Merging', () => {
       expect(settings.system.settings).toEqual(systemSettingsContent);
       expect(settings.user.settings).toEqual({});
       expect(settings.workspace.settings).toEqual({});
-      expect(settings.merged).toEqual(systemSettingsContent);
+      expect(settings.merged).toEqual({
+        ...systemSettingsContent,
+        customThemes: {},
+        mcpServers: {},
+      });
     });
 
     it('should load user settings if only user file exists', () => {
@@ -153,7 +160,11 @@ describe('Settings Loading and Merging', () => {
       );
       expect(settings.user.settings).toEqual(userSettingsContent);
       expect(settings.workspace.settings).toEqual({});
-      expect(settings.merged).toEqual(userSettingsContent);
+      expect(settings.merged).toEqual({
+        ...userSettingsContent,
+        customThemes: {},
+        mcpServers: {},
+      });
     });
 
     it('should load workspace settings if only workspace file exists', () => {
@@ -180,7 +191,11 @@ describe('Settings Loading and Merging', () => {
       );
       expect(settings.user.settings).toEqual({});
       expect(settings.workspace.settings).toEqual(workspaceSettingsContent);
-      expect(settings.merged).toEqual(workspaceSettingsContent);
+      expect(settings.merged).toEqual({
+        ...workspaceSettingsContent,
+        customThemes: {},
+        mcpServers: {},
+      });
     });
 
     it('should merge user and workspace settings, with workspace taking precedence', () => {
@@ -215,6 +230,8 @@ describe('Settings Loading and Merging', () => {
         sandbox: true,
         coreTools: ['tool1'],
         contextFileName: 'WORKSPACE_CONTEXT.md',
+        customThemes: {},
+        mcpServers: {},
       });
     });
 
@@ -262,6 +279,8 @@ describe('Settings Loading and Merging', () => {
         coreTools: ['tool1'],
         contextFileName: 'WORKSPACE_CONTEXT.md',
         allowMCPServers: ['server1', 'server2'],
+        customThemes: {},
+        mcpServers: {},
       });
     });
 
@@ -373,6 +392,134 @@ describe('Settings Loading and Merging', () => {
       (fs.readFileSync as Mock).mockReturnValue('{}');
       const settings = loadSettings(MOCK_WORKSPACE_DIR);
       expect(settings.merged.telemetry).toBeUndefined();
+      expect(settings.merged.customThemes).toEqual({});
+      expect(settings.merged.mcpServers).toEqual({});
+    });
+
+    it('should merge MCP servers correctly, with workspace taking precedence', () => {
+      (mockFsExistsSync as Mock).mockReturnValue(true);
+      const userSettingsContent = {
+        mcpServers: {
+          'user-server': {
+            command: 'user-command',
+            args: ['--user-arg'],
+            description: 'User MCP server',
+          },
+          'shared-server': {
+            command: 'user-shared-command',
+            description: 'User shared server config',
+          },
+        },
+      };
+      const workspaceSettingsContent = {
+        mcpServers: {
+          'workspace-server': {
+            command: 'workspace-command',
+            args: ['--workspace-arg'],
+            description: 'Workspace MCP server',
+          },
+          'shared-server': {
+            command: 'workspace-shared-command',
+            description: 'Workspace shared server config',
+          },
+        },
+      };
+
+      (fs.readFileSync as Mock).mockImplementation(
+        (p: fs.PathOrFileDescriptor) => {
+          if (p === USER_SETTINGS_PATH)
+            return JSON.stringify(userSettingsContent);
+          if (p === MOCK_WORKSPACE_SETTINGS_PATH)
+            return JSON.stringify(workspaceSettingsContent);
+          return '';
+        },
+      );
+
+      const settings = loadSettings(MOCK_WORKSPACE_DIR);
+
+      expect(settings.user.settings).toEqual(userSettingsContent);
+      expect(settings.workspace.settings).toEqual(workspaceSettingsContent);
+      expect(settings.merged.mcpServers).toEqual({
+        'user-server': {
+          command: 'user-command',
+          args: ['--user-arg'],
+          description: 'User MCP server',
+        },
+        'workspace-server': {
+          command: 'workspace-command',
+          args: ['--workspace-arg'],
+          description: 'Workspace MCP server',
+        },
+        'shared-server': {
+          command: 'workspace-shared-command',
+          description: 'Workspace shared server config',
+        },
+      });
+    });
+
+    it('should handle MCP servers when only in user settings', () => {
+      (mockFsExistsSync as Mock).mockImplementation(
+        (p: fs.PathLike) => p === USER_SETTINGS_PATH,
+      );
+      const userSettingsContent = {
+        mcpServers: {
+          'user-only-server': {
+            command: 'user-only-command',
+            description: 'User only server',
+          },
+        },
+      };
+      (fs.readFileSync as Mock).mockImplementation(
+        (p: fs.PathOrFileDescriptor) => {
+          if (p === USER_SETTINGS_PATH)
+            return JSON.stringify(userSettingsContent);
+          return '';
+        },
+      );
+
+      const settings = loadSettings(MOCK_WORKSPACE_DIR);
+      expect(settings.merged.mcpServers).toEqual({
+        'user-only-server': {
+          command: 'user-only-command',
+          description: 'User only server',
+        },
+      });
+    });
+
+    it('should handle MCP servers when only in workspace settings', () => {
+      (mockFsExistsSync as Mock).mockImplementation(
+        (p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH,
+      );
+      const workspaceSettingsContent = {
+        mcpServers: {
+          'workspace-only-server': {
+            command: 'workspace-only-command',
+            description: 'Workspace only server',
+          },
+        },
+      };
+      (fs.readFileSync as Mock).mockImplementation(
+        (p: fs.PathOrFileDescriptor) => {
+          if (p === MOCK_WORKSPACE_SETTINGS_PATH)
+            return JSON.stringify(workspaceSettingsContent);
+          return '';
+        },
+      );
+
+      const settings = loadSettings(MOCK_WORKSPACE_DIR);
+      expect(settings.merged.mcpServers).toEqual({
+        'workspace-only-server': {
+          command: 'workspace-only-command',
+          description: 'Workspace only server',
+        },
+      });
+    });
+
+    it('should have mcpServers as empty object if not in any settings file', () => {
+      (mockFsExistsSync as Mock).mockReturnValue(false); // No settings files exist
+      (fs.readFileSync as Mock).mockReturnValue('{}');
+      const settings = loadSettings(MOCK_WORKSPACE_DIR);
+      expect(settings.merged.mcpServers).toEqual({});
     });
 
     it('should handle JSON parsing errors gracefully', () => {
@@ -410,7 +557,10 @@ describe('Settings Loading and Merging', () => {
       // Check that settings are empty due to parsing errors
       expect(settings.user.settings).toEqual({});
       expect(settings.workspace.settings).toEqual({});
-      expect(settings.merged).toEqual({});
+      expect(settings.merged).toEqual({
+        customThemes: {},
+        mcpServers: {},
+      });
 
       // Check that error objects are populated in settings.errors
       expect(settings.errors).toBeDefined();
@@ -451,10 +601,13 @@ describe('Settings Loading and Merging', () => {
       );
 
       const settings = loadSettings(MOCK_WORKSPACE_DIR);
+      // @ts-expect-error: dynamic property for test
       expect(settings.user.settings.apiKey).toBe('user_api_key_from_env');
+      // @ts-expect-error: dynamic property for test
       expect(settings.user.settings.someUrl).toBe(
         'https://test.com/user_api_key_from_env',
       );
+      // @ts-expect-error: dynamic property for test
       expect(settings.merged.apiKey).toBe('user_api_key_from_env');
       delete process.env.TEST_API_KEY;
     });
@@ -483,6 +636,7 @@ describe('Settings Loading and Merging', () => {
       expect(settings.workspace.settings.nested.value).toBe(
         'workspace_endpoint_from_env',
       );
+      // @ts-expect-error: dynamic property for test
       expect(settings.merged.endpoint).toBe('workspace_endpoint_from_env/api');
       delete process.env.WORKSPACE_ENDPOINT;
     });
@@ -512,13 +666,16 @@ describe('Settings Loading and Merging', () => {
 
       const settings = loadSettings(MOCK_WORKSPACE_DIR);
 
+      // @ts-expect-error: dynamic property for test
       expect(settings.user.settings.configValue).toBe(
         'user_value_for_user_read',
       );
+      // @ts-expect-error: dynamic property for test
       expect(settings.workspace.settings.configValue).toBe(
         'workspace_value_for_workspace_read',
       );
       // Merged should take workspace's resolved value
+      // @ts-expect-error: dynamic property for test
       expect(settings.merged.configValue).toBe(
         'workspace_value_for_workspace_read',
       );
@@ -600,13 +757,16 @@ describe('Settings Loading and Merging', () => {
 
       const settings = loadSettings(MOCK_WORKSPACE_DIR);
 
+      // @ts-expect-error: dynamic property for test
       expect(settings.system.settings.configValue).toBe(
         'system_value_for_system_read',
       );
+      // @ts-expect-error: dynamic property for test
       expect(settings.workspace.settings.configValue).toBe(
         'workspace_value_for_workspace_read',
       );
-      // Merged should take workspace's resolved value
+      // Merged should take system's resolved value
+      // @ts-expect-error: dynamic property for test
       expect(settings.merged.configValue).toBe('system_value_for_system_read');
 
       // Restore original environment variable state
diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts
index 604e89dc..24b9e9e6 100644
--- a/packages/cli/src/config/settings.ts
+++ b/packages/cli/src/config/settings.ts
@@ -19,6 +19,7 @@ import {
 import stripJsonComments from 'strip-json-comments';
 import { DefaultLight } from '../ui/themes/default-light.js';
 import { DefaultDark } from '../ui/themes/default.js';
+import { CustomTheme } from '../ui/themes/theme.js';
 
 export const SETTINGS_DIRECTORY_NAME = '.gemini';
 export const USER_SETTINGS_DIR = path.join(homedir(), SETTINGS_DIRECTORY_NAME);
@@ -56,6 +57,7 @@ export interface AccessibilitySettings {
 
 export interface Settings {
   theme?: string;
+  customThemes?: Record<string, CustomTheme>;
   selectedAuthType?: AuthType;
   sandbox?: boolean | string;
   coreTools?: string[];
@@ -84,6 +86,7 @@ export interface Settings {
 
   // UI setting. Does not display the ANSI-controlled terminal title.
   hideWindowTitle?: boolean;
+
   hideTips?: boolean;
   hideBanner?: boolean;
 
@@ -132,10 +135,24 @@ export class LoadedSettings {
   }
 
   private computeMergedSettings(): Settings {
+    const system = this.system.settings;
+    const user = this.user.settings;
+    const workspace = this.workspace.settings;
+
     return {
-      ...this.user.settings,
-      ...this.workspace.settings,
-      ...this.system.settings,
+      ...user,
+      ...workspace,
+      ...system,
+      customThemes: {
+        ...(user.customThemes || {}),
+        ...(workspace.customThemes || {}),
+        ...(system.customThemes || {}),
+      },
+      mcpServers: {
+        ...(user.mcpServers || {}),
+        ...(workspace.mcpServers || {}),
+        ...(system.mcpServers || {}),
+      },
     };
   }
 
@@ -152,13 +169,12 @@ export class LoadedSettings {
     }
   }
 
-  setValue(
+  setValue<K extends keyof Settings>(
     scope: SettingScope,
-    key: keyof Settings,
-    value: string | Record<string, MCPServerConfig> | undefined,
+    key: K,
+    value: Settings[K],
   ): void {
     const settingsFile = this.forScope(scope);
-    // @ts-expect-error - value can be string | Record<string, MCPServerConfig>
     settingsFile.settings[key] = value;
     this._merged = this.computeMergedSettings();
     saveSettings(settingsFile);