Migrate LumiBot MCP runtime to protocol v2 - #1147
Conversation
📝 WalkthroughWalkthroughThe MCP integration now uses the official Python MCP v2 client and MCP 2026-07-28 transport contract. It negotiates with ChangesMCP v2 client migration
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant AgentRuntime
participant MCPClient
participant HTTPTransport
participant MCPServer
AgentRuntime->>MCPClient: Create client with automatic mode and metadata
MCPClient->>MCPServer: Negotiate with server/discover
MCPClient->>HTTPTransport: Send stateless POST request
HTTPTransport->>MCPServer: Forward JSON-RPC with protocol headers and _meta
MCPServer-->>HTTPTransport: Return tool result
HTTPTransport-->>MCPClient: Return response
MCPClient-->>AgentRuntime: Return serialized tool data
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_agent_runtime_remote_mcp.py`:
- Around line 74-81: Update the test fixture’s request handler around do_POST
and the corresponding GET/DELETE handlers to record every MCP request, including
method and headers/body as applicable. Add assertions in the affected tests that
all recorded MCP requests use POST, so standalone GET streams and DELETE session
cleanup are explicitly rejected.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b94a2e0f-2c58-4d26-a862-6b6ec835b01f
📒 Files selected for processing (9)
CHANGELOG.mddocs/AI_TRADING_AGENTS.mddocs/MCP_2026_TRANSPORT.mddocsrc/botspot_mcp.rstlumibot/components/agents/runtime.pyrequirements.txtsetup.pytests/test_agent_runtime_mcp_transports.pytests/test_agent_runtime_remote_mcp.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (20)
**/*.{py,md,rst,txt,yml,yaml,json,ini,env}
📄 CodeRabbit inference engine (CLAUDE.md)
Never fabricate, synthesize, forward-fill, interpolate, or default-fill missing market data in backtests; return empty / explicit absence instead, and remove any code that returns fake bars as real data.
Files:
setup.pyrequirements.txtdocs/MCP_2026_TRANSPORT.mdCHANGELOG.mddocsrc/botspot_mcp.rsttests/test_agent_runtime_remote_mcp.pydocs/AI_TRADING_AGENTS.mdtests/test_agent_runtime_mcp_transports.pylumibot/components/agents/runtime.py
**/*.{py,md,rst}
📄 CodeRabbit inference engine (CLAUDE.md)
If an environment variable is introduced or changed, make sure both engineering and public documentation stay synchronized with the new behavior.
Files:
setup.pydocs/MCP_2026_TRANSPORT.mdCHANGELOG.mddocsrc/botspot_mcp.rsttests/test_agent_runtime_remote_mcp.pydocs/AI_TRADING_AGENTS.mdtests/test_agent_runtime_mcp_transports.pylumibot/components/agents/runtime.py
setup.py
📄 CodeRabbit inference engine (CLAUDE.md)
Treat the
version=value insetup.pyas the authoritative project version source of truth.Treat the version= value in setup.py as the authoritative package version source.
Files:
setup.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Never commit private, account-specific, machine-specific, or credential-related information to tracked repository files; use placeholders and public environment-variable names instead. Rotate exposed credentials immediately.
Keep sensitive BotSpot/Lumiwealth operational credentials and private references in private repositories or secret stores, not the public LumiBot checkout.
Normal work must remain in the canonical checkout on the latest active version/X.Y.Z branch; do not create sibling worktrees, switch branches, or create additional branches without explicit authorization.
Do not create, manage, merge, close, or take over pull requests, publish releases, tag, deploy, update package versions, or modify downstream pins without explicit authorization.
Never push directly to dev; authorized release captains must use the approved version-branch release workflow.
Never run git checkout or destructive operations such as git reset --hard, git clean -f, or git stash; inspect dirty files and preserve other agents' changes.
Commit coherent changes in small logical chunks, review all diffs before committing, and coordinate overlapping tests, baselines, and CI workflow changes through handoffs.
Never delete shared caches; use versioned S3 namespaces for cold-cache simulations and delete cache objects only when explicitly requested and tightly scoped.
If a new environment variable is genuinely required for a user-facing feature, document it in docsrc/environment_variables.rst and relevant engineering notes.
Never launch ThetaTerminal locally with production credentials; local ThetaData credentials must remain untracked, use disposable $TMPDIR files when necessary, and never be used for backtests.
Wrap long pytest, backtest, and stress commands with bin/safe-timeout 1200s or another timeout wrapper, splitting work when practical.
Capture reproducible demonstration artifacts such as strategy/library logs, tear sheets, and downloader stress JSON files when showing fixe...
Files:
setup.pyrequirements.txtdocs/MCP_2026_TRANSPORT.mdCHANGELOG.mddocsrc/botspot_mcp.rsttests/test_agent_runtime_remote_mcp.pydocs/AI_TRADING_AGENTS.mdtests/test_agent_runtime_mcp_transports.pylumibot/components/agents/runtime.py
**/*.{py,yml,yaml,json,md,rst}
📄 CodeRabbit inference engine (AGENTS.md)
Document and test behavioral changes in the same change set, including regression tests and comments for non-obvious invariants.
Files:
setup.pydocs/MCP_2026_TRANSPORT.mdCHANGELOG.mddocsrc/botspot_mcp.rsttests/test_agent_runtime_remote_mcp.pydocs/AI_TRADING_AGENTS.mdtests/test_agent_runtime_mcp_transports.pylumibot/components/agents/runtime.py
**/*.{py,md,rst,log}
📄 CodeRabbit inference engine (AGENTS.md)
Never hardcode private downloader URLs; use localhost or placeholders and refer to DATADOWNLOADER_BASE_URL.
Files:
setup.pydocs/MCP_2026_TRANSPORT.mdCHANGELOG.mddocsrc/botspot_mcp.rsttests/test_agent_runtime_remote_mcp.pydocs/AI_TRADING_AGENTS.mdtests/test_agent_runtime_mcp_transports.pylumibot/components/agents/runtime.py
**
⚙️ CodeRabbit configuration file
**: Review every pull request as if LumiBot is a public open-source trading framework.
Prioritize real bugs, security/privacy issues, user-facing regressions, and release risk.Always check changed lines for:
- hardcoded credentials, API keys, tokens, account emails, private URLs, private hostnames, local credential paths, or personal filesystem paths;
- accidental leakage of BotSpot, Lumiwealth, customer, broker, paid-vendor, CI, or maintainer-only operational details into public code/docs/tests;
- code that logs, prints, persists, screenshots, or commits secrets or customer/broker data;
- test fixtures or docs that look fake but could be copied into real usage as credentials or private endpoints;
- changes that weaken authentication, authorization, data-source safety, broker/order safety, or CI/release gates.
If a finding depends on repository context, explain the specific source file and invariant instead of giving generic advice.
Files:
setup.pyrequirements.txtdocs/MCP_2026_TRANSPORT.mdCHANGELOG.mddocsrc/botspot_mcp.rsttests/test_agent_runtime_remote_mcp.pydocs/AI_TRADING_AGENTS.mdtests/test_agent_runtime_mcp_transports.pylumibot/components/agents/runtime.py
**/*.{png,jpg,jpeg,webp,gif,svg,md,rst}
📄 CodeRabbit inference engine (CLAUDE.md)
For any generated or AI-edited image or documentation visual, use Nano Banana MCP only; do not use fallback image generators, Mermaid screenshots, or manual diagram pipelines, and visually inspect every output before committing.
Files:
docs/MCP_2026_TRANSPORT.mdCHANGELOG.mddocsrc/botspot_mcp.rstdocs/AI_TRADING_AGENTS.md
docs/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When a workflow changes (including environment variables, cache semantics, or harness flags), update the relevant
docs/*page in the same change set.
Files:
docs/MCP_2026_TRANSPORT.mddocs/AI_TRADING_AGENTS.md
docs/**/*.{md,rst}
📄 CodeRabbit inference engine (CLAUDE.md)
When touching code, proactively check both
docs/anddocsrc/for gaps and update both locations if the behavior, API, or workflow changed.
Files:
docs/MCP_2026_TRANSPORT.mddocs/AI_TRADING_AGENTS.md
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/**/*.md: Use uppercase underscore-separated documentation filenames, with date-first names for handoffs and investigations.
New docs must begin with a title, one-line description, Last Updated date, Status, Audience, and an Overview section.
For every LumiBot code change, check and update engineering documentation as needed, including architecture, investigations, handoffs, and behavioral rationale.
Files:
docs/MCP_2026_TRANSPORT.mddocs/AI_TRADING_AGENTS.md
docs/**
⚙️ CodeRabbit configuration file
docs/**: Documentation is public. Flag private paths, credential examples, internal account
emails, private endpoints, stale security claims, or operational details that should
live only in private runbooks.
Files:
docs/MCP_2026_TRANSPORT.mddocs/AI_TRADING_AGENTS.md
CHANGELOG.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update
CHANGELOG.mdfor every deployment, release, or significant change, and include changes from all contributors since the last version bump.Update CHANGELOG.md for every deployment, release, significant bug fix, feature, breaking change, performance change, or dependency update, including all contributors' changes since the previous version.
Files:
CHANGELOG.md
docsrc/**/*.rst
📄 CodeRabbit inference engine (CLAUDE.md)
docsrc/**/*.rst: When making user-facing changes, update the relevant Sphinx docs underdocsrc/(for example brokers, strategy methods, lifecycle methods, entities, backtesting, FAQ, common mistakes, getting started, or deployment pages).
For user-facing behavior changes, keep public documentation examples, parameters, return values, and edge cases current, and build the docs locally to verify rendering when applicable.Update the relevant public Sphinx documentation whenever user-facing behavior, APIs, brokers, data sources, lifecycle methods, properties, environment variables, or deployment behavior changes.
Files:
docsrc/botspot_mcp.rst
docsrc/**
⚙️ CodeRabbit configuration file
docsrc/**: Sphinx documentation is public. Flag private paths, credential examples, internal
account emails, private endpoints, stale security claims, or operational details that
should live only in private runbooks.
Files:
docsrc/botspot_mcp.rst
tests/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
tests/**/*.py: Add unit tests for any new functionality
Ensure high level of test coverage using pytest with coverage reporting
Ensure all tests are well-documented and follow best practices
tests/**/*.py: Treat any test whose earliest commit date is before 2025-06-01 as LEGACY. For LEGACY tests, fix the code, not the test. Only change a LEGACY test when you can clearly justify that the old expectation was incorrect or behavior was intentionally changed for correctness, and document it in the test file.
Treat any test whose earliest commit date is before 2025-01-01 as FROZEN LEGACY (effectively 'do not change'). Only change a FROZEN LEGACY test in exceptional cases and include a clear write-up in the test and PR.
If you change any expected values or assertions in a test, add a short note near the change explaining why (what changed, and why the new expectation is correct). Prefer making the test more robust (less brittle) over updating magic numbers.Consider test age when resolving failures: tests older than one year generally require code fixes, while newer-test conflicts require investigation and documented judgment.
Files:
tests/test_agent_runtime_remote_mcp.pytests/test_agent_runtime_mcp_transports.py
tests/**/*.{py,sh}
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.{py,sh}: Do not add environment variables solely to skip or disable tests; use existing pytest markers and clear normal skips instead.
Backtests and tests must configure DATADOWNLOADER_BASE_URL and DATADOWNLOADER_API_KEY through the runtime environment and must not shortcut through Theta directly.
Files:
tests/test_agent_runtime_remote_mcp.pytests/test_agent_runtime_mcp_transports.py
tests/**
⚙️ CodeRabbit configuration file
tests/**: Tests are public. Flag hardcoded real-looking credentials, account emails, personal
paths, private endpoints, and fixtures that could accidentally exercise production or
paid-vendor resources without explicit opt-in.
Files:
tests/test_agent_runtime_remote_mcp.pytests/test_agent_runtime_mcp_transports.py
lumibot/**/*.{py,md,rst}
📄 CodeRabbit inference engine (AGENTS.md)
lumibot/**/*.{py,md,rst}: LumiBot broker and data-source adapters exclusively own provider-specific market-data and trading behavior; public strategy and data-source APIs must remain provider-generic.
Keep provider batching, request construction, response parsing, and provider error normalization inside the relevant adapter; never expose BotSpot credentials, Vault references, Node service credentials, or Bot Manager internals in public contracts.
Multi-asset reads must preserve per-symbol success or failure so callers can distinguish returned and missing symbols without substituting another provider.
Measure backtest accuracy against live broker behavior when possible; use vendor parity artifacts only as regression signals, not absolute truth.
Files:
lumibot/components/agents/runtime.py
lumibot/**
⚙️ CodeRabbit configuration file
lumibot/**: Review runtime/library changes for trading safety, broker/account isolation, data
correctness, secret handling, and public/private boundary violations. Flag hardcoded
BotSpot-specific behavior unless it is clearly provider-generic and appropriate for
open-source LumiBot.
Files:
lumibot/components/agents/runtime.py
🪛 Pylint (4.0.6)
tests/test_agent_runtime_mcp_transports.py
[error] 15-15: No name 'PandasDataBacktesting' in module 'lumibot.backtesting'
(E0611)
[error] 16-16: No name 'AgentRunResult' in module 'lumibot.components.agents'
(E0611)
[error] 16-16: No name 'MCPServer' in module 'lumibot.components.agents'
(E0611)
[error] 17-17: No name 'Asset' in module 'lumibot.entities'
(E0611)
[error] 17-17: No name 'Data' in module 'lumibot.entities'
(E0611)
[error] 18-18: No name 'Strategy' in module 'lumibot.strategies'
(E0611)
lumibot/components/agents/runtime.py
[error] 38-38: No name 'Client' in module 'mcp.client'
(E0611)
[error] 50-50: No name 'streamable_http_client' in module 'mcp.client.streamable_http'
(E0611)
🔇 Additional comments (9)
requirements.txt (1)
45-46: LGTM!setup.py (1)
104-105: LGTM!docs/MCP_2026_TRANSPORT.md (1)
1-35: LGTM!docs/AI_TRADING_AGENTS.md (1)
150-154: LGTM!CHANGELOG.md (1)
3-10: LGTM!docsrc/botspot_mcp.rst (1)
25-25: LGTM!Also applies to: 38-42, 157-170
lumibot/components/agents/runtime.py (1)
4-9: LGTM!Also applies to: 27-52, 1543-1589, 1608-1618, 1621-1631
tests/test_agent_runtime_mcp_transports.py (1)
1-48: LGTM!Also applies to: 253-273, 302-322
tests/test_agent_runtime_remote_mcp.py (1)
82-123: LGTM!Also applies to: 143-143
| requests = [] | ||
|
|
||
| def do_POST(self): | ||
| length = int(self.headers.get("Content-Length", "0")) | ||
| body = self.rfile.read(length) | ||
| data = json.loads(body.decode("utf-8")) | ||
| method = data.get("method") | ||
| if method == "tools/list": | ||
| self.__class__.requests.append({"body": data, "headers": dict(self.headers)}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Record and reject non-POST MCP traffic.
Line 81 records only POST requests. Lines 186-199 cannot detect a standalone GET stream or session cleanup DELETE request.
Record GET and DELETE requests in the fixture. Assert that every recorded MCP request uses POST.
As per coding guidelines, “Document and test behavioral changes in the same change set, including regression tests and comments for non-obvious invariants.”
Proposed regression coverage
def do_POST(self):
length = int(self.headers.get("Content-Length", "0"))
body = self.rfile.read(length)
data = json.loads(body.decode("utf-8"))
method = data.get("method")
- self.__class__.requests.append({"body": data, "headers": dict(self.headers)})
+ self.__class__.requests.append(
+ {"http_method": "POST", "body": data, "headers": dict(self.headers)}
+ )
if method == "server/discover":
...
+ def do_GET(self):
+ self.__class__.requests.append(
+ {"http_method": "GET", "headers": dict(self.headers)}
+ )
+ self.send_error(405)
+
+ def do_DELETE(self):
+ self.__class__.requests.append(
+ {"http_method": "DELETE", "headers": dict(self.headers)}
+ )
+ self.send_error(405)
+
...
methods = [request["body"]["method"] for request in _MCPHandler.requests]
+ assert {request["http_method"] for request in _MCPHandler.requests} == {"POST"}Also applies to: 186-199
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_agent_runtime_remote_mcp.py` around lines 74 - 81, Update the test
fixture’s request handler around do_POST and the corresponding GET/DELETE
handlers to record every MCP request, including method and headers/body as
applicable. Add assertions in the affected tests that all recorded MCP requests
use POST, so standalone GET streams and DELETE session cleanup are explicitly
rejected.
Source: Coding guidelines
1 participant
Summary\n\n- migrate HTTP and stdio MCP connections to official protocol-v2 client\n- remove initialize/session flow and duplicate raw HTTP fallback\n- preserve bearer and custom headers through one client path\n- update public docs, dependency bounds, and transport fixtures\n\n## Performance\n\n- focused MCP suite: 9.02s baseline wall → 7.48s after\n- final verification: 6 passed in 6.25s\n\n## Verification\n\n- focused HTTP and stdio MCP tests\n- changed-line review; repository lint still reports old untouched runtime/test violations\n- diff checks
Summary by CodeRabbit
New Features
Documentation
Tests