Make default settings apply (#15354)

This commit is contained in:
Dev Randalpura
2026-01-05 15:00:20 -05:00
committed by GitHub
parent dc6dda5c37
commit 3c92666ec2
7 changed files with 135 additions and 53 deletions

View File

@@ -666,7 +666,7 @@ export async function loadCliConfig(
screenReader,
},
telemetry: telemetrySettings,
usageStatisticsEnabled: settings.privacy?.usageStatisticsEnabled ?? true,
usageStatisticsEnabled: settings.privacy?.usageStatisticsEnabled,
fileFiltering,
checkpointing: settings.general?.checkpointing?.enabled,
proxy:
@@ -678,7 +678,7 @@ export async function loadCliConfig(
fileDiscoveryService: fileService,
bugCommand: settings.advanced?.bugCommand,
model: resolvedModel,
maxSessionTurns: settings.model?.maxSessionTurns ?? -1,
maxSessionTurns: settings.model?.maxSessionTurns,
experimentalZedIntegration: argv.experimentalAcp || false,
listExtensions: argv.listExtensions || false,
listSessions: argv.listSessions || false,
@@ -698,13 +698,12 @@ export async function loadCliConfig(
interactive,
trustedFolder,
useRipgrep: settings.tools?.useRipgrep,
enableInteractiveShell:
settings.tools?.shell?.enableInteractiveShell ?? true,
enableInteractiveShell: settings.tools?.shell?.enableInteractiveShell,
shellToolInactivityTimeout: settings.tools?.shell?.inactivityTimeout,
enableShellOutputEfficiency:
settings.tools?.shell?.enableShellOutputEfficiency ?? true,
skipNextSpeakerCheck: settings.model?.skipNextSpeakerCheck,
enablePromptCompletion: settings.general?.enablePromptCompletion ?? false,
enablePromptCompletion: settings.general?.enablePromptCompletion,
truncateToolOutputThreshold: settings.tools?.truncateToolOutputThreshold,
truncateToolOutputLines: settings.tools?.truncateToolOutputLines,
enableToolOutputTruncation: settings.tools?.enableToolOutputTruncation,
@@ -719,11 +718,11 @@ export async function loadCliConfig(
settings.experimental?.introspectionAgentSettings,
fakeResponses: argv.fakeResponses,
recordResponses: argv.recordResponses,
retryFetchErrors: settings.general?.retryFetchErrors ?? false,
retryFetchErrors: settings.general?.retryFetchErrors,
ptyInfo: ptyInfo?.name,
modelConfigServiceConfig: settings.modelConfigs,
// TODO: loading of hooks based on workspace trust
enableHooks: settings.tools?.enableHooks ?? false,
enableHooks: settings.tools?.enableHooks,
hooks: settings.hooks || {},
projectHooks: projectHooks || {},
onModelChange: (model: string) => saveModelChange(loadedSettings, model),

View File

@@ -35,6 +35,14 @@ vi.mock('./trustedFolders.js', () => ({
.mockReturnValue({ isTrusted: true, source: 'file' }),
}));
vi.mock('./settingsSchema.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('./settingsSchema.js')>();
return {
...actual,
getSettingsSchema: vi.fn(actual.getSettingsSchema),
};
});
// NOW import everything else, including the (now effectively re-exported) settings.js
import path, * as pathActual from 'node:path'; // Restored for MOCK_WORKSPACE_SETTINGS_PATH
import {
@@ -65,10 +73,16 @@ import {
SettingScope,
saveSettings,
type SettingsFile,
getDefaultsFromSchema,
} from './settings.js';
import { FatalConfigError, GEMINI_DIR } from '@google/gemini-cli-core';
import { ExtensionManager } from './extension-manager.js';
import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js';
import {
getSettingsSchema,
MergeStrategy,
type SettingsSchema,
} from './settingsSchema.js';
const MOCK_WORKSPACE_DIR = '/mock/workspace';
// Use the (mocked) GEMINI_DIR for consistency
@@ -149,14 +163,6 @@ describe('Settings Loading and Merging', () => {
});
describe('loadSettings', () => {
it('should load empty settings if no files exist', () => {
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.system.settings).toEqual({});
expect(settings.user.settings).toEqual({});
expect(settings.workspace.settings).toEqual({});
expect(settings.merged).toEqual({});
});
it.each([
{
scope: 'system',
@@ -201,7 +207,7 @@ describe('Settings Loading and Merging', () => {
expect(
settings[scope as 'system' | 'user' | 'workspace'].settings,
).toEqual(content);
expect(settings.merged).toEqual(content);
expect(settings.merged).toMatchObject(content);
},
);
@@ -265,7 +271,7 @@ describe('Settings Loading and Merging', () => {
expect(settings.system.settings).toEqual(systemSettingsContent);
expect(settings.user.settings).toEqual(userSettingsContent);
expect(settings.workspace.settings).toEqual(workspaceSettingsContent);
expect(settings.merged).toEqual({
expect(settings.merged).toMatchObject({
ui: {
theme: 'system-theme',
},
@@ -318,7 +324,7 @@ describe('Settings Loading and Merging', () => {
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged).toEqual({
expect(settings.merged).toMatchObject({
ui: {
theme: 'legacy-dark',
},
@@ -443,10 +449,33 @@ describe('Settings Loading and Merging', () => {
expect(settings.merged.advanced?.excludedEnvVars).toEqual(
expect.arrayContaining(['USER_VAR', 'WORKSPACE_VAR']),
);
expect(settings.merged.advanced?.excludedEnvVars).toHaveLength(2);
expect(settings.merged.advanced?.excludedEnvVars).toHaveLength(4);
});
it('should merge all settings files with the correct precedence', () => {
// Mock schema to test defaults application
const mockSchema = {
ui: { type: 'object', default: {}, properties: {} },
tools: { type: 'object', default: {}, properties: {} },
context: {
type: 'object',
default: {},
properties: {
discoveryMaxDirs: { type: 'number', default: 200 },
includeDirectories: {
type: 'array',
default: [],
mergeStrategy: MergeStrategy.CONCAT,
},
},
},
mcpServers: { type: 'object', default: {} },
};
(getSettingsSchema as Mock).mockReturnValue(
mockSchema as unknown as SettingsSchema,
);
(mockFsExistsSync as Mock).mockReturnValue(true);
const systemDefaultsContent = {
ui: {
@@ -510,7 +539,7 @@ describe('Settings Loading and Merging', () => {
expect(settings.workspace.settings).toEqual(workspaceSettingsContent);
expect(settings.merged).toEqual({
context: {
fileName: 'WORKSPACE_CONTEXT.md',
discoveryMaxDirs: 200,
includeDirectories: [
'/system/defaults/dir',
'/user/dir1',
@@ -518,14 +547,12 @@ describe('Settings Loading and Merging', () => {
'/workspace/dir',
'/system/dir',
],
fileName: 'WORKSPACE_CONTEXT.md',
},
mcpServers: {},
ui: { theme: 'system-theme' },
tools: { sandbox: false },
telemetry: false,
tools: {
sandbox: false,
},
ui: {
theme: 'system-theme',
},
});
});
@@ -660,7 +687,7 @@ describe('Settings Loading and Merging', () => {
},
expected: {
key: 'advanced.excludedEnvVars',
value: ['DEBUG', 'NODE_ENV', 'CUSTOM_VAR'],
value: ['DEBUG', 'DEBUG_MODE', 'NODE_ENV', 'CUSTOM_VAR'],
},
},
{
@@ -671,7 +698,7 @@ describe('Settings Loading and Merging', () => {
},
expected: {
key: 'advanced.excludedEnvVars',
value: ['WORKSPACE_DEBUG', 'WORKSPACE_VAR'],
value: ['DEBUG', 'DEBUG_MODE', 'WORKSPACE_DEBUG', 'WORKSPACE_VAR'],
},
},
])(
@@ -734,6 +761,7 @@ describe('Settings Loading and Merging', () => {
]);
expect(settings.merged.advanced?.excludedEnvVars).toEqual([
'DEBUG',
'DEBUG_MODE',
'NODE_ENV',
'USER_VAR',
'WORKSPACE_DEBUG',
@@ -814,8 +842,8 @@ describe('Settings Loading and Merging', () => {
(fs.readFileSync as Mock).mockReturnValue('{}');
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.telemetry).toBeUndefined();
expect(settings.merged.ui).toBeUndefined();
expect(settings.merged.mcpServers).toBeUndefined();
expect(settings.merged.ui).toBeDefined();
expect(settings.merged.mcpServers).toEqual({});
});
it('should merge MCP servers correctly, with workspace taking precedence', () => {
@@ -941,7 +969,7 @@ describe('Settings Loading and Merging', () => {
(mockFsExistsSync as Mock).mockReturnValue(false); // No settings files exist
(fs.readFileSync as Mock).mockReturnValue('{}');
const settings = loadSettings(MOCK_WORKSPACE_DIR);
expect(settings.merged.mcpServers).toBeUndefined();
expect(settings.merged.mcpServers).toEqual({});
});
it('should merge MCP servers from system, user, and workspace with system taking precedence', () => {
@@ -1075,10 +1103,10 @@ describe('Settings Loading and Merging', () => {
expected: 0.8,
},
{
description: 'should be undefined if not in any settings file',
description: 'should be default if not in any settings file',
userContent: {},
workspaceContent: {},
expected: undefined,
expected: 0.5,
},
])('$description', ({ userContent, workspaceContent, expected }) => {
(mockFsExistsSync as Mock).mockReturnValue(true);
@@ -1590,7 +1618,7 @@ describe('Settings Loading and Merging', () => {
);
expect(settings.system.path).toBe(MOCK_ENV_SYSTEM_SETTINGS_PATH);
expect(settings.system.settings).toEqual(systemSettingsContent);
expect(settings.merged).toEqual({
expect(settings.merged).toMatchObject({
...systemSettingsContent,
});
});
@@ -1692,8 +1720,9 @@ describe('Settings Loading and Merging', () => {
'DEBUG',
]);
expect(settings.merged.advanced?.excludedEnvVars).toEqual([
'NODE_ENV',
'DEBUG',
'DEBUG_MODE',
'NODE_ENV',
]);
});
@@ -1732,6 +1761,7 @@ describe('Settings Loading and Merging', () => {
]);
expect(settings.merged.advanced?.excludedEnvVars).toEqual([
'DEBUG',
'DEBUG_MODE',
'NODE_ENV',
'USER_VAR',
'WORKSPACE_DEBUG',
@@ -2444,4 +2474,42 @@ describe('Settings Loading and Merging', () => {
);
});
});
describe('getDefaultsFromSchema', () => {
it('should extract defaults from a schema', () => {
const mockSchema = {
prop1: {
type: 'string',
default: 'default1',
label: 'Prop 1',
category: 'General',
requiresRestart: false,
},
nested: {
type: 'object',
label: 'Nested',
category: 'General',
requiresRestart: false,
default: {},
properties: {
prop2: {
type: 'number',
default: 42,
label: 'Prop 2',
category: 'General',
requiresRestart: false,
},
},
},
};
const defaults = getDefaultsFromSchema(mockSchema as SettingsSchema);
expect(defaults).toEqual({
prop1: 'default1',
nested: {
prop2: 42,
},
});
});
});
});

View File

@@ -424,6 +424,24 @@ export function migrateSettingsToV1(
return v1Settings;
}
export function getDefaultsFromSchema(
schema: SettingsSchema = getSettingsSchema(),
): Settings {
const defaults: Record<string, unknown> = {};
for (const key in schema) {
const definition = schema[key];
if (definition.properties) {
const childDefaults = getDefaultsFromSchema(definition.properties);
if (Object.keys(childDefaults).length > 0) {
defaults[key] = childDefaults;
}
} else if (definition.default !== undefined) {
defaults[key] = definition.default;
}
}
return defaults as Settings;
}
function mergeSettings(
system: Settings,
systemDefaults: Settings,
@@ -432,16 +450,18 @@ function mergeSettings(
isTrusted: boolean,
): Settings {
const safeWorkspace = isTrusted ? workspace : ({} as Settings);
const schemaDefaults = getDefaultsFromSchema();
// Settings are merged with the following precedence (last one wins for
// single values):
// 1. System Defaults
// 2. User Settings
// 3. Workspace Settings
// 4. System Settings (as overrides)
// 1. Schema Defaults (Built-in)
// 2. System Defaults
// 3. User Settings
// 4. Workspace Settings
// 5. System Settings (as overrides)
return customDeepMerge(
getMergeStrategyForPath,
{}, // Start with an empty object
schemaDefaults,
systemDefaults,
user,
safeWorkspace,

View File

@@ -60,12 +60,11 @@ export const Footer: React.FC = () => {
const showMemoryUsage =
config.getDebugMode() || settings.merged.ui?.showMemoryUsage || false;
const hideCWD = settings.merged.ui?.footer?.hideCWD || false;
const hideSandboxStatus =
settings.merged.ui?.footer?.hideSandboxStatus || false;
const hideModelInfo = settings.merged.ui?.footer?.hideModelInfo || false;
const hideCWD = settings.merged.ui?.footer?.hideCWD;
const hideSandboxStatus = settings.merged.ui?.footer?.hideSandboxStatus;
const hideModelInfo = settings.merged.ui?.footer?.hideModelInfo;
const hideContextPercentage =
settings.merged.ui?.footer?.hideContextPercentage ?? true;
settings.merged.ui?.footer?.hideContextPercentage;
const pathLength = Math.max(20, Math.floor(mainAreaWidth * 0.25));
const displayPath = shortenPath(tildeifyPath(targetDir), pathLength);

View File

@@ -35,8 +35,8 @@ describe('ui-sizing', () => {
[80, true, true, 79], // -1 for alternate buffer
[100, true, true, 99],
// Default behavior (useFullWidth undefined or true)
[100, undefined, false, 100],
// Default behavior (useFullWidth true)
[100, true, false, 100],
// useFullWidth: false (Smart sizing)
[80, false, false, 78], // 98% of 80

View File

@@ -27,7 +27,7 @@ export const calculateMainAreaWidth = (
terminalWidth: number,
settings: LoadedSettings,
): number => {
if (settings.merged.ui?.useFullWidth !== false) {
if (settings.merged.ui?.useFullWidth) {
if (isAlternateBufferEnabled(settings)) {
return terminalWidth - 1;
}

View File

@@ -279,11 +279,7 @@ export function getSettingValue(
if (typeof value === 'boolean') {
return value;
}
// Fall back to default value, ensuring it's a boolean
const defaultValue = definition.default;
if (typeof defaultValue === 'boolean') {
return defaultValue;
}
return false; // Final fallback
}