Merge pull request #465 from Molecule-AI/fix/memory-recall-flood-limit

[Backend Engineer] fix(memories): hard cap of 50 on recall results (#377)

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 {

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)
+	}
+}