-
-
Notifications
You must be signed in to change notification settings - Fork 717
feat: Add HTTP-Streaming support for MCP with backward compatibility #746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Implemented complete HTTP-Streaming transport for Python and TypeScript - Added smart auto-detection: URLs ending with /sse use SSE, others use HTTP-streaming - Added explicit transport selection via transport parameter - Maintained 100% backward compatibility - all existing code works unchanged - Added comprehensive examples and documentation - Created backward compatibility test suite Co-authored-by: Mervin Praison <[email protected]>
WalkthroughSupport for MCP over HTTP-Streaming has been added across Python and TypeScript client libraries, including new transport implementations, auto-detection logic, and explicit transport selection. Documentation, usage examples, and backward compatibility tests are provided, ensuring both HTTP-Streaming and SSE transports function as intended without breaking existing workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MCPClient
participant HTTPStreamingTransport
participant MCPServer
User->>MCPClient: Initialize with URL/transport options
MCPClient->>HTTPStreamingTransport: Connect (auto-detects transport)
HTTPStreamingTransport->>MCPServer: Establish HTTP streaming connection
loop For each tool invocation
User->>MCPClient: Call tool with input
MCPClient->>HTTPStreamingTransport: Send JSON message
HTTPStreamingTransport->>MCPServer: Stream request
MCPServer-->>HTTPStreamingTransport: Stream response (NDJSON)
HTTPStreamingTransport-->>MCPClient: Deliver response
MCPClient-->>User: Return output
end
User->>MCPClient: Close
MCPClient->>HTTPStreamingTransport: Close connection
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested labels
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@claude review if this fixes the issue |
Claude finished @MervinPraison's task —— View job Code Review SummaryThis PR successfully implements HTTP-Streaming support for MCP as requested in issue #722. The implementation is well-designed with excellent backward compatibility. ✅ What Works Well
🔍 Potential Issues
💡 Suggestions for Improvement
✅ VerdictThis PR successfully fixes issue #722 by adding HTTP-Streaming support with:
The implementation is production-ready with minor suggestions for improvement. Great work on maintaining backward compatibility while adding this new feature! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @MervinPraison, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the Model Context Protocol (MCP) by integrating HTTP-Streaming as a new, more versatile transport mechanism. The core objective is to provide robust bidirectional communication capabilities while meticulously preserving full backward compatibility for all existing MCP integrations. This allows for seamless adoption of the new streaming method without disrupting current workflows, offering improved performance and flexibility for high-throughput and binary data scenarios.
Highlights
- New HTTP-Streaming Transport: Implemented full HTTP-Streaming support for the Model Context Protocol (MCP) in both Python and TypeScript, offering a bidirectional streaming alternative to existing SSE.
- Smart Auto-Detection: Introduced intelligent auto-detection logic where URLs ending with
/sse
(or similar patterns) default to SSE transport, while other HTTP URLs automatically utilize the new HTTP-Streaming transport. - Explicit Transport Selection: Added the ability to explicitly select the desired transport type (
sse
orhttp-streaming
) via atransport
parameter, overriding the auto-detection if needed. - Full Backward Compatibility: Ensured 100% backward compatibility, meaning all existing MCP usage patterns, including SSE, stdio commands, and NPX commands, continue to function without any code changes.
- Comprehensive Documentation and Examples: Provided detailed documentation (
docs/MCP_HTTP_STREAMING.md
) and dedicated example scripts in both Python and TypeScript to guide users on leveraging the new HTTP-Streaming capabilities.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces HTTP-Streaming support for MCP in both Python and TypeScript, with a focus on backward compatibility. The review identified a critical naming collision in TypeScript, and high severity issues related to transport detection and timeout handling in Python. A busy-wait loop in the Python implementation and use of a deprecated method in TypeScript were also noted.
/** | ||
* Backward-compatible alias for the main class. | ||
*/ | ||
export { MCPHttpStreaming as MCP }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exporting MCPHttpStreaming
with the alias MCP
creates a name collision with the unified MCP
class defined in mcp.ts
. Since index.ts
exports from both files, this can lead to unpredictable behavior depending on module resolution order, and consumers of the library might import the wrong class.
Given that mcp.ts
provides the intended unified entry point, this alias is dangerous and should be removed to avoid ambiguity.
if isinstance(command_or_string, str) and re.match(r'^https?://', command_or_string): | ||
# Import the SSE client implementation | ||
from .mcp_sse import SSEMCPClient | ||
self.sse_client = SSEMCPClient(command_or_string, debug=debug, timeout=timeout) | ||
self._tools = list(self.sse_client.tools) | ||
self.is_sse = True | ||
# Determine if it's SSE or HTTP streaming based on URL pattern | ||
if '/sse' in command_or_string or command_or_string.endswith('/sse'): | ||
# Import the SSE client implementation | ||
from .mcp_sse import SSEMCPClient | ||
self.sse_client = SSEMCPClient(command_or_string, debug=debug, timeout=timeout) | ||
self._tools = list(self.sse_client.tools) | ||
self.is_sse = True | ||
self.is_http_streaming = False | ||
else: | ||
# Import the HTTP streaming client implementation | ||
from .mcp_http_streaming import HTTPStreamingMCPClient | ||
# Extract headers from kwargs if provided | ||
headers = kwargs.get('headers', None) | ||
self.http_client = HTTPStreamingMCPClient(command_or_string, headers=headers, debug=debug, timeout=timeout) | ||
self._tools = list(self.http_client.tools) | ||
self.is_sse = False | ||
self.is_http_streaming = True | ||
self.is_npx = False | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The transport detection logic in __init__
is inconsistent with the new _detect_transport
method and does not handle the transport
keyword argument for explicit selection. This means features like forcing a transport type or using more complex SSE URL patterns (e.g., with query parameters) will not work as intended.
The __init__
method should be updated to use the _detect_transport
method to ensure consistent and correct behavior.
if isinstance(command_or_string, str) and re.match(r'^https?://', command_or_string):
transport_type = self._detect_transport(command_or_string, kwargs.get('transport'))
if self.debug:
logging.getLogger("mcp-wrapper").debug(f"Detected transport: {transport_type} for URL: {command_or_string}")
if transport_type == 'sse':
# Import the SSE client implementation
from .mcp_sse import SSEMCPClient
self.sse_client = SSEMCPClient(command_or_string, debug=debug, timeout=timeout)
self._tools = list(self.sse_client.tools)
self.is_sse = True
self.is_http_streaming = False
else: # http-streaming
# Import the HTTP streaming client implementation
from .mcp_http_streaming import HTTPStreamingMCPClient
# Extract headers from kwargs if provided
headers = kwargs.get('headers', None)
self.http_client = HTTPStreamingMCPClient(command_or_string, headers=headers, debug=debug, timeout=timeout)
self._tools = list(self.http_client.tools)
self.is_sse = False
self.is_http_streaming = True
self.is_npx = False
return
def __init__(self, url: str, headers: Optional[Dict[str, str]] = None): | ||
self.url = url | ||
self.headers = headers or {} | ||
self.client = httpx.AsyncClient(timeout=60.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timeout
for the httpx.AsyncClient
is hardcoded, which means the timeout value passed to HTTPStreamingMCPClient
is ignored. The transport should accept and use the timeout value.
def __init__(self, url: str, headers: Optional[Dict[str, str]] = None): | |
self.url = url | |
self.headers = headers or {} | |
self.client = httpx.AsyncClient(timeout=60.0) | |
def __init__(self, url: str, headers: Optional[Dict[str, str]] = None, timeout: float = 60.0): | |
self.url = url | |
self.headers = headers or {} | |
self.client = httpx.AsyncClient(timeout=timeout) |
while not self._closed: | ||
try: | ||
message = await asyncio.wait_for( | ||
self._request_queue.get(), | ||
timeout=0.1 | ||
) | ||
if message is None: | ||
break | ||
chunk = json.dumps(message).encode('utf-8') + b'\n' | ||
yield chunk | ||
except asyncio.TimeoutError: | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _request_iterator
uses asyncio.wait_for
with a short timeout in a loop. This creates a busy-wait pattern that consumes CPU cycles unnecessarily when there are no messages to send. A more efficient approach is to simply await
the queue's get()
method, which will suspend the coroutine until a message is available.
while not self._closed:
message = await self._request_queue.get()
if message is None:
break
chunk = json.dumps(message).encode('utf-8') + b'\n'
yield chunk
} | ||
|
||
private generateSessionId(): string { | ||
return `session_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The substr
method is considered a legacy feature and is deprecated. It's recommended to use substring
or slice
instead for better compatibility and to avoid using deprecated APIs.
return `session_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; | |
return `session_${Date.now()}_${Math.random().toString(36).slice(2, 11)}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: MCP Constructor Fails Transport Detection
The MCP
class constructor's transport detection logic is flawed. It uses a simple substring check ('/sse' in url
or url.endswith('/sse')
) for SSE, which can lead to incorrect client selection. Additionally, the constructor fails to extract and utilize the transport
parameter from kwargs
, rendering explicit transport selection (e.g., MCP(url, transport="sse")
) non-functional. Both issues arise because the constructor does not call the more robust _detect_transport
method, which was designed to handle comprehensive URL pattern matching and explicit transport specification.
src/praisonai-agents/praisonaiagents/mcp/mcp.py#L191-L212
PraisonAI/src/praisonai-agents/praisonaiagents/mcp/mcp.py
Lines 191 to 212 in 3570545
# Check if this is an HTTP URL | |
if isinstance(command_or_string, str) and re.match(r'^https?://', command_or_string): | |
# Determine if it's SSE or HTTP streaming based on URL pattern | |
if '/sse' in command_or_string or command_or_string.endswith('/sse'): | |
# Import the SSE client implementation | |
from .mcp_sse import SSEMCPClient | |
self.sse_client = SSEMCPClient(command_or_string, debug=debug, timeout=timeout) | |
self._tools = list(self.sse_client.tools) | |
self.is_sse = True | |
self.is_http_streaming = False | |
else: | |
# Import the HTTP streaming client implementation | |
from .mcp_http_streaming import HTTPStreamingMCPClient | |
# Extract headers from kwargs if provided | |
headers = kwargs.get('headers', None) | |
self.http_client = HTTPStreamingMCPClient(command_or_string, headers=headers, debug=debug, timeout=timeout) | |
self._tools = list(self.http_client.tools) | |
self.is_sse = False | |
self.is_http_streaming = True | |
self.is_npx = False | |
return |
Bug: Incorrect Stream Initialization in HTTP Client
The HTTPStreamingMCPClient._initialize
method incorrectly calls session.initialize()
with read_stream
and write_stream
. The ClientSession.initialize()
method does not accept these parameters; instead, the streams should be passed to the ClientSession
constructor. This will cause a runtime error.
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py#L317-L319
PraisonAI/src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py
Lines 317 to 319 in 3570545
# Initialize session with transport | |
await self.session.initialize(read_stream, write_stream) |
Bug: Async Task Reference Loss Causes Stream Termination
The asyncio.create_task()
call for _handle_stream
does not store a reference to the created task. This allows the task to be garbage collected prematurely, leading to unexpected stream termination. The task should be stored in an instance variable to prevent this.
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py#L72-L73
# Start background task for handling the stream | |
asyncio.create_task(self._handle_stream(headers)) |
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $50.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (7)
test_backward_compatibility.py (1)
20-143
: Improve boolean comparisons to follow Python idioms.The tests are comprehensive and well-structured. However, use Python's truthiness checks instead of explicit boolean comparisons for cleaner code.
Apply these changes throughout the file:
- assert mcp.is_sse == True + assert mcp.is_sse - assert mcp.is_http_streaming == False + assert not mcp.is_http_streamingThis applies to all similar comparisons in lines 24, 26, 36, 38, 48, 50, 61, 63, 74, 76, 78, 97, 98, 102, and 103.
src/praisonai-ts/examples/tools/mcp-http-streaming.ts (1)
95-100
: Use explicit function signatures instead of genericFunction
type.The
Function
type is too permissive and can be a source of bugs. Define a more specific type for tool functions.Replace the generic type with a specific signature:
- const toolFunctions: Record<string, Function> = {}; + const toolFunctions: Record<string, (params?: any) => Promise<any>> = {};src/praisonai-agents/praisonaiagents/mcp/mcp.py (1)
192-210
: Well-implemented transport auto-detection with HTTP-Streaming support!The logic correctly identifies SSE endpoints and defaults to HTTP-Streaming for other URLs, maintaining backward compatibility.
Minor simplification on line 206:
- headers = kwargs.get('headers', None) + headers = kwargs.get('headers')docs/MCP_HTTP_STREAMING.md (2)
93-93
: Add language specifier to code block.The Python architecture diagram should have a language specifier for better syntax highlighting.
-``` +```text MCP (main class)
105-105
: Add language specifier to code block.The TypeScript architecture diagram should have a language specifier for better syntax highlighting.
-``` +```text MCP (unified class)src/praisonai-ts/src/tools/httpStreamingTransport.ts (1)
396-417
: Consider making checkDuplexSupport public to avoid @ts-ignore.The factory function works well, but accessing a private method requires
@ts-ignore
. Consider makingcheckDuplexSupport
a public static method.- private checkDuplexSupport(): boolean { + public static checkDuplexSupport(): boolean {Then update the factory function:
- // @ts-ignore - if (!transport.checkDuplexSupport() || options.fallbackMode) { + if (!HTTPStreamingTransport.checkDuplexSupport() || options.fallbackMode) {src/praisonai-ts/src/tools/mcpHttpStreaming.ts (1)
57-57
: Use optional chaining for cleaner code.Static analysis correctly suggests using optional chaining here.
- if (result && result.content) { + if (result?.content) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/MCP_HTTP_STREAMING.md
(1 hunks)src/praisonai-agents/examples/mcp_http_streaming_example.py
(1 hunks)src/praisonai-agents/praisonaiagents/mcp/mcp.py
(6 hunks)src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py
(1 hunks)src/praisonai-ts/examples/tools/mcp-http-streaming.ts
(1 hunks)src/praisonai-ts/src/tools/httpStreamingTransport.ts
(1 hunks)src/praisonai-ts/src/tools/index.ts
(1 hunks)src/praisonai-ts/src/tools/mcp.ts
(1 hunks)src/praisonai-ts/src/tools/mcpHttpStreaming.ts
(1 hunks)test_backward_compatibility.py
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
`src/praisonai-ts/src/tools/!({README.md,index.ts,test.ts})`: Tool files in 'src...
src/praisonai-ts/src/tools/!({README.md,index.ts,test.ts})
: Tool files in 'src/tools/' should replicate the logic of their Python counterparts, implementing the same functionality in TypeScript.
📄 Source: CodeRabbit Inference Engine (src/praisonai-ts/.cursorrules)
List of files the instruction was applied to:
src/praisonai-ts/src/tools/index.ts
src/praisonai-ts/src/tools/mcp.ts
src/praisonai-ts/src/tools/httpStreamingTransport.ts
src/praisonai-ts/src/tools/mcpHttpStreaming.ts
`src/praisonai-ts/src/tools/index.ts`: The 'src/tools/index.ts' file should re-export tool functions for simplified import by consumers.
src/praisonai-ts/src/tools/index.ts
: The 'src/tools/index.ts' file should re-export tool functions for simplified import by consumers.
📄 Source: CodeRabbit Inference Engine (src/praisonai-ts/.cursorrules)
List of files the instruction was applied to:
src/praisonai-ts/src/tools/index.ts
`src/praisonai-ts/src/**/*.ts`: In TypeScript, change Python return types (e.g.,...
src/praisonai-ts/src/**/*.ts
: In TypeScript, change Python return types (e.g., dict, list) to appropriate TypeScript types (e.g., object, Record<string, any>, Promise, etc.).
Each class in the TypeScript codebase should mirror the constructor parameters and method signatures of its Python counterpart, adapted to TypeScript syntax and typing.
Implement error handling with error logging, retry mechanisms, fallback strategies, and error display functions as described in the Python library.
Ensure that each TypeScript class or function that corresponds to a Python class or function includes appropriate TypeScript type annotations for parameters and return values.
Use async/await for asynchronous operations, especially for LLM calls and tool functions that perform I/O.
📄 Source: CodeRabbit Inference Engine (src/praisonai-ts/.windsurfrules)
List of files the instruction was applied to:
src/praisonai-ts/src/tools/index.ts
src/praisonai-ts/src/tools/mcp.ts
src/praisonai-ts/src/tools/httpStreamingTransport.ts
src/praisonai-ts/src/tools/mcpHttpStreaming.ts
`src/praisonai-ts/src/tools/index.ts`: The 'src/tools/index.ts' file should re-export tool functions to simplify imports for consumers.
src/praisonai-ts/src/tools/index.ts
: The 'src/tools/index.ts' file should re-export tool functions to simplify imports for consumers.
📄 Source: CodeRabbit Inference Engine (src/praisonai-ts/.windsurfrules)
List of files the instruction was applied to:
src/praisonai-ts/src/tools/index.ts
`src/praisonai-agents/praisonaiagents/mcp/**/*.py`: Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-agents/praisonaiagents/mcp/**/*.py
: Implement MCP server and SSE support for distributed execution and real-time communication inpraisonaiagents/mcp/
.
📄 Source: CodeRabbit Inference Engine (src/praisonai-agents/CLAUDE.md)
List of files the instruction was applied to:
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py
src/praisonai-agents/praisonaiagents/mcp/mcp.py
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-ts/src/tools/index.ts (11)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/index.ts : The 'src/tools/index.ts' file should re-export tool functions to simplify imports for consumers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/tools/index.ts : The 'src/tools/index.ts' file should re-export tool functions for simplified import by consumers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/index.ts : Export main classes and functions from 'src/index.ts' to provide a simple import path for consumers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/index.ts : The main entry point 'src/index.ts' should re-export key classes and functions (such as 'Agent', 'Agents', 'Task', etc.) for easy import by consumers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/*Tools.ts : Add any required third-party TypeScript libraries (e.g., node-fetch, cheerio, duckdb, yaml, xml2js) as dependencies when implementing corresponding tool or utility functionality.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/*Tools.ts : Each tool file in 'src/tools/' should replicate the logic of its Python counterpart, implementing the same functionality in TypeScript.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/tools/!({README.md,index.ts,test.ts}) : Tool files in 'src/tools/' should replicate the logic of their Python counterparts, implementing the same functionality in TypeScript.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/test.ts : The 'src/tools/test.ts' file should serve as a script for running internal tests or examples for each tool.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/tools/test.ts : The 'src/tools/test.ts' file should provide a script for running each tool's internal test or example.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/{llm,agent,agents,task}/**/*.ts : Replace all references to 'LLM' or 'litellm' with 'aisdk' usage in TypeScript code.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Add any required third-party TypeScript libraries (such as 'node-fetch', 'cheerio', 'duckdb', 'yaml', 'xml2js', etc.) as dependencies when implementing corresponding functionality.
test_backward_compatibility.py (2)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/tests/**/*.py : Test files should be placed in the `tests/` directory and demonstrate specific usage patterns, serving as both test and documentation.
src/praisonai-agents/examples/mcp_http_streaming_example.py (1)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-ts/examples/tools/mcp-http-streaming.ts (10)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/test.ts : The 'src/tools/test.ts' file should serve as a script for running internal tests or examples for each tool.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/tools/test.ts : The 'src/tools/test.ts' file should provide a script for running each tool's internal test or example.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/tools/!({README.md,index.ts,test.ts}) : Tool files in 'src/tools/' should replicate the logic of their Python counterparts, implementing the same functionality in TypeScript.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/main.ts : The main script 'src/main.ts' should set up logging, callbacks, register display callbacks, and integrate with 'aisdk' if needed.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/index.ts : The main entry point 'src/index.ts' should re-export key classes and functions (such as 'Agent', 'Agents', 'Task', etc.) for easy import by consumers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/*Tools.ts : Add any required third-party TypeScript libraries (e.g., node-fetch, cheerio, duckdb, yaml, xml2js) as dependencies when implementing corresponding tool or utility functionality.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/main.ts : The 'src/main.ts' file should set up logging, callbacks, register display callbacks, and integrate with 'aisdk' if needed.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/*Tools.ts : Each tool file in 'src/tools/' should replicate the logic of its Python counterpart, implementing the same functionality in TypeScript.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/README.md : The 'src/tools/README.md' file should provide documentation for tool developers on how to write tools in JS/TS.
docs/MCP_HTTP_STREAMING.md (1)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-ts/src/tools/mcp.ts (6)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/index.ts : The main entry point 'src/index.ts' should re-export key classes and functions (such as 'Agent', 'Agents', 'Task', etc.) for easy import by consumers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/*Tools.ts : Each tool file in 'src/tools/' should replicate the logic of its Python counterpart, implementing the same functionality in TypeScript.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/index.ts : The 'src/tools/index.ts' file should re-export tool functions to simplify imports for consumers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/tools/index.ts : The 'src/tools/index.ts' file should re-export tool functions for simplified import by consumers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/index.ts : Export main classes and functions from 'src/index.ts' to provide a simple import path for consumers.
src/praisonai-ts/src/tools/httpStreamingTransport.ts (1)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py (1)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-agents/praisonaiagents/mcp/mcp.py (1)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-ts/src/tools/mcpHttpStreaming.ts (7)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/*Tools.ts : Add any required third-party TypeScript libraries (e.g., node-fetch, cheerio, duckdb, yaml, xml2js) as dependencies when implementing corresponding tool or utility functionality.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/*Tools.ts : Each tool file in 'src/tools/' should replicate the logic of its Python counterpart, implementing the same functionality in TypeScript.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/test.ts : The 'src/tools/test.ts' file should serve as a script for running internal tests or examples for each tool.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/tools/!({README.md,index.ts,test.ts}) : Tool files in 'src/tools/' should replicate the logic of their Python counterparts, implementing the same functionality in TypeScript.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.windsurfrules:0-0
Timestamp: 2025-06-30T10:06:44.129Z
Learning: Applies to src/praisonai-ts/src/tools/index.ts : The 'src/tools/index.ts' file should re-export tool functions to simplify imports for consumers.
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-ts/.cursorrules:0-0
Timestamp: 2025-06-30T10:05:51.843Z
Learning: Applies to src/praisonai-ts/src/tools/index.ts : The 'src/tools/index.ts' file should re-export tool functions for simplified import by consumers.
🧬 Code Graph Analysis (3)
src/praisonai-agents/examples/mcp_http_streaming_example.py (2)
src/praisonai-ts/src/tools/mcp.ts (1)
MCP
(33-173)src/praisonai-agents/praisonaiagents/mcp/mcp.py (1)
MCP
(103-547)
src/praisonai-agents/praisonaiagents/mcp/mcp.py (2)
src/praisonai-agents/praisonaiagents/mcp/mcp_sse.py (2)
SSEMCPClient
(143-228)to_openai_tools
(226-228)src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py (2)
HTTPStreamingMCPClient
(274-353)to_openai_tools
(346-348)
src/praisonai-ts/src/tools/mcpHttpStreaming.ts (3)
src/praisonai-ts/src/tools/httpStreamingTransport.ts (2)
HTTPStreamingTransportOptions
(6-15)createHTTPStreamingTransport
(399-417)src/praisonai-ts/src/tools/mcp.ts (3)
HTTPStreamingTransportOptions
(179-179)MCPTool
(176-176)MCPHttpStreaming
(178-178)src/praisonai-ts/src/tools/index.ts (1)
BaseTool
(8-20)
🪛 Ruff (0.11.9)
test_backward_compatibility.py
24-24: Avoid equality comparisons to True
; use if mcp.is_sse:
for truth checks
Replace with mcp.is_sse
(E712)
26-26: Avoid equality comparisons to False
; use if not mcp.is_http_streaming:
for false checks
Replace with not mcp.is_http_streaming
(E712)
36-36: Avoid equality comparisons to False
; use if not mcp.is_sse:
for false checks
Replace with not mcp.is_sse
(E712)
38-38: Avoid equality comparisons to True
; use if mcp.is_http_streaming:
for truth checks
Replace with mcp.is_http_streaming
(E712)
48-48: Avoid equality comparisons to False
; use if not mcp.is_sse:
for false checks
Replace with not mcp.is_sse
(E712)
50-50: Avoid equality comparisons to False
; use if not mcp.is_http_streaming:
for false checks
Replace with not mcp.is_http_streaming
(E712)
61-61: Avoid equality comparisons to False
; use if not mcp.is_sse:
for false checks
Replace with not mcp.is_sse
(E712)
63-63: Avoid equality comparisons to False
; use if not mcp.is_http_streaming:
for false checks
Replace with not mcp.is_http_streaming
(E712)
74-74: Avoid equality comparisons to True
; use if mcp.is_npx:
for truth checks
Replace with mcp.is_npx
(E712)
76-76: Avoid equality comparisons to False
; use if not mcp.is_sse:
for false checks
Replace with not mcp.is_sse
(E712)
78-78: Avoid equality comparisons to False
; use if not mcp.is_http_streaming:
for false checks
Replace with not mcp.is_http_streaming
(E712)
97-97: Avoid equality comparisons to True
; use if mcp_sse.is_sse:
for truth checks
Replace with mcp_sse.is_sse
(E712)
98-98: Avoid equality comparisons to False
; use if not mcp_sse.is_http_streaming:
for false checks
Replace with not mcp_sse.is_http_streaming
(E712)
102-102: Avoid equality comparisons to False
; use if not mcp_http.is_sse:
for false checks
Replace with not mcp_http.is_sse
(E712)
103-103: Avoid equality comparisons to True
; use if mcp_http.is_http_streaming:
for truth checks
Replace with mcp_http.is_http_streaming
(E712)
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py
12-12: typing.Callable
imported but unused
Remove unused import: typing.Callable
(F401)
15-15: mcp.client.session.BaseSession
imported but unused
Remove unused import: mcp.client.session.BaseSession
(F401)
16-16: mcp.shared.context.RequestContext
imported but unused
Remove unused import: mcp.shared.context.RequestContext
(F401)
17-17: mcp.types.Tool
imported but unused
Remove unused import: mcp.types.Tool
(F401)
src/praisonai-agents/praisonaiagents/mcp/mcp.py
206-206: Use kwargs.get('headers')
instead of kwargs.get('headers', None)
Replace kwargs.get('headers', None)
with kwargs.get('headers')
(SIM910)
🪛 Biome (1.9.4)
src/praisonai-ts/examples/tools/mcp-http-streaming.ts
[error] 95-96: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
src/praisonai-ts/src/tools/mcpHttpStreaming.ts
[error] 57-57: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 markdownlint-cli2 (0.17.2)
docs/MCP_HTTP_STREAMING.md
93-93: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
105-105: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (3)
- GitHub Check: test-core (3.11)
- GitHub Check: quick-test
- GitHub Check: Cursor BugBot
🔇 Additional comments (14)
src/praisonai-ts/src/tools/index.ts (1)
25-27
: LGTM!The new exports for HTTP-Streaming modules follow the established pattern and properly expose the new transport functionality.
src/praisonai-agents/examples/mcp_http_streaming_example.py (1)
1-109
: Excellent examples demonstrating all HTTP-Streaming features!The examples are comprehensive, well-structured, and clearly demonstrate auto-detection, explicit transport selection, custom headers, and backward compatibility.
src/praisonai-agents/praisonaiagents/mcp/mcp.py (1)
305-340
: Robust transport detection implementation!The
_detect_transport
method provides comprehensive pattern matching for SSE endpoints and proper validation for explicit transport selection. The regex patterns cover common SSE URL conventions effectively.src/praisonai-ts/src/tools/mcp.ts (4)
1-54
: Well-structured unified MCP client implementation!The class design with automatic transport detection and explicit selection options provides excellent flexibility. The constructor properly initializes all fields and sets sensible defaults.
56-84
: Robust transport detection logic!The method provides comprehensive URL pattern matching for SSE endpoints and sensible fallback to HTTP-Streaming. The normalization of transport types ensures consistent behavior.
86-126
: Excellent initialization with proper error handling!The method correctly handles idempotent initialization, creates the appropriate transport-specific client, and ensures cleanup on failure. The error handling pattern prevents resource leaks.
128-188
: Clean implementation of utility methods and exports!All methods properly delegate to the underlying client or provide useful metadata. The helper function
createMCPClient
offers a convenient initialization pattern.docs/MCP_HTTP_STREAMING.md (1)
1-251
: Excellent comprehensive documentation!The documentation thoroughly covers HTTP-Streaming support with clear examples, implementation details, and migration guidance. The structure makes it easy for developers to understand and adopt the new transport method.
src/praisonai-ts/src/tools/httpStreamingTransport.ts (2)
30-245
: Solid implementation of modern HTTP streaming transport!The class properly handles duplex streaming with good buffer management for partial messages. The
@ts-ignore
comment on line 77 is justified as the duplex property isn't in standard TypeScript types yet. Browser detection heuristics are reasonable for determining duplex support.
247-394
: Well-designed fallback transport for browser compatibility!The polling-based approach with session management provides good backward compatibility. The 100ms polling interval strikes a balance between responsiveness and server load.
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py (3)
21-34
: Good thread-safe event loop management!The singleton pattern with lock ensures thread-safe initialization, and the daemon thread allows proper cleanup on program exit.
36-165
: Well-implemented async HTTP streaming transport!The transport properly handles bidirectional streaming with good buffer management for NDJSON parsing. The adapter classes correctly bridge between the transport and MCP SDK expectations.
167-353
: Excellent MCP tool and client implementation!The dynamic signature generation from JSON schema is particularly clever, enabling proper function introspection. The async/sync bridge using the event loop allows synchronous code to use async MCP tools seamlessly.
src/praisonai-ts/src/tools/mcpHttpStreaming.ts (1)
101-209
: Well-structured HTTP streaming MCP client!The implementation provides excellent parity with the Python version, including proper initialization, tool discovery, and OpenAI compatibility. The helper function and backward-compatible export enhance usability.
constructor(toolInfo: { name: string; description: string; inputSchema?: any }, client: Client) { | ||
super(); | ||
this.toolInfo = toolInfo; | ||
this.client = client; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix super() call to pass required parameters.
The BaseTool
constructor expects name
and description
parameters, but they're not being passed in the super() call.
constructor(toolInfo: { name: string; description: string; inputSchema?: any }, client: Client) {
- super();
+ super(toolInfo.name, toolInfo.description);
this.toolInfo = toolInfo;
this.client = client;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
constructor(toolInfo: { name: string; description: string; inputSchema?: any }, client: Client) { | |
super(); | |
this.toolInfo = toolInfo; | |
this.client = client; | |
} | |
constructor(toolInfo: { name: string; description: string; inputSchema?: any }, client: Client) { | |
super(toolInfo.name, toolInfo.description); | |
this.toolInfo = toolInfo; | |
this.client = client; | |
} |
🤖 Prompt for AI Agents
In src/praisonai-ts/src/tools/mcpHttpStreaming.ts lines 31 to 35, the
constructor calls super() without passing the required name and description
parameters expected by the BaseTool constructor. Fix this by passing
toolInfo.name and toolInfo.description as arguments to the super() call to
properly initialize the base class.
Secure MCP Filesystem Server running on stdio |
Claude finished @MervinPraison's task —— View job Fixed MCP Schema Validation Error ✅I've successfully fixed the issue and committed the changes to the PR branch. What Was WrongThe HTTP-Streaming MCP client was missing the What I Fixed
The Fixdef _fix_array_schemas(self, schema):
"""Fix array schemas by adding missing 'items' attribute required by OpenAI."""
# ... (implementation adds default items for arrays)
def to_openai_tool(self) -> Dict[str, Any]:
"""Convert this tool to OpenAI function calling format."""
# Fix array schemas to include 'items' attribute
fixed_schema = self._fix_array_schemas(self.input_schema)
# ... (return OpenAI formatted tool) This ensures that MCP tools with array parameters (like SummaryThe HTTP-Streaming client now properly handles array schemas, matching the behavior of the SSE and standard MCP clients. This resolves the "array schema missing items" error you encountered with the MCP filesystem server. --- • View changes |
- Add _fix_array_schemas method to HTTPStreamingMCPTool class - Apply schema fix in to_openai_tool method to ensure array types have 'items' property - Remove unused imports (Callable, BaseSession, RequestContext, Tool) This fixes OpenAI function calling validation errors for MCP tools with array parameters. Co-authored-by: Mervin Praison <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py (2)
36-39
: Fix timeout parameter handling in HTTPStreamingTransport constructor.The timeout is hardcoded to 60.0 seconds, ignoring any timeout value passed to the client.
- def __init__(self, url: str, headers: Optional[Dict[str, str]] = None): + def __init__(self, url: str, headers: Optional[Dict[str, str]] = None, timeout: float = 60.0): self.url = url self.headers = headers or {} - self.client = httpx.AsyncClient(timeout=60.0) + self.client = httpx.AsyncClient(timeout=timeout)
98-112
: Replace busy-wait pattern with efficient queue blocking.The current implementation uses
asyncio.wait_for
with a short timeout in a loop, creating unnecessary CPU usage when no messages are available.async def _request_iterator(self) -> AsyncIterator[bytes]: """Generate request chunks from the queue.""" while not self._closed: - try: - message = await asyncio.wait_for( - self._request_queue.get(), - timeout=0.1 - ) - if message is None: - break - chunk = json.dumps(message).encode('utf-8') + b'\n' - yield chunk - except asyncio.TimeoutError: - continue + message = await self._request_queue.get() + if message is None: + break + chunk = json.dumps(message).encode('utf-8') + b'\n' + yield chunk
🧹 Nitpick comments (2)
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py (2)
367-367
: Add safety check for inputSchema attribute access.The code uses
hasattr
to check forinputSchema
but could be more robust in handling missing or invalid schemas.- input_schema=tool_info.inputSchema if hasattr(tool_info, 'inputSchema') else {}, + input_schema=getattr(tool_info, 'inputSchema', {}) if hasattr(tool_info, 'inputSchema') else {},
77-77
: Consider making the streaming endpoint path configurable.The hardcoded path
/mcp/v1/stream
might need to be configurable for different server implementations.- f"{self.url}/mcp/v1/stream", + f"{self.url}{getattr(self, 'stream_path', '/mcp/v1/stream')}",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`src/praisonai-agents/praisonaiagents/mcp/**/*.py`: Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-agents/praisonaiagents/mcp/**/*.py
: Implement MCP server and SSE support for distributed execution and real-time communication inpraisonaiagents/mcp/
.
📄 Source: CodeRabbit Inference Engine (src/praisonai-agents/CLAUDE.md)
List of files the instruction was applied to:
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py (1)
Learnt from: CR
PR: MervinPraison/PraisonAI#0
File: src/praisonai-agents/CLAUDE.md:0-0
Timestamp: 2025-06-30T10:06:17.673Z
Learning: Applies to src/praisonai-agents/praisonaiagents/mcp/**/*.py : Implement MCP server and SSE support for distributed execution and real-time communication in `praisonaiagents/mcp/`.
⏰ 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). (3)
- GitHub Check: test-core (3.11)
- GitHub Check: quick-test
- GitHub Check: quick-test
self._response_queue = asyncio.Queue() | ||
|
||
# Start background task for handling the stream | ||
asyncio.create_task(self._handle_stream(headers)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Store background task reference for proper cleanup.
The background task is created but not stored, making it difficult to cancel during shutdown.
# Start background task for handling the stream
- asyncio.create_task(self._handle_stream(headers))
+ self._stream_task = asyncio.create_task(self._handle_stream(headers))
And update the close method to cancel the task:
async def close(self):
"""Close the transport."""
self._closed = True
if hasattr(self, '_stream_task'):
self._stream_task.cancel()
try:
await self._stream_task
except asyncio.CancelledError:
pass
if self._request_queue:
await self._request_queue.put(None)
await self.client.aclose()
🤖 Prompt for AI Agents
In src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py at line 70,
the background task created by asyncio.create_task is not stored, preventing
proper cancellation during shutdown. Fix this by assigning the created task to
an instance variable like self._stream_task. Then, update the close method to
check for this task, cancel it, and await its completion while handling
asyncio.CancelledError, ensuring proper cleanup.
# Create transport | ||
transport = HTTPStreamingTransport(self.server_url, self.headers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass timeout parameter to HTTPStreamingTransport.
The transport is created without passing the timeout parameter, which means it will use the hardcoded default even when a custom timeout is specified.
- transport = HTTPStreamingTransport(self.server_url, self.headers)
+ transport = HTTPStreamingTransport(self.server_url, self.headers, self.timeout)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# Create transport | |
transport = HTTPStreamingTransport(self.server_url, self.headers) | |
# Create transport | |
transport = HTTPStreamingTransport(self.server_url, self.headers, self.timeout) |
🤖 Prompt for AI Agents
In src/praisonai-agents/praisonaiagents/mcp/mcp_http_streaming.py around lines
346 to 347, the HTTPStreamingTransport instance is created without passing the
timeout parameter, causing it to use the default timeout instead of a custom
one. Modify the transport creation to include the timeout argument by passing
the appropriate timeout value from the current context or class attribute to the
HTTPStreamingTransport constructor.
User description
Fixes #722
Summary
Implemented complete HTTP-Streaming support for MCP (Model Context Protocol) alongside existing SSE support with full backward compatibility.
Key Features
/sse
use SSE, others use HTTP-streamingtransport
parameterImplementation Details
Testing
Created
test_backward_compatibility.py
to ensure all existing usage patterns continue to work.Generated with Claude Code
PR Type
Enhancement
Description
Add HTTP-Streaming transport support for MCP protocol
Implement smart auto-detection based on URL patterns
Maintain 100% backward compatibility with existing code
Add comprehensive examples and documentation
Changes diagram
Changes walkthrough 📝
8 files
Add comprehensive HTTP-Streaming usage examples
Implement transport detection and HTTP-Streaming integration
Create HTTP-Streaming MCP client implementation
Add TypeScript HTTP-Streaming examples
Implement HTTP-Streaming transport for TypeScript
Export new HTTP-Streaming modules
Create unified MCP client with transport selection
Implement TypeScript HTTP-Streaming MCP client
1 files
Add backward compatibility test suite
1 files
Add comprehensive HTTP-Streaming documentation
Summary by CodeRabbit
New Features
Documentation
Examples
Tests