diff --git a/packages/cli/src/config/settings.test.ts b/packages/cli/src/config/settings.test.ts index eb7e991e6b..146383b923 100644 --- a/packages/cli/src/config/settings.test.ts +++ b/packages/cli/src/config/settings.test.ts @@ -3005,6 +3005,44 @@ describe('Settings Loading and Merging', () => { expect(snap1).toBe(snap2); }); + it('getSnapshot() should preserve readOnly metadata for each scope', () => { + const readonlySettings = new LoadedSettings( + { + path: getSystemSettingsPath(), + settings: {}, + originalSettings: {}, + readOnly: true, + }, + { + path: getSystemDefaultsPath(), + settings: {}, + originalSettings: {}, + readOnly: true, + }, + { + path: USER_SETTINGS_PATH, + settings: {}, + originalSettings: {}, + readOnly: false, + }, + { + path: MOCK_WORKSPACE_SETTINGS_PATH, + settings: {}, + originalSettings: {}, + readOnly: true, + }, + true, + [], + ); + + const snapshot = readonlySettings.getSnapshot(); + + expect(snapshot.system.readOnly).toBe(true); + expect(snapshot.systemDefaults.readOnly).toBe(true); + expect(snapshot.user.readOnly).toBe(false); + expect(snapshot.workspace.readOnly).toBe(true); + }); + it('setValue() should create a new snapshot reference and emit event', () => { const oldSnapshot = loadedSettings.getSnapshot(); const oldUserRef = oldSnapshot.user.settings; diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index cd6b3c61cb..7fba90eb83 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -398,8 +398,7 @@ export class LoadedSettings { private computeSnapshot(): LoadedSettingsSnapshot { const cloneSettingsFile = (file: SettingsFile): SettingsFile => ({ - path: file.path, - rawJson: file.rawJson, + ...file, settings: structuredClone(file.settings), originalSettings: structuredClone(file.originalSettings), }); diff --git a/packages/cli/src/test-utils/settings.ts b/packages/cli/src/test-utils/settings.ts index 20d0613f83..9c97e987b1 100644 --- a/packages/cli/src/test-utils/settings.ts +++ b/packages/cli/src/test-utils/settings.ts @@ -16,6 +16,7 @@ export interface MockSettingsFile { settings: any; originalSettings: any; path: string; + readOnly?: boolean; } interface CreateMockSettingsOptions { diff --git a/packages/cli/src/ui/components/SettingsDialog.test.tsx b/packages/cli/src/ui/components/SettingsDialog.test.tsx index cd16572ddc..53c14e5a0f 100644 --- a/packages/cli/src/ui/components/SettingsDialog.test.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.test.tsx @@ -25,7 +25,10 @@ import { waitFor } from '../../test-utils/async.js'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { SettingsDialog } from './SettingsDialog.js'; import { SettingScope } from '../../config/settings.js'; -import { createMockSettings } from '../../test-utils/settings.js'; +import { + createMockSettings, + type MockSettingsFile, +} from '../../test-utils/settings.js'; import { makeFakeConfig } from '@google/gemini-cli-core'; import { act } from 'react'; import { TEST_ONLY } from '../../utils/settingsUtils.js'; @@ -250,6 +253,17 @@ const renderDialog = async ( }, ); +const createSettingsFile = ( + path: string, + settings: Record = {}, + readOnly?: boolean, +): MockSettingsFile => ({ + settings, + originalSettings: settings, + path, + readOnly, +}); + describe('SettingsDialog', () => { beforeEach(() => { vi.clearAllMocks(); @@ -618,6 +632,131 @@ describe('SettingsDialog', () => { unmount(); }); + + it('should not offer read-only system settings as an editable target', async () => { + const settings = createMockSettings({ + system: createSettingsFile('', {}, true), + }); + const onSelect = vi.fn(); + + const { lastFrame, unmount } = await renderDialog(settings, onSelect); + + await waitFor(() => { + expect(lastFrame()).toContain('Apply To'); + }); + + const output = lastFrame(); + expect(output).toContain('User Settings'); + expect(output).toContain('Workspace Settings'); + expect(output).not.toContain('System Settings'); + + unmount(); + }); + + it('should not offer a read-only home-directory workspace as an editable target', async () => { + const settings = createMockSettings({ + user: createSettingsFile('/mock/home/.gemini/settings.json'), + system: createSettingsFile('/mock/system/settings.json', {}, true), + systemDefaults: createSettingsFile( + '/mock/system-defaults/settings.json', + {}, + true, + ), + workspace: createSettingsFile('', {}, true), + }); + const setValueSpy = vi.spyOn(settings, 'setValue'); + const onSelect = vi.fn(); + + const { lastFrame, stdin, unmount, waitUntilReady } = await renderDialog( + settings, + onSelect, + ); + + await waitFor(() => { + expect(lastFrame()).toContain('Vim Mode'); + }); + + const output = lastFrame(); + expect(output).not.toContain('Workspace Settings'); + expect(output).not.toContain('System Settings'); + + await act(async () => { + stdin.write(TerminalKeys.ENTER as string); + }); + await waitUntilReady(); + + expect(setValueSpy).toHaveBeenCalledWith( + SettingScope.User, + 'general.vimMode', + true, + ); + + unmount(); + }); + + it('should fall back to the first writable scope when the selected scope is read-only', async () => { + const settings = createMockSettings({ + user: createSettingsFile('', {}, true), + system: createSettingsFile('', {}, true), + workspace: createSettingsFile('/mock/workspace/.gemini/settings.json'), + }); + const setValueSpy = vi.spyOn(settings, 'setValue'); + const onSelect = vi.fn(); + + const { lastFrame, stdin, unmount, waitUntilReady } = await renderDialog( + settings, + onSelect, + ); + + await waitFor(() => { + expect(lastFrame()).toContain('Vim Mode'); + }); + + await act(async () => { + stdin.write(TerminalKeys.ENTER as string); + }); + await waitUntilReady(); + + expect(setValueSpy).toHaveBeenCalledWith( + SettingScope.Workspace, + 'general.vimMode', + true, + ); + + unmount(); + }); + + it('should not save when all editable scopes are read-only', async () => { + const settings = createMockSettings({ + user: createSettingsFile('', {}, true), + system: createSettingsFile('', {}, true), + workspace: createSettingsFile( + '/mock/workspace/.gemini/settings.json', + {}, + true, + ), + }); + const setValueSpy = vi.spyOn(settings, 'setValue'); + const onSelect = vi.fn(); + + const { lastFrame, stdin, unmount, waitUntilReady } = await renderDialog( + settings, + onSelect, + ); + + await waitFor(() => { + expect(lastFrame()).toContain('Vim Mode'); + }); + + await act(async () => { + stdin.write(TerminalKeys.ENTER as string); + }); + await waitUntilReady(); + + expect(setValueSpy).not.toHaveBeenCalled(); + + unmount(); + }); }); describe('Restart Prompt', () => { diff --git a/packages/cli/src/ui/components/SettingsDialog.tsx b/packages/cli/src/ui/components/SettingsDialog.tsx index 994bde6ed3..c358791d00 100644 --- a/packages/cli/src/ui/components/SettingsDialog.tsx +++ b/packages/cli/src/ui/components/SettingsDialog.tsx @@ -15,7 +15,10 @@ import { type LoadableSettingScope, type Settings, } from '../../config/settings.js'; -import { getScopeMessageForSetting } from '../../utils/dialogScopeUtils.js'; +import { + getScopeItems, + getScopeMessageForSetting, +} from '../../utils/dialogScopeUtils.js'; import { getDialogSettingKeys, getDisplayValue, @@ -105,6 +108,26 @@ export function SettingsDialog({ const [selectedScope, setSelectedScope] = useState( SettingScope.User, ); + const editableScopeItems = useMemo( + () => + getScopeItems().filter((item) => { + const settingsFile = settings.forScope(item.value); + return ( + settingsFile.readOnly !== true && + (item.value !== SettingScope.Workspace || + settingsFile.path !== undefined) + ); + }), + [settings], + ); + const writableSelectedScope = + editableScopeItems.find((item) => item.value === selectedScope)?.value ?? + editableScopeItems[0]?.value; + const effectiveSelectedScope = writableSelectedScope ?? SettingScope.User; + + if (writableSelectedScope && selectedScope !== writableSelectedScope) { + setSelectedScope(writableSelectedScope); + } // Snapshot restart-required values at mount time for diff tracking const [activeRestartRequiredSettings] = useState(() => @@ -205,7 +228,7 @@ export function SettingsDialog({ const scopeMessage = getScopeMessageForSetting( key, - selectedScope, + effectiveSelectedScope, settings, ); const label = def.label || key; @@ -218,7 +241,7 @@ export function SettingsDialog({ max = Math.max(max, lWidth, dWidth); } return max; - }, [selectedScope, settings]); + }, [effectiveSelectedScope, settings]); // Search input buffer const searchBuffer = useSearchBuffer({ @@ -229,7 +252,7 @@ export function SettingsDialog({ // Generate items for BaseSettingsDialog const settingKeys = searchQuery ? filteredKeys : getDialogSettingKeys(); const items: SettingsDialogItem[] = useMemo(() => { - const scopeSettings = settings.forScope(selectedScope).settings; + const scopeSettings = settings.forScope(effectiveSelectedScope).settings; const mergedSettings = settings.merged; return settingKeys.map((key) => { @@ -242,7 +265,7 @@ export function SettingsDialog({ // Get the scope message (e.g., "(Modified in Workspace)") const scopeMessage = getScopeMessageForSetting( key, - selectedScope, + effectiveSelectedScope, settings, ); @@ -266,7 +289,7 @@ export function SettingsDialog({ editValue, }; }); - }, [settingKeys, selectedScope, settings]); + }, [settingKeys, effectiveSelectedScope, settings]); const handleScopeChange = useCallback((scope: LoadableSettingScope) => { setSelectedScope(scope); @@ -275,12 +298,16 @@ export function SettingsDialog({ // Toggle handler for boolean/enum settings const handleItemToggle = useCallback( (key: string, _item: SettingsDialogItem) => { + if (!writableSelectedScope) { + return; + } + const definition = getSettingDefinition(key); if (!TOGGLE_TYPES.has(definition?.type)) { return; } - const scopeSettings = settings.forScope(selectedScope).settings; + const scopeSettings = settings.forScope(writableSelectedScope).settings; const currentValue = getEffectiveValue(key, scopeSettings); let newValue: SettingsValue; @@ -310,14 +337,18 @@ export function SettingsDialog({ `[DEBUG SettingsDialog] Saving ${key} immediately with value:`, newValue, ); - setSetting(selectedScope, key, newValue); + setSetting(writableSelectedScope, key, newValue); }, - [settings, selectedScope, setSetting], + [settings, writableSelectedScope, setSetting], ); // For inline editor const handleEditCommit = useCallback( (key: string, newValue: string, _item: SettingsDialogItem) => { + if (!writableSelectedScope) { + return; + } + const definition = getSettingDefinition(key); const type: SettingsType = definition?.type ?? 'string'; const parsed = parseEditedValue(type, newValue); @@ -326,22 +357,26 @@ export function SettingsDialog({ return; } - setSetting(selectedScope, key, parsed); + setSetting(writableSelectedScope, key, parsed); }, - [selectedScope, setSetting], + [writableSelectedScope, setSetting], ); // Clear/reset handler - removes the value from settings.json so it falls back to default const handleItemClear = useCallback( (key: string, _item: SettingsDialogItem) => { - setSetting(selectedScope, key, undefined); + if (!writableSelectedScope) { + return; + } + + setSetting(writableSelectedScope, key, undefined); }, - [selectedScope, setSetting], + [writableSelectedScope, setSetting], ); const handleClose = useCallback(() => { - onSelect(undefined, selectedScope as SettingScope); - }, [onSelect, selectedScope]); + onSelect(undefined, effectiveSelectedScope as SettingScope); + }, [onSelect, effectiveSelectedScope]); const globalKeyMatchers = useKeyMatchers(); const settingsKeyMatchers = useMemo( @@ -369,7 +404,6 @@ export function SettingsDialog({ ); // Decisions on what features to enable - const hasWorkspace = settings.workspace.path !== undefined; const showSearch = !showRestartPrompt; return ( @@ -379,8 +413,9 @@ export function SettingsDialog({ searchEnabled={showSearch} searchBuffer={searchBuffer} items={items} - showScopeSelector={hasWorkspace} - selectedScope={selectedScope} + showScopeSelector={editableScopeItems.length > 1} + scopeItems={editableScopeItems} + selectedScope={effectiveSelectedScope} onScopeChange={handleScopeChange} maxItemsToShow={MAX_ITEMS_TO_SHOW} availableHeight={availableTerminalHeight} diff --git a/packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx b/packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx index 25dbb1d8b4..b9cf2418fa 100644 --- a/packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx +++ b/packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx @@ -73,6 +73,11 @@ export interface BaseSettingsDialogProps { // Scope selector /** Whether to show the scope selector. Default: true */ showScopeSelector?: boolean; + /** Editable scope items to display. Defaults to all loadable scopes. */ + scopeItems?: Array<{ + label: string; + value: LoadableSettingScope; + }>; /** Currently selected scope */ selectedScope: LoadableSettingScope; /** Callback when scope changes */ @@ -128,6 +133,7 @@ export function BaseSettingsDialog({ searchBuffer, items, showScopeSelector = true, + scopeItems: providedScopeItems, selectedScope, onScopeChange, maxItemsToShow, @@ -143,9 +149,19 @@ export function BaseSettingsDialog({ }: BaseSettingsDialogProps): React.JSX.Element { const globalKeyMatchers = useKeyMatchers(); const keyMatchers = customKeyMatchers ?? globalKeyMatchers; + const scopeItems = useMemo( + () => + (providedScopeItems ?? getScopeItems()).map((item) => ({ + ...item, + key: item.value, + })), + [providedScopeItems], + ); + const showAvailableScopes = showScopeSelector && scopeItems.length > 1; + // Calculate effective max items and scope visibility based on terminal height const { effectiveMaxItemsToShow, finalShowScopeSelector } = useMemo(() => { - const initialShowScope = showScopeSelector; + const initialShowScope = showAvailableScopes; const initialMaxItems = maxItemsToShow; if (!availableHeight) { @@ -214,7 +230,7 @@ export function BaseSettingsDialog({ maxItemsToShow, items.length, searchEnabled, - showScopeSelector, + showAvailableScopes, footer, ]); @@ -248,12 +264,6 @@ export function BaseSettingsDialog({ ? 'settings' : focusSection; - // Scope selector items - const scopeItems = getScopeItems().map((item) => ({ - ...item, - key: item.value, - })); - // Calculate visible items based on scroll offset const visibleItems = items.slice( windowStart,