Skip to content

Commit edc3958

Browse files
committed
fix(mcp): surface managed-mcp.json errors in headless startup
The headless (-p) MCP consumer destructured only servers from the config promise and discarded errors. A fatal managed-mcp.json fail-closes every file-based MCP source, so scripted users saw an empty server list with no diagnostic — indistinguishable from an intentionally empty configuration. Emit the config errors on stderr in non-interactive sessions (interactive already surfaces them via the MCP error UI).
1 parent 535cb24 commit edc3958

3 files changed

Lines changed: 59 additions & 2 deletions

File tree

src/main.tsx

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ import { registerMcpXaaIdpCommand } from 'src/commands/mcp/xaaIdpCommand.js';
147147
import { fetchClaudeAIMcpConfigsIfEligible } from 'src/services/mcp/claudeai.js';
148148
import { clearServerCache } from 'src/services/mcp/client.js';
149149
import { areMcpConfigsAllowedWithEnterpriseMcpConfig, dedupClaudeAiMcpServers, doesEnterpriseMcpConfigExist, filterMcpServersByPolicy, getClaudeCodeMcpConfigs, getMcpServerSignature, parseMcpConfig, parseMcpConfigFromFilePath } from 'src/services/mcp/config.js';
150+
import { getHeadlessMcpConfigWarnings } from 'src/services/mcp/headlessErrors.js';
151+
import type { PluginError } from 'src/types/plugin.js';
150152
import { excludeCommandsByServer, excludeResourcesByServer } from 'src/services/mcp/utils.js';
151153
import { isXaaEnabled } from 'src/services/mcp/xaaIdpLogin.js';
152154
import { getRelevantTips } from 'src/services/tips/tipRegistry.js';
@@ -1778,7 +1780,8 @@ async function run(): Promise<CommanderCommand> {
17781780
// only explicit --mcp-config works. dynamicMcpConfig is spread onto
17791781
// allMcpConfigs downstream so it survives this skip.
17801782
const mcpConfigPromise = (strictMcpConfig || isBareMode() ? Promise.resolve({
1781-
servers: {} as Record<string, ScopedMcpServerConfig>
1783+
servers: {} as Record<string, ScopedMcpServerConfig>,
1784+
errors: [] as PluginError[]
17821785
}) : getClaudeCodeMcpConfigs(dynamicMcpConfig)).then(result => {
17831786
mcpConfigResolvedMs = Date.now() - mcpConfigStart;
17841787
return result;
@@ -2334,8 +2337,16 @@ async function run(): Promise<CommanderCommand> {
23342337
}
23352338

23362339
const {
2337-
servers: existingMcpConfigs
2340+
servers: existingMcpConfigs,
2341+
errors: mcpConfigErrors = []
23382342
} = await mcpConfigPromise;
2343+
// Headless (-p) has no MCP-error UI, so a fatal managed-mcp.json fail-closes
2344+
// every file-based source with no diagnostic — indistinguishable from an
2345+
// intentionally empty config. Surface those errors on stderr so scripted
2346+
// users see why nothing loaded. Interactive surfaces them via the MCP UI.
2347+
for (const line of getHeadlessMcpConfigWarnings(isNonInteractiveSession, mcpConfigErrors)) {
2348+
process.stderr.write(`${line}\n`);
2349+
}
23392350
// CLI flag (--mcp-config) should override file-based configs, matching settings precedence
23402351
const allMcpConfigs = {
23412352
...existingMcpConfigs,
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { expect, test } from 'bun:test'
2+
3+
import type { PluginError } from '../../types/plugin.js'
4+
import { getHeadlessMcpConfigWarnings } from './headlessErrors.js'
5+
6+
const managedError: PluginError = {
7+
type: 'generic-error',
8+
source: '/managed/managed-mcp.json',
9+
error: 'Managed MCP config is invalid (mcpServers.__proto__): reserved name',
10+
}
11+
12+
test('emits a warning line per error in a headless session', () => {
13+
const lines = getHeadlessMcpConfigWarnings(true, [managedError])
14+
expect(lines).toEqual([
15+
'Warning: Managed MCP config is invalid (mcpServers.__proto__): reserved name',
16+
])
17+
})
18+
19+
test('stays silent in an interactive session (MCP UI surfaces errors there)', () => {
20+
expect(getHeadlessMcpConfigWarnings(false, [managedError])).toEqual([])
21+
})
22+
23+
test('emits nothing when there are no errors', () => {
24+
expect(getHeadlessMcpConfigWarnings(true, [])).toEqual([])
25+
})

src/services/mcp/headlessErrors.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { getPluginErrorMessage, type PluginError } from '../../types/plugin.js'
2+
3+
/**
4+
* Build the stderr warning lines a headless (`-p`) session should emit for MCP
5+
* config errors.
6+
*
7+
* Interactive sessions surface these through the MCP error UI, but headless has
8+
* no such surface: a fatal managed-mcp.json fail-closes every file-based source
9+
* and, without this, the caller sees an empty server list with no reason why —
10+
* indistinguishable from an intentionally empty config. Returns [] when not
11+
* headless or when there is nothing to report, so the caller writes nothing.
12+
*/
13+
export function getHeadlessMcpConfigWarnings(
14+
isNonInteractiveSession: boolean,
15+
errors: PluginError[],
16+
): string[] {
17+
if (!isNonInteractiveSession || errors.length === 0) {
18+
return []
19+
}
20+
return errors.map(error => `Warning: ${getPluginErrorMessage(error)}`)
21+
}

0 commit comments

Comments
 (0)