Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89ee4f4127
ℹ️ 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".
|
Warning Rate limit exceeded@jlowin has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 19 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThis PR introduces transport tracking capabilities to the FastMCP server. A new Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/fastmcp/client/transports.py (1)
211-223: Docstring update is helpful; consider explicitly stating what happens whentimeoutis not provided.Given the behavior change (default timeouts now come from
create_mcp_http_client), calling out the “no factory + no read_timeout_seconds” case would reduce surprises.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
loq.tomlis excluded by none and included by nonetests/integration_tests/test_timeout_fix.pyis excluded by none and included by nonetests/server/test_context.pyis excluded by none and included by none
📒 Files selected for processing (5)
docs/servers/context.mdxsrc/fastmcp/client/transports.pysrc/fastmcp/server/context.pysrc/fastmcp/server/http.pysrc/fastmcp/server/server.py
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/context.mdx
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns
Follow existing patterns and maintain consistency across the codebase
Files:
src/fastmcp/server/context.pysrc/fastmcp/server/server.pysrc/fastmcp/server/http.pysrc/fastmcp/client/transports.py
🧬 Code graph analysis (2)
src/fastmcp/server/server.py (2)
src/fastmcp/server/context.py (3)
fastmcp(186-191)reset_transport(93-95)set_transport(86-90)src/fastmcp/utilities/logging.py (1)
temporary_log_level(103-151)
src/fastmcp/server/http.py (1)
src/fastmcp/server/context.py (3)
fastmcp(186-191)reset_transport(93-95)set_transport(86-90)
🪛 LanguageTool
docs/servers/context.mdx
[style] ~307-~307: This phrase is redundant. Consider using “outside”.
Context: ...lifetime. It returns None when called outside of a server context (for example, in unit ...
(OUTSIDE_OF)
[style] ~307-~307: This phrase is redundant. Consider using “outside”.
Context: ...ple, in unit tests or when running code outside of an MCP request). ```python from fastmc...
(OUTSIDE_OF)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (3)
src/fastmcp/server/context.py (1)
421-429:Context.transportproperty is straightforward and matches the intended API.Returning
_current_transport.get()with a defaulted ContextVar cleanly yieldsNoneoutside a transport-scoped context.src/fastmcp/server/server.py (1)
2117-2149: stdio transport scoping + finally-reset is solid; initialization options addition looks reasonable.Setting
set_transport("stdio")at entry and resetting infinallyis the right pattern, and threadingexperimental_capabilities+NotificationOptions(tools_changed=True)through initialization is consistent with the PR goals.src/fastmcp/client/transports.py (1)
28-29: No action needed on this import;mcp.shared._httpx_utilsis a documented public utility in the MCP SDK. [resolve]Despite the leading-underscore naming convention, the MCP SDK explicitly exposes
McpHttpClientFactoryandcreate_mcp_http_clientas documented utilities for external use. These are stable, supported APIs with clear signatures and documented defaults (30s connect timeout, 5min read timeout for HTTP clients).
| ### Transport | ||
|
|
||
| The `ctx.transport` property indicates which transport is being used to run the server. This is useful when your tool needs to behave differently depending on whether the server is running over STDIO, SSE, or Streamable HTTP. For example, you might want to return shorter responses over STDIO or adjust timeout behavior based on transport characteristics. | ||
|
|
||
| The transport type is set once when the server starts and remains constant for the server's lifetime. It returns `None` when called outside of a server context (for example, in unit tests or when running code outside of an MCP request). | ||
|
|
||
| ```python | ||
| from fastmcp import FastMCP, Context | ||
|
|
||
| mcp = FastMCP("example") | ||
|
|
||
| @mcp.tool | ||
| def connection_info(ctx: Context) -> str: | ||
| if ctx.transport == "stdio": | ||
| return "Connected via STDIO" | ||
| elif ctx.transport == "sse": | ||
| return "Connected via SSE" | ||
| elif ctx.transport == "streamable-http": | ||
| return "Connected via Streamable HTTP" | ||
| else: | ||
| return "Transport unknown" | ||
| ``` | ||
|
|
||
| **Property signature:** `ctx.transport -> Literal["stdio", "sse", "streamable-http"] | None` | ||
|
|
There was a problem hiding this comment.
Tighten phrasing and add a tiny “verify” hint for readers.
- Line 307: “outside of a server context” → “outside a server context” (per static analysis hint).
- Consider adding a one-liner after the code block like “Verify: call the tool and confirm it returns ‘Connected via …’” to align with the docs guideline of giving an expected outcome.
Proposed edit
-The transport type is set once when the server starts and remains constant for the server's lifetime. It returns `None` when called outside of a server context (for example, in unit tests or when running code outside of an MCP request).
+The transport type is set once when the server starts and remains constant for the server's lifetime. It returns `None` when called outside a server context (for example, in unit tests or when running code outside an MCP request).🧰 Tools
🪛 LanguageTool
[style] ~307-~307: This phrase is redundant. Consider using “outside”.
Context: ...lifetime. It returns None when called outside of a server context (for example, in unit ...
(OUTSIDE_OF)
[style] ~307-~307: This phrase is redundant. Consider using “outside”.
Context: ...ple, in unit tests or when running code outside of an MCP request). ```python from fastmc...
(OUTSIDE_OF)
Expose the current transport type (stdio, sse, streamable-http) via ctx.transport, enabling transport-specific behavior in tools.
89ee4f4 to
22c76d0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/servers/context.mdx (1)
314-324: Minor: Consider addingasyncto the example function for consistency.The
transportproperty access doesn't requireawait, so the sync function works correctly. However, most other Context examples in this file useasync defsince tools typically use async context methods. For consistency with other examples, you might consider:@mcp.tool -def connection_info(ctx: Context) -> str: +async def connection_info(ctx: Context) -> str:This is purely a consistency suggestion—the current sync version is functionally correct.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
loq.tomlis excluded by none and included by nonetests/server/test_context.pyis excluded by none and included by none
📒 Files selected for processing (4)
docs/servers/context.mdxsrc/fastmcp/server/context.pysrc/fastmcp/server/http.pysrc/fastmcp/server/server.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/fastmcp/server/http.py
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns
Follow existing patterns and maintain consistency across the codebase
Files:
src/fastmcp/server/context.pysrc/fastmcp/server/server.py
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/context.mdx
🧬 Code graph analysis (1)
src/fastmcp/server/server.py (2)
src/fastmcp/server/context.py (3)
fastmcp(186-191)reset_transport(93-95)set_transport(86-90)src/fastmcp/server/tasks/capabilities.py (1)
get_task_capabilities(12-34)
🪛 LanguageTool
docs/servers/context.mdx
[style] ~307-~307: This phrase is redundant. Consider using “outside”.
Context: ...lifetime. It returns None when called outside of a server context (for example, in unit ...
(OUTSIDE_OF)
[style] ~307-~307: This phrase is redundant. Consider using “outside”.
Context: ...ple, in unit tests or when running code outside of an MCP request). ```python from fastmc...
(OUTSIDE_OF)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (4)
src/fastmcp/server/context.py (2)
80-95: LGTM! Clean transport state management API.The
TransportTypeliteral,ContextVar, and theset_transport/reset_transportpair provide a well-designed, thread-safe mechanism for tracking transport state. The token-based reset pattern ensures proper cleanup and supports nested contexts correctly.
421-428: LGTM! Well-documented transport property.The property correctly exposes the transport type with clear documentation of possible values and the
Nonecase for out-of-context usage.docs/servers/context.mdx (1)
303-327: LGTM! Clear and well-structured documentation.The Transport section follows the documentation patterns established in the file:
- Clear explanation of purpose and use cases
- Lifecycle behavior documented
- Complete, runnable code example with all transport cases
- Property signature at the end
The code example demonstrates practical usage by showing how to branch behavior based on transport type.
src/fastmcp/server/server.py (1)
2117-2148: LGTM! Correct transport lifecycle management for stdio.The implementation properly:
- Sets transport state before entering the server context
- Uses try/finally to guarantee cleanup even on exceptions
- Captures the token for proper reset
This pattern is consistently applied across all transport types: stdio, SSE, and streamable-http transports all follow the same lifecycle management approach.
ContextVars set in lifespan don't propagate to Starlette request handler tasks. Move transport setting to RequestContextMiddleware which runs per-request, and store transport_type on app.state.
Tools sometimes need to know which transport the server is running on—for example, to adjust response formatting for STDIO or handle timeout behavior differently for HTTP transports.
This adds
ctx.transportwhich returns the current transport type:Returns
Literal["stdio", "sse", "streamable-http"]when running, orNoneoutside a server context.