Repository navigation
feat(labs): Hook B — labs-trading-data MCP service + openclaw wiring - #137
Conversation
…ws/onchain
Adds the labs-trading-data service deployed via the base-service chart:
- 6-tool MCP surface: bybit_taker_24h / bitget_taker_24h / aggregated_taker_24h
plus cryptopanic_recent_news / etherscan_gas_snapshot /
etherscan_whale_transactions
- Stateful WS workers with rolling 24x1h taker buckets (replicaCount=1
is a hard invariant — buckets live in process memory)
- NetworkPolicy restricts ingress to labs-openclaw pods only
- ExternalSecrets pull free-tier API keys + bearer token from
secret/data/teams/labs/trading-data/{cryptopanic,etherscan,client-token}
Closes the public-API CVD coverage gap left by Phase 1.6 + Phase 2 Hook A
(Binance + OKX inline). Bybit + Bitget have no usable public REST taker
endpoints (see reference_multi_exchange_taker_endpoints.md), so a stateful
WS service is the only path to extend CVD beyond two venues without paying
for CoinGlass STANDARD tier.
Image: ghcr.io/mctlhq/mctl-trading-data:0.1.0 (mctlhq/mctl-trading-data 0.1.0)
This commit is Area B of the Hook B plan; Area C (openclaw consumer wiring)
follows as a separate commit on this branch.
Adds the labs-trading-data MCP service as a stdio-bridge entry in the
openclaw configuration so the agent gets six new tools alongside the
existing mctl + coinglass servers:
- new ExternalSecret labs-trading-data-client pulling TRADING_DATA_TOKEN
from secret/data/teams/labs/trading-data/client-token (server-side
counterpart in services/labs/trading-data/values.yaml shares the same
Vault path; the two stay in sync without manual rotation)
- envFrom -> labs-trading-data-client makes TRADING_DATA_TOKEN available
to /scripts/trading-data-mcp-proxy.js
- new openclaw-scripts entry trading-data-mcp-proxy.js: ~50 LoC stdio
-> HTTP+Bearer bridge. Static token, no OAuth refresh; SSE-aware
response framing in case the FastAPI mount returns text/event-stream
- mcp.servers.trading-data registers the bridge in openclaw.json with
TRADING_DATA_URL pointing at the in-cluster service
Pod restart unavoidable (setup initContainer copies openclaw.json only at
start); `strategy: Recreate` + s3-sync mirror preserves OAuth + skill state.
Transient FailedCreate during Recreate is normal under tight quota -- see
feedback_openclaw_recreate_quota_deadlock memory.
This commit is Area C of the Hook B plan; pairs with the Area B service
manifests in the previous commit on this branch.
|
@codex review |
|
@copilot review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2181bfbfa0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "env": { | ||
| "TRADING_DATA_URL": "http://labs-trading-data.labs.svc.cluster.local:8080/mcp" | ||
| } |
There was a problem hiding this comment.
trading-data is configured with only TRADING_DATA_URL in mcp.servers.trading-data.env, but the proxy script requires both TRADING_DATA_URL and TRADING_DATA_TOKEN and immediately returns an error when the token is missing. In this same file, the setup comment explains that the bundle-mcp path uses mcp.servers.<name>.env verbatim for the spawned process, so pod-level envFrom secrets are not sufficient here; as committed, every call to this MCP server will fail with the proxy-not-configured error instead of reaching labs-trading-data.
Useful? React with 👍 / 👎.
…ptoCompare
CryptoPanic free Developer API plan was discontinued April 2026 (paid
Growth tier starts at USD 50/week). mctl-trading-data 0.2.0 swaps the
news upstream to CryptoCompare News v2 (Apikey header auth, ~100K
calls/month free) and renames the consumer-facing surface so a future
upstream swap stays a one-file change in mctl-trading-data:
- tool: cryptopanic_recent_news -> news_recent
- env: CRYPTOPANIC_API_KEY -> NEWS_API_KEY
- Vault path: secret/teams/labs/trading-data/cryptopanic
-> secret/teams/labs/trading-data/cryptocompare
Operator pre-flight (described in PR #137 body) is now: provision
.../cryptocompare with field api_key (free key from
cryptocompare.com/cryptopian/api-keys), not .../cryptopanic.
|
Re-requesting review after CryptoPanic→CryptoCompare swap (commit @codex review |
|
@copilot review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df358e857c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| return; | ||
| } | ||
| process.stdout.write(text.endsWith('\n') ? text : text + '\n'); |
There was a problem hiding this comment.
The new trading-data-mcp-proxy.js writes the upstream HTTP body straight to stdout for all non-SSE responses, without checking for empty or non-JSON content. If labs-trading-data (or an intermediate proxy) returns an empty body or plain-text/HTML error during a transient failure, the MCP client receives an invalid frame and the original request can stall or fail with protocol parse errors instead of a proper JSON-RPC error. This proxy should validate/parse the body and emit a structured {"jsonrpc":"2.0","id":...,"error":...} reply on invalid payloads, similar to the coinglass bridge behavior.
Useful? React with 👍 / 👎.
…odex P2)
The previous proxy wrote the raw upstream HTTP body to stdout for any
non-SSE response. If labs-trading-data (or an in-cluster proxy) returns
an empty body, plain-text error, or HTML gateway page during a transient
failure, the agent's MCP client receives an invalid frame and the
original tools/call request stalls or fails with a stdio framing
exception instead of a typed JSON-RPC error.
Bridge now:
- validates the response Content-Type
- for SSE: skips data: frames whose payload does not parse as JSON,
and emits an rpcError if zero valid frames came back (status code
+ frame count exposed)
- for non-SSE: rejects empty bodies and non-JSON bodies with a
structured rpcError including status code + content-type + a
short body preview for debugging
- extracts the rpcError emit into a helper for consistent shape
No behaviour change on the happy path (valid JSON / valid SSE frames
still pass through). Same protocol on the wire; only the failure modes
gained a typed error.
Codex P2 on PR #137 (review of df358e8).
|
Addressed P2 in commit @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 871c4d80a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function rpcError(id, message) { | ||
| writeLine({ jsonrpc: '2.0', id: id ?? null, error: { code: -32603, message: String(message) } }); | ||
| } |
There was a problem hiding this comment.
Guard rpcError for requests without an id; otherwise this bridge emits { "id": null, ... } responses for JSON-RPC notifications. This is problematic for standard MCP notification traffic (for example notifications/initialized) when the upstream returns an empty/202 body, because the empty body path will currently write an unsolicited error frame that does not correspond to any client request and can confuse strict MCP clients.
Useful? React with 👍 / 👎.
| for (const part of text.split(/\r?\n/)) { | ||
| const t = part.trim(); | ||
| if (!t.startsWith('data:')) continue; | ||
| const payload = t.slice(5).trim(); | ||
| if (!payload) continue; | ||
| try { JSON.parse(payload); } catch (_) { continue; } | ||
| process.stdout.write(payload + '\n'); |
There was a problem hiding this comment.
data: line
The SSE handler assumes each individual data: line is a complete JSON document, but SSE allows one event payload to be split across multiple data: lines that must be concatenated before parsing. With a compliant multi-line event, each fragment fails JSON.parse, emitted stays zero, and the proxy incorrectly returns no parseable SSE payload instead of forwarding the valid message.
Useful? React with 👍 / 👎.
…ex P2 r2) Two follow-ups on PR #137 from codex round-2 review of 871c4d8: 1. JSON-RPC notifications must not receive responses `rpcError` previously emitted `{ id: null, error: ... }` for any transport failure including those triggered by notification frames (a JSON-RPC request with no `id` member, e.g. `notifications/initialized`). Strict MCP clients tracking ids treat that as an orphan reply and warn or disconnect. `rpcError` now short-circuits when `'id' not in req`, and the empty-body path additionally treats notifications as already-handled (a 202 Accepted with no body is the canonical reply). 2. SSE allows multi-line `data:` blocks per event The previous handler `JSON.parse`'d each `data:` line individually, so any compliant event whose payload was split across multiple `data:` lines had every fragment fail to parse, `emitted` stayed zero, and the proxy returned `no parseable SSE payload` for valid traffic. New `parseSseEvents` splits the body on blank lines (event block separator) and joins all `data:` lines within a block with `\n` before parsing, per WHATWG SSE spec. The single-space-after- colon convention is honoured so `data: {x:1}` and `data:{x:1}` both parse to the same payload. Manual unit harness (`/tmp/test-proxy.js`) covers 11 cases including the regression: notifications, id=null, id=0, single-line, multi-line concat, no-space, multiple events, comment/event lines ignored, empty body, broken JSON dropped. All pass.
|
Addressed both P2s in commit
Verified locally with an 11-case Node harness covering single-line, multi-line concat, no-space variant, multiple events, comment/event lines ignored, empty body, broken JSON dropped, and all four notification/request id shapes. @codex review |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Smoke right after PR #137 merged surfaced HTTP 404 on every tools/list call. FastMCP defaults streamable_http_path="/mcp" inside its sub-app; combined with `api.mount("/mcp", mcp_app)` in trading-data the public path compounded into /mcp/mcp. The intermediate fix (0.2.1, never deployed) set streamable_http_path="/" so the path was /mcp/ — functional but at odds with the rest of the MCP ecosystem (api.mctl.ai/mcp, every public example) where the convention is bare /mcp without a trailing slash. 0.2.2 is the canonical fix: keep FastMCP's default streamable_http_path "/mcp" and mount its sub-app at FastAPI "/" instead of "/mcp". Public URL is now exactly /mcp. FastAPI explicit routes (/healthz, /readyz, /openapi.json, /docs) match before the mount falls through, so there is no route conflict. - bump labs-trading-data image to 0.2.2 (mctl-trading-data c7e8a37) - openclaw bridge URL stays at /mcp (no trailing slash)
Summary
Phase 2 Hook B for labs-trading-intel. Closes the Bybit + Bitget public-API CVD coverage gap left by Phase 1.6 (PR #127, inline indicators) and Hook A (PR #131, Binance + OKX). New stateful MCP service exposes 6 tools: 3 WS-aggregated taker (Bybit/Bitget/aggregated) + 3 REST helpers (CryptoCompare News, Etherscan gas, Etherscan whale tx).
Two-commit layout per the Hook B plan (
~/.claude/plans/golden-percolating-seal.md); a third commit bumps the image to 0.2.0 alongside the news-upstream swap (CryptoPanic free tier was discontinued April 2026 → CryptoCompare News v2).services/labs/trading-data/{values,catalog-info}.yamldeploys the new pod via the base-service chart. NetworkPolicy locks ingress tolabs-openclawonly.replicaCount=1is a hard invariant (in-process WS state).services/labs/openclaw/values.yamladds thelabs-trading-data-clientExternalSecret, thetrading-data-mcp-proxy.jsstdio bridge, and thetrading-dataMCP server registration inopenclaw.json.services/labs/trading-data/values.yamlto image tag0.2.0+ Vault path rename tocryptocompare.Image already published from PR-source repo:
ghcr.io/mctlhq/mctl-trading-data:0.2.0(mctlhq/mctl-trading-data tag 0.2.0).Coinglass tier stays HOBBYIST per operator decision; Coinglass paid tools (heatmap / whale Hyperliquid) are deliberately out of scope.
Pre-merge operator action required
ESO will fail to materialize the new Secrets until Vault is populated. Do NOT merge until the operator has written:
secret/teams/labs/trading-data/cryptocomparefieldapi_key— CryptoCompare News free-tier key (cryptocompare.com/cryptopian/api-keys; ~100K calls/month)secret/teams/labs/trading-data/etherscanfieldapi_key— Etherscan free-tier key (etherscan.io/myapikey)secret/teams/labs/trading-data/client-tokenfieldbearer_token—openssl rand -base64 48Once the gitops PR merges, ArgoCD reconciles in ~3 min, ESO syncs Vault on its 1h refreshInterval (or force-sync), pod becomes Ready, openclaw Recreate restarts and picks up the new MCP server. Expect transient
FailedCreateduring openclaw Recreate perfeedback_openclaw_recreate_quota_deadlock— self-heals within 2-3 min, do not abort.Test plan
@codex reviewclean (or copilot clean — merge gate perfeedback_pr_codex_review)kubectl -n labs get pod -l app.kubernetes.io/instance=labs-trading-dataReady 1/1kubectl -n labs exec deploy/labs-openclaw -- curl -sX POST -H "Authorization: Bearer $TRADING_DATA_TOKEN" -H 'Content-Type: application/json' -d '{"jsonrpc":"2.0","id":1,"method":"tools/list"}' http://labs-trading-data:8080/mcpreturns 6 tools (bybit_taker_24h,bitget_taker_24h,aggregated_taker_24h,news_recent,etherscan_gas_snapshot,etherscan_whale_transactions)bybit_taker_24h ETHUSDTreturnsmeta.window_incomplete=truefor first hourmctl_save_openclaw_skill labs eth-trading-intel):/eth ETHTelegram smoke showscvd_24h.per_exchange.{binance,okx,bybit,bitget}with N-leg agreement cascade and a populatednews_scorefrom CryptoCompare