MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-04-22 02:54:31 -07:00
Code Issues Packages Projects Releases Wiki Activity

perf(cli): cache loadSettings to reduce redundant disk I/O at startup (#21521)

Browse Source
This commit is contained in:
Sehoon Shon
2026-03-09 17:33:16 -04:00
committed by GitHub
parent ab64b15d51
commit 1fd42802be
6 changed files with 295 additions and 130 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli/src/config/extension.test.ts
+2
View File
@@ -31,6 +31,7 @@ import {
loadSettings,
createTestMergedSettings,
SettingScope,
resetSettingsCacheForTesting,
} from './settings.js';
import {
isWorkspaceTrusted,
@@ -161,6 +162,7 @@ describe('extension tests', () => {
beforeEach(() => {
vi.clearAllMocks();
resetSettingsCacheForTesting();
keychainData = {};
mockKeychainStorage = {
getSecret: vi
packages/cli/src/config/settings.test.ts
+253 -128
View File
@@ -13,7 +13,7 @@ vi.mock('os', async (importOriginal) => {
const actualOs = await importOriginal<typeof osActual>();
return {
...actualOs,
homedir: vi.fn(() => '/mock/home/user'),
homedir: vi.fn(() => path.resolve('/mock/home/user')),
platform: vi.fn(() => 'linux'),
};
});
@@ -76,6 +76,7 @@ import {
LoadedSettings,
sanitizeEnvVar,
createTestMergedSettings,
resetSettingsCacheForTesting,
} from './settings.js';
import {
FatalConfigError,
@@ -91,7 +92,7 @@ import {
} from './settingsSchema.js';
import { createMockSettings } from '../test-utils/settings.js';
const MOCK_WORKSPACE_DIR = '/mock/workspace';
const MOCK_WORKSPACE_DIR = path.resolve(path.resolve('/mock/workspace'));
// Use the (mocked) GEMINI_DIR for consistency
const MOCK_WORKSPACE_SETTINGS_PATH = path.join(
MOCK_WORKSPACE_DIR,
@@ -102,6 +103,10 @@ const MOCK_WORKSPACE_SETTINGS_PATH = path.join(
// A more flexible type for test data that allows arbitrary properties.
type TestSettings = Settings & { [key: string]: unknown };
// Helper to normalize paths for test assertions, making them OS-agnostic
const normalizePath = (p: string | fs.PathOrFileDescriptor) =>
path.normalize(p.toString());
vi.mock('fs', async (importOriginal) => {
// Get all the functions from the real 'fs' module
const actualFs = await importOriginal<typeof fs>();
@@ -174,12 +179,15 @@ describe('Settings Loading and Merging', () => {
beforeEach(() => {
vi.resetAllMocks();
resetSettingsCacheForTesting();
mockFsExistsSync = vi.mocked(fs.existsSync);
mockFsMkdirSync = vi.mocked(fs.mkdirSync);
mockStripJsonComments = vi.mocked(stripJsonComments);
vi.mocked(osActual.homedir).mockReturnValue('/mock/home/user');
vi.mocked(osActual.homedir).mockReturnValue(
path.resolve('/mock/home/user'),
);
(mockStripJsonComments as unknown as Mock).mockImplementation(
(jsonString: string) => jsonString,
);
@@ -224,20 +232,25 @@ describe('Settings Loading and Merging', () => {
},
])(
'should load $scope settings if only $scope file exists',
({ scope, path, content }) => {
({ scope, path: p, content }) => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === path,
(pathLike: fs.PathLike) =>
path.normalize(pathLike.toString()) === path.normalize(p),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === path) return JSON.stringify(content);
(pathDesc: fs.PathOrFileDescriptor) => {
if (path.normalize(pathDesc.toString()) === path.normalize(p))
return JSON.stringify(content);
return '{}';
},
);
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(fs.readFileSync).toHaveBeenCalledWith(path, 'utf-8');
expect(fs.readFileSync).toHaveBeenCalledWith(
expect.stringContaining(path.basename(p)),
'utf-8',
);
expect(
settings[scope as 'system' | 'user' | 'workspace'].settings,
).toEqual(content);
@@ -246,12 +259,14 @@ describe('Settings Loading and Merging', () => {
);
it('should merge system, user and workspace settings, with system taking precedence over workspace, and workspace over user', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) =>
p === getSystemSettingsPath() ||
p === USER_SETTINGS_PATH ||
p === MOCK_WORKSPACE_SETTINGS_PATH,
);
(mockFsExistsSync as Mock).mockImplementation((p: fs.PathLike) => {
const normP = path.normalize(p.toString());
return (
normP === path.normalize(getSystemSettingsPath()) ||
normP === path.normalize(USER_SETTINGS_PATH) ||
normP === path.normalize(MOCK_WORKSPACE_SETTINGS_PATH)
);
});
const systemSettingsContent = {
ui: {
theme: 'system-theme',
@@ -290,11 +305,12 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath())
const normP = path.normalize(p.toString());
if (normP === path.normalize(getSystemSettingsPath()))
return JSON.stringify(systemSettingsContent);
if (p === USER_SETTINGS_PATH)
if (normP === path.normalize(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normP === path.normalize(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '';
},
@@ -390,13 +406,13 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemDefaultsPath())
if (normalizePath(p) === normalizePath(getSystemDefaultsPath()))
return JSON.stringify(systemDefaultsContent);
if (p === getSystemSettingsPath())
if (normalizePath(p) === normalizePath(getSystemSettingsPath()))
return JSON.stringify(systemSettingsContent);
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '';
},
@@ -449,11 +465,11 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath())
if (normalizePath(p) === normalizePath(getSystemSettingsPath()))
return JSON.stringify(systemSettingsContent);
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -489,11 +505,11 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath())
if (normalizePath(p) === normalizePath(getSystemSettingsPath()))
return JSON.stringify(systemSettingsContent);
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -523,11 +539,11 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath())
if (normalizePath(p) === normalizePath(getSystemSettingsPath()))
return JSON.stringify(systemSettingsContent);
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -576,11 +592,12 @@ describe('Settings Loading and Merging', () => {
'should handle $description correctly',
({ path, content, expected }) => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === path,
(p: fs.PathLike) => normalizePath(p) === normalizePath(path),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === path) return JSON.stringify(content);
if (normalizePath(p) === normalizePath(path))
return JSON.stringify(content);
return '{}';
},
);
@@ -598,7 +615,8 @@ describe('Settings Loading and Merging', () => {
it('should merge excludedProjectEnvVars with workspace taking precedence over user', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) =>
p === USER_SETTINGS_PATH || p === MOCK_WORKSPACE_SETTINGS_PATH,
normalizePath(p) === normalizePath(USER_SETTINGS_PATH) ||
normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH),
);
const userSettingsContent = {
general: {},
@@ -611,9 +629,9 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '';
},
@@ -643,15 +661,16 @@ describe('Settings Loading and Merging', () => {
it('should default contextFileName to undefined if not in any settings file', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) =>
p === USER_SETTINGS_PATH || p === MOCK_WORKSPACE_SETTINGS_PATH,
normalizePath(p) === normalizePath(USER_SETTINGS_PATH) ||
normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH),
);
const userSettingsContent = { ui: { theme: 'dark' } };
const workspaceSettingsContent = { tools: { sandbox: true } };
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '';
},
@@ -678,11 +697,12 @@ describe('Settings Loading and Merging', () => {
'should load telemetry setting from $scope settings',
({ path, content, expected }) => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === path,
(p: fs.PathLike) => normalizePath(p) === normalizePath(path),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === path) return JSON.stringify(content);
if (normalizePath(p) === normalizePath(path))
return JSON.stringify(content);
return '{}';
},
);
@@ -697,9 +717,9 @@ describe('Settings Loading and Merging', () => {
const workspaceSettingsContent = { telemetry: { enabled: false } };
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -720,7 +740,8 @@ describe('Settings Loading and Merging', () => {
it('should merge MCP servers correctly, with workspace taking precedence', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) =>
p === USER_SETTINGS_PATH || p === MOCK_WORKSPACE_SETTINGS_PATH,
normalizePath(p) === normalizePath(USER_SETTINGS_PATH) ||
normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH),
);
const userSettingsContent = {
mcpServers: {
@@ -751,9 +772,9 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '';
},
@@ -822,11 +843,12 @@ describe('Settings Loading and Merging', () => {
'should handle MCP servers when only in $scope settings',
({ path, content, expected }) => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === path,
(p: fs.PathLike) => normalizePath(p) === normalizePath(path),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === path) return JSON.stringify(content);
if (normalizePath(p) === normalizePath(path))
return JSON.stringify(content);
return '{}';
},
);
@@ -881,11 +903,11 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath())
if (normalizePath(p) === normalizePath(getSystemSettingsPath()))
return JSON.stringify(systemSettingsContent);
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -932,11 +954,11 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath())
if (normalizePath(p) === normalizePath(getSystemSettingsPath()))
return JSON.stringify(systemSettingsContent);
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -983,8 +1005,11 @@ describe('Settings Loading and Merging', () => {
(mockFsExistsSync as Mock).mockReturnValue(true);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH) return JSON.stringify(userContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userContent);
if (
normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH)
)
return JSON.stringify(workspaceContent);
return '{}';
},
@@ -1008,9 +1033,9 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1038,13 +1063,13 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath())
if (normalizePath(p) === normalizePath(getSystemSettingsPath()))
return JSON.stringify(systemSettingsContent);
if (p === getSystemDefaultsPath())
if (normalizePath(p) === normalizePath(getSystemDefaultsPath()))
return JSON.stringify(systemDefaultsContent);
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1073,14 +1098,16 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH) {
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH)) {
// Simulate JSON.parse throwing for user settings
vi.spyOn(JSON, 'parse').mockImplementationOnce(() => {
throw userReadError;
});
return invalidJsonContent; // Content that would cause JSON.parse to throw
}
if (p === MOCK_WORKSPACE_SETTINGS_PATH) {
if (
normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH)
) {
// Simulate JSON.parse throwing for workspace settings
vi.spyOn(JSON, 'parse').mockImplementationOnce(() => {
throw workspaceReadError;
@@ -1119,11 +1146,12 @@ describe('Settings Loading and Merging', () => {
someUrl: 'https://test.com/${TEST_API_KEY}',
};
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(USER_SETTINGS_PATH),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -1149,11 +1177,12 @@ describe('Settings Loading and Merging', () => {
nested: { value: '$WORKSPACE_ENDPOINT' },
};
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1201,13 +1230,15 @@ describe('Settings Loading and Merging', () => {
(mockFsExistsSync as Mock).mockReturnValue(true);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath()) {
if (normalizePath(p) === normalizePath(getSystemSettingsPath())) {
return JSON.stringify(systemSettingsContent);
}
if (p === USER_SETTINGS_PATH) {
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH)) {
return JSON.stringify(userSettingsContent);
}
if (p === MOCK_WORKSPACE_SETTINGS_PATH) {
if (
normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH)
) {
return JSON.stringify(workspaceSettingsContent);
}
return '{}';
@@ -1266,9 +1297,9 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1280,14 +1311,15 @@ describe('Settings Loading and Merging', () => {
it('should use user dnsResolutionOrder if workspace is not defined', () => {
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(USER_SETTINGS_PATH),
);
const userSettingsContent = {
advanced: { dnsResolutionOrder: 'verbatim' },
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -1300,11 +1332,12 @@ describe('Settings Loading and Merging', () => {
it('should leave unresolved environment variables as is', () => {
const userSettingsContent: TestSettings = { apiKey: '$UNDEFINED_VAR' };
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(USER_SETTINGS_PATH),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -1326,11 +1359,12 @@ describe('Settings Loading and Merging', () => {
path: '/path/$VAR_A/${VAR_B}/end',
};
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(USER_SETTINGS_PATH),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -1350,11 +1384,12 @@ describe('Settings Loading and Merging', () => {
list: ['$ITEM_1', '${ITEM_2}', 'literal'],
};
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(USER_SETTINGS_PATH),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -1389,11 +1424,12 @@ describe('Settings Loading and Merging', () => {
};
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(USER_SETTINGS_PATH),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -1434,11 +1470,12 @@ describe('Settings Loading and Merging', () => {
serverAddress: '${TEST_HOST}:${TEST_PORT}/api',
};
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(USER_SETTINGS_PATH),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -1454,7 +1491,9 @@ describe('Settings Loading and Merging', () => {
});
describe('when GEMINI_CLI_SYSTEM_SETTINGS_PATH is set', () => {
const MOCK_ENV_SYSTEM_SETTINGS_PATH = '/mock/env/system/settings.json';
const MOCK_ENV_SYSTEM_SETTINGS_PATH = path.resolve(
'/mock/env/system/settings.json',
);
beforeEach(() => {
process.env['GEMINI_CLI_SYSTEM_SETTINGS_PATH'] =
@@ -1496,8 +1535,8 @@ describe('Settings Loading and Merging', () => {
});
it('should correctly skip workspace-level loading if workspaceDir is a symlink to home', () => {
const mockHomeDir = '/mock/home/user';
const mockSymlinkDir = '/mock/symlink/to/home';
const mockHomeDir = path.resolve('/mock/home/user');
const mockSymlinkDir = path.resolve('/mock/symlink/to/home');
const mockWorkspaceSettingsPath = path.join(
mockSymlinkDir,
GEMINI_DIR,
@@ -1541,6 +1580,79 @@ describe('Settings Loading and Merging', () => {
isWorkspaceHomeDirSpy.mockRestore();
}
});
describe('caching', () => {
it('should cache loadSettings results', () => {
const mockedRead = vi.mocked(fs.readFileSync);
mockedRead.mockClear();
mockedRead.mockReturnValue('{}');
(mockFsExistsSync as Mock).mockReturnValue(true);
const settings1 = loadSettings(MOCK_WORKSPACE_DIR);
const settings2 = loadSettings(MOCK_WORKSPACE_DIR);
expect(mockedRead).toHaveBeenCalledTimes(5); // system, systemDefaults, user, workspace, and potentially an env file
expect(settings1).toBe(settings2);
});
it('should use separate cache for different workspace directories', () => {
const mockedRead = vi.mocked(fs.readFileSync);
mockedRead.mockClear();
mockedRead.mockReturnValue('{}');
(mockFsExistsSync as Mock).mockReturnValue(true);
const workspace1 = path.resolve('/mock/workspace1');
const workspace2 = path.resolve('/mock/workspace2');
const settings1 = loadSettings(workspace1);
const settings2 = loadSettings(workspace2);
expect(mockedRead).toHaveBeenCalledTimes(10); // 5 for each workspace
expect(settings1).not.toBe(settings2);
});
it('should clear cache when saveSettings is called for user settings', () => {
const mockedRead = vi.mocked(fs.readFileSync);
mockedRead.mockClear();
mockedRead.mockReturnValue('{}');
(mockFsExistsSync as Mock).mockReturnValue(true);
const settings1 = loadSettings(MOCK_WORKSPACE_DIR);
expect(mockedRead).toHaveBeenCalledTimes(5);
saveSettings(settings1.user);
const settings2 = loadSettings(MOCK_WORKSPACE_DIR);
expect(mockedRead).toHaveBeenCalledTimes(10); // Should have re-read from disk
expect(settings1).not.toBe(settings2);
});
it('should clear all caches when saveSettings is called for workspace settings', () => {
const mockedRead = vi.mocked(fs.readFileSync);
mockedRead.mockClear();
mockedRead.mockReturnValue('{}');
(mockFsExistsSync as Mock).mockReturnValue(true);
const workspace1 = path.resolve('/mock/workspace1');
const workspace2 = path.resolve('/mock/workspace2');
const settings1W1 = loadSettings(workspace1);
const settings1W2 = loadSettings(workspace2);
expect(mockedRead).toHaveBeenCalledTimes(10);
// Save settings for workspace 1
saveSettings(settings1W1.workspace);
const settings2W1 = loadSettings(workspace1);
const settings2W2 = loadSettings(workspace2);
// Both workspace caches should have been cleared and re-read from disk (+10 reads)
expect(mockedRead).toHaveBeenCalledTimes(20);
expect(settings1W1).not.toBe(settings2W1);
expect(settings1W2).not.toBe(settings2W2);
});
});
});
describe('excludedProjectEnvVars integration', () => {
@@ -1562,12 +1674,13 @@ describe('Settings Loading and Merging', () => {
};
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === MOCK_WORKSPACE_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1578,16 +1691,18 @@ describe('Settings Loading and Merging', () => {
loadSettings as unknown as { findEnvFile: () => string }
).findEnvFile;
(loadSettings as unknown as { findEnvFile: () => string }).findEnvFile =
() => '/mock/project/.env';
() => path.resolve('/mock/project/.env');
// Mock fs.readFileSync for .env file content
const originalReadFileSync = fs.readFileSync;
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === '/mock/project/.env') {
if (p === path.resolve('/mock/project/.env')) {
return 'DEBUG=true\nDEBUG_MODE=1\nGEMINI_API_KEY=test-key';
}
if (p === MOCK_WORKSPACE_SETTINGS_PATH) {
if (
normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH)
) {
return JSON.stringify(workspaceSettingsContent);
}
return '{}';
@@ -1621,12 +1736,13 @@ describe('Settings Loading and Merging', () => {
};
(mockFsExistsSync as Mock).mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(USER_SETTINGS_PATH),
);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -1658,9 +1774,9 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1702,9 +1818,9 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1734,9 +1850,9 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1767,9 +1883,9 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1940,9 +2056,9 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(MOCK_WORKSPACE_SETTINGS_PATH))
return JSON.stringify(workspaceSettingsContent);
return '{}';
},
@@ -1966,7 +2082,7 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -1994,7 +2110,7 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -2039,7 +2155,7 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -2226,7 +2342,8 @@ describe('Settings Loading and Merging', () => {
it('should trigger migration automatically during loadSettings', () => {
mockFsExistsSync.mockImplementation(
(p: fs.PathLike) => p === USER_SETTINGS_PATH,
(p: fs.PathLike) =>
normalizePath(p) === normalizePath(USER_SETTINGS_PATH),
);
const userSettingsContent = {
general: {
@@ -2235,7 +2352,7 @@ describe('Settings Loading and Merging', () => {
};
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -2270,10 +2387,10 @@ describe('Settings Loading and Merging', () => {
vi.mocked(fs.existsSync).mockReturnValue(true);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath()) {
if (normalizePath(p) === normalizePath(getSystemSettingsPath())) {
return JSON.stringify(systemSettingsContent);
}
if (p === getSystemDefaultsPath()) {
if (normalizePath(p) === normalizePath(getSystemDefaultsPath())) {
return JSON.stringify(systemDefaultsContent);
}
return '{}';
@@ -2343,7 +2460,7 @@ describe('Settings Loading and Merging', () => {
vi.mocked(fs.existsSync).mockReturnValue(true);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath()) {
if (normalizePath(p) === normalizePath(getSystemSettingsPath())) {
return JSON.stringify(systemSettingsContent);
}
return '{}';
@@ -2394,7 +2511,7 @@ describe('Settings Loading and Merging', () => {
vi.mocked(fs.existsSync).mockReturnValue(true);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === USER_SETTINGS_PATH)
if (normalizePath(p) === normalizePath(USER_SETTINGS_PATH))
return JSON.stringify(userSettingsContent);
return '{}';
},
@@ -2430,13 +2547,16 @@ describe('Settings Loading and Merging', () => {
it('should save settings using updateSettingsFilePreservingFormat', () => {
const mockUpdateSettings = vi.mocked(updateSettingsFilePreservingFormat);
const settingsFile = createMockSettings({ ui: { theme: 'dark' } }).user;
settingsFile.path = '/mock/settings.json';
settingsFile.path = path.resolve('/mock/settings.json');
saveSettings(settingsFile);
expect(mockUpdateSettings).toHaveBeenCalledWith('/mock/settings.json', {
ui: { theme: 'dark' },
});
expect(mockUpdateSettings).toHaveBeenCalledWith(
path.resolve('/mock/settings.json'),
{
ui: { theme: 'dark' },
},
);
});
it('should create directory if it does not exist', () => {
@@ -2445,14 +2565,19 @@ describe('Settings Loading and Merging', () => {
mockFsExistsSync.mockReturnValue(false);
const settingsFile = createMockSettings({}).user;
settingsFile.path = '/mock/new/dir/settings.json';
settingsFile.path = path.resolve('/mock/new/dir/settings.json');
saveSettings(settingsFile);
expect(mockFsExistsSync).toHaveBeenCalledWith('/mock/new/dir');
expect(mockFsMkdirSync).toHaveBeenCalledWith('/mock/new/dir', {
recursive: true,
});
expect(mockFsExistsSync).toHaveBeenCalledWith(
path.resolve('/mock/new/dir'),
);
expect(mockFsMkdirSync).toHaveBeenCalledWith(
path.resolve('/mock/new/dir'),
{
recursive: true,
},
);
});
it('should emit error feedback if saving fails', () => {
@@ -2463,7 +2588,7 @@ describe('Settings Loading and Merging', () => {
});
const settingsFile = createMockSettings({}).user;
settingsFile.path = '/mock/settings.json';
settingsFile.path = path.resolve('/mock/settings.json');
saveSettings(settingsFile);
@@ -2491,7 +2616,7 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath()) {
if (normalizePath(p) === normalizePath(getSystemSettingsPath())) {
return JSON.stringify(systemSettingsContent);
}
return '{}';
@@ -2538,7 +2663,7 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath()) {
if (normalizePath(p) === normalizePath(getSystemSettingsPath())) {
return JSON.stringify(systemSettingsContent);
}
return '{}';
@@ -2579,7 +2704,7 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
if (p === getSystemSettingsPath()) {
if (normalizePath(p) === normalizePath(getSystemSettingsPath())) {
return JSON.stringify(systemSettingsContent);
}
return '{}';
@@ -2694,7 +2819,7 @@ describe('Settings Loading and Merging', () => {
beforeEach(() => {
const emptySettingsFile: SettingsFile = {
path: '/mock/path',
path: path.resolve('/mock/path'),
settings: {},
originalSettings: {},
};
@@ -3019,7 +3144,7 @@ describe('LoadedSettings Isolation and Serializability', () => {
// Create a minimal LoadedSettings instance
const emptyScope = {
path: '/mock/settings.json',
path: path.resolve('/mock/settings.json'),
settings: {},
originalSettings: {},
} as unknown as SettingsFile;
packages/cli/src/config/settings.ts
+28
View File
@@ -18,6 +18,7 @@ import {
coreEvents,
homedir,
type AdminControlsSettings,
createCache,
} from '@google/gemini-cli-core';
import stripJsonComments from 'strip-json-comments';
import { DefaultLight } from '../ui/themes/builtin/light/default-light.js';
@@ -615,6 +616,20 @@ export function loadEnvironment(
}
}
// Cache to store the results of loadSettings to avoid redundant disk I/O.
const settingsCache = createCache<string, LoadedSettings>({
storage: 'map',
defaultTtl: 10000, // 10 seconds
});
/**
* Resets the settings cache. Used exclusively for test isolation.
* @internal
*/
export function resetSettingsCacheForTesting() {
settingsCache.clear();
}
/**
* Loads settings from user and workspace directories.
* Project settings override user settings.
@@ -622,6 +637,16 @@ export function loadEnvironment(
export function loadSettings(
workspaceDir: string = process.cwd(),
): LoadedSettings {
const normalizedWorkspaceDir = path.resolve(workspaceDir);
return settingsCache.getOrCreate(normalizedWorkspaceDir, () =>
_doLoadSettings(normalizedWorkspaceDir),
);
}
/**
* Internal implementation of the settings loading logic.
*/
function _doLoadSettings(workspaceDir: string): LoadedSettings {
let systemSettings: Settings = {};
let systemDefaultSettings: Settings = {};
let userSettings: Settings = {};
@@ -1029,6 +1054,9 @@ export function migrateDeprecatedSettings(
}
export function saveSettings(settingsFile: SettingsFile): void {
// Clear the entire cache on any save.
settingsCache.clear();
try {
// Ensure the directory exists
const dirPath = path.dirname(settingsFile.path);
packages/cli/src/config/settings_validation_warning.test.ts
+2
View File
@@ -81,6 +81,7 @@ import {
loadSettings,
USER_SETTINGS_PATH,
type LoadedSettings,
resetSettingsCacheForTesting,
} from './settings.js';
const MOCK_WORKSPACE_DIR = '/mock/workspace';
@@ -88,6 +89,7 @@ const MOCK_WORKSPACE_DIR = '/mock/workspace';
describe('Settings Validation Warning', () => {
beforeEach(() => {
vi.clearAllMocks();
resetSettingsCacheForTesting();
(fs.readFileSync as Mock).mockReturnValue('{}');
(fs.existsSync as Mock).mockReturnValue(false);
});