diff --git a/src/steps/add-mcp-server-to-clients/clients/__tests__/codex.test.ts b/src/steps/add-mcp-server-to-clients/clients/__tests__/codex.test.ts index 8256997d7..99886d3cc 100644 --- a/src/steps/add-mcp-server-to-clients/clients/__tests__/codex.test.ts +++ b/src/steps/add-mcp-server-to-clients/clients/__tests__/codex.test.ts @@ -66,6 +66,18 @@ describe('CodexMCPClient', () => { await expect(client.isPluginInstalled()).resolves.toBe(false); }); + it('reads config.toml from CODEX_HOME when it is set', async () => { + vi.stubEnv('CODEX_HOME', '/custom/codex-home'); + readFileSyncMock.mockReturnValue('[marketplaces.posthog]\n'); + const client = new CodexMCPClient(); + await expect(client.isPluginInstalled()).resolves.toBe(true); + expect(readFileSyncMock).toHaveBeenCalledWith( + '/custom/codex-home/config.toml', + 'utf-8', + ); + vi.unstubAllEnvs(); + }); + it('returns false when config.toml cannot be read', async () => { readFileSyncMock.mockImplementation(() => { throw new Error('ENOENT'); @@ -166,9 +178,49 @@ describe('CodexMCPClient', () => { }); describe('supportsPlugin', () => { - it('returns true when codex is in PATH', () => { + const HELP_WITH_PLUGIN = [ + 'Commands:', + ' mcp Manage external MCP servers for Codex', + ' plugin Manage Codex plugins', + ' help Print this message', + ].join('\n'); + + const HELP_WITHOUT_PLUGIN = [ + 'Commands:', + ' exec Run Codex non-interactively', + ' login Manage login', + ].join('\n'); + + it('returns true when the CLI lists a plugin subcommand', () => { + spawnSyncMock.mockReturnValue({ status: 0, stdout: HELP_WITH_PLUGIN }); const client = new CodexMCPClient(); expect(client.supportsPlugin()).toBe(true); + expect(spawnSyncMock).toHaveBeenCalledWith(CODEX_PATH, ['--help'], { + encoding: 'utf-8', + }); + }); + + it('returns false on an older CLI with no plugin subcommand', () => { + spawnSyncMock.mockReturnValue({ status: 0, stdout: HELP_WITHOUT_PLUGIN }); + const client = new CodexMCPClient(); + expect(client.supportsPlugin()).toBe(false); + }); + + it('returns false when the binary cannot be spawned', () => { + spawnSyncMock.mockReturnValue({ + status: null, + error: new Error('spawn ENOENT'), + }); + const client = new CodexMCPClient(); + expect(client.supportsPlugin()).toBe(false); + }); + + it('caches the probe result across calls', () => { + spawnSyncMock.mockReturnValue({ status: 0, stdout: HELP_WITH_PLUGIN }); + const client = new CodexMCPClient(); + client.supportsPlugin(); + client.supportsPlugin(); + expect(spawnSyncMock).toHaveBeenCalledTimes(1); }); it('returns false when codex binary is not found', () => { @@ -177,6 +229,7 @@ describe('CodexMCPClient', () => { }); const client = new CodexMCPClient(); expect(client.supportsPlugin()).toBe(false); + expect(spawnSyncMock).not.toHaveBeenCalled(); }); }); @@ -219,5 +272,125 @@ describe('CodexMCPClient', () => { }), ); }); + + it('reports the spawn error when the process never starts', async () => { + spawnSyncMock.mockReturnValue({ + status: null, + signal: null, + error: Object.assign(new Error('spawn /usr/bin/codex EACCES'), { + code: 'EACCES', + }), + stdout: '', + stderr: '', + }); + const client = new CodexMCPClient(); + await expect(client.installPlugin()).resolves.toEqual({ success: false }); + expect(analytics.captureException).toHaveBeenCalledWith( + expect.objectContaining({ + message: + 'Codex plugin install failed: spawn error: spawn /usr/bin/codex EACCES', + }), + ); + }); + + it('reports the exit code and stdout when stderr is empty', async () => { + spawnSyncMock.mockReturnValue({ + status: 3, + signal: null, + stdout: 'something broke', + stderr: '', + }); + const client = new CodexMCPClient(); + await expect(client.installPlugin()).resolves.toEqual({ success: false }); + expect(analytics.captureException).toHaveBeenCalledWith( + expect.objectContaining({ + message: + 'Codex plugin install failed: exit 3 | stdout: something broke', + }), + ); + }); + + it('never captures an exception with an empty tail', async () => { + spawnSyncMock.mockReturnValue({ + status: 1, + signal: null, + stdout: '', + stderr: '', + }); + const client = new CodexMCPClient(); + await client.installPlugin(); + expect(analytics.captureException).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Codex plugin install failed: exit 1', + }), + ); + }); + + it('falls back to a placeholder when spawnSync reports nothing at all', async () => { + spawnSyncMock.mockReturnValue({ + status: null, + signal: null, + stdout: '', + stderr: '', + }); + const client = new CodexMCPClient(); + await client.installPlugin(); + expect(analytics.captureException).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Codex plugin install failed: no output, status unavailable', + }), + ); + }); + + it('does not capture when the CLI is too old for the marketplace subcommand', async () => { + spawnSyncMock.mockReturnValue({ + status: 2, + stderr: "error: unexpected argument 'marketplace' found\n", + }); + const client = new CodexMCPClient(); + await expect(client.installPlugin()).resolves.toEqual({ success: false }); + expect(analytics.captureException).not.toHaveBeenCalled(); + }); + + it('does not capture on a transient network failure', async () => { + spawnSyncMock.mockReturnValue({ + status: 128, + stderr: + 'fatal: unable to access https://github.com/PostHog/ai-plugin.git/: LibreSSL SSL_connect: SSL_ERROR_SYSCALL', + }); + const client = new CodexMCPClient(); + await expect(client.installPlugin()).resolves.toEqual({ success: false }); + expect(analytics.captureException).not.toHaveBeenCalled(); + }); + + it('does not capture when the user interrupts the install', async () => { + spawnSyncMock.mockReturnValue({ + status: null, + signal: 'SIGINT', + stdout: '', + stderr: '', + }); + const client = new CodexMCPClient(); + await expect(client.installPlugin()).resolves.toEqual({ success: false }); + expect(analytics.captureException).not.toHaveBeenCalled(); + }); + + it('clears the stale cache under CODEX_HOME when it is set', async () => { + vi.stubEnv('CODEX_HOME', '/custom/codex-home'); + spawnSyncMock + .mockReturnValueOnce({ + status: 1, + stderr: + "Error: marketplace 'posthog' is already added from a different source", + }) + .mockReturnValueOnce({ status: 0, stderr: '' }); + const client = new CodexMCPClient(); + await expect(client.installPlugin()).resolves.toEqual({ success: true }); + expect(fs.rmSync).toHaveBeenCalledWith( + '/custom/codex-home/.tmp/marketplaces/posthog', + { recursive: true, force: true }, + ); + vi.unstubAllEnvs(); + }); }); }); diff --git a/src/steps/add-mcp-server-to-clients/clients/codex.ts b/src/steps/add-mcp-server-to-clients/clients/codex.ts index 4c06a6bc9..8a84dd924 100644 --- a/src/steps/add-mcp-server-to-clients/clients/codex.ts +++ b/src/steps/add-mcp-server-to-clients/clients/codex.ts @@ -1,4 +1,5 @@ import { z } from 'zod'; +import type { SpawnSyncReturns } from 'node:child_process'; import { execSync, spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as os from 'node:os'; @@ -20,9 +21,68 @@ export const CodexMCPConfig = DefaultMCPClientConfig; export type CodexMCPConfig = z.infer; +/** + * Codex resolves its state directory from CODEX_HOME, falling back to + * ~/.codex. Anything that reads config.toml or the marketplace cache has to + * honour the same override, or it silently operates on the wrong directory. + */ +function codexHome(): string { + return process.env.CODEX_HOME ?? path.join(os.homedir(), '.codex'); +} + +/** + * `spawnSync` signals failure three different ways — a spawn-level `error` + * (ENOENT/EACCES), a non-zero `status`, or a terminating `signal` — and Codex + * sometimes writes the reason to stdout rather than stderr. Reading only + * stderr turns every one of those into an empty message, so fold them all in. + */ +function describeSpawnFailure(result: SpawnSyncReturns): string { + const parts: string[] = []; + if (result.error) parts.push(`spawn error: ${result.error.message}`); + if (result.signal) parts.push(`killed by ${result.signal}`); + else if (typeof result.status === 'number') + parts.push(`exit ${result.status}`); + + const stderr = (result.stderr ?? '').trim(); + const stdout = (result.stdout ?? '').trim(); + if (stderr) parts.push(`stderr: ${stderr}`); + if (stdout) parts.push(`stdout: ${stdout}`); + + // Never hand back an empty string — a blank tail is exactly what made this + // failure untriageable in the first place. + return parts.length > 0 ? parts.join(' | ') : 'no output, status unavailable'; +} + +/** + * A Codex CLI predating `plugin marketplace` rejects the subcommand outright. + * That is a user-environment limitation, not a wizard bug. + */ +function isUnsupportedSubcommand(output: string): boolean { + const lower = output.toLowerCase(); + return ( + lower.includes("unexpected argument 'marketplace'") || + lower.includes("unexpected argument 'plugin'") || + lower.includes('unrecognized subcommand') + ); +} + +/** Transient network trouble while Codex clones the marketplace repo. */ +function isTransientNetworkFailure(output: string): boolean { + const lower = output.toLowerCase(); + return ( + lower.includes('unable to access') || + lower.includes('could not resolve host') || + lower.includes('connection reset') || + lower.includes('connection timed out') || + lower.includes('ssl_connect') || + lower.includes('ssl routines') + ); +} + export class CodexMCPClient extends DefaultMCPClient implements PluginCapable { name = 'Codex'; private codexBinaryPath: string | null = null; + private pluginSupported: boolean | null = null; constructor() { super(); @@ -112,11 +172,23 @@ export class CodexMCPClient extends DefaultMCPClient implements PluginCapable { } supportsPlugin(): boolean { - return this.findCodexBinary() !== null; + if (this.pluginSupported !== null) return this.pluginSupported; + + const binary = this.findCodexBinary(); + if (!binary) return (this.pluginSupported = false); + + // Probing `codex plugin --help` is no good: clap short-circuits on --help + // and an older CLI happily prints top-level help with exit 0. The command + // list from `codex --help` is the reliable signal — older CLIs have no + // `plugin` entry at all. + const help = spawnSync(binary, ['--help'], { encoding: 'utf-8' }); + if (help.error || help.status !== 0) return (this.pluginSupported = false); + + return (this.pluginSupported = /^\s+plugin\s+/m.test(help.stdout ?? '')); } isPluginInstalled(): Promise { - const configPath = path.join(os.homedir(), '.codex', 'config.toml'); + const configPath = path.join(codexHome(), 'config.toml'); try { const contents = fs.readFileSync(configPath, 'utf-8'); // Marketplace installs appear as [marketplaces.posthog] in config.toml @@ -145,8 +217,7 @@ export class CodexMCPClient extends DefaultMCPClient implements PluginCapable { (result.stderr ?? '').includes('already added from a different source') ) { const staleDir = path.join( - os.homedir(), - '.codex', + codexHome(), '.tmp', 'marketplaces', 'posthog', @@ -159,14 +230,25 @@ export class CodexMCPClient extends DefaultMCPClient implements PluginCapable { result = run(); } - if (result.status !== 0) { + if (result.status === 0) return Promise.resolve({ success: true }); + + const details = describeSpawnFailure(result); + + // Not our bug and not actionable: an out-of-date Codex CLI, a user + // interrupt (Ctrl-C during the marketplace clone), or a flaky network. + const notActionable = + isUnsupportedSubcommand(details) || + isTransientNetworkFailure(details) || + result.signal === 'SIGINT' || + result.signal === 'SIGTERM'; + + if (!notActionable) { analytics.captureException( - new Error(`Codex plugin install failed: ${result.stderr ?? ''}`), + new Error(`Codex plugin install failed: ${details}`), ); - return Promise.resolve({ success: false }); } - return Promise.resolve({ success: true }); + return Promise.resolve({ success: false }); } }