Merge branch 'main' into refactor/audit-shared-helpers

This commit is contained in:
qwen-code-dev-bot 2026-08-19 01:57:58 +08:00 committed by GitHub
commit bc3df79d47
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 1 additions and 517 deletions

View file

@ -7,33 +7,20 @@
import { describe, it, expect } from 'vitest';
import {
// Schema utilities
getSettingsByCategory,
getSettingDefinition,
requiresRestart,
getDefaultValue,
getRestartRequiredSettings,
getEffectiveValue,
getAllSettingKeys,
getSettingsByType,
getSettingsRequiringRestart,
isValidSettingKey,
getSettingCategory,
shouldShowInDialog,
getDialogSettingsByCategory,
getDialogSettingsByType,
getDialogSettingKeys,
// Business logic utilities
getSettingValue,
isSettingModified,
TEST_ONLY,
settingExistsInScope,
setPendingSettingValue,
hasRestartRequiredSettings,
getRestartRequiredFromModified,
getDisplayValue,
isDefaultValue,
isValueInherited,
getEffectiveDisplayValue,
setNestedPropertySafe,
setNestedPropertyForce,
validateSettingValue,
@ -212,24 +199,6 @@ describe('SettingsUtils', () => {
});
describe('Schema Utilities', () => {
describe('getSettingsByCategory', () => {
it('should group settings by category', () => {
const categories = getSettingsByCategory();
expect(categories).toHaveProperty('Advanced');
expect(categories).toHaveProperty('Basic');
});
it('should include key property in grouped settings', () => {
const categories = getSettingsByCategory();
Object.entries(categories).forEach(([_category, settings]) => {
settings.forEach((setting) => {
expect(setting.key).toBeDefined();
});
});
});
});
describe('getSettingDefinition', () => {
it('should return definition for valid setting', () => {
const definition = getSettingDefinition('ui.theme');
@ -397,133 +366,6 @@ describe('SettingsUtils', () => {
});
});
describe('getSettingsByType', () => {
it('should return only boolean settings', () => {
const booleanSettings = getSettingsByType('boolean');
expect(booleanSettings.length).toBeGreaterThan(0);
booleanSettings.forEach((setting) => {
expect(setting.type).toBe('boolean');
});
});
});
describe('getSettingsRequiringRestart', () => {
it('should return only settings that require restart', () => {
const restartSettings = getSettingsRequiringRestart();
expect(restartSettings.length).toBeGreaterThan(0);
restartSettings.forEach((setting) => {
expect(setting.requiresRestart).toBe(true);
});
});
});
describe('isValidSettingKey', () => {
it('should return true for valid setting keys', () => {
expect(isValidSettingKey('ui.requiresRestart')).toBe(true);
expect(isValidSettingKey('ui.accessibility.enableLoadingPhrases')).toBe(
true,
);
});
it('should return false for invalid setting keys', () => {
expect(isValidSettingKey('invalidSetting')).toBe(false);
expect(isValidSettingKey('')).toBe(false);
});
});
describe('getSettingCategory', () => {
it('should return correct category for valid settings', () => {
expect(getSettingCategory('ui.requiresRestart')).toBe('UI');
expect(
getSettingCategory('ui.accessibility.enableLoadingPhrases'),
).toBe('UI');
});
it('should return undefined for invalid settings', () => {
expect(getSettingCategory('invalidSetting')).toBeUndefined();
});
});
describe('shouldShowInDialog', () => {
it('should return true for settings marked to show in dialog', () => {
expect(shouldShowInDialog('ui.requiresRestart')).toBe(true);
expect(shouldShowInDialog('general.vimMode')).toBe(true);
expect(shouldShowInDialog('ui.hideWindowTitle')).toBe(true);
});
it('should return false for settings marked to hide from dialog', () => {
expect(shouldShowInDialog('ui.theme')).toBe(false);
});
it('should return true for invalid settings (default behavior)', () => {
expect(shouldShowInDialog('invalidSetting')).toBe(true);
});
});
describe('getDialogSettingsByCategory', () => {
it('should only return settings marked for dialog display', async () => {
const categories = getDialogSettingsByCategory();
// Should include UI settings that are marked for dialog
expect(categories['UI']).toBeDefined();
const uiSettings = categories['UI'];
const uiKeys = uiSettings.map((s) => s.key);
expect(uiKeys).toContain('ui.requiresRestart');
expect(uiKeys).toContain('ui.accessibility.enableLoadingPhrases');
expect(uiKeys).not.toContain('ui.theme'); // This is now marked false
});
it('should not include Advanced category settings', () => {
const categories = getDialogSettingsByCategory();
// Advanced settings should be filtered out
expect(categories['Advanced']).toBeUndefined();
});
it('should include settings with showInDialog=true', () => {
const categories = getDialogSettingsByCategory();
const allSettings = Object.values(categories).flat();
const allKeys = allSettings.map((s) => s.key);
expect(allKeys).toContain('test');
expect(allKeys).toContain('ui.requiresRestart');
expect(allKeys).not.toContain('ui.theme'); // Now hidden
expect(allKeys).not.toContain('general.preferredEditor'); // Now hidden
});
});
describe('getDialogSettingsByType', () => {
it('should return only boolean dialog settings', () => {
const booleanSettings = getDialogSettingsByType('boolean');
const keys = booleanSettings.map((s) => s.key);
expect(keys).toContain('ui.requiresRestart');
expect(keys).toContain('ui.accessibility.enableLoadingPhrases');
expect(keys).not.toContain('privacy.usageStatisticsEnabled');
expect(keys).not.toContain('security.auth.selectedType'); // Advanced setting
expect(keys).not.toContain('security.auth.useExternal'); // Advanced setting
});
it('should return only string dialog settings', () => {
const stringSettings = getDialogSettingsByType('string');
const keys = stringSettings.map((s) => s.key);
// Note: theme and preferredEditor are now hidden from dialog
expect(keys).not.toContain('ui.theme'); // Now marked false
expect(keys).not.toContain('general.preferredEditor'); // Now marked false
expect(keys).not.toContain('security.auth.selectedType'); // Advanced setting
// Check that user-facing tool settings are included
expect(keys).toContain('tools.shell.pager');
// Check that advanced/hidden tool settings are excluded
expect(keys).not.toContain('tools.discoveryCommand');
expect(keys).not.toContain('tools.callCommand');
expect(keys.every((key) => !key.startsWith('advanced.'))).toBe(true);
});
});
describe('getDialogSettingKeys', () => {
it('should return only settings marked for dialog display', () => {
const dialogKeys = getDialogSettingKeys();
@ -601,7 +443,7 @@ describe('SettingsUtils', () => {
expect(existsInPending).toBe(true);
// Get the value from pending settings
const valueFromPending = getSettingValue(
const valueFromPending = getEffectiveValue(
key,
updatedPendingSettings,
{},
@ -633,64 +475,6 @@ describe('SettingsUtils', () => {
});
describe('Business Logic Utilities', () => {
describe('getSettingValue', () => {
it('should return value from settings when set', () => {
const settings = makeMockSettings({ ui: { requiresRestart: true } });
const mergedSettings = makeMockSettings({
ui: { requiresRestart: false },
});
const value = getSettingValue(
'ui.requiresRestart',
settings,
mergedSettings,
);
expect(value).toBe(true);
});
it('should return value from merged settings when not set in current scope', () => {
const settings = makeMockSettings({});
const mergedSettings = makeMockSettings({
ui: { requiresRestart: true },
});
const value = getSettingValue(
'ui.requiresRestart',
settings,
mergedSettings,
);
expect(value).toBe(true);
});
it('should return default value for invalid setting', () => {
const settings = makeMockSettings({});
const mergedSettings = makeMockSettings({});
const value = getSettingValue(
'invalidSetting',
settings,
mergedSettings,
);
expect(value).toBe(false); // Default fallback
});
});
describe('isSettingModified', () => {
it('should return true when value differs from default', () => {
expect(isSettingModified('ui.requiresRestart', true)).toBe(true);
expect(
isSettingModified('ui.accessibility.enableLoadingPhrases', true),
).toBe(true);
});
it('should return false when value matches default', () => {
expect(isSettingModified('ui.requiresRestart', false)).toBe(false);
expect(
isSettingModified('ui.accessibility.enableLoadingPhrases', false),
).toBe(false);
});
});
describe('settingExistsInScope', () => {
it('should return true for top-level settings that exist', () => {
const settings = makeMockSettings({ ui: { requiresRestart: true } });
@ -781,26 +565,6 @@ describe('SettingsUtils', () => {
});
});
describe('hasRestartRequiredSettings', () => {
it('should return true when modified settings require restart', () => {
const modifiedSettings = new Set<string>([
'advanced.autoConfigureMemory',
'ui.requiresRestart',
]);
expect(hasRestartRequiredSettings(modifiedSettings)).toBe(true);
});
it('should return false when no modified settings require restart', () => {
const modifiedSettings = new Set<string>(['test']);
expect(hasRestartRequiredSettings(modifiedSettings)).toBe(false);
});
it('should return false for empty set', () => {
const modifiedSettings = new Set<string>();
expect(hasRestartRequiredSettings(modifiedSettings)).toBe(false);
});
});
describe('getRestartRequiredFromModified', () => {
it('should return only settings that require restart', () => {
const modifiedSettings = new Set<string>([
@ -1159,108 +923,6 @@ describe('SettingsUtils', () => {
expect(result).toBe(false);
});
});
describe('isValueInherited', () => {
it('should return false for top-level settings that exist in scope', () => {
const settings = makeMockSettings({ ui: { requiresRestart: true } });
const mergedSettings = makeMockSettings({
ui: { requiresRestart: true },
});
const result = isValueInherited(
'ui.requiresRestart',
settings,
mergedSettings,
);
expect(result).toBe(false);
});
it('should return true for top-level settings that do not exist in scope', () => {
const settings = makeMockSettings({});
const mergedSettings = makeMockSettings({
ui: { requiresRestart: true },
});
const result = isValueInherited(
'ui.requiresRestart',
settings,
mergedSettings,
);
expect(result).toBe(true);
});
it('should return false for nested settings that exist in scope', () => {
const settings = makeMockSettings({
ui: { accessibility: { enableLoadingPhrases: true } },
});
const mergedSettings = makeMockSettings({
ui: { accessibility: { enableLoadingPhrases: true } },
});
const result = isValueInherited(
'ui.accessibility.enableLoadingPhrases',
settings,
mergedSettings,
);
expect(result).toBe(false);
});
it('should return true for nested settings that do not exist in scope', () => {
const settings = makeMockSettings({});
const mergedSettings = makeMockSettings({
ui: { accessibility: { enableLoadingPhrases: true } },
});
const result = isValueInherited(
'ui.accessibility.enableLoadingPhrases',
settings,
mergedSettings,
);
expect(result).toBe(true);
});
});
describe('getEffectiveDisplayValue', () => {
it('should return value from settings when available', () => {
const settings = makeMockSettings({ ui: { requiresRestart: true } });
const mergedSettings = makeMockSettings({
ui: { requiresRestart: false },
});
const result = getEffectiveDisplayValue(
'ui.requiresRestart',
settings,
mergedSettings,
);
expect(result).toBe(true);
});
it('should return value from merged settings when not in scope', () => {
const settings = makeMockSettings({});
const mergedSettings = makeMockSettings({
ui: { requiresRestart: true },
});
const result = getEffectiveDisplayValue(
'ui.requiresRestart',
settings,
mergedSettings,
);
expect(result).toBe(true);
});
it('should return default value for undefined values', () => {
const settings = makeMockSettings({});
const mergedSettings = makeMockSettings({});
const result = getEffectiveDisplayValue(
'ui.requiresRestart',
settings,
mergedSettings,
);
expect(result).toBe(false); // Default value
});
});
});
});

View file

@ -13,7 +13,6 @@ import type {
import type {
SettingDefinition,
SettingsSchema,
SettingsType,
SettingsValue,
} from '../config/settingsSchema.js';
import { getSettingsSchema } from '../config/settingsSchema.js';
@ -53,29 +52,6 @@ function clearFlattenedSchema() {
_FLATTENED_SCHEMA = undefined;
}
/**
* Get all settings grouped by category
*/
export function getSettingsByCategory(): Record<
string,
Array<SettingDefinition & { key: string }>
> {
const categories: Record<
string,
Array<SettingDefinition & { key: string }>
> = {};
Object.values(getFlattenedSchema()).forEach((definition) => {
const category = definition.category;
if (!categories[category]) {
categories[category] = [];
}
categories[category].push(definition);
});
return categories;
}
/**
* Get a setting definition by key
*/
@ -175,88 +151,6 @@ export function getAllSettingKeys(): string[] {
return Object.keys(getFlattenedSchema());
}
/**
* Get settings by type
*/
export function getSettingsByType(
type: SettingsType,
): Array<SettingDefinition & { key: string }> {
return Object.values(getFlattenedSchema()).filter(
(definition) => definition.type === type,
);
}
/**
* Get settings that require restart
*/
export function getSettingsRequiringRestart(): Array<
SettingDefinition & {
key: string;
}
> {
return Object.values(getFlattenedSchema()).filter(
(definition) => definition.requiresRestart,
);
}
/**
* Validate if a setting key exists in the schema
*/
export function isValidSettingKey(key: string): boolean {
return key in getFlattenedSchema();
}
/**
* Get the category for a setting
*/
export function getSettingCategory(key: string): string | undefined {
return getFlattenedSchema()[key]?.category;
}
/**
* Check if a setting should be shown in the settings dialog
*/
export function shouldShowInDialog(key: string): boolean {
return getFlattenedSchema()[key]?.showInDialog ?? true; // Default to true for backward compatibility
}
/**
* Get all settings that should be shown in the dialog, grouped by category
*/
export function getDialogSettingsByCategory(): Record<
string,
Array<SettingDefinition & { key: string }>
> {
const categories: Record<
string,
Array<SettingDefinition & { key: string }>
> = {};
Object.values(getFlattenedSchema())
.filter((definition) => definition.showInDialog !== false)
.forEach((definition) => {
const category = definition.category;
if (!categories[category]) {
categories[category] = [];
}
categories[category].push(definition);
});
return categories;
}
/**
* Get settings by type that should be shown in the dialog
*/
export function getDialogSettingsByType(
type: SettingsType,
): Array<SettingDefinition & { key: string }> {
return Object.values(getFlattenedSchema()).filter(
(definition) =>
definition.type === type && definition.showInDialog !== false,
);
}
/**
* Explicit display order for settings shown in the Settings Dialog.
* Settings are ordered by importance and logical grouping:
@ -387,46 +281,6 @@ export function getDialogSettingKeys(): string[] {
// BUSINESS LOGIC UTILITIES (Higher-level utilities for setting operations)
// ============================================================================
/**
* Get the current value for a setting in a specific scope
* Always returns a value (never undefined) - falls back to default if not set anywhere
*/
export function getSettingValue(
key: string,
settings: Settings,
mergedSettings: Settings,
): boolean {
const definition = getSettingDefinition(key);
if (!definition) {
return false; // Default fallback for invalid settings
}
const value = getEffectiveValue(key, settings, mergedSettings);
// Ensure we return a boolean value, converting from the more general type
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
}
/**
* Check if a setting value is modified from its default
*/
export function isSettingModified(key: string, value: boolean): boolean {
const defaultValue = getDefaultValue(key);
// Handle type comparison properly
if (typeof defaultValue === 'boolean') {
return value !== defaultValue;
}
// If default is not a boolean, consider it modified if value is true
return value === true;
}
/**
* Check if a setting exists in the original settings file for a scope
*/
@ -554,15 +408,6 @@ export function setPendingSettingValueAny(
return newSettings;
}
/**
* Check if any modified settings require a restart
*/
export function hasRestartRequiredSettings(
modifiedSettings: Set<string>,
): boolean {
return Array.from(modifiedSettings).some((key) => requiresRestart(key));
}
/**
* Get the restart required settings from a set of modified settings
*/
@ -673,29 +518,6 @@ export function isDefaultValue(key: string, settings: Settings): boolean {
return !settingExistsInScope(key, settings);
}
/**
* Check if a setting value is inherited (not set at current scope)
*/
export function isValueInherited(
key: string,
settings: Settings,
_mergedSettings: Settings,
): boolean {
return !settingExistsInScope(key, settings);
}
/**
* Get the effective value for display, considering inheritance
* Always returns a boolean value (never undefined)
*/
export function getEffectiveDisplayValue(
key: string,
settings: Settings,
mergedSettings: Settings,
): boolean {
return getSettingValue(key, settings, mergedSettings);
}
/**
* Backup a settings file before modification.
* Always creates a fresh backup with `.orig` suffix (overwrites any stale backup).