From 53bf77849760593cc9c1af9a4fb110a1a74acc4f Mon Sep 17 00:00:00 2001 From: Allen Hutchison Date: Sat, 31 May 2025 12:49:28 -0700 Subject: feat: allow custom filename for context files (#654) Co-authored-by: N. Taylor Mullen --- packages/cli/src/config/settings.test.ts | 309 +++++++++++++++++++++++++++++++ 1 file changed, 309 insertions(+) create mode 100644 packages/cli/src/config/settings.test.ts (limited to 'packages/cli/src/config/settings.test.ts') diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts new file mode 100644 index 00000000..4d61ed8b --- /dev/null +++ b/packages/cli/src/config/settings.test.ts @@ -0,0 +1,309 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/// + +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(); + 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(); + 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; + let mockStripJsonComments: Mocked; + let mockFsMkdirSync: Mocked; + + 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'); + }); + }); +}); -- cgit v1.2.3