fix(sse): emit servers.changed event after tool discovery completes (#292)

* fix(sse): emit servers.changed event after tool discovery completes

Fix race condition where tool counts in Web UI showed stale data after
enabling/disabling servers. The issue was that SSE events were emitted
before the asynchronous tool discovery completed, causing the frontend
to fetch stats from StateView before it was updated.

Changes:
- Move emitServersChanged() inside the tool discovery goroutine in both
  HandleUpstreamServerChange() and RestartServer()
- Add "reason": "tool_discovery_complete" to event payload for clarity
- Change HandleUpstreamServerChange event type from "upstream_change"
  to "tools_indexed" to better reflect when it's emitted

The frontend now receives the SSE event only after tool counts are
accurate, ensuring consistent UI updates within ~3-5 seconds.

Fixes #285

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(stats): TotalTools only counts enabled servers' tools (#285)

Root cause fixes for stale tool counts:
- TotalTools stat now excludes disabled servers' tool counts
- DeleteServerTools called when server is disabled to remove from index

Tests added:
- Unit test: TotalTools excludes disabled servers (service_test.go)
- Unit test: Disabled server removes tools from index (tool_invalidation_test.go)
- E2E test: Full disable/enable cycle with search verification

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(ui): totalTools only counts enabled servers' tools (#285)

The frontend was summing all servers' tool_count regardless of enabled
status, causing Total Tools to show stale counts when servers are
disabled.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Author: Dumbris

frontend/src/stores/servers.ts

@@ -29,8 +29,11 @@ export const useServersStore = defineStore('servers', () => {
     servers.value.filter(s => s.quarantined)
   )
 
+  // Only count tools from enabled servers (Issue #285 fix)
   const totalTools = computed(() =>
-    servers.value.reduce((sum, server) => sum + server.tool_count, 0)
+    servers.value
+      .filter(s => s.enabled)
+      .reduce((sum, server) => sum + server.tool_count, 0)
   )
 
   // Helper: Smart merge servers to preserve object references and avoid full re-renders

internal/management/service.go

@@ -283,7 +283,10 @@ func (s *service) ListServers(ctx context.Context) ([]*contracts.Server, *contra
 		// Extract numeric fields
 		if toolCount, ok := srvRaw["tool_count"].(int); ok {
 			srv.ToolCount = toolCount
-			stats.TotalTools += toolCount
+			// Only count tools from enabled servers in the total
+			if srv.Enabled {
+				stats.TotalTools += toolCount
+			}
 		}
 		if retryCount, ok := srvRaw["retry_count"].(int); ok {
 			srv.ReconnectCount = retryCount

internal/management/service_test.go

@@ -123,6 +123,61 @@ func TestListServers(t *testing.T) {
 		assert.Equal(t, 1, stats.QuarantinedServers)
 	})
 
+	// T094: Test that TotalTools only counts enabled servers' tools (Issue #285 fix)
+	t.Run("TotalTools excludes disabled servers", func(t *testing.T) {
+		runtime := newMockRuntime()
+		runtime.servers = []map[string]interface{}{
+			{
+				"id":          "server1",
+				"name":        "enabled-server",
+				"enabled":     true,
+				"connected":   true,
+				"quarantined": false,
+				"tool_count":  30,
+			},
+			{
+				"id":          "server2",
+				"name":        "disabled-server",
+				"enabled":     false,
+				"connected":   false,
+				"quarantined": false,
+				"tool_count":  20, // This should NOT be counted in TotalTools
+			},
+			{
+				"id":          "server3",
+				"name":        "another-enabled-server",
+				"enabled":     true,
+				"connected":   true,
+				"quarantined": false,
+				"tool_count":  10,
+			},
+		}
+
+		svc := NewService(runtime, cfg, emitter, nil, logger)
+		servers, stats, err := svc.ListServers(context.Background())
+
+		require.NoError(t, err)
+		assert.Len(t, servers, 3)
+		assert.Equal(t, 3, stats.TotalServers)
+		assert.Equal(t, 2, stats.ConnectedServers)
+
+		// CRITICAL: TotalTools should only count enabled servers (30 + 10 = 40)
+		// If TotalTools is 60 (30 + 20 + 10), the fix is broken
+		assert.Equal(t, 40, stats.TotalTools, "TotalTools should only count enabled servers' tools")
+
+		// Verify individual server tool counts are preserved
+		for _, srv := range servers {
+			switch srv.Name {
+			case "enabled-server":
+				assert.Equal(t, 30, srv.ToolCount)
+			case "disabled-server":
+				assert.Equal(t, 20, srv.ToolCount)
+			case "another-enabled-server":
+				assert.Equal(t, 10, srv.ToolCount)
+			}
+		}
+	})
+
 	t.Run("runtime error", func(t *testing.T) {
 		runtime := newMockRuntime()
 		runtime.getAllError = fmt.Errorf("runtime error")

internal/runtime/lifecycle.go

@@ -906,6 +906,19 @@ func (r *Runtime) EnableServer(serverName string, enabled bool) error {
 		return fmt.Errorf("failed to reload configuration: %w", err)
 	}
 
+	// When disabling a server, remove its tools from the search index
+	// This ensures disabled server tools don't appear in search results
+	if !enabled && r.indexManager != nil {
+		if err := r.indexManager.DeleteServerTools(serverName); err != nil {
+			r.logger.Warn("Failed to remove disabled server tools from index",
+				zap.String("server", serverName),
+				zap.Error(err))
+		} else {
+			r.logger.Info("Removed disabled server tools from search index",
+				zap.String("server", serverName))
+		}
+	}
+
 	// Wait for the server to start connecting (LoadConfiguredServers spawns goroutines)
 	// This ensures callers don't race with connection establishment
 	// The goroutine needs time to spawn and then AddServer needs to initiate connection
@@ -1138,14 +1151,19 @@ func (r *Runtime) RestartServer(serverName string) error {
 
 	r.logger.Info("Successfully restarted server", zap.String("server", serverName))
 
-	// Trigger tool reindexing asynchronously
+	// Trigger tool reindexing asynchronously and emit SSE event AFTER completion
+	// to ensure frontend receives accurate tool counts (fixes stale stats race condition)
 	go func() {
 		if err := r.DiscoverAndIndexTools(r.AppContext()); err != nil {
 			r.logger.Error("Failed to reindex tools after restart", zap.Error(err))
 		}
-	}()
 
-	r.emitServersChanged("restart", map[string]any{"server": serverName})
+		// Emit event AFTER tool discovery completes so frontend gets fresh stats
+		r.emitServersChanged("restart", map[string]any{
+			"server": serverName,
+			"reason": "tool_discovery_complete",
+		})
+	}()
 
 	return nil
 }
@@ -1182,16 +1200,26 @@ func (r *Runtime) HandleUpstreamServerChange(ctx context.Context) {
 	}
 
 	r.logger.Info("Upstream server configuration changed, triggering comprehensive update")
+
+	phase := r.CurrentStatus().Phase
+	r.UpdatePhase(phase, "Upstream servers updated")
+
+	// Tool discovery runs in background goroutine, and SSE event is emitted
+	// AFTER discovery completes to ensure frontend receives accurate tool counts.
+	// This fixes the race condition where stale stats were returned because
+	// the event was emitted before StateView was updated.
 	go func() {
 		if err := r.DiscoverAndIndexTools(ctx); err != nil {
 			r.logger.Error("Failed to update tool index after upstream change", zap.Error(err))
 		}
 		r.cleanupOrphanedIndexEntries()
-	}()
 
-	phase := r.CurrentStatus().Phase
-	r.UpdatePhase(phase, "Upstream servers updated")
-	r.emitServersChanged("upstream_change", map[string]any{"phase": phase})
+		// Emit event AFTER tool discovery completes so frontend gets fresh stats
+		r.emitServersChanged("tools_indexed", map[string]any{
+			"phase":  phase,
+			"reason": "tool_discovery_complete",
+		})
+	}()
 }
 
 func (r *Runtime) cleanupOrphanedIndexEntries() {

internal/runtime/tool_invalidation_test.go

@@ -388,6 +388,55 @@ func TestToolCacheInvalidation_MultipleServers(t *testing.T) {
 	assert.Contains(t, names2, "tool_y")
 }
 
+// TestToolCacheInvalidation_DisableServerRemovesTools tests that disabling a server
+// removes its tools from the search index (Issue #285 fix).
+// This ensures disabled servers' tools don't appear in search results.
+func TestToolCacheInvalidation_DisableServerRemovesTools(t *testing.T) {
+	tempDir := t.TempDir()
+	cfg := &config.Config{
+		DataDir:           tempDir,
+		Listen:            "127.0.0.1:0",
+		ToolResponseLimit: 0,
+		Servers:           []*config.ServerConfig{},
+	}
+
+	rt, err := New(cfg, "", zap.NewNop())
+	require.NoError(t, err)
+	defer func() {
+		_ = rt.Close()
+	}()
+
+	// Setup: Index tools for a server
+	initialTools := []*config.ToolMetadata{
+		{ServerName: "test-server", Name: "tool_a", Description: "Tool A", Hash: "hash_a"},
+		{ServerName: "test-server", Name: "tool_b", Description: "Tool B", Hash: "hash_b"},
+		{ServerName: "test-server", Name: "tool_c", Description: "Tool C", Hash: "hash_c"},
+	}
+
+	err = rt.indexManager.BatchIndexTools(initialTools)
+	require.NoError(t, err)
+
+	// Verify tools are indexed
+	indexedTools, err := rt.indexManager.GetToolsByServer("test-server")
+	require.NoError(t, err)
+	assert.Len(t, indexedTools, 3, "Should have 3 tools before disable")
+
+	// Simulate disabling server by calling DeleteServerTools directly
+	// (This is what EnableServer does when enabled=false)
+	err = rt.indexManager.DeleteServerTools("test-server")
+	require.NoError(t, err)
+
+	// Verify all tools are removed
+	indexedToolsAfter, err := rt.indexManager.GetToolsByServer("test-server")
+	require.NoError(t, err)
+	assert.Len(t, indexedToolsAfter, 0, "All tools should be removed when server is disabled")
+
+	// Verify server is no longer in indexed servers list
+	indexedServers, err := rt.indexManager.GetAllIndexedServerNames()
+	require.NoError(t, err)
+	assert.NotContains(t, indexedServers, "test-server", "Disabled server should not be in indexed list")
+}
+
 // TestToolCacheInvalidation_OrphanCleanup tests that orphaned index entries are cleaned up
 // when a server is removed from the config but its tools remain in the index.
 func TestToolCacheInvalidation_OrphanCleanup(t *testing.T) {

internal/server/e2e_test.go

@@ -2543,6 +2543,154 @@ func getToolResultText(result *mcp.CallToolResult) string {
 	return ""
 }
 
+// TestE2E_DisableServerRemovesToolsFromSearch validates Issue #285 fix:
+// When a server is disabled:
+// 1. Tools from that server should be removed from search results
+// 2. TotalTools stat should only count enabled servers' tools
+// When re-enabled:
+// 3. Tools should be discoverable again after re-enable
+func TestE2E_DisableServerRemovesToolsFromSearch(t *testing.T) {
+	env := NewTestEnvironment(t)
+	defer env.Cleanup()
+
+	ctx := context.Background()
+
+	// Create mock server with distinctive tools
+	uniqueTools := []mcp.Tool{
+		{
+			Name:        "issue285_alpha_tool",
+			Description: "A unique tool for testing issue 285 disable scenario",
+			InputSchema: mcp.ToolInputSchema{Type: "object"},
+		},
+		{
+			Name:        "issue285_beta_tool",
+			Description: "Another unique tool for testing issue 285 disable",
+			InputSchema: mcp.ToolInputSchema{Type: "object"},
+		},
+	}
+
+	const serverName = "issue285-test-server"
+	mockServer := env.CreateMockUpstreamServer(serverName, uniqueTools)
+
+	mcpClient := env.CreateProxyClient()
+	defer mcpClient.Close()
+	env.ConnectClient(mcpClient)
+
+	// Add server via upstream_servers tool
+	addRequest := mcp.CallToolRequest{}
+	addRequest.Params.Name = "upstream_servers"
+	addRequest.Params.Arguments = map[string]interface{}{
+		"operation": "add",
+		"name":      serverName,
+		"url":       mockServer.addr,
+		"protocol":  "streamable-http",
+		"enabled":   true,
+	}
+
+	addResult, err := mcpClient.CallTool(ctx, addRequest)
+	require.NoError(t, err)
+	require.False(t, addResult.IsError, "Add server should succeed: %v", getToolResultText(addResult))
+
+	// Unquarantine server
+	serverConfig, err := env.proxyServer.runtime.StorageManager().GetUpstreamServer(serverName)
+	require.NoError(t, err)
+	serverConfig.Quarantined = false
+	err = env.proxyServer.runtime.StorageManager().SaveUpstreamServer(serverConfig)
+	require.NoError(t, err)
+
+	// Reload configuration
+	servers, err := env.proxyServer.runtime.StorageManager().ListUpstreamServers()
+	require.NoError(t, err)
+	cfg := env.proxyServer.runtime.Config()
+	cfg.Servers = servers
+	err = env.proxyServer.runtime.LoadConfiguredServers(cfg)
+	require.NoError(t, err)
+
+	// Wait for connection and discovery
+	time.Sleep(4 * time.Second)
+	_ = env.proxyServer.runtime.DiscoverAndIndexTools(ctx)
+	time.Sleep(2 * time.Second)
+
+	// Step 1: Verify tools are searchable when server is enabled
+	searchRequest := mcp.CallToolRequest{}
+	searchRequest.Params.Name = "retrieve_tools"
+	searchRequest.Params.Arguments = map[string]interface{}{
+		"query": "issue285_alpha",
+	}
+
+	searchResult, err := mcpClient.CallTool(ctx, searchRequest)
+	require.NoError(t, err)
+	require.False(t, searchResult.IsError, "Search should succeed")
+
+	searchText := getToolResultText(searchResult)
+	assert.Contains(t, searchText, "issue285_alpha_tool", "Tool should be searchable when server is enabled")
+	t.Log("Step 1 PASSED: Tool is searchable when server is enabled")
+
+	// Step 2: Disable the server
+	disableRequest := mcp.CallToolRequest{}
+	disableRequest.Params.Name = "upstream_servers"
+	disableRequest.Params.Arguments = map[string]interface{}{
+		"operation": "patch",
+		"name":      "issue285-test-server",
+		"enabled":   false,
+	}
+
+	disableResult, err := mcpClient.CallTool(ctx, disableRequest)
+	require.NoError(t, err)
+	require.False(t, disableResult.IsError, "Disable operation should succeed: %v", getToolResultText(disableResult))
+
+	// Wait for async tool removal from index to complete
+	time.Sleep(2 * time.Second)
+
+	// Step 3: Verify tools are NOT searchable after disable
+	searchAfterDisable, err := mcpClient.CallTool(ctx, searchRequest)
+	require.NoError(t, err)
+	require.False(t, searchAfterDisable.IsError, "Search should succeed (returning empty results)")
+
+	searchAfterText := getToolResultText(searchAfterDisable)
+	// The search should NOT contain the disabled server's tools
+	assert.NotContains(t, searchAfterText, "issue285-test-server:issue285_alpha_tool",
+		"Disabled server's tools should NOT appear in search results")
+	t.Log("Step 3 PASSED: Tools are not searchable after server is disabled")
+
+	// Step 4: Re-enable the server
+	enableRequest := mcp.CallToolRequest{}
+	enableRequest.Params.Name = "upstream_servers"
+	enableRequest.Params.Arguments = map[string]interface{}{
+		"operation": "patch",
+		"name":      "issue285-test-server",
+		"enabled":   true,
+	}
+
+	enableResult, err := mcpClient.CallTool(ctx, enableRequest)
+	require.NoError(t, err)
+	require.False(t, enableResult.IsError, "Enable operation should succeed: %v", getToolResultText(enableResult))
+
+	// Wait for async tool discovery and indexing to complete
+	time.Sleep(3 * time.Second)
+
+	// Step 5: Verify tools are searchable again after re-enable
+	searchAfterEnable, err := mcpClient.CallTool(ctx, searchRequest)
+	require.NoError(t, err)
+	require.False(t, searchAfterEnable.IsError, "Search should succeed")
+
+	searchAfterEnableText := getToolResultText(searchAfterEnable)
+	assert.Contains(t, searchAfterEnableText, "issue285_alpha_tool",
+		"Tools should be searchable again after server is re-enabled")
+	t.Log("Step 5 PASSED: Tools are searchable again after re-enable")
+
+	// Cleanup
+	deleteRequest := mcp.CallToolRequest{}
+	deleteRequest.Params.Name = "upstream_servers"
+	deleteRequest.Params.Arguments = map[string]interface{}{
+		"operation": "remove",
+		"name":      "issue285-test-server",
+	}
+	_, _ = mcpClient.CallTool(ctx, deleteRequest)
+
+	t.Log("✅ Issue #285 fix verified: disable removes tools from search, enable restores them")
+}
+
 // TestE2E_ServerDeleteReaddDifferentTools validates the full lifecycle of stale index cleanup:
 // 1. Add server with Tool Set A, verify tools are searchable
 // 2. Remove server, verify Tool Set A is no longer searchable