fix: strip overrides removes keys missing from local config#50
fix: strip overrides removes keys missing from local config#50c10l wants to merge 1 commit intoiHildy:mainfrom
Conversation
When a user adds a config section to opencode-synced.overrides.jsonc (e.g. server) that exists in the repo's base config but not in the local config, stripOverrides was restoring the base value instead of removing it. This caused push to report "no changes" even though the override section should have been stripped from the repo. Fix: add check for currentValue === undefined before restoring baseValue. If an override declares a key that doesn't exist in the local config, it should always be stripped (the override means "this key is local-only"). Closes iHildy#49
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 resolves a critical bug in the 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
|
There was a problem hiding this comment.
Code Review
This pull request enhances the stripOverrides function in src/sync/config.ts by modifying its logic to correctly strip keys that are either not present in the local configuration or not present in the base configuration. This ensures that override keys are properly removed if they are not explicitly defined in the local config. Corresponding unit tests have been added in src/sync/config.test.ts to validate these new behaviors, covering cases for both object and scalar keys. There is no feedback to provide on the review comments as none were provided.
Summary
stripOverrides()restoring base values when an override key (object or scalar) is missing from the local config but exists in the base (repo) configcurrentValue === undefinedcheck to ensure override-declared keys are always stripped, regardless of base config presenceProblem
When a user adds a config section to
opencode-synced.overrides.jsonc(e.g.,server) that exists in the repo's base config but not in the local config,pushreports "No local changes to push" becausestripOverridesrestores the base value instead of removing it.Closes #49
Test plan
stripOverridescovering the bug scenarios