From 94291f50f0381b575783007bad56f2cb15ee3f8e Mon Sep 17 00:00:00 2001 From: 777genius Date: Thu, 16 Apr 2026 22:51:05 +0300 Subject: [PATCH] fix(extensions): require project context for local mcp scope --- .../extensions/install/McpInstallService.ts | 16 ++-- .../extensions/mcp/CustomMcpServerDialog.tsx | 8 +- .../extensions/mcp/McpServerDetailDialog.tsx | 10 +-- .../extensions/McpInstallService.test.ts | 40 +++++++++ .../mcp/CustomMcpServerDialog.test.ts | 58 +++++++++++- .../mcp/McpServerDetailDialog.test.ts | 90 ++++++++++++++++++- 6 files changed, 205 insertions(+), 17 deletions(-) diff --git a/src/main/services/extensions/install/McpInstallService.ts b/src/main/services/extensions/install/McpInstallService.ts index 779e6178..687c904a 100644 --- a/src/main/services/extensions/install/McpInstallService.ts +++ b/src/main/services/extensions/install/McpInstallService.ts @@ -37,6 +37,10 @@ const HEADER_KEY_RE = /^[A-Za-z][\w-]{0,100}$/; const TIMEOUT_MS = 30_000; +function scopeRequiresProjectPath(scope?: string): boolean { + return scope === 'local' || scope === 'project'; +} + export class McpInstallService { constructor(private readonly aggregator: McpCatalogAggregator) {} @@ -59,10 +63,10 @@ export class McpInstallService { }; } - if (scope === 'project' && !projectPath) { + if (scopeRequiresProjectPath(scope) && !projectPath) { return { state: 'error', - error: 'projectPath is required for project scope', + error: `projectPath is required for ${scope} scope`, }; } @@ -219,8 +223,8 @@ export class McpInstallService { return { state: 'error', error: `Invalid scope: "${scope}".` }; } - if (scope === 'project' && !projectPath) { - return { state: 'error', error: 'projectPath is required for project scope' }; + if (scopeRequiresProjectPath(scope) && !projectPath) { + return { state: 'error', error: `projectPath is required for ${scope} scope` }; } for (const key of Object.keys(envValues)) { @@ -330,10 +334,10 @@ export class McpInstallService { }; } - if (scope === 'project' && !projectPath) { + if (scopeRequiresProjectPath(scope) && !projectPath) { return { state: 'error', - error: 'projectPath is required for project scope', + error: `projectPath is required for ${scope} scope`, }; } diff --git a/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx b/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx index b096287e..7cb8e740 100644 --- a/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx +++ b/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx @@ -105,7 +105,7 @@ export const CustomMcpServerDialog = ({ }, [open]); useEffect(() => { - if (open && scope === 'project' && !projectPath) { + if (open && scope !== 'user' && !projectPath) { setScope('user'); } }, [open, projectPath, scope]); @@ -177,7 +177,7 @@ export const CustomMcpServerDialog = ({ const request: McpCustomInstallRequest = { serverName, scope, - projectPath: scope === 'project' ? (projectPath ?? undefined) : undefined, + projectPath: scope !== 'user' ? (projectPath ?? undefined) : undefined, installSpec, envValues, headers: headers.filter((h) => h.key.trim() && h.value.trim()), @@ -207,7 +207,7 @@ export const CustomMcpServerDialog = ({ const canSubmit = serverName.trim() && (transportMode === 'stdio' ? npmPackage.trim() : httpUrl.trim()) && - !(scope === 'project' && !projectPath) && + !(scope !== 'user' && !projectPath) && !installing; return ( @@ -386,7 +386,7 @@ export const CustomMcpServerDialog = ({ {opt.label} diff --git a/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx b/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx index 91f3b2f8..43b49c27 100644 --- a/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx +++ b/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx @@ -132,7 +132,7 @@ export const McpServerDetailDialog = ({ }, [open, scope, selectedInstalledEntry?.name, server]); useEffect(() => { - if (open && scope === 'project' && !projectPath) { + if (open && scope !== 'user' && !projectPath) { setScope('user'); } }, [open, projectPath, scope]); @@ -179,7 +179,7 @@ export const McpServerDetailDialog = ({ const isInstalledForScope = selectedInstalledEntry !== null; const uninstallServerName = selectedInstalledEntry?.name ?? serverName; const uninstallScope = selectedInstalledEntry?.scope ?? scope; - const scopeRequiresProjectPath = scope === 'project' && !projectPath; + const scopeRequiresProjectPath = scope !== 'user' && !projectPath; const installDisabled = !serverName.trim() || missingRequiredEnvVars || @@ -199,7 +199,7 @@ export const McpServerDetailDialog = ({ registryId: server.id, serverName, scope, - projectPath: scope === 'project' ? (projectPath ?? undefined) : undefined, + projectPath: scope !== 'user' ? (projectPath ?? undefined) : undefined, envValues, headers, }); @@ -210,7 +210,7 @@ export const McpServerDetailDialog = ({ server.id, uninstallServerName, uninstallScope, - uninstallScope === 'project' ? (projectPath ?? undefined) : undefined + uninstallScope !== 'user' ? (projectPath ?? undefined) : undefined ); }; @@ -417,7 +417,7 @@ export const McpServerDetailDialog = ({ {opt.label} diff --git a/test/main/services/extensions/McpInstallService.test.ts b/test/main/services/extensions/McpInstallService.test.ts index 1320d359..f67716ad 100644 --- a/test/main/services/extensions/McpInstallService.test.ts +++ b/test/main/services/extensions/McpInstallService.test.ts @@ -130,6 +130,7 @@ describe('McpInstallService', () => { registryId: 'upstash/context7-mcp', serverName: 'context7', scope: 'local', + projectPath: '/tmp/test', envValues: {}, headers: [], }); @@ -230,6 +231,20 @@ describe('McpInstallService', () => { expect(result.error).toContain('projectPath is required'); expect(mockExecCli).not.toHaveBeenCalled(); }); + + it('rejects local scope install without project path', async () => { + const result = await service.install({ + registryId: 'upstash/context7-mcp', + serverName: 'context7', + scope: 'local', + envValues: {}, + headers: [], + }); + + expect(result.state).toBe('error'); + expect(result.error).toContain('projectPath is required'); + expect(mockExecCli).not.toHaveBeenCalled(); + }); }); // ── install: error masking ────────────────────────────────────────────────── @@ -290,6 +305,23 @@ describe('McpInstallService', () => { expect(result.error).toContain('projectPath is required'); expect(mockExecCli).not.toHaveBeenCalled(); }); + + it('rejects local scope custom install without project path', async () => { + const result = await service.installCustom({ + serverName: 'custom-context7', + scope: 'local', + installSpec: { + type: 'stdio', + npmPackage: '@upstash/context7-mcp', + }, + envValues: {}, + headers: [], + }); + + expect(result.state).toBe('error'); + expect(result.error).toContain('projectPath is required'); + expect(mockExecCli).not.toHaveBeenCalled(); + }); }); // ── uninstall ─────────────────────────────────────────────────────────────── @@ -334,6 +366,14 @@ describe('McpInstallService', () => { expect(mockExecCli).not.toHaveBeenCalled(); }); + it('rejects local scope uninstall without project path', async () => { + const result = await service.uninstall('context7', 'local'); + + expect(result.state).toBe('error'); + expect(result.error).toContain('projectPath is required'); + expect(mockExecCli).not.toHaveBeenCalled(); + }); + it('returns error on CLI failure', async () => { mockExecCli.mockRejectedValue(new Error('Not found')); diff --git a/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts b/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts index 9dd675ef..31ef3d9f 100644 --- a/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts +++ b/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts @@ -125,7 +125,7 @@ describe('CustomMcpServerDialog project scope', () => { vi.unstubAllGlobals(); }); - it('disables project scope without an active project', async () => { + it('disables non-user scopes without an active project', async () => { const host = document.createElement('div'); document.body.appendChild(host); const root = createRoot(host); @@ -142,7 +142,9 @@ describe('CustomMcpServerDialog project scope', () => { }); const projectOption = host.querySelector('option[value="project"]') as HTMLOptionElement; + const localOption = host.querySelector('option[value="local"]') as HTMLOptionElement; expect(projectOption.disabled).toBe(true); + expect(localOption.disabled).toBe(true); await act(async () => { root.unmount(); @@ -203,4 +205,58 @@ describe('CustomMcpServerDialog project scope', () => { await Promise.resolve(); }); }); + + it('passes projectPath for local-scoped custom installs', async () => { + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + const onClose = vi.fn(); + const projectPath = '/tmp/custom-mcp-project'; + + await act(async () => { + root.render( + React.createElement(CustomMcpServerDialog, { + open: true, + onClose, + projectPath, + }) + ); + await Promise.resolve(); + }); + + const nameInput = host.querySelector('#custom-name') as HTMLInputElement; + const packageInput = host.querySelector('#custom-npm') as HTMLInputElement; + const scopeSelect = host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement; + + await act(async () => { + setNativeValue(nameInput, 'local-context7', 'input'); + setNativeValue(packageInput, '@upstash/context7-mcp', 'input'); + setNativeValue(scopeSelect, 'local', 'change'); + await Promise.resolve(); + }); + + const installButton = Array.from(host.querySelectorAll('button')).find( + (button) => button.textContent === 'Install' + ) as HTMLButtonElement; + expect(installButton.disabled).toBe(false); + + await act(async () => { + installButton.click(); + await Promise.resolve(); + }); + + expect(storeState.installCustomMcpServer).toHaveBeenCalledWith( + expect.objectContaining({ + serverName: 'local-context7', + scope: 'local', + projectPath, + }) + ); + expect(onClose).toHaveBeenCalledTimes(1); + + await act(async () => { + root.unmount(); + await Promise.resolve(); + }); + }); }); diff --git a/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts b/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts index 356415e9..262161bd 100644 --- a/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts +++ b/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts @@ -217,7 +217,7 @@ describe('McpServerDetailDialog installed entry handling', () => { 'io.github.upstash/context7', 'context7-local', 'local', - undefined + '/tmp/project' ); await act(async () => { @@ -258,6 +258,10 @@ describe('McpServerDetailDialog installed entry handling', () => { expect(lookupMock).toHaveBeenCalledTimes(1); expect(lookupMock).toHaveBeenCalledWith(['CONTEXT7_API_KEY']); + const projectOption = host.querySelector('option[value="project"]') as HTMLOptionElement; + const localOption = host.querySelector('option[value="local"]') as HTMLOptionElement; + expect(projectOption.disabled).toBe(true); + expect(localOption.disabled).toBe(true); await act(async () => { root.unmount(); @@ -352,6 +356,90 @@ describe('McpServerDetailDialog installed entry handling', () => { }); }); + it('passes project path for local-scoped installs and uninstalls', async () => { + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + const projectPath = '/tmp/local-context7'; + const installedEntry: InstalledMcpEntry = { + name: 'context7-local', + scope: 'local', + }; + + await act(async () => { + root.render( + React.createElement(McpServerDetailDialog, { + server: makeServer(), + isInstalled: true, + installedEntry, + installedEntries: [installedEntry], + diagnostic: null, + diagnosticsLoading: false, + projectPath, + open: true, + onClose: vi.fn(), + }) + ); + await Promise.resolve(); + }); + + const uninstallButton = host.querySelector('[data-testid="install-button"]') as HTMLButtonElement; + await act(async () => { + uninstallButton.click(); + await Promise.resolve(); + }); + + expect(storeState.uninstallMcpServer).toHaveBeenCalledWith( + 'io.github.upstash/context7', + 'context7-local', + 'local', + projectPath + ); + + await act(async () => { + root.render( + React.createElement(McpServerDetailDialog, { + server: makeServer(), + isInstalled: false, + installedEntry: null, + installedEntries: [], + diagnostic: null, + diagnosticsLoading: false, + projectPath, + open: true, + onClose: vi.fn(), + }) + ); + await Promise.resolve(); + }); + + const scopeSelect = host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement; + await act(async () => { + scopeSelect.value = 'local'; + scopeSelect.dispatchEvent(new Event('change', { bubbles: true })); + await Promise.resolve(); + }); + + const installButton = host.querySelector('[data-testid="install-button"]') as HTMLButtonElement; + await act(async () => { + installButton.click(); + await Promise.resolve(); + }); + + expect(storeState.installMcpServer).toHaveBeenCalledWith( + expect.objectContaining({ + registryId: 'io.github.upstash/context7', + scope: 'local', + projectPath, + }) + ); + + await act(async () => { + root.unmount(); + await Promise.resolve(); + }); + }); + it('uses selected scope instead of aggregated installed state', async () => { const host = document.createElement('div'); document.body.appendChild(host);