refactor: optimize MCP loading with lazy-load strategy#191
Conversation
- Add claude-code subagent for on-demand claude-code-mcp access - Remove claude-code-mcp_* from main agent tool lists (Build+, AI-DevOps) - Set 12 MCPs to lazy-load (enabled: false): claude-code-mcp, gsc, localwp, chrome-devtools, outscraper, amazon-order-history, google-analytics-mcp, dataforseo, shadcn, macos-automator, quickfile, MCP_DOCKER - Keep 8 MCPs eager-loaded for all main agents: osgrep, augment-context-engine, context7, repomix, playwriter, gh_grep, sentry, socket - Enable playwriter_* and repomix_* globally for all main agents - Add MCP loading policy enforcement to update existing configs This reduces OpenCode startup time by deferring MCP server initialization until the specific subagent that needs them is invoked.
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the Micro-service Control Plane (MCP) loading mechanism within OpenCode, shifting a significant portion of MCPs to a lazy-loading model. This change aims to improve application startup performance by only initializing services when they are explicitly required by a subagent. Concurrently, it introduces a dedicated subagent for Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis change introduces an eager/lazy MCP loading policy for the OpenCode agent framework, expanding tool permissions across 10+ agents with new subagent patterns (gh_grep, playwriter, claude-code-mcp), creating MCPs with classified startup behavior, and adding configuration documentation for the Claude Code MCP subagent. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a lazy-loading strategy for MCPs to optimize startup time, which is a significant improvement. The changes are primarily within the generate-opencode-agents.sh script, where MCPs are now explicitly categorized as eager or lazy. The addition of a new @claude-code subagent is also a valuable enhancement. My review highlights a few areas for improvement in the script's robustness and maintainability, including a hardcoded path that impacts portability, a potential issue with uncategorized MCPs, some redundant code, and an inconsistency with a template file. Addressing these points will help make the implementation cleaner and more reliable.
| import shutil | ||
| import platform | ||
| bun_path = shutil.which('bun') | ||
| npx_path = shutil.which('npx') or '/opt/homebrew/bin/npx' |
There was a problem hiding this comment.
The hardcoded fallback path for npx to /opt/homebrew/bin/npx compromises the script's portability. This path is specific to a particular setup on macOS (Homebrew on Apple Silicon) and will fail on other operating systems or different macOS configurations where npx might be located elsewhere.
To make the script more robust, it's better to rely solely on shutil.which('npx') to find the executable in the system's PATH. If npx is not found, you could then handle it gracefully by either printing a warning or exiting with an error message if it's a critical dependency.
| npx_path = shutil.which('npx') or '/opt/homebrew/bin/npx' | |
| npx_path = shutil.which('npx') |
| for mcp_name in list(config.get('mcp', {}).keys()): | ||
| if mcp_name in EAGER_MCPS: | ||
| config['mcp'][mcp_name]['enabled'] = True | ||
| elif mcp_name in LAZY_MCPS: | ||
| config['mcp'][mcp_name]['enabled'] = False |
There was a problem hiding this comment.
The current logic only applies the loading policy to MCPs that are explicitly listed in EAGER_MCPS or LAZY_MCPS. If an MCP is present in the configuration but not categorized in either list, its enabled state remains unchanged. This could lead to inconsistent behavior and undermine the goal of a centralized loading policy.
To improve robustness, I suggest adding a handler for uncategorized MCPs. For instance, you could log a warning to alert the user that a new MCP needs to be categorized. This ensures that all MCPs are intentionally configured.
| for mcp_name in list(config.get('mcp', {}).keys()): | |
| if mcp_name in EAGER_MCPS: | |
| config['mcp'][mcp_name]['enabled'] = True | |
| elif mcp_name in LAZY_MCPS: | |
| config['mcp'][mcp_name]['enabled'] = False | |
| for mcp_name in list(config.get('mcp', {}).keys()): | |
| if mcp_name in EAGER_MCPS: | |
| config['mcp'][mcp_name]['enabled'] = True | |
| elif mcp_name in LAZY_MCPS: | |
| config['mcp'][mcp_name]['enabled'] = False | |
| else: | |
| print(f" Warning: MCP '{mcp_name}' is not categorized as eager or lazy. Its loading policy is unchanged.") |
| else: | ||
| # Ensure existing config is set to lazy load | ||
| config['mcp']['outscraper']['enabled'] = False |
There was a problem hiding this comment.
This else block, which sets enabled: False for an existing outscraper configuration, is redundant. The loop at lines 520-524 already iterates through all configured MCPs and sets their enabled state based on the EAGER_MCPS and LAZY_MCPS lists.
This redundant pattern is also present for dataforseo (lines 611-612), shadcn (lines 627-628), and macos-automator (lines 658-659).
To improve code clarity and maintainability, these else blocks should be removed. The centralized logic at the beginning of the section should be the single source of truth for enforcing the loading policy.
| "type": "local", | ||
| "command": ["npx", "-y", "github:marcusquinn/claude-code-mcp"], | ||
| "enabled": True | ||
| "command": ["npx", "-y", "@steipete/claude-code-mcp"], |
There was a problem hiding this comment.
This line updates the claude-code-mcp command to use the upstream package @steipete/claude-code-mcp instead of a fork. This is a positive change. However, the corresponding template file, configs/mcp-templates/claude-code-mcp.json, still references the old fork (github:marcusquinn/claude-code-mcp).
This inconsistency could cause confusion or issues if the template is used for other configurations. To ensure consistency across the codebase, please consider updating configs/mcp-templates/claude-code-mcp.json to reflect this change as well.
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Jan 25 02:20:50 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
🤖 Augment PR SummarySummary: Refactors OpenCode agent generation to reduce startup time by lazy-loading most MCP servers.
enabled: false on MCP definitions to defer startup until a subagent invokes the relevant tools.
Startup impact: fewer MCP servers initialized at launch, with capabilities loaded on-demand as needed.
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| print(" Added gh_grep MCP (eager load - used by Plan+/Build+/AI-DevOps)") | ||
|
|
||
| # gh_grep tools disabled globally, enabled for specific agents | ||
| if 'gh_grep_*' not in config['tools']: |
There was a problem hiding this comment.
This only disables gh_grep_* when the key is missing; if an existing opencode.json already has it true, the intended restriction to Plan+/Build+/AI-DevOps won't take effect. Is preserving pre-existing global enablement intentional for upgrades?
Other Locations
.agent/scripts/generate-opencode-agents.sh:599.agent/scripts/generate-opencode-agents.sh:614.agent/scripts/generate-opencode-agents.sh:630.agent/scripts/generate-opencode-agents.sh:661
🤖 Was this useful? React with 👍 or 👎
| print(" Set gh_grep_* disabled globally (enabled for Plan+/Build+/AI-DevOps)") | ||
|
|
||
| # repomix_* enabled globally (used by all main agents) | ||
| config['tools']['repomix_*'] = True |
There was a problem hiding this comment.
repomix_* is enabled globally here, but this script doesn't appear to create a config['mcp']['repomix'] entry if it's missing, so fresh configs may end up with enabled tools but no server. If repomix is expected to be present from elsewhere, it may be worth clarifying that assumption.
🤖 Was this useful? React with 👍 or 👎
| elif mcp_name in LAZY_MCPS: | ||
| config['mcp'][mcp_name]['enabled'] = False | ||
|
|
||
| print(f" Applied MCP loading policy: {len(EAGER_MCPS)} eager, {len(LAZY_MCPS)} lazy") |
There was a problem hiding this comment.
LAZY_MCPS currently contains 13 entries (so the log prints {len(LAZY_MCPS)} lazy), but the PR description/comments mention 12 lazy-loaded MCPs. Worth double-checking which set is intended to avoid confusing output.
🤖 Was this useful? React with 👍 or 👎
|
|
||
| | Tool | Description | | ||
| |------|-------------| | ||
| | `claude_code` | Execute a prompt via Claude Code CLI | |
There was a problem hiding this comment.
This doc lists the tool as claude_code, but elsewhere the MCP indexing/naming pattern uses <mcp>_<tool> names (e.g., claude-code-mcp_run_claude_code). Consider verifying the exposed tool name so users can invoke the right command.
🤖 Was this useful? React with 👍 or 👎
![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
🤖 Fix all issues with AI agents
In @.agent/scripts/generate-opencode-agents.sh:
- Around line 502-506: The fallback for locating npx is macOS-specific
(/opt/homebrew/bin/npx); update the npx resolution so it does not hardcode a
Homebrew path: change how npx_path and pkg_runner are determined (variables
bun_path, npx_path, pkg_runner) to use a platform-agnostic fallback such as just
'npx' (or prefer shutil.which('npx') and if that returns None use 'npx') so
pkg_runner will resolve correctly on Linux and other systems.
🧹 Nitpick comments (3)
.agent/scripts/generate-opencode-agents.sh (2)
519-526: Consider defensive access when enforcing MCP policy.The loop assumes all existing MCP entries have a dict structure with an
enabledkey. If an MCP entry is malformed or has a different structure, this could raise aKeyErrororTypeError.♻️ Defensive access pattern
for mcp_name in list(config.get('mcp', {}).keys()): + mcp_cfg = config['mcp'].get(mcp_name, {}) + if not isinstance(mcp_cfg, dict): + continue if mcp_name in EAGER_MCPS: - config['mcp'][mcp_name]['enabled'] = True + mcp_cfg['enabled'] = True elif mcp_name in LAZY_MCPS: - config['mcp'][mcp_name]['enabled'] = False + mcp_cfg['enabled'] = False
789-793: Summary output should mention the new eager/lazy policy.The summary section mentions "MCPs disabled globally, enabled per-agent" which is the old messaging. Consider updating to reflect the new eager/lazy loading strategy more accurately.
♻️ Updated summary messaging
echo "MCP Loading Strategy:" -echo " - MCPs disabled globally, enabled per-agent (reduces context tokens)" +echo " - Eager MCPs: Start at launch (osgrep, context7, playwriter, etc.)" +echo " - Lazy MCPs: Start on-demand via subagents (claude-code-mcp, dataforseo, etc.)" echo " - Use 'mcp-index-helper.sh search <query>' to discover tools on-demand" echo " - Subagents enable specific MCPs via frontmatter tools: section".agent/tools/ai-assistants/claude-code.md (1)
107-112: Document the cost implications of nested Claude invocations.The "Avoid loops" guidance is solid and correctly prevents infinite recursion and token exhaustion. Consider expanding this best practice to mention the potential cost multiplier of nested Claude invocations, as this provides developers with additional context for the prohibition on sub-agent loops.
- Remove hardcoded /opt/homebrew/bin/npx fallback, use shutil.which only - Add warning for uncategorized MCPs in loading policy - Remove redundant else blocks that duplicated loading policy logic - Update claude-code-mcp.json template to use upstream @steipete package - Add sys import for stderr warnings
Keep using github:marcusquinn/claude-code-mcp fork until steipete/claude-code-mcp#40 is merged.
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Jan 25 02:25:25 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sun Jan 25 02:27:15 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
1 participant
Summary
@claude-codesubagent for on-demand claude-code-mcp access (instead of loading in main agents)playwriter_*andrepomix_*globally for all main agentsChanges
MCP Loading Policy
Eager-loaded (8) - Start at OpenCode launch, used by all main agents:
Lazy-loaded (12) - Start on-demand when subagent invokes them:
Agent Tool Changes
claude-code-mcp_*: truefrom Build+ and AI-DevOps (use@claude-codesubagent instead)gh_grep_*: trueto Plan+, Build+, AI-DevOpsplaywriter_*: trueandrepomix_*: trueto all main agentsNew Subagent
tools/ai-assistants/claude-code.md- Invokes claude-code-mcp on-demandImpact
Reduces OpenCode startup time by deferring 12 MCP server initializations until actually needed.
Testing
generate-opencode-agents.shruns successfullySummary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.