fix(mcp): serialize empty retrieve_tools result as [] instead of null

internal/cache/empty_records_test.go

@@ -0,0 +1,60 @@
+package cache
+
+import (
+	"testing"
+
+	"go.uber.org/zap"
+)
+
+// Issue #953 follow-up: an empty (or fully paginated-past) record set must
+// yield a non-nil Records slice so read_cache serializes "records": [] —
+// never null — for strict MCP clients that iterate the array.
+func TestGetRecords_EmptyContentReturnsNonNilRecords(t *testing.T) {
+	db := setupTestDB(t)
+	defer db.Close()
+
+	manager, err := NewManager(db, zap.NewNop())
+	if err != nil {
+		t.Fatalf("Failed to create cache manager: %v", err)
+	}
+	defer manager.Close()
+
+	if err := manager.Store("empty-key", "test_tool", nil, "[]", "", 0); err != nil {
+		t.Fatalf("Failed to store record: %v", err)
+	}
+
+	resp, err := manager.GetRecords("empty-key", 0, 10)
+	if err != nil {
+		t.Fatalf("GetRecords failed: %v", err)
+	}
+	if resp.Records == nil {
+		t.Fatal("Records must be a non-nil slice so it serializes as [], not null")
+	}
+	if len(resp.Records) != 0 {
+		t.Fatalf("expected 0 records, got %d", len(resp.Records))
+	}
+}
+
+// Paginating past the end of a non-empty set must also stay non-nil.
+func TestGetRecords_OffsetPastEndReturnsNonNilRecords(t *testing.T) {
+	db := setupTestDB(t)
+	defer db.Close()
+
+	manager, err := NewManager(db, zap.NewNop())
+	if err != nil {
+		t.Fatalf("Failed to create cache manager: %v", err)
+	}
+	defer manager.Close()
+
+	if err := manager.Store("two-key", "test_tool", nil, `["a","b"]`, "", 2); err != nil {
+		t.Fatalf("Failed to store record: %v", err)
+	}
+
+	resp, err := manager.GetRecords("two-key", 5, 10)
+	if err != nil {
+		t.Fatalf("GetRecords failed: %v", err)
+	}
+	if resp.Records == nil {
+		t.Fatal("Records must be a non-nil slice so it serializes as [], not null")
+	}
+}

internal/cache/manager.go

@@ -219,7 +219,9 @@ func (m *Manager) GetRecords(key string, offset, limit int) (*ReadCacheResponse,
 		end = totalRecords
 	}
 
-	var paginatedRecords []interface{}
+	// Non-nil so an empty page serializes as "records": [] — never null
+	// (issue #953: strict MCP clients crash iterating a null array).
+	paginatedRecords := make([]interface{}, 0)
 	if offset < totalRecords {
 		paginatedRecords = records[offset:end]
 	}

internal/registries/search.go

@@ -103,6 +103,14 @@ func SearchServers(ctx context.Context, registryID, tag, query string, limit int
 		filtered[i].Registry = reg.Name
 	}
 
+	// Non-nil so an empty result serializes as "servers": [] — never null
+	// (issue #953: strict MCP clients crash iterating a null array). An empty
+	// query skips filterServers' allocation, so a nil fetch result would
+	// otherwise flow straight through.
+	if filtered == nil {
+		filtered = []ServerEntry{}
+	}
+
 	return filtered, nil
 }
 

internal/registries/search_empty_test.go

@@ -0,0 +1,43 @@
+package registries
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// Issue #953 follow-up: a registry with zero servers must produce a non-nil
+// slice so search_servers serializes "servers": [] — never null — for strict
+// MCP clients that iterate the array. With an empty query filterServers
+// returns the fetched slice untouched, so the nil has to be stopped at the
+// SearchServers boundary.
+func TestSearchServers_EmptyRegistryReturnsNonNil(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"servers": []}`))
+	}))
+	defer server.Close()
+
+	originalList := registryList
+	registryList = []RegistryEntry{
+		{
+			ID:         "test-empty",
+			Name:       "Test Empty Registry",
+			ServersURL: server.URL,
+			Protocol:   "modelcontextprotocol/registry",
+		},
+	}
+	defer func() { registryList = originalList }()
+
+	servers, err := SearchServers(context.Background(), "test-empty", "", "", 10, nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if servers == nil {
+		t.Fatal("servers must be a non-nil slice so it serializes as [], not null")
+	}
+	if len(servers) != 0 {
+		t.Fatalf("expected 0 servers, got %d", len(servers))
+	}
+}

internal/server/mcp.go

@@ -1600,7 +1600,9 @@ func (p *MCPProxyServer) handleRetrieveToolsWithMode(ctx context.Context, reques
 	// ranked order of `results` is already final here — the mode selects
 	// serialization only (FR-007).
 	entryOpts := toolEntryOpts{includeStats: includeStats}
-	var mcpTools []map[string]interface{}
+	// Issue #953: must be non-nil so zero matches serialize as [] — strict MCP
+	// clients crash iterating a null tools array.
+	mcpTools := make([]map[string]interface{}, 0, len(results))
 	for _, result := range results {
 		mcpTools = append(mcpTools, p.buildToolEntry(result, responseMode, entryOpts))
 	}
@@ -3778,7 +3780,9 @@ func (p *MCPProxyServer) handleInspectQuarantinedTools(ctx context.Context, requ
 		return mcp.NewToolResultError(reason), nil
 	}
 
-	var toolsAnalysis []map[string]interface{}
+	// Non-nil so a tool-less server serializes as "tools": [] — never null
+	// (issue #953: strict MCP clients crash iterating a null array).
+	toolsAnalysis := make([]map[string]interface{}, 0)
 
 	// REQUEST TEMPORARY CONNECTION EXEMPTION FOR INSPECTION
 	p.logger.Warn("⚠️ Requesting temporary connection exemption for quarantined server inspection",

internal/server/mcp_empty_results_test.go

@@ -0,0 +1,50 @@
+package server
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	"github.com/mark3labs/mcp-go/mcp"
+	"github.com/stretchr/testify/require"
+)
+
+// assertJSONFieldIsEmptyArray decodes raw and asserts field is exactly [] on
+// the wire — the strict-client contract from issue #953 (null crashes clients
+// that iterate the array).
+func assertJSONFieldIsEmptyArray(t *testing.T, raw, field string) {
+	t.Helper()
+	var payload map[string]json.RawMessage
+	require.NoError(t, json.Unmarshal([]byte(raw), &payload))
+	require.Contains(t, payload, field)
+	require.JSONEq(t, `[]`, string(payload[field]),
+		"%q must serialize as an empty array, not %s", field, payload[field])
+}
+
+// Issue #953: a retrieve_tools search with zero matches must serialize the
+// tools array as [] — never null. Strict MCP clients iterate over the array
+// and crash on null (e.g. Python's `'NoneType' object is not iterable`).
+func TestRetrieveTools_EmptyResultSerializesEmptyArray(t *testing.T) {
+	proxy := createTestMCPProxyServer(t)
+	seedEntryBuilderFixture(t, proxy)
+
+	resp, raw := callRetrieve(t, proxy, map[string]interface{}{
+		"query": "zzz-no-such-tool-anywhere", "limit": float64(10),
+	})
+
+	require.Equal(t, 0, resp.Total, "fixture must not match the nonsense query")
+	assertJSONFieldIsEmptyArray(t, raw, "tools")
+}
+
+// Same contract for quarantine_security list_quarantined with no quarantined
+// servers: "servers" must be [] on the wire, never null.
+func TestListQuarantined_EmptySerializesEmptyArray(t *testing.T) {
+	proxy := createTestMCPProxyServer(t)
+
+	result, err := proxy.handleListQuarantinedUpstreams(context.Background())
+	require.NoError(t, err)
+	require.False(t, result.IsError)
+
+	raw := result.Content[0].(mcp.TextContent).Text
+	assertJSONFieldIsEmptyArray(t, raw, "servers")
+}

internal/server/testdata/retrieve_full_stats.golden.json

@@ -1 +1 @@
-{"query":"manage","tools":[{"call_with":"call_tool_read","description":"Get repository metadata to manage projects.","inputSchema":{"properties":{},"type":"object"},"name":"github:get_repo","score":0.011312138508548623,"server":"github"},{"call_with":"call_tool_read","description":"List issues to manage a repository backlog.","inputSchema":{"properties":{"repo":{"type":"string"},"state":{"enum":["open","closed","all"]}},"required":["repo"],"type":"object"},"name":"github:list_issues","score":0.008243798662285959,"server":"github"},{"call_with":"call_tool_read","description":"Search cities to manage location lookups across regions worldwide.","inputSchema":{"properties":{"id":{"type":["string","integer"]},"q":{"type":"string"}},"required":["q"],"type":"object"},"name":"weather:search_city","score":0.007666460133457465,"server":"weather"},{"call_with":"call_tool_read","description":"Get a weather forecast to manage travel plans.","inputSchema":{"properties":{"days":{"type":"integer"},"location":{"properties":{"lat":{"type":"number"},"lon":{"type":"number"}},"type":"object"}},"required":["location"],"type":"object"},"name":"weather:get_forecast","score":0.007056919206727505,"server":"weather"},{"call_with":"call_tool_read","description":"Create an issue to manage work. Supports labels and assignees.","inputSchema":{"properties":{"body":{"type":"string"},"labels":{"items":{"type":"string"},"type":"array"},"title":{"type":"string"},"ttl":{"default":3600,"type":"integer"}},"required":["title"],"type":"object"},"name":"github:create_issue","score":0.00646672399372599,"server":"github"}],"total":5,"usage_instructions":"TOOL SELECTION GUIDE: Check the 'call_with' field for each tool, then use the matching tool variant. DECISION RULES BY TOOL NAME: (1) READ (call_tool_read): search, query, list, get, fetch, find, check, view, read, show, describe, lookup, retrieve, browse, explore, discover, scan, inspect, analyze, examine, validate, verify. DEFAULT choice when unsure. (2) WRITE (call_tool_write): create, update, modify, add, set, send, edit, change, write, post, put, patch, insert, upload, submit, assign, configure, enable, register, subscribe, publish, move, copy, rename, merge. (3) DESTRUCTIVE (call_tool_destructive): delete, remove, drop, revoke, disable, destroy, purge, reset, clear, unsubscribe, cancel, terminate, close, archive, ban, block, disconnect, kill, wipe, truncate, force, hard. INTENT TRACKING: Always provide intent_reason (why you're calling this tool) and intent_data_sensitivity (public/internal/private/unknown) to enable activity auditing.","usage_summary":{"top_tools":null}}
+{"query":"manage","tools":[{"call_with":"call_tool_read","description":"Get repository metadata to manage projects.","inputSchema":{"properties":{},"type":"object"},"name":"github:get_repo","score":0.011312138508548623,"server":"github"},{"call_with":"call_tool_read","description":"List issues to manage a repository backlog.","inputSchema":{"properties":{"repo":{"type":"string"},"state":{"enum":["open","closed","all"]}},"required":["repo"],"type":"object"},"name":"github:list_issues","score":0.008243798662285959,"server":"github"},{"call_with":"call_tool_read","description":"Search cities to manage location lookups across regions worldwide.","inputSchema":{"properties":{"id":{"type":["string","integer"]},"q":{"type":"string"}},"required":["q"],"type":"object"},"name":"weather:search_city","score":0.007666460133457465,"server":"weather"},{"call_with":"call_tool_read","description":"Get a weather forecast to manage travel plans.","inputSchema":{"properties":{"days":{"type":"integer"},"location":{"properties":{"lat":{"type":"number"},"lon":{"type":"number"}},"type":"object"}},"required":["location"],"type":"object"},"name":"weather:get_forecast","score":0.007056919206727505,"server":"weather"},{"call_with":"call_tool_read","description":"Create an issue to manage work. Supports labels and assignees.","inputSchema":{"properties":{"body":{"type":"string"},"labels":{"items":{"type":"string"},"type":"array"},"title":{"type":"string"},"ttl":{"default":3600,"type":"integer"}},"required":["title"],"type":"object"},"name":"github:create_issue","score":0.00646672399372599,"server":"github"}],"total":5,"usage_instructions":"TOOL SELECTION GUIDE: Check the 'call_with' field for each tool, then use the matching tool variant. DECISION RULES BY TOOL NAME: (1) READ (call_tool_read): search, query, list, get, fetch, find, check, view, read, show, describe, lookup, retrieve, browse, explore, discover, scan, inspect, analyze, examine, validate, verify. DEFAULT choice when unsure. (2) WRITE (call_tool_write): create, update, modify, add, set, send, edit, change, write, post, put, patch, insert, upload, submit, assign, configure, enable, register, subscribe, publish, move, copy, rename, merge. (3) DESTRUCTIVE (call_tool_destructive): delete, remove, drop, revoke, disable, destroy, purge, reset, clear, unsubscribe, cancel, terminate, close, archive, ban, block, disconnect, kill, wipe, truncate, force, hard. INTENT TRACKING: Always provide intent_reason (why you're calling this tool) and intent_data_sensitivity (public/internal/private/unknown) to enable activity auditing.","usage_summary":{"top_tools":[]}}

internal/storage/empty_slices_test.go

@@ -0,0 +1,46 @@
+package storage
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+)
+
+func setupTestStorageForEmptySlices(t *testing.T) *Manager {
+	t.Helper()
+
+	tmpDir, err := os.MkdirTemp("", "empty_slices_test_*")
+	require.NoError(t, err)
+
+	manager, err := NewManager(tmpDir, zap.NewNop().Sugar())
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		manager.Close()
+		os.RemoveAll(tmpDir)
+	})
+	return manager
+}
+
+// Issue #953 follow-up: MCP responses built from these slices must serialize
+// as [] — never null — for strict clients that iterate the arrays.
+
+func TestListQuarantinedUpstreamServers_EmptyReturnsNonNil(t *testing.T) {
+	manager := setupTestStorageForEmptySlices(t)
+
+	servers, err := manager.ListQuarantinedUpstreamServers()
+	require.NoError(t, err)
+	require.NotNil(t, servers, "quarantined-server list must be non-nil so it serializes as [], not null")
+	require.Empty(t, servers)
+}
+
+func TestGetToolStats_NoStatsReturnsNonNil(t *testing.T) {
+	manager := setupTestStorageForEmptySlices(t)
+
+	stats, err := manager.GetToolStats(10)
+	require.NoError(t, err)
+	require.NotNil(t, stats, "tool stats must be non-nil so usage_summary.top_tools serializes as [], not null")
+	require.Empty(t, stats)
+}

internal/storage/manager.go

@@ -242,7 +242,9 @@ func (m *Manager) ListQuarantinedUpstreamServers() ([]*config.ServerConfig, erro
 	m.logger.Debugw("Retrieved all upstream records for quarantine filtering",
 		"total_records", len(records))
 
-	var quarantinedServers []*config.ServerConfig
+	// Non-nil so an empty list serializes as "servers": [] — never null
+	// (issue #953: strict MCP clients crash iterating a null array).
+	quarantinedServers := make([]*config.ServerConfig, 0)
 	for _, record := range records {
 		m.logger.Debugw("Checking server quarantine status",
 			"server", record.Name,
@@ -819,7 +821,9 @@ func (m *Manager) GetToolStats(topN int) ([]map[string]interface{}, error) {
 		return nil, err
 	}
 
-	var result []map[string]interface{}
+	// Non-nil so usage_summary.top_tools serializes as [] — never null
+	// (issue #953: strict MCP clients crash iterating a null array).
+	result := make([]map[string]interface{}, 0, len(stats.TopTools))
 	for _, tool := range stats.TopTools {
 		result = append(result, map[string]interface{}{
 			"tool_name": tool.ToolName,