fix(runtime): add config file watcher so external edits hot-reload (HOTRELOAD)

cmd/mcpproxy/telemetry_cmd.go

@@ -290,13 +290,20 @@ func runTelemetryDisable(cmd *cobra.Command, _ []string) error {
 	// command never appears to hang on the (best-effort) beacon below.
 	fmt.Println("Telemetry disabled.")
 
-	// One-time opt-out beacon. When a daemon is running it does NOT auto-reload
-	// this file (there is no fsnotify watcher), so the CLI is responsible for the
-	// beacon in the CLI-driven path. Route it through the SAME guarded server-side
-	// entry point (EmitOptOutBeacon applies the dev-build/semver, env, and
-	// anon-id guards and owns the single send) rather than duplicating the send
-	// or bypassing a guard. A short timeout keeps this from blocking on a slow
-	// endpoint; the CLI is short-lived so the send must complete before exit.
+	// One-time opt-out beacon. The CLI ALWAYS sends it after persisting the
+	// disable, routing through the SAME guarded server-side entry point
+	// (EmitOptOutBeacon applies the dev-build/semver, env, and anon-id guards
+	// and owns the single send) rather than duplicating the send or bypassing
+	// a guard. Accepted trade-off (PR #857): a running daemon hot-reloads this
+	// file via the fsnotify config watcher and may ALSO emit the beacon from
+	// its NotifyConfigChanged path — that double-send is benign (the telemetry
+	// backend dedupes opt-outs by anon_id), whereas gating the CLI send on
+	// daemon detection risks DROPPING the beacon entirely on a false positive
+	// (the socket check only stat()s the path, so a stale socket file — or a
+	// daemon on the same data dir that doesn't watch this --config file —
+	// would suppress the only send). A short timeout keeps this from blocking
+	// on a slow endpoint; the CLI is short-lived so the send must complete
+	// before exit.
 	if wasEnabled {
 		ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
 		defer cancel()

cmd/mcpproxy/telemetry_cmd_test.go

@@ -2,8 +2,11 @@ package main
 
 import (
 	"encoding/json"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"sync/atomic"
 	"testing"
 
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
@@ -159,3 +162,76 @@ func TestTelemetryConfigSavePath_PrefersConfigFileFlag(t *testing.T) {
 		t.Errorf("with configFile set: got %q, want %q", got, "/elsewhere/custom.json")
 	}
 }
+
+// writeBeaconTestConfig writes a config with telemetry enabled, a real
+// anonymous_id and the opt-out beacon endpoint pointed at a test server, so
+// runTelemetryDisable's beacon send (if any) is observable and hermetic.
+func writeBeaconTestConfig(t *testing.T, path, dataDir, endpoint string) {
+	t.Helper()
+	cfg := map[string]interface{}{
+		"listen":   "127.0.0.1:0",
+		"data_dir": dataDir,
+		"telemetry": map[string]interface{}{
+			"enabled":      true,
+			"anonymous_id": "0f5b62e0-1111-4222-8333-444455556666",
+			"endpoint":     endpoint,
+		},
+	}
+	data, err := json.MarshalIndent(cfg, "", "  ")
+	if err != nil {
+		t.Fatalf("marshal config: %v", err)
+	}
+	if err := os.WriteFile(path, data, 0600); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+}
+
+// beaconTestSetup neutralizes the telemetry env kill-switches (CI is set on
+// runners and would suppress the beacon entirely), sandboxes HOME, points the
+// beacon endpoint at a counting test server, and wires --config. Returns the
+// request counter.
+func beaconTestSetup(t *testing.T) *int32 {
+	t.Helper()
+	sandboxHome(t)
+	t.Setenv("CI", "")
+	t.Setenv("DO_NOT_TRACK", "")
+	t.Setenv("MCPPROXY_TELEMETRY", "")
+
+	var hits int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		atomic.AddInt32(&hits, 1)
+		w.WriteHeader(http.StatusOK)
+	}))
+	t.Cleanup(srv.Close)
+
+	tmpDir := t.TempDir()
+	customPath := filepath.Join(tmpDir, "custom_mcp_config.json")
+	writeBeaconTestConfig(t, customPath, tmpDir, srv.URL)
+
+	prev := configFile
+	configFile = customPath
+	t.Cleanup(func() { configFile = prev })
+
+	return &hits
+}
+
+// TestRunTelemetryDisable_SendsBeacon: the CLI always sends the opt-out
+// beacon (exactly once) after persisting the disable — it does NOT gate the
+// send on daemon detection. Accepted trade-off (PR #857): a running daemon
+// may also emit the beacon on config hot-reload, and the backend dedupes by
+// anon_id; a detection false positive dropping the only send would be worse.
+func TestRunTelemetryDisable_SendsBeacon(t *testing.T) {
+	hits := beaconTestSetup(t)
+
+	if err := runTelemetryDisable(nil, nil); err != nil {
+		t.Fatalf("runTelemetryDisable: %v", err)
+	}
+	if got := atomic.LoadInt32(hits); got != 1 {
+		t.Errorf("beacon sends = %d, want 1", got)
+	}
+	// The disable itself must be persisted.
+	enabled := readTelemetryEnabled(t, configFile)
+	if enabled == nil || *enabled {
+		t.Errorf("telemetry.enabled must be persisted false")
+	}
+}

docs/configuration.md

@@ -34,6 +34,15 @@ MCPProxy looks for configuration in these locations (in order):
 
 **Note:** At first launch, MCPProxy automatically generates a minimal configuration file if none exists.
 
+### Hot-Reload on File Edits
+
+A running MCPProxy core watches `mcp_config.json` and hot-reloads external edits automatically — whether written in place (`echo ... > mcp_config.json`) or atomically (`jq ... > tmp && mv tmp mcp_config.json`, the pattern most editors use). Behavior details:
+
+- Edits are debounced for ~500 ms, so rapid write bursts collapse into a single reload.
+- Invalid JSON is rejected safely: the running configuration is kept unchanged (a warning is logged) and the watcher picks up the next valid write.
+- MCPProxy's own saves (Web UI, REST `PATCH /api/v1/config`, CLI commands) do not trigger a redundant second reload.
+- Restart-required fields (e.g. `listen`, `data_dir`) are reloaded into memory but only take effect after a restart.
+
 ---
 
 ## Basic Configuration

go.mod

@@ -12,6 +12,7 @@ require (
 	github.com/denisbrodbeck/machineid v1.0.1
 	github.com/dop251/goja v0.0.0-20260305124333-6a7976c22267
 	github.com/evanw/esbuild v0.28.1
+	github.com/fsnotify/fsnotify v1.9.0
 	github.com/gen2brain/beeep v0.11.2
 	github.com/go-chi/chi/v5 v5.3.1
 	github.com/golang-jwt/jwt/v5 v5.3.1
@@ -86,7 +87,6 @@ require (
 	github.com/dlclark/regexp2 v1.11.4 // indirect
 	github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect
 	github.com/esiqveland/notify v0.13.3 // indirect
-	github.com/fsnotify/fsnotify v1.9.0 // indirect
 	github.com/go-logr/logr v1.4.3 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-ole/go-ole v1.3.0 // indirect

internal/runtime/config_commit_race_test.go

@@ -0,0 +1,107 @@
+package runtime
+
+import (
+	"fmt"
+	"path/filepath"
+	"sync"
+	"testing"
+
+	"go.uber.org/zap"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestConfigCommit_CommitPathsConverge is a regression test for the PR #857
+// config-store divergence race.
+//
+// The runtime keeps the active config in TWO stores that must stay in sync:
+//   - configSvc (served by ConfigSnapshot / Config() / live subscribers), and
+//   - the legacy r.cfg field (served by GetConfig / the REST /api/v1/config
+//     handlers).
+//
+// Several methods commit to BOTH stores but historically did so under r.mu in
+// different orders, releasing r.mu between the two writes:
+//   - ApplyConfig       (REST apply path)      — r.cfg first, configSvc last.
+//   - ReloadConfiguration (disk / fsnotify)    — configSvc first, r.cfg last.
+//   - UpdateConfig      (registry-source CRUD) — configSvc first, r.cfg last.
+//   - SaveConfiguration (enable/quarantine/…)  — snapshot read, then configSvc
+//   - r.cfg.Servers.
+//
+// Interleaved, two of these could leave r.cfg and configSvc reporting
+// different configs permanently — the REST surface disagreeing with the live
+// components with no further event to reconcile them. The fix serializes every
+// two-store commit under a shared configCommitMu (acquired outside r.mu). This
+// test drives all four paths concurrently and asserts the two stores always
+// converge. It is a logical lost-update race, not a data race, so -race alone
+// won't flag it — the convergence assertion is what catches it. Before the fix
+// this fails on an early round; after it, it always converges.
+func TestConfigCommit_CommitPathsConverge(t *testing.T) {
+	tmpDir := t.TempDir()
+	cfgPath := filepath.Join(tmpDir, "mcp_config.json")
+
+	base := config.DefaultConfig()
+	base.Listen = "127.0.0.1:0"
+	base.DataDir = tmpDir
+	require.NoError(t, config.SaveConfig(base, cfgPath))
+
+	rt, err := New(base, cfgPath, zap.NewNop())
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = rt.Close() })
+
+	editedWithLimit := func(limit int) *config.Config {
+		c := config.DefaultConfig()
+		c.Listen = base.Listen
+		c.DataDir = base.DataDir
+		c.ToolResponseLimit = limit
+		return c
+	}
+
+	const rounds = 200
+	for i := 0; i < rounds; i++ {
+		// Distinct per-path limits make any divergence between the two stores
+		// observable regardless of which path commits last.
+		applyLimit := 100000 + i
+		updateLimit := 200000 + i
+
+		listenAddr := fmt.Sprintf("127.0.0.1:%d", 40000+i)
+
+		var wg sync.WaitGroup
+		wg.Add(5)
+		go func() {
+			defer wg.Done()
+			_ = rt.ReloadConfiguration()
+		}()
+		go func() {
+			defer wg.Done()
+			_, _ = rt.ApplyConfig(editedWithLimit(applyLimit), cfgPath)
+		}()
+		go func() {
+			defer wg.Done()
+			rt.UpdateConfig(editedWithLimit(updateLimit), cfgPath)
+		}()
+		go func() {
+			defer wg.Done()
+			_ = rt.SaveConfiguration()
+		}()
+		go func() {
+			defer wg.Done()
+			_ = rt.UpdateListenAddress(listenAddr)
+		}()
+		wg.Wait()
+
+		// Once all commits have settled, r.cfg (GetConfig / REST) and configSvc
+		// (ConfigSnapshot / subscribers) must report the same config — both the
+		// hot-reloadable ToolResponseLimit and the Listen address.
+		legacy, gerr := rt.GetConfig()
+		require.NoError(t, gerr)
+		snap := rt.ConfigSnapshot().Config
+		require.Equal(t, legacy.ToolResponseLimit, snap.ToolResponseLimit,
+			"round %d: r.cfg and configSvc diverged on ToolResponseLimit (r.cfg=%d configSvc=%d)",
+			i, legacy.ToolResponseLimit, snap.ToolResponseLimit)
+		require.Equal(t, legacy.Listen, snap.Listen,
+			"round %d: r.cfg and configSvc diverged on Listen (r.cfg=%q configSvc=%q)",
+			i, legacy.Listen, snap.Listen)
+	}
+}