@@ -51,9 +51,14 @@ type RuntimeOverrides struct {
5151// goroutine races and, together with the shared lock.File, cross-process
5252// races on the config file.
5353//
54−// reloadMu serialises ReloadFromDisk calls to prevent concurrent reloads
55−// from racing on store fields. autoReload uses TryLock on reloadMu to
56−// skip redundant reloads when one is already in progress.
54+// writeMu serialises every operation that produces a new in-memory Config:
55+// the typed copy-on-write mutators (SetCompactMode, UpdatePreferredModel,
56+// ...) and ReloadFromDisk. Typed mutators take Lock; autoReload takes
57+// TryLock so a write triggered re-entrantly during a reload (e.g.
58+// configureProviders calling RemoveConfigField) skips the nested reload
59+// instead of deadlocking. This is what lets published Configs be treated
60+// as immutable: a mutator clones, mutates the clone, and swaps it in under
61+// writeMu rather than mutating the live Config in place.
5762type ConfigStore struct {
5863 config *Config
5964 workingDir string
@@ -66,15 +71,39 @@ type ConfigStore struct {
6671 trackedConfigPaths []string // unique, normalized config file paths
6772 snapshots map[string]fileSnapshot // path -> snapshot at last capture
6873
69− mu sync.Mutex // serialises config file writes
70− reloadMu sync.Mutex // serialises ReloadFromDisk calls
74+ // configMu guards the config pointer field against concurrent
75+ // readers (Config) and the writeMu-serialised swap (setConfig). It
76+ // protects the pointer word only; the pointed-to Config is treated
77+ // as immutable once published, since both reloads and typed mutators
78+ // build a fresh Config rather than mutating the live one.
79+ configMu sync.RWMutex
80+
81+ mu sync.Mutex // serialises config file writes
82+ writeMu sync.Mutex // serialises in-memory config production (mutators + reload)
7183}
7284
7385// Config returns the pure-data config struct (read-only after load).
86+//
87+// The pointer read is guarded by configMu so it can never tear against
88+// the reload swap in reloadFromDiskLocked. Reloads build a brand-new
89+// Config and swap it in rather than mutating the live one, so holding the
90+// returned pointer stays safe even across a concurrent reload — the reader
91+// keeps reading its (now immutable) snapshot.
7492func (s *ConfigStore) Config() *Config {
93+ s.configMu.RLock()
94+ defer s.configMu.RUnlock()
7595 return s.config
7696}
7797
98+// setConfig atomically swaps the active config pointer under configMu.
99+// Used by the reload path; in-place field mutators leave the pointer
100+// untouched and run under mu instead.
101+func (s *ConfigStore) setConfig(cfg *Config) {
102+ s.configMu.Lock()
103+ defer s.configMu.Unlock()
104+ s.config = cfg
105+}
106+
78107// WorkingDir returns the current working directory.
79108func (s *ConfigStore) WorkingDir() string {
80109 return s.workingDir
@@ -100,7 +129,7 @@ func (s *ConfigStore) KnownProviders() []catwalk.Provider {
100129
101130// SetupAgents configures the coder and task agents on the config.
102131func (s *ConfigStore) SetupAgents() {
103− s.config.SetupAgents()
132+ s.Config().SetupAgents()
104133}
105134
106135// Overrides returns the runtime overrides for this store.
@@ -212,25 +241,36 @@ func (s *ConfigStore) SetConfigField(scope Scope, key string, value any) error {
212241 return s.SetConfigFields(scope, map[string]any{key: value})
213242}
214243
215−// SetConfigFields sets multiple key/value pairs in the config file for the given
216−// scope in a single write. After a successful write, it automatically reloads
217−// config to keep in-memory state fresh. This is preferred over multiple
218−// SetConfigField calls when writing several fields atomically to avoid
219−// intermediate reloads with partial state.
244+// SetConfigFields sets multiple key/value pairs in the config file for the
245+// given scope in a single write, then reloads in-memory state from disk.
246+//
247+// Use this for arbitrary external edits where the in-memory effect of the
248+// change is not known ahead of time. The typed mutators (which know exactly
249+// what changed) go through update instead and skip the reload.
220250//
221251// The write is protected by an in-process mutex and a cross-process flock
222252// to prevent races between concurrent writers in different processes.
223253func (s *ConfigStore) SetConfigFields(scope Scope, kv map[string]any) error {
224− return s.setConfigFields(scope, kv, true)
254+ if err := s.writeConfigFields(scope, kv); err != nil {
255+ return err
256+ }
257+ // Auto-reload to keep in-memory state fresh after config edits.
258+ // We use context.Background() since this is an internal operation that
259+ // shouldn't be cancelled by user context.
260+ if err := s.autoReload(context.Background()); err != nil {
261+ // Log warning but don't fail the write - disk is already updated.
262+ slog.Warn("Config file updated but failed to reload in-memory state", "error", err)
263+ }
264+ return nil
225265}
226266
227−// setConfigFields performs the write and, when reload is true, refreshes
228−// in-memory state from disk. Callers that have already updated the relevant
229−// in-memory state themselves can pass reload=false to skip the expensive
230−// full reparse (which rebuilds the provider catalog and agents). The disk
231−// is the source of truth for other processes either way; skipping reload
232−// only avoids re-deriving state this process already holds.
233−func (s *ConfigStore) setConfigFields(scope Scope, kv map[string]any, reload bool) error {
267+// writeConfigFields persists key/value pairs to the config file. It does not
268+// touch in-memory config state or the staleness snapshot: callers either
269+// reload (SetConfigFields, whose reload recaptures the snapshot) or have
270+// already published an updated clone and capture the snapshot themselves
271+// (update). Both of those run under writeMu, which is what keeps the
272+// snapshot map free of concurrent writers.
273+func (s *ConfigStore) writeConfigFields(scope Scope, kv map[string]any) error {
234274 // Sort keys for deterministic output regardless of map iteration
235275 // order. This also ensures consistent results when callers pass
236276 // overlapping JSONPath keys (e.g. "a" and "a.b").
@@ -240,42 +280,71 @@ func (s *ConfigStore) setConfigFields(scope Scope, kv map[string]any, reload boo
240280 }
241281 slices.Sort(keys)
242282
243− err := s.atomicWrite(scope, func(data []byte) ([]byte, error) {
283+ return s.atomicWrite(scope, func(data []byte) ([]byte, error) {
244284 v := string(data)
245285 for _, key := range keys {
246286 var sErr error
247− v, sErr = sjson.Set(v, key, kv[key])
248− if sErr != nil {
287+ if v, sErr = sjson.Set(v, key, kv[key]); sErr != nil {
249288 return nil, fmt.Errorf("failed to set config field %s: %w", key, sErr)
250289 }
251290 }
252291 return []byte(v), nil
253292 })
254− if err != nil {
255− return err
256− }
293+}
257294
258− if !reload {
259− // The caller has already updated in-memory state; just refresh
260− // the staleness snapshot so the file watcher does not treat our
261− // own write as an external change.
262− if path, perr := s.configPath(scope); perr == nil {
263− s.captureStalenessSnapshot(append(slices.Clone(s.loadedPaths), path))
264− }
295+// mutateInMemory applies a copy-on-write change to the config without
296+// persisting. Under writeMu it clones the live config, lets mutate edit the
297+// clone, and publishes it. This is the single primitive every in-memory
298+// config change goes through, so a published Config is never mutated in
299+// place and readers always see a consistent snapshot.
300+func (s *ConfigStore) mutateInMemory(mutate func(*Config)) {
301+ s.writeMu.Lock()
302+ defer s.writeMu.Unlock()
303+
304+ nc := s.Config().cloneForWrite()
305+ mutate(nc)
306+ s.setConfig(nc)
307+}
308+
309+// update applies a copy-on-write change and persists the reported fields.
310+// mutate edits the clone and returns the JSON-path fields to write to disk;
311+// because the clone already reflects the change, no reload is needed.
312+// Returning an empty map publishes the clone without a disk write.
313+func (s *ConfigStore) update(scope Scope, mutate func(*Config) map[string]any) error {
314+ s.writeMu.Lock()
315+ defer s.writeMu.Unlock()
316+
317+ nc := s.Config().cloneForWrite()
318+ fields := mutate(nc)
319+ s.setConfig(nc)
320+ if len(fields) == 0 {
265321 return nil
266322 }
267−
268− // Auto-reload to keep in-memory state fresh after config edits.
269− // We use context.Background() since this is an internal operation that
270− // shouldn't be cancelled by user context.
271− if err := s.autoReload(context.Background()); err != nil {
272− // Log warning but don't fail the write - disk is already updated.
273− slog.Warn("Config file updated but failed to reload in-memory state", "error", err)
323+ if err := s.writeConfigFields(scope, fields); err != nil {
324+ return err
325+ }
326+ // Refresh the staleness snapshot so the file watcher does not treat
327+ // our own write as an external change. Safe to touch the snapshot map
328+ // here because we hold writeMu.
329+ if path, err := s.configPath(scope); err == nil {
330+ s.captureStalenessSnapshot(append(slices.Clone(s.loadedPaths), path))
274331 }
275−
276332 return nil
277333}
278334
335+// OverridePreferredModel sets the preferred model for the given type in
336+// memory only, without persisting. It is for per-run overrides (such as the
337+// non-interactive --model flags) that must not be written to the user's
338+// config file.
339+func (s *ConfigStore) OverridePreferredModel(modelType SelectedModelType, model SelectedModel) {
340+ s.mutateInMemory(func(c *Config) {
341+ if c.Models == nil {
342+ c.Models = make(map[SelectedModelType]SelectedModel)
343+ }
344+ c.Models[modelType] = model
345+ })
346+}
347+
279348// RemoveConfigField removes a key from the config file for the given scope.
280349// After a successful write, it automatically reloads config to keep in-memory
281350// state fresh.
@@ -304,47 +373,45 @@ func (s *ConfigStore) RemoveConfigField(scope Scope, key string) error {
304373// persists it to the config file at the given scope. The selected model and
305374// the recent-models list are written together in a single config write.
306375//
307−// The in-memory config is updated directly here, so the write skips the
308−// full disk reparse/reload. The reload would otherwise rebuild the entire
309−// provider catalog and agents on every model switch, which dominates the
310−// latency of selecting a model. Agents are refreshed separately by the
311−// caller (see UpdateAgentModel).
376+// The write skips the full disk reparse/reload (which would rebuild the
377+// provider catalog and agents on every model switch and dominate selection
378+// latency); agents are refreshed separately by the caller (see
379+// UpdateAgentModel).
312380func (s *ConfigStore) UpdatePreferredModel(scope Scope, modelType SelectedModelType, model SelectedModel) error {
313− s.config.Models[modelType] = model
314−
315− fields := map[string]any{
316− fmt.Sprintf("models.%s", modelType): model,
317− }
318−
319− // Compute the updated recent-models list in memory. When it changes,
320− // fold the write into the same batch so we avoid a second write.
321− if updated, changed := s.nextRecentModels(modelType, model); changed {
322− s.config.RecentModels[modelType] = updated
323− fields[fmt.Sprintf("recent_models.%s", modelType)] = updated
324− }
381+ return s.update(scope, func(c *Config) map[string]any {
382+ if c.Models == nil {
383+ c.Models = make(map[SelectedModelType]SelectedModel)
384+ }
385+ c.Models[modelType] = model
325386
326− if err := s.setConfigFields(scope, fields, false); err != nil {
327− return fmt.Errorf("failed to update preferred model: %w", err)
328− }
329− return nil
387+ fields := map[string]any{
388+ fmt.Sprintf("models.%s", modelType): model,
389+ }
390+ if updated, changed := nextRecentModels(c, modelType, model); changed {
391+ if c.RecentModels == nil {
392+ c.RecentModels = make(map[SelectedModelType][]SelectedModel)
393+ }
394+ c.RecentModels[modelType] = updated
395+ fields[fmt.Sprintf("recent_models.%s", modelType)] = updated
396+ }
397+ return fields
398+ })
330399}
331400
332401// SetCompactMode sets the compact mode setting and persists it.
333402func (s *ConfigStore) SetCompactMode(scope Scope, enabled bool) error {
334− if s.config.Options == nil {
335− s.config.Options = &Options{}
336− }
337− s.config.Options.TUI.CompactMode = enabled
338− return s.SetConfigField(scope, "options.tui.compact_mode", enabled)
403+ return s.update(scope, func(c *Config) map[string]any {
404+ c.ensureTUI().CompactMode = enabled
405+ return map[string]any{"options.tui.compact_mode": enabled}
406+ })
339407}
340408
341409// SetTransparentBackground sets the transparent background setting and persists it.
342410func (s *ConfigStore) SetTransparentBackground(scope Scope, enabled bool) error {
343− if s.config.Options == nil {
344− s.config.Options = &Options{}
345− }
346− s.config.Options.TUI.Transparent = &enabled
347− return s.SetConfigField(scope, "options.tui.transparent", enabled)
411+ return s.update(scope, func(c *Config) map[string]any {
412+ c.ensureTUI().Transparent = &enabled
413+ return map[string]any{"options.tui.transparent": enabled}
414+ })
348415}
349416
350417// SetProviderAPIKey sets the API key for a provider and persists it.
@@ -376,10 +443,11 @@ func (s *ConfigStore) SetProviderAPIKey(scope Scope, providerID string, apiKey a
376443 }
377444 }
378445
379− providerConfig, exists = s.config.Providers.Get(providerID)
446+ cfg := s.Config()
447+ providerConfig, exists = cfg.Providers.Get(providerID)
380448 if exists {
381449 setKeyOrToken()
382− s.config.Providers.Set(providerID, providerConfig)
450+ cfg.Providers.Set(providerID, providerConfig)
383451 return nil
384452 }
385453
@@ -406,7 +474,7 @@ func (s *ConfigStore) SetProviderAPIKey(scope Scope, providerID string, apiKey a
406474 } else {
407475 return fmt.Errorf("provider with ID %s not found in known providers", providerID)
408476 }
409− s.config.Providers.Set(providerID, providerConfig)
477+ cfg.Providers.Set(providerID, providerConfig)
410478 return nil
411479}
412480
@@ -420,7 +488,8 @@ func (s *ConfigStore) SetProviderAPIKey(scope Scope, providerID string, apiKey a
420488// already rotated the refresh token — the disk is re-checked under lock
421489// to recover the other process's token.
422490func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, providerID string) error {
423− providerConfig, exists := s.config.Providers.Get(providerID)
491+ cfg := s.Config()
492+ providerConfig, exists := cfg.Providers.Get(providerID)
424493 if !exists {
425494 return fmt.Errorf("provider %s not found", providerID)
426495 }
@@ -482,7 +551,7 @@ func (s *ConfigStore) RefreshOAuthToken(ctx context.Context, scope Scope, provid
482551 providerConfig.SetupGitHubCopilot()
483552 }
484553
485− s.config.Providers.Set(providerID, providerConfig)
554+ cfg.Providers.Set(providerID, providerConfig)
486555
487556 if err := s.SetConfigFields(scope, map[string]any{
488557 fmt.Sprintf("providers.%s.api_key", providerID): refreshedToken.AccessToken,
@@ -501,7 +570,7 @@ func (s *ConfigStore) applyToken(providerConfig ProviderConfig, token *oauth.Tok
501570 if providerID == string(catwalk.InferenceProviderCopilot) {
502571 providerConfig.SetupGitHubCopilot()
503572 }
504− s.config.Providers.Set(providerID, providerConfig)
573+ s.Config().Providers.Set(providerID, providerConfig)
505574 return nil
506575}
507576
@@ -541,20 +610,15 @@ func (s *ConfigStore) loadTokenFromDisk(scope Scope, providerID string) (*oauth.
541610}
542611
543612// nextRecentModels computes the recent-models list for the given type
544−// after recording the supplied model at the front, without persisting
545−// anything. It returns the new slice and whether it differs from the
546−// current list. It also lazily initializes the RecentModels map. Callers
547−// that want to persist the change can fold the result into a batched
548−// config write; recordRecentModel wraps it for the standalone case.
549−func (s *ConfigStore) nextRecentModels(modelType SelectedModelType, model SelectedModel) ([]SelectedModel, bool) {
613+// after recording the supplied model at the front, operating on the
614+// provided config without persisting anything. It returns the new slice
615+// and whether it differs from cfg's current list. Callers fold the result
616+// into a clone they are about to publish.
617+func nextRecentModels(cfg *Config, modelType SelectedModelType, model SelectedModel) ([]SelectedModel, bool) {
550618 if model.Provider == "" || model.Model == "" {
551619 return nil, false
552620 }
553621
554− if s.config.RecentModels == nil {
555− s.config.RecentModels = make(map[SelectedModelType][]SelectedModel)
556− }
557−
558622 eq := func(a, b SelectedModel) bool {
559623 return a.Provider == b.Provider && a.Model == b.Model
560624 }
@@ -564,7 +628,7 @@ func (s *ConfigStore) nextRecentModels(modelType SelectedModelType, model Select
564628 Model: model.Model,
565629 }
566630
567− current := s.config.RecentModels[modelType]
631+ current := cfg.RecentModels[modelType]
568632 withoutCurrent := slices.DeleteFunc(slices.Clone(current), func(existing SelectedModel) bool {
569633 return eq(existing, entry)
570634 })
@@ -581,25 +645,6 @@ func (s *ConfigStore) nextRecentModels(modelType SelectedModelType, model Select
581645 return updated, true
582646}
583647
584−// recordRecentModel records a model in the recent models list and persists
585−// the change. It is retained for callers that update recent models in
586−// isolation; UpdatePreferredModel batches the same change into a single
587−// config write instead.
588−func (s *ConfigStore) recordRecentModel(scope Scope, modelType SelectedModelType, model SelectedModel) error {
589− updated, changed := s.nextRecentModels(modelType, model)
590− if !changed {
591− return nil
592− }
593−
594− s.config.RecentModels[modelType] = updated
595−
596− if err := s.SetConfigField(scope, fmt.Sprintf("recent_models.%s", modelType), updated); err != nil {
597− return fmt.Errorf("failed to persist recent models: %w", err)
598− }
599−
600− return nil
601−}
602−
603648// NewTestStore creates a ConfigStore for testing purposes.
604649func NewTestStore(cfg *Config, loadedPaths ...string) *ConfigStore {
605650 return &ConfigStore{
@@ -777,17 +822,17 @@ func (s *ConfigStore) captureStalenessSnapshot(paths []string) {
777822// ReloadFromDisk re-runs the config load/merge flow and updates the in-memory
778823// config atomically. It rebuilds the staleness snapshot after successful reload.
779824// On failure, the store state is rolled back to its previous state.
780−// Concurrent calls are serialised via reloadMu.
825+// Concurrent calls are serialised via writeMu.
781826func (s *ConfigStore) ReloadFromDisk(ctx context.Context) error {
782827 if s.workingDir == "" {
783828 return fmt.Errorf("cannot reload: working directory not set")
784829 }
785− s.reloadMu.Lock()
786− defer s.reloadMu.Unlock()
830+ s.writeMu.Lock()
831+ defer s.writeMu.Unlock()
787832 return s.reloadFromDiskLocked(ctx)
788833}
789834
790−// reloadFromDiskLocked performs the actual reload. Caller must hold reloadMu.
835+// reloadFromDiskLocked performs the actual reload. Caller must hold writeMu.
791836func (s *ConfigStore) reloadFromDiskLocked(ctx context.Context) error {
792837 // Migrate deprecated disable_notifications before reloading config.
793838 migrateDisableNotifications()
@@ -800,8 +845,8 @@ func (s *ConfigStore) reloadFromDiskLocked(ctx context.Context) error {
800845
801846 // Apply defaults (using existing data directory if set)
802847 var dataDir string
803− if s.config != nil && s.config.Options != nil {
804− dataDir = s.config.Options.DataDirectory
848+ if cur := s.Config(); cur != nil && cur.Options != nil {
849+ dataDir = cur.Options.DataDirectory
805850 }
806851 cfg.setDefaults(s.workingDir, dataDir)
807852
@@ -842,7 +887,7 @@ func (s *ConfigStore) reloadFromDiskLocked(ctx context.Context) error {
842887 }
843888
844889 // Save current state for potential rollback
845− oldConfig := s.config
890+ oldConfig := s.Config()
846891 oldLoadedPaths := s.loadedPaths
847892 oldResolver := s.resolver
848893 oldKnownProviders := s.knownProviders
@@ -850,7 +895,7 @@ func (s *ConfigStore) reloadFromDiskLocked(ctx context.Context) error {
850895 oldWorkspacePath := s.workspacePath
851896
852897 // Update store state BEFORE running model/agent setup (so they see new config)
853− s.config = cfg
898+ s.setConfig(cfg)
854899 s.loadedPaths = loadedPaths
855900 s.resolver = resolver
856901 s.knownProviders = providers
@@ -871,7 +916,7 @@ func (s *ConfigStore) reloadFromDiskLocked(ctx context.Context) error {
871916
872917 // Rollback on setup failure
873918 if setupErr != nil {
874− s.config = oldConfig
919+ s.setConfig(oldConfig)
875920 s.loadedPaths = oldLoadedPaths
876921 s.resolver = oldResolver
877922 s.knownProviders = oldKnownProviders
@@ -904,9 +949,9 @@ func (s *ConfigStore) autoReload(ctx context.Context) error {
904949 // are rare and the next user action or file-watch tick will pick
905950 // up the change. Callers that need guaranteed fresh state after a
906951 // write should call ReloadFromDisk explicitly.
907− if !s.reloadMu.TryLock() {
952+ if !s.writeMu.TryLock() {
908953 return nil
909954 }
910− defer s.reloadMu.Unlock()
955+ defer s.writeMu.Unlock()
911956 return s.reloadFromDiskLocked(ctx)
912957}