fix(chat): load user MCP servers in Claude chats (parity with CLI) - #580
Conversation
ADE's Work-tab Claude chat (Agent SDK) locked MCP to managed-only (allowManagedMcpServersOnly + empty allowlist) — a context/perf trim from #294 that silently stripped the user's configured MCP servers (~/.claude.json, project .mcp.json) from chats, making the SDK chat strictly less capable than an `ade` CLI session for the same task (e.g. iOS automation / WebDriverAgent). ENABLE_TOOL_SEARCH now keeps large tool catalogs cheap, so the trim is no longer worth the capability loss. - Remove the unconditional lock so normal chats load user MCP via settingSources. - Keep lightweight side-jobs (auto-title / lane-naming) lean via strictMcpConfig. - Orchestration sessions keep managed-only isolation in their own block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
More reviews will be available in 22 minutes and 18 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesMCP Session Configuration Rework
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
@copilot review but do not make fixes |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Assert strictMcpConfig is absent in the normal-chat test, guarding against it leaking into non-lightweight sessions (which would silently re-block the user's MCP servers). Mirrors the lightweight test's positive assertion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f720f7ed99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Removing the base MCP lock let draft / role-marked orchestration leads (no bundle yet → orchestration MCP block skipped) load user/project MCP servers, which could restore tool capability the read-only lead is denied via disallowedTools. Apply strictMcpConfig whenever isOrchestrationLeadSession is true, not only when the orchestration MCP server exists. strictMcpConfig still permits the programmatic orchestration server for bundled leads. Workers/validators do real work and keep user MCP. Regression tests: draft-lead and role-marked-lead now assert strictMcpConfig. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
What
ADE's Work-tab Claude chat (Agent SDK) locked MCP to managed-only
(
allowManagedMcpServersOnly: true+ emptyallowedMcpServers) — a context/perftrim from #294. That silently stripped the user's configured MCP servers
(
~/.claude.json, project.mcp.json) from chats, making the SDK chat strictlyless capable than an
adeCLI session for the same task (e.g. iOSautomation / WebDriverAgent, where an Xcode/Appium MCP would be present in the
CLI but absent in the chat).
Why now
ENABLE_TOOL_SEARCHnow defers large tool catalogs on demand, so the originalcontext/perf rationale for the lock no longer outweighs the capability loss.
Change
buildClaudeQueryOptionssonormal chats load the user's MCP servers via
settingSources— parity with aterminal/CLI
claudesession.strictMcpConfig(they have no
settingSources, and the SDK loads all MCP sources whenunconstrained — this preserves their pre-change behavior).
in their own block.
Tests
managedSettingsundefined + project setting source enabled (user MCP loadable).
strictMcpConfig: true.agentChatService.test.tsgreen (425 tests); typecheck clean.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Greptile Summary
This PR removes the blanket managed-only MCP lock (
allowManagedMcpServersOnly: true+ emptyallowedMcpServers) that was previously applied to every Claude chat session, restoring parity between SDK chats and terminalclaudesessions for user-configured MCP servers. The rationale (context/perf savings) is superseded byENABLE_TOOL_SEARCH.~/.claude.jsonand project.mcp.jsonservers viasettingSources;managedSettingsis no longer injected at the base level.strictMcpConfig: trueto keep them from spawning the user's full MCP fleet; bundled leads additionally retain the orchestration block'sallowManagedMcpServersOnlyconstraint.strictMcpConfigis absent; lightweight and lead sessions assert it is present.Confidence Score: 5/5
Safe to merge — the change is well-scoped and all three session categories (normal, lightweight, orchestration lead) are correctly handled with matching test coverage.
The removed managed-only lock is cleanly replaced by a targeted strictMcpConfig spread that only fires for lightweight and orchestration-lead sessions. Normal chats regain user MCP access through settingSources, which is the intended parity. Bundled orchestration leads still receive the existing allowManagedMcpServersOnly guard from the orchestration block. The symmetry is verified by new regression tests in both directions.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[buildClaudeQueryOptions] --> B{isLightweightSession?} B -- Yes --> C[strictMcpConfig: true\nno settingSources] B -- No --> D{isOrchestrationLeadSession?} D -- Yes --> E[strictMcpConfig: true\nsettingSources: user/project/local\ndisallowedTools applied] D -- No --> F[Normal chat\nsettingSources: user/project/local\nuser MCP servers load] E --> G{orchestrationMcpServer exists?} F --> G G -- Yes --> H[managedSettings:\nallowManagedMcpServersOnly: true\nallowedMcpServers: orch-server only] G -- No --> I[no managedSettings\nstrictMcpConfig alone guards leads]Reviews (3): Last reviewed commit: "ship: iter 2 — isolate MCP for orchestra..." | Re-trigger Greptile