refactor(mcp): eliminate duplicate cursor cycle detection by delegating paginateAll to PaginateAll (#8049)

`paginateAll` and `PaginateAll` in `internal/mcp/pagination.go` each
independently implemented cursor cycle detection and page-limit
enforcement — identical safety logic duplicated across ~30 lines, with
subtly divergent ordering and error messages.

## Changes

- **`internal/mcp/pagination.go`** — `paginateAll` is now a thin adapter
over `PaginateAll`. A closure bridges the `paginatedPage[T]` fetch
signature to `PaginateAll`'s `([]T, string, error)` contract, adding
per-page logging. Errors are wrapped with `serverID`/`itemKind` context
to preserve richer diagnostics:

  ```go
func paginateAll[T any](serverID, itemKind string, fetch func(cursor
string) (paginatedPage[T], error)) ([]T, error) {
result, err := PaginateAll(paginateAllMaxPages, func(cursor string)
([]T, string, error) {
          page, err := fetch(cursor)
          if err != nil {
              return nil, "", err
          }
logConn.Printf("list%s: received page of %d %s from serverID=%s",
itemKind, len(page.Items), itemKind, serverID)
          return page.Items, page.NextCursor, nil
      })
      if err != nil {
return nil, fmt.Errorf("list%s: backend serverID=%s: %w", itemKind,
serverID, err)
      }
logConn.Printf("list%s: received %d %s total from serverID=%s",
itemKind, len(result), itemKind, serverID)
      return result, nil
  }
  ```

- **`internal/mcp/pagination_test.go`**,
**`internal/mcp/connection_test.go`** — Updated two test cases whose
error-message assertions referenced the old `paginateAll` wording
(`"more than"` / `"pages"`) to match the `PaginateAll` format
(`"exceeded"` / `"page limit"`).

The mid-pagination "total so far" log is intentionally dropped;
`PaginateAll` logs its own per-page progress, and the final total is
still emitted after all pages are collected.

Author: lpcox

internal/mcp/connection_test.go

@@ -990,8 +990,8 @@ func TestPaginateAll(t *testing.T) {
 			return paginatedPage[string]{Items: []string{"x"}, NextCursor: nextCursor}, nil
 		})
 		require.Error(t, err)
-		assert.ErrorContains(t, err, "more than")
-		assert.ErrorContains(t, err, "pages")
+		assert.ErrorContains(t, err, "exceeded")
+		assert.ErrorContains(t, err, "page limit")
 		// Must stop at the page limit, not run forever.
 		assert.Equal(t, paginateAllMaxPages, callCount)
 	})

internal/mcp/pagination.go

@@ -61,43 +61,30 @@ func PaginateAll[T any](maxPages int, fetch func(cursor string) ([]T, string, er
 }
 
 // paginateAll collects all items across paginated SDK list calls.
-// It returns an error if the backend returns more than paginateAllMaxPages pages,
-// protecting against runaway backends.
-// The canonical shared algorithm is PaginateAll; paginateAll adds server-specific
-// logging and richer error context on top of it.
+// It delegates to PaginateAll for the core pagination algorithm (cursor cycle
+// detection and page-limit enforcement) and adds server-specific logging and
+// richer error context on top of it.
 func paginateAll[T any](
 	serverID string,
 	itemKind string,
 	fetch func(cursor string) (paginatedPage[T], error),
 ) ([]T, error) {
-	first, err := fetch("")
-	if err != nil {
-		return nil, err
-	}
-	all := make([]T, len(first.Items), max(len(first.Items), 1))
-	copy(all, first.Items)
-	logConn.Printf("list%s: received page of %d %s from serverID=%s", itemKind, len(first.Items), itemKind, serverID)
-
-	cursor := first.NextCursor
-	seenCursors := make(map[string]struct{})
-	for pageCount := 1; cursor != ""; pageCount++ {
-		if pageCount >= paginateAllMaxPages {
-			return nil, fmt.Errorf("list%s: backend serverID=%s returned more than %d pages; aborting to prevent unbounded memory growth", itemKind, serverID, paginateAllMaxPages)
-		}
-		if _, seen := seenCursors[cursor]; seen {
-			return nil, fmt.Errorf("list%s: backend serverID=%s returned cyclical cursor %q", itemKind, serverID, cursor)
-		}
-		seenCursors[cursor] = struct{}{}
+	result, err := PaginateAll(paginateAllMaxPages, func(cursor string) ([]T, string, error) {
 		page, err := fetch(cursor)
 		if err != nil {
-			return nil, err
+			return nil, "", err
 		}
-		all = append(all, page.Items...)
-		logConn.Printf("list%s: received page of %d %s (total so far: %d) from serverID=%s", itemKind, len(page.Items), itemKind, len(all), serverID)
-		cursor = page.NextCursor
+		logConn.Printf("list%s: received page of %d %s from serverID=%s", itemKind, len(page.Items), itemKind, serverID)
+		return page.Items, page.NextCursor, nil
+	})
+	if err != nil {
+		return nil, fmt.Errorf("list%s: backend serverID=%s: %w", itemKind, serverID, err)
 	}
-	logConn.Printf("list%s: received %d %s total from serverID=%s", itemKind, len(all), itemKind, serverID)
-	return all, nil
+	if result == nil {
+		result = []T{}
+	}
+	logConn.Printf("list%s: received %d %s total from serverID=%s", itemKind, len(result), itemKind, serverID)
+	return result, nil
 }
 
 // listMCPItems is a generic helper for the list* family of MCP operations.

internal/mcp/pagination_test.go

@@ -246,6 +246,7 @@ func TestPaginateAllHelper(t *testing.T) {
 		items, err := paginateAll("server1", "Tools", fetch)
 
 		require.NoError(t, err)
+		assert.NotNil(t, items)
 		assert.Empty(t, items)
 	})
 
@@ -377,8 +378,8 @@ func TestPaginateAllHelper(t *testing.T) {
 		items, err := paginateAll("server1", "Tools", fetch)
 
 		require.Error(t, err)
-		assert.ErrorContains(t, err, "more than")
-		assert.ErrorContains(t, err, "pages")
+		assert.ErrorContains(t, err, "exceeded")
+		assert.ErrorContains(t, err, "page limit")
 		assert.Nil(t, items)
 	})