fix(mcp): allow third-party providers to approve project-scope .mcp.json servers (#696)
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.
This commit is contained in:
@@ -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')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 => <ClaudeMdExternalIncludesDialog onDone={done} isStandaloneDialog externalIncludes={externalIncludes} />);
|
||||
}
|
||||
// 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 => <ClaudeMdExternalIncludesDialog onDone={done} isStandaloneDialog externalIncludes={externalIncludes} />);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user