Polish simple shiny chat with mcp#418
Conversation
Extension release summary🚀 Will release on merge
See the contributing guide for details. |
amylin1249
force-pushed
the
polish-simple-shiny-chat-with-mcp
branch
from
2cd4041 to
8a34b49
Compare
amylin1249
force-pushed
the
polish-simple-shiny-chat-with-mcp
branch
2 times, most recently
from
295425a to
afa6057
Compare
…andling and system prompt Duplicate registrations raise in chatlas (a server-name collision in add_session, or a tool-name overlap) and are caught by the except, so a successful register always adds exactly one new session and the new-server diff is never empty. Drop the unreachable empty-diff guard and keep the list-comprehension form. Also strip indentation from the system prompt string.
amylin1249
force-pushed
the
polish-simple-shiny-chat-with-mcp
branch
from
afa6057 to
6c8a3dd
Compare
| mcp_http_client = ( | ||
| httpx.AsyncClient(headers={"Authorization": f"Key {visitor_api_key}"}) | ||
| if visitor_api_key | ||
| else None | ||
| ) |
There was a problem hiding this comment.
If I'm following correctly, passing http_client replaces the client mcp would build itself, which uses httpx.Timeout(30, read=300) and follow_redirects=True. A bare AsyncClient defaults to a 5s timeout and no redirects, so a Connect-hosted tool call over 5s raises ReadTimeout, and a redirecting URL (e.g., trailing slash) fails to register. Should this match mcp's defaults?
| if httpx.URL(url).host == connect_host: | ||
| transport_kwargs["http_client"] = mcp_http_client |
There was a problem hiding this comment.
Should this compare the full origin (scheme + host + port) rather than just the host?
| else None | ||
| ) | ||
| if mcp_http_client is not None: | ||
| session.on_ended(mcp_http_client.aclose) |
There was a problem hiding this comment.
Should this also call chat.cleanup_mcp_tools() before closing the client?
… by origin, clean up sessions
|
Fixed all three as suggested. On the session cleanup, I also wrapped it in try/finally so that the client still closes even if cleanup_mcp_tools() errors. |
2 participants
Fixes #407 (can be reviewed commit by commit)
X-MCP-Authorizationheadernest-asyncio, and reconciled to Python 3.11Most notable change: The app had a bug where it could no longer connect to any MCP server. Routing auth through a pre-built
httpxclient fixed it, so it should be working again.Tested by manually deploying the app.