feat: ability to edit prexisting mcp oauth details

Author: BearTS

core/bifrost.go

@@ -3723,6 +3723,14 @@ func (bifrost *Bifrost) UpdateMCPClient(id string, updatedConfig *schemas.MCPCli
 	return bifrost.MCPManager.UpdateClient(id, updatedConfig)
 }
 
+// UpdateMCPClientConnection reconnects an existing MCP client using updated headers
+func (bifrost *Bifrost) UpdateMCPClientConnection(id string, newConfig *schemas.MCPClientConfig) error {
+	if bifrost.MCPManager == nil {
+		return fmt.Errorf("mcp is not configured in this bifrost instance")
+	}
+	return bifrost.MCPManager.UpdateClientConnection(id, newConfig)
+}
+
 // ReconnectMCPClient attempts to reconnect an MCP client if it is disconnected.
 //
 // Parameters:

core/mcp/clientmanager.go

@@ -47,6 +47,12 @@ func (m *MCPManager) GetClients() []schemas.MCPClientState {
 // Returns:
 //   - error: Any error that occurred during reconnection
 func (m *MCPManager) ReconnectClient(id string) error {
+	// Acquire per-client reconnect/update guard before reading config snapshot.
+	if _, alreadyReconnecting := m.reconnectingClients.LoadOrStore(id, true); alreadyReconnecting {
+		return fmt.Errorf("reconnect already in progress for this client")
+	}
+	defer m.reconnectingClients.Delete(id)
+
 	m.mu.Lock()
 	client, ok := m.clientMap[id]
 	if !ok {
@@ -62,14 +68,6 @@ func (m *MCPManager) ReconnectClient(id string) error {
 	config := client.ExecutionConfig
 	m.mu.Unlock()
 
-	// Guard against concurrent reconnects for the same client from any caller
-	// (health monitor, manual API call, etc.). LoadOrStore is atomic — whichever
-	// caller arrives second gets the "already in progress" error immediately.
-	if _, alreadyReconnecting := m.reconnectingClients.LoadOrStore(id, true); alreadyReconnecting {
-		return fmt.Errorf("reconnect already in progress for this client")
-	}
-	defer m.reconnectingClients.Delete(id)
-
 	// Reconnect using the client's configuration
 	// Retry logic is handled internally by connectToMCPClient
 	if err := m.connectToMCPClient(config); err != nil {
@@ -347,6 +345,11 @@ func (m *MCPManager) DisableClient(id string) error {
 	if id == BifrostMCPClientKey {
 		return fmt.Errorf("cannot disable internal bifrost client")
 	}
+	// Use LoadOrStore (not Load) so the check and the sentinel insertion are atomic.
+	if _, alreadyInFlight := m.reconnectingClients.LoadOrStore(id, true); alreadyInFlight {
+		return fmt.Errorf("reconnect or connection credential update already in progress for MCP client %s", id)
+	}
+	defer m.reconnectingClients.Delete(id)
 
 	m.mu.Lock()
 	defer m.mu.Unlock()
@@ -482,6 +485,11 @@ func (m *MCPManager) EnableClient(id string) error {
 // Returns:
 //   - error: Any error that occurred during client update or tool retrieval
 func (m *MCPManager) UpdateClient(id string, updatedConfig *schemas.MCPClientConfig) error {
+	if _, alreadyInFlight := m.reconnectingClients.LoadOrStore(id, true); alreadyInFlight {
+		return fmt.Errorf("reconnect or connection credential update already in progress for MCP client %s", id)
+	}
+	defer m.reconnectingClients.Delete(id)
+
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
@@ -506,9 +514,17 @@ func (m *MCPManager) UpdateClient(id string, updatedConfig *schemas.MCPClientCon
 	if updatedConfig.InProcessServer != nil && updatedConfig.InProcessServer != client.ExecutionConfig.InProcessServer {
 		return fmt.Errorf("in-process server cannot be updated for client %s", id)
 	}
+	if updatedConfig.AuthType != "" && updatedConfig.AuthType != client.ExecutionConfig.AuthType {
+		return fmt.Errorf("auth_type cannot be updated for client %s", id)
+	}
 
 	oldName := client.ExecutionConfig.Name
 
+	oauthConfigID := client.ExecutionConfig.OauthConfigID
+	if updatedConfig.OauthConfigID != nil {
+		oauthConfigID = updatedConfig.OauthConfigID
+	}
+
 	// Create a new config struct (immutable pattern) to avoid race conditions
 	// with concurrent reads. Any snapshot holding the old ExecutionConfig pointer
 	// will continue to see consistent data.
@@ -519,7 +535,7 @@ func (m *MCPManager) UpdateClient(id string, updatedConfig *schemas.MCPClientCon
 		ConnectionString: client.ExecutionConfig.ConnectionString,
 		StdioConfig:      client.ExecutionConfig.StdioConfig,
 		AuthType:         client.ExecutionConfig.AuthType,
-		OauthConfigID:    client.ExecutionConfig.OauthConfigID,
+		OauthConfigID:    oauthConfigID,
 		State:            client.ExecutionConfig.State,
 		InProcessServer:  client.ExecutionConfig.InProcessServer,
 		ConfigHash:       client.ExecutionConfig.ConfigHash,
@@ -576,6 +592,73 @@ func (m *MCPManager) UpdateClient(id string, updatedConfig *schemas.MCPClientCon
 	return nil
 }
 
+// UpdateClientConnection updates auth-related fields (headers) for an existing MCP client by
+// closing the current connection and establishing a new one so the new credentials are verified
+// before being committed. Non-credential metadata (name, tools, etc.) is preserved from the
+// current execution config.
+//
+// On failure the clientMap entry is left in Disconnected state but its ExecutionConfig is restored
+// to the previous value, allowing the health monitor to recover the client using the old credentials.
+//
+// Parameters:
+//   - id: ID of the client whose credentials should be updated
+//   - newConfig: Partial config carrying the updated auth fields (Headers). All other fields
+//     are ignored and taken from the current execution config.
+//
+// Returns:
+//   - error: Any connection error; nil on success
+func (m *MCPManager) UpdateClientConnection(id string, newConfig *schemas.MCPClientConfig) error {
+	if newConfig == nil {
+		return fmt.Errorf("newConfig must not be nil")
+	}
+	// Hold the per-client reconnect guard for the entire read + long reconnect so
+	// UpdateClient/DisableClient cannot mutate ExecutionConfig while a failed reconnect
+	// restores the pre-attempt snapshot.
+	if _, alreadyReconnecting := m.reconnectingClients.LoadOrStore(id, true); alreadyReconnecting {
+		return fmt.Errorf("reconnect already in progress for this client")
+	}
+	defer m.reconnectingClients.Delete(id)
+
+	m.mu.RLock()
+	client, ok := m.clientMap[id]
+	if !ok {
+		m.mu.RUnlock()
+		return fmt.Errorf("client %s not found", id)
+	}
+	// Per-user OAuth clients have no persistent connection — reconnect is not applicable.
+	if client.ExecutionConfig != nil && client.ExecutionConfig.AuthType == schemas.MCPAuthTypePerUserOauth {
+		m.mu.RUnlock()
+		return fmt.Errorf("connection update is not supported for per_user_oauth clients")
+	}
+	if client.ExecutionConfig == nil {
+		m.mu.RUnlock()
+		return fmt.Errorf("client %s has no execution config; cannot update connection", id)
+	}
+	// Snapshot old execution config and build the merged config while still holding the
+	// read lock so the struct copy is consistent with what the map currently holds.
+	oldConfig := client.ExecutionConfig
+	mergedConfig := *oldConfig
+	if newConfig.Headers != nil {
+		mergedConfig.Headers = maps.Clone(newConfig.Headers)
+	}
+	if newConfig.OauthConfigID != nil {
+		mergedConfig.OauthConfigID = newConfig.OauthConfigID
+	}
+	m.mu.RUnlock()
+
+	// connectToMCPClient will close the current connection and create a new clientMap entry.
+	if err := m.connectToMCPClient(&mergedConfig); err != nil {
+		m.mu.Lock()
+		if cs, exists := m.clientMap[id]; exists {
+			cs.ExecutionConfig = oldConfig
+		}
+		m.mu.Unlock()
+		return fmt.Errorf("failed to reconnect with updated credentials: %w", err)
+	}
+
+	return nil
+}
+
 func stdioConfigEqual(a, b *schemas.MCPStdioConfig) bool {
 	if a == nil || b == nil {
 		return a == b

core/mcp/interface.go

@@ -59,6 +59,9 @@ type MCPManagerInterface interface {
 	// UpdateClient updates an existing MCP client configuration
 	UpdateClient(id string, updatedConfig *schemas.MCPClientConfig) error
 
+	// UpdateClientConnection reconnects an existing MCP client using updated headers
+	UpdateClientConnection(id string, newConfig *schemas.MCPClientConfig) error
+
 	// ReconnectClient reconnects an MCP client by ID
 	ReconnectClient(id string) error
 

docs/mcp/connecting-to-servers.mdx

@@ -112,6 +112,10 @@ Use OAuth 2.0 for secure, user-based authentication with automatic token refresh
 - Admin-managed third-party connections
 - Compliance with OAuth 2.0 standards
 
+<Note>
+For existing MCP clients, the edit flow supports rotating OAuth credentials (`client_id` and `client_secret`) while preserving the same client ID. Connection type, auth type, and connection URL remain immutable after creation.
+</Note>
+
 #### Per-User OAuth
 
 Use per-user OAuth when each end-user should access the upstream service under their own account (e.g., each user's personal Notion workspace or GitHub repos). Bifrost acts as an OAuth 2.1 Authorization Server - users authenticate through a consent flow and their tokens are stored per-identity.

framework/configstore/rdb.go

@@ -1509,6 +1509,22 @@ func (s *RDBConfigStore) UpdateMCPClientConfig(ctx context.Context, id string, c
 		if err != nil {
 			return fmt.Errorf("failed to marshal tool_pricing: %w", err)
 		}
+		discoveredToolsJSON := ""
+		if clientConfig.DiscoveredTools != nil {
+			data, marshalErr := json.Marshal(clientConfig.DiscoveredTools)
+			if marshalErr != nil {
+				return fmt.Errorf("failed to marshal discovered_tools: %w", marshalErr)
+			}
+			discoveredToolsJSON = string(data)
+		}
+		toolNameMappingJSON := ""
+		if clientConfig.DiscoveredToolNameMapping != nil {
+			data, marshalErr := json.Marshal(clientConfig.DiscoveredToolNameMapping)
+			if marshalErr != nil {
+				return fmt.Errorf("failed to marshal tool_name_mapping: %w", marshalErr)
+			}
+			toolNameMappingJSON = string(data)
+		}
 
 		headersJSONStr := string(headersJSON)
 		if encrypt.IsEnabled() && headersJSONStr != "" && headersJSONStr != "{}" {
@@ -1537,6 +1553,15 @@ func (s *RDBConfigStore) UpdateMCPClientConfig(ctx context.Context, id string, c
 		if encrypt.IsEnabled() {
 			updates["encryption_status"] = encryptionStatusEncrypted
 		}
+		if clientConfigCopy.OauthConfigID != nil {
+			updates["oauth_config_id"] = clientConfigCopy.OauthConfigID
+		}
+		if discoveredToolsJSON != "" {
+			updates["discovered_tools_json"] = discoveredToolsJSON
+		}
+		if toolNameMappingJSON != "" {
+			updates["tool_name_mapping_json"] = toolNameMappingJSON
+		}
 		// Config-file driven reconciliation passes ConfigHash. In this mode we should
 		// also sync connection/auth metadata from config.json and persist the hash.
 		if clientConfigCopy.ConfigHash != "" {