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

[Copilot]

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:

  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.

[Copilot]

Pull request overview

This PR refactors MCP pagination so the internal paginateAll helper delegates the core cursor pagination logic (cycle detection + page limit enforcement) to the shared PaginateAll implementation, reducing duplicated safety logic while keeping server-scoped logging and error context.

Changes:

• Refactor paginateAll to call PaginateAll via an adapter closure and wrap errors with serverID/itemKind context.
• Update error-message assertions in tests to match the PaginateAll error wording.

Show a summary per file

File	Description
internal/mcp/pagination.go	Refactors paginateAll to delegate pagination mechanics to PaginateAll while retaining list logging and contextual error wrapping.
internal/mcp/pagination_test.go	Updates assertions to match the new page-limit error message text.
internal/mcp/connection_test.go	Updates assertions to match the new page-limit error message text.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 3/3 changed files
• Comments generated: 1

[lpcox]

@copilot address review feedback

[Copilot]

@copilot address review feedback

Addressed in commit da8a2f7. paginateAll now normalizes a nil delegated result to a non-nil empty slice so list responses keep array shape ([]), and the helper test now asserts the empty result is non-nil.