Change Config to use named parameters. (#593)

2026-07-21 23:40:44 -07:00 · 2025-05-29 20:51:17 +00:00
parent a5029d5dca
commit 91ea43109a
4 changed files with 134 additions and 206 deletions
@@ -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);
  });
 });
@@ -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<string, MCPServerConfig>;
+  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<string, MCPServerConfig> | 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<string, MCPServerConfig> | 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<string, MCPServerConfig>,
-  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 {