diff --git a/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx b/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx index bba98439..66fc266d 100644 --- a/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx +++ b/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx @@ -3,7 +3,7 @@ * Supports stdio (npm package) and HTTP/SSE transports. */ -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { api } from '@renderer/api'; import { Button } from '@renderer/components/ui/button'; @@ -92,11 +92,15 @@ export const CustomMcpServerDialog = ({ const [envVars, setEnvVars] = useState([]); const [error, setError] = useState(null); const [installing, setInstalling] = useState(false); + const autoFilledValuesRef = useRef>({}); const envVarLookupNames = envVars .map((entry) => entry.key.trim()) .filter(Boolean) .sort() .join('\0'); + const apiKeyLookupProjectPath = isProjectScopedMcpScope(scope) + ? (projectPath ?? undefined) + : undefined; // Reset on open useEffect(() => { @@ -112,6 +116,7 @@ export const CustomMcpServerDialog = ({ setEnvVars([]); setError(null); setInstalling(false); + autoFilledValuesRef.current = {}; } }, [defaultSharedScope, open]); @@ -128,19 +133,50 @@ export const CustomMcpServerDialog = ({ const envVarNames = envVars.map((e) => e.key.trim()).filter(Boolean); if (envVarNames.length === 0) return; - void api.apiKeys.lookup(envVarNames, projectPath ?? undefined).then( + void api.apiKeys.lookup(envVarNames, apiKeyLookupProjectPath).then( (results) => { - if (results.length === 0) return; - const lookup = new Map(results.map((r) => [r.envVarName, r.value])); - setEnvVars((prev) => - prev.map((e) => (lookup.has(e.key) && !e.value ? { ...e, value: lookup.get(e.key)! } : e)) + const previousAutoFilledValues = autoFilledValuesRef.current; + const nextAutoFilledValues = Object.fromEntries( + results.map((result) => [result.envVarName, result.value]) ); + setEnvVars((prev) => { + let changed = false; + const next = prev.map((entry) => { + const envVarName = entry.key.trim(); + if (!envVarName) { + return entry; + } + + const previousValue = previousAutoFilledValues[envVarName]; + const nextValue = nextAutoFilledValues[envVarName]; + + if (!nextValue) { + if (previousValue && entry.value === previousValue) { + changed = true; + return { ...entry, value: '' }; + } + return entry; + } + + if (!entry.value || entry.value === previousValue) { + if (entry.value !== nextValue) { + changed = true; + return { ...entry, value: nextValue }; + } + } + + return entry; + }); + + return changed ? next : prev; + }); + autoFilledValuesRef.current = nextAutoFilledValues; }, () => { // Silently fail } ); - }, [envVarLookupNames, envVars, open, projectPath]); // eslint-disable-line react-hooks/exhaustive-deps + }, [apiKeyLookupProjectPath, envVarLookupNames, open]); // eslint-disable-line react-hooks/exhaustive-deps const handleInstall = async () => { setError(null); diff --git a/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx b/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx index 8ac44ee7..10fb5579 100644 --- a/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx +++ b/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx @@ -3,7 +3,7 @@ * Uses Radix UI Kit for all form elements. */ -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { api } from '@renderer/api'; import { Badge } from '@renderer/components/ui/badge'; @@ -92,6 +92,7 @@ export const McpServerDetailDialog = ({ const [headers, setHeaders] = useState([]); const [imgError, setImgError] = useState(false); const [autoFilledFields, setAutoFilledFields] = useState>(new Set()); + const autoFilledValuesRef = useRef>({}); const normalizedInstalledEntries = installedEntries.length ? installedEntries : installedEntry @@ -115,6 +116,9 @@ export const McpServerDetailDialog = ({ .map((entry) => entry.name) .sort() .join('\0') ?? ''; + const apiKeyLookupProjectPath = isProjectScopedMcpScope(scope) + ? (projectPath ?? undefined) + : undefined; // Initialize form when dialog opens or server changes useEffect(() => { @@ -138,6 +142,7 @@ export const McpServerDetailDialog = ({ setScope((preferredInstalledEntry?.scope as Scope | undefined) ?? defaultSharedScope); setImgError(false); setAutoFilledFields(new Set()); + autoFilledValuesRef.current = {}; }, [ defaultSharedScope, open, @@ -165,23 +170,38 @@ export const McpServerDetailDialog = ({ if (!server || !open || server.envVars.length === 0 || !api.apiKeys) return; const envVarNames = server.envVars.map((e) => e.name); - void api.apiKeys.lookup(envVarNames, projectPath ?? undefined).then( + void api.apiKeys.lookup(envVarNames, apiKeyLookupProjectPath).then( (results) => { - if (results.length === 0) return; - const filled = new Set(); - const values: Record = {}; + const previousAutoFilledValues = autoFilledValuesRef.current; + const nextAutoFilledValues: Record = {}; for (const r of results) { - values[r.envVarName] = r.value; - filled.add(r.envVarName); + nextAutoFilledValues[r.envVarName] = r.value; } - setEnvValues((prev) => ({ ...prev, ...values })); - setAutoFilledFields(filled); + setEnvValues((prev) => { + const next = { ...prev }; + + for (const [envVarName, previousValue] of Object.entries(previousAutoFilledValues)) { + if (!(envVarName in nextAutoFilledValues) && next[envVarName] === previousValue) { + next[envVarName] = ''; + } + } + + for (const [envVarName, nextValue] of Object.entries(nextAutoFilledValues)) { + if (!next[envVarName] || next[envVarName] === previousAutoFilledValues[envVarName]) { + next[envVarName] = nextValue; + } + } + + return next; + }); + setAutoFilledFields(new Set(Object.keys(nextAutoFilledValues))); + autoFilledValuesRef.current = nextAutoFilledValues; }, () => { // Silently fail — auto-fill is supplementary } ); - }, [envVarLookupNames, open, projectPath, server?.id]); // eslint-disable-line react-hooks/exhaustive-deps + }, [apiKeyLookupProjectPath, envVarLookupNames, open, server?.id]); // eslint-disable-line react-hooks/exhaustive-deps if (!server) return <>; diff --git a/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts b/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts index 29e23faa..177ffa11 100644 --- a/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts +++ b/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts @@ -180,7 +180,7 @@ describe('CustomMcpServerDialog project scope', () => { }); }); - it('passes projectPath into API key lookup for project-aware autofill', async () => { + it('looks up project-scoped API keys only when project scope is selected', async () => { const host = document.createElement('div'); document.body.appendChild(host); const root = createRoot(host); @@ -213,7 +213,88 @@ describe('CustomMcpServerDialog project scope', () => { await Promise.resolve(); }); - expect(lookupMock).toHaveBeenCalledWith(['CONTEXT7_API_KEY'], '/tmp/custom-mcp-project'); + expect(lookupMock).toHaveBeenCalledWith(['CONTEXT7_API_KEY'], undefined); + + const scopeSelect = host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement; + await act(async () => { + setNativeValue(scopeSelect, 'project', 'change'); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(lookupMock).toHaveBeenLastCalledWith(['CONTEXT7_API_KEY'], '/tmp/custom-mcp-project'); + + await act(async () => { + setNativeValue(scopeSelect, 'user', 'change'); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(lookupMock).toHaveBeenLastCalledWith(['CONTEXT7_API_KEY'], undefined); + + await act(async () => { + root.unmount(); + await Promise.resolve(); + }); + }); + + it('clears stale project auto-filled values when switching back to user scope', async () => { + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + lookupMock + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ envVarName: 'CONTEXT7_API_KEY', value: 'project-secret' }]) + .mockResolvedValueOnce([]); + + await act(async () => { + root.render( + React.createElement(CustomMcpServerDialog, { + open: true, + onClose: vi.fn(), + projectPath: '/tmp/custom-mcp-project', + }) + ); + await Promise.resolve(); + }); + + const addEnvButton = Array.from(host.querySelectorAll('button')).find((button) => + button.textContent?.includes('Add') + ) as HTMLButtonElement; + await act(async () => { + addEnvButton.click(); + await Promise.resolve(); + }); + + const envKeyInput = host.querySelector( + 'input[placeholder="ENV_VAR_NAME"]' + ) as HTMLInputElement; + const envValueInput = host.querySelector( + 'input[placeholder="value"]' + ) as HTMLInputElement; + const scopeSelect = host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement; + + await act(async () => { + setNativeValue(envKeyInput, 'CONTEXT7_API_KEY', 'input'); + await Promise.resolve(); + await Promise.resolve(); + }); + + await act(async () => { + setNativeValue(scopeSelect, 'project', 'change'); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(envValueInput.value).toBe('project-secret'); + + await act(async () => { + setNativeValue(scopeSelect, 'user', 'change'); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(envValueInput.value).toBe(''); 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 18c78c6d..14fe2b8c 100644 --- a/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts +++ b/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts @@ -278,7 +278,7 @@ describe('McpServerDetailDialog installed entry handling', () => { }); }); - it('passes projectPath into API key lookup for project-aware autofill', async () => { + it('looks up project-scoped API keys only when project scope is selected', async () => { const host = document.createElement('div'); document.body.appendChild(host); const root = createRoot(host); @@ -302,7 +302,81 @@ describe('McpServerDetailDialog installed entry handling', () => { await Promise.resolve(); }); - expect(lookupMock).toHaveBeenCalledWith(['CONTEXT7_API_KEY'], '/tmp/project-context7'); + expect(lookupMock).toHaveBeenCalledWith(['CONTEXT7_API_KEY'], undefined); + + const scopeSelect = host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement; + await act(async () => { + scopeSelect.value = 'project'; + scopeSelect.dispatchEvent(new Event('change', { bubbles: true })); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(lookupMock).toHaveBeenLastCalledWith(['CONTEXT7_API_KEY'], '/tmp/project-context7'); + + await act(async () => { + scopeSelect.value = 'user'; + scopeSelect.dispatchEvent(new Event('change', { bubbles: true })); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(lookupMock).toHaveBeenLastCalledWith(['CONTEXT7_API_KEY'], undefined); + + await act(async () => { + root.unmount(); + await Promise.resolve(); + }); + }); + + it('clears stale project auto-filled values when switching back to user scope', async () => { + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + const server = makeServer(); + server.envVars = [{ name: 'CONTEXT7_API_KEY', isSecret: true }]; + lookupMock + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ envVarName: 'CONTEXT7_API_KEY', value: 'project-secret' }]) + .mockResolvedValueOnce([]); + + await act(async () => { + root.render( + React.createElement(McpServerDetailDialog, { + server, + isInstalled: false, + installedEntry: null, + diagnostic: null, + diagnosticsLoading: false, + projectPath: '/tmp/project-context7', + open: true, + onClose: vi.fn(), + }) + ); + await Promise.resolve(); + await Promise.resolve(); + }); + + const scopeSelect = host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement; + const envValueInput = host.querySelector('input[type="password"]') as HTMLInputElement; + + await act(async () => { + scopeSelect.value = 'project'; + scopeSelect.dispatchEvent(new Event('change', { bubbles: true })); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(envValueInput.value).toBe('project-secret'); + + await act(async () => { + scopeSelect.value = 'user'; + scopeSelect.dispatchEvent(new Event('change', { bubbles: true })); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(envValueInput.value).toBe(''); await act(async () => { root.unmount();