feat(mcp): lazy on-demand MCP discovery for no_mcp platforms - #10
Closed
davidgut1982 wants to merge 2 commits into
Closed
feat(mcp): lazy on-demand MCP discovery for no_mcp platforms#10davidgut1982 wants to merge 2 commits into
davidgut1982 wants to merge 2 commits into
Conversation
…form in CLI chat/rl path
…md and config example
davidgut1982
force-pushed
the
feat/mcp-lazy-discovery-no-mcp
branch
from
June 17, 2026 22:12
32608ed to
6231485
Compare
Owner
Author
|
Superseded: lazy discovery code already merged into integrated. |
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
1 participant
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.
Intent
Lazy MCP discovery for no_mcp platforms. Platforms that start with no MCP context can discover and attach MCP toolsets on demand instead of eagerly loading every server at startup. This is Phase 2 of the MCP delegation improvements.
What Changed
ensure_mcp_discovered()andmark_eager_discovery_skipped()totools/mcp_tool.pyso platforms flaggedno_mcpskip startup MCP loading and instead discover and attach MCP toolsets on first useprofile_namethroughdelegate_tool.pyso profile-scoped MCP toolsets bypass theno_mcpparent toolset intersection, allowing delegated tasks to retain their profile's MCP contexthermes_cli/main.pypreventing background MCP startup from running onno_mcpplatforms in the CLI chat/RL path, with corresponding_active_platform_uses_no_mcpdetection helpers in both the CLI and gatewayRisk Assessment
✅ Low: The fix correctly closes the cli-entrypoint-bypass gap by gating the background MCP startup path on the no_mcp platform check, and no new functional issues were introduced; all four CLI entrypoints now consistently defer to lazy discovery on no_mcp platforms.
Testing
Ran 469 tests across the lazy MCP discovery core, MCP tool infrastructure, delegation toolset scoping, profile-based MCP context, and CLI platform-toolset logic — all passed with no failures. The feature correctly defers MCP server startup for
no_mcpplatforms and triggers exactly one thread-safe lazy discovery when a delegated child first requests MCP toolsets.Evidence: Full test run output (all 4 test files)
Evidence: Test evidence summary
12 passed: test_mcp_lazy_discovery.py (core lazy discovery logic) 166 passed: test_mcp_tool.py (MCP infrastructure + reconnection skip) 16 passed: test_delegate_toolset_scope.py (profile bypass + security) 4 passed: test_mcp_loop_profile_override.py (context propagation) 271 passed: CLI and tools_config tests TOTAL: 469 passed, 0 failedPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
Step was skipped.
🔧 **Review** - 5 issues found → auto-fixed ✅
hermes_cli/main.py:10623- The no_mcp gate in hermes_cli/main.py only guards the _run_inline_mcp_discovery branch. The background/TUI/acp/cron paths (start_background_mcp_discovery, _is_tui_chat_launch, acp, cron run/tick) bypass the gate entirely. In practice these run under platform=cli which lacks no_mcp, so current behavior is unchanged — but the feature invariant ('api_server skips eager discovery') is silently uncovered if api_server/no_mcp is ever launched via those entrypoints. Needs a decision: confirm api_server only enters via start_gateway() and document that assumption, or push the gate into start_background_mcp_discovery and the acp/cron paths.tools/mcp_tool.py:3700- ensure_mcp_discovered() sets _lazy_discovery_done permanently on first call (via the Event in the finally block) even when some servers failed. The eager path retries failed servers on each invocation, so lazy and eager discovery differ in fault recovery behavior. The docstring says 'same degradation as startup discovery failure' which is inaccurate — startup failures do get retried on the next turn; lazy failures do not. Should update the docstring to reflect this asymmetry.gateway/run.py:1637- _active_platform_uses_no_mcp in gateway/run.py and _active_platform_uses_no_mcp_at_startup in hermes_cli/main.py are near-identical (same env-var resolution, same platform_toolsets lookup, same no_mcp check). They differ only in config source and error handling. If the platform-resolution convention changes, both must be updated in lockstep. Consolidating into one shared helper would eliminate drift risk.tests/tools/test_mcp_lazy_discovery.py- TestActivePlatformUsesNoMcp only covers the gateway variant of _active_platform_uses_no_mcp. The CLI twin in hermes_cli/main.py — including its distinct read_raw_config() failure→False path — has no direct unit test. Given the duplication in finding perf(orchestrator): skip blocking Lore memory prefetch on parent agent #3, a parallel test class would guard against divergence, especially for the try/except edge case that is the only behavioral difference.gateway/run.py:1646- In gateway/run.py, if _load_gateway_config() raises inside the outer try/except at line ~16024, the exception is caught and logged as 'MCP tool discovery failed' — meaning discovery is skipped entirely (neither eager nor marked-skipped via mark_eager_discovery_skipped). Previously a config-load failure still attempted discover_mcp_tools() which loads its own config. This is a minor behavioral narrowing with low practical impact since _load_gateway_config already swallows its own errors and returns {}.🔧 Fix: guard background MCP startup against no_mcp platform in CLI chat/rl path
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
python -m pytest tests/tools/test_mcp_lazy_discovery.py -v --timeout=30— 12 tests covering mark_eager_discovery_skipped(), ensure_mcp_discovered() idempotency, thread-safety with 10 concurrent callers, failure tolerance, and _active_platform_uses_no_mcp() config detectionpython -m pytest tests/tools/test_mcp_tool.py -v --timeout=30— 166 tests including new TestReconnection::test_preflight_probe_skipped_when_already_ready for the MCP reconnection optimizationpython -m pytest tests/tools/test_delegate_toolset_scope.py -v --timeout=30— 16 tests covering profile MCP toolset bypass, non-MCP intersection security, and end-to-end no_mcp parent with profile retaining MCP toolsetspython -m pytest tests/tools/test_mcp_loop_profile_override.py -v --timeout=30— 4 tests covering HERMES_HOME context propagation to async MCP loop and concurrent scope isolationpython -m pytest tests/hermes_cli/test_commands.py tests/hermes_cli/test_mcp_tools_config.py tests/hermes_cli/test_tools_config.py -v --timeout=30— 271 CLI and tools config tests confirming no regression in existing platform toolset and startup logic✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.