Change Config to use named parameters. (#593)
This commit is contained in:
parent
d74c0f581b
commit
6a1b94529b
|
@ -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<Config> {
|
|||
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<string> {
|
||||
|
|
|
@ -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 {
|
||||
|
|
|
@ -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<ReturnType<typeof spawn>>;
|
||||
|
||||
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',
|
||||
);
|
||||
|
|
Loading…
Reference in New Issue