🔌 Add MCP server wrapper for advanced-swap-orders#57
🔌 Add MCP server wrapper for advanced-swap-orders#57RanSHammer wants to merge 4 commits intoorbs-network:masterfrom
Conversation
Adds a FastMCP-based MCP server that wraps the existing order.js skill as standard MCP tools, enabling registration in MCP directories (Official Registry, Smithery, LobeHub, etc.) Tools: - prepare_order: prepare gasless swap orders with EIP-712 typed data - submit_order: submit signed orders to relay network - query_orders: query order status by swapper or hash - get_supported_chains: return chain matrix from manifest.json - get_token_addressbook: return common token addresses per chain Supports stdio and HTTP transport via FastMCP. Zero changes to underlying order.js or contracts. Related: Orbs Agentic GTM V2 - MCP directory distribution strategy
|
few issues with this approach
|
There was a problem hiding this comment.
Pull request overview
Adds a FastMCP-based MCP server wrapper around the existing scripts/order.js skill so it can be registered and used as standard MCP tools (stdio or HTTP transport).
Changes:
- Added
mcp-server.pyimplementing MCP tools (prepare_order,submit_order,query_orders, etc.) backed by subprocess calls tonode scripts/order.js. - Added
start-mcp.shlauncher supporting stdio (default) and HTTP mode with a port argument. - Added
MCP-README.mdwith setup, client configuration examples, and tool/workflow docs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| skills/advanced-swap-orders/start-mcp.sh | Entrypoint script to start the MCP server with stdio/HTTP transport options. |
| skills/advanced-swap-orders/mcp-server.py | FastMCP server exposing the existing order CLI as MCP tools and loading manifest/addressbook assets. |
| skills/advanced-swap-orders/MCP-README.md | Documentation for running and configuring the MCP server and available tools. |
| # MCP Server — Orbs Advanced Swap Orders | ||
|
|
||
| Model Context Protocol (MCP) server exposing gasless, oracle-protected swap orders across 10 EVM chains. | ||
|
|
There was a problem hiding this comment.
This documentation claims the server supports “10 EVM chains”, but the skill’s manifest currently lists 8 chains (1, 56, 137, 146, 8453, 42161, 43114, 59144) and get_supported_chains() will return only those. Please update the README to match the manifest/server behavior, or update the manifest + server to actually expose the additional chains mentioned.
|
|
||
| ## Supported Chains | ||
|
|
||
| Ethereum (1), BNB Chain (56), Polygon (137), Sonic (146), Base (8453), Arbitrum One (42161), Avalanche (43114), Linea (59144) — plus Optimism (10) and Mantle (5000) via runtime config. |
There was a problem hiding this comment.
The “Supported Chains” line mentions Optimism (10) and Mantle (5000) “via runtime config”, but the MCP server reads chains only from manifest.json at startup and will not surface those unless they are present there. Either remove these chains from the list or make the server load/merge whatever runtime config source adds them.
| Ethereum (1), BNB Chain (56), Polygon (137), Sonic (146), Base (8453), Arbitrum One (42161), Avalanche (43114), Linea (59144) — plus Optimism (10) and Mantle (5000) via runtime config. | |
| Ethereum (1), BNB Chain (56), Polygon (137), Sonic (146), Base (8453), Arbitrum One (42161), Avalanche (43114), Linea (59144). |
|
|
||
| Non-custodial, decentralized, gasless swap orders with oracle-protected execution. | ||
| Supports market, limit, stop-loss, take-profit, delayed-start, and chunked/TWAP-style | ||
| orders across 10 EVM chains via ownerless, immutable, audited, and verified contracts. |
There was a problem hiding this comment.
The module docstring says the server supports “orders across 10 EVM chains”, but CHAIN_LIST is derived from manifest.json which currently contains 8 chains. Consider deriving this description from the manifest (e.g., use len(CHAINS_INFO)) or updating the wording so it can’t go stale.
| orders across 10 EVM chains via ownerless, immutable, audited, and verified contracts. | |
| orders across supported EVM chains via ownerless, immutable, audited, and verified contracts. |
| transport = args[i + 1] | ||
| i += 2 | ||
| elif args[i] == "--port" and i + 1 < len(args): | ||
| port = int(args[i + 1]) |
There was a problem hiding this comment.
--port parsing will raise an uncaught ValueError if a non-integer is provided (e.g. --port foo), which will crash the server without a clear message. Consider validating/constraining the port (and range 1-65535) and exiting with a user-friendly error payload.
| port = int(args[i + 1]) | |
| port_str = args[i + 1] | |
| try: | |
| port = int(port_str) | |
| except ValueError: | |
| print( | |
| json.dumps( | |
| { | |
| "error": f"Invalid port value {port_str!r}: must be an integer between 1 and 65535" | |
| } | |
| ), | |
| file=sys.stderr, | |
| ) | |
| sys.exit(1) | |
| if not (1 <= port <= 65535): | |
| print( | |
| json.dumps( | |
| { | |
| "error": f"Invalid port {port}: must be between 1 and 65535" | |
| } | |
| ), | |
| file=sys.stderr, | |
| ) | |
| sys.exit(1) |
| TRANSPORT="${1:-stdio}" | ||
| PORT="${2:-8000}" | ||
|
|
||
| exec python3 mcp-server.py --transport "$TRANSPORT" --port "$PORT" |
There was a problem hiding this comment.
start-mcp.sh always passes --port even when using stdio transport (where it’s ignored). It’d be clearer to only include --port when TRANSPORT is http, which also avoids accidentally encouraging callers to rely on an unused flag in stdio mode.
| exec python3 mcp-server.py --transport "$TRANSPORT" --port "$PORT" | |
| if [ "$TRANSPORT" = "http" ]; then | |
| exec python3 mcp-server.py --transport "$TRANSPORT" --port "$PORT" | |
| else | |
| exec python3 mcp-server.py --transport "$TRANSPORT" | |
| fi |
- Single file mcp-server.js with zero npm dependencies - Implements MCP stdio protocol (JSON-RPC 2.0) inline - Reads chains, contracts, config from manifest.json at startup - Reads token addressbook from assets/token-addressbook.md - Tool descriptions dynamically include supported chain list - Removed Python mcp-server.py and fastmcp dependency
|
Addressed all 3 points: 1. No external dependencies — Removed 2. Node.js, not Python — Entire server is now a single 3. No hardcoded values — Everything is read dynamically at startup:
Tested: initialize → tools/list → tools/call all working correctly over stdio. |
| const { jsonrpc, id, method, params } = msg; | ||
|
|
||
| // Notifications (no id) — acknowledge silently | ||
| if (id === undefined || id === null) { |
There was a problem hiding this comment.
JSON-RPC notifications are defined as requests without an id member. This handler treats id: null as a notification and returns no response, which is not JSON-RPC 2.0 compliant and can cause MCP clients to hang if they ever send id: null requests. Treat only id === undefined as a notification; if id is present (even null), return a response (or an Invalid Request error).
| if (id === undefined || id === null) { | |
| if (id === undefined) { |
| return new Promise((resolve) => { | ||
| const child = execFile('node', [ORDER_JS, ...args], { | ||
| cwd: SKILL_DIR, | ||
| timeout: 30000, | ||
| maxBuffer: 1024 * 1024, | ||
| }, (error, stdout, stderr) => { | ||
| if (error) { | ||
| const msg = stderr?.trim() || stdout?.trim() || error.message || 'Unknown error'; | ||
| resolve(JSON.stringify({ error: msg, exitCode: error.code })); | ||
| } else { | ||
| resolve(stdout.trim()); | ||
| } | ||
| }); |
There was a problem hiding this comment.
runOrderJs resolves an error payload as a JSON string when the subprocess fails, but the MCP tools/call response is still returned as a successful result (isError not set). This makes tool failures indistinguishable from success for MCP clients. Consider having runOrderJs reject/throw on non-zero/timeout and then set result.isError = true (or return a structured result that includes an error flag).
| return new Promise((resolve) => { | |
| const child = execFile('node', [ORDER_JS, ...args], { | |
| cwd: SKILL_DIR, | |
| timeout: 30000, | |
| maxBuffer: 1024 * 1024, | |
| }, (error, stdout, stderr) => { | |
| if (error) { | |
| const msg = stderr?.trim() || stdout?.trim() || error.message || 'Unknown error'; | |
| resolve(JSON.stringify({ error: msg, exitCode: error.code })); | |
| } else { | |
| resolve(stdout.trim()); | |
| } | |
| }); | |
| return new Promise((resolve, reject) => { | |
| const child = execFile( | |
| 'node', | |
| [ORDER_JS, ...args], | |
| { | |
| cwd: SKILL_DIR, | |
| timeout: 30000, | |
| maxBuffer: 1024 * 1024, | |
| }, | |
| (error, stdout, stderr) => { | |
| if (error) { | |
| const msg = (stderr && stderr.trim()) || (stdout && stdout.trim()) || error.message || 'Unknown error'; | |
| const payload = { | |
| error: msg, | |
| exitCode: typeof error.code === 'number' ? error.code : null, | |
| }; | |
| const err = new Error(msg); | |
| err.payload = payload; | |
| reject(err); | |
| } else { | |
| resolve(stdout.trim()); | |
| } | |
| } | |
| ); |
| /** | ||
| * Orbs Advanced Swap Orders — MCP Server (Node.js, zero dependencies) | ||
| * | ||
| * Implements the Model Context Protocol over stdio transport using | ||
| * JSON-RPC 2.0. No external dependencies — reads all config from | ||
| * manifest.json and assets/ at startup. | ||
| * | ||
| * Usage: | ||
| * node mcp-server.js | ||
| * npx @orbs-network/spot mcp | ||
| */ |
There was a problem hiding this comment.
PR description says the MCP server is built with FastMCP (Python) and supports both stdio and HTTP transport, but this change introduces a Node.js stdio-only implementation (mcp-server.js) and start-mcp.sh only launches stdio. Please update the PR description and/or docs to match the actual implementation, or add the promised FastMCP/HTTP transport.
|
|
||
| async query_orders({ swapper, order_hash }) { | ||
| if (!swapper && !order_hash) { | ||
| return [{ type: 'text', text: JSON.stringify({ error: 'Provide either swapper address or order_hash' }) }]; |
There was a problem hiding this comment.
query_orders returns an error as plain text content but does not set isError: true. For MCP clients, this will look like a successful call. Prefer throwing (so the catch sets isError) or explicitly returning { content, isError: true } for missing-argument validation failures.
| return [{ type: 'text', text: JSON.stringify({ error: 'Provide either swapper address or order_hash' }) }]; | |
| return { | |
| content: [ | |
| { | |
| type: 'text', | |
| text: JSON.stringify({ error: 'Provide either swapper address or order_hash' }), | |
| }, | |
| ], | |
| isError: true, | |
| }; |
| }, | ||
|
|
||
| async get_supported_chains() { | ||
| return [{ type: 'text', text: JSON.stringify(chains, null, 2) }]; |
There was a problem hiding this comment.
The tool schema defines params.chainId as a string, but order.js accepts chainId as either decimal string or 0x integer, and your schema enumerates stringified decimal IDs. This is fine, but returning get_supported_chains as a raw object keyed by string chain IDs can be inconsistent for clients expecting an array. Consider returning a normalized array of { chainId, name, adapter } with chainId as a string to match the schema and avoid key-order dependence.
| return [{ type: 'text', text: JSON.stringify(chains, null, 2) }]; | |
| const normalizedChains = Array.isArray(chains) | |
| ? chains | |
| .map((c) => { | |
| if (!c || typeof c !== 'object') return null; | |
| const rawId = c.chainId ?? c.id ?? c.chain_id; | |
| if (rawId == null) return null; | |
| return { | |
| chainId: String(rawId), | |
| name: c.name, | |
| adapter: c.adapter, | |
| }; | |
| }) | |
| .filter(Boolean) | |
| : Object.entries(chains).map(([chainId, info]) => ({ | |
| chainId: String(chainId), | |
| name: info && typeof info === 'object' ? info.name : undefined, | |
| adapter: info && typeof info === 'object' ? info.adapter : undefined, | |
| })); | |
| return [{ type: 'text', text: JSON.stringify(normalizedChains, null, 2) }]; |
| const SERVER_CAPABILITIES = { | ||
| tools: {}, | ||
| }; |
There was a problem hiding this comment.
SERVER_CAPABILITIES.tools is returned as an empty object during initialize, but the server actually supports tools. Some MCP clients rely on accurate capability advertisement. Consider populating this (or omit tools entirely if the spec expects a boolean/shape), so capabilities match tools/list.
| const SERVER_CAPABILITIES = { | |
| tools: {}, | |
| }; | |
| const SERVER_CAPABILITIES = {}; |
| Model Context Protocol (MCP) server exposing gasless, oracle-protected swap orders across multiple EVM chains. | ||
|
|
||
| **Zero external dependencies** — implements the MCP stdio protocol (JSON-RPC 2.0 over stdin/stdout) directly in Node.js. All configuration is read dynamically from `manifest.json` and `assets/`. | ||
|
|
There was a problem hiding this comment.
PR description lists an added mcp-server.py (FastMCP) and HTTP transport support, but the repo changes add mcp-server.js and start-mcp.sh only. Please align the documentation in this README with the actual deliverable (and/or add the missing files/transports) so users aren’t misled when following the PR summary.
| # node mcp-server.js # direct invocation | ||
|
|
||
| set -euo pipefail | ||
| cd "$(dirname "$0")" |
There was a problem hiding this comment.
This launch script claims it starts the MCP server, but it doesn’t set executable permissions (in git) and cd may fail if the script is invoked via symlink. Consider using cd -- "$(dirname -- "${BASH_SOURCE[0]}")" for robustness, and ensure the repo sets the executable bit for this script.
| cd "$(dirname "$0")" | |
| cd -- "$(dirname -- "${BASH_SOURCE[0]}")" |
…stale Python server
|
Addressed all Copilot review comments in 3909f20: Bug fixes in
Script/docs fixes: Notes:
|
| process.stdin.setEncoding('utf8'); | ||
| process.stdin.on('data', async (chunk) => { | ||
| buffer += chunk; | ||
|
|
||
| // MCP uses newline-delimited JSON | ||
| let newlineIdx; | ||
| while ((newlineIdx = buffer.indexOf('\n')) !== -1) { | ||
| const line = buffer.slice(0, newlineIdx).trim(); | ||
| buffer = buffer.slice(newlineIdx + 1); | ||
|
|
||
| if (!line) continue; | ||
|
|
||
| let msg; | ||
| try { | ||
| msg = JSON.parse(line); | ||
| } catch { | ||
| const errResp = { | ||
| jsonrpc: '2.0', | ||
| id: null, | ||
| error: { code: -32700, message: 'Parse error' }, | ||
| }; | ||
| process.stdout.write(JSON.stringify(errResp) + '\n'); | ||
| continue; | ||
| } | ||
|
|
||
| const response = await handleMessage(msg); | ||
| if (response) { | ||
| process.stdout.write(JSON.stringify(response) + '\n'); | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
The stdio data handler is async and awaits handleMessage inside the while-loop. If a request triggers a long-running tool call (subprocess up to 30s), additional data events can fire and re-enter this handler, mutating the shared buffer and potentially interleaving/reordering responses. Consider implementing a single-flight read/parse loop (queue parsed messages and process them sequentially), or pause stdin while processing and resume afterward.
| async function handleMessage(msg) { | ||
| const { jsonrpc, id, method, params } = msg; | ||
|
|
||
| // Notifications (no id field) — acknowledge silently per JSON-RPC 2.0 spec | ||
| // Note: id === null is a valid request id; only missing id (undefined) is a notification | ||
| if (id === undefined) { | ||
| return null; // no response for notifications | ||
| } |
There was a problem hiding this comment.
handleMessage destructures msg assuming it's an object. If the client sends valid JSON that isn't an object (e.g., null), this throws a TypeError. Add a guard (msg && typeof msg === 'object') and reply with JSON-RPC -32600 Invalid Request instead of throwing.
| let msg; | ||
| try { | ||
| msg = JSON.parse(line); | ||
| } catch { | ||
| const errResp = { | ||
| jsonrpc: '2.0', | ||
| id: null, | ||
| error: { code: -32700, message: 'Parse error' }, | ||
| }; | ||
| process.stdout.write(JSON.stringify(errResp) + '\n'); | ||
| continue; | ||
| } | ||
|
|
||
| const response = await handleMessage(msg); | ||
| if (response) { | ||
| process.stdout.write(JSON.stringify(response) + '\n'); | ||
| } | ||
| } |
There was a problem hiding this comment.
await handleMessage(msg) inside the stdin loop isn't wrapped in try/catch. Any unexpected exception in handleMessage (or a handler) will reject the async data callback, which can surface as an unhandled promise rejection and stop processing further input. Wrap the call in try/catch and emit a JSON-RPC -32603 Internal error response (with the same id when available).
| // Prevent unhandled errors from crashing | ||
| process.on('uncaughtException', (err) => { | ||
| process.stderr.write(`MCP server error: ${err.message}\n`); | ||
| }); |
There was a problem hiding this comment.
The uncaughtException handler logs the error but keeps the process running. In Node.js, continuing after an uncaught exception is generally unsafe because state may be corrupted. Prefer logging and then exiting (or at least setting a non-zero exit code), and consider also handling unhandledRejection similarly so the server fails fast and can be restarted cleanly by the MCP client.
| // Prevent unhandled errors from crashing | |
| process.on('uncaughtException', (err) => { | |
| process.stderr.write(`MCP server error: ${err.message}\n`); | |
| }); | |
| // Fail fast on unhandled errors so MCP client can restart cleanly | |
| process.on('uncaughtException', (err) => { | |
| const message = err && err.stack ? err.stack : String(err && err.message ? err.message : err); | |
| process.stderr.write(`MCP server uncaughtException: ${message}\n`); | |
| process.exit(1); | |
| }); | |
| process.on('unhandledRejection', (reason) => { | |
| const message = | |
| reason instanceof Error | |
| ? (reason.stack || reason.message) | |
| : String(reason); | |
| process.stderr.write(`MCP server unhandledRejection: ${message}\n`); | |
| process.exit(1); | |
| }); |
|
Fixed all 4 new Copilot comments:
|
| | Tool | Description | | ||
| |------|-------------| | ||
| | `prepare_order` | Prepare a gasless swap order (market, limit, stop-loss, take-profit, TWAP, delayed-start). Returns EIP-712 typed data for signing. | | ||
| | `submit_order` | Submit a signed order to the Orbs relay network for oracle-protected execution. | | ||
| | `query_orders` | Query order status by swapper address or order hash. | | ||
| | `get_supported_chains` | List all supported chains with IDs, names, and adapter addresses (from manifest.json). | | ||
| | `get_token_addressbook` | Common token addresses (WETH, USDC, USDT, etc.) for every supported chain. | |
There was a problem hiding this comment.
The markdown table under “Tools” uses double leading ||, which breaks table rendering in many markdown parsers. Use a standard table header row like | Tool | Description | (single pipe at start/end).
| ### Via npx (if published) | ||
|
|
||
| ```json | ||
| { | ||
| "mcpServers": { | ||
| "orbs-swap": { | ||
| "command": "npx", | ||
| "args": ["@orbs-network/spot", "mcp"] | ||
| } | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The “Via npx” config snippet implies npx @orbs-network/spot mcp works, but the current package.json in this repo does not define a bin/CLI entrypoint, so this configuration will fail. Either add the npm bin/CLI wiring in this PR or change the docs to only show the supported node .../mcp-server.js launch method.
| const tokenAddressbook = fs.readFileSync(TOKEN_ADDRESSBOOK_PATH, 'utf8'); | ||
|
|
||
| const chains = manifest.runtime?.chains ?? {}; | ||
| const contracts = manifest.runtime?.contracts ?? {}; |
There was a problem hiding this comment.
contracts is read from the manifest but never used, which makes the startup config look incomplete. Remove the unused variable or use it (e.g., include contract addresses in get_supported_chains output if intended).
| const contracts = manifest.runtime?.contracts ?? {}; |
| // Type guard: msg must be a non-null object | ||
| if (typeof msg !== 'object' || msg === null) { | ||
| return { | ||
| jsonrpc: '2.0', | ||
| id: null, | ||
| error: { code: -32600, message: 'Invalid Request: expected a JSON object' }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The JSON-RPC type guard accepts arrays (because typeof [] === 'object'), and a batch request would then be treated as a notification (id is undefined) and silently ignored. Either explicitly reject arrays with -32600 or implement JSON-RPC batch handling so clients can't accidentally get no response.
|
@copilot |
| async function handleMessage(msg) { | ||
| // Type guard: msg must be a non-null object | ||
| if (typeof msg !== 'object' || msg === null) { | ||
| return { | ||
| jsonrpc: '2.0', | ||
| id: null, | ||
| error: { code: -32600, message: 'Invalid Request: expected a JSON object' }, | ||
| }; | ||
| } | ||
|
|
||
| const { jsonrpc, id, method, params } = msg; | ||
|
|
||
| // Notifications (no id field) — acknowledge silently per JSON-RPC 2.0 spec | ||
| // Note: id === null is a valid request id; only missing id (undefined) is a notification | ||
| if (id === undefined) { | ||
| return null; // no response for notifications | ||
| } | ||
|
|
||
| switch (method) { | ||
| case 'initialize': |
There was a problem hiding this comment.
handleMessage treats any non-null object as a valid JSON-RPC request (including arrays/batch requests), and doesn’t validate jsonrpc === "2.0" or that method is a string. This can produce non-spec errors (e.g., “Method not found” for invalid requests) and break interoperability with some MCP/JSON-RPC clients. Consider explicitly rejecting arrays, validating jsonrpc/method/id types, and returning -32600 Invalid Request when the request shape is invalid.
|
Refactor the implementation to use the official @modelcontextprotocol/sdk in JavaScript. Technical Requirements: Architecture: Initialize an McpServer using the StdioServerTransport. Source of Truth: Consolidate all tool definitions, input schemas, and execution logic directly within the skill file. Avoid external manifests or configuration dependencies for tool metadata. Minimalism: Remove all legacy boilerplate, redundant helpers, and non-essential code. Use the leanest possible implementation of the MCP server pattern. Final Pass: Once the functional implementation is complete, perform a mandatory "trim and simplify" pass to refactor for maximum clarity and efficiency. |
Summary
Adds a Node.js MCP server that wraps the existing
order.jsskill as standard MCP tools, enabling registration in MCP directories.Why
MCP (Model Context Protocol) is now the dominant standard for AI agent tool integration — supported by Anthropic, OpenAI, Google, and Microsoft. To get listed in directories where agents discover tools (Official MCP Registry, Smithery, LobeHub, etc.), we need a proper MCP server with standard tool definitions.
Tools Exposed
prepare_ordersubmit_orderquery_ordersget_supported_chainsget_token_addressbookTechnical Details
mcp-server.jswith no build step or transpilationnode scripts/order.js— zero changes to underlying skill or contractsFiles Added
mcp-server.js— MCP server implementation (Node.js, zero deps)start-mcp.sh— Launch script (stdio transport)MCP-README.md— Usage documentationNext Steps (after merge)
Submit to MCP directories:
Related
Orbs Agentic GTM V2 — MCP directory distribution strategy