Skip to content

Commit d38ea79

Browse files
bespoyasovbespoyasov
authored
fix: adjust MCP installation scope for Claude (#109)
* test: overlay env in correct order * chore: extend testing utils * fix: migrate from project scope for Claude, refactor integrations * chore: update clean script, expose as pnpm script * test: update e2e
1 parent 016a269 commit d38ea79

44 files changed

Lines changed: 733 additions & 643 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
"lint": "pnpm -r run lint",
99
"typecheck": "pnpm -r run typecheck",
1010
"qa": "pnpm typecheck && pnpm lint && pnpm test",
11-
"prepare": "husky"
11+
"prepare": "husky",
12+
"clear": "./scripts/clean-dev-env.sh"
1213
},
1314
"devDependencies": {
1415
"@commitlint/cli": "^21.2.3",

‎packages/cli/src/features/mcp/status.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
import type { IdeId } from '@spotify-confidence/shared-kernel';
2-
import { getIntegration, type McpServerName, type McpServerStatus } from '@spotify-confidence/core';
2+
import { getIntegration, type McpStatusMap } from '@spotify-confidence/core';
33

4-
export async function getMcpStatuses(
5-
ideId: IdeId,
6-
projectDir: string,
7-
): Promise<Record<McpServerName, McpServerStatus>> {
4+
export async function getMcpStatuses(ideId: IdeId, projectDir: string): Promise<McpStatusMap> {
85
const integration = getIntegration(ideId);
96
return integration.detectMcpStatuses(projectDir);
107
}

‎packages/core/__tests__/integrations/mcp-disconnect.test.ts‎

Lines changed: 24 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -21,27 +21,29 @@ beforeEach(() => {
2121

2222
describe('claude disconnectMcpServer', () => {
2323
async function loadDisconnect() {
24-
const mod = await import('@integrations/claude/mcp.js');
24+
const mod = await import('@integrations/claude/mcp/index.js');
2525
return mod.disconnectMcpServer;
2626
}
2727

28-
it('calls claude mcp remove with the server name', async () => {
29-
const sut = await loadDisconnect();
28+
it.each(['local', 'project', 'user'] as const)(
29+
'calls claude mcp remove with %s scope',
30+
async (scope) => {
31+
const sut = await loadDisconnect();
3032

31-
await sut({ serverName: 'confidence-flags', projectDir: '/project' });
33+
await sut({ serverName: 'confidence-flags', projectDir: '/project' });
3234

33-
expect(execFile).toHaveBeenCalledWith(
34-
'claude',
35-
['mcp', 'remove', '--scope', 'project', 'confidence-flags'],
36-
{ cwd: '/project' },
37-
);
38-
});
35+
expect(execFile).toHaveBeenCalledWith(
36+
'claude',
37+
['mcp', 'remove', '--scope', scope, 'confidence-flags'],
38+
{ cwd: '/project' },
39+
);
40+
},
41+
);
3942

4043
it('removes the tool permission from settings.local.json', async () => {
4144
using project = createProjectDir('empty');
4245
writeClaudeSettings(project.path, {
4346
permissions: { allow: ['mcp__confidence-flags__*', 'mcp__other__*'] },
44-
enabledMcpjsonServers: ['confidence-flags', 'other-server'],
4547
});
4648
const sut = await loadDisconnect();
4749

@@ -51,7 +53,6 @@ describe('claude disconnectMcpServer', () => {
5153
readFileSync(join(project.path, '.claude', 'settings.local.json'), 'utf-8'),
5254
);
5355
expect(settings.permissions.allow).toEqual(['mcp__other__*']);
54-
expect(settings.enabledMcpjsonServers).toEqual(['other-server']);
5556
});
5657

5758
it('does not fail when settings.local.json does not exist', async () => {
@@ -67,32 +68,30 @@ describe('claude disconnectMcpServer', () => {
6768
using project = createProjectDir('empty');
6869
writeClaudeSettings(project.path, {
6970
permissions: { allow: ['mcp__confidence-flags__*'] },
70-
enabledMcpjsonServers: ['confidence-flags'],
7171
});
72-
execFile.mockRejectedValueOnce(new Error('claude not found'));
72+
execFile.mockRejectedValue(new Error('claude not found'));
7373
const sut = await loadDisconnect();
7474

75-
await expect(sut({ serverName: 'confidence-flags', projectDir: project.path })).rejects.toThrow(
76-
'claude not found',
77-
);
75+
await sut({ serverName: 'confidence-flags', projectDir: project.path });
7876

7977
const settings = JSON.parse(
8078
readFileSync(join(project.path, '.claude', 'settings.local.json'), 'utf-8'),
8179
);
8280
expect(settings.permissions.allow).toEqual([]);
83-
expect(settings.enabledMcpjsonServers).toEqual([]);
8481
});
8582
});
8683

8784
describe('cursor disconnectMcpServer', () => {
8885
async function loadDisconnect() {
89-
const mod = await import('@integrations/cursor/mcp.js');
86+
const mod = await import('@integrations/cursor/mcp/index.js');
9087
return mod.disconnectMcpServer;
9188
}
9289

93-
it('removes the server entry from project config and CLI permissions', async () => {
90+
it('removes the server from global config and CLI permissions', async () => {
9491
using project = createProjectDir('empty');
95-
writeCursorMcpConfig(project.path, {
92+
using home = createProjectDir('empty');
93+
vi.stubEnv('HOME', home.path);
94+
writeCursorMcpConfig(home.path, {
9695
mcpServers: {
9796
'confidence-flags': { type: 'http', url: 'https://example.com' },
9897
'other-server': { type: 'http', url: 'https://other.com' },
@@ -106,33 +105,14 @@ describe('cursor disconnectMcpServer', () => {
106105

107106
await sut({ serverName: 'confidence-flags', projectDir: project.path });
108107

109-
const updatedMcp = JSON.parse(readFileSync(join(project.path, '.cursor', 'mcp.json'), 'utf-8'));
110-
expect(updatedMcp.mcpServers).not.toHaveProperty('confidence-flags');
111-
expect(updatedMcp.mcpServers).toHaveProperty('other-server');
108+
const globalMcp = JSON.parse(readFileSync(join(home.path, '.cursor', 'mcp.json'), 'utf-8'));
109+
expect(globalMcp.mcpServers).not.toHaveProperty('confidence-flags');
110+
expect(globalMcp.mcpServers).toHaveProperty('other-server');
112111

113112
const updatedCli = JSON.parse(readFileSync(join(project.path, '.cursor', 'cli.json'), 'utf-8'));
114113
expect(updatedCli.permissions.allow).toEqual(['Mcp(other:*)']);
115114
});
116115

117-
it('does not remove the server from global config', async () => {
118-
using project = createProjectDir('empty');
119-
using home = createProjectDir('empty');
120-
vi.stubEnv('HOME', home.path);
121-
writeCursorMcpConfig(project.path, {
122-
mcpServers: { 'confidence-flags': { type: 'http', url: 'https://example.com' } },
123-
});
124-
writeCursorMcpConfig(home.path, {
125-
mcpServers: { 'confidence-flags': { type: 'http', url: 'https://example.com' } },
126-
});
127-
128-
const sut = await loadDisconnect();
129-
130-
await sut({ serverName: 'confidence-flags', projectDir: project.path });
131-
132-
const globalMcp = JSON.parse(readFileSync(join(home.path, '.cursor', 'mcp.json'), 'utf-8'));
133-
expect(globalMcp.mcpServers).toHaveProperty('confidence-flags');
134-
});
135-
136116
it('does not fail when config files do not exist', async () => {
137117
using project = createProjectDir('empty');
138118
const sut = await loadDisconnect();
@@ -145,7 +125,7 @@ describe('cursor disconnectMcpServer', () => {
145125

146126
describe('codex disconnectMcpServer', () => {
147127
async function loadDisconnect() {
148-
const mod = await import('@integrations/codex/mcp.js');
128+
const mod = await import('@integrations/codex/mcp/index.js');
149129
return mod.disconnectMcpServer;
150130
}
151131

‎packages/core/src/integrations/claude/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { IdeIntegration } from '../types.js';
22
import { launchChat } from './chat.js';
33
import { detectPlugin, installPlugin, updatePlugin, uninstallPlugin } from './plugins.js';
44
import { skillsDir } from './paths.js';
5-
import { detectMcpStatuses, connectMcpServer, disconnectMcpServer } from './mcp.js';
5+
import { detectMcpStatuses, connectMcpServer, disconnectMcpServer } from './mcp/index.js';
66
import { runOnboarding } from './onboarding.js';
77
import { prepare } from './prepare.js';
88

‎packages/core/src/integrations/claude/mcp.ts‎

Lines changed: 0 additions & 128 deletions
This file was deleted.

‎packages/core/src/integrations/claude/mcp/connect.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { execFile } from '../../../exec/exec.js';
2+
import type { McpConnectOpts, McpDisconnectOpts } from '../../types.js';
3+
import { allowMcpToolsInSettings, removeMcpToolsFromSettings } from './settings.js';
4+
5+
export async function connectMcpServer(opts: McpConnectOpts): Promise<void> {
6+
await unregisterServer(opts);
7+
await registerServer(opts);
8+
allowMcpToolsInSettings(opts);
9+
}
10+
11+
export async function disconnectMcpServer(opts: McpDisconnectOpts): Promise<void> {
12+
removeMcpToolsFromSettings(opts);
13+
await unregisterServer(opts);
14+
}
15+
16+
async function registerServer(opts: McpConnectOpts): Promise<void> {
17+
const headers = { ...opts.serverHeaders };
18+
const args = [
19+
'mcp',
20+
'add',
21+
'--transport',
22+
'http',
23+
'--scope',
24+
'local',
25+
opts.serverName,
26+
opts.serverUrl,
27+
];
28+
29+
if (opts.accessToken) {
30+
headers['Authorization'] = `Bearer ${opts.accessToken}`;
31+
}
32+
33+
for (const [key, value] of Object.entries(headers)) {
34+
args.push('--header', `${key}: ${value}`);
35+
}
36+
37+
await execFile('claude', args, { cwd: opts.projectDir });
38+
}
39+
40+
async function unregisterServer(opts: McpDisconnectOpts): Promise<void> {
41+
await Promise.allSettled(
42+
(['local', 'project', 'user'] as const).map((scope) =>
43+
execFile('claude', ['mcp', 'remove', '--scope', scope, opts.serverName], {
44+
cwd: opts.projectDir,
45+
}),
46+
),
47+
);
48+
}

‎packages/core/src/integrations/claude/mcp/detect.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { realpathSync, readFileSync } from 'node:fs';
2+
import type { McpStatusMap } from '../../mcp/servers.js';
3+
import { type McpServerName, detectMcpStatuses as detectShared } from '../../mcp/servers.js';
4+
import { onlyKnownServerNames } from '../../mcp/config.js';
5+
import { globalConfigPath } from '../paths.js';
6+
7+
type McpServerEntry = {
8+
headers?: Record<string, string>;
9+
};
10+
11+
type ClaudeProjectConfig = {
12+
mcpServers?: Record<string, McpServerEntry>;
13+
};
14+
15+
type ClaudeGlobalConfig = {
16+
projects?: Record<string, ClaudeProjectConfig>;
17+
};
18+
19+
function readProjectMcpServers(projectDir: string): Record<string, McpServerEntry> {
20+
try {
21+
const resolved = realpathSync(projectDir);
22+
const config = JSON.parse(readFileSync(globalConfigPath(), 'utf-8')) as ClaudeGlobalConfig;
23+
return (
24+
config.projects?.[resolved]?.mcpServers ?? config.projects?.[projectDir]?.mcpServers ?? {}
25+
);
26+
} catch {
27+
return {};
28+
}
29+
}
30+
31+
function getLocalScopeMcpNames(projectDir: string): string[] {
32+
return onlyKnownServerNames(Object.keys(readProjectMcpServers(projectDir)));
33+
}
34+
35+
function getLocalScopeAuthToken(projectDir: string, serverName: McpServerName): string | null {
36+
const bearer = readProjectMcpServers(projectDir)[serverName]?.headers?.['Authorization'];
37+
return bearer?.startsWith('Bearer ') ? bearer.slice(7) : null;
38+
}
39+
40+
export function detectMcpStatuses(projectDir: string): Promise<McpStatusMap> {
41+
return detectShared({
42+
getRegisteredNames: () => getLocalScopeMcpNames(projectDir),
43+
getAuthToken: (name) => getLocalScopeAuthToken(projectDir, name),
44+
});
45+
}

‎packages/core/src/integrations/claude/mcp/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export { detectMcpStatuses } from './detect.js';
2+
export { connectMcpServer, disconnectMcpServer } from './connect.js';

0 commit comments

Comments
 (0)