diff --git a/platform/internal/handlers/memories.go b/platform/internal/handlers/memories.go index 328eb7fb..cee4f890 100644 --- a/platform/internal/handlers/memories.go +++ b/platform/internal/handlers/memories.go @@ -84,6 +84,10 @@ func (h *MemoriesHandler) Commit(c *gin.Context) { c.JSON(http.StatusCreated, gin.H{"id": memoryID, "scope": body.Scope, "namespace": namespace}) } +// memoryRecallMaxLimit is the hard ceiling for results returned by Search. +// Callers may request fewer via ?limit=N but never more (#377). +const memoryRecallMaxLimit = 50 + // Search handles GET /workspaces/:id/memories // Searches memories visible to the requesting workspace. // @@ -92,11 +96,22 @@ func (h *MemoriesHandler) Commit(c *gin.Context) { // - ?q=... full-text search (ts_rank ordered) when len>=memoryFTSMinQueryLen; // falls back to ILIKE for shorter strings // - ?namespace=... additional filter on the Holaboss-style namespace tag +// - ?limit=N max results (1–50); values >50 are silently clamped to 50 (#377) func (h *MemoriesHandler) Search(c *gin.Context) { workspaceID := c.Param("id") scope := c.DefaultQuery("scope", "") query := c.DefaultQuery("q", "") namespace := c.DefaultQuery("namespace", "") + + // Parse and cap the limit. Anything ≤0 or absent → 50 (full page). + // Anything >50 → 50 (hard ceiling — never error, just clamp). + limit := memoryRecallMaxLimit + if raw := c.Query("limit"); raw != "" { + var n int + if _, err := fmt.Sscanf(raw, "%d", &n); err == nil && n > 0 && n < memoryRecallMaxLimit { + limit = n + } + } ctx := c.Request.Context() // Get workspace info for access control @@ -171,7 +186,8 @@ func (h *MemoriesHandler) Search(c *gin.Context) { } else { sqlQuery += ` ORDER BY created_at DESC` } - sqlQuery += ` LIMIT 50` + sqlQuery += ` LIMIT ` + nextArg(len(args)) + args = append(args, limit) rows, err := db.DB.QueryContext(ctx, sqlQuery, args...) if err != nil { diff --git a/platform/internal/handlers/memories_test.go b/platform/internal/handlers/memories_test.go index dc9b6fa5..4324a62f 100644 --- a/platform/internal/handlers/memories_test.go +++ b/platform/internal/handlers/memories_test.go @@ -505,3 +505,103 @@ func TestMemoriesSearch_NamespaceFilter(t *testing.T) { t.Errorf("unexpected result: %v", result) } } + +// ---------- MemoriesHandler: limit cap (#377) ---------- + +// TestMemoriesSearch_LimitCap_OverMaxClampsTo50 verifies that requesting +// more than 50 results (e.g. ?limit=100) is silently clamped to 50. +// The LIMIT argument passed to the DB must be 50, not 100. +func TestMemoriesSearch_LimitCap_OverMaxClampsTo50(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewMemoriesHandler() + + mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id"). + WithArgs("ws-limit-cap"). + WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow(nil)) + + // LOCAL scope: args are (workspace_id, limit). Expect limit arg = 50 even + // though the caller asked for 100. + mock.ExpectQuery("SELECT id, workspace_id, content, scope, namespace, created_at FROM agent_memories WHERE workspace_id"). + WithArgs("ws-limit-cap", 50). + WillReturnRows(sqlmock.NewRows([]string{"id", "workspace_id", "content", "scope", "namespace", "created_at"})) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-limit-cap"}} + c.Request = httptest.NewRequest("GET", "/memories?scope=LOCAL&limit=100", nil) + c.Request.URL.RawQuery = "scope=LOCAL&limit=100" + + handler.Search(c) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock expectations not met (limit was not clamped to 50): %v", err) + } +} + +// TestMemoriesSearch_LimitExplicit_HonouredWhenBelowMax verifies that +// ?limit=10 is honoured as-is (well under the 50 ceiling). +func TestMemoriesSearch_LimitExplicit_HonouredWhenBelowMax(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewMemoriesHandler() + + mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id"). + WithArgs("ws-limit-10"). + WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow(nil)) + + // Expect limit arg = 10. + mock.ExpectQuery("SELECT id, workspace_id, content, scope, namespace, created_at FROM agent_memories WHERE workspace_id"). + WithArgs("ws-limit-10", 10). + WillReturnRows(sqlmock.NewRows([]string{"id", "workspace_id", "content", "scope", "namespace", "created_at"})) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-limit-10"}} + c.Request = httptest.NewRequest("GET", "/memories?scope=LOCAL&limit=10", nil) + c.Request.URL.RawQuery = "scope=LOCAL&limit=10" + + handler.Search(c) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock expectations not met (limit=10 was not passed through): %v", err) + } +} + +// TestMemoriesSearch_LimitDefault_Is50 verifies that omitting ?limit uses +// the default ceiling of 50. +func TestMemoriesSearch_LimitDefault_Is50(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewMemoriesHandler() + + mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id"). + WithArgs("ws-limit-default"). + WillReturnRows(sqlmock.NewRows([]string{"parent_id"}).AddRow(nil)) + + // No ?limit param → expect DB arg = 50. + mock.ExpectQuery("SELECT id, workspace_id, content, scope, namespace, created_at FROM agent_memories WHERE workspace_id"). + WithArgs("ws-limit-default", 50). + WillReturnRows(sqlmock.NewRows([]string{"id", "workspace_id", "content", "scope", "namespace", "created_at"})) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-limit-default"}} + c.Request = httptest.NewRequest("GET", "/memories?scope=LOCAL", nil) + c.Request.URL.RawQuery = "scope=LOCAL" + + handler.Search(c) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock expectations not met (default limit should be 50): %v", err) + } +}