summaryrefslogtreecommitdiff
path: root/packages/cli/src
diff options
context:
space:
mode:
Diffstat (limited to 'packages/cli/src')
-rw-r--r--packages/cli/src/config/settings.test.ts154
-rw-r--r--packages/cli/src/config/settings.ts107
2 files changed, 105 insertions, 156 deletions
diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts
index d537eeb1..65a556be 100644
--- a/packages/cli/src/config/settings.test.ts
+++ b/packages/cli/src/config/settings.test.ts
@@ -949,23 +949,37 @@ describe('Settings Loading and Merging', () => {
delete process.env['WORKSPACE_ENDPOINT'];
});
- it('should prioritize user env variables over workspace env variables if keys clash after resolution', () => {
- const userSettingsContent = { configValue: '$SHARED_VAR' };
- const workspaceSettingsContent = { configValue: '$SHARED_VAR' };
+ it('should correctly resolve and merge env variables from different scopes', () => {
+ process.env['SYSTEM_VAR'] = 'system_value';
+ process.env['USER_VAR'] = 'user_value';
+ process.env['WORKSPACE_VAR'] = 'workspace_value';
+ process.env['SHARED_VAR'] = 'final_value';
- (mockFsExistsSync as Mock).mockReturnValue(true);
- const originalSharedVar = process.env['SHARED_VAR'];
- // Temporarily delete to ensure a clean slate for the test's specific manipulations
- delete process.env['SHARED_VAR'];
+ const systemSettingsContent = {
+ configValue: '$SHARED_VAR',
+ systemOnly: '$SYSTEM_VAR',
+ };
+ const userSettingsContent = {
+ configValue: '$SHARED_VAR',
+ userOnly: '$USER_VAR',
+ theme: 'dark',
+ };
+ const workspaceSettingsContent = {
+ configValue: '$SHARED_VAR',
+ workspaceOnly: '$WORKSPACE_VAR',
+ theme: 'light',
+ };
+ (mockFsExistsSync as Mock).mockReturnValue(true);
(fs.readFileSync as Mock).mockImplementation(
(p: fs.PathOrFileDescriptor) => {
+ if (p === getSystemSettingsPath()) {
+ return JSON.stringify(systemSettingsContent);
+ }
if (p === USER_SETTINGS_PATH) {
- process.env['SHARED_VAR'] = 'user_value_for_user_read'; // Set for user settings read
return JSON.stringify(userSettingsContent);
}
if (p === MOCK_WORKSPACE_SETTINGS_PATH) {
- process.env['SHARED_VAR'] = 'workspace_value_for_workspace_read'; // Set for workspace settings read
return JSON.stringify(workspaceSettingsContent);
}
return '{}';
@@ -974,115 +988,35 @@ describe('Settings Loading and Merging', () => {
const settings = loadSettings(MOCK_WORKSPACE_DIR);
+ // Check resolved values in individual scopes
// @ts-expect-error: dynamic property for test
- expect(settings.user.settings.configValue).toBe(
- 'user_value_for_user_read',
- );
+ expect(settings.system.settings.configValue).toBe('final_value');
// @ts-expect-error: dynamic property for test
- expect(settings.workspace.settings.configValue).toBe(
- 'workspace_value_for_workspace_read',
- );
- // Merged should take workspace's resolved value
+ expect(settings.system.settings.systemOnly).toBe('system_value');
// @ts-expect-error: dynamic property for test
- expect(settings.merged.configValue).toBe(
- 'workspace_value_for_workspace_read',
- );
-
- // Restore original environment variable state
- if (originalSharedVar !== undefined) {
- process.env['SHARED_VAR'] = originalSharedVar;
- } else {
- delete process.env['SHARED_VAR']; // Ensure it's deleted if it wasn't there before
- }
- });
-
- it('should prioritize workspace env variables over user env variables if keys clash after resolution', () => {
- const userSettingsContent = { configValue: '$SHARED_VAR' };
- const workspaceSettingsContent = { configValue: '$SHARED_VAR' };
-
- (mockFsExistsSync as Mock).mockReturnValue(true);
- const originalSharedVar = process.env['SHARED_VAR'];
- // Temporarily delete to ensure a clean slate for the test's specific manipulations
- delete process.env['SHARED_VAR'];
-
- (fs.readFileSync as Mock).mockImplementation(
- (p: fs.PathOrFileDescriptor) => {
- if (p === USER_SETTINGS_PATH) {
- process.env['SHARED_VAR'] = 'user_value_for_user_read'; // Set for user settings read
- return JSON.stringify(userSettingsContent);
- }
- if (p === MOCK_WORKSPACE_SETTINGS_PATH) {
- process.env['SHARED_VAR'] = 'workspace_value_for_workspace_read'; // Set for workspace settings read
- return JSON.stringify(workspaceSettingsContent);
- }
- return '{}';
- },
- );
-
- const settings = loadSettings(MOCK_WORKSPACE_DIR);
-
- expect(settings.user.settings.configValue).toBe(
- 'user_value_for_user_read',
- );
- expect(settings.workspace.settings.configValue).toBe(
- 'workspace_value_for_workspace_read',
- );
- // Merged should take workspace's resolved value
- expect(settings.merged.configValue).toBe(
- 'workspace_value_for_workspace_read',
- );
-
- // Restore original environment variable state
- if (originalSharedVar !== undefined) {
- process.env['SHARED_VAR'] = originalSharedVar;
- } else {
- delete process.env['SHARED_VAR']; // Ensure it's deleted if it wasn't there before
- }
- });
-
- it('should prioritize system env variables over workspace env variables if keys clash after resolution', () => {
- const workspaceSettingsContent = { configValue: '$SHARED_VAR' };
- const systemSettingsContent = { configValue: '$SHARED_VAR' };
-
- (mockFsExistsSync as Mock).mockReturnValue(true);
- const originalSharedVar = process.env['SHARED_VAR'];
- // Temporarily delete to ensure a clean slate for the test's specific manipulations
- delete process.env['SHARED_VAR'];
-
- (fs.readFileSync as Mock).mockImplementation(
- (p: fs.PathOrFileDescriptor) => {
- if (p === getSystemSettingsPath()) {
- process.env['SHARED_VAR'] = 'system_value_for_system_read'; // Set for system settings read
- return JSON.stringify(systemSettingsContent);
- }
- if (p === MOCK_WORKSPACE_SETTINGS_PATH) {
- process.env['SHARED_VAR'] = 'workspace_value_for_workspace_read'; // Set for workspace settings read
- return JSON.stringify(workspaceSettingsContent);
- }
- return '{}';
- },
- );
-
- const settings = loadSettings(MOCK_WORKSPACE_DIR);
+ expect(settings.user.settings.configValue).toBe('final_value');
+ // @ts-expect-error: dynamic property for test
+ expect(settings.user.settings.userOnly).toBe('user_value');
+ // @ts-expect-error: dynamic property for test
+ expect(settings.workspace.settings.configValue).toBe('final_value');
+ // @ts-expect-error: dynamic property for test
+ expect(settings.workspace.settings.workspaceOnly).toBe('workspace_value');
+ // Check merged values (system > workspace > user)
// @ts-expect-error: dynamic property for test
- expect(settings.system.settings.configValue).toBe(
- 'system_value_for_system_read',
- );
+ expect(settings.merged.configValue).toBe('final_value');
// @ts-expect-error: dynamic property for test
- expect(settings.workspace.settings.configValue).toBe(
- 'workspace_value_for_workspace_read',
- );
- // Merged should take system's resolved value
+ expect(settings.merged.systemOnly).toBe('system_value');
+ // @ts-expect-error: dynamic property for test
+ expect(settings.merged.userOnly).toBe('user_value');
// @ts-expect-error: dynamic property for test
- expect(settings.merged.configValue).toBe('system_value_for_system_read');
+ expect(settings.merged.workspaceOnly).toBe('workspace_value');
+ expect(settings.merged.theme).toBe('light'); // workspace overrides user
- // Restore original environment variable state
- if (originalSharedVar !== undefined) {
- process.env['SHARED_VAR'] = originalSharedVar;
- } else {
- delete process.env['SHARED_VAR']; // Ensure it's deleted if it wasn't there before
- }
+ delete process.env['SYSTEM_VAR'];
+ delete process.env['USER_VAR'];
+ delete process.env['WORKSPACE_VAR'];
+ delete process.env['SHARED_VAR'];
});
it('should correctly merge dnsResolutionOrder with workspace taking precedence', () => {
diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts
index 524eb16c..414caf11 100644
--- a/packages/cli/src/config/settings.ts
+++ b/packages/cli/src/config/settings.ts
@@ -70,6 +70,43 @@ export interface SettingsFile {
settings: Settings;
path: string;
}
+
+function mergeSettings(
+ system: Settings,
+ user: Settings,
+ workspace: Settings,
+): Settings {
+ // folderTrust is not supported at workspace level.
+ // eslint-disable-next-line @typescript-eslint/no-unused-vars
+ const { folderTrust, ...workspaceWithoutFolderTrust } = workspace;
+
+ return {
+ ...user,
+ ...workspaceWithoutFolderTrust,
+ ...system,
+ customThemes: {
+ ...(user.customThemes || {}),
+ ...(workspace.customThemes || {}),
+ ...(system.customThemes || {}),
+ },
+ mcpServers: {
+ ...(user.mcpServers || {}),
+ ...(workspace.mcpServers || {}),
+ ...(system.mcpServers || {}),
+ },
+ includeDirectories: [
+ ...(system.includeDirectories || []),
+ ...(user.includeDirectories || []),
+ ...(workspace.includeDirectories || []),
+ ],
+ chatCompression: {
+ ...(system.chatCompression || {}),
+ ...(user.chatCompression || {}),
+ ...(workspace.chatCompression || {}),
+ },
+ };
+}
+
export class LoadedSettings {
constructor(
system: SettingsFile,
@@ -96,39 +133,11 @@ export class LoadedSettings {
}
private computeMergedSettings(): Settings {
- const system = this.system.settings;
- const user = this.user.settings;
- const workspace = this.workspace.settings;
-
- // folderTrust is not supported at workspace level.
- // eslint-disable-next-line @typescript-eslint/no-unused-vars
- const { folderTrust, ...workspaceWithoutFolderTrust } = workspace;
-
- return {
- ...user,
- ...workspaceWithoutFolderTrust,
- ...system,
- customThemes: {
- ...(user.customThemes || {}),
- ...(workspace.customThemes || {}),
- ...(system.customThemes || {}),
- },
- mcpServers: {
- ...(user.mcpServers || {}),
- ...(workspace.mcpServers || {}),
- ...(system.mcpServers || {}),
- },
- includeDirectories: [
- ...(system.includeDirectories || []),
- ...(user.includeDirectories || []),
- ...(workspace.includeDirectories || []),
- ],
- chatCompression: {
- ...(system.chatCompression || {}),
- ...(user.chatCompression || {}),
- ...(workspace.chatCompression || {}),
- },
- };
+ return mergeSettings(
+ this.system.settings,
+ this.user.settings,
+ this.workspace.settings,
+ );
}
forScope(scope: SettingScope): SettingsFile {
@@ -339,10 +348,7 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
try {
if (fs.existsSync(systemSettingsPath)) {
const systemContent = fs.readFileSync(systemSettingsPath, 'utf-8');
- const parsedSystemSettings = JSON.parse(
- stripJsonComments(systemContent),
- ) as Settings;
- systemSettings = resolveEnvVarsInObject(parsedSystemSettings);
+ systemSettings = JSON.parse(stripJsonComments(systemContent)) as Settings;
}
} catch (error: unknown) {
settingsErrors.push({
@@ -355,10 +361,7 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
try {
if (fs.existsSync(USER_SETTINGS_PATH)) {
const userContent = fs.readFileSync(USER_SETTINGS_PATH, 'utf-8');
- const parsedUserSettings = JSON.parse(
- stripJsonComments(userContent),
- ) as Settings;
- userSettings = resolveEnvVarsInObject(parsedUserSettings);
+ userSettings = JSON.parse(stripJsonComments(userContent)) as Settings;
// Support legacy theme names
if (userSettings.theme && userSettings.theme === 'VS') {
userSettings.theme = DefaultLight.name;
@@ -378,10 +381,9 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
try {
if (fs.existsSync(workspaceSettingsPath)) {
const projectContent = fs.readFileSync(workspaceSettingsPath, 'utf-8');
- const parsedWorkspaceSettings = JSON.parse(
+ workspaceSettings = JSON.parse(
stripJsonComments(projectContent),
) as Settings;
- workspaceSettings = resolveEnvVarsInObject(parsedWorkspaceSettings);
if (workspaceSettings.theme && workspaceSettings.theme === 'VS') {
workspaceSettings.theme = DefaultLight.name;
} else if (
@@ -399,6 +401,22 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
}
}
+ // Create a temporary merged settings object to pass to loadEnvironment.
+ const tempMergedSettings = mergeSettings(
+ systemSettings,
+ userSettings,
+ workspaceSettings,
+ );
+
+ // loadEnviroment depends on settings so we have to create a temp version of
+ // the settings to avoid a cycle
+ loadEnvironment(tempMergedSettings);
+
+ // Now that the environment is loaded, resolve variables in the settings.
+ systemSettings = resolveEnvVarsInObject(systemSettings);
+ userSettings = resolveEnvVarsInObject(userSettings);
+ workspaceSettings = resolveEnvVarsInObject(workspaceSettings);
+
// Create LoadedSettings first
const loadedSettings = new LoadedSettings(
{
@@ -429,9 +447,6 @@ export function loadSettings(workspaceDir: string): LoadedSettings {
delete loadedSettings.merged.chatCompression;
}
- // Load environment with merged settings
- loadEnvironment(loadedSettings.merged);
-
return loadedSettings;
}
generated by cgit v1.2.3 (git 2.51.0) at 2025-11-12 18:24:21 +0000