diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 740142f0..14b02538 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -12,6 +12,7 @@ import { loadEnvironment, createServerConfig, loadServerHierarchicalMemory, + ConfigParameters, } from '@gemini-code/server'; import { Settings } from './settings.js'; import { readPackageUp } from 'read-package-up'; @@ -134,25 +135,26 @@ export async function loadCliConfig(settings: Settings): Promise { const apiKeyForServer = geminiApiKey || googleApiKey || ''; const useVertexAI = hasGeminiApiKey ? false : undefined; - return createServerConfig( - apiKeyForServer, - argv.model || DEFAULT_GEMINI_MODEL, - argv.sandbox ?? settings.sandbox ?? false, - process.cwd(), + const configParams: ConfigParameters = { + apiKey: apiKeyForServer, + model: argv.model || DEFAULT_GEMINI_MODEL, + sandbox: argv.sandbox ?? settings.sandbox ?? false, + targetDir: process.cwd(), debugMode, - argv.prompt || '', - argv.all_files || false, - settings.coreTools || undefined, - settings.toolDiscoveryCommand, - settings.toolCallCommand, - settings.mcpServerCommand, - settings.mcpServers, + question: argv.prompt || '', + fullContext: argv.all_files || false, + coreTools: settings.coreTools || undefined, + toolDiscoveryCommand: settings.toolDiscoveryCommand, + toolCallCommand: settings.toolCallCommand, + mcpServerCommand: settings.mcpServerCommand, + mcpServers: settings.mcpServers, userAgent, - memoryContent, - fileCount, - undefined, // alwaysSkipModificationConfirmation - not set by CLI args directly - useVertexAI, - ); + userMemory: memoryContent, + geminiMdFileCount: fileCount, + vertexai: useVertexAI, + }; + + return createServerConfig(configParams); } async function createUserAgent(): Promise { diff --git a/packages/server/src/config/config.test.ts b/packages/server/src/config/config.test.ts index 3fa8b6de..f84ad746 100644 --- a/packages/server/src/config/config.test.ts +++ b/packages/server/src/config/config.test.ts @@ -5,7 +5,7 @@ */ import { describe, it, expect, vi, beforeEach /*, afterEach */ } from 'vitest'; // afterEach removed as it was unused -import { Config, createServerConfig } from './config.js'; // Adjust import path +import { Config, createServerConfig, ConfigParameters } from './config.js'; // Adjust import path import * as path from 'path'; // import { ToolRegistry } from '../tools/tool-registry'; // ToolRegistry removed as it was unused @@ -41,6 +41,17 @@ describe('Server Config (config.ts)', () => { const FULL_CONTEXT = false; const USER_AGENT = 'ServerTestAgent/1.0'; const USER_MEMORY = 'Test User Memory'; + const baseParams: ConfigParameters = { + apiKey: API_KEY, + model: MODEL, + sandbox: SANDBOX, + targetDir: TARGET_DIR, + debugMode: DEBUG_MODE, + question: QUESTION, + fullContext: FULL_CONTEXT, + userAgent: USER_AGENT, + userMemory: USER_MEMORY, + }; beforeEach(() => { // Reset mocks if necessary @@ -48,22 +59,7 @@ describe('Server Config (config.ts)', () => { }); it('Config constructor should store userMemory correctly', () => { - const config = new Config( - API_KEY, - MODEL, - SANDBOX, - TARGET_DIR, - DEBUG_MODE, - QUESTION, - FULL_CONTEXT, - undefined, // coreTools - undefined, // toolDiscoveryCommand - undefined, // toolCallCommand - undefined, // mcpServerCommand - undefined, // mcpServers - USER_AGENT, - USER_MEMORY, // Pass memory here - ); + const config = new Config(baseParams); expect(config.getUserMemory()).toBe(USER_MEMORY); // Verify other getters if needed @@ -74,43 +70,15 @@ describe('Server Config (config.ts)', () => { }); it('Config constructor should default userMemory to empty string if not provided', () => { - const config = new Config( - API_KEY, - MODEL, - SANDBOX, - TARGET_DIR, - DEBUG_MODE, - QUESTION, - FULL_CONTEXT, - undefined, - undefined, - undefined, - undefined, - undefined, - USER_AGENT, - // No userMemory argument - ); + const paramsWithoutMemory: ConfigParameters = { ...baseParams }; + delete paramsWithoutMemory.userMemory; + const config = new Config(paramsWithoutMemory); expect(config.getUserMemory()).toBe(''); }); it('createServerConfig should pass userMemory to Config constructor', () => { - const config = createServerConfig( - API_KEY, - MODEL, - SANDBOX, - TARGET_DIR, - DEBUG_MODE, - QUESTION, - FULL_CONTEXT, - undefined, // coreTools - undefined, // toolDiscoveryCommand - undefined, // toolCallCommand - undefined, // mcpServerCommand - undefined, // mcpServers - USER_AGENT, - USER_MEMORY, // Pass memory here - ); + const config = createServerConfig(baseParams); // Check the result of the factory function expect(config).toBeInstanceOf(Config); @@ -120,22 +88,9 @@ describe('Server Config (config.ts)', () => { }); it('createServerConfig should default userMemory if omitted', () => { - const config = createServerConfig( - API_KEY, - MODEL, - SANDBOX, - TARGET_DIR, - DEBUG_MODE, - QUESTION, - FULL_CONTEXT, - undefined, // coreTools - undefined, // toolDiscoveryCommand - undefined, // toolCallCommand - undefined, // mcpServerCommand - undefined, // mcpServers - USER_AGENT, - // No userMemory argument - ); + const paramsWithoutMemory: ConfigParameters = { ...baseParams }; + delete paramsWithoutMemory.userMemory; + const config = createServerConfig(paramsWithoutMemory); expect(config).toBeInstanceOf(Config); expect(config.getUserMemory()).toBe(''); // Should default to empty string @@ -144,22 +99,11 @@ describe('Server Config (config.ts)', () => { it('createServerConfig should resolve targetDir', () => { const relativeDir = './relative/path'; const expectedResolvedDir = path.resolve(relativeDir); - const config = createServerConfig( - API_KEY, - MODEL, - SANDBOX, - relativeDir, - DEBUG_MODE, - QUESTION, - FULL_CONTEXT, - undefined, // coreTools - undefined, // toolDiscoveryCommand - undefined, // toolCallCommand - undefined, // mcpServerCommand - undefined, // mcpServers - USER_AGENT, - USER_MEMORY, - ); + const paramsWithRelativeDir: ConfigParameters = { + ...baseParams, + targetDir: relativeDir, + }; + const config = createServerConfig(paramsWithRelativeDir); expect(config.getTargetDir()).toBe(expectedResolvedDir); }); }); diff --git a/packages/server/src/config/config.ts b/packages/server/src/config/config.ts index ff35c16f..acdbf219 100644 --- a/packages/server/src/config/config.ts +++ b/packages/server/src/config/config.ts @@ -36,29 +36,66 @@ export class MCPServerConfig { ) {} } +export interface ConfigParameters { + apiKey: string; + model: string; + sandbox: boolean | string; + targetDir: string; + debugMode: boolean; + question?: string; + fullContext?: boolean; + coreTools?: string[]; + toolDiscoveryCommand?: string; + toolCallCommand?: string; + mcpServerCommand?: string; + mcpServers?: Record; + userAgent: string; + userMemory?: string; + geminiMdFileCount?: number; + alwaysSkipModificationConfirmation?: boolean; + vertexai?: boolean; +} + export class Config { private toolRegistry: ToolRegistry; + private readonly apiKey: string; + private readonly model: string; + private readonly sandbox: boolean | string; + private readonly targetDir: string; + private readonly debugMode: boolean; + private readonly question: string | undefined; + private readonly fullContext: boolean; + private readonly coreTools: string[] | undefined; + private readonly toolDiscoveryCommand: string | undefined; + private readonly toolCallCommand: string | undefined; + private readonly mcpServerCommand: string | undefined; + private readonly mcpServers: Record | undefined; + private readonly userAgent: string; + private userMemory: string; + private geminiMdFileCount: number; + private alwaysSkipModificationConfirmation: boolean; + private readonly vertexai: boolean | undefined; + + constructor(params: ConfigParameters) { + this.apiKey = params.apiKey; + this.model = params.model; + this.sandbox = params.sandbox; + this.targetDir = path.resolve(params.targetDir); + this.debugMode = params.debugMode; + this.question = params.question; + this.fullContext = params.fullContext ?? false; + this.coreTools = params.coreTools; + this.toolDiscoveryCommand = params.toolDiscoveryCommand; + this.toolCallCommand = params.toolCallCommand; + this.mcpServerCommand = params.mcpServerCommand; + this.mcpServers = params.mcpServers; + this.userAgent = params.userAgent; + this.userMemory = params.userMemory ?? ''; + this.geminiMdFileCount = params.geminiMdFileCount ?? 0; + this.alwaysSkipModificationConfirmation = + params.alwaysSkipModificationConfirmation ?? false; + this.vertexai = params.vertexai; - constructor( - private readonly apiKey: string, - private readonly model: string, - private readonly sandbox: boolean | string, - private readonly targetDir: string, - private readonly debugMode: boolean, - private readonly question: string | undefined, // Keep undefined possibility - private readonly fullContext: boolean = false, // Default value here - private readonly coreTools: string[] | undefined, - private readonly toolDiscoveryCommand: string | undefined, - private readonly toolCallCommand: string | undefined, - private readonly mcpServerCommand: string | undefined, - private readonly mcpServers: Record | undefined, - private readonly userAgent: string, - private userMemory: string = '', // Made mutable for refresh - private geminiMdFileCount: number = 0, - private alwaysSkipModificationConfirmation: boolean = false, - private readonly vertexai?: boolean, - ) { - // toolRegistry still needs initialization based on the instance this.toolRegistry = createToolRegistry(this); } @@ -174,44 +211,12 @@ export function loadEnvironment(): void { dotenv.config({ path: envFilePath }); } -export function createServerConfig( - apiKey: string, - model: string, - sandbox: boolean | string, - targetDir: string, - debugMode: boolean, - question: string, - fullContext?: boolean, - coreTools?: string[], - toolDiscoveryCommand?: string, - toolCallCommand?: string, - mcpServerCommand?: string, - mcpServers?: Record, - userAgent?: string, - userMemory?: string, - geminiMdFileCount?: number, - alwaysSkipModificationConfirmation?: boolean, - vertexai?: boolean, -): Config { - return new Config( - apiKey, - model, - sandbox, - path.resolve(targetDir), - debugMode, - question, - fullContext, - coreTools, - toolDiscoveryCommand, - toolCallCommand, - mcpServerCommand, - mcpServers, - userAgent ?? 'GeminiCLI/unknown', // Default user agent - userMemory ?? '', - geminiMdFileCount ?? 0, - alwaysSkipModificationConfirmation ?? false, - vertexai, - ); +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): ToolRegistry { diff --git a/packages/server/src/tools/tool-registry.test.ts b/packages/server/src/tools/tool-registry.test.ts index bb41b35c..6a960a27 100644 --- a/packages/server/src/tools/tool-registry.test.ts +++ b/packages/server/src/tools/tool-registry.test.ts @@ -16,7 +16,7 @@ import { } from 'vitest'; import { ToolRegistry, DiscoveredTool } from './tool-registry.js'; import { DiscoveredMCPTool } from './mcp-tool.js'; -import { Config } from '../config/config.js'; +import { Config, ConfigParameters } from '../config/config.js'; import { BaseTool, ToolResult } from './tools.js'; import { FunctionDeclaration } from '@google/genai'; import { execSync, spawn } from 'node:child_process'; // Import spawn here @@ -69,28 +69,32 @@ class MockTool extends BaseTool<{ param: string }, ToolResult> { } } +const baseConfigParams: ConfigParameters = { + apiKey: 'test-api-key', + model: 'test-model', + sandbox: false, + targetDir: '/test/dir', + debugMode: false, + question: undefined, + fullContext: false, + coreTools: undefined, + toolDiscoveryCommand: undefined, + toolCallCommand: undefined, + mcpServerCommand: undefined, + mcpServers: undefined, + userAgent: 'TestAgent/1.0', + userMemory: '', + geminiMdFileCount: 0, + alwaysSkipModificationConfirmation: false, + vertexai: false, +}; + describe('ToolRegistry', () => { let config: Config; let toolRegistry: ToolRegistry; beforeEach(() => { - // Provide a mock target directory for Config initialization - const mockTargetDir = '/test/dir'; - config = new Config( - 'test-api-key', - 'test-model', - false, // sandbox - mockTargetDir, // targetDir - false, // debugMode - undefined, // question - false, // fullContext - undefined, // coreTools - undefined, // toolDiscoveryCommand - undefined, // toolCallCommand - undefined, // mcpServerCommand - undefined, // mcpServers - 'TestAgent/1.0', // userAgent - ); + config = new Config(baseConfigParams); // Use base params toolRegistry = new ToolRegistry(config); vi.spyOn(console, 'warn').mockImplementation(() => {}); // Suppress console.warn }); @@ -208,27 +212,15 @@ describe('ToolRegistry', () => { const availableCoreToolClasses = [MockCoreToolAlpha, MockCoreToolBeta]; let currentConfig: Config; let currentToolRegistry: ToolRegistry; - const mockTargetDir = '/test/dir'; // As used in outer scope // Helper to set up Config, ToolRegistry, and simulate core tool registration const setupRegistryAndSimulateRegistration = ( coreToolsValueInConfig: string[] | undefined, ) => { - currentConfig = new Config( - 'test-api-key', - 'test-model', - false, // sandbox - mockTargetDir, // targetDir - false, // debugMode - undefined, // question - false, // fullContext - coreToolsValueInConfig, // coreTools setting being tested - undefined, // toolDiscoveryCommand - undefined, // toolCallCommand - undefined, // mcpServerCommand - undefined, // mcpServers - 'TestAgent/1.0', // userAgent - ); + currentConfig = new Config({ + ...baseConfigParams, // Use base and override coreTools + coreTools: coreToolsValueInConfig, + }); // We assume Config has a getter like getCoreTools() or stores it publicly. // For this test, we'll directly use coreToolsValueInConfig for the simulation logic, @@ -560,22 +552,7 @@ describe('DiscoveredTool', () => { let mockSpawnInstance: Partial>; beforeEach(() => { - const mockTargetDir = '/test/dir'; - config = new Config( - 'test-api-key', - 'test-model', - false, // sandbox - mockTargetDir, // targetDir - false, // debugMode - undefined, // question - false, // fullContext - undefined, // coreTools - undefined, // toolDiscoveryCommand - undefined, // toolCallCommand - undefined, // mcpServerCommand - undefined, // mcpServers - 'TestAgent/1.0', // userAgent - ); + config = new Config(baseConfigParams); // Use base params vi.spyOn(config, 'getToolDiscoveryCommand').mockReturnValue( 'discovery-cmd', );