Merge pull request #2081 from Molecule-AI/fix/peers-q-filter-1038

fix(discovery): apply ?q= filter to Peers list (#1038)

workspace-server/internal/handlers/discovery.go

@@ -177,6 +177,14 @@ func writeExternalWorkspaceURL(ctx context.Context, c *gin.Context, callerID, ta
 }
 
 // Peers handles GET /registry/:id/peers
+//
+// Optional ``?q=<substring>`` filters the result by case-insensitive
+// substring match against ``name`` or ``role`` (#1038). Filtering is done
+// in Go after the DB read — keeps the SQL identical to the no-filter path
+// (no injection risk, no DB-driver collation surprises) at the cost of
+// loading the unfiltered set first. Acceptable because the peer set is
+// always bounded by the small fanout of a single workspace's parent +
+// children + siblings (typically <50 rows).
 func (h *DiscoveryHandler) Peers(c *gin.Context) {
 	workspaceID := c.Param("id")
 	ctx := c.Request.Context()
@@ -241,12 +249,34 @@ func (h *DiscoveryHandler) Peers(c *gin.Context) {
 		peers = append(peers, parent...)
 	}
 
+	peers = filterPeersByQuery(peers, c.Query("q"))
+
 	if peers == nil {
 		peers = make([]map[string]interface{}, 0)
 	}
 	c.JSON(http.StatusOK, peers)
 }
 
+// filterPeersByQuery returns peers whose name or role case-insensitively
+// contains q. Whitespace-trimmed empty q is a no-op (returns input unchanged).
+func filterPeersByQuery(peers []map[string]interface{}, q string) []map[string]interface{} {
+	q = strings.TrimSpace(q)
+	if q == "" {
+		return peers
+	}
+	needle := strings.ToLower(q)
+	out := make([]map[string]interface{}, 0, len(peers))
+	for _, p := range peers {
+		name := p["name"].(string)
+		role := p["role"].(string)
+		if strings.Contains(strings.ToLower(name), needle) ||
+			strings.Contains(strings.ToLower(role), needle) {
+			out = append(out, p)
+		}
+	}
+	return out
+}
+
 // queryPeerMaps returns clean JSON-serializable maps instead of Workspace structs.
 func queryPeerMaps(query string, args ...interface{}) ([]map[string]interface{}, error) {
 	rows, err := db.DB.Query(query, args...)

workspace-server/internal/handlers/discovery_test.go

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -258,6 +259,149 @@ func TestPeers_RootWorkspace_NoPeers(t *testing.T) {
 	}
 }
 
+// ==================== Peers — ?q= filter (#1038) ====================
+
+// peersFilterFixture mocks the 4 SQL reads (parent_id lookup + siblings +
+// children + parent) with a known 3-peer set so each q-filter test can
+// focus on the post-fetch substring-match behaviour. Returns the handler
+// and the live mock so callers can assert ExpectationsWereMet at the end.
+func peersFilterFixture(t *testing.T) (*DiscoveryHandler, sqlmock.Sqlmock) {
+	t.Helper()
+	mock := setupTestDB(t)
+	setupTestRedis(t)
+
+	mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id =").
+		WithArgs("ws-self").
+		WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow("ws-pm"))
+
+	cols := []string{"id", "name", "role", "tier", "status", "agent_card", "url", "parent_id", "active_tasks"}
+
+	mock.ExpectQuery("SELECT w.id, w.name.*WHERE w.parent_id = \\$1 AND w.id != \\$2").
+		WithArgs("ws-pm", "ws-self").
+		WillReturnRows(sqlmock.NewRows(cols).
+			AddRow("ws-alpha", "Alpha Researcher", "researcher", 1, "online", []byte("null"), "http://a", "ws-pm", 0).
+			AddRow("ws-beta", "Beta Designer", "designer", 1, "online", []byte("null"), "http://b", "ws-pm", 0))
+
+	mock.ExpectQuery("SELECT w.id, w.name.*WHERE w.parent_id = \\$1 AND w.status").
+		WithArgs("ws-self").
+		WillReturnRows(sqlmock.NewRows(cols))
+
+	mock.ExpectQuery("SELECT w.id, w.name.*WHERE w.id = \\$1 AND w.status").
+		WithArgs("ws-pm").
+		WillReturnRows(sqlmock.NewRows(cols).
+			AddRow("ws-pm", "PM Workspace", "manager", 2, "online", []byte("null"), "http://pm", nil, 1))
+
+	return NewDiscoveryHandler(), mock
+}
+
+// runPeersWithQuery invokes Peers and returns BOTH the parsed peers and
+// the raw response body. The raw body is needed by TestPeers_Q_NoMatches
+// to distinguish JSON `[]` (intended) from `null` (regression of the
+// nil-guard at line 254-256) — once unmarshalled, both collapse to
+// len==0 and re-marshal to `[]`, so checking only the parsed value is
+// tautological.
+func runPeersWithQuery(t *testing.T, handler *DiscoveryHandler, q string) ([]map[string]interface{}, []byte) {
+	t.Helper()
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: "ws-self"}}
+	url := "/registry/ws-self/peers"
+	if q != "" {
+		url += "?q=" + q
+	}
+	c.Request = httptest.NewRequest("GET", url, nil)
+	handler.Peers(c)
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	body := w.Body.Bytes()
+	var peers []map[string]interface{}
+	if err := json.Unmarshal(body, &peers); err != nil {
+		t.Fatalf("parse response: %v", err)
+	}
+	return peers, body
+}
+
+// peerIDSet returns the set of peer ids — order-independent comparison
+// avoids fragile peers[0]["id"] asserts that would silently mask a future
+// sort/order change.
+func peerIDSet(peers []map[string]interface{}) map[string]struct{} {
+	out := make(map[string]struct{}, len(peers))
+	for _, p := range peers {
+		out[p["id"].(string)] = struct{}{}
+	}
+	return out
+}
+
+// TestPeers_QFilter covers the rule classifier — append-order
+// independent (uses set membership) so a future sort regression on the
+// production code can't slip through. NoMatches has its own raw-body
+// check (see TestPeers_Q_NoMatches_RawBodyIsArrayNotNull below) because
+// the `[]` vs `null` distinction collapses after json.Unmarshal.
+func TestPeers_QFilter(t *testing.T) {
+	cases := []struct {
+		name    string
+		q       string
+		wantIDs []string
+	}{
+		{"no-q returns all", "", []string{"ws-alpha", "ws-beta", "ws-pm"}},
+		{"name match", "alpha", []string{"ws-alpha"}},
+		{"name match case-insensitive", "ALPHA", []string{"ws-alpha"}},
+		{"role match", "design", []string{"ws-beta"}},
+		{"no matches", "nonexistent", nil},
+		{"whitespace-only is no-op", "%20%20", []string{"ws-alpha", "ws-beta", "ws-pm"}},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			handler, mock := peersFilterFixture(t)
+			peers, _ := runPeersWithQuery(t, handler, tc.q)
+
+			got := peerIDSet(peers)
+			want := make(map[string]struct{}, len(tc.wantIDs))
+			for _, id := range tc.wantIDs {
+				want[id] = struct{}{}
+			}
+			if len(got) != len(want) {
+				t.Fatalf("len: got %d %v, want %d %v", len(got), keysOf(got), len(want), tc.wantIDs)
+			}
+			for id := range want {
+				if _, ok := got[id]; !ok {
+					t.Errorf("missing id %q (got %v, want %v)", id, keysOf(got), tc.wantIDs)
+				}
+			}
+			if err := mock.ExpectationsWereMet(); err != nil {
+				t.Errorf("unmet sqlmock expectations: %v", err)
+			}
+		})
+	}
+}
+
+// TestPeers_Q_NoMatches_RawBodyIsArrayNotNull verifies the `peers = make(...)`
+// nil-guard at the end of Peers — when filtering reduces the slice to
+// non-nil-but-empty AND the original was nil, JSON must be `[]` not `null`.
+// This is the assertion the previous TestPeers_Q_NoMatches falsely claimed
+// to make: re-encoding an unmarshalled []map collapses null and [] both
+// to []. The fix here checks the recorder body bytes BEFORE unmarshal.
+func TestPeers_Q_NoMatches_RawBodyIsArrayNotNull(t *testing.T) {
+	handler, mock := peersFilterFixture(t)
+	_, body := runPeersWithQuery(t, handler, "nonexistent")
+	got := strings.TrimSpace(string(body))
+	if got != "[]" {
+		t.Errorf("raw body: got %q, want []", got)
+	}
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+func keysOf(m map[string]struct{}) []string {
+	out := make([]string, 0, len(m))
+	for k := range m {
+		out = append(out, k)
+	}
+	return out
+}
+
 // ==================== CheckAccess ====================
 
 func TestCheckAccess_BadJSON(t *testing.T) {