From 88ac0acdf82369cce8c1839dcd733ccbfe170c15 Mon Sep 17 00:00:00 2001 From: 777genius Date: Fri, 17 Apr 2026 15:06:36 +0300 Subject: [PATCH] fix(extensions): preserve mcp dialog state on runtime hydration --- .../extensions/mcp/CustomMcpServerDialog.tsx | 28 ++++++- .../extensions/mcp/McpServerDetailDialog.tsx | 35 +++++--- src/shared/utils/mcpScopes.ts | 4 + .../mcp/CustomMcpServerDialog.test.ts | 57 +++++++++++++ .../mcp/McpServerDetailDialog.test.ts | 82 +++++++++++++++++++ 5 files changed, 195 insertions(+), 11 deletions(-) diff --git a/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx b/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx index 4bfa4c0b..727d2603 100644 --- a/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx +++ b/src/renderer/components/extensions/mcp/CustomMcpServerDialog.tsx @@ -29,6 +29,7 @@ import { getDefaultMcpSharedScope, getMcpScopeLabel, isProjectScopedMcpScope, + isSharedMcpScope, } from '@shared/utils/mcpScopes'; import { Plus, Server, Trash2 } from 'lucide-react'; @@ -95,6 +96,8 @@ export const CustomMcpServerDialog = ({ const [error, setError] = useState(null); const [installing, setInstalling] = useState(false); const autoFilledValuesRef = useRef>({}); + const wasOpenRef = useRef(false); + const previousDefaultSharedScopeRef = useRef(defaultSharedScope); const envVarLookupNames = envVars .map((entry) => entry.key.trim()) .filter(Boolean) @@ -112,7 +115,8 @@ export const CustomMcpServerDialog = ({ // Reset on open useEffect(() => { - if (open) { + const justOpened = open && !wasOpenRef.current; + if (justOpened) { setServerName(''); setTransportMode('stdio'); setScope(defaultSharedScope); @@ -126,8 +130,30 @@ export const CustomMcpServerDialog = ({ setInstalling(false); autoFilledValuesRef.current = {}; } + wasOpenRef.current = open; + if (!open) { + previousDefaultSharedScopeRef.current = defaultSharedScope; + } }, [defaultSharedScope, open]); + useEffect(() => { + if (!open) { + previousDefaultSharedScopeRef.current = defaultSharedScope; + return; + } + + const previousDefaultSharedScope = previousDefaultSharedScopeRef.current; + if ( + previousDefaultSharedScope !== defaultSharedScope && + scope === previousDefaultSharedScope && + isSharedMcpScope(scope) + ) { + setScope(defaultSharedScope); + } + + previousDefaultSharedScopeRef.current = defaultSharedScope; + }, [defaultSharedScope, open, scope]); + useEffect(() => { if (open && isProjectScopedMcpScope(scope) && !projectPath) { setScope(defaultSharedScope); diff --git a/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx b/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx index 10fb5579..54d61b32 100644 --- a/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx +++ b/src/renderer/components/extensions/mcp/McpServerDetailDialog.tsx @@ -29,6 +29,7 @@ import { getDefaultMcpSharedScope, getMcpScopeLabel, isProjectScopedMcpScope, + isSharedMcpScope, } from '@shared/utils/mcpScopes'; import { getMcpInstallationSummaryLabel, @@ -93,6 +94,7 @@ export const McpServerDetailDialog = ({ const [imgError, setImgError] = useState(false); const [autoFilledFields, setAutoFilledFields] = useState>(new Set()); const autoFilledValuesRef = useRef>({}); + const previousDefaultSharedScopeRef = useRef(defaultSharedScope); const normalizedInstalledEntries = installedEntries.length ? installedEntries : installedEntry @@ -143,21 +145,34 @@ export const McpServerDetailDialog = ({ setImgError(false); setAutoFilledFields(new Set()); autoFilledValuesRef.current = {}; - }, [ - defaultSharedScope, - open, - preferredInstalledEntry?.name, - preferredInstalledEntry?.scope, - server?.id, - ]); + }, [open, preferredInstalledEntry?.name, preferredInstalledEntry?.scope, server?.id]); useEffect(() => { - if (!server || !open) { + if (!open) { + previousDefaultSharedScopeRef.current = defaultSharedScope; return; } - setServerName(selectedInstalledEntry?.name ?? sanitizeMcpServerName(server.name)); - }, [open, scope, selectedInstalledEntry?.name, server]); + const previousDefaultSharedScope = previousDefaultSharedScopeRef.current; + if ( + previousDefaultSharedScope !== defaultSharedScope && + !preferredInstalledEntry && + scope === previousDefaultSharedScope && + isSharedMcpScope(scope) + ) { + setScope(defaultSharedScope); + } + + previousDefaultSharedScopeRef.current = defaultSharedScope; + }, [defaultSharedScope, open, preferredInstalledEntry, scope]); + + useEffect(() => { + if (!server || !open || !selectedInstalledEntry) { + return; + } + + setServerName(selectedInstalledEntry.name); + }, [open, selectedInstalledEntry, server]); useEffect(() => { if (open && isProjectScopedMcpScope(scope) && !projectPath) { diff --git a/src/shared/utils/mcpScopes.ts b/src/shared/utils/mcpScopes.ts index 44903816..967357f3 100644 --- a/src/shared/utils/mcpScopes.ts +++ b/src/shared/utils/mcpScopes.ts @@ -8,6 +8,10 @@ export function getDefaultMcpSharedScope(flavor?: CliFlavor | null): McpSharedSc return flavor === 'agent_teams_orchestrator' ? 'global' : 'user'; } +export function isSharedMcpScope(scope?: string): scope is McpSharedScope { + return scope === 'user' || scope === 'global'; +} + export function isProjectScopedMcpScope(scope?: string): scope is 'project' | 'local' { return scope === 'project' || scope === 'local'; } diff --git a/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts b/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts index c8adaada..be0331ae 100644 --- a/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts +++ b/test/renderer/components/extensions/mcp/CustomMcpServerDialog.test.ts @@ -189,6 +189,63 @@ describe('CustomMcpServerDialog project scope', () => { }); }); + it('preserves entered values when multimodel scope metadata loads after open', async () => { + storeState.cliStatus = null; + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + + await act(async () => { + root.render( + React.createElement(CustomMcpServerDialog, { + open: true, + onClose: vi.fn(), + projectPath: null, + }) + ); + 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, 'late-hydration-server', 'input'); + setNativeValue(packageInput, '@example/late-hydration', 'input'); + await Promise.resolve(); + }); + + expect(scopeSelect.value).toBe('user'); + + storeState.cliStatus = { flavor: 'agent_teams_orchestrator' }; + await act(async () => { + root.render( + React.createElement(CustomMcpServerDialog, { + open: true, + onClose: vi.fn(), + projectPath: null, + }) + ); + await Promise.resolve(); + }); + + expect((host.querySelector('#custom-name') as HTMLInputElement).value).toBe( + 'late-hydration-server' + ); + expect((host.querySelector('#custom-npm') as HTMLInputElement).value).toBe( + '@example/late-hydration' + ); + expect((host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement).value).toBe( + 'global' + ); + + await act(async () => { + root.unmount(); + await Promise.resolve(); + }); + }); + it('disables installation when the runtime declares MCP writes unavailable', async () => { storeState.cliStatus = { flavor: 'agent_teams_orchestrator', diff --git a/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts b/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts index 14fe2b8c..d8fc52c0 100644 --- a/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts +++ b/test/renderer/components/extensions/mcp/McpServerDetailDialog.test.ts @@ -165,6 +165,15 @@ function makeServer(): McpCatalogItem { }; } +function setInputValue(element: HTMLInputElement | HTMLSelectElement, value: string): void { + const prototype = + element instanceof HTMLSelectElement ? HTMLSelectElement.prototype : HTMLInputElement.prototype; + const descriptor = Object.getOwnPropertyDescriptor(prototype, 'value'); + descriptor?.set?.call(element, value); + element.dispatchEvent(new Event('input', { bubbles: true })); + element.dispatchEvent(new Event('change', { bubbles: true })); +} + describe('McpServerDetailDialog installed entry handling', () => { beforeEach(() => { vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true); @@ -416,6 +425,79 @@ describe('McpServerDetailDialog installed entry handling', () => { }); }); + it('preserves edited fields when multimodel scope metadata loads after open', async () => { + storeState.cliStatus = null; + const host = document.createElement('div'); + document.body.appendChild(host); + const root = createRoot(host); + const server = makeServer(); + server.envVars = [{ name: 'CONTEXT7_API_KEY', isSecret: true }]; + + await act(async () => { + root.render( + React.createElement(McpServerDetailDialog, { + server, + isInstalled: false, + installedEntry: null, + installedEntries: [], + diagnostic: null, + diagnosticsLoading: false, + projectPath: null, + open: true, + onClose: vi.fn(), + }) + ); + await Promise.resolve(); + await Promise.resolve(); + }); + + const serverNameInput = host.querySelector('#server-name') as HTMLInputElement; + const envValueInput = host.querySelector('input[type="password"]') as HTMLInputElement; + const scopeSelect = host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement; + + await act(async () => { + setInputValue(serverNameInput, 'late-hydration-context7'); + setInputValue(envValueInput, 'secret'); + await Promise.resolve(); + }); + + expect(scopeSelect.value).toBe('user'); + + storeState.cliStatus = { flavor: 'agent_teams_orchestrator' }; + await act(async () => { + root.render( + React.createElement(McpServerDetailDialog, { + server, + isInstalled: false, + installedEntry: null, + installedEntries: [], + diagnostic: null, + diagnosticsLoading: false, + projectPath: null, + open: true, + onClose: vi.fn(), + }) + ); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect((host.querySelector('#server-name') as HTMLInputElement).value).toBe( + 'late-hydration-context7' + ); + expect((host.querySelector('input[type="password"]') as HTMLInputElement).value).toBe( + 'secret' + ); + expect((host.querySelector('[data-testid="scope-select"]') as HTMLSelectElement).value).toBe( + 'global' + ); + + await act(async () => { + root.unmount(); + await Promise.resolve(); + }); + }); + it('passes project path for project-scoped installs and uninstalls', async () => { const host = document.createElement('div'); document.body.appendChild(host);