fix(extensions): make mcp scope actions scope-aware
This commit is contained in:
parent
be8f4f45d2
commit
7418643dc9
7 changed files with 254 additions and 27 deletions
|
|
@ -11,7 +11,10 @@ import { Button } from '@renderer/components/ui/button';
|
|||
import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/components/ui/tooltip';
|
||||
import { useStore } from '@renderer/store';
|
||||
import { formatCompactNumber, formatRelativeTime } from '@renderer/utils/formatters';
|
||||
import { sanitizeMcpServerName } from '@shared/utils/extensionNormalizers';
|
||||
import {
|
||||
getMcpInstallationSummaryLabel,
|
||||
sanitizeMcpServerName,
|
||||
} from '@shared/utils/extensionNormalizers';
|
||||
import { Clock, Cloud, Globe, KeyRound, Lock, Monitor, Star, Tag, Wrench } from 'lucide-react';
|
||||
import { Github as GithubIcon } from 'lucide-react';
|
||||
|
||||
|
|
@ -28,6 +31,7 @@ interface McpServerCardProps {
|
|||
server: McpCatalogItem;
|
||||
isInstalled: boolean;
|
||||
installedEntry?: InstalledMcpEntry | null;
|
||||
installedEntries?: InstalledMcpEntry[];
|
||||
diagnostic?: McpServerDiagnostic | null;
|
||||
diagnosticsLoading?: boolean;
|
||||
onClick: (serverId: string) => void;
|
||||
|
|
@ -37,6 +41,7 @@ export const McpServerCard = ({
|
|||
server,
|
||||
isInstalled,
|
||||
installedEntry,
|
||||
installedEntries = [],
|
||||
diagnostic,
|
||||
diagnosticsLoading,
|
||||
onClick,
|
||||
|
|
@ -49,17 +54,22 @@ export const McpServerCard = ({
|
|||
server.repositoryUrl ? s.mcpGitHubStars[server.repositoryUrl] : undefined
|
||||
);
|
||||
const canAutoInstall = !!server.installSpec;
|
||||
const normalizedInstalledEntries = installedEntries.length
|
||||
? installedEntries
|
||||
: installedEntry
|
||||
? [installedEntry]
|
||||
: [];
|
||||
const requiresConfiguration =
|
||||
server.installSpec?.type === 'http' ||
|
||||
server.envVars.length > 0 ||
|
||||
server.requiresAuth ||
|
||||
(server.authHeaders?.length ?? 0) > 0;
|
||||
const defaultServerName = sanitizeMcpServerName(server.name);
|
||||
const userInstallEntry =
|
||||
normalizedInstalledEntries.find((entry) => entry.scope === 'user') ?? null;
|
||||
const installSummaryLabel = getMcpInstallationSummaryLabel(normalizedInstalledEntries);
|
||||
const supportsDirectInstalledAction =
|
||||
isInstalled &&
|
||||
installedEntry?.scope === 'user' &&
|
||||
installedEntry.name === defaultServerName &&
|
||||
!requiresConfiguration;
|
||||
isInstalled && userInstallEntry?.name === defaultServerName && !requiresConfiguration;
|
||||
const shouldShowDirectInstallButton =
|
||||
canAutoInstall && (!isInstalled ? !requiresConfiguration : supportsDirectInstalledAction);
|
||||
const [imgError, setImgError] = useState(false);
|
||||
|
|
@ -117,7 +127,7 @@ export const McpServerCard = ({
|
|||
className="border-emerald-500/30 bg-emerald-500/10 text-emerald-400"
|
||||
variant="outline"
|
||||
>
|
||||
Installed
|
||||
{installSummaryLabel ?? 'Installed'}
|
||||
</Badge>
|
||||
)}
|
||||
{isInstalled && diagnosticsLoading && !diagnostic && (
|
||||
|
|
@ -253,7 +263,7 @@ export const McpServerCard = ({
|
|||
})
|
||||
}
|
||||
onUninstall={() =>
|
||||
uninstallMcpServer(server.id, installedEntry?.name ?? defaultServerName, 'user')
|
||||
uninstallMcpServer(server.id, userInstallEntry?.name ?? defaultServerName, 'user')
|
||||
}
|
||||
size="sm"
|
||||
errorMessage={installError}
|
||||
|
|
|
|||
|
|
@ -25,7 +25,11 @@ import {
|
|||
SelectValue,
|
||||
} from '@renderer/components/ui/select';
|
||||
import { useStore } from '@renderer/store';
|
||||
import { sanitizeMcpServerName } from '@shared/utils/extensionNormalizers';
|
||||
import {
|
||||
getMcpInstallationSummaryLabel,
|
||||
getPreferredMcpInstallationEntry,
|
||||
sanitizeMcpServerName,
|
||||
} from '@shared/utils/extensionNormalizers';
|
||||
import { ExternalLink, Lock, Plus, Star, Trash2, Wrench } from 'lucide-react';
|
||||
|
||||
import { InstallButton } from '../common/InstallButton';
|
||||
|
|
@ -42,6 +46,7 @@ interface McpServerDetailDialogProps {
|
|||
server: McpCatalogItem | null;
|
||||
isInstalled: boolean;
|
||||
installedEntry?: InstalledMcpEntry | null;
|
||||
installedEntries?: InstalledMcpEntry[];
|
||||
diagnostic?: McpServerDiagnostic | null;
|
||||
diagnosticsLoading?: boolean;
|
||||
projectPath: string | null;
|
||||
|
|
@ -61,6 +66,7 @@ export const McpServerDetailDialog = ({
|
|||
server,
|
||||
isInstalled,
|
||||
installedEntry,
|
||||
installedEntries = [],
|
||||
diagnostic,
|
||||
diagnosticsLoading,
|
||||
projectPath,
|
||||
|
|
@ -83,6 +89,15 @@ export const McpServerDetailDialog = ({
|
|||
const [headers, setHeaders] = useState<McpHeaderDef[]>([]);
|
||||
const [imgError, setImgError] = useState(false);
|
||||
const [autoFilledFields, setAutoFilledFields] = useState<Set<string>>(new Set());
|
||||
const normalizedInstalledEntries = installedEntries.length
|
||||
? installedEntries
|
||||
: installedEntry
|
||||
? [installedEntry]
|
||||
: [];
|
||||
const preferredInstalledEntry = getPreferredMcpInstallationEntry(normalizedInstalledEntries);
|
||||
const selectedInstalledEntry =
|
||||
normalizedInstalledEntries.find((entry) => entry.scope === scope) ?? null;
|
||||
const installSummaryLabel = getMcpInstallationSummaryLabel(normalizedInstalledEntries);
|
||||
|
||||
// Initialize form when dialog opens or server changes
|
||||
useEffect(() => {
|
||||
|
|
@ -102,11 +117,19 @@ export const McpServerDetailDialog = ({
|
|||
locked: true,
|
||||
}))
|
||||
);
|
||||
setServerName(installedEntry?.name ?? sanitizeMcpServerName(server.name));
|
||||
setScope(installedEntry?.scope ?? 'user');
|
||||
setServerName(preferredInstalledEntry?.name ?? sanitizeMcpServerName(server.name));
|
||||
setScope(preferredInstalledEntry?.scope ?? 'user');
|
||||
setImgError(false);
|
||||
setAutoFilledFields(new Set());
|
||||
}, [installedEntry?.name, installedEntry?.scope, open, server?.id]);
|
||||
}, [open, preferredInstalledEntry?.name, preferredInstalledEntry?.scope, server?.id]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!server || !open) {
|
||||
return;
|
||||
}
|
||||
|
||||
setServerName(selectedInstalledEntry?.name ?? sanitizeMcpServerName(server.name));
|
||||
}, [open, scope, selectedInstalledEntry?.name, server]);
|
||||
|
||||
useEffect(() => {
|
||||
if (open && scope === 'project' && !projectPath) {
|
||||
|
|
@ -153,10 +176,10 @@ export const McpServerDetailDialog = ({
|
|||
const missingRequiredHeaders = headers.some(
|
||||
(header) => header.isRequired && !header.value.trim()
|
||||
);
|
||||
const uninstallServerName = installedEntry?.name ?? serverName;
|
||||
const uninstallScope = installedEntry?.scope ?? scope;
|
||||
const scopeRequiresProjectPath =
|
||||
(scope === 'project' || uninstallScope === 'project') && !projectPath;
|
||||
const isInstalledForScope = selectedInstalledEntry !== null;
|
||||
const uninstallServerName = selectedInstalledEntry?.name ?? serverName;
|
||||
const uninstallScope = selectedInstalledEntry?.scope ?? scope;
|
||||
const scopeRequiresProjectPath = scope === 'project' && !projectPath;
|
||||
const installDisabled =
|
||||
!serverName.trim() ||
|
||||
missingRequiredEnvVars ||
|
||||
|
|
@ -231,7 +254,7 @@ export const McpServerDetailDialog = ({
|
|||
className="border-emerald-500/30 bg-emerald-500/10 text-emerald-400"
|
||||
variant="outline"
|
||||
>
|
||||
Installed
|
||||
{installSummaryLabel ?? 'Installed'}
|
||||
</Badge>
|
||||
)}
|
||||
{server.source !== 'official' && <SourceBadge source={server.source} />}
|
||||
|
|
@ -325,7 +348,7 @@ export const McpServerDetailDialog = ({
|
|||
does not describe them. If connection fails after install, check the provider docs.
|
||||
</div>
|
||||
)}
|
||||
{(isInstalled || diagnosticsLoading) && (
|
||||
{isInstalledForScope && (
|
||||
<div className="space-y-2 rounded-md border border-border bg-surface-raised px-4 py-3">
|
||||
<div className="flex items-center justify-between gap-3">
|
||||
<span className="text-sm font-medium text-text">Claude Status</span>
|
||||
|
|
@ -364,7 +387,7 @@ export const McpServerDetailDialog = ({
|
|||
{canAutoInstall && (
|
||||
<div className="space-y-3 rounded-md border border-border bg-surface-raised p-4">
|
||||
<h4 className="text-sm font-medium text-text">
|
||||
{isInstalled ? 'Manage Installation' : 'Install Server'}
|
||||
{isInstalledForScope ? 'Manage Installation' : 'Install Server'}
|
||||
</h4>
|
||||
|
||||
{/* Server name */}
|
||||
|
|
@ -378,7 +401,7 @@ export const McpServerDetailDialog = ({
|
|||
onChange={(e) => setServerName(e.target.value)}
|
||||
placeholder="my-server"
|
||||
className="h-8 text-sm"
|
||||
disabled={isInstalled}
|
||||
disabled={isInstalledForScope}
|
||||
/>
|
||||
</div>
|
||||
|
||||
|
|
@ -502,7 +525,7 @@ export const McpServerDetailDialog = ({
|
|||
<div className="flex justify-end pt-1">
|
||||
<InstallButton
|
||||
state={installProgress}
|
||||
isInstalled={isInstalled}
|
||||
isInstalled={isInstalledForScope}
|
||||
onInstall={handleInstall}
|
||||
onUninstall={handleUninstall}
|
||||
disabled={installDisabled}
|
||||
|
|
|
|||
|
|
@ -16,7 +16,10 @@ import {
|
|||
import { useStore } from '@renderer/store';
|
||||
import { formatRelativeTime } from '@renderer/utils/formatters';
|
||||
import { CLI_NOT_FOUND_MARKER } from '@shared/constants/cli';
|
||||
import { sanitizeMcpServerName } from '@shared/utils/extensionNormalizers';
|
||||
import {
|
||||
getPreferredMcpInstallationEntry,
|
||||
sanitizeMcpServerName,
|
||||
} from '@shared/utils/extensionNormalizers';
|
||||
import { AlertTriangle, RefreshCw, Search, Server } from 'lucide-react';
|
||||
import { useShallow } from 'zustand/react/shallow';
|
||||
|
||||
|
|
@ -138,17 +141,24 @@ export const McpServersPanel = ({
|
|||
[installedServers]
|
||||
);
|
||||
|
||||
const installedEntriesByName = useMemo(
|
||||
() => new Map(installedServers.map((entry) => [entry.name.toLowerCase(), entry] as const)),
|
||||
[installedServers]
|
||||
);
|
||||
const installedEntriesByName = useMemo(() => {
|
||||
const entriesByName = new Map<string, InstalledMcpEntry[]>();
|
||||
for (const entry of installedServers) {
|
||||
const key = entry.name.toLowerCase();
|
||||
entriesByName.set(key, [...(entriesByName.get(key) ?? []), entry]);
|
||||
}
|
||||
return entriesByName;
|
||||
}, [installedServers]);
|
||||
|
||||
/** Check if a catalog server is installed by comparing sanitized names */
|
||||
const isServerInstalled = (server: McpCatalogItem): boolean =>
|
||||
installedNames.has(sanitizeMcpServerName(server.name));
|
||||
|
||||
const getInstalledEntries = (server: McpCatalogItem): InstalledMcpEntry[] =>
|
||||
installedEntriesByName.get(sanitizeMcpServerName(server.name)) ?? [];
|
||||
|
||||
const getInstalledEntry = (server: McpCatalogItem): InstalledMcpEntry | null =>
|
||||
installedEntriesByName.get(sanitizeMcpServerName(server.name)) ?? null;
|
||||
getPreferredMcpInstallationEntry(getInstalledEntries(server));
|
||||
|
||||
const getDiagnostic = (server: McpCatalogItem): McpServerDiagnostic | null => {
|
||||
const installedEntry = getInstalledEntry(server);
|
||||
|
|
@ -377,6 +387,7 @@ export const McpServersPanel = ({
|
|||
server={server}
|
||||
isInstalled={isServerInstalled(server)}
|
||||
installedEntry={getInstalledEntry(server)}
|
||||
installedEntries={getInstalledEntries(server)}
|
||||
diagnostic={getDiagnostic(server)}
|
||||
diagnosticsLoading={mcpDiagnosticsLoading}
|
||||
onClick={setSelectedMcpServerId}
|
||||
|
|
@ -404,6 +415,7 @@ export const McpServersPanel = ({
|
|||
server={selectedServer}
|
||||
isInstalled={selectedServer ? isServerInstalled(selectedServer) : false}
|
||||
installedEntry={selectedServer ? getInstalledEntry(selectedServer) : null}
|
||||
installedEntries={selectedServer ? getInstalledEntries(selectedServer) : []}
|
||||
diagnostic={selectedServer ? getDiagnostic(selectedServer) : null}
|
||||
diagnosticsLoading={mcpDiagnosticsLoading}
|
||||
projectPath={projectPath}
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@
|
|||
import type {
|
||||
CliInstallationStatus,
|
||||
InstallScope,
|
||||
InstalledMcpEntry,
|
||||
InstalledPluginEntry,
|
||||
PluginCapability,
|
||||
PluginCatalogItem,
|
||||
|
|
@ -144,6 +145,55 @@ export function getInstallationSummaryLabel(
|
|||
}
|
||||
}
|
||||
|
||||
const MCP_SCOPE_PRIORITY: Record<InstalledMcpEntry['scope'], number> = {
|
||||
user: 0,
|
||||
project: 1,
|
||||
local: 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.
|
||||
*/
|
||||
export function getPreferredMcpInstallationEntry(
|
||||
installations: InstalledMcpEntry[]
|
||||
): InstalledMcpEntry | null {
|
||||
if (installations.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return [...installations].sort(
|
||||
(left, right) => MCP_SCOPE_PRIORITY[left.scope] - MCP_SCOPE_PRIORITY[right.scope]
|
||||
)[0]!;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a concise install-status label for MCP badges.
|
||||
*/
|
||||
export function getMcpInstallationSummaryLabel(
|
||||
installations: Pick<InstalledMcpEntry, 'scope'>[]
|
||||
): string | null {
|
||||
const scopes = Array.from(new Set(installations.map((installation) => installation.scope)));
|
||||
if (scopes.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (scopes.length > 1) {
|
||||
return `Installed in ${scopes.length} scopes`;
|
||||
}
|
||||
|
||||
switch (scopes[0]) {
|
||||
case 'user':
|
||||
return 'Installed globally';
|
||||
case 'project':
|
||||
return 'Installed in project';
|
||||
case 'local':
|
||||
return 'Installed locally';
|
||||
default:
|
||||
return 'Installed';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Install actions require Claude auth, but uninstall only requires a working CLI.
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -175,6 +175,7 @@ describe('McpServerCard direct action safety', () => {
|
|||
server: makeServer(),
|
||||
isInstalled: true,
|
||||
installedEntry,
|
||||
installedEntries: [installedEntry],
|
||||
diagnostic: null,
|
||||
diagnosticsLoading: false,
|
||||
onClick: vi.fn(),
|
||||
|
|
@ -190,4 +191,37 @@ describe('McpServerCard direct action safety', () => {
|
|||
await Promise.resolve();
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps direct user action when the same server is installed in multiple scopes', async () => {
|
||||
const host = document.createElement('div');
|
||||
document.body.appendChild(host);
|
||||
const root = createRoot(host);
|
||||
const installedEntries: InstalledMcpEntry[] = [
|
||||
{ name: 'context7', scope: 'user' },
|
||||
{ name: 'context7', scope: 'project' },
|
||||
];
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(McpServerCard, {
|
||||
server: makeServer(),
|
||||
isInstalled: true,
|
||||
installedEntry: installedEntries[1],
|
||||
installedEntries,
|
||||
diagnostic: null,
|
||||
diagnosticsLoading: false,
|
||||
onClick: vi.fn(),
|
||||
})
|
||||
);
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(host.textContent).toContain('Installed in 2 scopes');
|
||||
expect(host.querySelector('[data-testid="install-button"]')).not.toBeNull();
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
await Promise.resolve();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -281,6 +281,7 @@ describe('McpServerDetailDialog installed entry handling', () => {
|
|||
server: makeServer(),
|
||||
isInstalled: true,
|
||||
installedEntry,
|
||||
installedEntries: [installedEntry],
|
||||
diagnostic: null,
|
||||
diagnosticsLoading: false,
|
||||
projectPath,
|
||||
|
|
@ -313,6 +314,7 @@ describe('McpServerDetailDialog installed entry handling', () => {
|
|||
server: makeServer(),
|
||||
isInstalled: false,
|
||||
installedEntry: null,
|
||||
installedEntries: [],
|
||||
diagnostic: null,
|
||||
diagnosticsLoading: false,
|
||||
projectPath,
|
||||
|
|
@ -349,4 +351,60 @@ describe('McpServerDetailDialog installed entry handling', () => {
|
|||
await Promise.resolve();
|
||||
});
|
||||
});
|
||||
|
||||
it('uses selected scope instead of aggregated installed state', async () => {
|
||||
const host = document.createElement('div');
|
||||
document.body.appendChild(host);
|
||||
const root = createRoot(host);
|
||||
const installedEntry: InstalledMcpEntry = {
|
||||
name: 'context7',
|
||||
scope: 'user',
|
||||
};
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(McpServerDetailDialog, {
|
||||
server: makeServer(),
|
||||
isInstalled: true,
|
||||
installedEntry,
|
||||
installedEntries: [installedEntry],
|
||||
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;
|
||||
await act(async () => {
|
||||
scopeSelect.value = 'project';
|
||||
scopeSelect.dispatchEvent(new Event('change', { bubbles: true }));
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
const actionButton = host.querySelector('[data-testid="install-button"]') as HTMLButtonElement;
|
||||
expect(actionButton.textContent).toBe('Install');
|
||||
|
||||
await act(async () => {
|
||||
actionButton.click();
|
||||
await Promise.resolve();
|
||||
});
|
||||
|
||||
expect(storeState.installMcpServer).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
registryId: 'io.github.upstash/context7',
|
||||
scope: 'project',
|
||||
projectPath: '/tmp/project',
|
||||
})
|
||||
);
|
||||
expect(storeState.uninstallMcpServer).not.toHaveBeenCalled();
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
await Promise.resolve();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import type { PluginCatalogItem } from '@shared/types/extensions';
|
||||
import type { InstalledMcpEntry, PluginCatalogItem } from '@shared/types/extensions';
|
||||
|
||||
import {
|
||||
buildPluginId,
|
||||
|
|
@ -8,6 +8,8 @@ import {
|
|||
getExtensionActionDisableReason,
|
||||
getCapabilityLabel,
|
||||
getInstallationSummaryLabel,
|
||||
getMcpInstallationSummaryLabel,
|
||||
getPreferredMcpInstallationEntry,
|
||||
getPluginOperationKey,
|
||||
getPrimaryCapabilityLabel,
|
||||
hasInstallationInScope,
|
||||
|
|
@ -202,6 +204,44 @@ describe('getInstallationSummaryLabel', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('getPreferredMcpInstallationEntry', () => {
|
||||
it('returns null when there are no MCP installs', () => {
|
||||
expect(getPreferredMcpInstallationEntry([])).toBeNull();
|
||||
});
|
||||
|
||||
it('prefers user scope over project and local', () => {
|
||||
const installations: InstalledMcpEntry[] = [
|
||||
{ name: 'context7', scope: 'project' },
|
||||
{ name: 'context7', scope: 'user' },
|
||||
{ name: 'context7', scope: 'local' },
|
||||
];
|
||||
|
||||
expect(getPreferredMcpInstallationEntry(installations)).toEqual({
|
||||
name: 'context7',
|
||||
scope: 'user',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('getMcpInstallationSummaryLabel', () => {
|
||||
it('returns null when there are no MCP installations', () => {
|
||||
expect(getMcpInstallationSummaryLabel([])).toBeNull();
|
||||
});
|
||||
|
||||
it('describes a single local MCP installation', () => {
|
||||
expect(getMcpInstallationSummaryLabel([{ scope: 'local' }])).toBe('Installed locally');
|
||||
});
|
||||
|
||||
it('summarizes multiple MCP scopes', () => {
|
||||
expect(
|
||||
getMcpInstallationSummaryLabel([
|
||||
{ scope: 'user' },
|
||||
{ scope: 'project' },
|
||||
])
|
||||
).toBe('Installed in 2 scopes');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getExtensionActionDisableReason', () => {
|
||||
it('requires auth only for install actions', () => {
|
||||
expect(
|
||||
|
|
|
|||
Loading…
Reference in a new issue