Make merged settings non-nullable and fix all lints related to that. (#16647)

This commit is contained in:
Jacob Richman
2026-01-15 09:26:10 -08:00
committed by GitHub
parent 2b6bfe4097
commit f7f38e2b9e
59 changed files with 964 additions and 744 deletions

View File

@@ -22,8 +22,9 @@ import {
Config,
DEFAULT_FILE_FILTERING_OPTIONS,
} from '@google/gemini-cli-core';
import type { Settings } from './settingsSchema.js';
import { createTestMergedSettings } from './settings.js';
import { http, HttpResponse } from 'msw';
import { setupServer } from 'msw/node';
export const server = setupServer();
@@ -212,7 +213,7 @@ describe('Configuration Integration Tests', () => {
const originalArgv = process.argv;
try {
process.argv = argv;
const parsedArgs = await parseArguments({} as Settings);
const parsedArgs = await parseArguments(createTestMergedSettings());
expect(parsedArgs.approvalMode).toBe(expected.approvalMode);
expect(parsedArgs.prompt).toBe(expected.prompt);
expect(parsedArgs.yolo).toBe(expected.yolo);
@@ -235,7 +236,9 @@ describe('Configuration Integration Tests', () => {
const originalArgv = process.argv;
try {
process.argv = argv;
await expect(parseArguments({} as Settings)).rejects.toThrow();
await expect(
parseArguments(createTestMergedSettings()),
).rejects.toThrow();
} finally {
process.argv = originalArgv;
}

File diff suppressed because it is too large Load Diff

View File

@@ -38,8 +38,12 @@ import {
type OutputFormat,
GEMINI_MODEL_ALIAS_AUTO,
} from '@google/gemini-cli-core';
import type { Settings } from './settings.js';
import { saveModelChange, loadSettings } from './settings.js';
import {
type Settings,
type MergedSettings,
saveModelChange,
loadSettings,
} from './settings.js';
import { loadSandboxConfig } from './sandboxConfig.js';
import { resolvePath } from '../utils/resolvePath.js';
@@ -54,7 +58,6 @@ import { requestConsentNonInteractive } from './extensions/consent.js';
import { promptForSetting } from './extensions/extensionSettings.js';
import type { EventEmitter } from 'node:stream';
import { runExitCleanup } from '../utils/cleanup.js';
import { getEnableHooks, getEnableHooksUI } from './settingsSchema.js';
export interface CliArgs {
query: string | undefined;
@@ -82,7 +85,9 @@ export interface CliArgs {
recordResponses: string | undefined;
}
export async function parseArguments(settings: Settings): Promise<CliArgs> {
export async function parseArguments(
settings: MergedSettings,
): Promise<CliArgs> {
const rawArgv = hideBin(process.argv);
const yargsInstance = yargs(rawArgv)
.locale('en')
@@ -280,16 +285,16 @@ export async function parseArguments(settings: Settings): Promise<CliArgs> {
return true;
});
if (settings?.experimental?.extensionManagement ?? true) {
if (settings.experimental.extensionManagement) {
yargsInstance.command(extensionsCommand);
}
if (settings?.experimental?.skills ?? false) {
if (settings.experimental.skills) {
yargsInstance.command(skillsCommand);
}
// Register hooks command if hooks are enabled
if (getEnableHooksUI(settings)) {
if (settings.tools.enableHooks) {
yargsInstance.command(hooksCommand);
}
@@ -392,7 +397,7 @@ export interface LoadCliConfigOptions {
}
export async function loadCliConfig(
settings: Settings,
settings: MergedSettings,
sessionId: string,
argv: CliArgs,
options: LoadCliConfigOptions = {},
@@ -590,10 +595,7 @@ export async function loadCliConfig(
}
}
const excludeTools = mergeExcludeTools(
settings,
extraExcludes.length > 0 ? extraExcludes : undefined,
);
const excludeTools = mergeExcludeTools(settings, extraExcludes);
// Create a settings object that includes CLI overrides for policy generation
const effectiveSettings: Settings = {
@@ -742,15 +744,17 @@ export async function loadCliConfig(
disableLLMCorrection: settings.tools?.disableLLMCorrection,
modelConfigServiceConfig: settings.modelConfigs,
// TODO: loading of hooks based on workspace trust
enableHooks: getEnableHooks(settings),
enableHooksUI: getEnableHooksUI(settings),
enableHooks:
(settings.tools?.enableHooks ?? true) &&
(settings.hooks?.enabled ?? false),
enableHooksUI: settings.tools?.enableHooks ?? true,
hooks: settings.hooks || {},
projectHooks: projectHooks || {},
onModelChange: (model: string) => saveModelChange(loadedSettings, model),
onReload: async () => {
const refreshedSettings = loadSettings(cwd);
return {
disabledSkills: refreshedSettings.merged.skills?.disabled,
disabledSkills: refreshedSettings.merged.skills.disabled,
agents: refreshedSettings.merged.agents,
};
},
@@ -758,12 +762,12 @@ export async function loadCliConfig(
}
function mergeExcludeTools(
settings: Settings,
extraExcludes?: string[] | undefined,
settings: MergedSettings,
extraExcludes: string[] = [],
): string[] {
const allExcludeTools = new Set([
...(settings.tools?.exclude || []),
...(extraExcludes || []),
...(settings.tools.exclude || []),
...extraExcludes,
]);
return [...allExcludeTools];
return Array.from(allExcludeTools);
}

View File

@@ -10,7 +10,7 @@ import * as path from 'node:path';
import * as os from 'node:os';
import { ExtensionManager } from './extension-manager.js';
import { debugLogger } from '@google/gemini-cli-core';
import { type Settings } from './settings.js';
import { createTestMergedSettings } from './settings.js';
import { createExtension } from '../test-utils/createExtension.js';
import { EXTENSIONS_DIRECTORY_NAME } from './extensions/variables.js';
@@ -52,10 +52,9 @@ describe('ExtensionManager agents loading', () => {
fs.mkdirSync(extensionsDir, { recursive: true });
extensionManager = new ExtensionManager({
settings: {
settings: createTestMergedSettings({
telemetry: { enabled: false },
trustedFolders: [tempDir],
} as unknown as Settings,
}),
requestConsent: vi.fn().mockResolvedValue(true),
requestSetting: vi.fn(),
workspaceDir: tempDir,

View File

@@ -9,7 +9,7 @@ import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';
import { ExtensionManager } from './extension-manager.js';
import type { Settings } from './settings.js';
import { createTestMergedSettings } from './settings.js';
import {
loadAgentsFromDirectory,
loadSkillsFromDir,
@@ -105,14 +105,10 @@ describe('ExtensionManager Settings Scope', () => {
workspaceDir: tempWorkspace,
requestConsent: async () => true,
requestSetting: async () => '',
settings: {
telemetry: {
enabled: false,
},
experimental: {
extensionConfig: true,
},
} as Settings,
settings: createTestMergedSettings({
telemetry: { enabled: false },
experimental: { extensionConfig: true },
}),
});
const extensions = await extensionManager.loadExtensions();
@@ -147,14 +143,10 @@ describe('ExtensionManager Settings Scope', () => {
workspaceDir: tempWorkspace,
requestConsent: async () => true,
requestSetting: async () => '',
settings: {
telemetry: {
enabled: false,
},
experimental: {
extensionConfig: true,
},
} as Settings,
settings: createTestMergedSettings({
telemetry: { enabled: false },
experimental: { extensionConfig: true },
}),
});
const extensions = await extensionManager.loadExtensions();
@@ -187,14 +179,10 @@ describe('ExtensionManager Settings Scope', () => {
workspaceDir: tempWorkspace,
requestConsent: async () => true,
requestSetting: async () => '',
settings: {
telemetry: {
enabled: false,
},
experimental: {
extensionConfig: true,
},
} as Settings,
settings: createTestMergedSettings({
telemetry: { enabled: false },
experimental: { extensionConfig: true },
}),
});
const extensions = await extensionManager.loadExtensions();

View File

@@ -10,7 +10,7 @@ import * as path from 'node:path';
import * as os from 'node:os';
import { ExtensionManager } from './extension-manager.js';
import { debugLogger, coreEvents } from '@google/gemini-cli-core';
import { type Settings } from './settings.js';
import { createTestMergedSettings } from './settings.js';
import { createExtension } from '../test-utils/createExtension.js';
import { EXTENSIONS_DIRECTORY_NAME } from './extensions/variables.js';
@@ -58,10 +58,9 @@ describe('ExtensionManager skills validation', () => {
fs.mkdirSync(extensionsDir, { recursive: true });
extensionManager = new ExtensionManager({
settings: {
settings: createTestMergedSettings({
telemetry: { enabled: false },
trustedFolders: [tempDir],
} as unknown as Settings,
}),
requestConsent: vi.fn().mockResolvedValue(true),
requestSetting: vi.fn(),
workspaceDir: tempDir,
@@ -134,10 +133,9 @@ describe('ExtensionManager skills validation', () => {
// 3. Create a fresh ExtensionManager to force loading from disk
const newExtensionManager = new ExtensionManager({
settings: {
settings: createTestMergedSettings({
telemetry: { enabled: false },
trustedFolders: [tempDir],
} as unknown as Settings,
}),
requestConsent: vi.fn().mockResolvedValue(true),
requestSetting: vi.fn(),
workspaceDir: tempDir,

View File

@@ -9,7 +9,7 @@ import * as path from 'node:path';
import { stat } from 'node:fs/promises';
import chalk from 'chalk';
import { ExtensionEnablementManager } from './extensions/extensionEnablement.js';
import { type Settings, SettingScope } from './settings.js';
import { type MergedSettings, SettingScope } from './settings.js';
import { createHash, randomUUID } from 'node:crypto';
import { loadInstallMetadata, type ExtensionConfig } from './extension.js';
import {
@@ -68,11 +68,10 @@ import {
ExtensionSettingScope,
} from './extensions/extensionSettings.js';
import type { EventEmitter } from 'node:stream';
import { getEnableHooks } from './settingsSchema.js';
interface ExtensionManagerParams {
enabledExtensionOverrides?: string[];
settings: Settings;
settings: MergedSettings;
requestConsent: (consent: string) => Promise<boolean>;
requestSetting: ((setting: ExtensionSetting) => Promise<string>) | null;
workspaceDir: string;
@@ -86,7 +85,7 @@ interface ExtensionManagerParams {
*/
export class ExtensionManager extends ExtensionLoader {
private extensionEnablementManager: ExtensionEnablementManager;
private settings: Settings;
private settings: MergedSettings;
private requestConsent: (consent: string) => Promise<boolean>;
private requestSetting:
| ((setting: ExtensionSetting) => Promise<string>)
@@ -143,7 +142,7 @@ export class ExtensionManager extends ExtensionLoader {
if (
(installMetadata.type === 'git' ||
installMetadata.type === 'github-release') &&
this.settings.security?.blockGitExtensions
this.settings.security.blockGitExtensions
) {
throw new Error(
'Installing extensions from remote sources is disallowed by your current settings.',
@@ -287,10 +286,7 @@ Would you like to attempt to install via "git clone" instead?`,
}
await fs.promises.mkdir(destinationPath, { recursive: true });
if (
this.requestSetting &&
(this.settings.experimental?.extensionConfig ?? false)
) {
if (this.requestSetting && this.settings.experimental.extensionConfig) {
if (isUpdate) {
await maybePromptForSettings(
newExtensionConfig,
@@ -308,14 +304,13 @@ Would you like to attempt to install via "git clone" instead?`,
}
}
const missingSettings =
(this.settings.experimental?.extensionConfig ?? false)
? await getMissingSettings(
newExtensionConfig,
extensionId,
this.workspaceDir,
)
: [];
const missingSettings = this.settings.experimental.extensionConfig
? await getMissingSettings(
newExtensionConfig,
extensionId,
this.workspaceDir,
)
: [];
if (missingSettings.length > 0) {
const message = `Extension "${newExtensionConfig.name}" has missing settings: ${missingSettings
.map((s) => s.name)
@@ -478,7 +473,7 @@ Would you like to attempt to install via "git clone" instead?`,
throw new Error('Extensions already loaded, only load extensions once.');
}
if (this.settings.admin?.extensions?.enabled === false) {
if (this.settings.admin.extensions.enabled === false) {
this.loadedExtensions = [];
return this.loadedExtensions;
}
@@ -511,7 +506,7 @@ Would you like to attempt to install via "git clone" instead?`,
if (
(installMetadata?.type === 'git' ||
installMetadata?.type === 'github-release') &&
this.settings.security?.blockGitExtensions
this.settings.security.blockGitExtensions
) {
return null;
}
@@ -535,7 +530,7 @@ Would you like to attempt to install via "git clone" instead?`,
let userSettings: Record<string, string> = {};
let workspaceSettings: Record<string, string> = {};
if (this.settings.experimental?.extensionConfig ?? false) {
if (this.settings.experimental.extensionConfig) {
userSettings = await getScopedEnvContents(
config,
extensionId,
@@ -553,10 +548,7 @@ Would you like to attempt to install via "git clone" instead?`,
config = resolveEnvVarsInObject(config, customEnv);
const resolvedSettings: ResolvedExtensionSetting[] = [];
if (
config.settings &&
(this.settings.experimental?.extensionConfig ?? false)
) {
if (config.settings && this.settings.experimental.extensionConfig) {
for (const setting of config.settings) {
const value = customEnv[setting.envVar];
let scope: 'user' | 'workspace' | undefined;
@@ -600,7 +592,7 @@ Would you like to attempt to install via "git clone" instead?`,
}
if (config.mcpServers) {
if (this.settings.admin?.mcp?.enabled === false) {
if (this.settings.admin.mcp.enabled === false) {
config.mcpServers = undefined;
} else {
config.mcpServers = Object.fromEntries(
@@ -619,7 +611,7 @@ Would you like to attempt to install via "git clone" instead?`,
.filter((contextFilePath) => fs.existsSync(contextFilePath));
let hooks: { [K in HookEventName]?: HookDefinition[] } | undefined;
if (getEnableHooks(this.settings)) {
if (this.settings.tools.enableHooks && this.settings.hooks.enabled) {
hooks = await this.loadExtensionHooks(effectiveExtensionPath, {
extensionPath: effectiveExtensionPath,
workspacePath: this.workspaceDir,

View File

@@ -26,7 +26,11 @@ import {
loadAgentsFromDirectory,
loadSkillsFromDir,
} from '@google/gemini-cli-core';
import { loadSettings, SettingScope } from './settings.js';
import {
loadSettings,
createTestMergedSettings,
SettingScope,
} from './settings.js';
import {
isWorkspaceTrusted,
resetTrustedFoldersForTesting,
@@ -201,7 +205,7 @@ describe('extension tests', () => {
});
vi.spyOn(process, 'cwd').mockReturnValue(tempWorkspaceDir);
const settings = loadSettings(tempWorkspaceDir).merged;
(settings.experimental ??= {}).extensionConfig = true;
settings.experimental.extensionConfig = true;
extensionManager = new ExtensionManager({
workspaceDir: tempWorkspaceDir,
requestConsent: mockRequestConsent,
@@ -628,11 +632,9 @@ describe('extension tests', () => {
},
});
const blockGitExtensionsSetting = {
security: {
blockGitExtensions: true,
},
};
const blockGitExtensionsSetting = createTestMergedSettings({
security: { blockGitExtensions: true },
});
extensionManager = new ExtensionManager({
workspaceDir: tempWorkspaceDir,
requestConsent: mockRequestConsent,
@@ -652,7 +654,6 @@ describe('extension tests', () => {
version: '1.0.0',
});
const loadedSettings = loadSettings(tempWorkspaceDir).merged;
(loadedSettings.admin ??= {}).extensions ??= {};
loadedSettings.admin.extensions.enabled = false;
extensionManager = new ExtensionManager({
@@ -676,7 +677,6 @@ describe('extension tests', () => {
},
});
const loadedSettings = loadSettings(tempWorkspaceDir).merged;
(loadedSettings.admin ??= {}).mcp ??= {};
loadedSettings.admin.mcp.enabled = false;
extensionManager = new ExtensionManager({
@@ -701,7 +701,6 @@ describe('extension tests', () => {
},
});
const loadedSettings = loadSettings(tempWorkspaceDir).merged;
(loadedSettings.admin ??= {}).mcp ??= {};
loadedSettings.admin.mcp.enabled = true;
extensionManager = new ExtensionManager({
@@ -837,7 +836,6 @@ describe('extension tests', () => {
);
const settings = loadSettings(tempWorkspaceDir).merged;
if (!settings.hooks) settings.hooks = {};
settings.hooks.enabled = true;
extensionManager = new ExtensionManager({
@@ -873,7 +871,6 @@ describe('extension tests', () => {
);
const settings = loadSettings(tempWorkspaceDir).merged;
if (!settings.hooks) settings.hooks = {};
settings.hooks.enabled = false;
extensionManager = new ExtensionManager({
@@ -1098,11 +1095,9 @@ describe('extension tests', () => {
it('should not install a github extension if blockGitExtensions is set', async () => {
const gitUrl = 'https://somehost.com/somerepo.git';
const blockGitExtensionsSetting = {
security: {
blockGitExtensions: true,
},
};
const blockGitExtensionsSetting = createTestMergedSettings({
security: { blockGitExtensions: true },
});
extensionManager = new ExtensionManager({
workspaceDir: tempWorkspaceDir,
requestConsent: mockRequestConsent,

View File

@@ -11,7 +11,6 @@ import * as fs from 'node:fs';
import { getMissingSettings } from './extensionSettings.js';
import type { ExtensionConfig } from '../extension.js';
import { ExtensionStorage } from './storage.js';
import type { Settings } from '../settings.js';
import {
KeychainTokenStorage,
debugLogger,
@@ -21,6 +20,7 @@ import {
} from '@google/gemini-cli-core';
import { EXTENSION_SETTINGS_FILENAME } from './variables.js';
import { ExtensionManager } from '../extension-manager.js';
import { createTestMergedSettings } from '../settings.js';
vi.mock('node:fs', async (importOriginal) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -247,12 +247,10 @@ describe('extensionUpdates', () => {
const manager = new ExtensionManager({
workspaceDir: tempWorkspaceDir,
settings: {
telemetry: {
enabled: false,
},
settings: createTestMergedSettings({
telemetry: { enabled: false },
experimental: { extensionConfig: true },
} as unknown as Settings,
}),
requestConsent: vi.fn().mockResolvedValue(true),
requestSetting: null, // Simulate non-interactive
});

View File

@@ -24,12 +24,24 @@ import { DefaultDark } from '../ui/themes/default.js';
import { isWorkspaceTrusted } from './trustedFolders.js';
import {
type Settings,
type MergedSettings,
type MemoryImportFormat,
type MergeStrategy,
type SettingsSchema,
type SettingDefinition,
getSettingsSchema,
} from './settingsSchema.js';
export {
type Settings,
type MergedSettings,
type MemoryImportFormat,
type MergeStrategy,
type SettingsSchema,
type SettingDefinition,
getSettingsSchema,
};
import { resolveEnvVarsInObject } from '../utils/envVarResolver.js';
import { customDeepMerge } from '../utils/deepMerge.js';
import { updateSettingsFilePreservingFormat } from '../utils/commentJson.js';
@@ -59,8 +71,6 @@ function getMergeStrategyForPath(path: string[]): MergeStrategy | undefined {
return current?.mergeStrategy;
}
export type { Settings, MemoryImportFormat };
export const USER_SETTINGS_PATH = Storage.getGlobalSettingsPath();
export const USER_SETTINGS_DIR = path.dirname(USER_SETTINGS_PATH);
export const DEFAULT_EXCLUDED_ENV_VARS = ['DEBUG', 'DEBUG_MODE'];
@@ -201,10 +211,7 @@ export function getDefaultsFromSchema(
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;
}
defaults[key] = getDefaultsFromSchema(definition.properties);
} else if (definition.default !== undefined) {
defaults[key] = definition.default;
}
@@ -212,13 +219,13 @@ export function getDefaultsFromSchema(
return defaults as Settings;
}
function mergeSettings(
export function mergeSettings(
system: Settings,
systemDefaults: Settings,
user: Settings,
workspace: Settings,
isTrusted: boolean,
): Settings {
): MergedSettings {
const safeWorkspace = isTrusted ? workspace : ({} as Settings);
const schemaDefaults = getDefaultsFromSchema();
@@ -236,7 +243,24 @@ function mergeSettings(
user,
safeWorkspace,
system,
) as Settings;
) as MergedSettings;
}
/**
* Creates a fully populated MergedSettings object for testing purposes.
* It merges the provided overrides with the default settings from the schema.
*
* @param overrides Partial settings to override the defaults.
* @returns A complete MergedSettings object.
*/
export function createTestMergedSettings(
overrides: Partial<Settings> = {},
): MergedSettings {
return customDeepMerge(
getMergeStrategyForPath,
getDefaultsFromSchema(),
overrides,
) as MergedSettings;
}
export class LoadedSettings {
@@ -264,14 +288,14 @@ export class LoadedSettings {
readonly isTrusted: boolean;
readonly errors: SettingsError[];
private _merged: Settings;
private _merged: MergedSettings;
private _remoteAdminSettings: Partial<Settings> | undefined;
get merged(): Settings {
get merged(): MergedSettings {
return this._merged;
}
private computeMergedSettings(): Settings {
private computeMergedSettings(): MergedSettings {
const merged = mergeSettings(
this.system.settings,
this.systemDefaults.settings,
@@ -293,7 +317,7 @@ export class LoadedSettings {
(path: string[]) => getMergeStrategyForPath(['admin', ...path]),
adminDefaults,
this._remoteAdminSettings?.admin ?? {},
) as Settings['admin'];
) as MergedSettings['admin'];
}
return merged;
}

View File

@@ -14,6 +14,7 @@ import type {
BugCommandSettings,
TelemetrySettings,
AuthType,
AgentOverride,
} from '@google/gemini-cli-core';
import {
DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES,
@@ -799,7 +800,7 @@ const SETTINGS_SCHEMA = {
label: 'Agent Overrides',
category: 'Advanced',
requiresRestart: true,
default: {},
default: {} as Record<string, AgentOverride>,
description:
'Override settings for specific agents, e.g. to disable the agent, set a custom model config, or run config.',
showInDialog: false,
@@ -2262,12 +2263,17 @@ type InferSettings<T extends SettingsSchema> = {
: T[K]['default'];
};
type InferMergedSettings<T extends SettingsSchema> = {
-readonly [K in keyof T]-?: T[K] extends { properties: SettingsSchema }
? InferMergedSettings<T[K]['properties']>
: T[K]['type'] extends 'enum'
? T[K]['options'] extends readonly SettingEnumOption[]
? T[K]['options'][number]['value']
: T[K]['default']
: T[K]['default'] extends boolean
? boolean
: T[K]['default'];
};
export type Settings = InferSettings<SettingsSchemaType>;
export function getEnableHooksUI(settings: Settings): boolean {
return settings.tools?.enableHooks ?? true;
}
export function getEnableHooks(settings: Settings): boolean {
return getEnableHooksUI(settings) && (settings.hooks?.enabled ?? false);
}
export type MergedSettings = InferMergedSettings<SettingsSchemaType>;