diff --git a/pkg/config/config.go b/pkg/config/config.go index 76a186bad..9a69f4d86 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -310,7 +310,16 @@ func LoadConfig(path string) (*Config, error) { func SaveConfig(path string, cfg *Config) error { cfg.mu.RLock() defer cfg.mu.RUnlock() + return saveConfigLocked(path, cfg) +} +// SaveConfigLocked writes cfg to path without acquiring cfg's mutex. +// Use this when the caller manages synchronization externally. +func SaveConfigLocked(path string, cfg *Config) error { + return saveConfigLocked(path, cfg) +} + +func saveConfigLocked(path string, cfg *Config) error { data, err := json.MarshalIndent(cfg, "", " ") if err != nil { return err @@ -324,9 +333,23 @@ func SaveConfig(path string, cfg *Config) error { return os.WriteFile(path, data, 0600) } +func (c *Config) Lock() { c.mu.Lock() } +func (c *Config) Unlock() { c.mu.Unlock() } func (c *Config) RLock() { c.mu.RLock() } func (c *Config) RUnlock() { c.mu.RUnlock() } +// CopyFrom copies all configuration fields from src into c. +// The caller must hold c's write lock. src's mutex is not acquired. +func (c *Config) CopyFrom(src *Config) { + c.LLM = src.LLM + c.Agents = src.Agents + c.Channels = src.Channels + c.Gateway = src.Gateway + c.Tools = src.Tools + c.Heartbeat = src.Heartbeat + c.RateLimits = src.RateLimits +} + func (c *Config) WorkspacePath() string { c.mu.RLock() defer c.mu.RUnlock() diff --git a/pkg/gateway/handlers.go b/pkg/gateway/handlers.go index d7d260498..7c120bb8b 100644 --- a/pkg/gateway/handlers.go +++ b/pkg/gateway/handlers.go @@ -67,7 +67,15 @@ func (s *Server) handlePutConfig(w http.ResponseWriter, r *http.Request) { return } - if err := config.SaveConfig(s.configPath, &newCfg); err != nil { + // Hold write lock: save file first, then update memory only on success. + // Any concurrent GET /api/config will block until both are done. + s.cfg.Lock() + err = config.SaveConfigLocked(s.configPath, &newCfg) + if err == nil { + s.cfg.CopyFrom(&newCfg) + } + s.cfg.Unlock() + if err != nil { writeJSONError(w, http.StatusInternalServerError, "failed to save config: "+err.Error()) return } diff --git a/pkg/gateway/server_test.go b/pkg/gateway/server_test.go index 71eaa1eae..0f8970d91 100644 --- a/pkg/gateway/server_test.go +++ b/pkg/gateway/server_test.go @@ -1303,9 +1303,9 @@ func TestHandlePutConfig_MultipleSectionsSimultaneous(t *testing.T) { } } -// --- PUT /api/config: in-memory config not updated --- +// --- PUT /api/config: in-memory config updated immediately --- -func TestHandlePutConfig_InMemoryConfigUnchanged(t *testing.T) { +func TestHandlePutConfig_InMemoryConfigUpdated(t *testing.T) { cfg := config.DefaultConfig() cfg.LLM.Model = "original" @@ -1323,9 +1323,9 @@ func TestHandlePutConfig_InMemoryConfigUnchanged(t *testing.T) { t.Fatalf("status = %d, want 200", rr.Code) } - // In-memory config should still have old value (disk is updated, restart reloads) - if s.cfg.LLM.Model != "original" { - t.Errorf("in-memory model = %q, want %q (should remain unchanged until restart)", s.cfg.LLM.Model, "original") + // In-memory config should be updated immediately + if s.cfg.LLM.Model != "updated" { + t.Errorf("in-memory model = %q, want %q", s.cfg.LLM.Model, "updated") } // Disk should have new value @@ -1645,7 +1645,7 @@ func TestHandlePutConfig_MCPMapUpdate(t *testing.T) { } } -func TestHandlePutConfig_MCPMapInMemoryUnchanged(t *testing.T) { +func TestHandlePutConfig_MCPMapInMemoryUpdated(t *testing.T) { cfg := config.DefaultConfig() cfg.Tools.MCP = map[string]config.MCPServerConfig{ "original": {Command: "orig", Enabled: true}, @@ -1665,12 +1665,12 @@ func TestHandlePutConfig_MCPMapInMemoryUnchanged(t *testing.T) { t.Fatalf("status = %d, want 200", rr.Code) } - // In-memory config should NOT have the added server - if _, found := s.cfg.Tools.MCP["added"]; found { - t.Error("in-memory MCP should not contain 'added' (deep copy should prevent mutation)") + // In-memory config should now contain the added server + if _, found := s.cfg.Tools.MCP["added"]; !found { + t.Error("in-memory MCP should contain 'added' after PUT") } - if len(s.cfg.Tools.MCP) != 1 { - t.Errorf("in-memory MCP count = %d, want 1", len(s.cfg.Tools.MCP)) + if len(s.cfg.Tools.MCP) != 2 { + t.Errorf("in-memory MCP count = %d, want 2", len(s.cfg.Tools.MCP)) } }