Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 119 additions & 0 deletions internal/runtime/get_server_tools_fallback_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package runtime

import (
"testing"

"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
"github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime/stateview"
)

// seedStateViewServer registers a server in the supervisor StateView with the
// given tool set, simulating a connected server whose StateView per-server tool
// cache holds exactly `tools` (possibly empty).
func seedStateViewServer(t *testing.T, rt *Runtime, name string, connected bool, tools []stateview.ToolInfo) {
t.Helper()
sv := rt.supervisor.StateView()
if sv == nil {
t.Fatal("StateView not available on test runtime")
}
sv.UpdateServer(name, func(status *stateview.ServerStatus) {
status.Connected = connected
status.State = "ready"
status.Tools = tools
status.ToolCount = len(tools)
})
}

// TestGetServerTools_FallsBackToIndexWhenStateViewEmpty reproduces MCP-2083:
// after approving (unquarantining) a server, the per-server StateView tool list
// is transiently cleared by the disconnect/reconnect cycle, but the durable
// search index still holds the server's tools. GetServerTools must never serve
// an empty list when the index has tools for a known server.
func TestGetServerTools_FallsBackToIndexWhenStateViewEmpty(t *testing.T) {
rt := newTestRuntime(t)

const server = "com.googleapis.sqladmin/mcp"
// Index holds 2 tools (analogue of the 15 indexed after approval).
if err := rt.indexManager.BatchIndexTools([]*config.ToolMetadata{
{ServerName: server, Name: "list_instances", Description: "List instances", ParamsJSON: `{"type":"object"}`},
{ServerName: server, Name: "get_instance", Description: "Get instance"},
}); err != nil {
t.Fatalf("failed to seed index: %v", err)
}

// StateView has the server present and connected, but with an empty tool list
// (the bug state right after the unquarantine reconnect).
seedStateViewServer(t, rt, server, true, nil)

tools, err := rt.GetServerTools(server)
if err != nil {
t.Fatalf("GetServerTools returned error: %v", err)
}
if len(tools) != 2 {
t.Fatalf("expected fallback to index (2 tools), got %d: %#v", len(tools), tools)
}

got := map[string]bool{}
for _, tool := range tools {
name, _ := tool["name"].(string)
got[name] = true
if sn, _ := tool["server_name"].(string); sn != server {
t.Fatalf("expected server_name %q, got %q", server, sn)
}
}
for _, want := range []string{"list_instances", "get_instance"} {
if !got[want] {
t.Fatalf("missing tool %q in fallback result: %#v", want, got)
}
}
}

// TestGetServerTools_PrefersStateViewWhenPopulated guards against the fallback
// overriding a populated StateView: when StateView already has tools, the index
// is not consulted (StateView is the freshest source).
func TestGetServerTools_PrefersStateViewWhenPopulated(t *testing.T) {
rt := newTestRuntime(t)

const server = "srv"
if err := rt.indexManager.BatchIndexTools([]*config.ToolMetadata{
{ServerName: server, Name: "stale_indexed_tool", Description: "stale"},
}); err != nil {
t.Fatalf("failed to seed index: %v", err)
}

seedStateViewServer(t, rt, server, true, []stateview.ToolInfo{
{Name: "live_a", Description: "A"},
{Name: "live_b", Description: "B"},
})

tools, err := rt.GetServerTools(server)
if err != nil {
t.Fatalf("GetServerTools returned error: %v", err)
}
if len(tools) != 2 {
t.Fatalf("expected StateView tools (2), got %d: %#v", len(tools), tools)
}
for _, tool := range tools {
if name, _ := tool["name"].(string); name == "stale_indexed_tool" {
t.Fatalf("fallback incorrectly overrode a populated StateView with stale index data")
}
}
}

// TestGetServerTools_EmptyStateViewAndIndexReturnsEmpty ensures a genuinely
// empty server (no StateView tools, no index tools) still reports empty — the
// fallback must not invent tools.
func TestGetServerTools_EmptyStateViewAndIndexReturnsEmpty(t *testing.T) {
rt := newTestRuntime(t)

const server = "empty-srv"
seedStateViewServer(t, rt, server, true, nil)

tools, err := rt.GetServerTools(server)
if err != nil {
t.Fatalf("GetServerTools returned error: %v", err)
}
if len(tools) != 0 {
t.Fatalf("expected empty result for a server with no tools, got %d: %#v", len(tools), tools)
}
}
43 changes: 43 additions & 0 deletions internal/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -2111,6 +2111,49 @@ func (r *Runtime) GetServerTools(serverName string) ([]map[string]interface{}, e
tools = append(tools, toolMap)
}

// Defensive fallback (MCP-2083): the per-server StateView tool list is volatile
// derived state — it is cleared on disconnect (supervisor clears Tools on a
// connection-down event) and only repopulated asynchronously by background
// discovery. Approving a quarantined server triggers a disconnect/reconnect
// cycle, and in the field this can leave StateView holding zero tools even
// though the durable search index already indexed the server's tools. Serving
// that empty snapshot makes the Tools tab show "No tools available" for a
// connected server that demonstrably has tools. When StateView reports no tools
// for a server, fall back to the authoritative search index so we never serve
// an empty list when indexed tools exist.
if len(tools) == 0 && r.indexManager != nil {
if indexed, err := r.indexManager.GetToolsByServer(serverName); err == nil && len(indexed) > 0 {
tools = make([]map[string]interface{}, 0, len(indexed))
for _, tool := range indexed {
// Index stores the full tool name; normalize to the bare tool name
// (no "server:" prefix) to match the StateView/approval-record
// convention used by the enrichment layer.
name := tool.Name
if idx := strings.Index(name, ":"); idx != -1 {
name = name[idx+1:]
}
toolMap := map[string]interface{}{
"name": name,
"description": tool.Description,
"server_name": serverName,
}
if tool.ParamsJSON != "" {
var inputSchema map[string]interface{}
if err := json.Unmarshal([]byte(tool.ParamsJSON), &inputSchema); err == nil {
toolMap["inputSchema"] = inputSchema
}
}
if tool.Annotations != nil {
toolMap["annotations"] = tool.Annotations
}
tools = append(tools, toolMap)
}
r.logger.Debug("GetServerTools: StateView empty, served tools from search index fallback",
zap.String("server", serverName),
zap.Int("tool_count", len(tools)))
}
}

return tools, nil
}

Expand Down
Loading