fix(runtime): guard Truncator swap/read and un-capture stale truncator pointer (#861)

cmd/mcpproxy/call_cmd.go

@@ -486,7 +486,7 @@ func runCallToolVariantStandalone(ctx context.Context, toolVariant string, args
 		indexManager,
 		upstreamManager,
 		cacheManager,
-		truncator,
+		func() *truncate.Truncator { return truncator },
 		logger,
 		nil, // mainServer not needed for CLI calls
 		false,

cmd/mcpproxy/code_cmd.go

@@ -253,7 +253,7 @@ func runCodeExecStandalone(globalConfig *config.Config, code string, input map[s
 		indexManager,
 		upstreamManager,
 		cacheManager,
-		truncator,
+		func() *truncate.Truncator { return truncator },
 		logger,
 		nil,
 		false,

internal/runtime/config_watcher_test.go

@@ -409,12 +409,10 @@ func TestConfigWatcher_ReloadRebuildsTruncator(t *testing.T) {
 		return rt.ConfigSnapshot().Config.ToolResponseLimit == 50
 	}, 5*time.Second, 25*time.Millisecond, "snapshot must pick up the external edit")
 
-	// The LIVE truncator must now enforce the new limit. Read it under r.mu:
-	// the reload path swaps the field under the same lock.
+	// The LIVE truncator must now enforce the new limit. Read it via the
+	// lock-free accessor: the field is an atomic.Pointer swapped on reload (#861).
 	require.Eventually(t, func() bool {
-		rt.mu.RLock()
-		tr := rt.truncator
-		rt.mu.RUnlock()
+		tr := rt.Truncator()
 		return len(tr.Truncate(content, "srv:tool", nil).TruncatedContent) < len(content)
 	}, 5*time.Second, 25*time.Millisecond,
 		"watcher reload must rebuild the truncator with the new tool_response_limit")

internal/runtime/runtime.go

@@ -14,6 +14,7 @@ import (
 	"sort"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"go.uber.org/zap"
@@ -99,11 +100,15 @@ type Runtime struct {
 	eventMu   sync.RWMutex
 	eventSubs map[chan Event]struct{}
 
-	storageManager   *storage.Manager
-	indexManager     *index.Manager
-	upstreamManager  *upstream.Manager
-	cacheManager     *cache.Manager
-	truncator        *truncate.Truncator
+	storageManager  *storage.Manager
+	indexManager    *index.Manager
+	upstreamManager *upstream.Manager
+	cacheManager    *cache.Manager
+	// truncator is swapped on config hot-reload (tool_response_limit) while the
+	// MCP serving path reads it via Truncator(). An atomic.Pointer makes the
+	// swap/read race-free (#861) and independent of r.mu, so the accessor stays
+	// lock-free on the hot path and composes with the config commit serialization.
+	truncator        atomic.Pointer[truncate.Truncator]
 	sigCache         *toolsig.Cache // Spec 085 FR-008: single process-wide signature cache (indexing warms, MCP reads)
 	secretResolver   *secret.Resolver
 	tokenizer        tokens.Tokenizer
@@ -319,7 +324,6 @@ func New(cfg *config.Config, cfgPath string, logger *zap.Logger) (*Runtime, erro
 		indexManager:     indexManager,
 		upstreamManager:  upstreamManager,
 		cacheManager:     cacheManager,
-		truncator:        truncator,
 		sigCache:         toolsig.NewCache(),
 		secretResolver:   secretResolver,
 		tokenizer:        tokenizer,
@@ -341,6 +345,7 @@ func New(cfg *config.Config, cfgPath string, logger *zap.Logger) (*Runtime, erro
 		lastGoodTools:     make(map[string][]*config.ToolMetadata),
 		profileMembership: make(map[string][]string),
 	}
+	rt.truncator.Store(truncator)
 
 	// Spec 047: drainer goroutine that publishes coalesced servers.changed
 	// events. Lifetime is tied to appCtx so it shuts down with the runtime.
@@ -635,9 +640,13 @@ func (r *Runtime) CacheManager() *cache.Manager {
 	return r.cacheManager
 }
 
-// Truncator exposes the truncator utility.
+// Truncator exposes the current tool-response truncator. It is safe to call
+// concurrently with a config hot-reload that swaps the truncator (#861): the
+// field is an atomic.Pointer, so the read never tears against the swap. Callers
+// on the serving path MUST call this at use time rather than caching the
+// returned pointer, so a hot-reloaded tool_response_limit takes effect.
 func (r *Runtime) Truncator() *truncate.Truncator {
-	return r.truncator
+	return r.truncator.Load()
 }
 
 // SignatureCache exposes the process-wide compact-signature cache (Spec 085
@@ -2586,7 +2595,9 @@ func (r *Runtime) applyComponentConfigLocked(oldCfg, newCfg *config.Config) {
 		r.logger.Info("Tool response limit changed, updating truncator",
 			zap.Int("old_limit", oldCfg.ToolResponseLimit),
 			zap.Int("new_limit", newCfg.ToolResponseLimit))
-		r.truncator = truncate.NewTruncator(newCfg.ToolResponseLimit)
+		// Atomic swap: the serving path reads via Truncator() without r.mu, so
+		// the new limit takes effect on the next tool call (#861).
+		r.truncator.Store(truncate.NewTruncator(newCfg.ToolResponseLimit))
 	}
 
 	// Apply observability usage cadence (Spec 069 A2 — hot-reloadable). The

internal/runtime/truncator_race_test.go

@@ -0,0 +1,72 @@
+package runtime
+
+import (
+	"path/filepath"
+	"sync"
+	"testing"
+
+	"go.uber.org/zap"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestTruncatorSwapRace (#861 defect 1): Runtime.Truncator() must be safe to
+// call concurrently while a config apply swaps the truncator under the lock.
+// Before the fix, Truncator() read r.truncator without synchronization while
+// ApplyConfig wrote it, a data race the -race detector flags. Run with -race.
+func TestTruncatorSwapRace(t *testing.T) {
+	tmpDir := t.TempDir()
+	cfgPath := filepath.Join(tmpDir, "config.json")
+
+	initialCfg := config.DefaultConfig()
+	initialCfg.Listen = "127.0.0.1:8080"
+	initialCfg.DataDir = tmpDir
+	initialCfg.ToolResponseLimit = 1000
+	require.NoError(t, config.SaveConfig(initialCfg, cfgPath))
+
+	rt, err := New(initialCfg, cfgPath, zap.NewNop())
+	require.NoError(t, err)
+	defer func() { _ = rt.Close() }()
+
+	const readers = 8
+	const iterations = 40
+
+	var wg sync.WaitGroup
+
+	// Readers: hammer the accessor (and use the returned value) concurrently.
+	for i := 0; i < readers; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for j := 0; j < iterations*4; j++ {
+				tr := rt.Truncator()
+				if tr != nil {
+					_ = tr.ShouldTruncate("some content that may exceed a limit")
+				}
+			}
+		}()
+	}
+
+	// Writer: repeatedly apply configs that flip the tool-response limit, which
+	// swaps the truncator each time.
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		for j := 0; j < iterations; j++ {
+			newCfg := config.DefaultConfig()
+			newCfg.Listen = "127.0.0.1:8080"
+			newCfg.DataDir = tmpDir
+			if j%2 == 0 {
+				newCfg.ToolResponseLimit = 2000
+			} else {
+				newCfg.ToolResponseLimit = 1000
+			}
+			_, applyErr := rt.ApplyConfig(newCfg, cfgPath)
+			require.NoError(t, applyErr)
+		}
+	}()
+
+	wg.Wait()
+}

internal/server/mcp.go

@@ -110,12 +110,16 @@ func mcpServerVersion() string {
 
 // MCPProxyServer implements an MCP server that acts as a proxy
 type MCPProxyServer struct {
-	server               *mcpserver.MCPServer
-	storage              *storage.Manager
-	index                *index.Manager
-	upstreamManager      *upstream.Manager
-	cacheManager         *cache.Manager
-	truncator            *truncate.Truncator
+	server          *mcpserver.MCPServer
+	storage         *storage.Manager
+	index           *index.Manager
+	upstreamManager *upstream.Manager
+	cacheManager    *cache.Manager
+	// truncatorFn resolves the current tool-response truncator at use time. It
+	// must NOT be replaced with a captured *truncate.Truncator: the runtime
+	// swaps its truncator on a tool_response_limit hot-reload, and the serving
+	// path has to observe the new limit (#861). Access via currentTruncator().
+	truncatorFn          func() *truncate.Truncator
 	outputValidator      *outputvalidation.Validator // Spec 056: output-schema validation (nil when disabled)
 	inputValidator       *inputValidator             // Spec 085 FR-013: pre-dispatch argument validation (never nil)
 	sanitisationDetector *security.Detector          // Spec 054 Track B: secret detector for redact/block (nil when neither used)
@@ -217,13 +221,24 @@ func (p *MCPProxyServer) finishToolCall(span oteltrace.Span, serverName, toolNam
 	}
 }
 
+// currentTruncator resolves the live tool-response truncator. The serving path
+// must call this at use time (never cache the result) so a hot-reloaded
+// tool_response_limit takes effect on the next tool call (#861). Returns nil
+// when no truncator is wired (standalone/test constructions may pass nil).
+func (p *MCPProxyServer) currentTruncator() *truncate.Truncator {
+	if p.truncatorFn == nil {
+		return nil
+	}
+	return p.truncatorFn()
+}
+
 // NewMCPProxyServer creates a new MCP proxy server
 func NewMCPProxyServer(
 	storage *storage.Manager,
 	index *index.Manager,
 	upstreamManager *upstream.Manager,
 	cacheManager *cache.Manager,
-	truncator *truncate.Truncator,
+	truncatorFn func() *truncate.Truncator,
 	logger *zap.Logger,
 	mainServer *Server,
 	debugSearch bool,
@@ -459,7 +474,7 @@ func NewMCPProxyServer(
 		index:                index,
 		upstreamManager:      upstreamManager,
 		cacheManager:         cacheManager,
-		truncator:            truncator,
+		truncatorFn:          truncatorFn,
 		outputValidator:      outputValidator,
 		inputValidator:       newInputValidator(logger),
 		sanitisationDetector: sanitisationDetector,
@@ -2191,7 +2206,7 @@ func (p *MCPProxyServer) handleCallToolVariant(ctx context.Context, request mcp.
 		toonDetectionText, toonDecisions = p.encodeToonBlocks(serverName, actualToolName, contentTrust, args, ctr)
 	}
 
-	forwarded, response, wasTruncated := forwardContentResult(result, p.truncator, p.cacheManager, p.logger, toolName, args)
+	forwarded, response, wasTruncated := forwardContentResult(result, p.currentTruncator(), p.cacheManager, p.logger, toolName, args)
 
 	// Spec 056: output-schema validation. Strict mode blocks a violating result
 	// (returns an error); warn mode forwards unchanged after recording a
@@ -2597,7 +2612,7 @@ func (p *MCPProxyServer) handleCallTool(ctx context.Context, request mcp.CallToo
 	legacyResponseBytes := rawByteSize(result)
 	legacyRequestBytes := rawByteSize(activityArgs)
 
-	forwarded, response, wasTruncated := forwardContentResult(result, p.truncator, p.cacheManager, p.logger, toolName, args)
+	forwarded, response, wasTruncated := forwardContentResult(result, p.currentTruncator(), p.cacheManager, p.logger, toolName, args)
 
 	// Spec 056: output-schema validation. Strict mode blocks a violating result
 	// (returns an error); warn mode forwards unchanged after recording a
@@ -4812,7 +4827,7 @@ func (p *MCPProxyServer) handleReadCache(ctx context.Context, request mcp.CallTo
 		"read_cache",
 		args,
 		len(response.Records),
-		p.truncator,
+		p.currentTruncator(),
 		p.cacheManager,
 		p.logger,
 	)

internal/server/mcp_activation_test.go

@@ -79,7 +79,7 @@ func buildMCPProxyWithActivation(t *testing.T) (*MCPProxyServer, *runtime.Runtim
 	tr := truncate.NewTruncator(0)
 
 	mainSrv := &Server{runtime: rt, logger: logger}
-	proxy := NewMCPProxyServer(rt.StorageManager(), idx, um, cm, tr, logger, mainSrv, false, cfg, rt.SignatureCache())
+	proxy := NewMCPProxyServer(rt.StorageManager(), idx, um, cm, func() *truncate.Truncator { return tr }, logger, mainSrv, false, cfg, rt.SignatureCache())
 
 	return proxy, rt, svc.ActivationDB()
 }

internal/server/mcp_auth_scope_test.go

@@ -51,7 +51,7 @@ func createTestMCPProxyServer(t *testing.T) *MCPProxyServer {
 
 	tr := truncate.NewTruncator(0)
 
-	proxy := NewMCPProxyServer(sm, idx, um, cm, tr, logger, nil, false, cfg, nil)
+	proxy := NewMCPProxyServer(sm, idx, um, cm, func() *truncate.Truncator { return tr }, logger, nil, false, cfg, nil)
 	return proxy
 }
 

internal/server/mcp_block_tools_test.go

@@ -56,7 +56,7 @@ func createTestProxyWithRuntime(t *testing.T, servers []*config.ServerConfig) (*
 	tr := truncate.NewTruncator(0)
 
 	mainSrv := &Server{runtime: rt}
-	proxy := NewMCPProxyServer(sm, idx, um, cm, tr, logger, mainSrv, false, cfg, rt.SignatureCache())
+	proxy := NewMCPProxyServer(sm, idx, um, cm, func() *truncate.Truncator { return tr }, logger, mainSrv, false, cfg, rt.SignatureCache())
 	return proxy, rt
 }
 

internal/server/mcp_code_execution_test.go

@@ -59,7 +59,7 @@ func TestCodeExecution_WithNilMainServer(t *testing.T) {
 		indexManager,
 		upstreamManager,
 		cacheManager,
-		truncator,
+		func() *truncate.Truncator { return truncator },
 		logger,
 		nil, // mainServer = nil (CLI mode)
 		false,
@@ -121,7 +121,7 @@ func newCodeExecProxy(t *testing.T) *server.MCPProxyServer {
 
 	mcpProxy := server.NewMCPProxyServer(
 		storageManager, indexManager, upstreamManager, cacheManager,
-		truncator, logger, nil, false, cfg, nil,
+		func() *truncate.Truncator { return truncator }, logger, nil, false, cfg, nil,
 	)
 	t.Cleanup(func() { mcpProxy.Close() })
 	return mcpProxy
@@ -257,7 +257,7 @@ func TestCodeExecution_TypeScript(t *testing.T) {
 		indexManager,
 		upstreamManager,
 		cacheManager,
-		truncator,
+		func() *truncate.Truncator { return truncator },
 		logger,
 		nil,
 		false,
@@ -319,7 +319,7 @@ func TestCodeExecution_JavaScriptBackwardCompat(t *testing.T) {
 		indexManager,
 		upstreamManager,
 		cacheManager,
-		truncator,
+		func() *truncate.Truncator { return truncator },
 		logger,
 		nil,
 		false,
@@ -381,7 +381,7 @@ func TestCodeExecution_InvalidLanguage(t *testing.T) {
 		indexManager,
 		upstreamManager,
 		cacheManager,
-		truncator,
+		func() *truncate.Truncator { return truncator },
 		logger,
 		nil,
 		false,

internal/server/mcp_sigcache_wiring_test.go

@@ -35,7 +35,7 @@ func newRuntimeBackedProxy(t *testing.T) (*MCPProxyServer, *runtime.Runtime) {
 		rt.IndexManager(),
 		rt.UpstreamManager(),
 		rt.CacheManager(),
-		rt.Truncator(),
+		rt.Truncator,
 		logger,
 		srv,
 		false,

internal/server/server.go

@@ -183,7 +183,7 @@ func NewServerWithConfigPath(cfg *config.Config, configPath string, logger *zap.
 		rt.IndexManager(),
 		rt.UpstreamManager(),
 		rt.CacheManager(),
-		rt.Truncator(),
+		rt.Truncator, // getter, re-read at use time so hot-reloaded limits apply (#861)
 		logger,
 		server,
 		cfg.DebugSearch,

internal/server/toon_detection_parity_test.go

@@ -55,7 +55,7 @@ func findingSet(det *security.Detector, text string) []security.Detection {
 // handleCallToolVariant runs at mcp.go ~2129-2141 when the seam returns
 // ("", nil). The result is mutated the same way the real pipeline mutates it.
 func offPathScanText(p *MCPProxyServer, serverName, toolName, contentTrust string, args map[string]interface{}, result *mcp.CallToolResult) string {
-	forwarded, response, _ := forwardContentResult(result, p.truncator, nil, nil, serverName+":"+toolName, args)
+	forwarded, response, _ := forwardContentResult(result, p.currentTruncator(), nil, nil, serverName+":"+toolName, args)
 	p.spotlightForwarded(serverName, toolName, contentTrust, forwarded)
 	return forwardedText(forwarded, response)
 }
@@ -197,7 +197,7 @@ func TestToonDetectionParity_OverLimit(t *testing.T) {
 
 	t.Run("secret survives truncation", func(t *testing.T) {
 		p := newToonProxy("off")
-		p.truncator = truncate.NewTruncator(1500)
+		p.setStaticTruncator(truncate.NewTruncator(1500))
 		payload := secretTable(40, 0, secret) // ~5KB, secret in the first row
 		require.Greater(t, len(payload), 1500, "fixture must exceed the limit")
 		mkResult := func() *mcp.CallToolResult { return toonTextResult(payload) }
@@ -209,7 +209,7 @@ func TestToonDetectionParity_OverLimit(t *testing.T) {
 
 	t.Run("secret truncated away on both paths", func(t *testing.T) {
 		p := newToonProxy("off")
-		p.truncator = truncate.NewTruncator(1500)
+		p.setStaticTruncator(truncate.NewTruncator(1500))
 		payload := secretTable(40, 39, secret) // secret in the last row, past the budget
 		require.Greater(t, len(payload), 1500, "fixture must exceed the limit")
 		mkResult := func() *mcp.CallToolResult { return toonTextResult(payload) }