Retire mcp-loader.ts in favour of pi's built-in MCP support
pi has had MCP as a built-in extension (builtin:mcp) since 0.99.0. It reads ~/.pi/agent/mcp.json and adds `pi mcp add|list|login`, a /mcp manager, OAuth, per-tool exposure and resources. An extension that registers /mcp REPLACES the built-in, so with mcp-loader installed pi never read mcp.json and printed "registers command /mcp, so built-in extension mcp was not loaded" on every start. Measured on pi 1.1.0 (pi-devbox v1.11.0, tor-ms22) before removing it: - time to the (auto) footer in a real pty, 7 interleaved runs each: -ne 0.276 s | -ne -e mcp-loader.ts 0.283 s | -ne -e builtin:mcp 0.272 s - with no servers, the same prompt costs 79 input + 7680 cached tokens with the loader, the built-in, or neither - a stdio server (server-everything) connected via `pi mcp list` in 3 s, and its echo tool ran end to end from `pi -p` with direct exposure - no device in the fleet had an `mcp` block, so nothing needs migrating install.sh now prunes links into this repo whose target is gone (.ts and .ts.off), so the stale mcp-loader.ts link on a preserved pi-devbox volume is removed on the first boot of an image without the file. It also warns once if settings.json still carries an `mcp` block. Tested two-sided in a temp HOME: the new script removes the repo link and a stale .ts.off, and keeps a foreign dangling link and a real file. The old script leaves the dangling link. Re-runs are idempotent, and there is no warning without an mcp block. mempalace.ts (mempalace-toolkit) is unaffected: it registers mempalace_* tools itself. Its vendored RemoteMcpClient loses its sync partner; that is handled in mempalace-toolkit.
This commit is contained in:
@@ -23,7 +23,6 @@ extensions/
|
||||
notify.ts # Native terminal notification when agent finishes
|
||||
ext-toggle.ts # /ext slash command — list & toggle extensions at runtime
|
||||
todo.ts # `todo` tool for the agent + /todos for the user (copy of upstream example)
|
||||
mcp-loader.ts # Generic MCP server loader + /mcp slash command
|
||||
task.ts # `task` tool — pi-task (isolated child, PASS/FAIL envelope, boundary diff) as a tool
|
||||
fork-gate.ts # tool_call hook: block forks whose brief needs `task`; reason = the task(...) to make
|
||||
aws-sso.ts # /aws-sso: renew the AWS SSO session when it ends (local: host browser; remote: device code + ntfy push)
|
||||
@@ -68,7 +67,14 @@ LICENSE # MIT.
|
||||
Creates `~/.pi/agent/extensions/` proactively.
|
||||
2. Calls `build_install_set` to resolve which extensions to install based on
|
||||
`--only` / `--skip` flags (default: all `.ts` files in `extensions/`).
|
||||
3. For each selected extension: symlinks `extensions/<name>.ts` →
|
||||
3. `prune_stale_links` (after the confirmation prompt) — removes links in
|
||||
`~/.pi/agent/extensions/` (`*.ts` and `*.ts.off`) that point into THIS repo
|
||||
and whose target no longer exists. That is how a retired extension leaves a
|
||||
preserved pi-devbox volume: the link outlives the file in every new image.
|
||||
Foreign links and real files are never touched. `warn_retired_mcp_block`
|
||||
(before the prompt, read-only) warns if `settings.json` still has the
|
||||
retired loader's top-level `mcp` key.
|
||||
4. For each selected extension: symlinks `extensions/<name>.ts` →
|
||||
`~/.pi/agent/extensions/<name>.ts`. Backs up any pre-existing real file or
|
||||
foreign symlink.
|
||||
|
||||
@@ -345,97 +351,35 @@ cp /opt/homebrew/Cellar/pi-coding-agent/*/libexec/lib/node_modules/@earendil-wor
|
||||
# review diff, then commit
|
||||
```
|
||||
|
||||
### `mcp-loader.ts`
|
||||
### `mcp-loader.ts` — RETIRED 2026-10-12
|
||||
|
||||
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.
|
||||
Removed in favour of pi's built-in MCP support (`builtin:mcp`, pi ≥ 0.99.0:
|
||||
`~/.pi/agent/mcp.json`, `pi mcp add|list|login`, the `/mcp` manager, OAuth,
|
||||
per-tool `exposure`, resources). Reasons, measured on pi 1.1.0 before removal
|
||||
(README "MCP servers" has the numbers):
|
||||
|
||||
**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, two
|
||||
transports.
|
||||
- **The two could not coexist.** pi treats an extension that registers `/mcp`
|
||||
as a *replacement* for the built-in, so with the loader installed `mcp.json`
|
||||
was never read and every start printed `… registers command /mcp, so
|
||||
built-in extension mcp was not loaded`.
|
||||
- **No cost to switching.** Time-to-ready is the same within noise (0.283 s
|
||||
vs 0.272 s), and with no servers configured the built-in adds zero tokens
|
||||
to a request.
|
||||
- **The fleet had no `mcp` block anywhere** (checked on tor-ms22; all pi-devbox
|
||||
devices share one configuration), so nothing needed migrating.
|
||||
|
||||
**Key design decisions:**
|
||||
What survives elsewhere: `mempalace-toolkit/extensions/pi/mempalace.ts`
|
||||
carries a vendored copy of the loader's `RemoteMcpClient` (streamable-HTTP).
|
||||
That copy is now the only one, so there is no longer a sync partner for its
|
||||
`MCP-STREAMABLE-HTTP-CLIENT-SYNC` token. The design notes below the old
|
||||
loader's header — tool-name sanitisation for Bedrock, fail-soft per server,
|
||||
404 session re-init — live on in git history (`git show a88fa99:extensions/mcp-loader.ts`).
|
||||
|
||||
- **Settings.json shape matches opencode / Claude Desktop verbatim.** A user
|
||||
can copy-paste their `mcp` block from one harness's config to the other.
|
||||
- **Tool names are prefixed with the server name** to avoid collisions when
|
||||
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, 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 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.
|
||||
- **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.
|
||||
Do **not** reintroduce an extension that registers `/mcp`, `codemode` or
|
||||
`tool_search` without meaning to replace the built-in: pi then drops the
|
||||
built-in of the same name, with nothing but one startup warning to show for it.
|
||||
|
||||
**API used:**
|
||||
|
||||
- `pi.registerTool({ name, label, description, parameters, execute })` for
|
||||
each MCP tool exposed by each connected server.
|
||||
- `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 clients:**
|
||||
|
||||
- `class StdioMcpClient` — spawn subprocess, write newline-delimited
|
||||
JSON-RPC, parse newline-delimited responses, match by request `id`.
|
||||
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 extensions:**
|
||||
|
||||
- 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.
|
||||
- Schema sanitization for MCP servers that emit malformed `inputSchema`
|
||||
(some return `{type: "object", properties: {...}}` without `required`).
|
||||
## Testing
|
||||
|
||||
No framework. Manual:
|
||||
|
||||
|
||||
Reference in New Issue
Block a user