fix(cli): preserve manually added configurations in settings.json#25927
fix(cli): preserve manually added configurations in settings.json#25927cynthialong0-0 wants to merge 2 commits into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! 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 addresses a bug where manually added configurations in Highlights
Using Gemini Code AssistThe 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
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 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. Footnotes
|
gemini-cli
Bot
added
priority/p1
|
Size Change: -32 B (0%) Total Size: 33.7 MB
ℹ️ View Unchanged
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the configuration synchronization logic to ensure that settings are only deleted when explicitly set to undefined, rather than being removed when omitted. This change prevents the accidental wiping of manually added or unknown configuration keys. The updates span the MCP server removal command, settings migration logic, and the core applyKeyDiff utility, supported by updated test cases. Feedback was provided to use object spread syntax instead of direct mutation when updating the MCP servers configuration to improve code consistency and avoid side effects.
cynthialong0-0
added
priority/p2
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request modifies the settings management logic to prevent accidental deletion of configuration keys. Instead of removing keys that are omitted from the update object, the system now requires keys to be explicitly set to undefined to be deleted. This change is implemented in the applyKeyDiff utility and applied across MCP server removal and settings migration logic. The test suite has been updated to verify that omitted fields are preserved while explicit undefined values trigger deletion. I have no feedback to provide.
2 participants
Summary
Fixes bug where manually added configurations in
settings.json(e.g.,hooks) were wiped whenever settings were saved via the CLI (like toggling a theme).Details
The
applyKeyDifffunction incommentJson.tsused "sync-by-omission" semantics, which explicitly deleted any key in the file not present in the CLI's in-memory settings object. Since unknown or manually added keys are often missing from this object, they were lost on every save.This PR refactors
applyKeyDiffto be additive. It now preserves existing keys unless they are explicitly set toundefinedin the update object. Migration logic insettings.tsand thegemini mcp removecommand have been updated to useundefinedas the explicit deletion signal.Related Issues
Fixes #23138
How to Validate
settings.json(e.g.,"hooks": { "onStartup": ["echo test"] }).theme).settings.json.npm test -w @google/gemini-cli -- src/utils/commentJson.test.tsto verify the automated tests.Pre-Merge Checklist