Repository navigation
Handle paginated MCP tool discovery - #27290
daniel-oai wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 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 { |
There was a problem hiding this comment.
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 👍 / 👎.
|
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. |
Summary
Fixes #26094.
tools/listduring startup tool discovery instead of only exposing the first page.tools/list, and initialized-session continuity.Test plan
just fmtjust test -p codex-mcpwithtools_list_cursor_guard_rejects_cursor_cyclesjust test -p codex-core streamable_http_bearer_env_var_discovers_paginated_toolsjust fix -p codex-mcpjust fix -p codex-rmcp-clientjust fix -p codex-coreAdversarial review
test_streamable_http_serverbinary over streamable HTTP rather than mocking the client in-process.bearer_token_env_varand the server enforces authorization, but assertions and the test-only state endpoint expose only session ids, never token values.tools/listmcp-session-idheaders and the integration test asserts at least two paginated calls, all nonempty, all equal.echotool and the second-pagesecond_page_tool.HashSet, andcodex-mcphas a focused unit test for that cycle.tools/listpagination loop shared by MCP clients; stdio transport construction and launch paths are unchanged.