816453b · crush · git.iko.soy

git.iko.soy/crush

on main help sign in

feat(tools/crush_info): staleness detection and auto-reload

816453bChristian Rocha6 months ago

internal/config/load.go+19−11

compare with main

unifiedsplit

@@ -91,6 +91,12 @@ func Load(workingDir, dataDir string, debug bool) (*ConfigStore, error) {

91 // Configure providers

92 valueResolver := NewShellVariableResolver(env)

93 store.resolver = valueResolver

94+

95+ // Disable auto-reload during initial load to prevent nested calls from

96+ // config-modifying operations inside configureProviders.

97+ store.autoReloadDisabled = true

98+ defer func() { store.autoReloadDisabled = false }()

99+

94 if err := cfg.configureProviders(store, env, valueResolver, store.knownProviders); err != nil {

100 if err := cfg.configureProviders(store, env, valueResolver, store.knownProviders); err != nil {

95 return nil, fmt.Errorf("failed to configure providers: %w", err)

101 return nil, fmt.Errorf("failed to configure providers: %w", err)

96 }

102 }

@@ -100,7 +106,7 @@ func Load(workingDir, dataDir string, debug bool) (*ConfigStore, error) {

100 return store, nil

106 return store, nil

101 }

107 }

102

108

103− if err := configureSelectedModels(store, store.knownProviders); err != nil {

109+ if err := configureSelectedModels(store, store.knownProviders, true); err != nil {

104 return nil, fmt.Errorf("failed to configure selected models: %w", err)

110 return nil, fmt.Errorf("failed to configure selected models: %w", err)

105 }

111 }

106 store.SetupAgents()

112 store.SetupAgents()

@@ -236,7 +242,9 @@ func (c *Config) configureProviders(store *ConfigStore, env env.Env, resolver Va

236 switch {

242 switch {

237 case p.ID == catwalk.InferenceProviderAnthropic && config.OAuthToken != nil:

243 case p.ID == catwalk.InferenceProviderAnthropic && config.OAuthToken != nil:

238 // Claude Code subscription is not supported anymore. Remove to show onboarding.

244 // Claude Code subscription is not supported anymore. Remove to show onboarding.

245+ if !store.reloadInProgress {

239− store.RemoveConfigField(ScopeGlobal, "providers.anthropic")

246+ store.RemoveConfigField(ScopeGlobal, "providers.anthropic")

247+ }

240 c.Providers.Del(string(p.ID))

248 c.Providers.Del(string(p.ID))

241 continue

249 continue

242 case p.ID == catwalk.InferenceProviderCopilot && config.OAuthToken != nil:

250 case p.ID == catwalk.InferenceProviderCopilot && config.OAuthToken != nil:

@@ -553,7 +561,7 @@ func (c *Config) defaultModelSelection(knownProviders []catwalk.Provider) (large

553 return largeModel, smallModel, err

561 return largeModel, smallModel, err

554}

562}

555

563

556−func configureSelectedModels(store *ConfigStore, knownProviders []catwalk.Provider) error {

564+func configureSelectedModels(store *ConfigStore, knownProviders []catwalk.Provider, persist bool) error {

557 c := store.config

565 c := store.config

558 defaultLarge, defaultSmall, err := c.defaultModelSelection(knownProviders)

566 defaultLarge, defaultSmall, err := c.defaultModelSelection(knownProviders)

559 if err != nil {

567 if err != nil {

@@ -572,10 +580,10 @@ func configureSelectedModels(store *ConfigStore, knownProviders []catwalk.Provid

572 model := c.GetModel(large.Provider, large.Model)

580 model := c.GetModel(large.Provider, large.Model)

573 if model == nil {

581 if model == nil {

574 large = defaultLarge

582 large = defaultLarge

575− // override the model type to large

583+ if persist {

576− err := store.UpdatePreferredModel(ScopeGlobal, SelectedModelTypeLarge, large)

584+ if err := store.UpdatePreferredModel(ScopeGlobal, SelectedModelTypeLarge, large); err != nil {

577− if err != nil {

578− return fmt.Errorf("failed to update preferred large model: %w", err)

585+ return fmt.Errorf("failed to update preferred large model: %w", err)

586+ }

579 }

587 }

580 } else {

588 } else {

581 if largeModelSelected.MaxTokens > 0 {

589 if largeModelSelected.MaxTokens > 0 {

@@ -616,10 +624,10 @@ func configureSelectedModels(store *ConfigStore, knownProviders []catwalk.Provid

616 model := c.GetModel(small.Provider, small.Model)

624 model := c.GetModel(small.Provider, small.Model)

617 if model == nil {

625 if model == nil {

618 small = defaultSmall

626 small = defaultSmall

619− // override the model type to small

627+ if persist {

620− err := store.UpdatePreferredModel(ScopeGlobal, SelectedModelTypeSmall, small)

628+ if err := store.UpdatePreferredModel(ScopeGlobal, SelectedModelTypeSmall, small); err != nil {

621− if err != nil {

622− return fmt.Errorf("failed to update preferred small model: %w", err)

629+ return fmt.Errorf("failed to update preferred small model: %w", err)

630+ }

623 }

631 }

624 } else {

632 } else {

625 if smallModelSelected.MaxTokens > 0 {

633 if smallModelSelected.MaxTokens > 0 {

internal/config/load_test.go+46−3

@@ -1310,6 +1310,49 @@ func TestConfig_setDefaultsDisableDefaultProvidersEnvVar(t *testing.T) {

1310}

1311

1312func TestConfig_configureSelectedModels(t *testing.T) {

1313+ t.Run("reload mode should not persist fallback defaults", func(t *testing.T) {

1314+ dir := t.TempDir()

1315+ globalPath := filepath.Join(dir, "crush.json")

1316+ require.NoError(t, os.WriteFile(globalPath, []byte(`{"models":{"large":{"provider":"ghost","model":"missing"}}}`), 0o600))

1317+

1318+ knownProviders := []catwalk.Provider{

1319+ {

1320+ ID: "openai",

1321+ APIKey: "abc",

1322+ DefaultLargeModelID: "large-model",

1323+ DefaultSmallModelID: "small-model",

1324+ Models: []catwalk.Model{

1325+ {ID: "large-model", DefaultMaxTokens: 1000},

1326+ {ID: "small-model", DefaultMaxTokens: 500},

1327+ },

1328+ },

1329+ }

1330+

1331+ cfg := &Config{

1332+ Models: map[SelectedModelType]SelectedModel{

1333+ SelectedModelTypeLarge: {Provider: "ghost", Model: "missing"},

1334+ },

1335+ }

1336+ cfg.setDefaults(dir, "")

1337+ store := &ConfigStore{config: cfg, globalDataPath: globalPath}

1338+ env := env.NewFromMap(map[string]string{})

1339+ resolver := NewEnvironmentVariableResolver(env)

1340+ err := cfg.configureProviders(store, env, resolver, knownProviders)

1341+ require.NoError(t, err)

1342+

1343+ err = configureSelectedModels(store, knownProviders, false)

1344+ require.NoError(t, err)

1345+

1346+ // In-memory falls back to default.

1347+ require.Equal(t, "openai", cfg.Models[SelectedModelTypeLarge].Provider)

1348+ require.Equal(t, "large-model", cfg.Models[SelectedModelTypeLarge].Model)

1349+

1350+ // Disk remains unchanged in reload mode.

1351+ data, readErr := os.ReadFile(globalPath)

1352+ require.NoError(t, readErr)

1353+ require.Contains(t, string(data), `"provider":"ghost"`)

1354+ require.Contains(t, string(data), `"model":"missing"`)

1355+ })

1313 t.Run("should override defaults", func(t *testing.T) {

1356 t.Run("should override defaults", func(t *testing.T) {

1314 knownProviders := []catwalk.Provider{

1357 knownProviders := []catwalk.Provider{

1315 {

1358 {

@@ -1347,7 +1390,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {

1347 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1390 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1348 require.NoError(t, err)

1391 require.NoError(t, err)

1349

1392

1350− err = configureSelectedModels(testStore(cfg), knownProviders)

1393+ err = configureSelectedModels(testStore(cfg), knownProviders, true)

1351 require.NoError(t, err)

1394 require.NoError(t, err)

1352 large := cfg.Models[SelectedModelTypeLarge]

1395 large := cfg.Models[SelectedModelTypeLarge]

1353 small := cfg.Models[SelectedModelTypeSmall]

1396 small := cfg.Models[SelectedModelTypeSmall]

@@ -1409,7 +1452,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {

1409 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1452 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1410 require.NoError(t, err)

1453 require.NoError(t, err)

1411

1454

1412− err = configureSelectedModels(testStore(cfg), knownProviders)

1455+ err = configureSelectedModels(testStore(cfg), knownProviders, true)

1413 require.NoError(t, err)

1456 require.NoError(t, err)

1414 large := cfg.Models[SelectedModelTypeLarge]

1457 large := cfg.Models[SelectedModelTypeLarge]

1415 small := cfg.Models[SelectedModelTypeSmall]

1458 small := cfg.Models[SelectedModelTypeSmall]

@@ -1454,7 +1497,7 @@ func TestConfig_configureSelectedModels(t *testing.T) {

1454 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1497 err := cfg.configureProviders(testStore(cfg), env, resolver, knownProviders)

1455 require.NoError(t, err)

1498 require.NoError(t, err)

1456

1499

1457− err = configureSelectedModels(testStore(cfg), knownProviders)

1500+ err = configureSelectedModels(testStore(cfg), knownProviders, true)

1458 require.NoError(t, err)

1501 require.NoError(t, err)

1459 large := cfg.Models[SelectedModelTypeLarge]

1502 large := cfg.Models[SelectedModelTypeLarge]

1460 require.Equal(t, "large-model", large.Model)

1503 require.Equal(t, "large-model", large.Model)

internal/config/store.go+69−4

@@ -48,6 +48,8 @@ type ConfigStore struct {

48 overrides RuntimeOverrides

49 trackedConfigPaths []string // unique, normalized config file paths

50 snapshots map[string]fileSnapshot // path -> snapshot at last capture

51+ autoReloadDisabled bool // set during load/reload to prevent re-entrancy

52+ reloadInProgress bool // set during reload to avoid disk writes mid-reload

51}

53}

53// Config returns the pure-data config struct (read-only after load).

55// Config returns the pure-data config struct (read-only after load).

@@ -121,6 +123,8 @@ func (s *ConfigStore) HasConfigField(scope Scope, key string) bool {

121}

123}

122

124

123// SetConfigField sets a key/value pair in the config file for the given scope.

125// SetConfigField sets a key/value pair in the config file for the given scope.

126+// After a successful write, it automatically reloads config to keep in-memory

127+// state fresh.

124func (s *ConfigStore) SetConfigField(scope Scope, key string, value any) error {

128func (s *ConfigStore) SetConfigField(scope Scope, key string, value any) error {

125 path, err := s.configPath(scope)

129 path, err := s.configPath(scope)

126 if err != nil {

130 if err != nil {

@@ -145,10 +149,21 @@ func (s *ConfigStore) SetConfigField(scope Scope, key string, value any) error {

145 if err := os.WriteFile(path, []byte(newValue), 0o600); err != nil {

149 if err := os.WriteFile(path, []byte(newValue), 0o600); err != nil {

146 return fmt.Errorf("failed to write config file: %w", err)

150 return fmt.Errorf("failed to write config file: %w", err)

147 }

151 }

152+

153+ // Auto-reload to keep in-memory state fresh after config edits.

154+ // We use context.Background() since this is an internal operation that

155+ // shouldn't be cancelled by user context.

156+ if err := s.autoReload(context.Background()); err != nil {

157+ // Log warning but don't fail the write - disk is already updated.

158+ slog.Warn("Config file updated but failed to reload in-memory state", "error", err)

159+ }

160+

148 return nil

161 return nil

149}

162}

150

163

151// RemoveConfigField removes a key from the config file for the given scope.

164// RemoveConfigField removes a key from the config file for the given scope.

165+// After a successful write, it automatically reloads config to keep in-memory

166+// state fresh.

152func (s *ConfigStore) RemoveConfigField(scope Scope, key string) error {

167func (s *ConfigStore) RemoveConfigField(scope Scope, key string) error {

153 path, err := s.configPath(scope)

168 path, err := s.configPath(scope)

154 if err != nil {

169 if err != nil {

@@ -169,6 +184,12 @@ func (s *ConfigStore) RemoveConfigField(scope Scope, key string) error {

169 if err := os.WriteFile(path, []byte(newValue), 0o600); err != nil {

184 if err := os.WriteFile(path, []byte(newValue), 0o600); err != nil {

170 return fmt.Errorf("failed to write config file: %w", err)

185 return fmt.Errorf("failed to write config file: %w", err)

171 }

186 }

187+

188+ // Auto-reload to keep in-memory state fresh after config edits.

189+ if err := s.autoReload(context.Background()); err != nil {

190+ slog.Warn("Config file updated but failed to reload in-memory state", "error", err)

191+ }

192+

172 return nil

193 return nil

173}

194}

174

195

@@ -529,12 +550,21 @@ func (s *ConfigStore) captureStalenessSnapshot(paths []string) {

529}

550}

530

551

531// ReloadFromDisk re-runs the config load/merge flow and updates the in-memory

552// ReloadFromDisk re-runs the config load/merge flow and updates the in-memory

532−// config safely. It rebuilds the staleness snapshot after successful reload.

553+// config atomically. It rebuilds the staleness snapshot after successful reload.

554+// On failure, the store state is rolled back to its previous state.

533func (s *ConfigStore) ReloadFromDisk(ctx context.Context) error {

555func (s *ConfigStore) ReloadFromDisk(ctx context.Context) error {

534 if s.workingDir == "" {

556 if s.workingDir == "" {

535 return fmt.Errorf("cannot reload: working directory not set")

557 return fmt.Errorf("cannot reload: working directory not set")

536 }

558 }

537

559

560+ // Disable auto-reload during reload to prevent nested/re-entrant calls.

561+ s.autoReloadDisabled = true

562+ s.reloadInProgress = true

563+ defer func() {

564+ s.autoReloadDisabled = false

565+ s.reloadInProgress = false

566+ }()

567+

538 configPaths := lookupConfigs(s.workingDir)

568 configPaths := lookupConfigs(s.workingDir)

539 cfg, loadedPaths, err := loadFromConfigPaths(configPaths)

569 cfg, loadedPaths, err := loadFromConfigPaths(configPaths)

540 if err != nil {

570 if err != nil {

@@ -575,6 +605,14 @@ func (s *ConfigStore) ReloadFromDisk(ctx context.Context) error {

575 return fmt.Errorf("failed to configure providers during reload: %w", err)

605 return fmt.Errorf("failed to configure providers during reload: %w", err)

576 }

606 }

577

607

608+ // Save current state for potential rollback

609+ oldConfig := s.config

610+ oldLoadedPaths := s.loadedPaths

611+ oldResolver := s.resolver

612+ oldKnownProviders := s.knownProviders

613+ oldOverrides := s.overrides

614+ oldWorkspacePath := s.workspacePath

615+

578 // Update store state BEFORE running model/agent setup (so they see new config)

616 // Update store state BEFORE running model/agent setup (so they see new config)

579 s.config = cfg

617 s.config = cfg

580 s.loadedPaths = loadedPaths

618 s.loadedPaths = loadedPaths

@@ -584,13 +622,26 @@ func (s *ConfigStore) ReloadFromDisk(ctx context.Context) error {

584 s.workspacePath = workspacePath

622 s.workspacePath = workspacePath

585

623

586 // Mirror startup flow: setup models and agents against NEW config

624 // Mirror startup flow: setup models and agents against NEW config

625+ var setupErr error

587 if !cfg.IsConfigured() {

626 if !cfg.IsConfigured() {

588 slog.Warn("No providers configured after reload")

627 slog.Warn("No providers configured after reload")

589 } else {

628 } else {

590− if err := configureSelectedModels(s, providers); err != nil {

629+ if err := configureSelectedModels(s, providers, false); err != nil {

591− return fmt.Errorf("failed to configure selected models during reload: %w", err)

630+ setupErr = fmt.Errorf("failed to configure selected models during reload: %w", err)

631+ } else {

632+ s.SetupAgents()

592 }

633 }

593− s.SetupAgents()

634+ }

635+

636+ // Rollback on setup failure

637+ if setupErr != nil {

638+ s.config = oldConfig

639+ s.loadedPaths = oldLoadedPaths

640+ s.resolver = oldResolver

641+ s.knownProviders = oldKnownProviders

642+ s.overrides = oldOverrides

643+ s.workspacePath = oldWorkspacePath

644+ return setupErr

594 }

645 }

595

646

596 // Rebuild staleness tracking

647 // Rebuild staleness tracking

@@ -598,3 +649,17 @@ func (s *ConfigStore) ReloadFromDisk(ctx context.Context) error {

598

649

599 return nil

650 return nil

600}

651}

652+

653+// autoReload conditionally reloads config from disk after writes.

654+// It returns nil (no error) for expected skip cases: when auto-reload is

655+// disabled during load/reload flows, or when working directory is not set

656+// (e.g., during testing). Only actual reload failures return an error.

657+func (s *ConfigStore) autoReload(ctx context.Context) error {

658+ if s.autoReloadDisabled {

659+ return nil // Expected skip: already in load/reload flow

660+ }

661+ if s.workingDir == "" {

662+ return nil // Expected skip: working directory not set

663+ }

664+ return s.ReloadFromDisk(ctx)

665+}

internal/config/store_test.go+135

@@ -375,3 +375,138 @@ func TestReloadFromDisk_UsesNewConfigValues(t *testing.T) {

375 require.Equal(t, "anthropic", store.config.Models[SelectedModelTypeLarge].Provider)

376 require.Equal(t, "claude-3", store.config.Models[SelectedModelTypeLarge].Model)

377}

378+

379+// TestSetConfigField_AutoReloads verifies that SetConfigField automatically

380+// reloads config into memory after writing, so subsequent reads see the new value.

381+func TestSetConfigField_AutoReloads(t *testing.T) {

382+ t.Parallel()

383+

384+ dir := t.TempDir()

385+ configPath := filepath.Join(dir, "crush.json")

386+

387+ // Create initial config file with debug = false

388+ initialConfig := `{"options": {"debug": false}}`

389+ require.NoError(t, os.WriteFile(configPath, []byte(initialConfig), 0o600))

390+

391+ // Load initial config

392+ store, err := Load(dir, dir, false)

393+ require.NoError(t, err)

394+

395+ // Verify initial state

396+ require.False(t, store.config.Options.Debug)

397+

398+ // Set globalDataPath and capture snapshot for staleness tracking

399+ store.globalDataPath = configPath

400+ store.CaptureStalenessSnapshot([]string{configPath})

401+

402+ // Use SetConfigField to change debug to true

403+ err = store.SetConfigField(ScopeGlobal, "options.debug", true)

404+ require.NoError(t, err)

405+

406+ // Verify in-memory state was automatically reloaded and reflects the change

407+ require.True(t, store.config.Options.Debug, "Expected config to auto-reload and show debug = true")

408+

409+ // Verify staleness is clean after the reload

410+ staleness := store.ConfigStaleness()

411+ require.False(t, staleness.Dirty, "Expected staleness to be clean after auto-reload")

412+}

413+

414+// TestRemoveConfigField_AutoReloads verifies that RemoveConfigField automatically

415+// reloads config into memory after writing.

416+func TestRemoveConfigField_AutoReloads(t *testing.T) {

417+ t.Parallel()

418+

419+ dir := t.TempDir()

420+ configPath := filepath.Join(dir, "crush.json")

421+

422+ // Create initial config file with a custom option

423+ initialConfig := `{"options": {"debug": true, "custom_field": "value"}}`

424+ require.NoError(t, os.WriteFile(configPath, []byte(initialConfig), 0o600))

425+

426+ // Load initial config

427+ store, err := Load(dir, dir, false)

428+ require.NoError(t, err)

429+

430+ // Set globalDataPath and capture snapshot

431+ store.globalDataPath = configPath

432+ store.CaptureStalenessSnapshot([]string{configPath})

433+

434+ // Verify the field exists initially (indirectly - store loaded successfully)

435+ require.True(t, store.config.Options.Debug)

436+

437+ // Remove the debug field

438+ err = store.RemoveConfigField(ScopeGlobal, "options.debug")

439+ require.NoError(t, err)

440+

441+ // Verify auto-reload occurred and stale state is clean

442+ staleness := store.ConfigStaleness()

443+ require.False(t, staleness.Dirty, "Expected staleness to be clean after auto-reload from RemoveConfigField")

444+}

445+

446+// TestSetConfigField_AutoReloadSkipsWhenNoWorkingDir verifies that auto-reload

447+// gracefully skips when working directory is not set (e.g., during testing).

448+func TestSetConfigField_AutoReloadSkipsWhenNoWorkingDir(t *testing.T) {

449+ t.Parallel()

450+

451+ dir := t.TempDir()

452+ configPath := filepath.Join(dir, "crush.json")

453+

454+ // Create a store without working directory (like some test setups)

455+ store := &ConfigStore{

456+ config: &Config{},

457+ globalDataPath: configPath,

458+ // workingDir is empty

459+ }

460+

461+ // SetConfigField should succeed even without workingDir (auto-reload skips)

462+ err := store.SetConfigField(ScopeGlobal, "foo", "bar")

463+ require.NoError(t, err)

464+

465+ // Verify file was still written

466+ data, err := os.ReadFile(configPath)

467+ require.NoError(t, err)

468+ require.Contains(t, string(data), "foo")

469+}

470+

471+// TestAutoReloadDisabledDuringReload verifies that auto-reload is suppressed

472+// during ReloadFromDisk to prevent re-entrant/nested reload calls.

473+func TestAutoReloadDisabledDuringReload(t *testing.T) {

474+ t.Parallel()

475+

476+ dir := t.TempDir()

477+ configPath := filepath.Join(dir, "crush.json")

478+

479+ // Create initial config with a provider that will trigger config modification during reload

480+ // (simulating the anthropic OAuth token removal case)

481+ initialConfig := `{

482+ "providers": {

483+ "anthropic": {

484+ "api_key": "test-key",

485+ "oauth": {"access_token": "token", "refresh_token": "refresh"}

486+ }

487+ }

488+ }`

489+ require.NoError(t, os.WriteFile(configPath, []byte(initialConfig), 0o600))

490+

491+ // Load will trigger configureProviders which removes anthropic OAuth config

492+ // This should NOT cause infinite recursion thanks to autoReloadDisabled guard

493+ store, err := Load(dir, dir, false)

494+ require.NoError(t, err)

495+

496+ // Verify the store loaded successfully and autoReloadDisabled was unset

497+ require.False(t, store.autoReloadDisabled)

498+

499+ // Capture snapshot and verify reload also works without recursion

500+ store.globalDataPath = configPath

501+ store.CaptureStalenessSnapshot([]string{configPath})

502+

503+ // Modify file and reload - this should work without re-entrancy issues

504+ time.Sleep(10 * time.Millisecond)

505+ require.NoError(t, os.WriteFile(configPath, []byte(`{"options": {"debug": true}}`), 0o600))

506+

507+ err = store.ReloadFromDisk(context.Background())

508+ require.NoError(t, err)

509+

510+ // Verify reload completed successfully

511+ require.False(t, store.autoReloadDisabled, "autoReloadDisabled should be false after ReloadFromDisk")

512+}