From e9e04e28834c810b69e329c702415d1c4b175f69 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:44:15 -0400 Subject: [PATCH 1/3] fix(chat): load user MCP servers in Claude chats (parity with CLI) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../services/chat/agentChatService.test.ts | 54 +++++++++++++++++-- .../main/services/chat/agentChatService.ts | 17 ++++-- 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 20ca83dda..c87b2f560 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -2291,7 +2291,7 @@ describe("createAgentChatService", () => { ]); }); - it("disables Claude MCP while keeping project setting source enabled", async () => { + it("loads user/project MCP servers in normal chats (no managed-only lock)", async () => { fs.writeFileSync(path.join(tmpRoot, ".mcp.json"), JSON.stringify({ mcpServers: { projectTools: { @@ -2325,13 +2325,57 @@ describe("createAgentChatService", () => { settingSources?: string[]; } | undefined; expect(opts).toBeTruthy(); + // Project/user setting sources stay enabled so the SDK reads the user's + // configured MCP servers (.mcp.json / ~/.claude.json) — same as a terminal session. expect(opts?.settingSources).toEqual(expect.arrayContaining(["project"])); + // ADE does not inject mcpServers into a normal chat (only orchestration does), + // and it no longer locks MCP to managed-only — so the user's servers can load. expect(opts).not.toHaveProperty("mcpServers"); - expect(opts?.managedSettings).toMatchObject({ - allowedMcpServers: [], - allowManagedMcpServersOnly: true, - strictPluginOnlyCustomization: ["mcp"], + expect(opts?.managedSettings).toBeUndefined(); + }); + + it("keeps lightweight sessions lean by ignoring on-disk MCP config (strictMcpConfig)", async () => { + fs.writeFileSync(path.join(tmpRoot, ".mcp.json"), JSON.stringify({ + mcpServers: { + projectTools: { + command: "node", + args: ["mcp-server.js"], + }, + }, + })); + vi.mocked(claudeSdkCreateSessionCompat).mockReturnValue({ + send: vi.fn(), + stream: vi.fn(async function* () { + return; + }), + close: vi.fn(), + sessionId: "sdk-session-light-mcp", + } as any); + + const { service } = createService(); + await service.createSession({ + laneId: "lane-1", + provider: "claude", + model: "sonnet", + sessionProfile: "light", }); + + await vi.waitFor(() => { + expect(claudeSdkCreateSessionCompat).toHaveBeenCalled(); + }); + + const opts = vi.mocked(claudeSdkCreateSessionCompat).mock.calls[0]?.[0] as { + strictMcpConfig?: boolean; + managedSettings?: Record; + settingSources?: string[]; + } | undefined; + expect(opts).toBeTruthy(); + // Lightweight side-jobs (auto-title / lane-naming) don't get settingSources, + // and the SDK loads all MCP sources when unconstrained — so strictMcpConfig must + // be set to keep them from spawning the user's whole MCP fleet for a trivial job. + expect(opts?.settingSources).toBeUndefined(); + expect(opts?.strictMcpConfig).toBe(true); + expect(opts?.managedSettings).toBeUndefined(); }); it("attaches ADE orchestration tools to Claude lead sessions through an SDK MCP server", async () => { diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index d25dfd4da..bcbd53760 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -16937,11 +16937,18 @@ export function createAgentChatService(args: { cwd: managed.laneWorktreePath, env: claudeEnv, pathToClaudeCodeExecutable: claudeExecutable.path, - managedSettings: { - allowedMcpServers: [], - allowManagedMcpServersOnly: true, - strictPluginOnlyCustomization: ["mcp"], - }, + // User-configured MCP servers (~/.claude.json, project .mcp.json) load via + // settingSources below, matching a terminal `claude` session. We previously + // locked this to managed-only (allowManagedMcpServersOnly + empty allowlist) as + // a context/perf trim in #294, but that silently stripped the user's MCP tools + // from chats — e.g. iOS-automation servers — making the SDK chat strictly less + // capable than an `ade` CLI session for the same task. ENABLE_TOOL_SEARCH now + // keeps large tool catalogs cheap, so the trim is no longer worth the capability + // loss. Orchestration sessions re-apply managed-only isolation in their own block. + // Lightweight side-jobs (auto-title / lane-naming) get no settingSources, and the + // SDK loads all MCP sources when unconstrained — so keep them lean with + // strictMcpConfig (ignores on-disk .mcp.json / user MCP), preserving prior behavior. + ...(lightweight ? { strictMcpConfig: true } : {}), settings: { outputStyle, enabledPlugins: CLAUDE_SESSION_DISABLED_PLUGINS, From f720f7ed99907722918721dcd571ccaa8acbfe25 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 15 Jun 2026 12:00:02 -0400 Subject: [PATCH 2/3] =?UTF-8?q?ship:=20iter=201=20=E2=80=94=20harden=20nor?= =?UTF-8?q?mal-chat=20MCP=20test=20(Greptile=20P2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- apps/desktop/src/main/services/chat/agentChatService.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index c87b2f560..fa5bd8caf 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -2323,6 +2323,7 @@ describe("createAgentChatService", () => { const opts = vi.mocked(claudeSdkCreateSessionCompat).mock.calls[0]?.[0] as { managedSettings?: Record; settingSources?: string[]; + strictMcpConfig?: boolean; } | undefined; expect(opts).toBeTruthy(); // Project/user setting sources stay enabled so the SDK reads the user's @@ -2332,6 +2333,9 @@ describe("createAgentChatService", () => { // and it no longer locks MCP to managed-only — so the user's servers can load. expect(opts).not.toHaveProperty("mcpServers"); expect(opts?.managedSettings).toBeUndefined(); + // Inverse of the lightweight test: strictMcpConfig must NOT leak into normal + // chats, or it would silently re-block the user's MCP servers we just enabled. + expect(opts?.strictMcpConfig).toBeUndefined(); }); it("keeps lightweight sessions lean by ignoring on-disk MCP config (strictMcpConfig)", async () => { From cde738afa9030ccdf8b2bc222da95e69a716ad4e Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 15 Jun 2026 12:16:26 -0400 Subject: [PATCH 3/3] =?UTF-8?q?ship:=20iter=202=20=E2=80=94=20isolate=20MC?= =?UTF-8?q?P=20for=20orchestration=20lead=20sessions=20(Codex=20P1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/main/services/chat/agentChatService.test.ts | 7 +++++++ apps/desktop/src/main/services/chat/agentChatService.ts | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index fa5bd8caf..ea0c07aeb 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -2454,6 +2454,10 @@ describe("createAgentChatService", () => { const opts = vi.mocked(claudeSdkCreateSessionCompat).mock.calls[0]?.[0] as any; expect(opts?.mcpServers?.["ade-orchestration"]).toBeUndefined(); + // Regression guard (removed base MCP lock): a draft lead has no managed MCP block + // yet, so strictMcpConfig must isolate it — user/project MCP servers must not restore + // tool capability the read-only lead is denied. + expect(opts?.strictMcpConfig).toBe(true); expect(opts?.disallowedTools).toEqual(expect.arrayContaining([ "Agent", "Bash", @@ -2491,6 +2495,9 @@ describe("createAgentChatService", () => { }); const opts = vi.mocked(claudeSdkCreateSessionCompat).mock.calls[0]?.[0] as any; + // Role-marked lead (no interactionMode, no bundle) is still a read-only lead, so it + // must be MCP-isolated too (regression guard for the removed base MCP lock). + expect(opts?.strictMcpConfig).toBe(true); expect(opts?.disallowedTools).toEqual(expect.arrayContaining([ "Agent", "Bash", diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index bcbd53760..1ded75948 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -16948,7 +16948,12 @@ export function createAgentChatService(args: { // Lightweight side-jobs (auto-title / lane-naming) get no settingSources, and the // SDK loads all MCP sources when unconstrained — so keep them lean with // strictMcpConfig (ignores on-disk .mcp.json / user MCP), preserving prior behavior. - ...(lightweight ? { strictMcpConfig: true } : {}), + // Orchestration LEAD sessions are read-only planners (their direct Claude tools are + // denied below); isolate their MCP the same way so user/project MCP servers can't + // hand a draft lead (no bundle yet → orchestration block skipped) tool capability + // back. Workers/validators do real work and keep user MCP. strictMcpConfig still + // permits the programmatic orchestration MCP server added below for bundled leads. + ...((lightweight || isOrchestrationLeadSession(managed.session)) ? { strictMcpConfig: true } : {}), settings: { outputStyle, enabledPlugins: CLAUDE_SESSION_DISABLED_PLUGINS,