fix(extensions): require project context for local mcp scope
This commit is contained in:
parent
a3c5b7dca9
commit
94291f50f0
6 changed files with 205 additions and 17 deletions
|
|
@ -37,6 +37,10 @@ const HEADER_KEY_RE = /^[A-Za-z][\w-]{0,100}$/;
|
||||||
|
|
||||||
const TIMEOUT_MS = 30_000;
|
const TIMEOUT_MS = 30_000;
|
||||||
|
|
||||||
|
function scopeRequiresProjectPath(scope?: string): boolean {
|
||||||
|
return scope === 'local' || scope === 'project';
|
||||||
|
}
|
||||||
|
|
||||||
export class McpInstallService {
|
export class McpInstallService {
|
||||||
constructor(private readonly aggregator: McpCatalogAggregator) {}
|
constructor(private readonly aggregator: McpCatalogAggregator) {}
|
||||||
|
|
||||||
|
|
@ -59,10 +63,10 @@ export class McpInstallService {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
if (scope === 'project' && !projectPath) {
|
if (scopeRequiresProjectPath(scope) && !projectPath) {
|
||||||
return {
|
return {
|
||||||
state: 'error',
|
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}".` };
|
return { state: 'error', error: `Invalid scope: "${scope}".` };
|
||||||
}
|
}
|
||||||
|
|
||||||
if (scope === 'project' && !projectPath) {
|
if (scopeRequiresProjectPath(scope) && !projectPath) {
|
||||||
return { state: 'error', error: 'projectPath is required for project scope' };
|
return { state: 'error', error: `projectPath is required for ${scope} scope` };
|
||||||
}
|
}
|
||||||
|
|
||||||
for (const key of Object.keys(envValues)) {
|
for (const key of Object.keys(envValues)) {
|
||||||
|
|
@ -330,10 +334,10 @@ export class McpInstallService {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
if (scope === 'project' && !projectPath) {
|
if (scopeRequiresProjectPath(scope) && !projectPath) {
|
||||||
return {
|
return {
|
||||||
state: 'error',
|
state: 'error',
|
||||||
error: 'projectPath is required for project scope',
|
error: `projectPath is required for ${scope} scope`,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -105,7 +105,7 @@ export const CustomMcpServerDialog = ({
|
||||||
}, [open]);
|
}, [open]);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (open && scope === 'project' && !projectPath) {
|
if (open && scope !== 'user' && !projectPath) {
|
||||||
setScope('user');
|
setScope('user');
|
||||||
}
|
}
|
||||||
}, [open, projectPath, scope]);
|
}, [open, projectPath, scope]);
|
||||||
|
|
@ -177,7 +177,7 @@ export const CustomMcpServerDialog = ({
|
||||||
const request: McpCustomInstallRequest = {
|
const request: McpCustomInstallRequest = {
|
||||||
serverName,
|
serverName,
|
||||||
scope,
|
scope,
|
||||||
projectPath: scope === 'project' ? (projectPath ?? undefined) : undefined,
|
projectPath: scope !== 'user' ? (projectPath ?? undefined) : undefined,
|
||||||
installSpec,
|
installSpec,
|
||||||
envValues,
|
envValues,
|
||||||
headers: headers.filter((h) => h.key.trim() && h.value.trim()),
|
headers: headers.filter((h) => h.key.trim() && h.value.trim()),
|
||||||
|
|
@ -207,7 +207,7 @@ export const CustomMcpServerDialog = ({
|
||||||
const canSubmit =
|
const canSubmit =
|
||||||
serverName.trim() &&
|
serverName.trim() &&
|
||||||
(transportMode === 'stdio' ? npmPackage.trim() : httpUrl.trim()) &&
|
(transportMode === 'stdio' ? npmPackage.trim() : httpUrl.trim()) &&
|
||||||
!(scope === 'project' && !projectPath) &&
|
!(scope !== 'user' && !projectPath) &&
|
||||||
!installing;
|
!installing;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
|
|
@ -386,7 +386,7 @@ export const CustomMcpServerDialog = ({
|
||||||
<SelectItem
|
<SelectItem
|
||||||
key={opt.value}
|
key={opt.value}
|
||||||
value={opt.value}
|
value={opt.value}
|
||||||
disabled={opt.value === 'project' && !projectPath}
|
disabled={opt.value !== 'user' && !projectPath}
|
||||||
>
|
>
|
||||||
{opt.label}
|
{opt.label}
|
||||||
</SelectItem>
|
</SelectItem>
|
||||||
|
|
|
||||||
|
|
@ -132,7 +132,7 @@ export const McpServerDetailDialog = ({
|
||||||
}, [open, scope, selectedInstalledEntry?.name, server]);
|
}, [open, scope, selectedInstalledEntry?.name, server]);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (open && scope === 'project' && !projectPath) {
|
if (open && scope !== 'user' && !projectPath) {
|
||||||
setScope('user');
|
setScope('user');
|
||||||
}
|
}
|
||||||
}, [open, projectPath, scope]);
|
}, [open, projectPath, scope]);
|
||||||
|
|
@ -179,7 +179,7 @@ export const McpServerDetailDialog = ({
|
||||||
const isInstalledForScope = selectedInstalledEntry !== null;
|
const isInstalledForScope = selectedInstalledEntry !== null;
|
||||||
const uninstallServerName = selectedInstalledEntry?.name ?? serverName;
|
const uninstallServerName = selectedInstalledEntry?.name ?? serverName;
|
||||||
const uninstallScope = selectedInstalledEntry?.scope ?? scope;
|
const uninstallScope = selectedInstalledEntry?.scope ?? scope;
|
||||||
const scopeRequiresProjectPath = scope === 'project' && !projectPath;
|
const scopeRequiresProjectPath = scope !== 'user' && !projectPath;
|
||||||
const installDisabled =
|
const installDisabled =
|
||||||
!serverName.trim() ||
|
!serverName.trim() ||
|
||||||
missingRequiredEnvVars ||
|
missingRequiredEnvVars ||
|
||||||
|
|
@ -199,7 +199,7 @@ export const McpServerDetailDialog = ({
|
||||||
registryId: server.id,
|
registryId: server.id,
|
||||||
serverName,
|
serverName,
|
||||||
scope,
|
scope,
|
||||||
projectPath: scope === 'project' ? (projectPath ?? undefined) : undefined,
|
projectPath: scope !== 'user' ? (projectPath ?? undefined) : undefined,
|
||||||
envValues,
|
envValues,
|
||||||
headers,
|
headers,
|
||||||
});
|
});
|
||||||
|
|
@ -210,7 +210,7 @@ export const McpServerDetailDialog = ({
|
||||||
server.id,
|
server.id,
|
||||||
uninstallServerName,
|
uninstallServerName,
|
||||||
uninstallScope,
|
uninstallScope,
|
||||||
uninstallScope === 'project' ? (projectPath ?? undefined) : undefined
|
uninstallScope !== 'user' ? (projectPath ?? undefined) : undefined
|
||||||
);
|
);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
@ -417,7 +417,7 @@ export const McpServerDetailDialog = ({
|
||||||
<SelectItem
|
<SelectItem
|
||||||
key={opt.value}
|
key={opt.value}
|
||||||
value={opt.value}
|
value={opt.value}
|
||||||
disabled={opt.value === 'project' && !projectPath}
|
disabled={opt.value !== 'user' && !projectPath}
|
||||||
>
|
>
|
||||||
{opt.label}
|
{opt.label}
|
||||||
</SelectItem>
|
</SelectItem>
|
||||||
|
|
|
||||||
|
|
@ -130,6 +130,7 @@ describe('McpInstallService', () => {
|
||||||
registryId: 'upstash/context7-mcp',
|
registryId: 'upstash/context7-mcp',
|
||||||
serverName: 'context7',
|
serverName: 'context7',
|
||||||
scope: 'local',
|
scope: 'local',
|
||||||
|
projectPath: '/tmp/test',
|
||||||
envValues: {},
|
envValues: {},
|
||||||
headers: [],
|
headers: [],
|
||||||
});
|
});
|
||||||
|
|
@ -230,6 +231,20 @@ describe('McpInstallService', () => {
|
||||||
expect(result.error).toContain('projectPath is required');
|
expect(result.error).toContain('projectPath is required');
|
||||||
expect(mockExecCli).not.toHaveBeenCalled();
|
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 ──────────────────────────────────────────────────
|
// ── install: error masking ──────────────────────────────────────────────────
|
||||||
|
|
@ -290,6 +305,23 @@ describe('McpInstallService', () => {
|
||||||
expect(result.error).toContain('projectPath is required');
|
expect(result.error).toContain('projectPath is required');
|
||||||
expect(mockExecCli).not.toHaveBeenCalled();
|
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 ───────────────────────────────────────────────────────────────
|
// ── uninstall ───────────────────────────────────────────────────────────────
|
||||||
|
|
@ -334,6 +366,14 @@ describe('McpInstallService', () => {
|
||||||
expect(mockExecCli).not.toHaveBeenCalled();
|
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 () => {
|
it('returns error on CLI failure', async () => {
|
||||||
mockExecCli.mockRejectedValue(new Error('Not found'));
|
mockExecCli.mockRejectedValue(new Error('Not found'));
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -125,7 +125,7 @@ describe('CustomMcpServerDialog project scope', () => {
|
||||||
vi.unstubAllGlobals();
|
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');
|
const host = document.createElement('div');
|
||||||
document.body.appendChild(host);
|
document.body.appendChild(host);
|
||||||
const root = createRoot(host);
|
const root = createRoot(host);
|
||||||
|
|
@ -142,7 +142,9 @@ describe('CustomMcpServerDialog project scope', () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
const projectOption = host.querySelector('option[value="project"]') as HTMLOptionElement;
|
const projectOption = host.querySelector('option[value="project"]') as HTMLOptionElement;
|
||||||
|
const localOption = host.querySelector('option[value="local"]') as HTMLOptionElement;
|
||||||
expect(projectOption.disabled).toBe(true);
|
expect(projectOption.disabled).toBe(true);
|
||||||
|
expect(localOption.disabled).toBe(true);
|
||||||
|
|
||||||
await act(async () => {
|
await act(async () => {
|
||||||
root.unmount();
|
root.unmount();
|
||||||
|
|
@ -203,4 +205,58 @@ describe('CustomMcpServerDialog project scope', () => {
|
||||||
await Promise.resolve();
|
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();
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -217,7 +217,7 @@ describe('McpServerDetailDialog installed entry handling', () => {
|
||||||
'io.github.upstash/context7',
|
'io.github.upstash/context7',
|
||||||
'context7-local',
|
'context7-local',
|
||||||
'local',
|
'local',
|
||||||
undefined
|
'/tmp/project'
|
||||||
);
|
);
|
||||||
|
|
||||||
await act(async () => {
|
await act(async () => {
|
||||||
|
|
@ -258,6 +258,10 @@ describe('McpServerDetailDialog installed entry handling', () => {
|
||||||
|
|
||||||
expect(lookupMock).toHaveBeenCalledTimes(1);
|
expect(lookupMock).toHaveBeenCalledTimes(1);
|
||||||
expect(lookupMock).toHaveBeenCalledWith(['CONTEXT7_API_KEY']);
|
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 () => {
|
await act(async () => {
|
||||||
root.unmount();
|
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 () => {
|
it('uses selected scope instead of aggregated installed state', async () => {
|
||||||
const host = document.createElement('div');
|
const host = document.createElement('div');
|
||||||
document.body.appendChild(host);
|
document.body.appendChild(host);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue