fix(setup): stop enable/disable from leaking local overrides into project settings · Entire

fix(setup): stop enable/disable from leaking local overrides into project settings

3987a85→main·

suhaanthayyil·6d ago·2 files·+145 added/-16 removed

runEnable/runDisable loaded the merged LoadEntireSettings view (project + settings.local.json overrides flattened together), flipped the enabled bit, and wrote the whole struct back. Once #1140 made a bare entire enable resolve to settings.json when the project file already exists, this started writing local-only overrides (local_dev, log_level, personal strategy_options/checkpoint_remote, ...) into the shared, committed project file on every re-enable.

Flip the enabled key on the target file's own raw JSON via the existing LoadProjectRaw/SaveProjectRaw and LoadLocalRaw/SaveLocalRaw scoped accessors instead, preserving each file's other keys untouched. Merge semantics are kept for runtime reads (IsEnabled, etc.) — only the enable/disable write path changes.

Changes

2

// Writes to the target file (local by default, project with --project),
// and also updates the other file if it exists, so they can't get out of sync.
func runEnable(ctx context.Context, w io.Writer, useProjectSettings bool) error {
    s, err := LoadEntireSettings(ctx)
    if err != nil {
        return fmt.Errorf("failed to load settings: %w", err)
    }

s.Enabled = true

if err := saveEnabledState(ctx, s, useProjectSettings); err != nil {
    if err := setEnabledFlag(ctx, true, useProjectSettings); err != nil {
        return err
    }
}

func runDisable(ctx context.Context, w io.Writer, useProjectSettings bool) error {
    s, err := LoadEntireSettings(ctx)
    if err != nil {
        return fmt.Errorf("failed to load settings: %w", err)
    }

s.Enabled = false

if err := saveEnabledState(ctx, s, useProjectSettings); err != nil {
    if err := setEnabledFlag(ctx, false, useProjectSettings); err != nil {
        return err
    }
}

return nil
}

// setEnabledFlag flips only the "enabled" key in the target settings file's
// raw JSON, and also updates the other scope's file if it exists, so
// local/project can't get out of sync. Unlike saveEnabledState, this operates
// on each file's own raw content rather than the LoadEntireSettings merged
// view: that view flattens settings.local.json overrides (local_dev,
// log_level, personal strategy_options/checkpoint_remote, ...) on top of
// settings.json, so writing the merged struct back through SaveEntireSettings
// would leak a developer's local-only overrides into the shared, committed
// project file whenever a bare `entire enable`/`entire disable` resolves to
// settings.json (#1140). Merge semantics still apply everywhere enable/
// disable *read* current state (e.g. IsEnabled); only the write path needs to
// stay scoped to the target file.
func setEnabledFlag(ctx context.Context, enabled, useProjectSettings bool) error {
    if useProjectSettings {
        if err := setEnabledRaw(ctx, settings.LoadProjectRaw, settings.SaveProjectRaw, enabled); err != nil {
            return fmt.Errorf("failed to save settings: %w", err)
        }
        // Also update local if it exists, so it doesn't override.
        if localExists(ctx) {
            if err := setEnabledRaw(ctx, settings.LoadLocalRaw, settings.SaveLocalRaw, enabled); err != nil {
                return fmt.Errorf("failed to save local settings: %w", err)
            }
        }
    } else {
        if err := setEnabledRaw(ctx, settings.LoadLocalRaw, settings.SaveLocalRaw, enabled); err != nil {
            return fmt.Errorf("failed to save local settings: %w", err)
        }
    }
    return nil
}

// setEnabledRaw loads a settings file via load, sets its "enabled" key, and
// writes it back via save, preserving every other key already in that file.
func setEnabledRaw(
    ctx context.Context,
    load func(context.Context) (path string, raw map[string]json.RawMessage, exists bool, err error),
    save func(path string, raw map[string]json.RawMessage) error,
    enabled bool,
) error {
    path, raw, _, err := load(ctx)
    if err != nil {
        return err
    }
    value, err := json.Marshal(enabled)
    if err != nil {
        return fmt.Errorf("marshal enabled flag: %w", err)
    }
    raw["enabled"] = value
    return save(path, raw)
}

// saveEnabledState writes settings to the target file and also updates the
// other settings file if it exists, preventing local/project from getting
// out of sync on the enabled field.

// TestRunEnable_ProjectFlag_DoesNotLeakLocalOverrides verifies that // entire enable --project with a local-only override present (e.g. // local_dev, set via settings.local.json) does not write that override into // the shared, committed project settings.json — only the enabled flag should // change there (#1140 finding: runEnable must not round-trip the merged // settings view through the project file). func TestRunEnable_ProjectFlag_DoesNotLeakLocalOverrides(t *testing.T) { setupTestDir(t) writeSettings(t, testSettingsDisabled) writeLocalSettings(t, {\"enabled\": true, \"local_dev\": true})

var buf bytes.Buffer if err := runEnable(context.Background(), &buf, true); err != nil { t.Fatalf("runEnable(project=true) error = %v", err) }

// The merged view is correctly enabled. enabled, err := IsEnabled(context.Background()) if err != nil { t.Fatalf("IsEnabled() error = %v", err) } if !enabled { t.Error("expected enabled after runEnable --project") }

// The project file must be flipped to enabled, and must NOT gain the // local-only override. projectContent, err := os.ReadFile(EntireSettingsFile) if err != nil { t.Fatalf("failed to read project settings: %v", err) } if !strings.Contains(string(projectContent), ""enabled":true") && !strings.Contains(string(projectContent), ""enabled": true") { t.Errorf("project settings should have enabled:true, got: %s", projectContent) } if strings.Contains(string(projectContent), "local_dev") { t.Errorf("project settings must not leak local-only override local_dev, got: %s", projectContent) }

// The local file's own override must be preserved untouched. localContent, err := os.ReadFile(EntireSettingsLocalFile) if err != nil { t.Fatalf("failed to read local settings: %v", err) } if !strings.Contains(string(localContent), "local_dev") { t.Errorf("local settings should still contain local_dev override, got: %s", localContent) } }

// TestRunEnable_LocalScope_PreservesLocalOnlyFields verifies that entire // enable (default, no --project) with an existing local-only override only // flips the enabled flag in settings.local.json and leaves the rest of that // file's own content (like local_dev) intact. func TestRunEnable_LocalScope_PreservesLocalOnlyFields(t *testing.T) { setupTestDir(t) writeSettings(t, testSettingsEnabled) writeLocalSettings(t, {\"enabled\": false, \"local_dev\": true})

var buf bytes.Buffer if err := runEnable(context.Background(), &buf, false); err != nil { t.Fatalf("runEnable(project=false) error = %v", err) }

enabled, err := IsEnabled(context.Background()) if err != nil { t.Fatalf("IsEnabled() error = %v", err) } if !enabled { t.Error("expected enabled after runEnable") }

localContent, err := os.ReadFile(EntireSettingsLocalFile) if err != nil { t.Fatalf("failed to read local settings: %v", err) } if !strings.Contains(string(localContent), ""enabled":true") && !strings.Contains(string(localContent), ""enabled": true") { t.Errorf("local settings should have enabled:true, got: %s", localContent) } if !strings.Contains(string(localContent), "local_dev") { t.Errorf("local settings should still contain local_dev override, got: %s", localContent) }

// Project settings must be untouched by the local-scope write. projectContent, err := os.ReadFile(EntireSettingsFile) if err != nil { t.Fatalf("failed to read project settings: %v", err) } if strings.Contains(string(projectContent), "local_dev") { t.Errorf("project settings must not gain local-only override local_dev, got: %s", projectContent) } }

func TestDetermineSettingsTarget_ExplicitLocalFlag(t *testing.T) { tmpDir := t.TempDir()