Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions packages/cli/src/config/settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2512,6 +2512,50 @@ describe('Settings Loading and Merging', () => {
});
});

describe('Reactivity & Snapshots', () => {
let loadedSettings: LoadedSettings;

beforeEach(() => {
const emptySettingsFile: SettingsFile = {
path: '/mock/path',
settings: {},
originalSettings: {},
};

loadedSettings = new LoadedSettings(
{ ...emptySettingsFile, path: getSystemSettingsPath() },
{ ...emptySettingsFile, path: getSystemDefaultsPath() },
{ ...emptySettingsFile, path: USER_SETTINGS_PATH },
{ ...emptySettingsFile, path: MOCK_WORKSPACE_SETTINGS_PATH },
true, // isTrusted
[],
);
});

it('getSnapshot() should return stable reference if no changes occur', () => {
const snap1 = loadedSettings.getSnapshot();
const snap2 = loadedSettings.getSnapshot();
expect(snap1).toBe(snap2);
});

it('setValue() should create a new snapshot reference and emit event', () => {
const oldSnapshot = loadedSettings.getSnapshot();
const oldUserRef = oldSnapshot.user.settings;

loadedSettings.setValue(SettingScope.User, 'ui.theme', 'high-contrast');

const newSnapshot = loadedSettings.getSnapshot();

expect(newSnapshot).not.toBe(oldSnapshot);
expect(newSnapshot.user.settings).not.toBe(oldUserRef);
expect(newSnapshot.user.settings.ui?.theme).toBe('high-contrast');

expect(newSnapshot.system.settings).not.toBe(oldSnapshot.system.settings);

expect(mockCoreEvents.emitSettingsChanged).toHaveBeenCalled();
});
});

describe('Security and Sandbox', () => {
let originalArgv: string[];
let originalEnv: NodeJS.ProcessEnv;
Expand Down
48 changes: 48 additions & 0 deletions packages/cli/src/config/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { platform } from 'node:os';
import * as dotenv from 'dotenv';
import process from 'node:process';
import {
CoreEvent,
FatalConfigError,
GEMINI_DIR,
getErrorMessage,
Expand Down Expand Up @@ -281,6 +282,20 @@ export function createTestMergedSettings(
) as MergedSettings;
}

/**
* An immutable snapshot of settings state.
* Used with useSyncExternalStore for reactive updates.
*/
export interface LoadedSettingsSnapshot {
system: SettingsFile;
systemDefaults: SettingsFile;
user: SettingsFile;
workspace: SettingsFile;
isTrusted: boolean;
errors: SettingsError[];
merged: MergedSettings;
}

export class LoadedSettings {
constructor(
system: SettingsFile,
Expand All @@ -300,6 +315,7 @@ export class LoadedSettings {
: this.createEmptyWorkspace(workspace);
this.errors = errors;
this._merged = this.computeMergedSettings();
this._snapshot = this.computeSnapshot();
}

readonly system: SettingsFile;
Expand All @@ -311,6 +327,7 @@ export class LoadedSettings {

private _workspaceFile: SettingsFile;
private _merged: MergedSettings;
private _snapshot: LoadedSettingsSnapshot;
private _remoteAdminSettings: Partial<Settings> | undefined;

get merged(): MergedSettings {
Expand Down Expand Up @@ -364,6 +381,36 @@ export class LoadedSettings {
return merged;
}

private computeSnapshot(): LoadedSettingsSnapshot {
const cloneSettingsFile = (file: SettingsFile): SettingsFile => ({
path: file.path,
rawJson: file.rawJson,
settings: structuredClone(file.settings),
originalSettings: structuredClone(file.originalSettings),
});
return {
system: cloneSettingsFile(this.system),
systemDefaults: cloneSettingsFile(this.systemDefaults),
user: cloneSettingsFile(this.user),
workspace: cloneSettingsFile(this.workspace),
isTrusted: this.isTrusted,
errors: [...this.errors],
merged: structuredClone(this._merged),
};
}

// Passing this along with getSnapshot to useSyncExternalStore allows for idiomatic reactivity on settings changes
// React will pass a listener fn into this subscribe fn
// that listener fn will perform an object identity check on the snapshot and trigger a React re render if the snapshot has changed
subscribe(listener: () => void): () => void {
coreEvents.on(CoreEvent.SettingsChanged, listener);
return () => coreEvents.off(CoreEvent.SettingsChanged, listener);
}

getSnapshot(): LoadedSettingsSnapshot {
return this._snapshot;
}

forScope(scope: LoadableSettingScope): SettingsFile {
switch (scope) {
case SettingScope.User:
Expand Down Expand Up @@ -405,6 +452,7 @@ export class LoadedSettings {
}

this._merged = this.computeMergedSettings();
this._snapshot = this.computeSnapshot();
coreEvents.emitSettingsChanged();
}

Expand Down
167 changes: 167 additions & 0 deletions packages/cli/src/ui/contexts/SettingsContext.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import type React from 'react';
import { Component, type ReactNode } from 'react';
import { renderHook, render } from '../../test-utils/render.js';
import { act } from 'react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { SettingsContext, useSettingsStore } from './SettingsContext.js';
import {
type LoadedSettings,
SettingScope,
type LoadedSettingsSnapshot,
type SettingsFile,
createTestMergedSettings,
} from '../../config/settings.js';

const createMockSettingsFile = (path: string): SettingsFile => ({
path,
settings: {},
originalSettings: {},
});

const mockSnapshot: LoadedSettingsSnapshot = {
system: createMockSettingsFile('/system'),
systemDefaults: createMockSettingsFile('/defaults'),
user: createMockSettingsFile('/user'),
workspace: createMockSettingsFile('/workspace'),
isTrusted: true,
errors: [],
merged: createTestMergedSettings({
ui: { theme: 'default-theme' },
}),
};

class ErrorBoundary extends Component<
{ children: ReactNode; onError: (error: Error) => void },
{ hasError: boolean }
> {
constructor(props: { children: ReactNode; onError: (error: Error) => void }) {
super(props);
this.state = { hasError: false };
}

static getDerivedStateFromError(_error: Error) {
return { hasError: true };
}

override componentDidCatch(error: Error) {
this.props.onError(error);
}

override render() {
if (this.state.hasError) {
return null;
}
return this.props.children;
}
}

const TestHarness = () => {
useSettingsStore();
return null;
};

describe('SettingsContext', () => {
let mockLoadedSettings: LoadedSettings;
let listeners: Array<() => void> = [];

beforeEach(() => {
listeners = [];

mockLoadedSettings = {
subscribe: vi.fn((listener: () => void) => {
listeners.push(listener);
return () => {
listeners = listeners.filter((l) => l !== listener);
};
}),
getSnapshot: vi.fn(() => mockSnapshot),
setValue: vi.fn(),
} as unknown as LoadedSettings;
});

const wrapper = ({ children }: { children: React.ReactNode }) => (
<SettingsContext.Provider value={mockLoadedSettings}>
{children}
</SettingsContext.Provider>
);

it('should provide the correct initial state', () => {
const { result } = renderHook(() => useSettingsStore(), { wrapper });

expect(result.current.settings.merged).toEqual(mockSnapshot.merged);
expect(result.current.settings.isTrusted).toBe(true);
});

it('should allow accessing settings for a specific scope', () => {
const { result } = renderHook(() => useSettingsStore(), { wrapper });

const userSettings = result.current.settings.forScope(SettingScope.User);
expect(userSettings).toBe(mockSnapshot.user);

const workspaceSettings = result.current.settings.forScope(
SettingScope.Workspace,
);
expect(workspaceSettings).toBe(mockSnapshot.workspace);
});

it('should trigger re-renders when settings change (external event)', () => {
const { result } = renderHook(() => useSettingsStore(), { wrapper });

expect(result.current.settings.merged.ui?.theme).toBe('default-theme');

const newSnapshot = {
...mockSnapshot,
merged: { ui: { theme: 'new-theme' } },
};
(
mockLoadedSettings.getSnapshot as ReturnType<typeof vi.fn>
).mockReturnValue(newSnapshot);

// Trigger the listeners (simulate coreEvents emission)
act(() => {
listeners.forEach((l) => l());
});

expect(result.current.settings.merged.ui?.theme).toBe('new-theme');
});

it('should call store.setValue when setSetting is called', () => {
const { result } = renderHook(() => useSettingsStore(), { wrapper });

act(() => {
result.current.setSetting(SettingScope.User, 'ui.theme', 'dark');
});

expect(mockLoadedSettings.setValue).toHaveBeenCalledWith(
SettingScope.User,
'ui.theme',
'dark',
);
});

it('should throw error if used outside provider', () => {
const onError = vi.fn();
// Suppress console.error (React logs error boundary info)
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});

render(
<ErrorBoundary onError={onError}>
<TestHarness />
</ErrorBoundary>,
);

expect(onError).toHaveBeenCalledWith(
expect.objectContaining({
message: 'useSettingsStore must be used within a SettingsProvider',
}),
);

consoleSpy.mockRestore();
});
});
70 changes: 67 additions & 3 deletions packages/cli/src/ui/contexts/SettingsContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,81 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useContext } from 'react';
import type { LoadedSettings } from '../../config/settings.js';
import React, { useContext, useMemo, useSyncExternalStore } from 'react';
import type {
LoadableSettingScope,
LoadedSettings,
LoadedSettingsSnapshot,
SettingsFile,
} from '../../config/settings.js';
import { SettingScope } from '../../config/settings.js';

export const SettingsContext = React.createContext<LoadedSettings | undefined>(
undefined,
);

export const useSettings = () => {
export const useSettings = (): LoadedSettings => {
const context = useContext(SettingsContext);
if (context === undefined) {
throw new Error('useSettings must be used within a SettingsProvider');
}
return context;
};

export interface SettingsState extends LoadedSettingsSnapshot {
forScope: (scope: LoadableSettingScope) => SettingsFile;
}

export interface SettingsStoreValue {
settings: SettingsState;
setSetting: (
scope: LoadableSettingScope,
key: string,
value: unknown,
) => void;
}

// Components that call this hook will re render when a settings change event is emitted
export const useSettingsStore = (): SettingsStoreValue => {
const store = useContext(SettingsContext);
if (store === undefined) {
throw new Error('useSettingsStore must be used within a SettingsProvider');
}

// React passes a listener fn into the subscribe function
// When the listener runs, it re renders the component if the snapshot changed
const snapshot = useSyncExternalStore(
(listener) => store.subscribe(listener),
() => store.getSnapshot(),
);

const settings: SettingsState = useMemo(
() => ({
...snapshot,
forScope: (scope: LoadableSettingScope) => {
switch (scope) {
case SettingScope.User:
return snapshot.user;
case SettingScope.Workspace:
return snapshot.workspace;
case SettingScope.System:
return snapshot.system;
case SettingScope.SystemDefaults:
return snapshot.systemDefaults;
default:
throw new Error(`Invalid scope: ${scope}`);
}
},
}),
[snapshot],
);

return useMemo(
() => ({
settings,
setSetting: (scope: LoadableSettingScope, key: string, value: unknown) =>
store.setValue(scope, key, value),
}),
[settings, store],
);
};
Loading