fix(config): improve configuration merges

go.mod

@@ -34,7 +34,6 @@ require (
 	github.com/nxadm/tail v1.4.11
 	github.com/openai/openai-go v1.12.0
 	github.com/pressly/goose/v3 v3.26.0
-	github.com/qjebbs/go-jsons v1.0.0-alpha.4
 	github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06
 	github.com/sahilm/fuzzy v0.1.1
 	github.com/spf13/cobra v1.10.1

go.sum

@@ -243,8 +243,6 @@ github.com/posthog/posthog-go v1.6.11 h1:5G8Y3pxnOpc3S4+PK1z1dCmZRuldiWxBsqqvvSf
 github.com/posthog/posthog-go v1.6.11/go.mod h1:LcC1Nu4AgvV22EndTtrMXTy+7RGVC0MhChSw7Qk5XkY=
 github.com/pressly/goose/v3 v3.26.0 h1:KJakav68jdH0WDvoAcj8+n61WqOIaPGgH0bJWS6jpmM=
 github.com/pressly/goose/v3 v3.26.0/go.mod h1:4hC1KrritdCxtuFsqgs1R4AU5bWtTAf+cnWvfhf2DNY=
-github.com/qjebbs/go-jsons v1.0.0-alpha.4 h1:Qsb4ohRUHQODIUAsJKdKJ/SIDbsO7oGOzsfy+h1yQZs=
-github.com/qjebbs/go-jsons v1.0.0-alpha.4/go.mod h1:wNJrtinHyC3YSf6giEh4FJN8+yZV7nXBjvmfjhBIcw4=
 github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE=
 github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo=
 github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=

internal/config/config.go

@@ -1,9 +1,11 @@
 package config
 
 import (
+	"cmp"
 	"context"
 	"fmt"
 	"log/slog"
+	"maps"
 	"net/http"
 	"net/url"
 	"os"
@@ -116,6 +118,28 @@ type MCPConfig struct {
 	Headers map[string]string `json:"headers,omitempty" jsonschema:"description=HTTP headers for HTTP/SSE MCP servers"`
 }
 
+func (m MCPConfig) merge(o MCPConfig) MCPConfig {
+	// headers and env gets merged, new replacing existing values
+	maps.Copy(m.Env, o.Env)
+	maps.Copy(m.Headers, o.Headers)
+
+	// bools are true if any is true
+	m.Disabled = o.Disabled || m.Disabled
+
+	// max timeout
+	m.Timeout = max(o.Timeout, m.Timeout)
+
+	// everything else is replaced if non-zero
+	m.Command = cmp.Or(o.Command, m.Command)
+	if len(o.Args) > 0 {
+		m.Args = o.Args
+	}
+	m.Type = cmp.Or(o.Type, m.Type)
+	m.URL = cmp.Or(o.URL, m.URL)
+
+	return m
+}
+
 type LSPConfig struct {
 	Disabled    bool              `json:"disabled,omitempty" jsonschema:"description=Whether this LSP server is disabled,default=false"`
 	Command     string            `json:"command,omitempty" jsonschema:"required,description=Command to execute for the LSP server,example=gopls"`
@@ -127,6 +151,43 @@ type LSPConfig struct {
 	Options     map[string]any    `json:"options,omitempty" jsonschema:"description=LSP server-specific settings passed during initialization"`
 }
 
+func (l LSPConfig) merge(o LSPConfig) LSPConfig {
+	// all maps gets merged, new replacing existing values
+	if l.Env == nil {
+		l.Env = make(map[string]string)
+	}
+	maps.Copy(l.Env, o.Env)
+	if l.InitOptions == nil {
+		l.InitOptions = make(map[string]any)
+	}
+	maps.Copy(l.InitOptions, o.InitOptions)
+	if l.Options == nil {
+		l.Options = make(map[string]any)
+	}
+	maps.Copy(l.Options, o.Options)
+
+	// filetypes and rootmarkers get merged
+	l.RootMarkers = append(l.RootMarkers, o.RootMarkers...)
+	l.FileTypes = append(l.FileTypes, o.FileTypes...)
+	slices.Sort(l.RootMarkers)
+	slices.Sort(l.FileTypes)
+	l.RootMarkers = slices.Compact(l.RootMarkers)
+	l.FileTypes = slices.Compact(l.FileTypes)
+
+	// disabled if any disabled
+	l.Disabled = l.Disabled || o.Disabled
+
+	// args get replaced if non-empty
+	if len(o.Args) > 0 {
+		l.Args = o.Args
+	}
+
+	// command takes precedence:
+	l.Command = cmp.Or(o.Command, l.Command)
+
+	return l
+}
+
 type TUIOptions struct {
 	CompactMode bool   `json:"compact_mode,omitempty" jsonschema:"description=Enable compact mode for the TUI interface,default=false"`
 	DiffMode    string `json:"diff_mode,omitempty" jsonschema:"description=Diff mode for the TUI interface,enum=unified,enum=split"`
@@ -136,6 +197,14 @@ type TUIOptions struct {
 	Completions Completions `json:"completions,omitzero" jsonschema:"description=Completions UI options"`
 }
 
+func (o TUIOptions) merge(t TUIOptions) TUIOptions {
+	o.CompactMode = o.CompactMode || t.CompactMode
+	o.DiffMode = cmp.Or(t.DiffMode, o.DiffMode)
+	o.Completions.MaxDepth = cmp.Or(t.Completions.MaxDepth, o.Completions.MaxDepth)
+	o.Completions.MaxItems = cmp.Or(t.Completions.MaxDepth, o.Completions.MaxDepth)
+	return o
+}
+
 // Completions defines options for the completions UI.
 type Completions struct {
 	MaxDepth *int `json:"max_depth,omitempty" jsonschema:"description=Maximum depth for the ls tool,default=0,example=10"`
@@ -169,6 +238,23 @@ type Options struct {
 	DisableMetrics            bool         `json:"disable_metrics,omitempty" jsonschema:"description=Disable sending metrics,default=false"`
 }
 
+func (o Options) merge(t Options) Options {
+	o.ContextPaths = append(o.ContextPaths, t.ContextPaths...)
+	o.Debug = o.Debug || t.Debug
+	o.DebugLSP = o.DebugLSP || t.DebugLSP
+	o.DisableProviderAutoUpdate = o.DisableProviderAutoUpdate || t.DisableProviderAutoUpdate
+	o.DisableMetrics = o.DisableMetrics || t.DisableMetrics
+	o.DataDirectory = cmp.Or(t.DataDirectory, o.DataDirectory)
+	o.DisabledTools = append(o.DisabledTools, t.DisabledTools...)
+	*o.TUI = o.TUI.merge(*t.TUI)
+	if t.Attribution != nil {
+		o.Attribution = &Attribution{}
+		o.Attribution.CoAuthoredBy = o.Attribution.CoAuthoredBy || t.Attribution.CoAuthoredBy
+		o.Attribution.GeneratedWith = o.Attribution.GeneratedWith || t.Attribution.GeneratedWith
+	}
+	return o
+}
+
 type MCPs map[string]MCPConfig
 
 type MCP struct {
@@ -263,6 +349,12 @@ type Tools struct {
 	Ls ToolLs `json:"ls,omitzero"`
 }
 
+func (o Tools) merge(t Tools) Tools {
+	o.Ls.MaxDepth = cmp.Or(t.Ls.MaxDepth, o.Ls.MaxDepth)
+	o.Ls.MaxItems = cmp.Or(t.Ls.MaxDepth, o.Ls.MaxDepth)
+	return o
+}
+
 type ToolLs struct {
 	MaxDepth *int `json:"max_depth,omitempty" jsonschema:"description=Maximum depth for the ls tool,default=0,example=10"`
 	MaxItems *int `json:"max_items,omitempty" jsonschema:"description=Maximum number of items to return for the ls tool,default=1000,example=100"`
@@ -302,6 +394,42 @@ type Config struct {
 	knownProviders []catwalk.Provider `json:"-"`
 }
 
+func (c Config) merge(t Config) Config {
+	for name, mcp := range t.MCP {
+		existing, ok := c.MCP[name]
+		if !ok {
+			c.MCP[name] = mcp
+			continue
+		}
+		c.MCP[name] = existing.merge(mcp)
+	}
+	for name, lsp := range t.LSP {
+		existing, ok := c.LSP[name]
+		if !ok {
+			c.LSP[name] = lsp
+			continue
+		}
+		c.LSP[name] = existing.merge(lsp)
+	}
+	// simple override
+	maps.Copy(c.Models, t.Models)
+	c.Schema = cmp.Or(c.Schema, t.Schema)
+	if t.Options != nil {
+		*c.Options = c.Options.merge(*t.Options)
+	}
+	if t.Permissions != nil {
+		c.Permissions.AllowedTools = append(c.Permissions.AllowedTools, t.Permissions.AllowedTools...)
+	}
+	if c.Providers != nil {
+		for key, value := range t.Providers.Seq2() {
+			c.Providers.Set(key, value)
+		}
+	}
+	c.Tools = c.Tools.merge(t.Tools)
+
+	return c
+}
+
 func (c *Config) WorkingDir() string {
 	return c.workingDir
 }

internal/config/load.go

@@ -26,21 +26,6 @@ import (
 
 const defaultCatwalkURL = "https://catwalk.charm.sh"
 
-// LoadReader config via io.Reader.
-func LoadReader(fd io.Reader) (*Config, error) {
-	data, err := io.ReadAll(fd)
-	if err != nil {
-		return nil, err
-	}
-
-	var config Config
-	err = json.Unmarshal(data, &config)
-	if err != nil {
-		return nil, err
-	}
-	return &config, err
-}
-
 // Load loads the configuration from the default paths.
 func Load(workingDir, dataDir string, debug bool) (*Config, error) {
 	configPaths := lookupConfigs(workingDir)
@@ -574,12 +559,19 @@ func loadFromReaders(readers []io.Reader) (*Config, error) {
 		return &Config{}, nil
 	}
 
-	merged, err := Merge(readers)
-	if err != nil {
-		return nil, fmt.Errorf("failed to merge configuration readers: %w", err)
+	result := newConfig()
+	for _, r := range readers {
+		bts, err := io.ReadAll(r)
+		if err != nil {
+			return nil, fmt.Errorf("could not read: %w", err)
+		}
+		config := newConfig()
+		if err := json.Unmarshal(bts, &config); err != nil {
+			return nil, err
+		}
+		*result = result.merge(*config)
 	}
-
-	return LoadReader(merged)
+	return result, nil
 }
 
 func hasVertexCredentials(env env.Env) bool {
@@ -674,3 +666,17 @@ func isInsideWorktree() bool {
 	).CombinedOutput()
 	return err == nil && strings.TrimSpace(string(bts)) == "true"
 }
+
+func newConfig() *Config {
+	return &Config{
+		Agents: map[string]Agent{},
+		MCP:    map[string]MCPConfig{},
+		LSP:    map[string]LSPConfig{},
+		Models: map[SelectedModelType]SelectedModel{},
+		Options: &Options{
+			TUI: &TUIOptions{},
+		},
+		Permissions: &Permissions{},
+		Providers:   csync.NewMap[string, ProviderConfig](),
+	}
+}

internal/config/load_test.go

@@ -5,7 +5,6 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/charmbracelet/catwalk/pkg/catwalk"
@@ -22,21 +21,6 @@ func TestMain(m *testing.M) {
 	os.Exit(exitVal)
 }
 
-func TestConfig_LoadFromReaders(t *testing.T) {
-	data1 := strings.NewReader(`{"providers": {"openai": {"api_key": "key1", "base_url": "https://api.openai.com/v1"}}}`)
-	data2 := strings.NewReader(`{"providers": {"openai": {"api_key": "key2", "base_url": "https://api.openai.com/v2"}}}`)
-	data3 := strings.NewReader(`{"providers": {"openai": {}}}`)
-
-	loadedConfig, err := loadFromReaders([]io.Reader{data1, data2, data3})
-
-	require.NoError(t, err)
-	require.NotNil(t, loadedConfig)
-	require.Equal(t, 1, loadedConfig.Providers.Len())
-	pc, _ := loadedConfig.Providers.Get("openai")
-	require.Equal(t, "key2", pc.APIKey)
-	require.Equal(t, "https://api.openai.com/v2", pc.BaseURL)
-}
-
 func TestConfig_setDefaults(t *testing.T) {
 	cfg := &Config{}