From 45faf4d31b10792582cdce5ca693fc5a548d1edc Mon Sep 17 00:00:00 2001 From: Jack Wotherspoon Date: Thu, 12 Mar 2026 14:38:09 +0100 Subject: [PATCH] fix: register themes on extension load not start (#22148) --- .../cli/src/config/extension-manager.test.ts | 65 ++++++++++++++++++- packages/cli/src/config/extension-manager.ts | 9 ++- packages/cli/src/ui/themes/theme-manager.ts | 11 ++++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/config/extension-manager.test.ts b/packages/cli/src/config/extension-manager.test.ts index 5b44c07194..13c1de15fa 100644 --- a/packages/cli/src/config/extension-manager.test.ts +++ b/packages/cli/src/config/extension-manager.test.ts @@ -12,12 +12,13 @@ import { ExtensionManager } from './extension-manager.js'; import { createTestMergedSettings, type MergedSettings } from './settings.js'; import { createExtension } from '../test-utils/createExtension.js'; import { EXTENSIONS_DIRECTORY_NAME } from './extensions/variables.js'; +import { themeManager } from '../ui/themes/theme-manager.js'; import { TrustLevel, loadTrustedFolders, isWorkspaceTrusted, } from './trustedFolders.js'; -import { getRealPath } from '@google/gemini-cli-core'; +import { getRealPath, type CustomTheme } from '@google/gemini-cli-core'; const mockHomedir = vi.hoisted(() => vi.fn(() => '/tmp/mock-home')); @@ -38,6 +39,26 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => { }; }); +const testTheme: CustomTheme = { + type: 'custom', + name: 'MyTheme', + background: { + primary: '#282828', + diff: { added: '#2b3312', removed: '#341212' }, + }, + text: { + primary: '#ebdbb2', + secondary: '#a89984', + link: '#83a598', + accent: '#d3869b', + }, + status: { + success: '#b8bb26', + warning: '#fabd2f', + error: '#fb4934', + }, +}; + describe('ExtensionManager', () => { let tempHomeDir: string; let tempWorkspaceDir: string; @@ -65,6 +86,7 @@ describe('ExtensionManager', () => { }); afterEach(() => { + themeManager.clearExtensionThemes(); try { fs.rmSync(tempHomeDir, { recursive: true, force: true }); } catch (_e) { @@ -484,4 +506,45 @@ describe('ExtensionManager', () => { ).rejects.toThrow(/already installed/); }); }); + + describe('early theme registration', () => { + it('should register themes with ThemeManager during loadExtensions for active extensions', async () => { + createExtension({ + extensionsDir: userExtensionsDir, + name: 'themed-ext', + version: '1.0.0', + themes: [testTheme], + }); + + await extensionManager.loadExtensions(); + + expect(themeManager.getCustomThemeNames()).toContain( + 'MyTheme (themed-ext)', + ); + }); + + it('should not register themes for inactive extensions', async () => { + createExtension({ + extensionsDir: userExtensionsDir, + name: 'disabled-ext', + version: '1.0.0', + themes: [testTheme], + }); + + // Disable the extension by creating an enablement override + const manager = new ExtensionManager({ + enabledExtensionOverrides: ['none'], + settings: createTestMergedSettings(), + workspaceDir: tempWorkspaceDir, + requestConsent: vi.fn().mockResolvedValue(true), + requestSetting: null, + }); + + await manager.loadExtensions(); + + expect(themeManager.getCustomThemeNames()).not.toContain( + 'MyTheme (disabled-ext)', + ); + }); + }); }); diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts index 80c48193e2..68617bcbcd 100644 --- a/packages/cli/src/config/extension-manager.ts +++ b/packages/cli/src/config/extension-manager.ts @@ -564,7 +564,7 @@ Would you like to attempt to install via "git clone" instead?`, protected override async startExtension(extension: GeminiCLIExtension) { await super.startExtension(extension); - if (extension.themes) { + if (extension.themes && !themeManager.hasExtensionThemes(extension.name)) { themeManager.registerExtensionThemes(extension.name, extension.themes); } } @@ -624,6 +624,13 @@ Would you like to attempt to install via "git clone" instead?`, this.loadedExtensions = builtExtensions; + // Register extension themes early so they're available at startup. + for (const ext of this.loadedExtensions) { + if (ext.isActive && ext.themes) { + themeManager.registerExtensionThemes(ext.name, ext.themes); + } + } + await Promise.all( this.loadedExtensions.map((ext) => this.maybeStartExtension(ext)), ); diff --git a/packages/cli/src/ui/themes/theme-manager.ts b/packages/cli/src/ui/themes/theme-manager.ts index 00fed5ce20..66826bb87e 100644 --- a/packages/cli/src/ui/themes/theme-manager.ts +++ b/packages/cli/src/ui/themes/theme-manager.ts @@ -240,6 +240,17 @@ class ThemeManager { } } + /** + * Checks if themes for a given extension are already registered. + * @param extensionName The name of the extension. + * @returns True if any themes from the extension are registered. + */ + hasExtensionThemes(extensionName: string): boolean { + return Array.from(this.extensionThemes.keys()).some((name) => + name.endsWith(`(${extensionName})`), + ); + } + /** * Clears all registered extension themes. * This is primarily for testing purposes to reset state between tests.