Repository navigation
feat(core): add MCP transport over incoming client sockets - #57502
Conversation
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks sound. I ran the new tests in packages/core (11 pass, stable across 10 runs, including the real SDK Client ↔ McpServer round trip), and bun run typecheck passes. I also traced the ready/open/closed transitions: onclose fires once whether the socket closes via close() or ended(), frames buffered before start are dropped if the socket ends first, and send is rejected outside the open state. I have no concerns that would block this.
Public API is unchanged: this is a new internal module in packages/core that nothing calls yet.
There was a problem hiding this comment.
The new commit looks sound. close() now rejects when the socket's close throws, and onclose still fires once. Frames delivered while start() is flushing the buffer now keep their order, and parsing uses Result.try instead of running an Effect. All 15 tests in test/mcp-socket-client.test.ts pass, and bun run typecheck in packages/core passes. I have no new concerns.
Public API is unchanged: this is an internal module in packages/core.
9e8f20c to
3d7233f
Compare
There was a problem hiding this comment.
The socket transport is unchanged since my last review. This push folds in #57503 (Mcp.attach, the attached McpClient.connect type, and routing tool calls to attached clients), so this PR now covers stack steps 1 and 2. In packages/core, test/mcp-socket-client.test.ts and test/mcp-attach.test.ts pass (28 tests; the attach tests passed six repeated runs), and bun run typecheck passes.
Public API is unchanged: Mcp.Interface.attach is internal to core, and the attached connection type is never read from config.
Concerns:
- An attached client whose name matches a configured server takes over that server's tools and permission rules (see the inline comment).
test/mcp-attach.test.tsis 402 lines for 13 cases and relies on polling helpers (waitUntil,registered). Please cut it down to the core cases: routing with the session ID, removal on close, fallback to the previous connection, and staying hidden from server status. That would make it easier to follow and less sensitive to slow CI.
| callTool: Effect.fn("MCP.callTool")(function* (input) { | ||
| const target = yield* requireServer(ServerName.make(input.server)) | ||
| const server = ServerName.make(input.server) | ||
| // An attached client owns the tools it lists, even when a configured server shares its name, and |
There was a problem hiding this comment.
When an attached client uses the same name as a configured MCP server, its tools take over that server's tools of the same name, both here and in tools(). They also pick up that server's permission rules, because McpTool builds the permission action from the server name (<server>_<tool>). A rule like github_*: allow would then also allow tools served by a client that attached as github. Could attach reject a name that matches a configured server, or put attached names in their own namespace? Then attached tools could never override configured ones or inherit their permissions.
Stack 1/5, based on
dev. Merge in order.Part of the client MCP stack (experimental): a client (TUI/desktop/web) opens a WebSocket to the server, and the server treats that incoming socket as an internal MCP server scoped to a location. Tool calls carry
_meta["ai.opencode/sessionID"]so the client acts on the calling session. MCP has no official WebSocket transport, so this is a custom transport, which the spec allows.This PR
Adds
McpSocketClient(packages/core/src/mcp/socket-client.ts): an MCP SDKTransportover an already-accepted client WebSocket. The server never dials out.onerrorand the connection stays openstart()are buffereddeliver(text)andended()hooks for the socket loop;close()closes with 1000;onclosefires onceTests: unit tests over a fake socket, plus a real SDK
Client↔McpServerround trip through two transports.Stack
→ 1. #57502 feat(core): add MCP transport over incoming client sockets
2. #57503 feat(core): attach client-hosted MCP servers
3. #57504 feat(client): add connectClientMcp helper for client-hosted MCP tools
4. #57505 feat(server): add client MCP websocket endpoint (experimental)
5. #57506 feat(tui): expose open_terminal over client MCP (experimental)