feat: disable on-demand MCPs globally in opencode.json#277
Conversation
Adds disable_ondemand_mcps() to setup.sh that sets enabled=false for: - playwriter (~3K tokens) - augment-context-engine (~1K tokens) - gh-grep (~600 tokens) This actually reduces session startup context by ~4.6K tokens. The previous PR only changed agent permissions, not MCP loading. MCPs can still be enabled on-demand via their respective subagents: - @playwriter - @augment-context-engine - @github-search (uses rg/bash, no MCP needed)
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 focuses on optimizing the startup performance of the application by strategically disabling certain Managed Code Providers (MCPs) that are not essential for initial session loading. By moving these MCPs to an on-demand activation model via subagents, the system's initial context footprint is reduced, leading to a more efficient startup process without sacrificing overall functionality. 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
|
WalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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. ✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a new function to disable on-demand MCPs in opencode.json, which is a great optimization to reduce session startup context. The implementation is clear and follows the existing script's style. I've added one suggestion to improve the robustness of the JSON parsing within the new function to prevent potential script failures with slightly malformed configuration files.
| if jq -e ".mcp[\"$mcp\"]" "$tmp_config" > /dev/null 2>&1; then | ||
| local current_enabled | ||
| current_enabled=$(jq -r ".mcp[\"$mcp\"].enabled // \"true\"" "$tmp_config") | ||
| if [[ "$current_enabled" != "false" ]]; then | ||
| jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | ||
| ((disabled++)) | ||
| fi | ||
| fi |
There was a problem hiding this comment.
This change improves the script in two ways:
- Robustness: It uses
.mcp? | has(\"$mcp\")to safely check for an MCP's existence, preventing script failure if the top-level.mcpobject is missing inopencode.json. - Readability: It combines the check for the
enabledstatus directly within theifcondition, removing the need for an intermediatecurrent_enabledvariable.
| if jq -e ".mcp[\"$mcp\"]" "$tmp_config" > /dev/null 2>&1; then | |
| local current_enabled | |
| current_enabled=$(jq -r ".mcp[\"$mcp\"].enabled // \"true\"" "$tmp_config") | |
| if [[ "$current_enabled" != "false" ]]; then | |
| jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | |
| ((disabled++)) | |
| fi | |
| fi | |
| if jq -e ".mcp? | has(\"$mcp\")" "$tmp_config" > /dev/null 2>&1; then | |
| if [[ $(jq -r ".mcp[\"$mcp\"].enabled // \"true\"" "$tmp_config") != "false" ]]; then | |
| jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | |
| ((disabled++)) | |
| fi | |
| fi |
🤖 Augment PR SummarySummary: This PR reduces OpenCode session startup context by globally disabling select MCP servers in Changes:
Technical Notes: Uses 🤖 Was this summary useful? React with 👍 or 👎 |
| local -a ondemand_mcps=( | ||
| "playwriter" | ||
| "augment-context-engine" | ||
| "gh-grep" |
There was a problem hiding this comment.
In ondemand_mcps, the entry uses gh-grep, but other parts of the repo appear to name this MCP gh_grep (underscore). If opencode.json uses the underscore key, this loop will never match and gh_grep won’t actually be disabled.
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| current_enabled=$(jq -r ".mcp[\"$mcp\"].enabled // \"true\"" "$tmp_config") | ||
| if [[ "$current_enabled" != "false" ]]; then | ||
| jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | ||
| ((disabled++)) |
| confirm_step "Migrate loop state from .claude/ to .agent/loop-state/" && migrate_loop_state_directories | ||
| confirm_step "Cleanup deprecated agent paths" && cleanup_deprecated_paths | ||
| confirm_step "Cleanup deprecated MCP entries (hetzner, serper, etc.)" && cleanup_deprecated_mcps | ||
| confirm_step "Disable on-demand MCPs globally (playwriter, augment, gh-grep)" && disable_ondemand_mcps |
There was a problem hiding this comment.
This runs before update_opencode_config, but the OpenCode generator script enforces MCP enabled states and currently sets playwriter/augment-context-engine/gh_grep to enabled. As a result, the disablement here may be overwritten later in the same setup.sh run.
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@setup.sh`:
- Around line 333-340: The loop that processes each mcp currently increments the
disabled counter immediately after running jq and mv; change this so disabled is
only incremented when the write actually succeeds by checking the exit status of
the jq->mv operation (the jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" >
"${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" sequence) — e.g.
perform the jq write and mv, test their combined success, and only then execute
((disabled++)) for the given mcp and tmp_config; ensure any failure path does
not increment disabled so reporting and backup logic remain accurate.
- Line 3390: The current call to disable_ondemand_mcps runs before
update_opencode_config and is being undone by the generator's EAGER_MCPS policy;
either move the confirm_step "Disable on-demand MCPs globally (playwriter,
augment, gh-grep)" && disable_ondemand_mcps so it executes after
update_opencode_config, or modify the generator logic to remove playwriter,
augment-context-engine, and gh_grep from the EAGER_MCPS list and add them to
LAZY_MCPS so the generator no longer forces enabled=True for those MCPs; update
references in the generator where EAGER_MCPS/LAZY_MCPS are defined to keep
behavior consistent.
| for mcp in "${ondemand_mcps[@]}"; do | ||
| # Check if MCP exists and is currently enabled (or has no enabled field) | ||
| if jq -e ".mcp[\"$mcp\"]" "$tmp_config" > /dev/null 2>&1; then | ||
| local current_enabled | ||
| current_enabled=$(jq -r ".mcp[\"$mcp\"].enabled // \"true\"" "$tmp_config") | ||
| if [[ "$current_enabled" != "false" ]]; then | ||
| jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | ||
| ((disabled++)) |
There was a problem hiding this comment.
Only increment disabled after a successful jq write.
If jq or mv fails, the counter still increments and the script may back up/overwrite without an actual change. Gate the increment on success to keep reporting and backup behavior accurate.
🔧 Suggested fix
- if [[ "$current_enabled" != "false" ]]; then
- jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config"
- ((disabled++))
- fi
+ if [[ "$current_enabled" != "false" ]]; then
+ if jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config"; then
+ ((disabled++))
+ fi
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for mcp in "${ondemand_mcps[@]}"; do | |
| # Check if MCP exists and is currently enabled (or has no enabled field) | |
| if jq -e ".mcp[\"$mcp\"]" "$tmp_config" > /dev/null 2>&1; then | |
| local current_enabled | |
| current_enabled=$(jq -r ".mcp[\"$mcp\"].enabled // \"true\"" "$tmp_config") | |
| if [[ "$current_enabled" != "false" ]]; then | |
| jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | |
| ((disabled++)) | |
| for mcp in "${ondemand_mcps[@]}"; do | |
| # Check if MCP exists and is currently enabled (or has no enabled field) | |
| if jq -e ".mcp[\"$mcp\"]" "$tmp_config" > /dev/null 2>&1; then | |
| local current_enabled | |
| current_enabled=$(jq -r ".mcp[\"$mcp\"].enabled // \"true\"" "$tmp_config") | |
| if [[ "$current_enabled" != "false" ]]; then | |
| if jq ".mcp[\"$mcp\"].enabled = false" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config"; then | |
| ((disabled++)) | |
| fi | |
| fi |
🤖 Prompt for AI Agents
In `@setup.sh` around lines 333 - 340, The loop that processes each mcp currently
increments the disabled counter immediately after running jq and mv; change this
so disabled is only incremented when the write actually succeeds by checking the
exit status of the jq->mv operation (the jq ".mcp[\"$mcp\"].enabled = false"
"$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config"
sequence) — e.g. perform the jq write and mv, test their combined success, and
only then execute ((disabled++)) for the given mcp and tmp_config; ensure any
failure path does not increment disabled so reporting and backup logic remain
accurate.
| confirm_step "Migrate loop state from .claude/ to .agent/loop-state/" && migrate_loop_state_directories | ||
| confirm_step "Cleanup deprecated agent paths" && cleanup_deprecated_paths | ||
| confirm_step "Cleanup deprecated MCP entries (hetzner, serper, etc.)" && cleanup_deprecated_mcps | ||
| confirm_step "Disable on-demand MCPs globally (playwriter, augment, gh-grep)" && disable_ondemand_mcps |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the general structure and locate the functions
git ls-files | grep -E "setup\.sh|generate.*opencode"Repository: marcusquinn/aidevops
Length of output: 303
🏁 Script executed:
# Look at the context around line 3390 in setup.sh
sed -n '3385,3395p' setup.shRepository: marcusquinn/aidevops
Length of output: 1033
🏁 Script executed:
# Find the disable_ondemand_mcps function
rg -n "disable_ondemand_mcps" setup.shRepository: marcusquinn/aidevops
Length of output: 205
🏁 Script executed:
# Find the update_opencode_config function
rg -n "update_opencode_config" setup.shRepository: marcusquinn/aidevops
Length of output: 175
🏁 Script executed:
# Locate the generator script
fd -name "*generate*opencode*" -o -name "*opencode*generator*"Repository: marcusquinn/aidevops
Length of output: 294
🏁 Script executed:
# Find generator scripts with correct syntax
find . -name "generate-opencode*.sh"Repository: marcusquinn/aidevops
Length of output: 155
🏁 Script executed:
# View the order of execution and surrounding context (wider range)
sed -n '3385,3410p' setup.shRepository: marcusquinn/aidevops
Length of output: 2181
🏁 Script executed:
# Examine disable_ondemand_mcps function
sed -n '309,340p' setup.shRepository: marcusquinn/aidevops
Length of output: 1127
🏁 Script executed:
# Examine update_opencode_config function
sed -n '2042,2080p' setup.shRepository: marcusquinn/aidevops
Length of output: 1644
🏁 Script executed:
# Check the generator script for MCP config handling
cat .agent/scripts/generate-opencode-agents.sh | head -100Repository: marcusquinn/aidevops
Length of output: 4157
🏁 Script executed:
# Get the full generate-opencode-agents.sh script
wc -l .agent/scripts/generate-opencode-agents.shRepository: marcusquinn/aidevops
Length of output: 110
🏁 Script executed:
# Extract the Python section that handles MCP config
sed -n '/python3/,/PYEOF/p' .agent/scripts/generate-opencode-agents.sh | head -200Repository: marcusquinn/aidevops
Length of output: 7855
🏁 Script executed:
# Look specifically for how the script handles 'mcp' or 'enabled' in the Python code
rg -A 5 -B 5 '"mcp"|enabled' .agent/scripts/generate-opencode-agents.shRepository: marcusquinn/aidevops
Length of output: 9487
Move disable_ondemand_mcps after update_opencode_config or update the generator's MCP loading policy.
The generator script enforces an MCP loading policy that sets playwriter, augment-context-engine, and gh_grep to enabled=True via the EAGER_MCPS list. This overwrites the enabled=false flags set by disable_ondemand_mcps on line 3390. Either reorder these steps (move disable_ondemand_mcps to line 3400+) or update the generator to classify these as LAZY_MCPS instead of EAGER_MCPS to align with the on-demand loading strategy documented in the code comments.
🤖 Prompt for AI Agents
In `@setup.sh` at line 3390, The current call to disable_ondemand_mcps runs before
update_opencode_config and is being undone by the generator's EAGER_MCPS policy;
either move the confirm_step "Disable on-demand MCPs globally (playwriter,
augment, gh-grep)" && disable_ondemand_mcps so it executes after
update_opencode_config, or modify the generator logic to remove playwriter,
augment-context-engine, and gh_grep from the EAGER_MCPS list and add them to
LAZY_MCPS so the generator no longer forces enabled=True for those MCPs; update
references in the generator where EAGER_MCPS/LAZY_MCPS are defined to keep
behavior consistent.
Summary
Actually reduces session startup context by ~4.6K tokens by disabling MCPs in
opencode.json.The previous PR (#275) only changed agent permissions - the MCP tool definitions were still being loaded.
Changes
Adds
disable_ondemand_mcps()function tosetup.shthat setsenabled: falsefor:playwriteraugment-context-enginegh-grepHow it works
{ "mcp": { "playwriter": { "enabled": false // <-- Prevents tool definitions from loading } } }On-demand access
MCPs can still be enabled via subagents when needed:
@playwriter- browser automation@augment-context-engine- semantic search fallback@github-search- uses rg/bash (no MCP needed)Testing
./setup.shSummary by CodeRabbit