From 5ae055d30b6e57ddee892d143a7475bb856a199e Mon Sep 17 00:00:00 2001 From: gnanam1990 Date: Tue, 28 Apr 2026 12:13:33 +0530 Subject: [PATCH] fix(mcp): allow third-party providers to approve project-scope .mcp.json servers (#696) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user runs openclaude with a third-party provider, project-scope MCP servers added via `openclaude mcp add -s project ...` were silently dropped from `/mcp` and `openclaude mcp list`. Re-adding the same server printed "MCP server already exists in .mcp.json" but the server never actually loaded. Reporter @gbmerrall pinpointed the cause to the `if (usesAnthropicSetup)` gate around `handleMcpjsonServerApprovals` in src/interactiveHelpers.tsx. The MCP approval dialog and the CLAUDE.md external-includes warning are about workspace trust, not about Anthropic auth. Gating them on `usesAnthropicSetup` meant 3P-provider users never saw the dialog that writes `enableAllProjectMcpServers: true` and `enabledMcpjsonServers: [...]` to settings.local.json — without those settings, the MCP server isn't loaded for use. Drop the `usesAnthropicSetup` gate around the approval flow. The inner logic is unchanged and `handleMcpjsonServerApprovals` already early-returns when no servers are pending, so users without project `.mcp.json` see no new behavior. - src/interactiveHelpers.tsx: drop the gate, add a comment explaining why (and cite #696 so future readers can find context). - src/__tests__/bugfixes.test.ts: +2 regression tests asserting the gate is gone and the issue is referenced. Verified locally on Linux: build passes (v0.7.0), 1634 tests pass, the 4 remaining failures (StartupScreen.test.ts, thinking.test.ts) reproduce on main and are unrelated. The bundled dist/cli.mjs shows `handleMcpjsonServerApprovals(root2)` running directly after `setSessionTrustAccepted(true)` with no auth gate. --- src/__tests__/bugfixes.test.ts | 27 ++++++++++++++++++++++++++ src/interactiveHelpers.tsx | 35 +++++++++++++++++----------------- 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/__tests__/bugfixes.test.ts b/src/__tests__/bugfixes.test.ts index edaa2715..863f3639 100644 --- a/src/__tests__/bugfixes.test.ts +++ b/src/__tests__/bugfixes.test.ts @@ -288,3 +288,30 @@ describe('Context overflow 500 fix', () => { expect(content).toContain('automatic compaction has failed') }) }) + +// --------------------------------------------------------------------------- +// Fix N: Project-scope MCP servers from .mcp.json not detected for 3P providers (issue #696) +// --------------------------------------------------------------------------- +describe('Project-scope MCP approval — third-party providers (issue #696)', () => { + test('handleMcpjsonServerApprovals is NOT gated behind usesAnthropicSetup', async () => { + const content = await file('interactiveHelpers.tsx').text() + + // The call site for handleMcpjsonServerApprovals must not sit inside an + // `if (usesAnthropicSetup) { ... }` block, or third-party providers will + // never get the dialog and project-scope .mcp.json servers will be silently + // dropped from /mcp listings (issue #696). + const approvalCallIdx = content.indexOf('await handleMcpjsonServerApprovals(root)') + expect(approvalCallIdx).toBeGreaterThan(-1) + + // Look at the 800 chars BEFORE the call site for any `if (usesAnthropicSetup)` + // block that would still be open. Pick a window that's definitely inside the + // showSetupScreens function but not in earlier dialogs. + const before = content.slice(Math.max(0, approvalCallIdx - 800), approvalCallIdx) + expect(before).not.toMatch(/if\s*\(\s*usesAnthropicSetup\s*\)\s*{[^}]*$/) + }) + + test('issue #696 is referenced from the comment so future readers can find context', async () => { + const content = await file('interactiveHelpers.tsx').text() + expect(content).toContain('#696') + }) +}) diff --git a/src/interactiveHelpers.tsx b/src/interactiveHelpers.tsx index 60ee7c16..c655f38e 100644 --- a/src/interactiveHelpers.tsx +++ b/src/interactiveHelpers.tsx @@ -158,24 +158,25 @@ export async function showSetupScreens(root: Root, permissionMode: PermissionMod // Now that trust is established, prefetch system context if it wasn't already void getSystemContext(); - // Skip MCP approval dialogs for third-party providers (no interactive auth prompts) - if (usesAnthropicSetup) { - // If settings are valid, check for any mcp.json servers that need approval - const { - errors: allErrors - } = getSettingsWithAllErrors(); - if (allErrors.length === 0) { - await handleMcpjsonServerApprovals(root); - } + // MCP approval and external-includes warnings are about workspace + // trust, not about Anthropic auth. They must run for all providers + // — including third-party — otherwise project-scoped .mcp.json + // servers never get the approval that writes + // enableAllProjectMcpServers / enabledMcpjsonServers into + // settings.local.json, and the servers are silently dropped from + // /mcp and `mcp list` (issue #696). + const { errors: allErrors } = getSettingsWithAllErrors(); + if (allErrors.length === 0) { + await handleMcpjsonServerApprovals(root); + } - // Check for claude.md includes that need approval - if (await shouldShowClaudeMdExternalIncludesWarning()) { - const externalIncludes = getExternalClaudeMdIncludes(await getMemoryFiles(true)); - const { - ClaudeMdExternalIncludesDialog - } = await import('./components/ClaudeMdExternalIncludesDialog.js'); - await showSetupDialog(root, done => ); - } + // Check for claude.md includes that need approval + if (await shouldShowClaudeMdExternalIncludesWarning()) { + const externalIncludes = getExternalClaudeMdIncludes(await getMemoryFiles(true)); + const { + ClaudeMdExternalIncludesDialog + } = await import('./components/ClaudeMdExternalIncludesDialog.js'); + await showSetupDialog(root, done => ); } }