refactor(cli): Reactive useSettingsStore hook

[psinha40898]

Summary

The SettingsDialog is bloated through excessive use of React State, Effect, unnecessary redundant states, and the use of React Anti Patterns to sync the UI with the LoadedSettings.

Anti Pattern:
SettingsContext is a thin DI for a mutable object, forcing a React anti-pattern in SettingsDialog: updating state manually after mutation instead of one idiomatic update.

Before the simple refactors, it makes sense to incrementally merge the requirements to remove this Anti Pattern from UI that is Reactive with Settings.

Details

PR1: Convert LoadedSettings to a store and create a second hook in SettingsContext that re-renders when a settings changed event is emitted

Problem: Components needing to re-render on settings changes (like SettingsDialog) currently use an anti-pattern: mutate LoadedSettings, then manually call setState to trigger re-render.

Solution:
Convert LoadedSettings to a store

• It manages an immutable snapshot
• It exposes a subscribe function that allows React's useSyncExternalStore to subscribe to the event bus. Whenever a settings-changed event is emitted, React checks if the snapshot has changed in object identity. If it has, it triggers the re-render we need.

const { settings, setSetting } = useSettingsStore();
// settings is reactive - re-renders on change
// setSetting is the single mutation point

Why a second hook: Exposing this as a new hook (useSettingsStore) rather than changing useSettings means:

• Zero code churn for existing consumers
• Opt-in reactivity only where needed

Files changed:

• packages/cli/src/config/settings.ts
• packages/cli/src/config/settings.test.ts
• packages/cli/src/ui/contexts/SettingsContext.tsx
• packages/cli/src/ui/contexts/SettingsContext.test.tsx

Next Steps:

PR2: Refactor SettingsDialog business logic to use the reactive store

This PR will ship with one small UX change - if a user toggles all restart required settings back to the value they initially were, the restart required prompt will also toggle off (b/c it's not required to apply those settings at that state). This was just a natural extension of removing complicated logic such as synchronizing many React states and batching mutations.

Replaces the imperative state-syncing model in SettingsDialog with the reactive useSettingsStore() from PR1.

• Deletes 5 local states (pendingSettings, modifiedSettings, globalPendingChanges, _restartRequiredSettings, showRestartPrompt) and 5 obsolete utility functions from settingsUtils.ts
• Simplifies all callbacks to call setSetting() directly — no intermediate state
• Restart-required tracking uses a snapshot diff with comparison, only tracking dialog-visible restart settings via new getDialogRestartRequiredSettings()
• Removes settings prop from SettingsDialog (reads from context instead)
• Updates all tests mechanically

If the value you need can be computed entirely from the current props or other state, remove that redundant state altogether.

https://react.dev/reference/react/useState

Files changed:

• packages/cli/src/ui/components/SettingsDialog.tsx
• packages/cli/src/ui/components/SettingsDialog.test.tsx
• packages/cli/src/ui/components/DialogManager.tsx
• packages/cli/src/utils/settingsUtils.ts
• packages/cli/src/utils/settingsUtils.test.ts
• packages/cli/src/utils/dialogScopeUtils.ts

PR 2 against PR 1 (outdated post-merge):

psinha40898/gemini-cli@pyush/refactor/settings-4...psinha40898:gemini-cli:pyush/refactor/settings-dialog-business

204 additions and 686 deletions.

PR3: Extract hooks from BaseSettingsDialog

Pure React cleanup — no business logic changes, no external API changes.

1. (Optional) A reducer to describe state changes for navigation in BaseSettingsDialog, wrapped in a custom hook (useDialogNavigation)

As your components grow in complexity, it can get harder to see at a glance all the different ways in which a component's state gets updated.

https://react.dev/learn/extracting-state-logic-into-a-reducer

2. (Optional, but wrapping an effect like the cursor blink into a custom hook is considered good React) Refactors inline editing state into a custom hook (useInlineEditor) in BaseSettingsDialog

Whenever you write an Effect, consider whether it would be clearer to also wrap it in a custom Hook.

https://react.dev/learn/reusing-logic-with-custom-hooks#when-to-use-custom-hooks

3. Derives effect when possible in BaseSettingsDialog (like the effect that forces focus on the Settings menu when the Scope menu is hidden)

You don't need Effects to transform data for rendering. (...) To avoid the unnecessary render passes, transform all the data at the top level of your components.

https://react.dev/learn/you-might-not-need-an-effect

Files changed:

• packages/cli/src/ui/hooks/useDialogNavigation.ts (new)
• packages/cli/src/ui/hooks/useInlineEditor.ts (new)
• packages/cli/src/ui/components/shared/BaseSettingsDialog.tsx

PR 3 against PR 2: (outdated post-merge):

psinha40898/gemini-cli@pyush/refactor/settings-dialog-business...psinha40898:gemini-cli:pyush/refactor/settings-dialog-base-hooks

392 additions and 183 deletions.

Future Work:

• Remove similar anti-patterns from AgentConfigDialog.tsx
• An alternate buffer version of the SettingsDialog that utilizes the new Scrollable Containers

Related Issues

RELATED TO #15840

How to Validate

Pre-Merge Checklist

• Updated relevant documentation and README (if needed)
• Added/updated tests (if needed)
• Noted breaking changes (if any)
• Validated on required platforms/methods:
  • MacOS
    • npm run
    • npx
    • Docker
    • Podman
    • Seatbelt
  • Windows
    • npm run
    • npx
    • Docker
  • Linux
    • npm run
    • npx
    • Docker

[gemini-code-assist]

Summary of Changes

Hello @psinha40898, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request undertakes a substantial refactor of the SettingsDialog component to align with modern React patterns. The primary goal is to simplify state management by consolidating related state into a useReducer and extracting complex UI logic into a custom hook. This approach aims to make the component more maintainable, predictable, and performant by reducing redundant state and effects.

Highlights

• State Management Refactor: The SettingsDialog.tsx component has been significantly refactored to centralize state management using a useReducer hook. This replaces numerous useState calls for navigation, search, and pending changes, reducing useState instances from 15 to 1.
• Custom Hook for Inline Editing: Inline editing logic (key, buffer, cursor position, and visibility) has been encapsulated into a new useInlineEdit custom hook, improving modularity and reusability. This also contributes to the reduction of useState calls.
• Derived State and Effects: Several states and effects are now derived from existing state, adhering to React best practices. For example, showRestartPrompt is derived from globalPendingChanges, and a focus correction effect is replaced by a derived effectiveFocusSection.
• Reduced useEffect Calls: The refactoring has led to a reduction in useEffect calls from 4 to 1, indicating a more efficient and less side-effect-prone component.
• New Reducer and Hook Implementations: Two new files, settingsDialogReducer.ts and useInlineEdit.ts, have been added to implement the new state management logic and custom hook, respectively. Both include comprehensive unit tests.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This is an excellent refactoring of the SettingsDialog component. The introduction of useReducer to manage the component's complex state and the extraction of inline editing logic into the useInlineEdit custom hook have significantly improved the code's structure, readability, and maintainability. Deriving state where possible, like for showRestartPrompt and effectiveFocusSection, is a great application of React best practices. The addition of comprehensive tests for the new reducer and hook is also a major plus, ensuring the new logic is robust.

I found one logic issue in how default values for restart-required settings are handled, which I've detailed in a specific comment. Besides that, this is a very strong pull request that greatly enhances the quality of the codebase.

[psinha40898]

The PR successfully refactors SettingsDialog.tsx to use useReducer and useInlineEdit, significantly improving state management and readability. I have reviewed the code and tests, and here are my findings.

Code Quality & Patterns

• Refactor: The move to useReducer for complex state (navigation, search, pending changes) is excellent. It makes the state transitions explicit and testable.
• Hooks: Extraction of useInlineEdit cleans up the component logic significantly.
• React Anti-Pattern: The retention of pendingSettings to drive renders (due to the mutable nature of Settings) is acceptable in this context, as noted in the description.
• Logging: The use of debugLogger is good.
  • Note: There is a console.error left in SettingsDialog.tsx (line 235: console.error('Failed to toggle vim mode:', error);). Consider using debugLogger or a UI error state. Given it's a catch block for an async action in the UI, it might get lost or mess up the TUI.

Tests

• New Tests: The new tests for settingsDialogReducer and useInlineEdit are well-structured and use vitest correctly.
• Render Wrapper: Usage of renderHook from ../../test-utils/render.js correctly wraps interactions in act, following the project's testing guidelines.
• Existing Tests: It is great to see that SettingsDialog.test.tsx passed without modification, indicating the refactor preserved behavior.

Logic Issue (Reset to Default)

I confirmed the potential issue with resetting to default values for restart-required settings:

• The PendingValue type in settingsDialogReducer.ts is boolean | number | string.
• In SettingsDialog.tsx, when resetting a restart-required setting:

  if (
    (currentSetting.type === 'boolean' && typeof defaultValue === 'boolean') ||
    // ...
  ) {
     dispatch({ type: 'ADD_PENDING_CHANGE', ... });
  }

• If defaultValue is undefined (which getDefaultValue returns for settings with no explicit default in the schema), this block is skipped.
• Consequently, the "reset" (unsetting the value) is not tracked in globalPendingChanges, and the user's intent to reset it will be lost if they don't save immediately.
• Suggestion: Update PendingValue to include undefined or a symbol to represent "unset", and handle this case in the reducer and saveModifiedSettings logic.

// Example fix for the PendingValue type
export type PendingValue = boolean | number | string | undefined; // undefined represents 'unset'

Recommendation

This is a high-quality refactor. Please address the console.error and the default value logic issue.

[psinha40898]

Logic Issue (Reset to Default)

I confirmed the potential issue with resetting to default values for restart-required settings:

• The PendingValue type in settingsDialogReducer.ts is boolean | number | string.
• In SettingsDialog.tsx, when resetting a restart-required setting:

  if (
    (currentSetting.type === 'boolean' && typeof defaultValue === 'boolean') ||
    // ...
  ) {
     dispatch({ type: 'ADD_PENDING_CHANGE', ... });
  }

• If defaultValue is undefined (which getDefaultValue returns for settings with no explicit default in the schema), this block is skipped.
• Consequently, the "reset" (unsetting the value) is not tracked in globalPendingChanges, and the user's intent to reset it will be lost if they don't save immediately.
• Suggestion: Update PendingValue to include undefined or a symbol to represent "unset", and handle this case in the reducer and saveModifiedSettings logic.

// Example fix for the PendingValue type
export type PendingValue = boolean | number | string | undefined; // undefined represents 'unset'

I can push a defect test a long with a fix but will note it for now and wait for a reviewer to decide whether this refactor PR should modify any SettingsDialog.test.tsx tests. Because the defect test would also fail on main.

• Logging: The use of debugLogger is good.

  • Note: There is a console.error left in SettingsDialog.tsx (line 235: console.error('Failed to toggle vim mode:', error);). Consider using debugLogger or a UI error state. Given it's a catch block for an async action in the UI, it might get lost or mess up the TUI.

Same here because the console.error is inherited from main.

Because the focus of the PR is on React usage

[psinha40898]

Thanks for the significant refactor! This cleans up the state management in SettingsDialog and aligns well with our React guidelines.

I've reviewed the changes and the discussion. Here are the items to address before merging:

1. Reset to Default Logic (Undefined Values):
   Re: your comment about the logic issue with undefined default values: Please go ahead and include the fix and the test case in this PR. It is better to resolve this known bug now, especially since you have identified the solution.

   • Update PendingValue in settingsDialogReducer.ts:

     export type PendingValue = boolean | number | string | undefined;

   • Update the logic in SettingsDialog.tsx to handle undefined correctly.
   • Add the test case to settingsDialogReducer.test.ts.

2. Console Error:
   Re: your comment about the inherited console.error:

   Same here because the console.error is inherited from main.

   While I understand it is pre-existing, GEMINI.md explicitly advises against console logs in production code. Since you are already modifying this file significantly, please switch it to debugLogger.log (or handle it in the UI) to be consistent with the rest of the project. It's a small change that improves code quality.

3. Exhaustive Check:
   In settingsDialogReducer.ts, please use the checkExhaustive helper in the default case of the switch statement. This is a project pattern to ensure all action types are handled.

   • Import it from ../../utils/checks.js.
   • Usage:

     default:
       checkExhaustive(action);
       return state;

Verification:
The new tests in settingsDialogReducer.test.ts and useInlineEdit.test.ts look correct and use the proper test utils.

Great work on simplifying this component!

[jacob314]

think this is generally on the right track. need to go a bit further to make sure we don't have any odd patterns forcing updates left. commented on the couple remaining cases where I see us forcing updates in a somewhat hacky way. if possible would be nice to remove as well.

[jacob314]

let me know when this is ready fore view. there are a few merge conflicts.

[psinha40898]

Instead of refactoring settings to be a store it is probably better to use the event bus architecture in place

[bdmorgan]

Hi @psinha40898, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this.

We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines.

Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed.

Thank you for your understanding and for being a part of our community!

[gemini-cli]

Hi there! Thank you for your contribution to Gemini CLI.

To improve our contribution process and better track changes, we now require all pull requests to be associated with an existing issue, as announced in our recent discussion and as detailed in our CONTRIBUTING.md.

This pull request is being closed because it is not currently linked to an issue. You can easily reopen this PR once you have linked it to an issue.

How to link an issue:
Add a keyword followed by the issue number (e.g., Fixes #123) in the description of your pull request. For more details, see the GitHub Documentation.

Thank you for your understanding and for being a part of our community!

[jacob314]

Pull Request Review: refactor(cli): Reactive useSettingsStore hook

Overall, this is a very high-quality refactor that effectively addresses the reactive settings anti-pattern. The introduction of useSyncExternalStore is the idiomatic React way to handle external state subscription in this codebase.

Highlights:

• Architectural Improvement: The shift from imperative state syncing to a reactive store significantly simplifies component logic, especially in complex areas like the SettingsDialog.
• Idiomatic React: Proper use of useSyncExternalStore and useMemo ensures efficient re-renders and stable object identities.
• Solid Testing: The new tests correctly utilize our specialized renderHook and render utilities from packages/cli/src/test-utils/render.js, ensuring proper act() wrapping and avoiding test flakiness.

Suggested Improvement:

• Exhaustiveness Checking: In packages/cli/src/ui/contexts/SettingsContext.tsx, inside the useSettingsStore hook's forScope function, consider using checkExhaustive in the default case of the switch statement. This ensures type safety if LoadableSettingScope is expanded in the future.

// packages/cli/src/ui/contexts/SettingsContext.tsx

default:
  return checkExhaustive(scope);

Great work on this!

[jacob314]