Introduce ContentGeneratorConfig (#826)

2025-12-20 16:57:46 +00:00 · 2025-06-07 16:17:27 -07:00
parent e95a6086fc
commit 389907ce65
12 changed files with 156 additions and 141 deletions
--- a/packages/cli/src/config/config.integration.test.ts
+++ b/packages/cli/src/config/config.integration.test.ts
@@ -8,7 +8,17 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';
 import * as fs from 'fs';
 import * as path from 'path';
 import { tmpdir } from 'os';
-import { Config, ConfigParameters } from '@gemini-cli/core';
+import {
+  Config,
+  ConfigParameters,
+  ContentGeneratorConfig,
+} from '@gemini-cli/core';
+
+const TEST_CONTENT_GENERATOR_CONFIG: ContentGeneratorConfig = {
+  apiKey: 'test-key',
+  model: 'test-model',
+  userAgent: 'test-agent',
+};

 // Mock file discovery service and tool registry
 vi.mock('@gemini-cli/core', async () => {
@@ -43,12 +53,11 @@ describe('Configuration Integration Tests', () => {
  describe('File Filtering Configuration', () => {
    it('should load default file filtering settings', async () => {
      const configParams: ConfigParameters = {
-        apiKey: 'test-key',
-        model: 'test-model',
+        contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
+        embeddingModel: 'test-embedding-model',
        sandbox: false,
        targetDir: tempDir,
        debugMode: false,
-        userAgent: 'test-agent',
        fileFilteringRespectGitIgnore: undefined, // Should default to true
        fileFilteringAllowBuildArtifacts: undefined, // Should default to false
      };
@@ -61,12 +70,11 @@ describe('Configuration Integration Tests', () => {

    it('should load custom file filtering settings from configuration', async () => {
      const configParams: ConfigParameters = {
-        apiKey: 'test-key',
-        model: 'test-model',
+        contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
+        embeddingModel: 'test-embedding-model',
        sandbox: false,
        targetDir: tempDir,
        debugMode: false,
-        userAgent: 'test-agent',
        fileFilteringRespectGitIgnore: false,
        fileFilteringAllowBuildArtifacts: true,
      };
@@ -79,12 +87,11 @@ describe('Configuration Integration Tests', () => {

    it('should merge user and workspace file filtering settings', async () => {
      const configParams: ConfigParameters = {
-        apiKey: 'test-key',
-        model: 'test-model',
+        contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
+        embeddingModel: 'test-embedding-model',
        sandbox: false,
        targetDir: tempDir,
        debugMode: false,
-        userAgent: 'test-agent',
        fileFilteringRespectGitIgnore: true,
        fileFilteringAllowBuildArtifacts: true,
      };
@@ -99,12 +106,11 @@ describe('Configuration Integration Tests', () => {
  describe('Configuration Integration', () => {
    it('should handle partial configuration objects gracefully', async () => {
      const configParams: ConfigParameters = {
-        apiKey: 'test-key',
-        model: 'test-model',
+        contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
+        embeddingModel: 'test-embedding-model',
        sandbox: false,
        targetDir: tempDir,
        debugMode: false,
-        userAgent: 'test-agent',
        fileFilteringRespectGitIgnore: false,
        fileFilteringAllowBuildArtifacts: undefined, // Should default to false
      };
@@ -120,12 +126,11 @@ describe('Configuration Integration Tests', () => {

    it('should handle empty configuration objects gracefully', async () => {
      const configParams: ConfigParameters = {
-        apiKey: 'test-key',
-        model: 'test-model',
+        contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
+        embeddingModel: 'test-embedding-model',
        sandbox: false,
        targetDir: tempDir,
        debugMode: false,
-        userAgent: 'test-agent',
        fileFilteringRespectGitIgnore: undefined,
        fileFilteringAllowBuildArtifacts: undefined,
      };
@@ -139,12 +144,11 @@ describe('Configuration Integration Tests', () => {

    it('should handle missing configuration sections gracefully', async () => {
      const configParams: ConfigParameters = {
-        apiKey: 'test-key',
-        model: 'test-model',
+        contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
+        embeddingModel: 'test-embedding-model',
        sandbox: false,
        targetDir: tempDir,
        debugMode: false,
-        userAgent: 'test-agent',
        // Missing fileFiltering configuration
      };

@@ -159,12 +163,11 @@ describe('Configuration Integration Tests', () => {
  describe('Real-world Configuration Scenarios', () => {
    it('should handle a security-focused configuration', async () => {
      const configParams: ConfigParameters = {
-        apiKey: 'test-key',
-        model: 'test-model',
+        contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
+        embeddingModel: 'test-embedding-model',
        sandbox: false,
        targetDir: tempDir,
        debugMode: false,
-        userAgent: 'test-agent',
        fileFilteringRespectGitIgnore: true,
        fileFilteringAllowBuildArtifacts: false,
      };
@@ -177,12 +180,11 @@ describe('Configuration Integration Tests', () => {

    it('should handle a development-focused configuration', async () => {
      const configParams: ConfigParameters = {
-        apiKey: 'test-key',
-        model: 'test-model',
+        contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
+        embeddingModel: 'test-embedding-model',
        sandbox: false,
        targetDir: tempDir,
        debugMode: false,
-        userAgent: 'test-agent',
        fileFilteringRespectGitIgnore: true,
        fileFilteringAllowBuildArtifacts: true,
      };
@@ -194,12 +196,11 @@ describe('Configuration Integration Tests', () => {

    it('should handle a CI/CD environment configuration', async () => {
      const configParams: ConfigParameters = {
-        apiKey: 'test-key',
-        model: 'test-model',
+        contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
+        embeddingModel: 'test-embedding-model',
        sandbox: false,
        targetDir: tempDir,
        debugMode: false,
-        userAgent: 'test-agent',
        fileFilteringRespectGitIgnore: false, // CI might need to see all files
        fileFilteringAllowBuildArtifacts: true,
      };
--- a/packages/cli/src/config/config.ts
+++ b/packages/cli/src/config/config.ts
@@ -14,6 +14,7 @@ import {
  setGeminiMdFilename as setServerGeminiMdFilename,
  getCurrentGeminiMdFilename,
  ApprovalMode,
+  ContentGeneratorConfig,
 } from '@gemini-cli/core';
 import { Settings } from './settings.js';
 import { getEffectiveModel } from '../utils/modelCheck.js';
@@ -121,6 +122,62 @@ export async function loadCliConfig(
 ): Promise<Config> {
  loadEnvironment();

+  const argv = await parseArguments();
+  const debugMode = argv.debug || false;
+
+  // Set the context filename in the server's memoryTool module BEFORE loading memory
+  // TODO(b/343434939): This is a bit of a hack. The contextFileName should ideally be passed
+  // directly to the Config constructor in core, and have core handle setGeminiMdFilename.
+  // However, loadHierarchicalGeminiMemory is called *before* createServerConfig.
+  if (settings.contextFileName) {
+    setServerGeminiMdFilename(settings.contextFileName);
+  } else {
+    // Reset to default if not provided in settings.
+    setServerGeminiMdFilename(getCurrentGeminiMdFilename());
+  }
+
+  // Call the (now wrapper) loadHierarchicalGeminiMemory which calls the server's version
+  const { memoryContent, fileCount } = await loadHierarchicalGeminiMemory(
+    process.cwd(),
+    debugMode,
+  );
+
+  const contentGeneratorConfig = await createContentGeneratorConfig(argv);
+
+  return new Config({
+    contentGeneratorConfig,
+    embeddingModel: DEFAULT_GEMINI_EMBEDDING_MODEL,
+    sandbox: argv.sandbox ?? settings.sandbox ?? argv.yolo ?? false,
+    targetDir: process.cwd(),
+    debugMode,
+    question: argv.prompt || '',
+    fullContext: argv.all_files || false,
+    coreTools: settings.coreTools || undefined,
+    toolDiscoveryCommand: settings.toolDiscoveryCommand,
+    toolCallCommand: settings.toolCallCommand,
+    mcpServerCommand: settings.mcpServerCommand,
+    mcpServers: settings.mcpServers,
+    userMemory: memoryContent,
+    geminiMdFileCount: fileCount,
+    approvalMode: argv.yolo || false ? ApprovalMode.YOLO : ApprovalMode.DEFAULT,
+    showMemoryUsage:
+      argv.show_memory_usage || settings.showMemoryUsage || false,
+    geminiIgnorePatterns,
+    accessibility: settings.accessibility,
+    telemetry:
+      argv.telemetry !== undefined
+        ? argv.telemetry
+        : (settings.telemetry ?? false),
+    // Git-aware file filtering settings
+    fileFilteringRespectGitIgnore: settings.fileFiltering?.respectGitIgnore,
+    fileFilteringAllowBuildArtifacts:
+      settings.fileFiltering?.allowBuildArtifacts,
+  });
+}
+
+async function createContentGeneratorConfig(
+  argv: CliArgs,
+): Promise<ContentGeneratorConfig> {
  const geminiApiKey = process.env.GEMINI_API_KEY;
  const googleApiKey = process.env.GOOGLE_API_KEY;
  const googleCloudProject = process.env.GOOGLE_CLOUD_PROJECT;
@@ -144,65 +201,16 @@ export async function loadCliConfig(
    process.exit(1);
  }

-  const argv = await parseArguments();
-  const debugMode = argv.debug || false;
+  const config: ContentGeneratorConfig = {
+    model: argv.model || DEFAULT_GEMINI_MODEL,
+    apiKey: geminiApiKey || googleApiKey || '',
+    vertexai: hasGeminiApiKey ? false : undefined,
+    userAgent: `GeminiCLI/${getCliVersion()}/(${process.platform}; ${process.arch})`,
+  };

-  // Set the context filename in the server's memoryTool module BEFORE loading memory
-  // TODO(b/343434939): This is a bit of a hack. The contextFileName should ideally be passed
-  // directly to the Config constructor in core, and have core handle setGeminiMdFilename.
-  // However, loadHierarchicalGeminiMemory is called *before* createServerConfig.
-  if (settings.contextFileName) {
-    setServerGeminiMdFilename(settings.contextFileName);
-  } else {
-    // Reset to default if not provided in settings.
-    setServerGeminiMdFilename(getCurrentGeminiMdFilename());
+  if (config.apiKey) {
+    config.model = await getEffectiveModel(config.apiKey, config.model);
  }

-  // Call the (now wrapper) loadHierarchicalGeminiMemory which calls the server's version
-  const { memoryContent, fileCount } = await loadHierarchicalGeminiMemory(
-    process.cwd(),
-    debugMode,
-  );
-
-  const userAgent = `GeminiCLI/${getCliVersion()}/(${process.platform}; ${process.arch})`;
-  const apiKeyForServer = geminiApiKey || googleApiKey || '';
-  const useVertexAI = hasGeminiApiKey ? false : undefined;
-
-  let modelToUse = argv.model || DEFAULT_GEMINI_MODEL;
-  if (apiKeyForServer) {
-    modelToUse = await getEffectiveModel(apiKeyForServer, modelToUse);
-  }
-
-  return new Config({
-    apiKey: apiKeyForServer,
-    model: modelToUse,
-    embeddingModel: DEFAULT_GEMINI_EMBEDDING_MODEL,
-    sandbox: argv.sandbox ?? settings.sandbox ?? argv.yolo ?? false,
-    targetDir: process.cwd(),
-    debugMode,
-    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,
-    userMemory: memoryContent,
-    geminiMdFileCount: fileCount,
-    approvalMode: argv.yolo || false ? ApprovalMode.YOLO : ApprovalMode.DEFAULT,
-    vertexai: useVertexAI,
-    showMemoryUsage:
-      argv.show_memory_usage || settings.showMemoryUsage || false,
-    geminiIgnorePatterns,
-    accessibility: settings.accessibility,
-    telemetry:
-      argv.telemetry !== undefined
-        ? argv.telemetry
-        : (settings.telemetry ?? false),
-    // Git-aware file filtering settings
-    fileFilteringRespectGitIgnore: settings.fileFiltering?.respectGitIgnore,
-    fileFilteringAllowBuildArtifacts:
-      settings.fileFiltering?.allowBuildArtifacts,
-  });
+  return config;
 }
--- a/packages/cli/src/ui/App.test.tsx
+++ b/packages/cli/src/ui/App.test.tsx
@@ -179,13 +179,15 @@ describe('App UI', () => {
  beforeEach(() => {
    const ServerConfigMocked = vi.mocked(ServerConfig, true);
    mockConfig = new ServerConfigMocked({
-      apiKey: 'test-key',
-      model: 'test-model',
+      contentGeneratorConfig: {
+        apiKey: 'test-key',
+        model: 'test-model',
+        userAgent: 'test-agent',
+      },
      embeddingModel: 'test-embedding-model',
      sandbox: false,
      targetDir: '/test/dir',
      debugMode: false,
-      userAgent: 'test-agent',
      userMemory: '',
      geminiMdFileCount: 0,
      showMemoryUsage: false,