From a3c5b7dca9ecd6a32605896a83f8b723ac7000cf Mon Sep 17 00:00:00 2001 From: 777genius Date: Thu, 16 Apr 2026 22:48:43 +0300 Subject: [PATCH] fix(extensions): honor local mcp scope precedence --- .../state/McpInstallationStateService.ts | 82 +++++++++++++------ .../extensions/mcp/McpServerCard.tsx | 5 +- src/shared/utils/extensionNormalizers.ts | 8 +- .../McpInstallationStateService.test.ts | 52 ++++++++++++ .../extensions/mcp/McpServerCard.test.ts | 5 +- .../mcp/McpServerDetailDialog.test.ts | 38 +++++++++ .../shared/utils/extensionNormalizers.test.ts | 6 +- 7 files changed, 162 insertions(+), 34 deletions(-) diff --git a/src/main/services/extensions/state/McpInstallationStateService.ts b/src/main/services/extensions/state/McpInstallationStateService.ts index 6e5950ec..a550237b 100644 --- a/src/main/services/extensions/state/McpInstallationStateService.ts +++ b/src/main/services/extensions/state/McpInstallationStateService.ts @@ -3,8 +3,8 @@ * * Sources: * - User scope: ~/.claude.json → mcpServers + * - Local scope: ~/.claude.json → projects[projectPath].mcpServers * - Project scope: .mcp.json in project root - * - Local scope: determined by Claude CLI (may also be in ~/.claude.json) * * Both files are managed by the Claude CLI. This service is read-only. */ @@ -30,7 +30,7 @@ export class McpInstallationStateService { private cache = new Map>(); /** - * Get all installed MCP servers across user and project scopes. + * Get all installed MCP servers across user, local, and project scopes. */ async getInstalled(projectPath?: string): Promise { const cacheKey = projectPath ?? '__user__'; @@ -40,15 +40,14 @@ export class McpInstallationStateService { } const entries: InstalledMcpEntry[] = []; + const claudeConfig = await this.readClaudeConfig(); // User scope: ~/.claude.json - const userEntries = await this.readUserMcpServers(); - entries.push(...userEntries); + entries.push(...this.readUserMcpServers(claudeConfig)); - // Project scope: .mcp.json if (projectPath) { - const projectEntries = await this.readProjectMcpServers(projectPath); - entries.push(...projectEntries); + entries.push(...this.readLocalMcpServers(claudeConfig, projectPath)); + entries.push(...(await this.readProjectMcpServers(projectPath))); } this.cache.set(cacheKey, { data: entries, fetchedAt: Date.now() }); @@ -64,9 +63,37 @@ export class McpInstallationStateService { // ── Private ──────────────────────────────────────────────────────────── - private async readUserMcpServers(): Promise { + private async readClaudeConfig(): Promise | null> { const configPath = path.join(getHomeDir(), '.claude.json'); - return this.readMcpServersFromFile(configPath, 'user'); + try { + const raw = await fs.readFile(configPath, 'utf-8'); + return JSON.parse(raw) as Record; + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + return null; + } + logger.error(`Failed to read MCP servers from ${configPath}:`, err); + return null; + } + } + + private readUserMcpServers(config: Record | null): InstalledMcpEntry[] { + return this.readMcpServersFromConfig(config?.mcpServers, 'user'); + } + + private readLocalMcpServers( + config: Record | null, + projectPath: string + ): InstalledMcpEntry[] { + const projects = + config && typeof config.projects === 'object' && config.projects + ? (config.projects as Record) + : null; + const projectConfig = + projects && typeof projects[projectPath] === 'object' && projects[projectPath] + ? (projects[projectPath] as Record) + : null; + return this.readMcpServersFromConfig(projectConfig?.mcpServers, 'local'); } private async readProjectMcpServers(projectPath: string): Promise { @@ -74,6 +101,27 @@ export class McpInstallationStateService { return this.readMcpServersFromFile(configPath, 'project'); } + private readMcpServersFromConfig( + value: unknown, + scope: 'user' | 'project' | 'local' + ): InstalledMcpEntry[] { + const mcpServers = + value && typeof value === 'object' + ? (value as Record) + : null; + if (!mcpServers) { + return []; + } + + return Object.entries(mcpServers).map(([name, config]): InstalledMcpEntry => { + let transport: string | undefined; + if (config.command) transport = 'stdio'; + else if (config.url) transport = 'http'; + + return { name, scope, transport }; + }); + } + private async readMcpServersFromFile( filePath: string, scope: 'user' | 'project' @@ -81,21 +129,7 @@ export class McpInstallationStateService { try { const raw = await fs.readFile(filePath, 'utf-8'); const json = JSON.parse(raw) as Record; - const mcpServers = json.mcpServers as - | Record - | undefined; - - if (!mcpServers || typeof mcpServers !== 'object') { - return []; - } - - return Object.entries(mcpServers).map(([name, config]): InstalledMcpEntry => { - let transport: string | undefined; - if (config.command) transport = 'stdio'; - else if (config.url) transport = 'http'; - - return { name, scope, transport }; - }); + return this.readMcpServersFromConfig(json.mcpServers, scope); } catch (err) { if ((err as NodeJS.ErrnoException).code === 'ENOENT') { return []; diff --git a/src/renderer/components/extensions/mcp/McpServerCard.tsx b/src/renderer/components/extensions/mcp/McpServerCard.tsx index aec1c0c3..c1e0d260 100644 --- a/src/renderer/components/extensions/mcp/McpServerCard.tsx +++ b/src/renderer/components/extensions/mcp/McpServerCard.tsx @@ -69,7 +69,10 @@ export const McpServerCard = ({ normalizedInstalledEntries.find((entry) => entry.scope === 'user') ?? null; const installSummaryLabel = getMcpInstallationSummaryLabel(normalizedInstalledEntries); const supportsDirectInstalledAction = - isInstalled && userInstallEntry?.name === defaultServerName && !requiresConfiguration; + isInstalled && + normalizedInstalledEntries.length === 1 && + userInstallEntry?.name === defaultServerName && + !requiresConfiguration; const shouldShowDirectInstallButton = canAutoInstall && (!isInstalled ? !requiresConfiguration : supportsDirectInstalledAction); const [imgError, setImgError] = useState(false); diff --git a/src/shared/utils/extensionNormalizers.ts b/src/shared/utils/extensionNormalizers.ts index 11c4e931..9eb456c9 100644 --- a/src/shared/utils/extensionNormalizers.ts +++ b/src/shared/utils/extensionNormalizers.ts @@ -146,14 +146,14 @@ export function getInstallationSummaryLabel( } const MCP_SCOPE_PRIORITY: Record = { - user: 0, + local: 0, project: 1, - local: 2, + user: 2, }; /** - * Pick a stable MCP installation entry when multiple scopes are installed. - * Prefer user scope because it is the only safe direct-card action target. + * Pick the MCP installation entry that Claude will actually use. + * Scope precedence matches Claude Code: local > project > user. */ export function getPreferredMcpInstallationEntry( installations: InstalledMcpEntry[] diff --git a/test/main/services/extensions/McpInstallationStateService.test.ts b/test/main/services/extensions/McpInstallationStateService.test.ts index 9e65a2be..61bfb566 100644 --- a/test/main/services/extensions/McpInstallationStateService.test.ts +++ b/test/main/services/extensions/McpInstallationStateService.test.ts @@ -23,6 +23,44 @@ describe('McpInstallationStateService', () => { }); describe('getInstalled', () => { + it('includes local scope from the current project entry in ~/.claude.json', async () => { + mockedFs.readFile.mockImplementation(async (filePath) => { + const normalizedPath = String(filePath); + if (normalizedPath === '/tmp/mock-home/.claude.json') { + return JSON.stringify({ + mcpServers: { + context7: { command: 'npx -y @upstash/context7-mcp' }, + }, + projects: { + '/tmp/project-a': { + mcpServers: { + stripe: { url: 'https://mcp.stripe.com' }, + }, + }, + }, + }); + } + + if (normalizedPath === '/tmp/project-a/.mcp.json') { + return JSON.stringify({ + mcpServers: { + paypal: { url: 'https://mcp.paypal.com/mcp' }, + }, + }); + } + + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + }); + + const entries = await service.getInstalled('/tmp/project-a'); + + expect(entries).toEqual([ + { name: 'context7', scope: 'user', transport: 'stdio' }, + { name: 'stripe', scope: 'local', transport: 'http' }, + { name: 'paypal', scope: 'project', transport: 'http' }, + ]); + }); + it('caches results within TTL for the same project path', async () => { mockedFs.readFile.mockImplementation(async (filePath) => { const normalizedPath = String(filePath); @@ -59,6 +97,18 @@ describe('McpInstallationStateService', () => { mcpServers: { context7: { command: 'npx -y @upstash/context7-mcp' }, }, + projects: { + '/tmp/project-a': { + mcpServers: { + stripe: { url: 'https://mcp.stripe.com' }, + }, + }, + '/tmp/project-b': { + mcpServers: { + github: { command: 'uvx github-mcp' }, + }, + }, + }, }); } @@ -86,10 +136,12 @@ describe('McpInstallationStateService', () => { expect(projectAEntries).toEqual([ { name: 'context7', scope: 'user', transport: 'stdio' }, + { name: 'stripe', scope: 'local', transport: 'http' }, { name: 'repo-a-server', scope: 'project', transport: 'http' }, ]); expect(projectBEntries).toEqual([ { name: 'context7', scope: 'user', transport: 'stdio' }, + { name: 'github', scope: 'local', transport: 'stdio' }, { name: 'repo-b-server', scope: 'project', transport: 'stdio' }, ]); expect(mockedFs.readFile).toHaveBeenCalledTimes(4); diff --git a/test/renderer/components/extensions/mcp/McpServerCard.test.ts b/test/renderer/components/extensions/mcp/McpServerCard.test.ts index 2e2a2be2..a122cd66 100644 --- a/test/renderer/components/extensions/mcp/McpServerCard.test.ts +++ b/test/renderer/components/extensions/mcp/McpServerCard.test.ts @@ -192,7 +192,7 @@ describe('McpServerCard direct action safety', () => { }); }); - it('keeps direct user action when the same server is installed in multiple scopes', async () => { + it('falls back to Manage when the same server is installed in multiple scopes', async () => { const host = document.createElement('div'); document.body.appendChild(host); const root = createRoot(host); @@ -217,7 +217,8 @@ describe('McpServerCard direct action safety', () => { }); expect(host.textContent).toContain('Installed in 2 scopes'); - expect(host.querySelector('[data-testid="install-button"]')).not.toBeNull(); + expect(host.querySelector('[data-testid="install-button"]')).toBeNull(); + expect(host.textContent).toContain('Manage'); await act(async () => { root.unmount(); diff --git a/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts b/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts index a19aa6b7..356415e9 100644 --- a/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts +++ b/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts @@ -407,4 +407,42 @@ describe('McpServerDetailDialog installed entry handling', () => { await Promise.resolve(); }); }); + + it('defaults to the highest-precedence installed scope', async () => { + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + const installedEntries: InstalledMcpEntry[] = [ + { name: 'context7', scope: 'user' }, + { name: 'context7-shared', scope: 'project' }, + ]; + + await act(async () => { + root.render( + React.createElement(McpServerDetailDialog, { + server: makeServer(), + isInstalled: true, + installedEntry: installedEntries[0], + installedEntries, + diagnostic: null, + diagnosticsLoading: false, + projectPath: '/tmp/project', + open: true, + onClose: vi.fn(), + }) + ); + await Promise.resolve(); + }); + + const scopeSelect = host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement; + const serverNameInput = host.querySelector('#server-name') as HTMLInputElement; + + expect(scopeSelect.value).toBe('project'); + expect(serverNameInput.value).toBe('context7-shared'); + + await act(async () => { + root.unmount(); + await Promise.resolve(); + }); + }); }); diff --git a/test/shared/utils/extensionNormalizers.test.ts b/test/shared/utils/extensionNormalizers.test.ts index a558f227..6b4708fc 100644 --- a/test/shared/utils/extensionNormalizers.test.ts +++ b/test/shared/utils/extensionNormalizers.test.ts @@ -209,16 +209,16 @@ describe('getPreferredMcpInstallationEntry', () => { expect(getPreferredMcpInstallationEntry([])).toBeNull(); }); - it('prefers user scope over project and local', () => { + it('prefers local scope over project and user', () => { const installations: InstalledMcpEntry[] = [ - { name: 'context7', scope: 'project' }, { name: 'context7', scope: 'user' }, + { name: 'context7', scope: 'project' }, { name: 'context7', scope: 'local' }, ]; expect(getPreferredMcpInstallationEntry(installations)).toEqual({ name: 'context7', - scope: 'user', + scope: 'local', }); }); });