diff --git a/workspace-server/internal/handlers/discovery.go b/workspace-server/internal/handlers/discovery.go index d8edf814..6422e002 100644 --- a/workspace-server/internal/handlers/discovery.go +++ b/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=`` 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...) diff --git a/workspace-server/internal/handlers/discovery_test.go b/workspace-server/internal/handlers/discovery_test.go index 7c90005e..e2368aac 100644 --- a/workspace-server/internal/handlers/discovery_test.go +++ b/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) {