feat: allow custom filename for context files (#654)

Co-authored-by: N. Taylor Mullen <ntaylormullen@google.com>
This commit is contained in:
Allen Hutchison
2025-05-31 12:49:28 -07:00
committed by GitHub
parent cbc1614b84
commit 53bf778497
15 changed files with 1710 additions and 888 deletions

View File

@@ -13,6 +13,8 @@ import {
createServerConfig,
loadServerHierarchicalMemory,
ConfigParameters,
setGeminiMdFilename as setServerGeminiMdFilename,
getCurrentGeminiMdFilename,
} from '@gemini-code/core';
import { Settings } from './settings.js';
import { readPackageUp } from 'read-package-up';
@@ -132,6 +134,17 @@ export async function loadCliConfig(settings: Settings): Promise<Config> {
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(),
@@ -159,7 +172,8 @@ export async function loadCliConfig(settings: Settings): Promise<Config> {
userMemory: memoryContent,
geminiMdFileCount: fileCount,
vertexai: useVertexAI,
showMemoryUsage: argv.show_memory_usage || false,
showMemoryUsage:
argv.show_memory_usage || settings.showMemoryUsage || false,
};
return createServerConfig(configParams);

View File

@@ -0,0 +1,309 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
/// <reference types="vitest/globals" />
const MOCK_HOME_DIR = '/mock/home/user'; // MUST BE FIRST
// Mock 'os' first. Its factory uses MOCK_HOME_DIR.
import * as osActual from 'os'; // Import for type info for the mock factory
vi.mock('os', async (importOriginal) => {
const actualOs = await importOriginal<typeof osActual>();
return {
...actualOs,
homedir: vi.fn(() => MOCK_HOME_DIR),
};
});
// Mock './settings.js' to ensure it uses the mocked 'os.homedir()' for its internal constants.
vi.mock('./settings.js', async (importActual) => {
const originalModule = await importActual<typeof import('./settings.js')>();
return {
__esModule: true, // Ensure correct module shape
...originalModule, // Re-export all original members
// We are relying on originalModule's USER_SETTINGS_PATH being constructed with mocked os.homedir()
};
});
// NOW import everything else, including the (now effectively re-exported) settings.js
import * as pathActual from 'path'; // Restored for MOCK_WORKSPACE_SETTINGS_PATH
import {
describe,
it,
expect,
vi,
beforeEach,
afterEach,
type Mocked,
type Mock,
} from 'vitest';
import * as fs from 'fs'; // fs will be mocked separately
import stripJsonComments from 'strip-json-comments'; // Will be mocked separately
// These imports will get the versions from the vi.mock('./settings.js', ...) factory.
import {
LoadedSettings,
loadSettings,
USER_SETTINGS_PATH, // This IS the mocked path.
SETTINGS_DIRECTORY_NAME, // This is from the original module, but used by the mock.
SettingScope,
} from './settings.js';
const MOCK_WORKSPACE_DIR = '/mock/workspace';
// Use the (mocked) SETTINGS_DIRECTORY_NAME for consistency
const MOCK_WORKSPACE_SETTINGS_PATH = pathActual.join(
MOCK_WORKSPACE_DIR,
SETTINGS_DIRECTORY_NAME,
'settings.json',
);
vi.mock('fs');
vi.mock('strip-json-comments', () => ({
default: vi.fn((content) => content),
}));
describe('Settings Loading and Merging', () => {
let mockFsExistsSync: Mocked<typeof fs.existsSync>;
let mockStripJsonComments: Mocked<typeof stripJsonComments>;
let mockFsMkdirSync: Mocked<typeof fs.mkdirSync>;
beforeEach(() => {
vi.resetAllMocks();
mockFsExistsSync = vi.mocked(fs.existsSync);
mockFsMkdirSync = vi.mocked(fs.mkdirSync);
mockStripJsonComments = vi.mocked(stripJsonComments);
vi.mocked(osActual.homedir).mockReturnValue(MOCK_HOME_DIR);
(mockStripJsonComments as unknown as Mock).mockImplementation(
(jsonString: string) => jsonString,
);
(mockFsExistsSync as Mock).mockReturnValue(false);
(fs.readFileSync as Mock).mockReturnValue('{}'); // Return valid empty JSON
(mockFsMkdirSync as Mock).mockImplementation(() => undefined);
});
afterEach(() => {
vi.restoreAllMocks();
});
describe('loadSettings', () => {
it('should load empty settings if no files exist', () => {
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.user.settings).toEqual({});
expect(settings.workspace.settings).toEqual({});
expect(settings.merged).toEqual({});
});
it('should load user settings if only user file exists', () => {
const expectedUserSettingsPath = USER_SETTINGS_PATH; // Use the path actually resolved by the (mocked) module
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === expectedUserSettingsPath,
);
const userSettingsContent = {
theme: 'dark',
contextFileName: 'USER_CONTEXT.md',
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === expectedUserSettingsPath)
return JSON.stringify(userSettingsContent);
return '{}';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(fs.readFileSync).toHaveBeenCalledWith(
expectedUserSettingsPath,
'utf-8',
);
expect(settings.user.settings).toEqual(userSettingsContent);
expect(settings.workspace.settings).toEqual({});
expect(settings.merged).toEqual(userSettingsContent);
});
it('should load workspace settings if only workspace file exists', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH,
);
const workspaceSettingsContent = {
sandbox: true,
contextFileName: 'WORKSPACE_CONTEXT.md',
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(fs.readFileSync).toHaveBeenCalledWith(
MOCK_WORKSPACE_SETTINGS_PATH,
'utf-8',
);
expect(settings.user.settings).toEqual({});
expect(settings.workspace.settings).toEqual(workspaceSettingsContent);
expect(settings.merged).toEqual(workspaceSettingsContent);
});
it('should merge user and workspace settings, with workspace taking precedence', () => {
(mockFsExistsSync as Mock).mockReturnValue(true);
const userSettingsContent = {
theme: 'dark',
sandbox: false,
contextFileName: 'USER_CONTEXT.md',
};
const workspaceSettingsContent = {
sandbox: true,
coreTools: ['tool1'],
contextFileName: 'WORKSPACE_CONTEXT.md',
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.user.settings).toEqual(userSettingsContent);
expect(settings.workspace.settings).toEqual(workspaceSettingsContent);
expect(settings.merged).toEqual({
theme: 'dark',
sandbox: true,
coreTools: ['tool1'],
contextFileName: 'WORKSPACE_CONTEXT.md',
});
});
it('should handle contextFileName correctly when only in user settings', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
);
const userSettingsContent = { contextFileName: 'CUSTOM.md' };
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
return JSON.stringify(userSettingsContent);
return '';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.contextFileName).toBe('CUSTOM.md');
});
it('should handle contextFileName correctly when only in workspace settings', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH,
);
const workspaceSettingsContent = {
contextFileName: 'PROJECT_SPECIFIC.md',
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.contextFileName).toBe('PROJECT_SPECIFIC.md');
});
it('should default contextFileName to undefined if not in any settings file', () => {
(mockFsExistsSync as Mock).mockReturnValue(true);
const userSettingsContent = { theme: 'dark' };
const workspaceSettingsContent = { sandbox: true };
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
return JSON.stringify(workspaceSettingsContent);
return '';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.contextFileName).toBeUndefined();
});
it('should handle JSON parsing errors gracefully', () => {
(mockFsExistsSync as Mock).mockReturnValue(true);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
// Make it return invalid json for the paths it will try to read
if (p === USER_SETTINGS_PATH || p === MOCK_WORKSPACE_SETTINGS_PATH)
return 'invalid json';
return '';
},
);
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.user.settings).toEqual({});
expect(settings.workspace.settings).toEqual({});
expect(settings.merged).toEqual({});
expect(consoleErrorSpy).toHaveBeenCalledTimes(2);
consoleErrorSpy.mockRestore();
});
});
describe('LoadedSettings class', () => {
it('setValue should update the correct scope and recompute merged settings', () => {
(mockFsExistsSync as Mock).mockReturnValue(false);
const loadedSettings = loadSettings(MOCK_WORKSPACE_DIR) as LoadedSettings;
vi.mocked(fs.writeFileSync).mockImplementation(() => {});
// mkdirSync is mocked in beforeEach to return undefined, which is fine for void usage
loadedSettings.setValue(SettingScope.User, 'theme', 'matrix');
expect(loadedSettings.user.settings.theme).toBe('matrix');
expect(loadedSettings.merged.theme).toBe('matrix');
expect(fs.writeFileSync).toHaveBeenCalledWith(
USER_SETTINGS_PATH,
JSON.stringify({ theme: 'matrix' }, null, 2),
'utf-8',
);
loadedSettings.setValue(
SettingScope.Workspace,
'contextFileName',
'MY_AGENTS.md',
);
expect(loadedSettings.workspace.settings.contextFileName).toBe(
'MY_AGENTS.md',
);
expect(loadedSettings.merged.contextFileName).toBe('MY_AGENTS.md');
expect(loadedSettings.merged.theme).toBe('matrix');
expect(fs.writeFileSync).toHaveBeenCalledWith(
MOCK_WORKSPACE_SETTINGS_PATH,
JSON.stringify({ contextFileName: 'MY_AGENTS.md' }, null, 2),
'utf-8',
);
loadedSettings.setValue(SettingScope.Workspace, 'theme', 'ocean');
expect(loadedSettings.workspace.settings.theme).toBe('ocean');
expect(loadedSettings.merged.theme).toBe('ocean');
});
});
});

View File

@@ -30,6 +30,7 @@ export interface Settings {
mcpServerCommand?: string;
mcpServers?: Record<string, MCPServerConfig>;
showMemoryUsage?: boolean;
contextFileName?: string;
// Add other settings here.
}