Repository navigation
fix(settings): preserve disk edits when changing authentication - #29762
seokjeongeum wants to merge 1 commit 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 data loss issue where changing authentication methods would overwrite the entire settings file with a stale snapshot, causing loss of any user edits made since the application started. The changes introduce granular settings updates, ensuring that only the modified property is persisted to disk while preserving other manual changes, comments, and environment placeholders. Additionally, documentation has been improved to clearly define the side effects of authentication and exit commands. 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 the 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 counterproductive. 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
|
|
📊 PR Size: size/L
|
There was a problem hiding this comment.
Code Review
This pull request improves settings persistence by allowing the CLI to save only the explicitly changed configuration properties rather than overwriting the entire file with a stale startup snapshot, which preserves concurrent or external edits. It updates the format-preserving JSON update utility to support a property path and adds comprehensive tests. Documentation for /auth subcommands and /quit settings has also been updated. Feedback on the changes highlights a potential type mismatch bug in applyPropertyUpdate where an array and a plain object are both treated as property containers, which could lead to silent data loss or schema violations if their types do not match.
545043b to
a6b0760
Compare
|
Hi there! Thank you for your interest in contributing to Gemini CLI. To ensure we maintain high code quality and focus on our prioritized roadmap, we only guarantee review and consideration of pull requests for issues that are explicitly labeled as 'help wanted'. This PR will be closed in 7 days if it remains without that designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
|
This pull request is being closed as it has been open for 14 days without a 'help wanted' designation. We encourage you to find and contribute to existing 'help wanted' issues in our backlog! Thank you for your understanding. |
Summary
Preserve settings-file edits made after Gemini CLI starts when a setting, including the authentication method, is saved. Save the requested property against the current disk contents instead of rewriting the startup snapshot.
Details
Limit property writes to the changed path while retaining unrelated disk additions, deletions, comments, environment placeholders, and array/object changes. If a required ancestor has become incompatible, recreate the ancestors needed for that property while keeping unrelated settings. Object-valued assignments still replace the assigned object. Malformed JSON is reported and left intact.
Document
/auth signin/login,/auth signout/logout, and normal/quitpersistence. Opening or cancelling authentication selection does not reset preferences; signout removes the selected authentication type, while normal quit retains saved preferences and credentials. No public settings/schema or package-version changes.Related Issues
Related to #23138. Its stale
originalSettingssnapshot also reproduces when changing authentication on the upstream base used here.How to Validate
Validated final head
a6b076020dac1024a7a34e687d43308076157518, based on upstreamed96faedfd5571f055c44f2c9509d62321d1138d.node ../../node_modules/vitest/vitest.mjs run src/config/settings.persistence.test.ts src/config/settings.test.ts src/utils/commentJson.test.ts src/ui/commands/authCommand.test.ts src/ui/commands/quitCommand.test.ts --coverage.enabled=false --maxWorkers=1 --minWorkers=1frompackages/cli: 176/176 pass on Linux Node 24.21.0 and native Windows Node 24.14.1.npm run build,npm run typecheck -w @google/gemini-cli-core -w @google/gemini-cli, changed-file ESLint/Prettier, andgit diff --checkpass. Native Windows core/devtools/CLI npm builds and affected typechecks pass. All 3,027 regular source blobs in the isolated NTFS checkout match this head; the documentation symlink is explicitly materialized.npm run start -- --version/--helpand isolated packednpx --offline --no-install gemini --version/--helppass on Linux. Native source version launch and actual offline npm-exec version/help pass. Native packed CLI/core files are extracted into an isolated prefix and hash-checked, using native third-party dependencies from a matching lockfile checkout; a fresh Windows registry installation remains unverified after an offline cache miss and bounded network timeout. No global binary is used for package validation.gemini-auth-pr29762:a6b0760installs the exact CLI/core tarballs and passes all six real-file scenarios plus offline npx version/help on Node 20.20.2. This is a Linux artifact-runtime container hosted by Windows Docker, not native Windows-container coverage or a repository-Dockerfile source build.npm run preflightcompleted in a disposable checkout on this head under Node 20.19.0, with two Vitest workers. Install/format/build/full lint/all typechecks pass; tracked source remains clean. CLI reports 10 failed, 7,418 passed, 4 skipped; core reports 76 failed, 8,296 passed, 57 skipped. Every failing record, including repeated parameterized cases, reproduces on upstreamed96faedfwith the same runtime, dependencies, isolated home, and worker bounds: no candidate-only failure. Failures concern mocked home/workspace paths, keychain mocks, and unavailablebwrap. The script and SEA suites skipped by the failed workspace chain were run separately on this head:npm run test:scriptspasses 202/202 andnpm run test:sea-launchpasses 17/17.The previous review thread was checked against this head and remains resolved; no unresolved thread was found in the fully paginated audit.
GitHub Testing: CI reports
action_required; a maintainer must approve the external-contributor workflow run. Local results do not imply CI passed. macOS/Seatbelt are unavailable, and Podman fixtures remain pending because host sudo authentication is unavailable.Pre-Merge Checklist