Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions packages/cli/src/config/config.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import type {
ConfigParameters,
ContentGeneratorConfig,
} from '@google/gemini-cli-core';
import { Config } from '@google/gemini-cli-core';
import {
Config,
DEFAULT_FILE_FILTERING_OPTIONS,
} from '@google/gemini-cli-core';
import { http, HttpResponse } from 'msw';
import { setupServer } from 'msw/node';

Expand Down Expand Up @@ -78,12 +81,14 @@ describe('Configuration Integration Tests', () => {
sandbox: false,
targetDir: tempDir,
debugMode: false,
fileFilteringRespectGitIgnore: undefined, // Should default to true
fileFilteringRespectGitIgnore: undefined, // Should default to DEFAULT_FILE_FILTERING_OPTIONS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nit: DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes good catch!

};

const config = new Config(configParams);

expect(config.getFileFilteringRespectGitIgnore()).toBe(true);
expect(config.getFileFilteringRespectGitIgnore()).toBe(
DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
);
});

it('should load custom file filtering settings from configuration', async () => {
Expand Down Expand Up @@ -112,7 +117,9 @@ describe('Configuration Integration Tests', () => {
sandbox: false,
targetDir: tempDir,
debugMode: false,
fileFilteringRespectGitIgnore: true,
fileFiltering: {
respectGitIgnore: true,
},
};

const config = new Config(configParams);
Expand Down Expand Up @@ -149,13 +156,15 @@ describe('Configuration Integration Tests', () => {
sandbox: false,
targetDir: tempDir,
debugMode: false,
fileFilteringRespectGitIgnore: undefined,
fileFiltering: {},
};

const config = new Config(configParams);

// All settings should use defaults
expect(config.getFileFilteringRespectGitIgnore()).toBe(true);
expect(config.getFileFilteringRespectGitIgnore()).toBe(
DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
);
});

it('should handle missing configuration sections gracefully', async () => {
Expand All @@ -172,7 +181,9 @@ describe('Configuration Integration Tests', () => {
const config = new Config(configParams);

// All git-aware settings should use defaults
expect(config.getFileFilteringRespectGitIgnore()).toBe(true);
expect(config.getFileFilteringRespectGitIgnore()).toBe(
DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
);
});
});

Expand All @@ -185,7 +196,9 @@ describe('Configuration Integration Tests', () => {
sandbox: false,
targetDir: tempDir,
debugMode: false,
fileFilteringRespectGitIgnore: true,
fileFiltering: {
respectGitIgnore: true,
},
};

const config = new Config(configParams);
Expand Down
7 changes: 6 additions & 1 deletion packages/cli/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@
OutputFormat,
type GeminiCLIExtension,
} from '@google/gemini-cli-core';
import { loadCliConfig, parseArguments, type CliArgs } from './config.js';
import {
loadCliConfig,
loadHierarchicalGeminiMemory,
parseArguments,
type CliArgs,
} from './config.js';
import type { Settings } from './settings.js';
import { ExtensionStorage } from './extension.js';
import * as ServerConfig from '@google/gemini-cli-core';
Expand Down Expand Up @@ -2031,7 +2036,7 @@
const config = await loadCliConfig(
{
model: {
name: 'gemini-9001-ultra',

Check warning on line 2039 in packages/cli/src/config/config.test.ts

View workflow job for this annotation

GitHub Actions / Lint

Found sensitive keyword "gemini-9001". Please make sure this change is appropriate to submit.
},
},
[],
Expand All @@ -2043,7 +2048,7 @@
argv,
);

expect(config.getModel()).toBe('gemini-9001-ultra');

Check warning on line 2051 in packages/cli/src/config/config.test.ts

View workflow job for this annotation

GitHub Actions / Lint

Found sensitive keyword "gemini-9001". Please make sure this change is appropriate to submit.
});

it('uses the default gemini model if nothing is set', async () => {
Expand All @@ -2066,12 +2071,12 @@
});

it('always prefers model from argvs', async () => {
process.argv = ['node', 'script.js', '--model', 'gemini-8675309-ultra'];

Check warning on line 2074 in packages/cli/src/config/config.test.ts

View workflow job for this annotation

GitHub Actions / Lint

Found sensitive keyword "gemini-8675309". Please make sure this change is appropriate to submit.
const argv = await parseArguments({} as Settings);
const config = await loadCliConfig(
{
model: {
name: 'gemini-9001-ultra',

Check warning on line 2079 in packages/cli/src/config/config.test.ts

View workflow job for this annotation

GitHub Actions / Lint

Found sensitive keyword "gemini-9001". Please make sure this change is appropriate to submit.
},
},
[],
Expand All @@ -2083,11 +2088,11 @@
argv,
);

expect(config.getModel()).toBe('gemini-8675309-ultra');

Check warning on line 2091 in packages/cli/src/config/config.test.ts

View workflow job for this annotation

GitHub Actions / Lint

Found sensitive keyword "gemini-8675309". Please make sure this change is appropriate to submit.
});

it('selects the model from argvs if provided', async () => {
process.argv = ['node', 'script.js', '--model', 'gemini-8675309-ultra'];

Check warning on line 2095 in packages/cli/src/config/config.test.ts

View workflow job for this annotation

GitHub Actions / Lint

Found sensitive keyword "gemini-8675309". Please make sure this change is appropriate to submit.
const argv = await parseArguments({} as Settings);
const config = await loadCliConfig(
{
Expand All @@ -2102,7 +2107,7 @@
argv,
);

expect(config.getModel()).toBe('gemini-8675309-ultra');

Check warning on line 2110 in packages/cli/src/config/config.test.ts

View workflow job for this annotation

GitHub Actions / Lint

Found sensitive keyword "gemini-8675309". Please make sure this change is appropriate to submit.
});
});

Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ export async function loadCliConfig(
},
telemetry: telemetrySettings,
usageStatisticsEnabled: settings.privacy?.usageStatisticsEnabled ?? true,
fileFiltering: settings.context?.fileFiltering,
fileFiltering,
checkpointing:
argv.checkpointing || settings.general?.checkpointing?.enabled,
proxy:
Expand Down
41 changes: 29 additions & 12 deletions packages/cli/src/ui/commands/memoryCommand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import { MessageType } from '../types.js';
import type { LoadedSettings } from '../../config/settings.js';
import {
getErrorMessage,
loadServerHierarchicalMemory,
type FileDiscoveryService,
} from '@google/gemini-cli-core';
import type { LoadServerHierarchicalMemoryResponse } from '@google/gemini-cli-core/index.js';
import { loadHierarchicalGeminiMemory } from '../../config/config.js';

vi.mock('@google/gemini-cli-core', async (importOriginal) => {
const original =
Expand All @@ -27,11 +27,19 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
if (error instanceof Error) return error.message;
return String(error);
}),
loadServerHierarchicalMemory: vi.fn(),
};
});

const mockLoadServerHierarchicalMemory = loadServerHierarchicalMemory as Mock;
vi.mock('../../config/config.js', async (importOriginal) => {
const original =
await importOriginal<typeof import('../../config/config.js')>();
return {
...original,
loadHierarchicalGeminiMemory: vi.fn(),
};
});

const mockLoadHierarchicalGeminiMemory = loadHierarchicalGeminiMemory as Mock;

describe('memoryCommand', () => {
let mockContext: CommandContext;
Expand Down Expand Up @@ -177,7 +185,7 @@ describe('memoryCommand', () => {
ignore: [],
include: [],
}),
getFolderTrust: () => false,
isTrustedFolder: () => false,
};

mockContext = createMockCommandContext({
Expand All @@ -186,11 +194,17 @@ describe('memoryCommand', () => {
settings: {
merged: {
memoryDiscoveryMaxDirs: 1000,
context: {
importFormat: 'tree',
},
},
} as LoadedSettings,
},
ui: {
setGeminiMdFileCount: vi.fn(),
},
});
mockLoadServerHierarchicalMemory.mockClear();
mockLoadHierarchicalGeminiMemory.mockClear();
});

it('should display success message when memory is refreshed with content', async () => {
Expand All @@ -201,7 +215,7 @@ describe('memoryCommand', () => {
fileCount: 2,
filePaths: ['/path/one/GEMINI.md', '/path/two/GEMINI.md'],
};
mockLoadServerHierarchicalMemory.mockResolvedValue(refreshResult);
mockLoadHierarchicalGeminiMemory.mockResolvedValue(refreshResult);

await refreshCommand.action(mockContext, '');

Expand All @@ -213,7 +227,7 @@ describe('memoryCommand', () => {
expect.any(Number),
);

expect(loadServerHierarchicalMemory).toHaveBeenCalledOnce();
expect(mockLoadHierarchicalGeminiMemory).toHaveBeenCalledOnce();
expect(mockSetUserMemory).toHaveBeenCalledWith(
refreshResult.memoryContent,
);
Expand All @@ -223,6 +237,9 @@ describe('memoryCommand', () => {
expect(mockSetGeminiMdFilePaths).toHaveBeenCalledWith(
refreshResult.filePaths,
);
expect(mockContext.ui.setGeminiMdFileCount).toHaveBeenCalledWith(
refreshResult.fileCount,
);

expect(mockContext.ui.addItem).toHaveBeenCalledWith(
{
Expand All @@ -237,11 +254,11 @@ describe('memoryCommand', () => {
if (!refreshCommand.action) throw new Error('Command has no action');

const refreshResult = { memoryContent: '', fileCount: 0, filePaths: [] };
mockLoadServerHierarchicalMemory.mockResolvedValue(refreshResult);
mockLoadHierarchicalGeminiMemory.mockResolvedValue(refreshResult);

await refreshCommand.action(mockContext, '');

expect(loadServerHierarchicalMemory).toHaveBeenCalledOnce();
expect(mockLoadHierarchicalGeminiMemory).toHaveBeenCalledOnce();
expect(mockSetUserMemory).toHaveBeenCalledWith('');
expect(mockSetGeminiMdFileCount).toHaveBeenCalledWith(0);
expect(mockSetGeminiMdFilePaths).toHaveBeenCalledWith([]);
Expand All @@ -259,11 +276,11 @@ describe('memoryCommand', () => {
if (!refreshCommand.action) throw new Error('Command has no action');

const error = new Error('Failed to read memory files.');
mockLoadServerHierarchicalMemory.mockRejectedValue(error);
mockLoadHierarchicalGeminiMemory.mockRejectedValue(error);

await refreshCommand.action(mockContext, '');

expect(loadServerHierarchicalMemory).toHaveBeenCalledOnce();
expect(mockLoadHierarchicalGeminiMemory).toHaveBeenCalledOnce();
expect(mockSetUserMemory).not.toHaveBeenCalled();
expect(mockSetGeminiMdFileCount).not.toHaveBeenCalled();
expect(mockSetGeminiMdFilePaths).not.toHaveBeenCalled();
Expand Down Expand Up @@ -298,7 +315,7 @@ describe('memoryCommand', () => {
expect.any(Number),
);

expect(loadServerHierarchicalMemory).not.toHaveBeenCalled();
expect(mockLoadHierarchicalGeminiMemory).not.toHaveBeenCalled();
});
});

Expand Down
17 changes: 8 additions & 9 deletions packages/cli/src/ui/commands/memoryCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

import {
getErrorMessage,
loadServerHierarchicalMemory,
} from '@google/gemini-cli-core';
import { getErrorMessage } from '@google/gemini-cli-core';
import { MessageType } from '../types.js';
import { loadHierarchicalGeminiMemory } from '../../config/config.js';
import type { SlashCommand, SlashCommandActionReturn } from './types.js';
import { CommandKind } from './types.js';

Expand Down Expand Up @@ -82,25 +80,26 @@ export const memoryCommand: SlashCommand = {

try {
const config = await context.services.config;
const settings = context.services.settings;
if (config) {
const { memoryContent, fileCount, filePaths } =
await loadServerHierarchicalMemory(
await loadHierarchicalGeminiMemory(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that these are wrappers of each other.

LG.

config.getWorkingDir(),
config.shouldLoadMemoryFromIncludeDirectories()
? config.getWorkspaceContext().getDirectories()
: [],
config.getDebugMode(),
config.getFileService(),
settings.merged,
config.getExtensionContextFilePaths(),
config.getFolderTrust(),
context.services.settings.merged.context?.importFormat ||
'tree', // Use setting or default to 'tree'
config.isTrustedFolder(),
settings.merged.context?.importFormat || 'tree',
config.getFileFilteringOptions(),
context.services.settings.merged.context?.discoveryMaxDirs,
);
config.setUserMemory(memoryContent);
config.setGeminiMdFileCount(fileCount);
config.setGeminiMdFilePaths(filePaths);
context.ui.setGeminiMdFileCount(fileCount);

const successMessage =
memoryContent.length > 0
Expand Down
10 changes: 8 additions & 2 deletions packages/core/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import type { Mock } from 'vitest';
import type { ConfigParameters, SandboxConfig } from './config.js';
import { Config, ApprovalMode } from './config.js';
import {
Config,
ApprovalMode,
DEFAULT_FILE_FILTERING_OPTIONS,
} from './config.js';
import * as path from 'node:path';
import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../tools/memoryTool.js';
import {
Expand Down Expand Up @@ -318,7 +322,9 @@ describe('Server Config (config.ts)', () => {

it('should set default file filtering settings when not provided', () => {
const config = new Config(baseParams);
expect(config.getFileFilteringRespectGitIgnore()).toBe(true);
expect(config.getFileFilteringRespectGitIgnore()).toBe(
DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
);
});

it('should set custom file filtering settings when provided', () => {
Expand Down
15 changes: 11 additions & 4 deletions packages/core/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,14 @@ export interface ExtensionInstallMetadata {

import type { FileFilteringOptions } from './constants.js';
import {
DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
DEFAULT_FILE_FILTERING_OPTIONS,
DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
} from './constants.js';

export type { FileFilteringOptions };
export {
DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
DEFAULT_FILE_FILTERING_OPTIONS,
DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
};

export const DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD = 4_000_000;
Expand Down Expand Up @@ -405,8 +405,12 @@ export class Config {
this.usageStatisticsEnabled = params.usageStatisticsEnabled ?? true;

this.fileFiltering = {
respectGitIgnore: params.fileFiltering?.respectGitIgnore ?? true,
respectGeminiIgnore: params.fileFiltering?.respectGeminiIgnore ?? true,
respectGitIgnore:
params.fileFiltering?.respectGitIgnore ??
DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
respectGeminiIgnore:
params.fileFiltering?.respectGeminiIgnore ??
DEFAULT_FILE_FILTERING_OPTIONS.respectGeminiIgnore,
enableRecursiveFileSearch:
params.fileFiltering?.enableRecursiveFileSearch ?? true,
disableFuzzySearch: params.fileFiltering?.disableFuzzySearch ?? false,
Expand Down Expand Up @@ -885,6 +889,9 @@ export class Config {
return this.ideMode;
}

/***
* TODO: Review if this is actually used at it seems it is only used but the FileCommandLoader
*/
getFolderTrustFeature(): boolean {
return this.folderTrustFeature;
}
Expand Down