Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR consolidates settings parsing into a dedicated settings package and implements strict JSON validation to prevent silent failures from malformed configuration or unrecognized keys.
Changes:
- Introduced a new
settingspackage with strict JSON parsing usingDisallowUnknownFields()to reject unknown configuration keys - Refactored settings loading from 3 different locations (
config.go,strategy/push_common.go,strategy/hooks.go) to use the centralizedsettings.Load()function - Maintained backwards compatibility by re-exporting types and constants from the
clipackage
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/settings/settings.go | New settings package with strict JSON parsing, consolidating settings loading/saving logic with DisallowUnknownFields() validation |
| cmd/entire/cli/settings/settings_test.go | Tests verifying strict JSON parsing rejects unknown keys in both base and local settings files |
| cmd/entire/cli/config.go | Refactored to delegate to settings package while maintaining backwards compatibility via re-exports |
| cmd/entire/cli/config_test.go | Added tests for unknown key rejection in base and local settings files |
| cmd/entire/cli/setup.go | Updated to use settings.LoadFromFile() for loading individual settings files in status display |
| cmd/entire/cli/strategy/push_common.go | Simplified to use settings.Load() and IsPushSessionsDisabled() method instead of custom parsing |
| cmd/entire/cli/strategy/hooks.go | Refactored isLocalDev() to use settings.Load() instead of custom JSON parsing |
1. loadFromFile - Now uses json.NewDecoder with DisallowUnknownFields() instead of json.Unmarshal 2. mergeJSON - Added strict validation at the start that decodes into a temp struct with DisallowUnknownFields() before doing the field-by-field merge Also added settings_test.go with three tests: - TestLoad_RejectsUnknownKeys - verifies main settings file rejects unknown keys - TestLoad_AcceptsValidKeys - verifies all valid keys are accepted - TestLoad_LocalSettingsRejectsUnknownKeys - verifies local override file also rejects unknown keys Entire-Checkpoint: c4a4c0bba140
Changes made: - Removed output_filter from test JSON and assertions (field was removed from struct) - Replaced os.Chdir() with t.Chdir() which handles cleanup automatically Entire-Checkpoint: b703660e28ea
Previously, settings were parsed in three separate places:
- settings/settings.go (the dedicated package)
- config.go (duplicated EntireSettings struct and Load/Save logic)
- strategy/hooks.go and push_common.go (ad-hoc JSON parsing)
This consolidates all settings parsing into the settings package:
1. settings/settings.go now contains:
- The canonical EntireSettings struct
- Load(), Save(), SaveLocal(), LoadFromFile() functions
- Helper methods: IsSummarizeEnabled(), IsMultiSessionWarningDisabled(),
IsPushSessionsDisabled()
2. config.go now delegates to settings package:
- EntireSettings is a type alias to settings.EntireSettings
- LoadEntireSettings() calls settings.Load()
- SaveEntireSettings() calls settings.Save()
- Removed ~200 lines of duplicated parsing code
3. strategy/hooks.go:
- isLocalDev() now uses settings.Load().LocalDev
- Removed ad-hoc JSON parsing
4. strategy/push_common.go:
- isPushSessionsDisabled() now uses settings.Load().IsPushSessionsDisabled()
- Removed readPushSessionsFromFile() and 50+ lines of ad-hoc parsing
Benefits:
- Single place to update when adding new settings fields
- Consistent strict parsing (DisallowUnknownFields) everywhere
- Easier to maintain and test
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Entire-Checkpoint: d9e47174f862
…iables. Entire-Checkpoint: d82cce910879
toothbrush
force-pushed
the
20260205-strict-settings-parsing
branch
from
37fb3b2 to
36e0583
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Entire-Checkpoint: 1b8e9b9a1c07
georg
approved these changes
Feb 6, 2026
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
2 participants
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.
Description
Note
Medium Risk
Touches core configuration IO and hook/runtime behavior; strict decoding may break users with previously-ignored/typoed config keys and cause commands/hooks to error until configs are fixed.
Overview
Settings loading/saving is consolidated into the
settingspackage:cli/config.go, hook installation (strategy/hooks.go), push-session logic (strategy/push_common.go), and detailed status output (setup.go) now callsettings.Load/Save/SaveLocal/LoadFromFileinstead of re-parsing JSON in multiple places.Settings JSON parsing is now strict: both base and local settings files reject unknown keys via
DisallowUnknownFields, with new tests incli/config_test.goandsettings/settings_test.gocovering the failure cases. The settings package also addsIsPushSessionsDisabled()helper and centralized file write logic usingjsonutil.Written by Cursor Bugbot for commit 2a4b60a. This will update automatically on new commits. Configure here.