-
Notifications
You must be signed in to change notification settings - Fork 209
fix: enable --agent respects settings.local.json overrides#339
fix: enable --agent respects settings.local.json overrides#339adrianmg wants to merge 3 commits intoentireio:mainfrom
Conversation
Summary
entire enable --agentalways wrote tosettings.json, ignoring a staleenabled: falseinsettings.local.jsonleft by a priorentire disable- This caused
entire statusto show "Disabled" immediately after a successful enable
Root Cause
runDisable() writes enabled: false to settings.local.json. But setupAgentHooksNonInteractive() (the --agent code path) only wrote enabled: true to settings.json. Since settings.Load() merges local over project, the stale false in settings.local.json always won.
The interactive enable and --strategy paths already handled this correctly via determineSettingsTarget() -- the --agent path was the only one missing it.
Fix
After writing enabled: true to settings.json, check if settings.local.json exists with a stale enabled: false and clear it. No signature changes, no new params -- just 4 lines of targeted cleanup.
Repro (before fix)
entire enable --agent gemini # writes enabled:true - settings.json
entire disable # writes enabled:false - settings.local.json
entire enable --agent gemini # writes enabled:true - settings.json (local untouched)
entire status # - "Disabled (manual-commit)"
Affects
All agents using the `--agent` flag -- not specific to any one agent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where entire enable --agent failed to respect stale enabled: false settings in .entire/settings.local.json, causing entire status to incorrectly show "Disabled" immediately after a successful enable operation.
Changes:
- Added cleanup logic in
setupAgentHooksNonInteractiveto detect and clear staleenabled: falseinsettings.local.jsonafter writingenabled: truetosettings.json
| // Clear stale enabled:false in settings.local.json left by a prior "entire disable" | ||
| localPath, pathErr := paths.AbsPath(EntireSettingsLocalFile) | ||
| if pathErr == nil { | ||
| if ls, err := settings_pkg.LoadFromFile(localPath); err == nil && !ls.Enabled { | ||
| ls.Enabled = true | ||
| if err := SaveEntireSettingsLocal(ls); err != nil { | ||
| return fmt.Errorf("failed to clear stale local settings: %w", err) | ||
| } | ||
| } | ||
| } |
Copilot
AI
Feb 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bug fix lacks test coverage. Consider adding a test that reproduces the original bug scenario: enable with --agent, disable, then enable again with --agent, and verify that the status shows "Enabled". The existing TestEnableDisable integration test uses --strategy which already handled this correctly via determineSettingsTarget, so it wouldn't catch this specific bug in the --agent code path.
|
Thanks for the contribution! We ask that all PRs have a corresponding GitHub issue so we can discuss the approach first. Could you open an issue describing the problem this solves? Once we've aligned there, we'll prioritize the review. This helps make sure your effort doesn't go to waste. |