diff --git a/scripts/lib/install/apply.js b/scripts/lib/install/apply.js index ff49ddf53..e58b2044d 100644 --- a/scripts/lib/install/apply.js +++ b/scripts/lib/install/apply.js @@ -257,6 +257,31 @@ function isMcpConfigPath(filePath) { return basename === '.mcp.json' || basename === 'mcp.json'; } +function withMcpMergeExclusions(plan, disabledServers) { + if (disabledServers.length === 0) { + return plan; + } + const filterOperation = operation => ( + operation.kind === 'merge-json' && isMcpConfigPath(operation.destinationPath) + && operation.mergePayload !== undefined + ? { + ...operation, + mergePayload: filterMcpConfig(operation.mergePayload, disabledServers).config, + } + : operation + ); + // Writes, recovery checkpoints, and the final ownership ledger must use the + // same payload. Repair and uninstall later run without the install environment. + return { + ...plan, + operations: plan.operations.map(filterOperation), + statePreview: { + ...plan.statePreview, + operations: plan.statePreview.operations.map(filterOperation), + }, + }; +} + function assertSafeInstallOperation(plan, operation) { if (!operation || typeof operation.destinationPath !== 'string') { throw new Error('Refusing to apply install operation: missing destination path.'); @@ -705,7 +730,8 @@ function preflightQwenAgentOperations(plan) { } } -function previewInstallPlan(plan) { +function previewInstallPlan(rawPlan) { + const plan = withMcpMergeExclusions(rawPlan, parseDisabledMcpServers(process.env.ECC_DISABLED_MCPS)); preflightQwenAgentOperations(plan); assertOpenCodeScriptBoundary(plan); assertOpenCodeHookDeactivationReady(plan); @@ -768,7 +794,9 @@ function applyInstallPlan(plan, dependencies = {}) { : applyInstallPlanLocked(plan, dependencies, false); } -function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = false) { +function applyInstallPlanLocked(rawPlan, dependencies = {}, settingsLockHeld = false) { + const disabledServers = parseDisabledMcpServers(process.env.ECC_DISABLED_MCPS); + const plan = withMcpMergeExclusions(rawPlan, disabledServers); preflightQwenAgentOperations(plan); const persistInstallState = dependencies.writeInstallState || writeInstallState; const beforeInstallStateRead = dependencies.beforeInstallStateRead; @@ -796,7 +824,6 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals operations: migration.appliedOperations, }; const preparedClaudeSettings = preflightClaudeSettingsOperations(appliedPlan); - const disabledServers = parseDisabledMcpServers(process.env.ECC_DISABLED_MCPS); const linkIndex = buildLinkIndexForPlan(appliedPlan); const hasLegacyMigration = migration.legacyOperationsToRemove.length > 0; const hookRemovalCount = appliedPlan.operations.filter(operation => ( @@ -878,17 +905,11 @@ function applyInstallPlanLocked(plan, dependencies = {}, settingsLockHeld = fals throw new Error(`Missing merge payload for ${operation.destinationPath}`); } - const filteredPayload = ( - isMcpConfigPath(operation.destinationPath) && disabledServers.length > 0 - ) - ? filterMcpConfig(payload, disabledServers).config - : payload; - const currentValue = readOptionalJsonObject( operation.destinationPath, 'existing JSON config' ); - const mergedValue = deepMergeJson(currentValue, filteredPayload); + const mergedValue = deepMergeJson(currentValue, payload); fs.writeFileSync(operation.destinationPath, formatJson(mergedValue), 'utf8'); writtenDestinations.add(operation.destinationPath); continue; diff --git a/tests/lib/install-mcp-exclusions.test.js b/tests/lib/install-mcp-exclusions.test.js new file mode 100644 index 000000000..3e47caf87 --- /dev/null +++ b/tests/lib/install-mcp-exclusions.test.js @@ -0,0 +1,253 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { test } = require('node:test'); + +const { createInstallPlanFromRequest } = require('../../scripts/lib/install/runtime'); +const { applyInstallPlan, previewInstallPlan } = require('../../scripts/lib/install/apply'); +const { readInstallState } = require('../../scripts/lib/install-state'); +const { + buildDoctorReport, + repairInstalledStates, + uninstallInstalledStates, +} = require('../../scripts/lib/install-lifecycle'); + +const EXCLUDED_SERVER = { command: 'excluded-mcp', args: ['--stdio'] }; +const INCLUDED_SERVER = { command: 'included-mcp', args: ['--stdio'] }; +const USER_SERVER = { command: 'user-mcp' }; + +function writeJson(filePath, value) { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, `${JSON.stringify(value, null, 2)}\n`); +} + +function createFixture(t, { allExcluded = false, userConfig = false } = {}) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-mcp-exclusions-')); + const previousDisabled = process.env.ECC_DISABLED_MCPS; + delete process.env.ECC_DISABLED_MCPS; + t.after(() => { + if (previousDisabled === undefined) delete process.env.ECC_DISABLED_MCPS; + else process.env.ECC_DISABLED_MCPS = previousDisabled; + fs.rmSync(root, { recursive: true, force: true }); + }); + const sourceRoot = path.join(root, 'source'); + const homeDir = path.join(root, 'home'); + const projectRoot = path.join(root, 'project'); + const targetRoot = path.join(projectRoot, '.cursor'); + const configPath = path.join(targetRoot, 'mcp.json'); + const statePath = path.join(targetRoot, 'ecc-install-state.json'); + const json = (relativePath, value) => writeJson(path.join(sourceRoot, relativePath), value); + const module = (id, paths) => ({ + id, kind: 'platform', description: 'MCP lifecycle fixture', paths, + targets: ['cursor'], dependencies: [], defaultInstall: false, + cost: 'light', stability: 'stable', + }); + json('package.json', { version: '1.0.0' }); + json('manifests/install-modules.json', { + version: 1, + modules: [ + module('platform-configs', ['mcp-configs', 'scripts/after-mcp.js']), + module('helper-tools', ['scripts/helper.js']), + ], + }); + json('manifests/install-profiles.json', { + version: 1, + profiles: { minimal: { description: 'MCP fixture', modules: ['platform-configs'] } }, + }); + json('.mcp.json', { + mcpServers: allExcluded + ? { excluded: EXCLUDED_SERVER } + : { excluded: EXCLUDED_SERVER, included: INCLUDED_SERVER }, + }); + fs.mkdirSync(path.join(sourceRoot, 'mcp-configs'), { recursive: true }); + fs.mkdirSync(path.join(sourceRoot, 'scripts'), { recursive: true }); + fs.writeFileSync(path.join(sourceRoot, 'scripts/after-mcp.js'), 'module.exports = "after MCP";\n'); + fs.writeFileSync(path.join(sourceRoot, 'scripts/helper.js'), 'module.exports = "helper";\n'); + fs.mkdirSync(homeDir, { recursive: true }); + fs.mkdirSync(projectRoot, { recursive: true }); + if (userConfig) writeJson(configPath, { + mcpServers: { excluded: EXCLUDED_SERVER, custom: USER_SERVER }, + }); + const options = { sourceRoot, homeDir, projectRoot, env: {} }; + const lifecycleOptions = { repoRoot: sourceRoot, homeDir, projectRoot, targets: ['cursor'], env: {} }; + const plan = (moduleIds = ['platform-configs']) => createInstallPlanFromRequest({ + mode: 'manifest', target: 'cursor', profileId: null, moduleIds, + includeComponentIds: [], excludeComponentIds: [], hookConsent: 'declined', + }, options); + return { + configPath, + statePath, + plan, + afterMcpPath: path.join(targetRoot, 'scripts/after-mcp.js'), + install: (dependencies, moduleIds) => applyInstallPlan(plan(moduleIds), dependencies), + readConfig: () => JSON.parse(fs.readFileSync(configPath, 'utf8')), + writeConfig: value => writeJson(configPath, value), + repair: () => repairInstalledStates(lifecycleOptions), + doctor: () => buildDoctorReport(lifecycleOptions), + uninstall: () => uninstallInstalledStates(lifecycleOptions), + }; +} + +function installWithExclusion(fixture, dependencies) { + process.env.ECC_DISABLED_MCPS = 'excluded'; + try { + return fixture.install(dependencies); + } finally { + // Lifecycle commands must honor recorded ownership without this environment. + delete process.env.ECC_DISABLED_MCPS; + } +} + +function lifecycleResult(report) { + assert.equal(report.results.length, 1); + assert.notEqual(report.results[0].status, 'error', report.results[0].error); + return report.results[0]; +} + +function assertRecordedPayload(fixture, expectedServers) { + const operation = readInstallState(fixture.statePath).operations.find(entry => ( + entry.kind === 'merge-json' && entry.destinationPath === fixture.configPath + )); + assert.ok(operation, 'The installed JSON merge must remain tracked'); + assert.deepEqual(operation.mergePayload, { mcpServers: expectedServers }); +} + +test('repair preserves MCP exclusions without the installation environment', t => { + const fixture = createFixture(t); + installWithExclusion(fixture); + assert.deepEqual(fixture.readConfig(), { mcpServers: { included: INCLUDED_SERVER } }); + + // The remaining managed server must still be repairable. + fixture.writeConfig({ mcpServers: { custom: USER_SERVER } }); + const repair = lifecycleResult(fixture.repair()); + assert.ok(repair.repairedPaths.includes(fixture.configPath)); + assert.deepEqual(fixture.readConfig(), { + mcpServers: { custom: USER_SERVER, included: INCLUDED_SERVER }, + }); + assertRecordedPayload(fixture, { included: INCLUDED_SERVER }); + assert.equal(lifecycleResult(fixture.doctor()).status, 'ok'); + assert.equal(lifecycleResult(fixture.uninstall()).status, 'uninstalled'); + assert.deepEqual(fixture.readConfig(), { mcpServers: { custom: USER_SERVER } }); +}); + +test('repair does not restore MCP servers when every bundled server was excluded', t => { + const fixture = createFixture(t, { allExcluded: true }); + installWithExclusion(fixture); + assert.deepEqual(fixture.readConfig(), { mcpServers: {} }); + const repair = lifecycleResult(fixture.repair()); + assert.equal(repair.status, 'ok'); + assert.deepEqual(repair.repairedPaths, []); + assert.deepEqual(fixture.readConfig(), { mcpServers: {} }); + assertRecordedPayload(fixture, {}); +}); + +test('uninstall preserves a user MCP server excluded from the install', t => { + const fixture = createFixture(t, { userConfig: true }); + installWithExclusion(fixture); + assert.deepEqual(fixture.readConfig(), { + mcpServers: { excluded: EXCLUDED_SERVER, custom: USER_SERVER, included: INCLUDED_SERVER }, + }); + assert.equal(lifecycleResult(fixture.uninstall()).status, 'uninstalled'); + assert.deepEqual(fixture.readConfig(), { + mcpServers: { excluded: EXCLUDED_SERVER, custom: USER_SERVER }, + }); + assert.ok(!fs.existsSync(fixture.statePath)); +}); + +for (const failure of ['after MCP write', 'final state write']) { + test(`failed install preserves MCP exclusions (${failure})`, t => { + const fixture = createFixture(t, { userConfig: true }); + let stateWrites = 0; + const dependencies = failure === 'after MCP write' ? { + beforeOperationWrite({ operation }) { + if (operation.destinationPath === fixture.afterMcpPath) { + throw new Error('Injected failure after MCP write'); + } + }, + } : { + beforeInstallStateWrite() { + stateWrites += 1; + if (stateWrites === 2) throw new Error('Injected final state write failure'); + }, + }; + assert.throws(() => installWithExclusion(fixture, dependencies), /Injected.*failure/); + assert.deepEqual(fixture.readConfig().mcpServers.included, INCLUDED_SERVER); + assert.ok(fs.existsSync(fixture.statePath), 'Partial installs retain a recovery ledger'); + lifecycleResult(fixture.repair()); + assert.equal(lifecycleResult(fixture.uninstall()).status, 'uninstalled'); + assert.deepEqual(fixture.readConfig(), { + mcpServers: { excluded: EXCLUDED_SERVER, custom: USER_SERVER }, + }); + }); +} + +test('repeated and selective installs retain the filtered MCP ownership', t => { + const fixture = createFixture(t, { userConfig: true }); + installWithExclusion(fixture); + installWithExclusion(fixture); + // Installing a different module does not select MCP configuration again. + fixture.install(undefined, ['helper-tools']); + lifecycleResult(fixture.repair()); + assertRecordedPayload(fixture, { included: INCLUDED_SERVER }); + assert.equal(lifecycleResult(fixture.uninstall()).status, 'uninstalled'); + assert.deepEqual(fixture.readConfig(), { + mcpServers: { excluded: EXCLUDED_SERVER, custom: USER_SERVER }, + }); +}); + +test('an install without exclusions still repairs and removes both managed servers', t => { + const fixture = createFixture(t); + fixture.install(); + fixture.writeConfig({ mcpServers: { custom: USER_SERVER } }); + lifecycleResult(fixture.repair()); + assert.deepEqual(fixture.readConfig(), { + mcpServers: { custom: USER_SERVER, excluded: EXCLUDED_SERVER, included: INCLUDED_SERVER }, + }); + assert.equal(lifecycleResult(fixture.uninstall()).status, 'uninstalled'); + assert.deepEqual(fixture.readConfig(), { mcpServers: { custom: USER_SERVER } }); +}); + +test('preview and apply share MCP filtering without changing the plan or other JSON merges', t => { + const fixture = createFixture(t); + const originalPlan = fixture.plan(); + const mcpOperation = originalPlan.operations.find(operation => operation.kind === 'merge-json'); + const ordinaryPath = path.join(path.dirname(fixture.configPath), 'preferences.json'); + const ordinaryMerge = { ...mcpOperation, destinationPath: ordinaryPath }; + const plan = { + ...originalPlan, + operations: [...originalPlan.operations, ordinaryMerge], + statePreview: { + ...originalPlan.statePreview, + operations: [...originalPlan.statePreview.operations, ordinaryMerge], + }, + }; + const before = structuredClone(plan); + const expectedMcp = { mcpServers: { included: INCLUDED_SERVER } }; + const expectedOrdinary = { mcpServers: { excluded: EXCLUDED_SERVER, included: INCLUDED_SERVER } }; + const payloadFor = (operations, destinationPath) => operations.find(operation => ( + operation.destinationPath === destinationPath + )).mergePayload; + process.env.ECC_DISABLED_MCPS = 'excluded'; + try { + const preview = previewInstallPlan(plan); + assert.ok(!fs.existsSync(fixture.statePath)); + assert.ok(!fs.existsSync(fixture.configPath)); + for (const operations of [preview.operations, preview.statePreview.operations]) { + assert.deepEqual(payloadFor(operations, fixture.configPath), expectedMcp); + assert.deepEqual(payloadFor(operations, ordinaryPath), expectedOrdinary); + } + assert.deepEqual(plan, before, 'Preview must not mutate the original plan'); + applyInstallPlan(plan); + } finally { + delete process.env.ECC_DISABLED_MCPS; + } + assert.deepEqual(plan, before, 'Apply must not mutate the original plan'); + assert.deepEqual(fixture.readConfig(), expectedMcp); + assert.deepEqual(JSON.parse(fs.readFileSync(ordinaryPath, 'utf8')), expectedOrdinary); + const operations = readInstallState(fixture.statePath).operations; + assert.deepEqual(payloadFor(operations, fixture.configPath), expectedMcp); + assert.deepEqual(payloadFor(operations, ordinaryPath), expectedOrdinary); +});