Conversation
Owner
tis24dev
commented
Feb 4, 2026
tis24dev
commented
- Log updated missing config keys after upgrade
- Insert missing template keys respecting user order
- Group unanchored missing keys into upgrade section
After running the configuration upgrade, log an informational message when missing keys were added. Adds a conditional that checks sessionLogger and cfgUpgradeResult and logs the count and comma-separated list of cfgUpgradeResult.MissingKeys to help surface which keys were introduced during the upgrade.
Improve config upgrade logic to preserve user key positions and insert missing template entries near related user keys. parseEnvValues now records key ranges (keyRange) for each user entry and returns them; computeConfigUpgrade collects template entries, computes missing entries, and builds ordered insert operations (using sort) to merge template lines into the original file without unnecessary rewrites. Also adjust handling for block (multi-line) values, preserve extra keys and casing warnings, and update tests (rename and expectations) to reflect preserved extra keys and original casing for blocks.
Add an index field to templateEntry to track template order and use it for insert ordering. When missing template entries have no nearby anchor, collect them and append a single "# Added by upgrade" section (with a separating blank line if appropriate) at the append index instead of trying to insert them in-place. Adjust insertOp ordering accordingly. Add a test (TestUpgradeConfigAddsMissingKeysUnderUpgradeSectionWhenNoAnchor) to verify legacy keys are preserved and missing keys are added under the upgrade section.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR syncs changes from dev to main, implementing significant improvements to the configuration upgrade system. The changes refactor the upgrade logic to better preserve user customizations while ensuring all required template keys are present.
Changes:
- Refactored config upgrade to preserve original user file structure and insert missing keys at appropriate positions
- Added tracking of user key positions (keyRange) to enable intelligent insertion of missing template keys after their predecessors
- Added logging of missing config keys after upgrade for better user visibility
- Grouped unanchored missing keys (those without predecessors in user's config) into a dedicated "Added by upgrade" section
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/config/upgrade.go | Major refactoring from template-walking to user-data-preserving approach; added keyRange tracking, intelligent key insertion logic, and unanchored key grouping |
| internal/config/upgrade_test.go | Updated test name to reflect new behavior; removed check for obsolete custom section header; fixed casing expectations to match new behavior; added test for unanchored keys |
| cmd/proxsave/upgrade.go | Added informational logging for missing keys after configuration upgrade |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/config/upgrade.go
Outdated
| // So `processedUserKeys["Backup_Enabled"]` is true. We skip. | ||
| // Correct. | ||
| if templateKey, ok := templateKeyByUpper[upperKey]; ok && templateKey != key && !caseConflicts[upperKey] { | ||
| warnings = append(warnings, fmt.Sprintf("Key %q differs only by case from template key %q; preserved as custom entry", key, templateKey)) |
There was a problem hiding this comment.
The warning message is misleading. It states the key is "preserved as custom entry", but the key is not added to extraKeys, meaning it's not actually treated as a custom entry. The key is matched to the template (case-insensitively) and preserved in place. Consider either adding the key to extraKeys to match the warning message, or updating the warning message to say something like "preserved with original casing" instead of "preserved as custom entry".
Suggested change
| warnings = append(warnings, fmt.Sprintf("Key %q differs only by case from template key %q; preserved as custom entry", key, templateKey)) | |
| warnings = append(warnings, fmt.Sprintf("Key %q differs only by case from template key %q; preserved with original casing", key, templateKey)) |
Update the warning emitted in computeConfigUpgrade when a config key differs only by case from a template key. The message now says the entry is "preserved with original casing" instead of "preserved as custom entry" to more accurately describe the behavior and reduce confusion. Affects internal/config/upgrade.go.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.