🪢 fix: Paginate MCP tools/list to Load All Tools#66
Closed
TomasPalsson wants to merge 1 commit into
Closed
Conversation
MCP `tools/list` is cursor-paginated, but LibreChat only ever read the first page. `MCPConnection.fetchTools()` called `client.listTools()` once and discarded `nextCursor`, and `MCPServerInspector` — which builds the agent-facing tool registry at startup and per request — called the raw `client.listTools()` directly. Servers that paginate (e.g. an aggregating gateway exposing hundreds of tools) only ever exposed page one; tools on later pages were never registered, and invoking one returned "This tool's MCP server is temporarily unavailable." - `MCPConnection.fetchTools()` now follows `nextCursor` across pages and concatenates every page's tools, bounded by a configurable page cap (`MCP_TOOLS_LIST_MAX_PAGES`, default 50) and a repeated-cursor guard so a misbehaving server cannot loop forever. Tools already fetched are returned if a later page fails, and the no-throw error contract is unchanged. - `MCPServerInspector.getToolFunctions()` and `fetchServerCapabilities()` now route through `fetchTools()`, so the canonical startup and per-request tool registry is fully paginated too.
📝 WalkthroughWalkthrough
ChangesfetchTools Cursor Pagination
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/api/src/mcp/mcpConfig.ts`:
- Around line 53-54: The TOOLS_LIST_MAX_PAGES configuration uses Math.max(1,
math(...)) which can still produce NaN or Infinity values if the math() function
evaluates to non-finite numbers, breaking the loop bound guarantee in
fetchTools(). Ensure that after evaluating the math expression from the
environment variable, the result is clamped to a finite integer (validate with
Number.isFinite()), then apply Math.max with 1 to guarantee a minimum of 1. The
order should be: parse/evaluate with math(), validate/clamp to finite integer,
then apply Math.max to ensure the final value is always a valid positive
integer.
In `@packages/api/src/mcp/registry/MCPServerInspector.ts`:
- Around line 153-154: The issue is that fetchTools() is being called twice
during inspection—once around line 153-154 in the current context and again
around lines 174-175—causing redundant pagination traversals. Refactor by
fetching tools once and storing the result in a shared variable or Promise, then
reuse that single fetched tool list for both the tools name assignment (in the
code near line 153-154) and the tool functions retrieval (near line 174-175) to
eliminate the duplicate network calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0566bdc3-d70e-410f-b754-ed044fe23963
📒 Files selected for processing (11)
packages/api/src/mcp/__tests__/MCPConnectionAgentLifecycle.test.tspackages/api/src/mcp/__tests__/MCPConnectionFactory.oauthSdk.integration.test.tspackages/api/src/mcp/__tests__/MCPConnectionFetchTools.test.tspackages/api/src/mcp/__tests__/MCPConnectionSSRF.test.tspackages/api/src/mcp/__tests__/dbSourced.integration.test.tspackages/api/src/mcp/connection.tspackages/api/src/mcp/mcpConfig.tspackages/api/src/mcp/registry/MCPServerInspector.tspackages/api/src/mcp/registry/__tests__/MCPReinitRecovery.integration.test.tspackages/api/src/mcp/registry/__tests__/MCPServerInspector.test.tspackages/api/src/mcp/registry/__tests__/mcpConnectionsMock.helper.ts
Comment on lines
+53
to
+54
| /** Max number of `tools/list` pages to request when an MCP server paginates its tool list. Clamped to >= 1. Default: 50 */ | ||
| TOOLS_LIST_MAX_PAGES: Math.max(1, math(process.env.MCP_TOOLS_LIST_MAX_PAGES ?? 50)), |
There was a problem hiding this comment.
Clamp TOOLS_LIST_MAX_PAGES to a finite integer before applying Math.max.
The current expression can still produce NaN or Infinity (e.g., expression-based env values), which breaks the loop-bound guarantee in fetchTools().
Suggested fix
+const parsedToolsListMaxPages = math(process.env.MCP_TOOLS_LIST_MAX_PAGES ?? 50);
+const normalizedToolsListMaxPages = Number.isFinite(parsedToolsListMaxPages)
+ ? Math.floor(parsedToolsListMaxPages)
+ : 50;
+
export const mcpConfig: {
@@
- TOOLS_LIST_MAX_PAGES: Math.max(1, math(process.env.MCP_TOOLS_LIST_MAX_PAGES ?? 50)),
+ TOOLS_LIST_MAX_PAGES: Math.max(1, normalizedToolsListMaxPages),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/mcp/mcpConfig.ts` around lines 53 - 54, The
TOOLS_LIST_MAX_PAGES configuration uses Math.max(1, math(...)) which can still
produce NaN or Infinity values if the math() function evaluates to non-finite
numbers, breaking the loop bound guarantee in fetchTools(). Ensure that after
evaluating the math expression from the environment variable, the result is
clamped to a finite integer (validate with Number.isFinite()), then apply
Math.max with 1 to guarantee a minimum of 1. The order should be: parse/evaluate
with math(), validate/clamp to finite integer, then apply Math.max to ensure the
final value is always a valid positive integer.
Comment on lines
+153
to
+154
| const tools = await this.connection!.fetchTools(); | ||
| this.config.tools = tools.map((tool) => tool.name).join(', '); |
There was a problem hiding this comment.
Avoid fetching paginated tools twice during one inspection pass.
fetchServerCapabilities() and getToolFunctions() now each call fetchTools(), so one inspection does two full pagination traversals. Reuse a single fetched tool list (or a shared Promise) for both config.tools and toolFunctions to prevent doubled network load and startup latency.
Refactor sketch
- await Promise.allSettled([
- this.fetchServerInstructions(),
- this.fetchServerCapabilities(),
- this.fetchToolFunctions(),
- ]);
+ const toolsPromise = this.connection!.fetchTools();
+ await Promise.allSettled([
+ this.fetchServerInstructions(),
+ this.fetchServerCapabilities(toolsPromise),
+ this.fetchToolFunctions(toolsPromise),
+ ]);Also applies to: 174-175
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/mcp/registry/MCPServerInspector.ts` around lines 153 - 154,
The issue is that fetchTools() is being called twice during inspection—once
around line 153-154 in the current context and again around lines
174-175—causing redundant pagination traversals. Refactor by fetching tools once
and storing the result in a shared variable or Promise, then reuse that single
fetched tool list for both the tools name assignment (in the code near line
153-154) and the tool functions retrieval (near line 174-175) to eliminate the
duplicate network calls.
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I fixed MCP tool discovery silently truncating to the first page when a server paginates
tools/list.Root cause
tools/listis a cursor-paginated method in the MCP spec, but LibreChat only ever read the first page in two places:MCPConnection.fetchTools(packages/api/src/mcp/connection.ts) calledthis.client.listTools()once and returnedresult.tools, discardingnextCursor.MCPServerInspector(packages/api/src/mcp/registry/MCPServerInspector.ts) — which builds the agent-facing tool registry (config.toolFunctions, consumed byinitializeMCPs→MCPManager.getAppToolFunctions) at startup and per request — called the rawconnection.client.listTools()directly in bothgetToolFunctionsandfetchServerCapabilities.So any server returning more tools than fit in one page only ever exposed page 1. With an aggregating gateway (observed: AWS Bedrock AgentCore Gateway returning 256 tools across 6 pages) only the first ~41 tools were registered; the rest were invisible to the agent, and invoking one failed with
"This tool's MCP server is temporarily unavailable"because it was never in the registry.Approach
fetchToolsnow followsnextCursor, passing{ cursor }tolistToolsfor each subsequent page and concatenatingtoolsin order until the server stops returning a cursor. Two defensive guards bound the loop so a misbehaving server cannot stall discovery:mcpConfig.TOOLS_LIST_MAX_PAGES(envMCP_TOOLS_LIST_MAX_PAGES, default50, clamped to>= 1). On hitting the cap the loop stops and emits alogger.warn.seenCursorsset — if a server repeats a cursor it already returned (an infinite-loop signature), the loop stops early and warns.MCPServerInspector.getToolFunctionsandfetchServerCapabilitiesnow route throughconnection.fetchTools()instead of the raw single-pageclient.listTools(), so the canonical startup and per-request tool registry is fully paginated.What is deliberately not changed / out of scope
emitError(error, 'Failed to fetch tools'). On a mid-pagination error the pages already fetched are returned (consistent with the page-cap partial return); a first-page error still yields[].client.listTools()twice in the same two methods); pagination now applies to both. Collapsing them into one fetch is a reasonable follow-up.fetchResourcesandfetchPromptsshare the same single-page limitation. I left them out to keep this PR focused on the reported tool-loading bug; happy to follow up.Changes
packages/api/src/mcp/connection.ts—fetchToolsfollowsnextCursorwith page-cap and repeated-cursor guards, returning already-fetched pages on error.packages/api/src/mcp/mcpConfig.ts— adds typedTOOLS_LIST_MAX_PAGES(envMCP_TOOLS_LIST_MAX_PAGES, default50).packages/api/src/mcp/registry/MCPServerInspector.ts—getToolFunctionsandfetchServerCapabilitiesuse the paginatedfetchTools().packages/api/src/mcp/__tests__/MCPConnectionFetchTools.test.ts— new unit test (8 cases) exercising the realMCPConnection.fetchToolsvia an injectedlistToolsstub.packages/api/src/mcp/registry/__tests__/MCPServerInspector.test.tsandmcpConnectionsMock.helper.ts— updated so the inspector tests drive the paginatedfetchToolspath.mcpConfigmocks now setTOOLS_LIST_MAX_PAGESsofetchToolsruns the real pagination loop instead of short-circuiting on an undefined cap.Change Type
Testing
Run on Node
v24.16.0:cd packages/api && npx jest src/mcp/__tests__/MCPConnectionFetchTools.test.ts— 8/8 passing. Cases: single page (one request), multi-page concat + cursor passthrough, page cap + warn, repeated cursor + warn, empty intermediate page, empty-string cursor (valid continuation), mid-pagination failure (already-fetched pages returned), first-page failure ([]).cd packages/api && npx jest src/mcp/registry/__tests__/MCPServerInspector.test.ts— passing (inspector now drives the paginated path).cd packages/api && npm run test:ci— full@librechat/apisuite green, no regressions.npx tsc --noEmit -p packages/api/tsconfig.json— clean.npx eslint <changed files>— clean.npx prettier --check <changed files>— formatted.node scripts/sort-imports.mts --check <changed files>— sorted.The new test constructs a real
MCPConnectionand injects a stubclient.listTools, so it exercises the actualfetchToolsimplementation (matching howMCPConnectionAgentLifecycle.test.tsdrivesfetchTools) rather than a reimplementation of the loop.Test Configuration:
v24.16.0packages/apiChecklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests