refactor: handle remaining Config writes through Service

Kujtim Hoxha created 1 month ago

Add OverrideModel to Service for non-interactive model overrides.
Move Permissions writes in root.go to use Service.SetPermissions.
Remove unused setConfigField, removeConfigField, configStore, store,
and dataConfigDir from Config.

🐾 Generated with Crush

Assisted-by: Claude Opus 4.6 via Crush <[e-mail]>

Change summary

internal/app/app.go        | 10 +++++-----
internal/cmd/root.go       |  9 ++++-----
internal/config/config.go  | 17 -----------------
internal/config/load.go    |  7 ++-----
internal/config/service.go |  6 ++++++
5 files changed, 17 insertions(+), 32 deletions(-)

Detailed changes

internal/app/app.go

@@ -345,10 +345,10 @@ func (app *App) overrideModelsForNonInteractive(ctx context.Context, largeModel,
 		}
 		largeProviderID = found.provider
 		slog.Info("Overriding large model for non-interactive run", "provider", found.provider, "model", found.modelID)
-		app.config.Models[config.SelectedModelTypeLarge] = config.SelectedModel{
+		app.configService.OverrideModel(config.SelectedModelTypeLarge, config.SelectedModel{
 			Provider: found.provider,
 			Model:    found.modelID,
-		}
+		})
 	}
 
 	// Override small model.
@@ -359,15 +359,15 @@ func (app *App) overrideModelsForNonInteractive(ctx context.Context, largeModel,
 			return err
 		}
 		slog.Info("Overriding small model for non-interactive run", "provider", found.provider, "model", found.modelID)
-		app.config.Models[config.SelectedModelTypeSmall] = config.SelectedModel{
+		app.configService.OverrideModel(config.SelectedModelTypeSmall, config.SelectedModel{
 			Provider: found.provider,
 			Model:    found.modelID,
-		}
+		})
 
 	case largeModel != "":
 		// No small model specified, but large model was - use provider's default.
 		smallCfg := app.GetDefaultSmallModel(largeProviderID)
-		app.config.Models[config.SelectedModelTypeSmall] = smallCfg
+		app.configService.OverrideModel(config.SelectedModelTypeSmall, smallCfg)
 	}
 
 	return app.AgentCoordinator.UpdateModels(ctx)

internal/cmd/root.go

@@ -199,12 +199,11 @@ func setupApp(cmd *cobra.Command) (*app.App, error) {
 	if err != nil {
 		return nil, err
 	}
-	cfg := svc.Config()
-
-	if cfg.Permissions == nil {
-		cfg.Permissions = &config.Permissions{}
+	if svc.Permissions() == nil {
+		svc.SetPermissions(&config.Permissions{})
 	}
-	cfg.Permissions.SkipRequests = yolo
+	p := svc.Permissions()
+	p.SkipRequests = yolo
 
 	if err := createDotCrushDir(svc.DataDirectory()); err != nil {
 		return nil, err

internal/config/config.go

@@ -384,8 +384,6 @@ type Config struct {
 	workingDir string `json:"-"`
 	// TODO: find a better way to do this this should probably not be part of the config
 	resolver       VariableResolver
-	store          Store              `json:"-"`
-	dataConfigDir  string             `json:"-"`
 	knownProviders []catwalk.Provider `json:"-"`
 }
 
@@ -461,21 +459,6 @@ func (c *Config) Resolve(key string) (string, error) {
 	return c.resolver.ResolveValue(key)
 }
 
-func (c *Config) setConfigField(key string, value any) error {
-	return SetField(c.configStore(), key, value)
-}
-
-func (c *Config) removeConfigField(key string) error {
-	return RemoveField(c.configStore(), key)
-}
-
-func (c *Config) configStore() Store {
-	if c.store == nil {
-		c.store = NewFileStore(c.dataConfigDir)
-	}
-	return c.store
-}
-
 func allToolNames() []string {
 	return []string{
 		"agent",

internal/config/load.go

@@ -46,9 +46,6 @@ func Load(workingDir, dataDir string, debug bool) (*Service, error) {
 		workingDir: workingDir,
 	}
 
-	// Keep dataConfigDir in sync for the transitional configStore() accessor.
-	cfg.dataConfigDir = GlobalConfigData()
-
 	cfg.setDefaults(workingDir, dataDir)
 
 	if debug {
@@ -562,7 +559,7 @@ func (s *Service) configureSelectedModels(knownProviders []catwalk.Provider) err
 		if model == nil {
 			large = defaultLarge
 			c.Models[SelectedModelTypeLarge] = large
-			if err := c.setConfigField(fmt.Sprintf("models.%s", SelectedModelTypeLarge), large); err != nil {
+			if err := s.SetConfigField(fmt.Sprintf("models.%s", SelectedModelTypeLarge), large); err != nil {
 				return fmt.Errorf("failed to update preferred large model: %w", err)
 			}
 		} else {
@@ -605,7 +602,7 @@ func (s *Service) configureSelectedModels(knownProviders []catwalk.Provider) err
 		if model == nil {
 			small = defaultSmall
 			c.Models[SelectedModelTypeSmall] = small
-			if err := c.setConfigField(fmt.Sprintf("models.%s", SelectedModelTypeSmall), small); err != nil {
+			if err := s.SetConfigField(fmt.Sprintf("models.%s", SelectedModelTypeSmall), small); err != nil {
 				return fmt.Errorf("failed to update preferred small model: %w", err)
 			}
 		} else {

internal/config/service.go

@@ -204,6 +204,12 @@ func (s *Service) SetPermissions(p *Permissions) {
 	s.cfg.Permissions = p
 }
 
+// OverrideModel overrides the in-memory model for the given type
+// without persisting. Used for non-interactive model overrides.
+func (s *Service) OverrideModel(modelType SelectedModelType, model SelectedModel) {
+	s.cfg.Models[modelType] = model
+}
+
 // ToolLsConfig returns the ls tool configuration.
 func (s *Service) ToolLsConfig() ToolLs {
 	return s.cfg.Tools.Ls