add python sdk#16
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the sam-mcp-python SDK, providing a Python client for the Sovereign Agent Mesh (SAM) using the Model Context Protocol (MCP) over Unix Domain Sockets. The implementation includes a core client, a JSON-RPC protocol handler, a transport layer with HTTP-like framing, and an adapter for LangChain. Feedback highlights several critical issues: the manual construction of the initialized notification risks protocol desync and incorrect length calculation; the transport layer may hang indefinitely if the Content-Length is zero; and the LangChain adapter needs to extract inputSchema to provide an args_schema for proper agent interaction. Additionally, the transport should be updated to validate HTTP status codes to handle server errors gracefully.
| await self.transport.writer.write(( | ||
| f"POST /mcp HTTP/1.1\r\n" | ||
| f"Host: localhost\r\n" | ||
| f"Content-Type: application/json\r\n" | ||
| f"Content-Length: {len(json.dumps(notif))}\r\n" | ||
| f"\r\n" | ||
| f"{json.dumps(notif)}" | ||
| ).encode('utf-8')) | ||
| await self.transport.writer.drain() |
There was a problem hiding this comment.
The manual construction of the HTTP request for the initialized notification has several critical issues:
- Protocol Desync: It does not read the HTTP response from the server. Since this is a persistent Unix socket, the unread response will remain in the buffer and be incorrectly consumed as the response for the next request (e.g.,
get_tools), causing a protocol failure. - Incorrect Content-Length:
len(json.dumps(notif))calculates the number of characters, which is incorrect for multi-byte UTF-8 strings. It must use the byte length. - Encapsulation: It accesses
self.transport.writerdirectly, bypassing the transport's messaging logic.
You should use self.transport.send_message which handles framing, byte-length calculation, and response consumption correctly.
notif = Protocol.create_request("notifications/initialized")
await self.transport.send_message(json.dumps(notif))| if content_length == 0: | ||
| # If no content length, read until EOF | ||
| body = await self.reader.read(-1) | ||
| return body.decode('utf-8') |
There was a problem hiding this comment.
await self.reader.read(-1) will block until the connection is closed (EOF). Since the SAM node is a long-running process and this transport uses a persistent Unix socket, the connection is unlikely to be closed after a single request. This will cause the client to hang indefinitely if the server omits the Content-Length header or if the response is empty. If content_length is 0, the method should return an empty string immediately.
| if content_length == 0: | |
| # If no content length, read until EOF | |
| body = await self.reader.read(-1) | |
| return body.decode('utf-8') | |
| if content_length == 0: | |
| return "" |
| lc_tool = StructuredTool.from_function( | ||
| name=name, | ||
| description=description, | ||
| coroutine=make_call(name) | ||
| ) |
There was a problem hiding this comment.
The StructuredTool is created without an args_schema. MCP tools provide an inputSchema (JSON Schema) which is essential for LLMs to understand the required parameters. Without this, the resulting LangChain tools will have an empty or generic schema, making them difficult for agents to use correctly. Consider extracting the inputSchema from the tool definition and providing it to the StructuredTool (e.g., by dynamically generating a Pydantic model or using a custom args_schema class).
| headers_str = headers_data.decode('utf-8') | ||
| lines = headers_str.split("\r\n") |
There was a problem hiding this comment.
The transport parses headers but ignores the HTTP status line (e.g., HTTP/1.1 200 OK). If the SAM node returns an HTTP error (e.g., 401 Unauthorized or 500 Internal Server Error), the client will attempt to parse the error body as a valid JSON-RPC response, which may lead to confusing error messages. It is recommended to verify that the status code is in the 2xx range.
1 participant
No description provided.