t1162: Worker MCP isolation for Claude CLI dispatch#2102
Conversation
…cp-config (t1162) - Refactor generate_worker_mcp_config() to be CLI-aware ($2=ai_cli) - For Claude CLI: generates standalone mcpServers JSON, passed via --mcp-config --strict-mcp-config - For OpenCode: preserves existing XDG_CONFIG_HOME override behavior - Add --mcp-config flag to build_cli_cmd() for Claude CLI run action - Thread MCP config through build_dispatch_cmd, build_verify_dispatch_cmd - Update all 4 dispatch sites: cmd_dispatch, do_prompt_repeat, cmd_reprompt, deploy review-fix - Add config dir cleanup to cleanup_worker_processes() - Heavy indexers (osgrep, augment-context-engine) filtered from worker configs for both CLIs
|
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. WalkthroughThe PR introduces per-worker MCP config generation to isolate Claude workers from global MCP configuration. Changes add new helper functions to generate worker-specific configs for both Claude and OpenCode paths, update CLI command builders to accept and propagate MCP config parameters, and extend cleanup routines to remove per-worker config directories. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Dispatcher as Dispatch System
participant MCP as MCP Config<br/>Generator
participant Claude as Claude CLI
participant Worker as Worker Process
participant Cleanup as Cleanup System
User->>Dispatcher: Initiate task dispatch
Dispatcher->>MCP: generate_worker_mcp_config(task_id, path)
rect rgba(100, 150, 200, 0.5)
Note over MCP: For Claude path:<br/>Merge user + project config<br/>Filter heavy indexers
end
MCP->>MCP: Create per-worker config<br/>at pids/${task_id}-config
MCP-->>Dispatcher: Return config path
Dispatcher->>Claude: build_cli_cmd with<br/>--mcp-config flag
Dispatcher->>Claude: Invoke with --strict-mcp-config
Claude->>Worker: Execute with isolated<br/>MCP configuration
Worker-->>Claude: Return results
Dispatcher->>Cleanup: Task complete
Cleanup->>Cleanup: Remove pids/${task_id}-config
Cleanup-->>User: Cleanup finished
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
🔍 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: Sat Feb 21 09:27:33 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
… (t1162) 12 new tests covering: - generate_worker_mcp_config() for Claude CLI (JSON output, heavy indexer filtering) - generate_worker_mcp_config() for OpenCode (backward compat, XDG_CONFIG_HOME) - build_cli_cmd --mcp-config flag (present for Claude, absent for OpenCode) - --strict-mcp-config flag inclusion - cleanup_worker_processes removes per-worker config directory
🔍 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: Sat Feb 21 09:29:11 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
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 introduces a significant enhancement to worker isolation by implementing CLI-aware Multi-Client Protocol (MCP) configuration generation. The primary goal is to optimize resource usage and ensure predictable behavior for AI workers, particularly when using the Claude CLI. By generating tailored MCP configurations that exclude heavy indexers like osgrep and augment-context-engine, the system prevents unnecessary CPU consumption during worker startup. This change also standardizes how MCP configurations are applied across different AI clients, using CLI flags for Claude and environment variables for OpenCode, while maintaining full backward compatibility. Highlights
Changelog
Activity
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
|
![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
The pull request successfully implements worker MCP isolation for the Claude CLI, addressing performance issues caused by heavy indexers. However, there are several opportunities to improve the code's robustness and adherence to the project's style guide. Key areas for improvement include replacing eval with safer array construction, avoiding blanket error suppression with 2>/dev/null, ensuring all functions have explicit return statements, and consolidating jq operations for better efficiency.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/test-dispatch-claude-cli.sh (2)
1152-1184: Replace hardcoded/tmp/test-mcp-config.jsonwith a$TEST_DIR-scoped path for test isolation.Lines 1163 and 1215 pass
/tmp/test-mcp-config.jsonas the--mcp-configargument. Since the test only verifies thatbuild_cli_cmdpropagates the flag verbatim (the file doesn't need to exist), the path value itself is arbitrary — but/tmp/is global and shared across concurrent test runs. Using$TEST_DIRkeeps the value consistent with the rest of the file's isolation strategy and eliminates any theoretical collision with other processes.♻️ Proposed fix
- build_cli_cmd --cli claude --action run --output array \ - --model 'anthropic/claude-sonnet-4-6' \ - --mcp-config '/tmp/test-mcp-config.json' \ - --prompt 'Test prompt' + build_cli_cmd --cli claude --action run --output array \ + --model 'anthropic/claude-sonnet-4-6' \ + --mcp-config '$TEST_DIR/test-mcp-config.json' \ + --prompt 'Test prompt'Apply the same substitution at line 1179 (
grep -q "/tmp/test-mcp-config.json"→grep -q "$TEST_DIR/test-mcp-config.json") and line 1215.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test-dispatch-claude-cli.sh` around lines 1152 - 1184, Replace the hardcoded /tmp/test-mcp-config.json with a TEST_DIR-scoped path by using "$TEST_DIR/test-mcp-config.json" where the mcp config is passed to build_cli_cmd (the mcp_cmd assignment) and in the subsequent grep check that asserts the config file path; update the grep pattern that looks for "/tmp/test-mcp-config.json" accordingly. Ensure you reference the same variable TEST_DIR and leave the other checks for --mcp-config and --strict-mcp-config unchanged so the test still validates flag propagation for build_cli_cmd and the mcp_cmd output.
1117-1150: Add ajqprerequisite guard — missingjqcauses FAILs instead of SKIPs.
jqis invoked at four places (lines 1118, 1126–1127, 1140–1141, 1259) with no availability check. On a host withoutjq:
- Line 1118's compound condition short-circuits to
false→ reports FAIL (not SKIP).- Lines 1126–1127 produce empty strings → the
[[ "$has_osgrep" == "false" ]]comparisons fail silently → FAIL.This can produce misleading CI failures that have nothing to do with the feature under test.
♻️ Proposed fix: add a jq prerequisite skip block at the top of Section 11
section "Worker MCP Config Generation (t1162)" + +if ! command -v jq &>/dev/null; then + skip "generate_worker_mcp_config (jq not available — install jq to run MCP config tests)" + # Jump to summary; remaining assertions in this section depend on jq +elseThen close the
elseblock just before the SUMMARY comment (line 1292):+fi # end jq prerequisite guard + # ============================================================ # SUMMARY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test-dispatch-claude-cli.sh` around lines 1117 - 1150, Add a guard that checks for the jq executable before Section 11’s tests run and SKIPs the entire block if jq is missing: detect jq with command -v jq >/dev/null and if not found call skip (or echo SKIP) for the whole Claude MCP config test block that contains the "Generated config is valid JSON with mcpServers key", "Heavy indexers are excluded", and "Non-heavy MCP servers are preserved" checks (which use jq on $claude_mcp_result); wrap those tests in an if/else so jq-missing yields SKIP and ensure you close the else block just before the SUMMARY comment so the rest of the script behavior is unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/supervisor/dispatch.sh:
- Around line 1811-1958: The function generate_worker_mcp_config currently
assumes project .mcp.json locations and must accept a repo/worktree root
parameter and propagate it to _generate_worker_mcp_config_claude so
project-level MCPs are discovered; update generate_worker_mcp_config signature
to add a third arg (repo_root), pass it through to
_generate_worker_mcp_config_claude, and inside
_generate_worker_mcp_config_claude replace the hardcoded
"$HOME/Git/aidevops/.mcp.json" and "./.mcp.json" checks with a lookup that uses
the passed repo_root (e.g., "$repo_root/.mcp.json") while still falling back to
other sensible locations, then update all callers (e.g., cmd_dispatch,
do_prompt_repeat, cmd_reprompt and the review-fix dispatch sites) to pass the
worktree_path/repo root when invoking generate_worker_mcp_config.
In `@tests/test-dispatch-claude-cli.sh`:
- Around line 1066-1069: Remove the incorrect "duplicate SECTION 11" claim in
the test header for "Worker MCP Config Generation (t1162)"; replace both
hardcoded /tmp/test-mcp-config.json occurrences passed as mock CLI args with
"$TEST_DIR/test-mcp-config.json" so the test uses the same per-run artifact
variable; and add a prerequisite check near the top of this test (before any jq
usage) such as: command -v jq >/dev/null || skip "jq required" so jq absence
produces a clear skip rather than obscure failures (leave existing jq error
suppression and file-existence guards in place).
---
Nitpick comments:
In `@tests/test-dispatch-claude-cli.sh`:
- Around line 1152-1184: Replace the hardcoded /tmp/test-mcp-config.json with a
TEST_DIR-scoped path by using "$TEST_DIR/test-mcp-config.json" where the mcp
config is passed to build_cli_cmd (the mcp_cmd assignment) and in the subsequent
grep check that asserts the config file path; update the grep pattern that looks
for "/tmp/test-mcp-config.json" accordingly. Ensure you reference the same
variable TEST_DIR and leave the other checks for --mcp-config and
--strict-mcp-config unchanged so the test still validates flag propagation for
build_cli_cmd and the mcp_cmd output.
- Around line 1117-1150: Add a guard that checks for the jq executable before
Section 11’s tests run and SKIPs the entire block if jq is missing: detect jq
with command -v jq >/dev/null and if not found call skip (or echo SKIP) for the
whole Claude MCP config test block that contains the "Generated config is valid
JSON with mcpServers key", "Heavy indexers are excluded", and "Non-heavy MCP
servers are preserved" checks (which use jq on $claude_mcp_result); wrap those
tests in an if/else so jq-missing yields SKIP and ensure you close the else
block just before the SUMMARY comment so the rest of the script behavior is
unchanged.
marcusquinn
dismissed
coderabbitai[bot]’s stale review
Auto-dismissed: bot review does not block autonomous pipeline
🔍 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: Sat Feb 21 09:58:22 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
1 participant
Summary
Worker MCP isolation for Claude CLI dispatch — generates per-worker MCP config JSON for
--mcp-config --strict-mcp-configflags. Equivalent togenerate_worker_mcp_config()for OpenCode.Ref #1759
Changes
Core:
dispatch.shgenerate_worker_mcp_config()to be CLI-aware ($2=ai_cli):opencode: Preserves existing behavior — copiesopencode.jsonwith heavy indexers disabled, returnsXDG_CONFIG_HOMEpathclaude: Generates standalonemcpServersJSON from user's~/.claude/settings.jsonand project.mcp.json, filters out heavy indexers (osgrep, augment-context-engine), returns JSON file path--mcp-configflag tobuild_cli_cmd(): When building Claude CLI commands, emits--mcp-config <path> --strict-mcp-configto ensure workers only get specified MCP serverscmd_dispatch(main dispatch)do_prompt_repeat(prompt-repeat retry)cmd_reprompt(retry dispatch)deploy.shreview-fix dispatchCleanup:
cleanup.shcleanup_worker_processes()now removes per-worker${task_id}-config/directoryTests:
test-dispatch-claude-cli.shbuild_cli_cmdwith/without--mcp-configflagVerification
.shfilesSummary by CodeRabbit
New Features
Tests