mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-31 16:33:34 +08:00
fix(mcp): delete servers declared in a parent .mcp.json (#1107)
Project scope reads and writes disagreed about which file owns a server.
getMcpConfigsByScope('project') walks from the cwd up to the filesystem root
and inherits every .mcp.json it finds, so a server declared in an ancestor
directory shows up for a nested project -- with canRemove: true, because the
DTO derives that from the scope alone. removeMcpConfig('project') only ever
read the cwd's own .mcp.json, so it threw a plain Error for exactly those
servers, and deleteServer let it fall through to the errorHandler's 500 catch.
That is the reported pair of symptoms: the delete does nothing and the toast
says "An unexpected error occurred". Windows hits it easily, since dropping
.mcp.json in the home directory (mistaking it for global config) makes every
project below inherit servers that none of them can remove.
removeMcpConfig now resolves the file that actually declares the server, using
the same nearest-first order the reader uses, and edits that file. The edit is
applied to the raw parsed JSON rather than to a rebuilt McpJsonConfig: the old
rebuild ran through parseMcpConfigFromFilePath with expandVars: true, so
rewriting the file baked resolved ${VAR} values into it and dropped every
top-level key outside mcpServers, since the zod schema strips them. Lookup
matches on the raw object too, because validation is all-or-nothing per file --
one malformed entry would otherwise make every server in that file unremovable,
and would leave lookup and rewrite working off different views.
addMcpConfig had the same rebuild, so adding any project-scoped server rewrote
its siblings the same way. Measured against a live server with MY_MCP_TOKEN
set, "Bearer ${MY_MCP_TOKEN}" came back as "Bearer super-secret-value" -- a
plaintext credential written into a file that is normally committed. It now
inserts into the raw object as well, and its duplicate-name check reads the raw
file so a name sitting in an otherwise-invalid .mcp.json is still reported as a
conflict instead of being silently overwritten.
Two smaller corrections follow from the same asymmetry. deleteServer reports
why a removal failed (400 with the underlying message) instead of an opaque
500, and rejects unknown or non-editable scopes with a reason. configLocation
pointed at join(getCwd(), '.mcp.json') for every project-scoped server, naming
a file that does not exist for inherited ones; it now names the declaring file,
which is what tells a user where the server came from.
getProjectMcpConfigsFromCwd() has no callers left. Kept rather than deleted --
it is vendored upstream code and removing it widens future sync conflicts --
with its comment corrected, since it claimed to serve add/remove.
Verified over real HTTP against a server with an isolated CLAUDE_CONFIG_DIR,
not only through handleMcpApi: before the change the delete returned 500 with
the generic message and left the file untouched; after it returns 200, removes
the entry from the parent file, creates no .mcp.json in the cwd, and preserves
both $schema and the unexpanded ${MY_MCP_TOKEN} of a sibling server. The add
path was confirmed the same way. check:server: 226 files, 2405 tests, 0 failed.
This commit is contained in:
parent
efb8b5c9b4
commit
eecc873535
@ -18,6 +18,7 @@ let originalConfigDir: string | undefined
|
||||
let connectSpy: ReturnType<typeof spyOn> | undefined
|
||||
let getClaudeCodeMcpConfigsSpy: ReturnType<typeof spyOn> | undefined
|
||||
let getAllMcpConfigsSpy: ReturnType<typeof spyOn> | undefined
|
||||
let getMcpConfigByNameSpy: ReturnType<typeof spyOn> | undefined
|
||||
let reconnectSpy: ReturnType<typeof spyOn> | undefined
|
||||
let hostPreflightSpy: ReturnType<typeof spyOn> | undefined
|
||||
let originalRequestControl: typeof conversationService.requestControl
|
||||
@ -97,6 +98,8 @@ describe('MCP API', () => {
|
||||
getClaudeCodeMcpConfigsSpy = undefined
|
||||
getAllMcpConfigsSpy?.mockRestore()
|
||||
getAllMcpConfigsSpy = undefined
|
||||
getMcpConfigByNameSpy?.mockRestore()
|
||||
getMcpConfigByNameSpy = undefined
|
||||
reconnectSpy?.mockRestore()
|
||||
reconnectSpy = undefined
|
||||
hostPreflightSpy?.mockRestore()
|
||||
@ -491,6 +494,185 @@ describe('MCP API', () => {
|
||||
expect(listBody.servers.some((server: { name: string }) => server.name === 'context7')).toBe(false)
|
||||
})
|
||||
|
||||
it('deletes project MCP servers inherited from a parent .mcp.json', async () => {
|
||||
const parent = path.join(tmpDir, 'workspace')
|
||||
const child = path.join(parent, 'nested-project')
|
||||
await fs.mkdir(child, { recursive: true })
|
||||
const parentMcpJson = path.join(parent, '.mcp.json')
|
||||
await fs.writeFile(
|
||||
parentMcpJson,
|
||||
JSON.stringify(
|
||||
{
|
||||
mcpServers: {
|
||||
inherited: { type: 'stdio', command: 'npx', args: ['inherited-mcp'] },
|
||||
},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
)
|
||||
|
||||
// The server is visible from the nested cwd and advertised as removable.
|
||||
const list = makeRequest('GET', `/api/mcp?cwd=${encodeURIComponent(child)}`)
|
||||
const listRes = await handleMcpApi(list.req, list.url, list.segments)
|
||||
const listBody = await listRes.json()
|
||||
expect(listBody.servers).toContainEqual(
|
||||
expect.objectContaining({
|
||||
name: 'inherited',
|
||||
scope: 'project',
|
||||
canRemove: true,
|
||||
configLocation: parentMcpJson,
|
||||
}),
|
||||
)
|
||||
|
||||
const remove = makeRequest(
|
||||
'DELETE',
|
||||
`/api/mcp/inherited?scope=project&cwd=${encodeURIComponent(child)}`,
|
||||
)
|
||||
const removeRes = await handleMcpApi(remove.req, remove.url, remove.segments)
|
||||
expect(removeRes.status).toBe(200)
|
||||
|
||||
// Removed from the file that declares it, without creating one in the cwd.
|
||||
const parentConfig = JSON.parse(await fs.readFile(parentMcpJson, 'utf8'))
|
||||
expect(parentConfig.mcpServers?.inherited).toBeUndefined()
|
||||
await expect(fs.stat(path.join(child, '.mcp.json'))).rejects.toThrow()
|
||||
|
||||
const listAfter = makeRequest('GET', `/api/mcp?cwd=${encodeURIComponent(child)}`)
|
||||
const listAfterRes = await handleMcpApi(listAfter.req, listAfter.url, listAfter.segments)
|
||||
const listAfterBody = await listAfterRes.json()
|
||||
expect(listAfterBody.servers.some((server: { name: string }) => server.name === 'inherited')).toBe(false)
|
||||
})
|
||||
|
||||
it('edits project MCP servers inherited from a parent .mcp.json', async () => {
|
||||
const parent = path.join(tmpDir, 'edit-workspace')
|
||||
const child = path.join(parent, 'nested-project')
|
||||
await fs.mkdir(child, { recursive: true })
|
||||
const parentMcpJson = path.join(parent, '.mcp.json')
|
||||
await fs.writeFile(
|
||||
parentMcpJson,
|
||||
JSON.stringify(
|
||||
{ mcpServers: { inherited: { type: 'stdio', command: 'npx', args: ['old-args'] } } },
|
||||
null,
|
||||
2,
|
||||
),
|
||||
)
|
||||
|
||||
const update = makeRequest('PUT', '/api/mcp/inherited', {
|
||||
cwd: child,
|
||||
previousCwd: child,
|
||||
scope: 'project',
|
||||
config: { type: 'stdio', command: 'npx', args: ['new-args'], env: {} },
|
||||
})
|
||||
const updateRes = await handleMcpApi(update.req, update.url, update.segments)
|
||||
expect(updateRes.status).toBe(200)
|
||||
|
||||
// The edit moves the entry out of the inherited file into the target cwd.
|
||||
const parentConfig = JSON.parse(await fs.readFile(parentMcpJson, 'utf8'))
|
||||
const childConfig = JSON.parse(await fs.readFile(path.join(child, '.mcp.json'), 'utf8'))
|
||||
expect(parentConfig.mcpServers?.inherited).toBeUndefined()
|
||||
expect(childConfig.mcpServers?.inherited).toMatchObject({
|
||||
type: 'stdio',
|
||||
command: 'npx',
|
||||
args: ['new-args'],
|
||||
})
|
||||
})
|
||||
|
||||
it('preserves unrelated content when rewriting .mcp.json on delete', async () => {
|
||||
const proj = path.join(tmpDir, 'preserve')
|
||||
await fs.mkdir(proj, { recursive: true })
|
||||
const mcpJsonPath = path.join(proj, '.mcp.json')
|
||||
await fs.writeFile(
|
||||
mcpJsonPath,
|
||||
JSON.stringify(
|
||||
{
|
||||
$schema: 'https://example.com/mcp.schema.json',
|
||||
mcpServers: {
|
||||
doomed: { type: 'stdio', command: 'npx', args: ['doomed-mcp'] },
|
||||
kept: {
|
||||
type: 'http',
|
||||
url: 'https://mcp.example.com/mcp',
|
||||
headers: { Authorization: 'Bearer ${MY_MCP_TOKEN}' },
|
||||
},
|
||||
},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
)
|
||||
|
||||
const remove = makeRequest(
|
||||
'DELETE',
|
||||
`/api/mcp/doomed?scope=project&cwd=${encodeURIComponent(proj)}`,
|
||||
)
|
||||
const removeRes = await handleMcpApi(remove.req, remove.url, remove.segments)
|
||||
expect(removeRes.status).toBe(200)
|
||||
|
||||
const rewritten = JSON.parse(await fs.readFile(mcpJsonPath, 'utf8'))
|
||||
expect(rewritten.mcpServers.doomed).toBeUndefined()
|
||||
// Top-level keys outside mcpServers survive, and ${VAR} placeholders are
|
||||
// written back verbatim rather than expanded into resolved values.
|
||||
expect(rewritten.$schema).toBe('https://example.com/mcp.schema.json')
|
||||
expect(rewritten.mcpServers.kept.headers.Authorization).toBe('Bearer ${MY_MCP_TOKEN}')
|
||||
})
|
||||
|
||||
it('reports why a delete failed instead of a generic 500', async () => {
|
||||
const create = makeRequest('POST', '/api/mcp', {
|
||||
cwd: projectRoot,
|
||||
name: 'mismatched',
|
||||
scope: 'user',
|
||||
config: { type: 'stdio', command: 'npx', args: ['mismatched-mcp'] },
|
||||
})
|
||||
await handleMcpApi(create.req, create.url, create.segments)
|
||||
|
||||
// Asking to remove a user-scoped server from local scope cannot succeed; the
|
||||
// caller needs the reason, not an opaque "unexpected error occurred".
|
||||
const remove = makeRequest(
|
||||
'DELETE',
|
||||
`/api/mcp/mismatched?scope=local&cwd=${encodeURIComponent(projectRoot)}`,
|
||||
)
|
||||
const removeRes = await handleMcpApi(remove.req, remove.url, remove.segments)
|
||||
|
||||
expect(removeRes.status).toBe(400)
|
||||
await expect(removeRes.json()).resolves.toMatchObject({
|
||||
error: 'BAD_REQUEST',
|
||||
message: 'No project-local MCP server found with name: mismatched',
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects delete requests for scopes that cannot be edited', async () => {
|
||||
getMcpConfigByNameSpy = spyOn(mcpConfig, 'getMcpConfigByName').mockReturnValue({
|
||||
type: 'stdio',
|
||||
command: 'npx',
|
||||
args: ['managed-mcp'],
|
||||
scope: 'enterprise',
|
||||
})
|
||||
|
||||
const remove = makeRequest(
|
||||
'DELETE',
|
||||
`/api/mcp/managed-server?scope=enterprise&cwd=${encodeURIComponent(projectRoot)}`,
|
||||
)
|
||||
const removeRes = await handleMcpApi(remove.req, remove.url, remove.segments)
|
||||
|
||||
expect(removeRes.status).toBe(400)
|
||||
await expect(removeRes.json()).resolves.toMatchObject({
|
||||
message: 'MCP server "managed-server" cannot be removed from scope "enterprise"',
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects delete requests carrying an unknown scope', async () => {
|
||||
const remove = makeRequest(
|
||||
'DELETE',
|
||||
`/api/mcp/whatever?scope=bogus&cwd=${encodeURIComponent(projectRoot)}`,
|
||||
)
|
||||
const removeRes = await handleMcpApi(remove.req, remove.url, remove.segments)
|
||||
|
||||
expect(removeRes.status).toBe(400)
|
||||
await expect(removeRes.json()).resolves.toMatchObject({
|
||||
error: 'BAD_REQUEST',
|
||||
message: expect.stringContaining('Invalid scope: bogus'),
|
||||
})
|
||||
})
|
||||
|
||||
it('syncs MCP toggles into the active CLI session control channel', async () => {
|
||||
const create = makeRequest('POST', '/api/mcp', {
|
||||
cwd: projectRoot,
|
||||
|
||||
@ -9,6 +9,7 @@ import {
|
||||
} from '../../services/mcp/client.js'
|
||||
import {
|
||||
addMcpConfig,
|
||||
findProjectMcpConfigPath,
|
||||
getAllMcpConfigs,
|
||||
getClaudeCodeMcpConfigs,
|
||||
getMcpConfigByName,
|
||||
@ -98,6 +99,14 @@ function optionalString(value: unknown): string | undefined {
|
||||
return typeof value === 'string' && value.length > 0 ? value : undefined
|
||||
}
|
||||
|
||||
function parseScopeParam(value: string | null): ConfigScope {
|
||||
try {
|
||||
return ensureConfigScope(value || undefined)
|
||||
} catch (error) {
|
||||
throw ApiError.badRequest(error instanceof Error ? error.message : String(error))
|
||||
}
|
||||
}
|
||||
|
||||
async function syncMcpToggleToSession(
|
||||
sessionId: string | undefined,
|
||||
serverName: string,
|
||||
@ -170,6 +179,16 @@ function serializeEditableConfig(config: ScopedMcpServerConfig): McpEditableConf
|
||||
return { type: config.type }
|
||||
}
|
||||
|
||||
function describeServerConfigLocation(name: string, scope: ConfigScope): string {
|
||||
// Project scope inherits from parent directories, so the cwd's .mcp.json is
|
||||
// not necessarily the file that declares this server. Point at the real one.
|
||||
if (scope === 'project') {
|
||||
return findProjectMcpConfigPath(name) ?? describeMcpConfigFilePath(scope)
|
||||
}
|
||||
|
||||
return describeMcpConfigFilePath(scope)
|
||||
}
|
||||
|
||||
function getSummary(config: ScopedMcpServerConfig): string {
|
||||
if (!config.type || config.type === 'stdio') {
|
||||
const stdioConfig = config as McpStdioServerConfig
|
||||
@ -307,7 +326,7 @@ function buildServerDto(
|
||||
status: status.status,
|
||||
statusLabel: status.statusLabel,
|
||||
statusDetail: status.statusDetail,
|
||||
configLocation: describeMcpConfigFilePath(config.scope),
|
||||
configLocation: describeServerConfigLocation(name, config.scope),
|
||||
summary: getSummary(config),
|
||||
canEdit,
|
||||
canRemove: EDITABLE_SCOPES.has(config.scope),
|
||||
@ -539,13 +558,24 @@ async function updateServer(name: string, body: Record<string, unknown>): Promis
|
||||
}
|
||||
|
||||
async function deleteServer(name: string, url: URL): Promise<Response> {
|
||||
const scope = ensureConfigScope(url.searchParams.get('scope') || undefined)
|
||||
const scope = parseScopeParam(url.searchParams.get('scope'))
|
||||
const existing = getMcpConfigByName(name)
|
||||
if (!existing) {
|
||||
throw ApiError.notFound(`MCP server not found: ${name}`)
|
||||
}
|
||||
|
||||
await removeMcpConfig(name, scope)
|
||||
if (!EDITABLE_SCOPES.has(scope)) {
|
||||
throw ApiError.badRequest(`MCP server "${name}" cannot be removed from scope "${scope}"`)
|
||||
}
|
||||
|
||||
try {
|
||||
await removeMcpConfig(name, scope)
|
||||
} catch (error) {
|
||||
// Report why the config file could not be rewritten. Falling through to the
|
||||
// generic handler would surface an opaque 500 with no actionable detail.
|
||||
throw ApiError.badRequest(error instanceof Error ? error.message : String(error))
|
||||
}
|
||||
|
||||
cleanupSecureStorage(name, existing)
|
||||
await clearServerCache(name, existing).catch(() => {})
|
||||
|
||||
|
||||
203
src/services/mcp/__tests__/projectMcpConfigWrites.test.ts
Normal file
203
src/services/mcp/__tests__/projectMcpConfigWrites.test.ts
Normal file
@ -0,0 +1,203 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'bun:test'
|
||||
import * as fs from 'fs/promises'
|
||||
import * as os from 'os'
|
||||
import * as path from 'path'
|
||||
import { runWithCwdOverride } from '../../../utils/cwd.js'
|
||||
import { addMcpConfig, findProjectMcpConfigPath, removeMcpConfig } from '../config.js'
|
||||
|
||||
let tmpDir: string
|
||||
|
||||
async function writeMcpJson(dir: string, config: unknown) {
|
||||
await fs.mkdir(dir, { recursive: true })
|
||||
const filePath = path.join(dir, '.mcp.json')
|
||||
await fs.writeFile(filePath, JSON.stringify(config, null, 2))
|
||||
return filePath
|
||||
}
|
||||
|
||||
function readMcpJson(filePath: string) {
|
||||
return fs.readFile(filePath, 'utf8').then(contents => JSON.parse(contents))
|
||||
}
|
||||
|
||||
const STDIO_SERVER = { type: 'stdio', command: 'npx', args: ['some-mcp'] }
|
||||
|
||||
describe('project-scoped MCP config writes', () => {
|
||||
beforeEach(async () => {
|
||||
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcp-project-removal-'))
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
describe('findProjectMcpConfigPath', () => {
|
||||
it('resolves the nearest .mcp.json that declares the server', async () => {
|
||||
const parent = path.join(tmpDir, 'workspace')
|
||||
const child = path.join(parent, 'project')
|
||||
await writeMcpJson(parent, { mcpServers: { shared: STDIO_SERVER } })
|
||||
const childPath = await writeMcpJson(child, { mcpServers: { shared: STDIO_SERVER } })
|
||||
|
||||
expect(runWithCwdOverride(child, () => findProjectMcpConfigPath('shared'))).toBe(childPath)
|
||||
})
|
||||
|
||||
it('walks up to a parent declaration when the cwd has no .mcp.json', async () => {
|
||||
const parent = path.join(tmpDir, 'workspace')
|
||||
const child = path.join(parent, 'nested', 'deep')
|
||||
const parentPath = await writeMcpJson(parent, { mcpServers: { inherited: STDIO_SERVER } })
|
||||
await fs.mkdir(child, { recursive: true })
|
||||
|
||||
expect(runWithCwdOverride(child, () => findProjectMcpConfigPath('inherited'))).toBe(parentPath)
|
||||
})
|
||||
|
||||
it('returns null when no ancestor declares the server', async () => {
|
||||
const proj = path.join(tmpDir, 'project')
|
||||
await writeMcpJson(proj, { mcpServers: { other: STDIO_SERVER } })
|
||||
|
||||
expect(runWithCwdOverride(proj, () => findProjectMcpConfigPath('missing'))).toBeNull()
|
||||
})
|
||||
|
||||
it('still resolves a server declared alongside an invalid entry', async () => {
|
||||
const proj = path.join(tmpDir, 'project')
|
||||
// `broken` has no command, so schema validation rejects the whole file.
|
||||
const projPath = await writeMcpJson(proj, {
|
||||
mcpServers: { broken: { type: 'stdio' }, target: STDIO_SERVER },
|
||||
})
|
||||
|
||||
expect(runWithCwdOverride(proj, () => findProjectMcpConfigPath('target'))).toBe(projPath)
|
||||
})
|
||||
})
|
||||
|
||||
describe('removeMcpConfig with project scope', () => {
|
||||
it('removes the entry from the declaring parent file', async () => {
|
||||
const parent = path.join(tmpDir, 'workspace')
|
||||
const child = path.join(parent, 'project')
|
||||
const parentPath = await writeMcpJson(parent, {
|
||||
mcpServers: { inherited: STDIO_SERVER, sibling: STDIO_SERVER },
|
||||
})
|
||||
await fs.mkdir(child, { recursive: true })
|
||||
|
||||
await runWithCwdOverride(child, () => removeMcpConfig('inherited', 'project'))
|
||||
|
||||
const parentConfig = await readMcpJson(parentPath)
|
||||
expect(parentConfig.mcpServers.inherited).toBeUndefined()
|
||||
expect(parentConfig.mcpServers.sibling).toEqual(STDIO_SERVER)
|
||||
// The cwd must not gain a .mcp.json as a side effect of the removal.
|
||||
await expect(fs.stat(path.join(child, '.mcp.json'))).rejects.toThrow()
|
||||
})
|
||||
|
||||
it('keeps unrelated top-level keys and unexpanded variables intact', async () => {
|
||||
const proj = path.join(tmpDir, 'project')
|
||||
const projPath = await writeMcpJson(proj, {
|
||||
$schema: 'https://example.com/mcp.schema.json',
|
||||
inputs: [{ id: 'token', type: 'promptString' }],
|
||||
mcpServers: {
|
||||
doomed: STDIO_SERVER,
|
||||
kept: {
|
||||
type: 'http',
|
||||
url: 'https://mcp.example.com/mcp',
|
||||
headers: { Authorization: 'Bearer ${MY_MCP_TOKEN}' },
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
await runWithCwdOverride(proj, () => removeMcpConfig('doomed', 'project'))
|
||||
|
||||
const config = await readMcpJson(projPath)
|
||||
expect(config.mcpServers.doomed).toBeUndefined()
|
||||
expect(config.$schema).toBe('https://example.com/mcp.schema.json')
|
||||
expect(config.inputs).toEqual([{ id: 'token', type: 'promptString' }])
|
||||
expect(config.mcpServers.kept.headers.Authorization).toBe('Bearer ${MY_MCP_TOKEN}')
|
||||
})
|
||||
|
||||
it('leaves invalid sibling entries in place instead of dropping them', async () => {
|
||||
const proj = path.join(tmpDir, 'project')
|
||||
const projPath = await writeMcpJson(proj, {
|
||||
mcpServers: { broken: { type: 'stdio' }, target: STDIO_SERVER },
|
||||
})
|
||||
|
||||
await runWithCwdOverride(proj, () => removeMcpConfig('target', 'project'))
|
||||
|
||||
const config = await readMcpJson(projPath)
|
||||
expect(config.mcpServers.target).toBeUndefined()
|
||||
expect(config.mcpServers.broken).toEqual({ type: 'stdio' })
|
||||
})
|
||||
|
||||
it('throws when no ancestor .mcp.json declares the server', async () => {
|
||||
const proj = path.join(tmpDir, 'project')
|
||||
await writeMcpJson(proj, { mcpServers: { other: STDIO_SERVER } })
|
||||
|
||||
await expect(
|
||||
runWithCwdOverride(proj, () => removeMcpConfig('missing', 'project')),
|
||||
).rejects.toThrow('No MCP server found with name: missing in .mcp.json')
|
||||
})
|
||||
})
|
||||
|
||||
describe('addMcpConfig with project scope', () => {
|
||||
const previousToken = process.env.MY_MCP_TOKEN
|
||||
|
||||
beforeEach(() => {
|
||||
process.env.MY_MCP_TOKEN = 'super-secret-value'
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
if (previousToken === undefined) delete process.env.MY_MCP_TOKEN
|
||||
else process.env.MY_MCP_TOKEN = previousToken
|
||||
})
|
||||
|
||||
it('never writes a resolved ${VAR} value back over a sibling entry', async () => {
|
||||
const proj = path.join(tmpDir, 'project')
|
||||
const projPath = await writeMcpJson(proj, {
|
||||
mcpServers: {
|
||||
existing: {
|
||||
type: 'http',
|
||||
url: 'https://mcp.example.com/mcp',
|
||||
headers: { Authorization: 'Bearer ${MY_MCP_TOKEN}' },
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
await runWithCwdOverride(proj, () => addMcpConfig('added', STDIO_SERVER, 'project'))
|
||||
|
||||
const config = await readMcpJson(projPath)
|
||||
expect(config.mcpServers.added).toEqual(STDIO_SERVER)
|
||||
expect(config.mcpServers.existing.headers.Authorization).toBe('Bearer ${MY_MCP_TOKEN}')
|
||||
expect(JSON.stringify(config)).not.toContain('super-secret-value')
|
||||
})
|
||||
|
||||
it('keeps unrelated top-level keys when inserting a server', async () => {
|
||||
const proj = path.join(tmpDir, 'project')
|
||||
const projPath = await writeMcpJson(proj, {
|
||||
$schema: 'https://example.com/mcp.schema.json',
|
||||
inputs: [{ id: 'token', type: 'promptString' }],
|
||||
mcpServers: { existing: STDIO_SERVER },
|
||||
})
|
||||
|
||||
await runWithCwdOverride(proj, () => addMcpConfig('added', STDIO_SERVER, 'project'))
|
||||
|
||||
const config = await readMcpJson(projPath)
|
||||
expect(config.$schema).toBe('https://example.com/mcp.schema.json')
|
||||
expect(config.inputs).toEqual([{ id: 'token', type: 'promptString' }])
|
||||
expect(Object.keys(config.mcpServers).sort()).toEqual(['added', 'existing'])
|
||||
})
|
||||
|
||||
it('creates .mcp.json when the cwd has none', async () => {
|
||||
const proj = path.join(tmpDir, 'fresh')
|
||||
await fs.mkdir(proj, { recursive: true })
|
||||
|
||||
await runWithCwdOverride(proj, () => addMcpConfig('added', STDIO_SERVER, 'project'))
|
||||
|
||||
const config = await readMcpJson(path.join(proj, '.mcp.json'))
|
||||
expect(config).toEqual({ mcpServers: { added: STDIO_SERVER } })
|
||||
})
|
||||
|
||||
it('reports a conflict for a name declared in an otherwise invalid file', async () => {
|
||||
const proj = path.join(tmpDir, 'project')
|
||||
await writeMcpJson(proj, {
|
||||
mcpServers: { broken: { type: 'stdio' }, taken: STDIO_SERVER },
|
||||
})
|
||||
|
||||
await expect(
|
||||
runWithCwdOverride(proj, () => addMcpConfig('taken', STDIO_SERVER, 'project')),
|
||||
).rejects.toThrow('MCP server taken already exists in .mcp.json')
|
||||
})
|
||||
})
|
||||
})
|
||||
@ -91,13 +91,16 @@ function addScopeToServers(
|
||||
}
|
||||
|
||||
/**
|
||||
* Internal utility: Write MCP config to .mcp.json file.
|
||||
* Internal utility: Write MCP config to a .mcp.json file.
|
||||
* Preserves file permissions and flushes to disk before rename.
|
||||
* Uses the original path for rename (does not follow symlinks).
|
||||
* Defaults to the cwd's .mcp.json; project-scoped servers inherited from a
|
||||
* parent directory are written back to the file that declares them.
|
||||
*/
|
||||
async function writeMcpjsonFile(config: McpJsonConfig): Promise<void> {
|
||||
const mcpJsonPath = join(getCwd(), '.mcp.json')
|
||||
|
||||
async function writeMcpjsonFile(
|
||||
config: McpJsonConfig | Record<string, unknown>,
|
||||
mcpJsonPath: string = join(getCwd(), '.mcp.json'),
|
||||
): Promise<void> {
|
||||
// Read existing file permissions to preserve them
|
||||
let existingMode: number | undefined
|
||||
try {
|
||||
@ -689,8 +692,10 @@ export async function addMcpConfig(
|
||||
// Check if server already exists in the target scope
|
||||
switch (scope) {
|
||||
case 'project': {
|
||||
const { servers } = getProjectMcpConfigsFromCwd()
|
||||
if (servers[name]) {
|
||||
// Check the raw file so a name already present in a .mcp.json that fails
|
||||
// validation is still reported as a conflict rather than silently
|
||||
// overwritten by the raw rewrite below.
|
||||
if (readCwdMcpJsonServers().servers[name]) {
|
||||
throw new Error(`MCP server ${name} already exists in .mcp.json`)
|
||||
}
|
||||
break
|
||||
@ -720,21 +725,16 @@ export async function addMcpConfig(
|
||||
// Add based on scope
|
||||
switch (scope) {
|
||||
case 'project': {
|
||||
const { servers: existingServers } = getProjectMcpConfigsFromCwd()
|
||||
|
||||
const mcpServers: Record<string, McpServerConfig> = {}
|
||||
for (const [serverName, serverConfig] of Object.entries(
|
||||
existingServers,
|
||||
)) {
|
||||
const { scope: _, ...configWithoutScope } = serverConfig
|
||||
mcpServers[serverName] = configWithoutScope
|
||||
}
|
||||
// Insert into the raw file so unrelated servers keep their unexpanded
|
||||
// ${VAR} values and any other top-level keys survive the rewrite.
|
||||
const { path: mcpJsonPath, config: mcpConfig, servers: mcpServers } =
|
||||
readCwdMcpJsonServers()
|
||||
mcpServers[name] = validatedConfig
|
||||
const mcpConfig = { mcpServers }
|
||||
mcpConfig.mcpServers = mcpServers
|
||||
|
||||
// Write back to .mcp.json
|
||||
try {
|
||||
await writeMcpjsonFile(mcpConfig)
|
||||
await writeMcpjsonFile(mcpConfig, mcpJsonPath)
|
||||
} catch (error) {
|
||||
throw new Error(`Failed to write to .mcp.json: ${error}`)
|
||||
}
|
||||
@ -780,27 +780,28 @@ export async function removeMcpConfig(
|
||||
): Promise<void> {
|
||||
switch (scope) {
|
||||
case 'project': {
|
||||
const { servers: existingServers } = getProjectMcpConfigsFromCwd()
|
||||
|
||||
if (!existingServers[name]) {
|
||||
// Resolve the file that declares the server rather than assuming it lives
|
||||
// in the cwd: project scope inherits from parent directories, so a server
|
||||
// shown for this cwd may be declared further up the tree.
|
||||
const mcpJsonPath = findProjectMcpConfigPath(name)
|
||||
if (!mcpJsonPath) {
|
||||
throw new Error(`No MCP server found with name: ${name} in .mcp.json`)
|
||||
}
|
||||
|
||||
// Strip scope information when writing back to .mcp.json
|
||||
const mcpServers: Record<string, McpServerConfig> = {}
|
||||
for (const [serverName, serverConfig] of Object.entries(
|
||||
existingServers,
|
||||
)) {
|
||||
if (serverName !== name) {
|
||||
const { scope: _, ...configWithoutScope } = serverConfig
|
||||
mcpServers[serverName] = configWithoutScope
|
||||
}
|
||||
const mcpConfig = readRawMcpJsonFile(mcpJsonPath)
|
||||
const mcpServers = mcpConfig?.mcpServers
|
||||
if (!mcpConfig || !mcpServers || typeof mcpServers !== 'object') {
|
||||
throw new Error(`No MCP server found with name: ${name} in ${mcpJsonPath}`)
|
||||
}
|
||||
const mcpConfig = { mcpServers }
|
||||
|
||||
// Edit the raw object so unrelated servers, unexpanded ${VAR} values and
|
||||
// any other top-level keys survive the rewrite untouched.
|
||||
delete (mcpServers as Record<string, unknown>)[name]
|
||||
|
||||
try {
|
||||
await writeMcpjsonFile(mcpConfig)
|
||||
await writeMcpjsonFile(mcpConfig, mcpJsonPath)
|
||||
} catch (error) {
|
||||
throw new Error(`Failed to remove from .mcp.json: ${error}`)
|
||||
throw new Error(`Failed to remove from ${mcpJsonPath}: ${error}`)
|
||||
}
|
||||
break
|
||||
}
|
||||
@ -843,8 +844,11 @@ export async function removeMcpConfig(
|
||||
|
||||
/**
|
||||
* Get MCP configs from current directory only (no parent traversal).
|
||||
* Used by addMcpConfig and removeMcpConfig to modify the local .mcp.json file.
|
||||
* Exported for testing purposes.
|
||||
*
|
||||
* Note: addMcpConfig and removeMcpConfig no longer read through this. They edit
|
||||
* the raw .mcp.json instead (see readCwdMcpJsonServers), because rewriting a
|
||||
* file from validated configs expands ${VAR} placeholders and drops unknown
|
||||
* top-level keys. This currently has no callers.
|
||||
*
|
||||
* @returns Servers with scope information and any validation errors from current directory's .mcp.json
|
||||
*/
|
||||
@ -888,6 +892,109 @@ export function getProjectMcpConfigsFromCwd(): {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* List candidate .mcp.json paths from the cwd up to (but excluding) the
|
||||
* filesystem root, nearest first. Mirrors the traversal in
|
||||
* getMcpConfigsByScope('project'), where files closer to the cwd win.
|
||||
*/
|
||||
function listProjectMcpJsonPaths(): string[] {
|
||||
const paths: string[] = []
|
||||
let currentDir = getCwd()
|
||||
|
||||
while (currentDir !== parse(currentDir).root) {
|
||||
paths.push(join(currentDir, '.mcp.json'))
|
||||
currentDir = dirname(currentDir)
|
||||
}
|
||||
|
||||
return paths
|
||||
}
|
||||
|
||||
/**
|
||||
* Find the .mcp.json file that actually declares a project-scoped server.
|
||||
*
|
||||
* getMcpConfigsByScope('project') inherits servers from parent directories, so
|
||||
* a server can be visible from the cwd while being declared further up the
|
||||
* tree. Editing or removing it has to target the declaring file, not the cwd.
|
||||
*
|
||||
* @param name The name of the server
|
||||
* @returns Absolute path to the declaring .mcp.json, or null when no file in
|
||||
* the cwd's ancestry declares the server
|
||||
*/
|
||||
export function findProjectMcpConfigPath(name: string): string | null {
|
||||
if (!isSettingSourceEnabled('projectSettings')) {
|
||||
return null
|
||||
}
|
||||
|
||||
for (const mcpJsonPath of listProjectMcpJsonPaths()) {
|
||||
// Match on the raw JSON rather than the validated config: a single entry
|
||||
// that fails schema validation invalidates the whole file, which would
|
||||
// otherwise make every server in it unremovable. This also keeps lookup and
|
||||
// rewrite working off the same view of the file.
|
||||
const mcpServers = readRawMcpJsonFile(mcpJsonPath)?.mcpServers
|
||||
if (
|
||||
mcpServers &&
|
||||
typeof mcpServers === 'object' &&
|
||||
!Array.isArray(mcpServers) &&
|
||||
name in mcpServers
|
||||
) {
|
||||
return mcpJsonPath
|
||||
}
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a .mcp.json file as raw JSON, preserving every field verbatim.
|
||||
*
|
||||
* Rewriting a file from parsed-and-validated configs would expand ${VAR}
|
||||
* placeholders into their resolved values and drop top-level keys other than
|
||||
* mcpServers. Validation is also all-or-nothing per file, so one malformed
|
||||
* entry would take the rest of the file down with it. Editing the raw object
|
||||
* keeps unrelated content intact.
|
||||
*/
|
||||
function readRawMcpJsonFile(mcpJsonPath: string): Record<string, unknown> | null {
|
||||
const fs = getFsImplementation()
|
||||
|
||||
let contents: string
|
||||
try {
|
||||
contents = fs.readFileSync(mcpJsonPath, { encoding: 'utf8' })
|
||||
} catch (error: unknown) {
|
||||
if (getErrnoCode(error) === 'ENOENT') {
|
||||
return null
|
||||
}
|
||||
throw error
|
||||
}
|
||||
|
||||
const parsed = safeParseJSON(contents)
|
||||
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
|
||||
return null
|
||||
}
|
||||
|
||||
return parsed as Record<string, unknown>
|
||||
}
|
||||
|
||||
/**
|
||||
* Open the cwd's .mcp.json for editing, as a raw object plus a mutable handle
|
||||
* on its mcpServers map. Missing or unusable files yield empty structures so
|
||||
* callers can treat creation and update the same way.
|
||||
*/
|
||||
function readCwdMcpJsonServers(): {
|
||||
path: string
|
||||
config: Record<string, unknown>
|
||||
servers: Record<string, unknown>
|
||||
} {
|
||||
const path = join(getCwd(), '.mcp.json')
|
||||
const config = readRawMcpJsonFile(path) ?? {}
|
||||
const existing = config.mcpServers
|
||||
const servers =
|
||||
existing && typeof existing === 'object' && !Array.isArray(existing)
|
||||
? (existing as Record<string, unknown>)
|
||||
: {}
|
||||
|
||||
return { path, config, servers }
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all MCP configurations from a specific scope
|
||||
* @param scope The configuration scope
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user