fix(codex): make plugin install failures triageable and skip unsupported CLIs

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();
+    });
   });
 });

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<typeof DefaultMCPClientConfig>;
 
+/**
+ * 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>): 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<boolean> {
-    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 });
   }
 }