mcp-loader v2: streamable-HTTP transport for remote MCP servers (context7)
- New RemoteMcpClient implementing MCP streamable-HTTP per spec 2025-03-26:
POST JSON-RPC, parse application/json or text/event-stream responses,
round-trip optional Mcp-Session-Id, optional auth via 'headers' config.
- Refactor StdioMcpClient to share an IMcpClient interface with the remote
client; extension entry dispatches on cfg.type. Drops the v1 'remote
skipped with warning' code path.
- Bump MCP_PROTOCOL_VERSION to 2025-11-25 (single constant, both clients).
- 404 self-heal: when a remote returns 404 to a request carrying our
Mcp-Session-Id, drop the id, re-initialize, retry the request once
(per spec 2025-11-25 \u00a72.2). allowReinitOn404=false on the retry path
prevents recursion. Verified via mock-server smoke test.
- Sanitize pi-facing tool names to ^[A-Za-z][A-Za-z0-9_]{0,63}$. Anthropic
allows hyphens but Bedrock's Anthropic shim rejects them, causing entire
turns to 4xx silently when context7's hyphenated tools (resolve-library-id,
query-docs) were registered. Original MCP-side names are preserved in the
tool-execute closure, so sanitization is purely pi-facing.
- /mcp slash command: drop 'remote (skipped)' status label.
- Docs: README and AGENTS updated for transports, headers config, 404
self-heal, tool-name sanitization rationale, OAuth limitation.
End-to-end verified: context7 connects through pi, returns useful docs
(Bun streaming/SSE example fetched successfully).
This commit is contained in:
@@ -249,13 +249,15 @@ cp /opt/homebrew/Cellar/pi-coding-agent/*/libexec/lib/node_modules/@mariozechner
|
||||
|
||||
### `mcp-loader.ts`
|
||||
|
||||
Generic MCP stdio client — reads an `mcp` block from `~/.pi/agent/settings.json`
|
||||
and connects to each declared server, exposing the tools as namespaced pi tools.
|
||||
Generic MCP client — reads an `mcp` block from `~/.pi/agent/settings.json`
|
||||
and connects to each declared server (stdio subprocess or streamable-HTTP
|
||||
endpoint), exposing the tools as namespaced pi tools.
|
||||
|
||||
**Why this exists:** pi has no built-in MCP loader (unlike opencode and Claude
|
||||
Desktop). Adding each new MCP server as a hand-rolled extension (the way
|
||||
`mempalace.ts` does it) doesn't scale past 2-3 servers. This loader is the
|
||||
config-driven generalization — one extension, any number of servers.
|
||||
config-driven generalization — one extension, any number of servers, two
|
||||
transports.
|
||||
|
||||
**Key design decisions:**
|
||||
|
||||
@@ -265,42 +267,72 @@ config-driven generalization — one extension, any number of servers.
|
||||
multiple servers expose tools with the same short name. Tools that already
|
||||
start with `<servername>_` (the convention some servers like mempalace
|
||||
follow internally) skip the prefix to avoid double-prefixing.
|
||||
- **Tool names are sanitized to `^[A-Za-z][A-Za-z0-9_]{0,63}$`** before
|
||||
registering with pi. This is the strictest regex we have to satisfy:
|
||||
Anthropic's Messages API allows hyphens, but AWS Bedrock's Anthropic shim
|
||||
rejects them outright — a single hyphenated tool name causes the whole
|
||||
turn to 4xx silently, manifesting as "no output at all" once the offending
|
||||
server is enabled. context7 surfaces this because its tools are named
|
||||
`resolve-library-id` and `query-docs`. We replace any non-`[A-Za-z0-9_]`
|
||||
char with `_`, prepend `t_` if the result doesn't start with a letter, and
|
||||
truncate to 64 chars. The original MCP-side name is kept in the closure
|
||||
used by `client.callTool`, so the sanitization is purely pi-facing.
|
||||
- **Fail-soft per server.** A server that won't start (binary missing, init
|
||||
handshake fails) logs one stderr line and is skipped. Other servers
|
||||
continue. Pi keeps working.
|
||||
- **Stdio transport only in v1.** Remote/streamable-HTTP servers are
|
||||
detected and skipped with a warning. Adding remote support is the
|
||||
obvious v2 (context7 is the prime motivator). Avoided in v1 because
|
||||
streamable-HTTP needs SSE parsing, session management, and reconnect
|
||||
logic that triples the implementation size.
|
||||
handshake fails, remote 4xx/5xx) logs one stderr line and is skipped.
|
||||
Other servers continue. Pi keeps working.
|
||||
- **Two transports behind one interface.** `IMcpClient` (`start`, `tools`,
|
||||
`callTool`, `stop`) is implemented by `StdioMcpClient` (subprocess +
|
||||
newline-delimited JSON-RPC) and `RemoteMcpClient` (HTTP POST + JSON or
|
||||
SSE response). The extension entry dispatches on `cfg.type` and the
|
||||
rest of the code is transport-agnostic.
|
||||
- **Does not replace `mempalace.ts`.** The mempalace bridge has bespoke
|
||||
agent-identity injection from `$MEMPALACE_AGENT_NAME` that's worth
|
||||
preserving. Loader and bridge coexist. Listing `mempalace` in the `mcp`
|
||||
block would produce duplicate tool registrations — don't.
|
||||
- **No reconnect on subprocess death.** If a server's subprocess crashes
|
||||
- **No stdio reconnect on death.** If a stdio subprocess crashes
|
||||
mid-session, its tools become permanently unavailable until pi `/reload`s.
|
||||
Same limitation as the mempalace bridge today; not worth complicating v1.
|
||||
Same limitation as the mempalace bridge today.
|
||||
- **Remote sessions self-heal on 404.** Per spec 2025-11-25 §2.2, a server
|
||||
that has terminated a session MUST respond with HTTP 404 to requests
|
||||
carrying the stale `Mcp-Session-Id`. `RemoteMcpClient` catches this
|
||||
condition (404 + sessionAtStart was non-null), drops the id, runs a fresh
|
||||
`initialize` + `notifications/initialized`, and retries the original
|
||||
request once. Persistent 404s after refresh surface as errors.
|
||||
|
||||
**API used:**
|
||||
|
||||
- `pi.registerTool({ name, label, description, parameters, execute })` for
|
||||
each MCP tool exposed by each connected server.
|
||||
- `pi.on("session_shutdown", ...)` to SIGTERM all subprocesses cleanly.
|
||||
- `pi.on("session_shutdown", ...)` to SIGTERM stdio subprocesses and DELETE
|
||||
remote sessions cleanly.
|
||||
- `Type.Unsafe<...>(inputSchema)` from typebox to pass the MCP server's
|
||||
JSON schema through to pi without conversion (TypeBox schemas are
|
||||
plain JSON Schema at runtime).
|
||||
|
||||
**Internal MCP client:**
|
||||
**Internal MCP clients:**
|
||||
|
||||
- `class StdioMcpClient` — spawn subprocess, write newline-delimited
|
||||
JSON-RPC, parse newline-delimited responses, match by request `id`.
|
||||
- Sends `initialize` handshake with `protocolVersion: 2024-11-05`, then
|
||||
`notifications/initialized`, then `tools/list` to discover tools.
|
||||
- Stderr drained silently unless `PI_MCP_LOADER_DEBUG=1`.
|
||||
Stderr drained silently unless `PI_MCP_LOADER_DEBUG=1`.
|
||||
- `class RemoteMcpClient` — streamable-HTTP per MCP spec 2025-03-26.
|
||||
POST JSON-RPC body to a single URL with `Accept: application/json,
|
||||
text/event-stream`. Server replies either with one JSON response or an
|
||||
SSE stream containing `event: message` / `data: <json>` blocks; we
|
||||
consume the stream until the response with our `id` arrives, then
|
||||
cancel the reader. Optional `Mcp-Session-Id` header is captured on
|
||||
initialize and round-tripped on every subsequent request, then DELETEd
|
||||
on stop. Per-server `headers` config (e.g. `Authorization`) is merged
|
||||
into every request. No GET-stream subscription — server-initiated
|
||||
notifications are not consumed.
|
||||
- Both share `interface IMcpClient { serverName, tools, start, callTool, stop }`.
|
||||
Both send `initialize` handshake with `protocolVersion: MCP_PROTOCOL_VERSION`
|
||||
(currently `2025-11-25`, per the constant at the top of the file),
|
||||
then `notifications/initialized`, then `tools/list` to discover tools.
|
||||
|
||||
**Future v2 extensions:**
|
||||
**Future extensions:**
|
||||
|
||||
- Streamable-HTTP transport for remote servers (context7).
|
||||
- OAuth flow for remote servers that require it (today: pre-issued bearer
|
||||
tokens via `headers` only).
|
||||
- Per-tool enable/disable in settings.json (e.g.
|
||||
`"mcp.searxng.tools": ["web_search"]` to expose only a subset).
|
||||
- Reconnect-on-crash with exponential backoff.
|
||||
|
||||
Reference in New Issue
Block a user