fix(mcp): resync /mcp gateway tools/list on connect, reconnect, and tool-sync drift

The gateway's tools/list served a stale snapshot rebuilt only on admin config
mutations (add/update/remove/enable/disable a client). Boot-time client dialing,
manual/health-monitor reconnects, and periodic tool-sync drift all mutated a
client's live ToolMap without ever triggering a resync, so /mcp could serve
{"tools":[]} indefinitely after a clean restart until an admin toggled a client.

Adds an onToolsUpdated hook on MCPManager, coalesced through a buffered channel
so bursts of concurrent connects collapse into at most one extra resync, fired
from connectToMCPClient and performSync only on their success paths. The
transport wires this to SyncAllMCPServers in Bootstrap, and Bifrost retains the
callback so it's re-applied to any MCPManager constructed later by a lazy-init
path (AddMCPClient, VerifyPerUserOAuthConnection, VerifyHeadersConnection,
SetMCPManager) for processes that start with no MCP configured.

Fixes #4998

Author: Shaik-Sirajuddin

core/bifrost.go

@@ -89,6 +89,8 @@ type Bifrost struct {
 	MCPManager          mcp.MCPManagerInterface             // MCP integration manager (nil if MCP not configured)
 	mcpCredStore        schemas.MCPCredentialStore          // Per-call credential resolver for MCP tool execution (wraps oauth2Provider for OAuth-flavored auth types)
 	mcpInitOnce         sync.Once                           // Ensures MCP manager is initialized only once
+	mcpToolsUpdatedMu   sync.RWMutex                        // Guards mcpToolsUpdatedFn
+	mcpToolsUpdatedFn   func()                              // Set by SetOnMCPToolsUpdated; applied to MCPManager at boot and re-applied to any manager created later by a lazy-init path (AddMCPClient, VerifyPerUserOAuthConnection, VerifyHeadersConnection), since those construct a fresh MCPManager that wouldn't otherwise see a hook set before it existed
 	dropExcessRequests  atomic.Bool                         // If true, in cases where the queue is full, requests will not wait for the queue to be empty and will be dropped instead.
 	keySelector         schemas.KeySelector                 // Custom key selector function
 	keyPoolFilter       schemas.KeyPoolFilter               // optional hook to veto keys before selection (nil = all eligible)
@@ -340,6 +342,7 @@ func Init(ctx context.Context, config schemas.BifrostConfig) (*Bifrost, error) {
 			}
 			codeMode := starlark.NewStarlarkCodeMode(codeModeConfig, bifrost.logger)
 			bifrost.MCPManager = mcp.NewMCPManager(bifrostCtx, mcpConfig, bifrost.mcpCredStore, bifrost.logger, codeMode)
+			bifrost.applyMCPToolsUpdatedHook()
 			bifrost.logger.Info("MCP integration initialized successfully")
 		})
 	}
@@ -3978,6 +3981,41 @@ func (bifrost *Bifrost) ConnectConfiguredMCPClients(ctx context.Context) {
 	}
 }
 
+// SetOnMCPToolsUpdated registers a callback invoked whenever a connected MCP client's live
+// tool set changes (connect, reconnect, or periodic tool sync). Call before
+// ConnectConfiguredMCPClients so boot-time discovery is covered.
+//
+// The callback is retained on the Bifrost instance (not just handed to the current
+// MCPManager) and re-applied by applyMCPToolsUpdatedHook whenever a *new* MCPManager is
+// constructed later — AddMCPClient, VerifyPerUserOAuthConnection, and
+// VerifyHeadersConnection all lazily create one via mcpInitOnce if MCP wasn't configured
+// at boot. Without retaining the callback here, a process that starts with no MCP config
+// and has its first client added at runtime would end up with a manager whose
+// onToolsUpdated hook was never set, silently reintroducing the gateway staleness bug
+// this callback exists to fix.
+func (bifrost *Bifrost) SetOnMCPToolsUpdated(fn func()) {
+	bifrost.mcpToolsUpdatedMu.Lock()
+	bifrost.mcpToolsUpdatedFn = fn
+	bifrost.mcpToolsUpdatedMu.Unlock()
+	if bifrost.MCPManager != nil {
+		bifrost.MCPManager.SetOnToolsUpdated(fn)
+	}
+}
+
+// applyMCPToolsUpdatedHook wires any previously-registered SetOnMCPToolsUpdated callback
+// onto a freshly constructed MCPManager. Call this immediately after every
+// mcp.NewMCPManager(...) assignment to bifrost.MCPManager, including lazy-init sites, so
+// the hook is never silently missing on a manager created after boot. No-op if
+// SetOnMCPToolsUpdated was never called (e.g. non-transport embedders of core).
+func (bifrost *Bifrost) applyMCPToolsUpdatedHook() {
+	bifrost.mcpToolsUpdatedMu.RLock()
+	fn := bifrost.mcpToolsUpdatedFn
+	bifrost.mcpToolsUpdatedMu.RUnlock()
+	if fn != nil && bifrost.MCPManager != nil {
+		bifrost.MCPManager.SetOnToolsUpdated(fn)
+	}
+}
+
 func (bifrost *Bifrost) AddMCPClient(ctx context.Context, config *schemas.MCPClientConfig) error {
 	if bifrost.MCPManager == nil {
 		// Use sync.Once to ensure thread-safe initialization
@@ -3998,6 +4036,7 @@ func (bifrost *Bifrost) AddMCPClient(ctx context.Context, config *schemas.MCPCli
 			// Create Starlark CodeMode for code execution (with default config)
 			codeMode := starlark.NewStarlarkCodeMode(nil, bifrost.logger)
 			bifrost.MCPManager = mcp.NewMCPManager(bifrost.ctx, mcpConfig, bifrost.mcpCredStore, bifrost.logger, codeMode)
+			bifrost.applyMCPToolsUpdatedHook()
 		})
 	}
 
@@ -4059,6 +4098,10 @@ func (bifrost *Bifrost) SetMCPManager(manager mcp.MCPManagerInterface) {
 			},
 		)
 	}
+	// Wire any previously-registered SetOnMCPToolsUpdated callback onto this externally
+	// supplied manager too, for the same reason as the lazy-init sites: a callback set
+	// before this manager existed would otherwise be silently dropped.
+	bifrost.applyMCPToolsUpdatedHook()
 }
 
 // UpdateMCPClient updates the MCP client.
@@ -4148,6 +4191,7 @@ func (bifrost *Bifrost) VerifyPerUserOAuthConnection(ctx context.Context, config
 			}
 			codeMode := starlark.NewStarlarkCodeMode(nil, bifrost.logger)
 			bifrost.MCPManager = mcp.NewMCPManager(bifrost.ctx, mcpConfig, bifrost.mcpCredStore, bifrost.logger, codeMode)
+			bifrost.applyMCPToolsUpdatedHook()
 		})
 	}
 	if bifrost.MCPManager == nil {
@@ -4176,6 +4220,7 @@ func (bifrost *Bifrost) VerifyHeadersConnection(ctx context.Context, config *sch
 			}
 			codeMode := starlark.NewStarlarkCodeMode(nil, bifrost.logger)
 			bifrost.MCPManager = mcp.NewMCPManager(bifrost.ctx, mcpConfig, bifrost.mcpCredStore, bifrost.logger, codeMode)
+			bifrost.applyMCPToolsUpdatedHook()
 		})
 	}
 	if bifrost.MCPManager == nil {

core/mcp/clientmanager.go

@@ -1606,6 +1606,11 @@ func (m *MCPManager) connectToMCPClient(requestCtx context.Context, config *sche
 		}
 	}
 
+	// Notify the transport (if wired) so gateway-facing tool registries (e.g. the /mcp
+	// tools/list snapshot) refresh from this client's freshly discovered ToolMap instead
+	// of waiting for an unrelated admin config mutation to trigger a resync.
+	m.notifyToolsUpdated()
+
 	return nil
 }
 

core/mcp/interface.go

@@ -62,6 +62,12 @@ type MCPManagerInterface interface {
 	// PreMCPConnectionHook sees the full plugin set.
 	ConnectConfiguredClients(ctx context.Context)
 
+	// SetOnToolsUpdated registers a callback invoked whenever a client's live tool
+	// set changes (connect, reconnect, or periodic tool sync), so callers can resync
+	// any derived tool registry (e.g. a gateway's tools/list snapshot) without relying
+	// on unrelated admin config mutations to trigger it.
+	SetOnToolsUpdated(fn func())
+
 	// RemoveClient removes an MCP client by ID
 	RemoveClient(id string) error
 

core/mcp/mcp.go

@@ -51,6 +51,20 @@ type MCPManager struct {
 	// existing execute-tool hooks.
 	pluginPipelineProvider func() PluginPipeline
 	releasePluginPipeline  func(pipeline PluginPipeline)
+
+	// onToolsUpdatedMu guards onToolsUpdated so SetOnToolsUpdated (called once during
+	// transport wiring) can never race with notifyToolsUpdated (called from connect/
+	// reconnect/tool-sync goroutines throughout the process lifetime).
+	onToolsUpdatedMu sync.RWMutex
+	onToolsUpdated   func() // Optional hook invoked whenever a client's live ToolMap changes
+
+	// toolsUpdatedCh coalesces bursts of notifyToolsUpdated calls (e.g. N clients
+	// connecting concurrently at boot) into a single pending signal, so the hook fires
+	// at most once more after any in-flight run finishes rather than once per caller.
+	// Buffered at 1: a full channel means a sync is already pending or running and will
+	// observe this change too (the hook re-reads live state each time it runs), so the
+	// send is safely dropped instead of queued.
+	toolsUpdatedCh chan struct{}
 }
 
 // MCPToolFunction is a generic function type for handling tool calls with typed arguments.
@@ -103,7 +117,9 @@ func NewMCPManager(ctx context.Context, config schemas.MCPConfig, credStore sche
 		healthMonitorManager: NewHealthMonitorManager(),
 		toolSyncManager:      NewToolSyncManager(config.ToolSyncInterval),
 		credStore:            credStore,
+		toolsUpdatedCh:       make(chan struct{}, 1),
 	}
+	go manager.runToolsUpdatedDispatcher()
 	// Convert plugin pipeline provider functions to the interface expected by ToolsManager
 	var pluginPipelineProvider func() PluginPipeline
 	var releasePluginPipeline func(pipeline PluginPipeline)
@@ -205,6 +221,53 @@ func (m *MCPManager) connectConfiguredClients(ctx context.Context) {
 	wg.Wait()
 }
 
+// SetOnToolsUpdated registers a callback invoked after a client's live ToolMap changes —
+// on connect, reconnect (manual or health-monitor-driven), and periodic tool sync. Wire
+// this to the transport's gateway-registry resync (e.g. SyncAllMCPServers) so /mcp's
+// tools/list reflects live client state instead of a snapshot that only refreshes on
+// admin config mutations. Safe to call at most once during transport startup, before
+// ConnectConfiguredClients runs.
+func (m *MCPManager) SetOnToolsUpdated(fn func()) {
+	m.onToolsUpdatedMu.Lock()
+	defer m.onToolsUpdatedMu.Unlock()
+	m.onToolsUpdated = fn
+}
+
+// runToolsUpdatedDispatcher is the single consumer of toolsUpdatedCh, started once from
+// NewMCPManager. Running the hook from one dedicated goroutine (rather than inline in
+// every connect/reconnect/tool-sync caller) is what makes the channel's coalescing
+// effective: while a run is in flight, any number of new signals collapse into the one
+// already buffered, so a burst of concurrent connects triggers at most one extra run
+// after the current one finishes, not one run per caller.
+func (m *MCPManager) runToolsUpdatedDispatcher() {
+	for {
+		select {
+		case <-m.ctx.Done():
+			return
+		case <-m.toolsUpdatedCh:
+			m.onToolsUpdatedMu.RLock()
+			fn := m.onToolsUpdated
+			m.onToolsUpdatedMu.RUnlock()
+			if fn != nil {
+				fn()
+			}
+		}
+	}
+}
+
+// notifyToolsUpdated signals that a client's live ToolMap changed. It never blocks and
+// never calls the hook directly — it just pings the dispatcher, dropping the signal if
+// one is already pending/in-flight (a no-op early-return for the "already synced, or
+// about to be" case) since that pending run will read current state and cover this
+// change too. Callers must not hold m.mu (read or write) when calling this.
+func (m *MCPManager) notifyToolsUpdated() {
+	select {
+	case m.toolsUpdatedCh <- struct{}{}:
+	default:
+		// A sync is already queued or running; it will pick up this change as well.
+	}
+}
+
 // SetPluginPipeline updates the plugin pipeline provider and release function on the manager's
 // ToolsManager and CodeMode. Call this after attaching an externally-created MCPManager to a Bifrost
 // instance so that nested tool calls in code mode can run through Bifrost's plugin hooks.

core/mcp/toolsync.go

@@ -158,6 +158,12 @@ func (cts *ClientToolSyncer) performSync() {
 	} else {
 		cts.logger.Debug("%s Tool sync completed for %s: %d tools (no change)", MCPLogPrefix, cts.clientID, newToolCount)
 	}
+
+	// Notify the transport so gateway-facing tool registries pick up the change (or
+	// confirm no drift) instead of only refreshing on the next unrelated admin mutation.
+	// Fired unconditionally: the old/new count comparison above is only for logging and
+	// can miss same-count renames, so don't gate the notify on it.
+	cts.manager.notifyToolsUpdated()
 }
 
 // ToolSyncManager manages all client tool syncers

core/mcp_lazy_hook_verify_test.go

@@ -0,0 +1,125 @@
+package bifrost
+
+import (
+	"context"
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/maximhq/bifrost/core/schemas"
+)
+
+// startStubMCPServer spins up a minimal in-process JSON-RPC HTTP server that answers
+// initialize/tools/list/ping, enough for connectToMCPClient to succeed and populate a
+// real ToolMap (which is what triggers notifyToolsUpdated on the success path).
+func startStubMCPServer(t *testing.T) *httptest.Server {
+	t.Helper()
+	mux := http.NewServeMux()
+	mux.HandleFunc("/mcp", func(w http.ResponseWriter, r *http.Request) {
+		var req map[string]any
+		body, _ := io.ReadAll(r.Body)
+		_ = json.Unmarshal(body, &req)
+		method, _ := req["method"].(string)
+		id := req["id"]
+
+		w.Header().Set("Content-Type", "application/json")
+		switch method {
+		case "initialize":
+			_ = json.NewEncoder(w).Encode(map[string]any{
+				"jsonrpc": "2.0", "id": id,
+				"result": map[string]any{
+					"protocolVersion": "2025-03-26",
+					"capabilities":    map[string]any{"tools": map[string]any{}},
+					"serverInfo":      map[string]any{"name": "stub", "version": "0.1"},
+				},
+			})
+		case "notifications/initialized":
+			w.WriteHeader(http.StatusAccepted)
+		case "tools/list":
+			_ = json.NewEncoder(w).Encode(map[string]any{
+				"jsonrpc": "2.0", "id": id,
+				"result": map[string]any{
+					"tools": []map[string]any{
+						{"name": "echo", "description": "echo text", "inputSchema": map[string]any{"type": "object"}},
+					},
+				},
+			})
+		case "ping":
+			_ = json.NewEncoder(w).Encode(map[string]any{"jsonrpc": "2.0", "id": id, "result": map[string]any{}})
+		default:
+			if id == nil {
+				w.WriteHeader(http.StatusAccepted)
+				return
+			}
+			_ = json.NewEncoder(w).Encode(map[string]any{"jsonrpc": "2.0", "id": id, "error": map[string]any{"code": -32601, "message": "nope"}})
+		}
+	})
+	srv := httptest.NewServer(mux)
+	t.Cleanup(srv.Close)
+	return srv
+}
+
+// TestLazyMCPManagerGetsToolsUpdatedHook covers the #4998 fix's lazy-init gap:
+// SetOnMCPToolsUpdated, called once at boot when MCPConfig is nil (so no MCPManager
+// exists yet), must still reach an MCPManager constructed later by a lazy-init path
+// (AddMCPClient here). Without applyMCPToolsUpdatedHook wired into that lazy path, the
+// hook would be permanently nil on the manager actually serving traffic, silently
+// reintroducing the gateway staleness bug for any deployment that starts with no MCP
+// config and adds clients dynamically afterward.
+func TestLazyMCPManagerGetsToolsUpdatedHook(t *testing.T) {
+	stub := startStubMCPServer(t)
+
+	bf, err := Init(context.Background(), schemas.BifrostConfig{
+		Account: NewMockAccount(),
+		// MCPConfig intentionally nil: mirrors a process that boots with no MCP
+		// configured, so MCPManager is nil at Init time.
+	})
+	if err != nil {
+		t.Fatalf("Init failed: %v", err)
+	}
+	defer bf.Shutdown()
+
+	if bf.MCPManager != nil {
+		t.Fatalf("expected MCPManager to be nil before any client is added")
+	}
+
+	var hookCalls int32
+	done := make(chan struct{}, 1)
+	bf.SetOnMCPToolsUpdated(func() {
+		atomic.AddInt32(&hookCalls, 1)
+		select {
+		case done <- struct{}{}:
+		default:
+		}
+	})
+
+	// This is the lazy-init path under test: MCPManager is nil, so AddMCPClient
+	// constructs a fresh one via mcpInitOnce. Before the fix, that fresh manager's
+	// onToolsUpdated hook would be nil regardless of the SetOnMCPToolsUpdated call
+	// above, since it ran before this manager existed.
+	if err := bf.AddMCPClient(context.Background(), &schemas.MCPClientConfig{
+		ID:               "stub-1",
+		Name:             "stub",
+		ConnectionType:   schemas.MCPConnectionTypeHTTP,
+		ConnectionString: schemas.NewSecretVar(stub.URL + "/mcp"),
+		AuthType:         schemas.MCPAuthTypeNone,
+	}); err != nil {
+		t.Fatalf("AddMCPClient failed: %v", err)
+	}
+
+	if bf.MCPManager == nil {
+		t.Fatalf("expected AddMCPClient to lazily construct MCPManager")
+	}
+
+	select {
+	case <-done:
+		// Hook fired — the lazily-constructed manager was correctly wired.
+	case <-time.After(2 * time.Second):
+		t.Fatalf("onToolsUpdated hook never fired after AddMCPClient connected successfully; " +
+			"lazily-constructed MCPManager did not have the hook wired (hookCalls=%d)", atomic.LoadInt32(&hookCalls))
+	}
+}

transports/bifrost-http/server/server.go

@@ -1874,6 +1874,17 @@ func (s *BifrostHTTPServer) Bootstrap(ctx context.Context) error {
 		}
 		return fmt.Errorf("failed to initialize inference routes: %v", err)
 	}
+	// Keep the /mcp gateway's tool registry in sync with live client tool state.
+	// Without this, the registry only refreshes on admin config mutations (add/update/
+	// remove/enable/disable), leaving boot-time discovery, reconnects, and periodic
+	// tool-sync drift unreflected in tools/list until an unrelated admin action happens
+	// to trigger a resync. Must be wired before ConnectConfiguredMCPClients below so
+	// the very first boot-time discovery is covered.
+	s.Client.SetOnMCPToolsUpdated(func() {
+		if err := s.MCPServerHandler.SyncAllMCPServers(context.Background()); err != nil {
+			logger.Warn("failed to sync MCP servers after tools update: %v", err)
+		}
+	})
 	// Dial configured MCP clients now that every plugin is registered in the core.
 	// Construction (bifrost.Init) no longer connects MCP, so connecting here ensures
 	// each client's PreMCPConnectionHook runs against the full plugin set rather than