fix(cli) : fixed bug #8310 where /memory refresh will create discrepancies with initial memory load ignoring settings/config for trusted folder and file filters (#10611)

Co-authored-by: Bryan Morgan <bryanmorgan@google.com>
This commit is contained in:
sgnagnarella
2025-10-10 12:04:15 -04:00
committed by GitHub
parent 65b9e367f0
commit 971eb64e98
7 changed files with 84 additions and 37 deletions
@@ -12,7 +12,10 @@ import type {
ConfigParameters, ConfigParameters,
ContentGeneratorConfig, ContentGeneratorConfig,
} from '@google/gemini-cli-core'; } 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 { http, HttpResponse } from 'msw';
import { setupServer } from 'msw/node'; import { setupServer } from 'msw/node';
@@ -78,12 +81,14 @@ describe('Configuration Integration Tests', () => {
sandbox: false, sandbox: false,
targetDir: tempDir, targetDir: tempDir,
debugMode: false, debugMode: false,
fileFilteringRespectGitIgnore: undefined, // Should default to true fileFilteringRespectGitIgnore: undefined, // Should default to DEFAULT_FILE_FILTERING_OPTIONS
}; };
const config = new Config(configParams); 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 () => { it('should load custom file filtering settings from configuration', async () => {
@@ -112,7 +117,9 @@ describe('Configuration Integration Tests', () => {
sandbox: false, sandbox: false,
targetDir: tempDir, targetDir: tempDir,
debugMode: false, debugMode: false,
fileFilteringRespectGitIgnore: true, fileFiltering: {
respectGitIgnore: true,
},
}; };
const config = new Config(configParams); const config = new Config(configParams);
@@ -149,13 +156,15 @@ describe('Configuration Integration Tests', () => {
sandbox: false, sandbox: false,
targetDir: tempDir, targetDir: tempDir,
debugMode: false, debugMode: false,
fileFilteringRespectGitIgnore: undefined, fileFiltering: {},
}; };
const config = new Config(configParams); const config = new Config(configParams);
// All settings should use defaults // 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 () => { it('should handle missing configuration sections gracefully', async () => {
@@ -172,7 +181,9 @@ describe('Configuration Integration Tests', () => {
const config = new Config(configParams); const config = new Config(configParams);
// All git-aware settings should use defaults // All git-aware settings should use defaults
expect(config.getFileFilteringRespectGitIgnore()).toBe(true); expect(config.getFileFilteringRespectGitIgnore()).toBe(
DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
);
}); });
}); });
@@ -185,7 +196,9 @@ describe('Configuration Integration Tests', () => {
sandbox: false, sandbox: false,
targetDir: tempDir, targetDir: tempDir,
debugMode: false, debugMode: false,
fileFilteringRespectGitIgnore: true, fileFiltering: {
respectGitIgnore: true,
},
}; };
const config = new Config(configParams); const config = new Config(configParams);
+6 -1
View File
@@ -16,7 +16,12 @@ import {
OutputFormat, OutputFormat,
type GeminiCLIExtension, type GeminiCLIExtension,
} from '@google/gemini-cli-core'; } 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 type { Settings } from './settings.js';
import { ExtensionStorage } from './extension.js'; import { ExtensionStorage } from './extension.js';
import * as ServerConfig from '@google/gemini-cli-core'; import * as ServerConfig from '@google/gemini-cli-core';
+1 -1
View File
@@ -715,7 +715,7 @@ export async function loadCliConfig(
}, },
telemetry: telemetrySettings, telemetry: telemetrySettings,
usageStatisticsEnabled: settings.privacy?.usageStatisticsEnabled ?? true, usageStatisticsEnabled: settings.privacy?.usageStatisticsEnabled ?? true,
fileFiltering: settings.context?.fileFiltering, fileFiltering,
checkpointing: checkpointing:
argv.checkpointing || settings.general?.checkpointing?.enabled, argv.checkpointing || settings.general?.checkpointing?.enabled,
proxy: proxy:
@@ -13,10 +13,10 @@ import { MessageType } from '../types.js';
import type { LoadedSettings } from '../../config/settings.js'; import type { LoadedSettings } from '../../config/settings.js';
import { import {
getErrorMessage, getErrorMessage,
loadServerHierarchicalMemory,
type FileDiscoveryService, type FileDiscoveryService,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
import type { LoadServerHierarchicalMemoryResponse } from '@google/gemini-cli-core/index.js'; import type { LoadServerHierarchicalMemoryResponse } from '@google/gemini-cli-core/index.js';
import { loadHierarchicalGeminiMemory } from '../../config/config.js';
vi.mock('@google/gemini-cli-core', async (importOriginal) => { vi.mock('@google/gemini-cli-core', async (importOriginal) => {
const original = const original =
@@ -27,11 +27,19 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
if (error instanceof Error) return error.message; if (error instanceof Error) return error.message;
return String(error); 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', () => { describe('memoryCommand', () => {
let mockContext: CommandContext; let mockContext: CommandContext;
@@ -177,7 +185,7 @@ describe('memoryCommand', () => {
ignore: [], ignore: [],
include: [], include: [],
}), }),
getFolderTrust: () => false, isTrustedFolder: () => false,
}; };
mockContext = createMockCommandContext({ mockContext = createMockCommandContext({
@@ -186,11 +194,17 @@ describe('memoryCommand', () => {
settings: { settings: {
merged: { merged: {
memoryDiscoveryMaxDirs: 1000, memoryDiscoveryMaxDirs: 1000,
context: {
importFormat: 'tree',
},
}, },
} as LoadedSettings, } as LoadedSettings,
}, },
ui: {
setGeminiMdFileCount: vi.fn(),
},
}); });
mockLoadServerHierarchicalMemory.mockClear(); mockLoadHierarchicalGeminiMemory.mockClear();
}); });
it('should display success message when memory is refreshed with content', async () => { it('should display success message when memory is refreshed with content', async () => {
@@ -201,7 +215,7 @@ describe('memoryCommand', () => {
fileCount: 2, fileCount: 2,
filePaths: ['/path/one/GEMINI.md', '/path/two/GEMINI.md'], filePaths: ['/path/one/GEMINI.md', '/path/two/GEMINI.md'],
}; };
mockLoadServerHierarchicalMemory.mockResolvedValue(refreshResult); mockLoadHierarchicalGeminiMemory.mockResolvedValue(refreshResult);
await refreshCommand.action(mockContext, ''); await refreshCommand.action(mockContext, '');
@@ -213,7 +227,7 @@ describe('memoryCommand', () => {
expect.any(Number), expect.any(Number),
); );
expect(loadServerHierarchicalMemory).toHaveBeenCalledOnce(); expect(mockLoadHierarchicalGeminiMemory).toHaveBeenCalledOnce();
expect(mockSetUserMemory).toHaveBeenCalledWith( expect(mockSetUserMemory).toHaveBeenCalledWith(
refreshResult.memoryContent, refreshResult.memoryContent,
); );
@@ -223,6 +237,9 @@ describe('memoryCommand', () => {
expect(mockSetGeminiMdFilePaths).toHaveBeenCalledWith( expect(mockSetGeminiMdFilePaths).toHaveBeenCalledWith(
refreshResult.filePaths, refreshResult.filePaths,
); );
expect(mockContext.ui.setGeminiMdFileCount).toHaveBeenCalledWith(
refreshResult.fileCount,
);
expect(mockContext.ui.addItem).toHaveBeenCalledWith( expect(mockContext.ui.addItem).toHaveBeenCalledWith(
{ {
@@ -237,11 +254,11 @@ describe('memoryCommand', () => {
if (!refreshCommand.action) throw new Error('Command has no action'); if (!refreshCommand.action) throw new Error('Command has no action');
const refreshResult = { memoryContent: '', fileCount: 0, filePaths: [] }; const refreshResult = { memoryContent: '', fileCount: 0, filePaths: [] };
mockLoadServerHierarchicalMemory.mockResolvedValue(refreshResult); mockLoadHierarchicalGeminiMemory.mockResolvedValue(refreshResult);
await refreshCommand.action(mockContext, ''); await refreshCommand.action(mockContext, '');
expect(loadServerHierarchicalMemory).toHaveBeenCalledOnce(); expect(mockLoadHierarchicalGeminiMemory).toHaveBeenCalledOnce();
expect(mockSetUserMemory).toHaveBeenCalledWith(''); expect(mockSetUserMemory).toHaveBeenCalledWith('');
expect(mockSetGeminiMdFileCount).toHaveBeenCalledWith(0); expect(mockSetGeminiMdFileCount).toHaveBeenCalledWith(0);
expect(mockSetGeminiMdFilePaths).toHaveBeenCalledWith([]); expect(mockSetGeminiMdFilePaths).toHaveBeenCalledWith([]);
@@ -259,11 +276,11 @@ describe('memoryCommand', () => {
if (!refreshCommand.action) throw new Error('Command has no action'); if (!refreshCommand.action) throw new Error('Command has no action');
const error = new Error('Failed to read memory files.'); const error = new Error('Failed to read memory files.');
mockLoadServerHierarchicalMemory.mockRejectedValue(error); mockLoadHierarchicalGeminiMemory.mockRejectedValue(error);
await refreshCommand.action(mockContext, ''); await refreshCommand.action(mockContext, '');
expect(loadServerHierarchicalMemory).toHaveBeenCalledOnce(); expect(mockLoadHierarchicalGeminiMemory).toHaveBeenCalledOnce();
expect(mockSetUserMemory).not.toHaveBeenCalled(); expect(mockSetUserMemory).not.toHaveBeenCalled();
expect(mockSetGeminiMdFileCount).not.toHaveBeenCalled(); expect(mockSetGeminiMdFileCount).not.toHaveBeenCalled();
expect(mockSetGeminiMdFilePaths).not.toHaveBeenCalled(); expect(mockSetGeminiMdFilePaths).not.toHaveBeenCalled();
@@ -298,7 +315,7 @@ describe('memoryCommand', () => {
expect.any(Number), expect.any(Number),
); );
expect(loadServerHierarchicalMemory).not.toHaveBeenCalled(); expect(mockLoadHierarchicalGeminiMemory).not.toHaveBeenCalled();
}); });
}); });
@@ -4,11 +4,9 @@
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
import { import { getErrorMessage } from '@google/gemini-cli-core';
getErrorMessage,
loadServerHierarchicalMemory,
} from '@google/gemini-cli-core';
import { MessageType } from '../types.js'; import { MessageType } from '../types.js';
import { loadHierarchicalGeminiMemory } from '../../config/config.js';
import type { SlashCommand, SlashCommandActionReturn } from './types.js'; import type { SlashCommand, SlashCommandActionReturn } from './types.js';
import { CommandKind } from './types.js'; import { CommandKind } from './types.js';
@@ -82,25 +80,26 @@ export const memoryCommand: SlashCommand = {
try { try {
const config = await context.services.config; const config = await context.services.config;
const settings = context.services.settings;
if (config) { if (config) {
const { memoryContent, fileCount, filePaths } = const { memoryContent, fileCount, filePaths } =
await loadServerHierarchicalMemory( await loadHierarchicalGeminiMemory(
config.getWorkingDir(), config.getWorkingDir(),
config.shouldLoadMemoryFromIncludeDirectories() config.shouldLoadMemoryFromIncludeDirectories()
? config.getWorkspaceContext().getDirectories() ? config.getWorkspaceContext().getDirectories()
: [], : [],
config.getDebugMode(), config.getDebugMode(),
config.getFileService(), config.getFileService(),
settings.merged,
config.getExtensionContextFilePaths(), config.getExtensionContextFilePaths(),
config.getFolderTrust(), config.isTrustedFolder(),
context.services.settings.merged.context?.importFormat || settings.merged.context?.importFormat || 'tree',
'tree', // Use setting or default to 'tree'
config.getFileFilteringOptions(), config.getFileFilteringOptions(),
context.services.settings.merged.context?.discoveryMaxDirs,
); );
config.setUserMemory(memoryContent); config.setUserMemory(memoryContent);
config.setGeminiMdFileCount(fileCount); config.setGeminiMdFileCount(fileCount);
config.setGeminiMdFilePaths(filePaths); config.setGeminiMdFilePaths(filePaths);
context.ui.setGeminiMdFileCount(fileCount);
const successMessage = const successMessage =
memoryContent.length > 0 memoryContent.length > 0
+8 -2
View File
@@ -7,7 +7,11 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import type { Mock } from 'vitest'; import type { Mock } from 'vitest';
import type { ConfigParameters, SandboxConfig } from './config.js'; 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 * as path from 'node:path';
import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../tools/memoryTool.js'; import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../tools/memoryTool.js';
import { import {
@@ -318,7 +322,9 @@ describe('Server Config (config.ts)', () => {
it('should set default file filtering settings when not provided', () => { it('should set default file filtering settings when not provided', () => {
const config = new Config(baseParams); 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', () => { it('should set custom file filtering settings when provided', () => {
+11 -4
View File
@@ -144,14 +144,14 @@ export interface ExtensionInstallMetadata {
import type { FileFilteringOptions } from './constants.js'; import type { FileFilteringOptions } from './constants.js';
import { import {
DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
DEFAULT_FILE_FILTERING_OPTIONS, DEFAULT_FILE_FILTERING_OPTIONS,
DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
} from './constants.js'; } from './constants.js';
export type { FileFilteringOptions }; export type { FileFilteringOptions };
export { export {
DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
DEFAULT_FILE_FILTERING_OPTIONS, DEFAULT_FILE_FILTERING_OPTIONS,
DEFAULT_MEMORY_FILE_FILTERING_OPTIONS,
}; };
export const DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD = 4_000_000; export const DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD = 4_000_000;
@@ -405,8 +405,12 @@ export class Config {
this.usageStatisticsEnabled = params.usageStatisticsEnabled ?? true; this.usageStatisticsEnabled = params.usageStatisticsEnabled ?? true;
this.fileFiltering = { this.fileFiltering = {
respectGitIgnore: params.fileFiltering?.respectGitIgnore ?? true, respectGitIgnore:
respectGeminiIgnore: params.fileFiltering?.respectGeminiIgnore ?? true, params.fileFiltering?.respectGitIgnore ??
DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
respectGeminiIgnore:
params.fileFiltering?.respectGeminiIgnore ??
DEFAULT_FILE_FILTERING_OPTIONS.respectGeminiIgnore,
enableRecursiveFileSearch: enableRecursiveFileSearch:
params.fileFiltering?.enableRecursiveFileSearch ?? true, params.fileFiltering?.enableRecursiveFileSearch ?? true,
disableFuzzySearch: params.fileFiltering?.disableFuzzySearch ?? false, disableFuzzySearch: params.fileFiltering?.disableFuzzySearch ?? false,
@@ -885,6 +889,9 @@ export class Config {
return this.ideMode; return this.ideMode;
} }
/***
* TODO: Review if this is actually used at it seems it is only used but the FileCommandLoader
*/
getFolderTrustFeature(): boolean { getFolderTrustFeature(): boolean {
return this.folderTrustFeature; return this.folderTrustFeature;
} }