Repository navigation
fix(labs): clean /mcp endpoint — image 0.2.2 - #139
Conversation
|
@codex review |
|
@copilot review |
|
To use Codex here, create an environment for this repo. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ 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)
919de4b to
00a05f8
Compare
mashkovd
changed the title
|
Force-pushed simpler diff: 0.2.1 (which left path at @codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ 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". |
Summary
Smoke right after PR #137 merged surfaced HTTP 404 on every
tools/listcall. Root cause: FastMCP defaultsstreamable_http_path="/mcp"inside its own sub-app, which compounded withapi.mount("/mcp", mcp_app)in mctl-trading-data into the surprise public path/mcp/mcp.Canonical fix lives in mctl-trading-data 0.2.2 (commit c7e8a37, image already published): mount the FastMCP sub-app at FastAPI
"/"and keep FastMCP's defaultstreamable_http_path="/mcp". Public path is now exactly/mcp— no trailing slash, no/mcp/mcp, matches every other MCP endpoint in the ecosystem (api.mctl.ai/mcp, every public example).Net effect on this gitops change: a single one-line bump of the labs-trading-data image tag from
0.2.0to0.2.2. The labs-openclaw bridge URL is already correct at/mcp(no trailing slash) — no change needed there.tests/test_app_routes.pyin mctl-trading-data locks the regression — it asserts/mcpis in routes and explicitly forbids both/mcp/and/mcp/mcp.Test plan
kubectl -n labs get pod -l app.kubernetes.io/instance=labs-trading-dataReady 1/1 (rolling update from 0.2.0)kubectl -n labs exec deploy/labs-openclaw-base-service -- curl -sf http://labs-trading-data-base-service.labs.svc.cluster.local:8080/healthz→ 200kubectl -n labs port-forward svc/labs-trading-data-base-service 18080:8080then full MCP handshake againsthttp://127.0.0.1:18080/mcpreturns the 6 expected tools@codex reviewclean (or copilot clean — merge gate perfeedback_pr_codex_review)