Polish simple mcp server#417
Conversation
Extension release summary🚀 Will release on merge
See the contributing guide for details. |
amylin1249
force-pushed
the
polish-simple-mcp-server
branch
2 times, most recently
from
d3bd60a to
595ffbf
Compare
amylin1249
force-pushed
the
polish-simple-mcp-server
branch
from
11e4a0c to
2a5abe2
Compare
amylin1249
force-pushed
the
polish-simple-mcp-server
branch
2 times, most recently
from
6bd51fb to
779eebe
Compare
| mcp_app = mcp.http_app(path="/mcp", stateless_http=True) | ||
| app = FastAPI(title="Simple MCP Server with FastAPI", lifespan=mcp_app.lifespan) | ||
| app = FastAPI(title="MCP Server", lifespan=lifespan) | ||
| templates = Jinja2Templates(directory=".") |
There was a problem hiding this comment.
| templates = Jinja2Templates(directory=".") | |
| templates = Jinja2Templates(directory=".", autoescape=True) |
I believe the new starlette version we're using has different behavior that no longer forces autoescape so we should add it explicitly.
There was a problem hiding this comment.
Fixed in 2653ff6 - I set templates.env.autoescape = True in a separate line since the suggestion (although it had the right intent) raised a TypeError.
| session_token = request.headers.get("posit-connect-user-session-token") | ||
| if session_token: | ||
| try: | ||
| me = get_visitor_client(session_token).me |
There was a problem hiding this comment.
get_visitor_client() and .me are blocking HTTP calls but this handler is async, so they run on the event loop. While the event loop is blocked the process can't serve anything else, including MCP tool calls. Same problem in connect_whoami (line 107). I think to fix this we can make get_index_page a plain def (FastAPI will run it in a worker thread), and wrap the posit-sdk calls in connect_whoami in anyio.to_thread.run_sync. The cache will then be shared across threads and needs a lock: @cached(client_cache, lock=threading.Lock()).
There was a problem hiding this comment.
I moved the Connect calls in connect_whoami onto a background thread, and added the lock to the cache. But for get_index_page, I didn't make it a def because it has to stay async. It calls get_tools_info() (async), so I moved its blocking Connect call onto a thread instead. get_tools_info is async because it uses the official SDK's mcp.list_tools(), which we adopted when we replaced fastmcp to clear its package vulnerabilities (a lot was surfaced when I ran the package-vulnerability-scanner).
| "index.html.jinja", | ||
| { | ||
| "server_name": mcp.name, | ||
| "title": "MCP Server", |
There was a problem hiding this comment.
just curious why we switched to hardcoding the name here?
There was a problem hiding this comment.
This was my mistake, I think when I tried to make sure the app itself didn't include "FastAPI: " I hardcoded in two spots. Both should be reverted in the most recent commit.
2 participants
Fixes #416 (can be reviewed commit by commit)
fastmcppackage with the official MCP Python SDK (mcp), clearing all known package vulnerabilities (two critical) and roughly halving the dependency treeconnect_whoamiresolves the viewer from the injected session token, not API-key auth; removed the unusedx-mcp-authorizationheaderrequirements.txtfrom Python 3.11 (was exported for 3.14) and relaxed pins sofastapi/starletteresolve to patched releases; cleaned uppyproject.tomlMost notable change: the landing page greets the signed-in viewer by name, a live demo of viewer-identity auth. Before you add a "Connect Visitor API Key" integration it tells you to; once added, the same box confirms success by showing your name (or username).
Tested by manually deploying the app.