From d6cf4d5b0b9aa5ab074c4d84e2aae1a2f36b529b Mon Sep 17 00:00:00 2001 From: Tommaso Sciortino Date: Sat, 7 Jun 2025 13:49:00 -0700 Subject: [PATCH] Eliminate createServerConfig() (#821) --- packages/cli/src/config/config.test.ts | 2 +- packages/cli/src/config/config.ts | 8 +-- packages/core/src/config/config.test.ts | 72 +------------------------ packages/core/src/config/config.ts | 10 +--- 4 files changed, 5 insertions(+), 87 deletions(-) diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 802cf588..fffc5883 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -34,7 +34,7 @@ vi.mock('@gemini-code/core', async () => { return { ...actualServer, loadEnvironment: vi.fn(), - createServerConfig: vi.fn((params) => ({ + Config: vi.fn((params) => ({ // Mock the config object and its methods getApiKey: () => params.apiKey, getModel: () => params.model, diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 6ab1453f..3a932090 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -10,9 +10,7 @@ import process from 'node:process'; import { Config, loadEnvironment, - createServerConfig, loadServerHierarchicalMemory, - ConfigParameters, setGeminiMdFilename as setServerGeminiMdFilename, getCurrentGeminiMdFilename, ApprovalMode, @@ -175,7 +173,7 @@ export async function loadCliConfig( modelToUse = await getEffectiveModel(apiKeyForServer, modelToUse); } - const configParams: ConfigParameters = { + return new Config({ apiKey: apiKeyForServer, model: modelToUse, embeddingModel: DEFAULT_GEMINI_EMBEDDING_MODEL, @@ -206,7 +204,5 @@ export async function loadCliConfig( fileFilteringRespectGitIgnore: settings.fileFiltering?.respectGitIgnore, fileFilteringAllowBuildArtifacts: settings.fileFiltering?.allowBuildArtifacts, - }; - - return createServerConfig(configParams); + }); } diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 3800585d..0ce8f5ee 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -5,7 +5,7 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { Config, createServerConfig, ConfigParameters } from './config.js'; +import { Config, ConfigParameters } from './config.js'; import * as path from 'path'; import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../tools/memoryTool.js'; @@ -87,51 +87,6 @@ describe('Server Config (config.ts)', () => { expect(config.getUserMemory()).toBe(''); }); - it('createServerConfig should pass userMemory to Config constructor', () => { - const config = createServerConfig(baseParams); - - // Check the result of the factory function - expect(config).toBeInstanceOf(Config); - expect(config.getUserMemory()).toBe(USER_MEMORY); - expect(config.getApiKey()).toBe(API_KEY); - expect(config.getUserAgent()).toBe(USER_AGENT); - }); - - it('createServerConfig should default userMemory if omitted', () => { - const paramsWithoutMemory: ConfigParameters = { ...baseParams }; - delete paramsWithoutMemory.userMemory; - const config = createServerConfig(paramsWithoutMemory); - - expect(config).toBeInstanceOf(Config); - expect(config.getUserMemory()).toBe(''); // Should default to empty string - }); - - it('createServerConfig should resolve targetDir', () => { - const relativeDir = './relative/path'; - const expectedResolvedDir = path.resolve(relativeDir); - const paramsWithRelativeDir: ConfigParameters = { - ...baseParams, - targetDir: relativeDir, - }; - const config = createServerConfig(paramsWithRelativeDir); - expect(config.getTargetDir()).toBe(expectedResolvedDir); - }); - - it('createServerConfig should call setGeminiMdFilename with contextFileName if provided', () => { - const contextFileName = 'CUSTOM_AGENTS.md'; - const paramsWithContextFile: ConfigParameters = { - ...baseParams, - contextFileName, - }; - createServerConfig(paramsWithContextFile); - expect(mockSetGeminiMdFilename).toHaveBeenCalledWith(contextFileName); - }); - - it('createServerConfig should not call setGeminiMdFilename if contextFileName is not provided', () => { - createServerConfig(baseParams); // baseParams does not have contextFileName - expect(mockSetGeminiMdFilename).not.toHaveBeenCalled(); - }); - it('Config constructor should call setGeminiMdFilename with contextFileName if provided', () => { const contextFileName = 'CUSTOM_AGENTS.md'; const paramsWithContextFile: ConfigParameters = { @@ -189,31 +144,6 @@ describe('Server Config (config.ts)', () => { expect(config.getTelemetryEnabled()).toBe(TELEMETRY); }); - it('createServerConfig should pass telemetry to Config constructor when true', () => { - const paramsWithTelemetry: ConfigParameters = { - ...baseParams, - telemetry: true, - }; - const config = createServerConfig(paramsWithTelemetry); - expect(config.getTelemetryEnabled()).toBe(true); - }); - - it('createServerConfig should pass telemetry to Config constructor when false', () => { - const paramsWithTelemetry: ConfigParameters = { - ...baseParams, - telemetry: false, - }; - const config = createServerConfig(paramsWithTelemetry); - expect(config.getTelemetryEnabled()).toBe(false); - }); - - it('createServerConfig should default telemetry (to false via Config constructor) if omitted', () => { - const paramsWithoutTelemetry: ConfigParameters = { ...baseParams }; - delete paramsWithoutTelemetry.telemetry; - const config = createServerConfig(paramsWithoutTelemetry); - expect(config.getTelemetryEnabled()).toBe(TELEMETRY); - }); - it('should have a getFileService method that returns FileDiscoveryService', async () => { const config = new Config(baseParams); const fileService = await config.getFileService(); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 75db970b..3eb6ecf1 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -126,7 +126,7 @@ export class Config { this.toolCallCommand = params.toolCallCommand; this.mcpServerCommand = params.mcpServerCommand; this.mcpServers = params.mcpServers; - this.userAgent = params.userAgent; + this.userAgent = params.userAgent ?? 'GeminiCLI/unknown'; this.userMemory = params.userMemory ?? ''; this.geminiMdFileCount = params.geminiMdFileCount ?? 0; this.approvalMode = params.approvalMode ?? ApprovalMode.DEFAULT; @@ -329,14 +329,6 @@ export function loadEnvironment(): void { } } -export function createServerConfig(params: ConfigParameters): Config { - return new Config({ - ...params, - targetDir: path.resolve(params.targetDir), // Ensure targetDir is resolved - userAgent: params.userAgent ?? 'GeminiCLI/unknown', // Default user agent - }); -} - export function createToolRegistry(config: Config): Promise { const registry = new ToolRegistry(config); const targetDir = config.getTargetDir();