feat(mcp): lazy server activation (deferLoading + activate_server) - #184
Closed
raymondginger2018-sudo wants to merge 3 commits into
Closed
feat(mcp): lazy server activation (deferLoading + activate_server)#184raymondginger2018-sudo wants to merge 3 commits into
raymondginger2018-sudo wants to merge 3 commits into
Conversation
Add McpServerDefinition.defer_loading (deferLoading). Servers marked deferred are skipped by ensure_started (status stays 'deferred', no connection, no tools) and are brought up on demand via McpSessionRuntime.activate_server, which starts the connection, registers the server's tools into the ToolRegistry, and publishes status/capabilities. Idempotent; startup failures mark the server 'failed' without raising. Derived from the MCP lifecycle study of Hmbown/CodeWhale.
Collaborator
|
感谢 PR!已在最新 main(合并后)上试合验证:无冲突,你附带的 5 个用例与既有 MCP 运行时测试全部通过(15 passed),全量 1475 passed,ruff 通过。 deferLoading + activate_server 的设计我们认可 —— 按需激活对多 server 场景确实有价值,启动开销不该由未被用到的 server 承担。 两个小建议:
|
core/mcp/test_runtime_lazy.py sat outside pytest testpaths (tests/ + quant_trading/tests), so CI never collected the 5 new cases. Move it to tests/test_mcp_runtime_lazy.py; now collected and run by CI.
Contributor
Author
|
Thanks for the review! Both suggestions are addressed:
|
Contributor
Author
|
Hi Zongwei,
Thanks for the quick review! Both points are addressed:
1. Test location - moved to tests/test_mcp_runtime_lazy.py. This actually
mattered more than convention: pytest.ini sets testpaths = ["tests",
"quant_trading/tests"], so the old core/mcp/test_runtime_lazy.py was never
collected by CI - only run manually. It is now picked up by the CI runs
(all green).
2. PR description - rewritten (the body was garbled by an encoding issue).
Also following up on the other two reviews:
#183 (P1-4/P1-5): added an explicit no-op fallback for the
core.observability.events import (introduced by #181) - on ImportError,
emit_event is set to None and the deposit event is skipped with a comment,
so it is safe regardless of merge order.
#181: split per your suggestion into three independent PRs, all based on
current upstream main:
- #188 feat(keyring) - new files only, 13 tests
- #189 feat(classifier) - new files only, 14 tests
- #190 feat(core) loop suite + observability + memory P1 modules - 227
tests locally
#181 is closed as superseded.
One thing you may want to check (not PR-specific): the "Dependency and
license audit" job fails on every PR - pip_audit reports known
vulnerabilities in the desktop sidecar site-packages. Looks like an
upstream main issue (sidecar deps need upgrading), not caused by any PR.
Happy to adjust anything. Thanks!
Zongwei Li ***@***.***> 于2026年8月17日周一 18:44写道:
… *Zongwei9888* left a comment (HKUDS/DeepCode#184)
<#184 (comment)>
感谢 PR!已在最新 main(合并后)上试合验证:无冲突,你附带的 5 个用例与既有 MCP 运行时测试全部通过(15 passed),全量
1475 passed,ruff 通过。
deferLoading + activate_server 的设计我们认可 —— 按需激活对多 server
场景确实有价值,启动开销不该由未被用到的 server 承担。
两个小建议:
1. 测试文件建议移到 tests/(仓库惯例是实现放 core/、测试放 tests/),当前在
core/mcp/test_runtime_lazy.py。
2. 本 PR 的描述正文目前显示为乱码,似乎是编码问题,方便的话重新编辑一下,便于其他人评审。
—
Reply to this email directly, view it on GitHub
<#184?email_source=notifications&email_token=CGK5R7WOUAG7A7MLEXF4XHT5KLOZXA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZRGUYDCOJQGA32M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5315019007>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CGK5R7XXJ2LJOCA322ATWTT5KLOZXAVCNFSNUABFKJSXA33TNF2G64TZHM4TQMZSGQ3DKNRYHNEXG43VMU5TKMJWHAZDMMBTGQY2C5QC>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/CGK5R7VGCOZGR3WAPDSKXAD5KLOZXA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZRGUYDCOJQGA32M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/CGK5R7T5ADG7E265BPYXNC35KLOZXA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZRGUYDCOJQGA32M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Zongwei9888
added a commit
that referenced
this pull request
Aug 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
特性
MCP 服务器延迟激活(deferLoading + activate_server):多 server 场景下,启动开销不应由未被用到的 server 承担。
改动
McpServerDefinition.defer_loading()(即deferLoading标记):ensure_started时跳过被标记的服务器(状态为deferred,不注册工具)。McpSessionRuntime.activate_server(server_id):按需启动延迟的服务器,并注册其工具到ToolRegistry;激活失败时状态置为failed。_register_server_tools/ensure_started均兼容activate_server路径。动机
参考 CodeWhale 的 MCP 延迟连接设计:未被使用的服务器不应承担启动开销。
测试
tests/test_mcp_runtime_lazy.py5 个用例:deferred 不启动 / activate 注册工具 / 幂等 / 未知 server 返回 False / 激活失败置 failed。