mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-14 13:53:02 -07:00
fix(cli): hide read-only settings scopes (#26249)
This commit is contained in:
@@ -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<string, unknown> = {},
|
||||
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', () => {
|
||||
|
||||
@@ -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<LoadableSettingScope>(
|
||||
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}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user