Skip to content

Handle paginated MCP tool discovery - #27290

Closed
daniel-oai wants to merge 1 commit into
mainfrom
codex/fix-26094-remote-mcp-tools
Closed

daniel-oai wants to merge 1 commit into
mainfrom
codex/fix-26094-remote-mcp-tools

Conversation

@daniel-oai

Copy link
Copy Markdown
Contributor

Summary

Fixes #26094.

  • Follow every page returned by MCP tools/list during startup tool discovery instead of only exposing the first page.
  • Add a seen-cursor guard so pathological pagination loops, including A -> B -> A, fail instead of spinning forever.
  • Extend the streamable HTTP MCP regression server/test to cover bearer-token env vars, paginated tools/list, and initialized-session continuity.

Test plan

  • just fmt
  • just test -p codex-mcp with tools_list_cursor_guard_rejects_cursor_cycles
  • just test -p codex-core streamable_http_bearer_env_var_discovers_paginated_tools
  • just fix -p codex-mcp
  • just fix -p codex-rmcp-client
  • just fix -p codex-core

Adversarial review

  • Real streamable HTTP path: the core regression starts the existing test_streamable_http_server binary over streamable HTTP rather than mocking the client in-process.
  • Bearer token from env: the regression configures bearer_token_env_var and the server enforces authorization, but assertions and the test-only state endpoint expose only session ids, never token values.
  • Session continuity: the test server records actual tools/list mcp-session-id headers and the integration test asserts at least two paginated calls, all nonempty, all equal.
  • All paginated tools exposed: the model request must include the first-page echo tool and the second-page second_page_tool.
  • A -> B -> A duplicate cursor guard: production pagination tracks all seen cursors with a HashSet, and codex-mcp has a focused unit test for that cycle.
  • Stdio unaffected: the change is limited to the post-initialize tools/list pagination loop shared by MCP clients; stdio transport construction and launch paths are unchanged.
  • Install-vs-connect wording: this fixes runtime discovery/connect behavior after a server is configured; it does not add server-specific installation assumptions or change auth/install UI copy.

@daniel-oai
daniel-oai marked this pull request as ready for review June 10, 2026 03:19
@daniel-oai
daniel-oai requested a review from a team as a code owner June 10, 2026 03:19

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a91f6ac3ab

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

let mut cursor: Option<String> = None;
let mut seen_cursors = HashSet::new();

loop {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Cap paginated tool discovery

When an MCP server keeps returning fresh nextCursor values, this loop has no page/tool cap, so startup accumulates every page before exposing the tools to model context. A buggy server can hang startup or inject an unbounded tool list; please add a hard cap. guidance

Useful? React with 👍 / 👎.

@daniel-oai

Copy link
Copy Markdown
Contributor Author

Closing per principal review. The pagination direction may be valid, but this is not a slam-dunk patch: it changes MCP tool discovery/session behavior and test-server semantics, so it needs a fresh owner-level review before reopening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remote MCP server installs incompletely or not at all, even though initialize and tools/list succeed

1 participant