diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6b36d851..c2cabe82 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -26,3 +26,11 @@ jobs: - uses: oven-sh/setup-bun@v2 - run: bun install --frozen-lockfile - run: bun run test:cli:integration + + windows-codex-mcp: + runs-on: windows-latest + steps: + - uses: actions/checkout@v4 + - uses: oven-sh/setup-bun@v2 + - run: bun install --frozen-lockfile + - run: bun run --cwd cli test -- src/codex/utils/codexMcpProxy.test.ts diff --git a/cli/README.md b/cli/README.md index 5a8d3f05..eafc56a7 100644 --- a/cli/README.md +++ b/cli/README.md @@ -92,6 +92,23 @@ See `src/ui/doctor.ts`. - `hapi hub` - Start the bundled hub (single binary workflow). - `hapi server` - Alias for `hapi hub`. +### Codex MCP servers + +Codex sessions keep the MCP servers configured in the user's Codex +`config.toml`. HAPI adds its own `hapi` bridge without replacing other user +servers. Runner-spawned Codex sessions copy only `config.toml` into their +temporary `CODEX_HOME`, so MCP settings are preserved while authentication +state remains isolated. The `hapi` server name is reserved by HAPI. + +On Windows, known package-manager shims (`uvx`, `npx`, `npm`, `pnpm`, `yarn`, +`bunx`, and `.cmd`/`.bat` commands) use a short-lived HAPI stdio compatibility +proxy before reaching the configured MCP server. The proxy keeps the original +command and arguments in a session-temporary file and forwards MCP JSON-RPC +bytes without putting environment-variable values into arguments or that file. +Secret and network variables still need to be listed in the MCP entry's +`env_vars` (or supplied through `env`); HAPI does not forward the whole host +environment automatically. + ## Configuration See `src/configuration.ts` for all options. diff --git a/cli/src/codex/codexRemoteLauncher.test.ts b/cli/src/codex/codexRemoteLauncher.test.ts index 27ab11f0..89c46750 100644 --- a/cli/src/codex/codexRemoteLauncher.test.ts +++ b/cli/src/codex/codexRemoteLauncher.test.ts @@ -1560,6 +1560,77 @@ describe('codexRemoteLauncher', () => { }); }); + it('forwards user-configured MCP servers into fresh and resumed threads', async () => { + harness.configReadResponse = { + config: { + mcp_servers: { + 'package-manager': { + command: 'uvx', + args: ['example-mcp', 'serve'], + environment_id: 'local', + enabled: true, + tool_timeout_sec: 60 + }, + remote: { + url: 'https://example.test/mcp', + bearer_token_env_var: 'REMOTE_MCP_TOKEN' + } + } + } + }; + + const fresh = createSessionStub(); + await codexRemoteLauncher(fresh.session as never); + const freshConfig = harness.startThreadParams[0]?.config as Record | undefined; + const freshPackageManager = freshConfig?.['mcp_servers.package-manager'] as { + command?: string; + args?: string[]; + } | undefined; + expect(harness.startThreadParams[0]?.config).toMatchObject({ + 'mcp_servers.remote': { + url: 'https://example.test/mcp', + bearer_token_env_var: 'REMOTE_MCP_TOKEN' + } + }); + expect(freshPackageManager).toEqual(expect.objectContaining({ + environment_id: 'local', + enabled: true, + tool_timeout_sec: 60 + })); + if (process.platform === 'win32') { + expect(freshPackageManager?.args).toContain('mcp-proxy'); + } else { + expect(freshPackageManager).toMatchObject({ + command: 'uvx', + args: ['example-mcp', 'serve'] + }); + } + + harness.startThreadParams = []; + const resumed = createSessionStub(); + resumed.session.sessionId = 'thread-existing'; + await codexRemoteLauncher(resumed.session as never); + const resumedConfig = harness.resumeThreadParams[0]?.config as Record | undefined; + const resumedPackageManager = resumedConfig?.['mcp_servers.package-manager'] as { + command?: string; + args?: string[]; + } | undefined; + expect(harness.resumeThreadParams[0]?.config).toMatchObject({ + 'mcp_servers.remote': { + url: 'https://example.test/mcp' + } + }); + expect(resumedPackageManager).toBeDefined(); + if (process.platform === 'win32') { + expect(resumedPackageManager?.args).toContain('mcp-proxy'); + } else { + expect(resumedPackageManager).toMatchObject({ + command: 'uvx', + args: ['example-mcp', 'serve'] + }); + } + }); + it('keeps remote sessions working when config/read is unavailable', async () => { harness.failConfigRead = true; const { session } = createSessionStub(); diff --git a/cli/src/codex/codexRemoteLauncher.ts b/cli/src/codex/codexRemoteLauncher.ts index 0562497d..d922a7ca 100644 --- a/cli/src/codex/codexRemoteLauncher.ts +++ b/cli/src/codex/codexRemoteLauncher.ts @@ -21,6 +21,12 @@ import { buildTurnStartParams, type CodexContextManagementConfig } from './utils/appServerConfig'; +import { + extractCodexMcpServers, + mergeCodexMcpServers, + type CodexMcpServersConfig +} from './utils/codexMcpServers'; +import { prepareCodexMcpServers } from './utils/codexMcpProxy'; import type { SkillMetadata, ThreadGoal, ThreadGoalStatus } from './appServerTypes'; import { shouldIgnoreTerminalEvent } from './utils/terminalEventGuard'; import { parseCodexSpecialCommand } from './codexSpecialCommands'; @@ -232,6 +238,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { private reasoningProcessor: ReasoningProcessor | null = null; private diffProcessor: DiffProcessor | null = null; private happyServer: HappyServer | null = null; + private mcpProxyCleanup: (() => Promise) | null = null; private abortController: AbortController = new AbortController(); /** Invalidates queued-message steer handlers after abort or cleanup. */ private steerEpoch = 0; @@ -3528,7 +3535,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { failPendingAgentStartsForSpawnArgumentError(spawnAgentError); }); - const { server: happyServer, mcpServers } = await buildHapiMcpBridge(session.client, { + const { server: happyServer, mcpServers: hapiMcpServers } = await buildHapiMcpBridge(session.client, { // In app-server/collab mode, child agents share this MCP bridge. // If the MCP handler writes the title directly, child title calls // leak into the parent HAPI session. Defer the side effect until @@ -3537,6 +3544,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { emitTitleSummary: false }); this.happyServer = happyServer; + let mcpServers: CodexMcpServersConfig = hapiMcpServers; this.setupAbortHandlers(session.client.rpcHandlerManager, { onAbort: () => this.handleAbort(), @@ -3576,6 +3584,22 @@ class CodexRemoteLauncher extends RemoteLauncherBase { cwd: session.path, includeLayers: false })).config; + try { + const userMcpServers = extractCodexMcpServers(effectiveConfig); + const preparedMcpServers = await prepareCodexMcpServers(userMcpServers); + this.mcpProxyCleanup = preparedMcpServers.cleanup; + mcpServers = mergeCodexMcpServers(preparedMcpServers.servers, hapiMcpServers); + if (Object.keys(userMcpServers).length > 0) { + logger.debug(`[Codex] Loaded ${Object.keys(userMcpServers).length} user MCP server(s)`); + } + if (preparedMcpServers.proxiedServerNames.length > 0) { + logger.debug( + `[Codex] Added stdio compatibility proxy for ${preparedMcpServers.proxiedServerNames.length} Windows MCP server(s)` + ); + } + } catch (error) { + logger.warn(`[Codex] Failed to merge user MCP servers; using HAPI bridge only: ${errorMessage(error)}`); + } const modelContextWindow = effectiveConfig.model_context_window; const modelAutoCompactTokenLimit = effectiveConfig.model_auto_compact_token_limit; contextManagementConfig = { @@ -4332,6 +4356,15 @@ class CodexRemoteLauncher extends RemoteLauncherBase { this.happyServer = null; } + if (this.mcpProxyCleanup) { + try { + await this.mcpProxyCleanup(); + } catch (error) { + logger.debug('[codex-remote]: Error cleaning MCP proxy specs', error); + } + this.mcpProxyCleanup = null; + } + this.permissionHandler?.reset(); this.reasoningProcessor?.abort(); this.diffProcessor?.reset(); diff --git a/cli/src/codex/happyMcpStdioProxy.ts b/cli/src/codex/happyMcpStdioProxy.ts new file mode 100644 index 00000000..48a68b04 --- /dev/null +++ b/cli/src/codex/happyMcpStdioProxy.ts @@ -0,0 +1,155 @@ +/** + * HAPI-owned stdio-to-stdio proxy for external Codex MCP servers. + * + * This command is intentionally internal. It lets the compiled HAPI binary + * bridge Windows command shims without requiring a separate Node runtime. + */ + +import { execFileSync, type ChildProcessWithoutNullStreams } from 'node:child_process'; +import { readFile } from 'node:fs/promises'; +import spawn from 'cross-spawn'; + +type StdioProxySpec = { + command: string; + args: string[]; + cwd?: string; +}; + +function parseSpecPath(argv: string[]): string | null { + const index = argv.indexOf('--spec'); + const value = index >= 0 ? argv[index + 1] : undefined; + return typeof value === 'string' && value.trim().length > 0 ? value : null; +} + +function parseSpec(value: unknown): StdioProxySpec { + if (!value || typeof value !== 'object' || Array.isArray(value)) { + throw new Error('MCP proxy spec must be an object'); + } + + const record = value as Record; + if (typeof record.command !== 'string' || record.command.trim().length === 0) { + throw new Error('MCP proxy spec command must be a non-empty string'); + } + if (!Array.isArray(record.args) || record.args.some((arg) => typeof arg !== 'string')) { + throw new Error('MCP proxy spec args must be an array of strings'); + } + if (record.cwd !== undefined && typeof record.cwd !== 'string') { + throw new Error('MCP proxy spec cwd must be a string'); + } + + return { + command: record.command, + args: record.args, + ...(typeof record.cwd === 'string' ? { cwd: record.cwd } : {}) + }; +} + +function resolveWindowsCommand(command: string): string { + if (process.platform !== 'win32' || /[\\/]/.test(command) || /\.(exe|cmd|bat)$/i.test(command)) { + return command; + } + + try { + const entries = execFileSync('where.exe', [command], { + encoding: 'utf8', + windowsHide: true + }) + .split(/\r?\n/) + .map((entry) => entry.trim()) + .filter(Boolean); + return entries.find((entry) => /\.(exe|cmd|bat)$/i.test(entry)) ?? entries[0] ?? command; + } catch { + return command; + } +} + +export async function runHappyMcpStdioProxy(argv: string[]): Promise { + const specPath = parseSpecPath(argv); + if (!specPath) { + process.stderr.write('[hapi-mcp-proxy] Missing --spec path\n'); + process.exitCode = 2; + return; + } + + const pendingInput: Buffer[] = []; + let child: ChildProcessWithoutNullStreams | null = null; + let childReady = false; + let inputEnded = false; + let settled = false; + + const forwardInput = (chunk: Buffer | Uint8Array) => { + const data = Buffer.from(chunk); + if (!child || !childReady) { + pendingInput.push(data); + return; + } + if (!child.stdin.destroyed && !child.stdin.writableEnded) { + child.stdin.write(data); + } + }; + const endChildInput = () => { + inputEnded = true; + if (childReady && child && !child.stdin.destroyed && !child.stdin.writableEnded) { + child.stdin.end(); + } + }; + const forwardSignal = () => { + if (child && !child.killed) { + child.kill(); + } + }; + + process.stdin.on('data', forwardInput); + process.stdin.on('end', endChildInput); + process.once('SIGTERM', forwardSignal); + process.once('SIGINT', forwardSignal); + + try { + const spec = parseSpec(JSON.parse(await readFile(specPath, 'utf8'))); + const command = resolveWindowsCommand(spec.command); + child = spawn(command, spec.args, { + cwd: spec.cwd ?? process.cwd(), + env: { ...process.env }, + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: process.platform === 'win32' + }) as unknown as ChildProcessWithoutNullStreams; + + child.stdout.on('data', (chunk) => process.stdout.write(chunk)); + child.stderr.on('data', (chunk) => process.stderr.write(chunk)); + child.once('spawn', () => { + childReady = true; + for (const chunk of pendingInput) { + forwardInput(chunk); + } + pendingInput.length = 0; + if (inputEnded) { + endChildInput(); + } + }); + + await new Promise((resolve) => { + const finish = (code: number) => { + if (settled) { + return; + } + settled = true; + process.exitCode = code; + resolve(); + }; + child?.once('error', (error) => { + process.stderr.write(`[hapi-mcp-proxy] ${error.message}\n`); + finish(1); + }); + child?.once('close', (code) => finish(typeof code === 'number' ? code : 1)); + }); + } catch (error) { + process.stderr.write(`[hapi-mcp-proxy] ${error instanceof Error ? error.message : String(error)}\n`); + process.exitCode = 1; + } finally { + process.stdin.off('data', forwardInput); + process.stdin.off('end', endChildInput); + process.off('SIGTERM', forwardSignal); + process.off('SIGINT', forwardSignal); + process.stdin.pause(); + } +} diff --git a/cli/src/codex/utils/appServerConfig.test.ts b/cli/src/codex/utils/appServerConfig.test.ts index befddadb..d45e677f 100644 --- a/cli/src/codex/utils/appServerConfig.test.ts +++ b/cli/src/codex/utils/appServerConfig.test.ts @@ -102,6 +102,40 @@ describe('appServerConfig', () => { }); }); + it('preserves user MCP transport fields when building thread config', () => { + const params = buildThreadStartParams({ + cwd: '/workspace/project', + mode: { permissionMode: 'default', collaborationMode: 'default' }, + mcpServers: { + 'package-manager': { + command: 'uvx', + args: ['example-mcp', 'serve'], + env: { EXAMPLE_TOKEN: 'from-process-environment' } + }, + remote: { + url: 'https://example.test/mcp', + bearer_token_env_var: 'REMOTE_MCP_TOKEN' + }, + hapi: { + command: 'node', + args: ['mcp'] + } + } + }); + + expect(params.config).toMatchObject({ + 'mcp_servers.package-manager': { + command: 'uvx', + args: ['example-mcp', 'serve'], + env: { EXAMPLE_TOKEN: 'from-process-environment' } + }, + 'mcp_servers.remote': { + url: 'https://example.test/mcp', + bearer_token_env_var: 'REMOTE_MCP_TOKEN' + } + }); + }); + it('ignores CLI overrides when permission mode is not default', () => { const params = buildThreadStartParams({ cwd: '/workspace/project', diff --git a/cli/src/codex/utils/appServerConfig.ts b/cli/src/codex/utils/appServerConfig.ts index 5278bec8..fcd70510 100644 --- a/cli/src/codex/utils/appServerConfig.ts +++ b/cli/src/codex/utils/appServerConfig.ts @@ -1,6 +1,6 @@ import type { EnhancedMode } from '../loop'; import type { CodexCliOverrides } from './codexCliOverrides'; -import type { McpServersConfig } from './buildHapiMcpBridge'; +import type { CodexMcpServersConfig } from './codexMcpServers'; import { getCodexSystemPrompt } from './systemPrompt'; import type { ApprovalPolicy, @@ -102,15 +102,11 @@ export function supportsReasoningSummary(model: string | undefined): boolean { return !MODELS_WITHOUT_REASONING_SUMMARY.has(modelName); } -function buildMcpServerConfig(mcpServers: McpServersConfig): Record { +function buildMcpServerConfig(mcpServers: CodexMcpServersConfig): Record { const config: Record = {}; for (const [name, server] of Object.entries(mcpServers)) { - config[`mcp_servers.${name}`] = { - command: server.command, - args: server.args, - ...(server.tools ? { tools: server.tools } : {}) - }; + config[`mcp_servers.${name}`] = { ...server }; } return config; @@ -197,7 +193,7 @@ export function buildUserInputFromMessage( export function buildThreadStartParams(args: { cwd: string; mode: EnhancedMode; - mcpServers: McpServersConfig; + mcpServers: CodexMcpServersConfig; cliOverrides?: CodexCliOverrides; baseInstructions?: string; developerInstructions?: string; diff --git a/cli/src/codex/utils/buildHapiMcpBridge.ts b/cli/src/codex/utils/buildHapiMcpBridge.ts index f92621f7..85d1c47e 100644 --- a/cli/src/codex/utils/buildHapiMcpBridge.ts +++ b/cli/src/codex/utils/buildHapiMcpBridge.ts @@ -9,6 +9,7 @@ import { startHappyServer } from '@/claude/utils/startHappyServer'; import { getHappyCliCommand } from '@/utils/spawnHappyCLI'; import type { ApiSessionClient } from '@/api/apiSession'; import { exportHapiSessionEnv } from '@/agent/hapiSessionEnv'; +import type { CodexMcpServerConfig } from './codexMcpServers'; /** * MCP server entry configuration. @@ -19,7 +20,7 @@ export interface McpServerToolConfig { approval_mode?: McpToolApprovalMode; } -export interface McpServerEntry { +export interface McpServerEntry extends CodexMcpServerConfig { command: string; args: string[]; tools?: Record; diff --git a/cli/src/codex/utils/codexHome.test.ts b/cli/src/codex/utils/codexHome.test.ts new file mode 100644 index 00000000..fdc9d932 --- /dev/null +++ b/cli/src/codex/utils/codexHome.test.ts @@ -0,0 +1,43 @@ +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { copyCodexConfigFile, resolveCodexHome } from './codexHome'; + +const temporaryDirectories: string[] = []; + +async function createTemporaryDirectory(): Promise { + const directory = await mkdtemp(join(tmpdir(), 'hapi-codex-home-test-')); + temporaryDirectories.push(directory); + return directory; +} + +afterEach(async () => { + await Promise.all(temporaryDirectories.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))); +}); + +describe('codexHome', () => { + it('uses CODEX_HOME when configured and trims surrounding whitespace', () => { + expect(resolveCodexHome({ CODEX_HOME: ' C:\\codex-home ' })).toBe('C:\\codex-home'); + }); + + it('copies config.toml without copying authentication or other state', async () => { + const sourceHome = await createTemporaryDirectory(); + const targetHome = await createTemporaryDirectory(); + await writeFile(join(sourceHome, 'config.toml'), '[mcp_servers.example]\ncommand = "server"\n'); + await writeFile(join(sourceHome, 'auth.json'), '{"access_token":"not-copied"}\n'); + + await expect(copyCodexConfigFile(sourceHome, targetHome)) + .resolves.toBe(join(targetHome, 'config.toml')); + await expect(readFile(join(targetHome, 'config.toml'), 'utf8')) + .resolves.toBe('[mcp_servers.example]\ncommand = "server"\n'); + await expect(readFile(join(targetHome, 'auth.json'), 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('treats a missing user config as a valid first-run state', async () => { + const sourceHome = await createTemporaryDirectory(); + const targetHome = await createTemporaryDirectory(); + + await expect(copyCodexConfigFile(sourceHome, targetHome)).resolves.toBeNull(); + }); +}); diff --git a/cli/src/codex/utils/codexHome.ts b/cli/src/codex/utils/codexHome.ts new file mode 100644 index 00000000..a4fb8ac4 --- /dev/null +++ b/cli/src/codex/utils/codexHome.ts @@ -0,0 +1,32 @@ +import { copyFile } from 'node:fs/promises'; +import { homedir } from 'node:os'; +import { join, resolve } from 'node:path'; + +export function resolveCodexHome(env: NodeJS.ProcessEnv = process.env): string { + const configuredHome = env.CODEX_HOME?.trim(); + return configuredHome ? configuredHome : join(homedir(), '.codex'); +} + +/** + * Copy only Codex's user config into a runner-owned home. + * + * Runner-spawned Codex sessions use a temporary CODEX_HOME for token auth. + * Copying config.toml preserves user MCP settings without copying auth files + * or unrelated state. A missing source config is a valid first-run state. + */ +export async function copyCodexConfigFile(sourceHome: string, targetHome: string): Promise { + if (resolve(sourceHome) === resolve(targetHome)) { + return null; + } + + const targetConfigPath = join(targetHome, 'config.toml'); + try { + await copyFile(join(sourceHome, 'config.toml'), targetConfigPath); + return targetConfigPath; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return null; + } + throw error; + } +} diff --git a/cli/src/codex/utils/codexMcpProxy.test.ts b/cli/src/codex/utils/codexMcpProxy.test.ts new file mode 100644 index 00000000..f08f2917 --- /dev/null +++ b/cli/src/codex/utils/codexMcpProxy.test.ts @@ -0,0 +1,187 @@ +import { existsSync } from 'node:fs'; +import { spawn } from 'node:child_process'; +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { describe, expect, it } from 'vitest'; + +import { getHappyCliCommand } from '@/utils/spawnHappyCLI'; +import { + prepareCodexMcpServers, + shouldProxyCodexMcpStdio +} from './codexMcpProxy'; + +const windowsIt = process.platform === 'win32' ? it : it.skip; + +describe('codexMcpProxy', () => { + it('limits the compatibility proxy to Windows command shims', () => { + expect(shouldProxyCodexMcpStdio('uvx', 'win32')).toBe(true); + expect(shouldProxyCodexMcpStdio('C:\\Tools\\uvx.exe', 'win32')).toBe(true); + expect(shouldProxyCodexMcpStdio('C:\\Tools\\server.cmd', 'win32')).toBe(true); + expect(shouldProxyCodexMcpStdio('node', 'win32')).toBe(false); + expect(shouldProxyCodexMcpStdio('uvx', 'linux')).toBe(false); + }); + + it('preserves server fields while replacing only the Windows shim launcher', async () => { + const prepared = await prepareCodexMcpServers({ + 'package-manager': { + command: 'uvx', + args: ['--from', 'example-mcp==1.0.0', 'example-mcp', 'serve'], + env_vars: ['EXAMPLE_TOKEN'], + enabled: true, + tool_timeout_sec: 60 + }, + nodeServer: { + command: 'node', + args: ['server.js'], + enabled: true + }, + remoteShim: { + command: 'uvx', + args: ['remote-mcp', 'serve'], + environment_id: 'remote', + enabled: true + }, + remote: { + url: 'https://example.test/mcp', + bearer_token_env_var: 'REMOTE_MCP_TOKEN' + } + }, 'win32'); + + try { + const proxied = prepared.servers['package-manager'] as { + command: string; + args: string[]; + [key: string]: unknown; + }; + expect(proxied).toEqual(expect.objectContaining({ + env_vars: ['EXAMPLE_TOKEN'], + enabled: true, + tool_timeout_sec: 60 + })); + + const specPath = proxied.args.at(-1); + expect(typeof specPath).toBe('string'); + if (!specPath) { + throw new Error('Expected a proxy spec path'); + } + expect(proxied.args).toContain('mcp-proxy'); + expect(proxied.command).toBe(getHappyCliCommand(['mcp-proxy', '--spec', specPath]).command); + expect(existsSync(specPath)).toBe(true); + await expect(readFile(specPath, 'utf8')).resolves.toBe(JSON.stringify({ + command: 'uvx', + args: ['--from', 'example-mcp==1.0.0', 'example-mcp', 'serve'] + })); + + expect(prepared.servers.nodeServer).toEqual({ + command: 'node', + args: ['server.js'], + enabled: true + }); + expect(prepared.servers.remoteShim).toEqual({ + command: 'uvx', + args: ['remote-mcp', 'serve'], + environment_id: 'remote', + enabled: true + }); + expect(prepared.servers.remote).toEqual({ + url: 'https://example.test/mcp', + bearer_token_env_var: 'REMOTE_MCP_TOKEN' + }); + } finally { + await prepared.cleanup(); + } + + const cleanedProxy = prepared.servers['package-manager'] as { args: string[] }; + expect(existsSync(cleanedProxy.args.at(-1) as string)).toBe(false); + await prepared.cleanup(); + }); + + windowsIt('launches a bare command through a temporary Windows shim', async () => { + const directory = await mkdtemp(join(tmpdir(), 'hapi-codex-mcp-shim-test-')); + const serverPath = join(directory, 'server.js'); + const shimPath = join(directory, 'example-mcp.cmd'); + const specPath = join(directory, 'spec.json'); + const originalPath = process.env.PATH ?? ''; + const shimArgs = [ + 'C:\\Users\\Jane Doe\\repo', + 'literal&value', + 'percent%value', + 'caret^value' + ]; + const serverScript = [ + "const receivedArgs = JSON.stringify(process.argv.slice(2));", + "let buffer = '';", + "process.stdin.setEncoding('utf8');", + "process.stdin.on('data', (chunk) => {", + " buffer += chunk;", + " const newline = buffer.indexOf('\\n');", + " if (newline < 0) return;", + " const request = JSON.parse(buffer.slice(0, newline));", + " if (request.method === 'initialize') {", + " process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: request.id, result: { protocolVersion: request.params.protocolVersion, capabilities: {}, serverInfo: { name: 'shim-test', version: receivedArgs } } }) + '\\n');", + " }", + "});" + ].join('\n'); + + try { + await writeFile(serverPath, serverScript, 'utf8'); + await writeFile(shimPath, '@echo off\r\nnode "%~dp0server.js" %*\r\n', 'utf8'); + await writeFile(specPath, JSON.stringify({ command: 'example-mcp', args: shimArgs }), 'utf8'); + + const proxyCommand = getHappyCliCommand(['mcp-proxy', '--spec', specPath]); + const child = spawn(proxyCommand.command, proxyCommand.args, { + env: { ...process.env, PATH: `${directory};${originalPath}` }, + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true + }); + + const response = await new Promise>((resolve, reject) => { + let output = ''; + const timeout = setTimeout(() => { + child.kill(); + reject(new Error('Timed out waiting for the Windows MCP shim response')); + }, 5_000); + const finish = (error: Error | null, value?: Record) => { + clearTimeout(timeout); + if (error) { + reject(error); + } else if (value) { + resolve(value); + } + }; + child.once('error', (error) => finish(error)); + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (chunk) => { + output += chunk; + const newline = output.indexOf('\n'); + if (newline < 0) return; + try { + finish(null, JSON.parse(output.slice(0, newline)) as Record); + } catch (error) { + finish(error instanceof Error ? error : new Error(String(error))); + } + child.kill(); + }); + child.stderr.resume(); + child.stdin.write(JSON.stringify({ + jsonrpc: '2.0', + id: 1, + method: 'initialize', + params: { protocolVersion: '2025-06-18' } + }) + '\n'); + }); + + expect(response).toMatchObject({ + jsonrpc: '2.0', + id: 1, + result: { + serverInfo: { name: 'shim-test', version: JSON.stringify(shimArgs) } + } + }); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); +}); diff --git a/cli/src/codex/utils/codexMcpProxy.ts b/cli/src/codex/utils/codexMcpProxy.ts new file mode 100644 index 00000000..92f96026 --- /dev/null +++ b/cli/src/codex/utils/codexMcpProxy.ts @@ -0,0 +1,127 @@ +import { randomUUID } from 'node:crypto'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { getHappyCliCommand } from '@/utils/spawnHappyCLI'; +import type { CodexMcpServersConfig } from './codexMcpServers'; + +const WINDOWS_COMMAND_SHIMS = new Set([ + 'bunx', + 'npx', + 'npm', + 'pnpm', + 'uv', + 'uvx', + 'yarn' +]); + +type StdioProxySpec = { + command: string; + args: string[]; + cwd?: string; +}; + +export type PreparedCodexMcpServers = { + servers: CodexMcpServersConfig; + proxiedServerNames: string[]; + cleanup: () => Promise; +}; + +function commandBaseName(command: string): string { + const normalized = command.trim().replace(/[\\/]+$/, ''); + const lastSeparator = Math.max(normalized.lastIndexOf('\\'), normalized.lastIndexOf('/')); + return (lastSeparator >= 0 ? normalized.slice(lastSeparator + 1) : normalized).toLowerCase(); +} + +/** + * Codex currently launches Windows MCP commands verbatim. Package-manager + * shims can close before returning the initialize response when launched by + * the Rust stdio launcher, while the same process works through a native + * HAPI child process. Keep the workaround narrow to known command shims. + */ +export function shouldProxyCodexMcpStdio(command: string, platform = process.platform): boolean { + if (platform !== 'win32') { + return false; + } + + const baseName = commandBaseName(command); + const executableName = baseName.endsWith('.exe') ? baseName.slice(0, -4) : baseName; + return WINDOWS_COMMAND_SHIMS.has(baseName) + || WINDOWS_COMMAND_SHIMS.has(executableName) + || baseName.endsWith('.cmd') + || baseName.endsWith('.bat'); +} + +function runsOnRemoteEnvironment(server: Record): boolean { + return server.experimental_environment === 'remote' + || (typeof server.environment_id === 'string' && server.environment_id !== 'local'); +} + +async function writeProxySpec(spec: StdioProxySpec): Promise<{ directory: string; path: string }> { + const directory = await mkdtemp(join(tmpdir(), 'hapi-codex-mcp-')); + const path = join(directory, `${randomUUID()}.json`); + await writeFile(path, JSON.stringify(spec), { encoding: 'utf8', mode: 0o600 }); + return { directory, path }; +} + +/** + * Prepare user MCP entries for a Codex session. + * + * Only Windows package-manager shims are proxied. All other entries, URL + * transports, environment fields, and newer Codex fields remain unchanged. + */ +export async function prepareCodexMcpServers( + servers: CodexMcpServersConfig, + platform = process.platform +): Promise { + const prepared: CodexMcpServersConfig = {}; + const proxiedServerNames: string[] = []; + const temporaryDirectories: string[] = []; + + try { + for (const [name, server] of Object.entries(servers)) { + if ( + typeof server.command !== 'string' + || runsOnRemoteEnvironment(server) + || !shouldProxyCodexMcpStdio(server.command, platform) + ) { + prepared[name] = server; + continue; + } + + const spec = await writeProxySpec({ + command: server.command, + args: Array.isArray(server.args) + ? server.args.filter((arg): arg is string => typeof arg === 'string') + : [], + ...(typeof server.cwd === 'string' && server.cwd.trim().length > 0 + ? { cwd: server.cwd } + : {}) + }); + temporaryDirectories.push(spec.directory); + + const proxyCommand = getHappyCliCommand(['mcp-proxy', '--spec', spec.path]); + prepared[name] = { + ...server, + command: proxyCommand.command, + args: proxyCommand.args + }; + proxiedServerNames.push(name); + } + } catch (error) { + await Promise.all(temporaryDirectories.map((directory) => rm(directory, { force: true, recursive: true }))); + throw error; + } + + let cleaned = false; + const cleanup = async () => { + if (cleaned) { + return; + } + cleaned = true; + await Promise.all(temporaryDirectories.map((directory) => rm(directory, { force: true, recursive: true }))); + }; + + return { servers: prepared, proxiedServerNames, cleanup }; +} diff --git a/cli/src/codex/utils/codexMcpServers.test.ts b/cli/src/codex/utils/codexMcpServers.test.ts new file mode 100644 index 00000000..729b9f4a --- /dev/null +++ b/cli/src/codex/utils/codexMcpServers.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from 'vitest'; +import { + extractCodexMcpServers, + HAPI_MCP_SERVER_NAME, + mergeCodexMcpServers +} from './codexMcpServers'; + +describe('codexMcpServers', () => { + it('extracts stdio and HTTP servers while preserving transport-specific fields', () => { + const result = extractCodexMcpServers({ + mcp_servers: { + 'package-manager': { + command: 'uvx', + args: ['example-mcp', 'serve'], + env: { EXAMPLE_TOKEN: 'from-process-environment' }, + environment_id: 'local', + enabled: true, + tool_timeout_sec: 60 + }, + remote: { + url: 'https://example.test/mcp', + bearer_token_env_var: 'REMOTE_MCP_TOKEN' + } + } + }); + + expect(result).toEqual({ + 'package-manager': { + command: 'uvx', + args: ['example-mcp', 'serve'], + env: { EXAMPLE_TOKEN: 'from-process-environment' }, + environment_id: 'local', + enabled: true, + tool_timeout_sec: 60 + }, + remote: { + url: 'https://example.test/mcp', + bearer_token_env_var: 'REMOTE_MCP_TOKEN' + } + }); + }); + + it('returns no servers when the effective config has no MCP table', () => { + expect(extractCodexMcpServers({ model: 'gpt-5' })).toEqual({}); + expect(extractCodexMcpServers(null)).toEqual({}); + }); + + it('rejects malformed MCP entries before they reach app-server', () => { + expect(() => extractCodexMcpServers({ + mcp_servers: { + broken: { + command: ['not', 'a', 'command'] + } + } + })).toThrow('Invalid Codex MCP server configuration for "broken"'); + }); + + it('omits null defaults returned by config/read', () => { + expect(extractCodexMcpServers({ + mcp_servers: { + external: { + command: 'server', + args: [], + environment_id: 'local', + enabled: true, + tool_timeout_sec: null, + startup_timeout_sec: undefined + } + } + })).toEqual({ + external: { + command: 'server', + args: [], + environment_id: 'local', + enabled: true + } + }); + }); + + it('reserves the HAPI server name and keeps the HAPI bridge on merge', () => { + const merged = mergeCodexMcpServers( + { + [HAPI_MCP_SERVER_NAME]: { command: 'user-hapi', args: [] }, + external: { command: 'external', args: [] } + }, + { + [HAPI_MCP_SERVER_NAME]: { command: 'hapi', args: ['mcp'] } + } + ); + + expect(merged).toEqual({ + external: { command: 'external', args: [] }, + [HAPI_MCP_SERVER_NAME]: { command: 'hapi', args: ['mcp'] } + }); + }); +}); diff --git a/cli/src/codex/utils/codexMcpServers.ts b/cli/src/codex/utils/codexMcpServers.ts new file mode 100644 index 00000000..1ee72181 --- /dev/null +++ b/cli/src/codex/utils/codexMcpServers.ts @@ -0,0 +1,88 @@ +/** + * Helpers for carrying user-configured Codex MCP servers into HAPI sessions. + * + * The app-server reports the effective Codex configuration, so keep external + * entries opaque here. This preserves newer Codex MCP fields without making + * HAPI duplicate Codex's transport schema. + */ + +export interface CodexMcpServerConfig { + [key: string]: unknown; +} + +export type CodexMcpServersConfig = Record; + +export const HAPI_MCP_SERVER_NAME = 'hapi'; + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isMcpServerConfig(value: unknown): value is CodexMcpServerConfig { + if (!isRecord(value)) { + return false; + } + + if (value.command !== undefined && typeof value.command !== 'string') { + return false; + } + if (value.args !== undefined && ( + !Array.isArray(value.args) + || value.args.some((arg) => typeof arg !== 'string') + )) { + return false; + } + if (value.url !== undefined && typeof value.url !== 'string') { + return false; + } + + return typeof value.command === 'string' || typeof value.url === 'string'; +} + +function omitNullishFields(value: CodexMcpServerConfig): CodexMcpServerConfig { + return Object.fromEntries( + Object.entries(value).filter(([, entryValue]) => entryValue !== null && entryValue !== undefined) + ); +} + +/** + * Extract the MCP server table from a Codex config/read response. + * + * `hapi` is intentionally excluded: that name is reserved for the bridge + * HAPI injects into every Codex session. + */ +export function extractCodexMcpServers(config: unknown): CodexMcpServersConfig { + if (!isRecord(config) || !isRecord(config.mcp_servers)) { + return {}; + } + + const servers: CodexMcpServersConfig = {}; + for (const [name, value] of Object.entries(config.mcp_servers)) { + if (name === HAPI_MCP_SERVER_NAME) { + continue; + } + if (!isMcpServerConfig(value)) { + throw new Error(`Invalid Codex MCP server configuration for "${name}"`); + } + servers[name] = omitNullishFields(value); + } + return servers; +} + +/** + * Merge user entries with HAPI's bridge configuration. + * + * HAPI wins on the reserved name, while all other user entries remain opaque + * so Codex can handle the transport-specific fields it understands. + */ +export function mergeCodexMcpServers( + userMcpServers: CodexMcpServersConfig, + hapiMcpServers: CodexMcpServersConfig +): CodexMcpServersConfig { + const merged = { ...userMcpServers }; + delete merged[HAPI_MCP_SERVER_NAME]; + return { + ...merged, + ...hapiMcpServers + }; +} diff --git a/cli/src/commands/mcpProxy.ts b/cli/src/commands/mcpProxy.ts new file mode 100644 index 00000000..85aadc00 --- /dev/null +++ b/cli/src/commands/mcpProxy.ts @@ -0,0 +1,10 @@ +import { runHappyMcpStdioProxy } from '@/codex/happyMcpStdioProxy' +import type { CommandDefinition } from './types' + +export const mcpProxyCommand: CommandDefinition = { + name: 'mcp-proxy', + requiresRuntimeAssets: false, + run: async ({ commandArgs }) => { + await runHappyMcpStdioProxy(commandArgs) + } +} diff --git a/cli/src/commands/registry.ts b/cli/src/commands/registry.ts index 24d547dc..30f3c32b 100644 --- a/cli/src/commands/registry.ts +++ b/cli/src/commands/registry.ts @@ -16,6 +16,7 @@ import { opencodeCommand } from './opencode' import { piCommand } from './pi' import { hookForwarderCommand } from './hookForwarder' import { mcpCommand } from './mcp' +import { mcpProxyCommand } from './mcpProxy' import { notifyCommand } from './notify' import { hubCommand } from './hub' import { pingPeerCommand } from './pingPeer' @@ -52,6 +53,7 @@ const COMMANDS: CommandDefinition[] = [ opencodeCommand, piCommand, mcpCommand, + mcpProxyCommand, hubCommand, { ...hubCommand, name: 'server' }, hookForwarderCommand, diff --git a/cli/src/runner/README.md b/cli/src/runner/README.md index 186fc32b..0d758434 100644 --- a/cli/src/runner/README.md +++ b/cli/src/runner/README.md @@ -83,7 +83,7 @@ The runner supports spawning sessions with different AI agents: | Agent | Command | Token Environment | |-------|---------|-------------------| | `claude` (default) | `hapi claude` | `CLAUDE_CODE_OAUTH_TOKEN` | -| `codex` | `hapi codex` | `CODEX_HOME` (temp directory with `auth.json`) | +| `codex` | `hapi codex` | `CODEX_HOME` (temp directory with `auth.json` and a copy of user `config.toml`) | | `grok` | `hapi grok` | Grok CLI login or `XAI_API_KEY` | | `opencode` | `hapi opencode` | OpenCode config (no token injection) | @@ -91,7 +91,7 @@ The runner supports spawning sessions with different AI agents: When spawning a session with a token: - **Claude**: Sets `CLAUDE_CODE_OAUTH_TOKEN` environment variable -- **Codex**: Creates temp directory at `os.tmpdir()/hapi-codex-*`, writes token to `auth.json`, sets `CODEX_HOME` +- **Codex**: Creates temp directory at `os.tmpdir()/hapi-codex-*`, copies the user's `config.toml` when present, writes the token to `auth.json`, and sets `CODEX_HOME`. Only the config file is copied so user MCP settings survive without copying unrelated Codex state; the copied config is cleaned up when the child exits or fails to start. Windows package-manager MCP commands are proxied by the Codex launcher path; listed `env_vars` remain the source of external MCP credentials. - **Grok Build**: No token injection; relies on Grok CLI login or `XAI_API_KEY` in the runner environment - **OpenCode**: No token injection; relies on OpenCode's own configuration diff --git a/cli/src/runner/run.ts b/cli/src/runner/run.ts index 245dc398..41b04c82 100644 --- a/cli/src/runner/run.ts +++ b/cli/src/runner/run.ts @@ -31,6 +31,7 @@ import { hashRunnerCliApiToken, hashRunnerExtraHeaders } from './runnerIdentity' import { scheduleCursorModelsPrewarm } from '@/modules/common/cursorModelsPrewarm'; import { isLinkedGitWorktree } from '@/utils/isLinkedGitWorktree'; import { agentUnavailableMessage, getAgentAvailability } from '@/agent/agentAvailability'; +import { copyCodexConfigFile, resolveCodexHome } from '@/codex/utils/codexHome'; /** * Deduplicates a preallocated HAPI-row spawn only while its child is alive. @@ -529,6 +530,21 @@ export async function startRunner(options: { workspaceRoots?: string[] } = {}): let spawnDirectory = directory; let worktreeInfo: WorktreeInfo | null = null; let happyProcess: ReturnType | null = null; + let copiedCodexConfigPath: string | null = null; + + const cleanupCopiedCodexConfig = async (reason: string): Promise => { + const configPath = copiedCodexConfigPath; + copiedCodexConfigPath = null; + if (!configPath) { + return; + } + try { + await fs.rm(configPath, { force: true }); + logger.debug(`[RUNNER RUN] Removed temporary Codex config after ${reason}`); + } catch (error) { + logger.debug(`[RUNNER RUN] Failed to remove temporary Codex config after ${reason}`, error); + } + }; if (sessionType === 'simple') { const validation = await validateWorkspaceDirectory(directory, { @@ -647,6 +663,9 @@ export async function startRunner(options: { workspaceRoots?: string[] } = {}): // Create a temporary directory for Codex const codexHomeDir = await fs.mkdtemp(join(os.tmpdir(), 'hapi-codex-')); + // Preserve user MCP/config settings while keeping token auth isolated. + copiedCodexConfigPath = await copyCodexConfigFile(resolveCodexHome(), codexHomeDir); + // Write the token to the temporary directory await fs.writeFile(join(codexHomeDir, 'auth.json'), options.token); @@ -702,6 +721,9 @@ export async function startRunner(options: { workspaceRoots?: string[] } = {}): ...extraEnv } }); + happyProcess.once('exit', () => { + void cleanupCopiedCodexConfig('child-exit'); + }); happyProcess.stderr?.on('data', (data) => { stderrTail = appendTail(stderrTail, data); @@ -728,6 +750,7 @@ export async function startRunner(options: { workspaceRoots?: string[] } = {}): message: errorMessage } }); + await cleanupCopiedCodexConfig('no-pid'); await maybeCleanupWorktree('no-pid'); return { type: 'error', @@ -853,7 +876,11 @@ export async function startRunner(options: { workspaceRoots?: string[] } = {}): // (the actual claude/codex agent) are also reaped, and that // SIGTERM → SIGKILL escalation kicks in if needed. if (happyProcess) { - void killProcessByChildProcess(happyProcess); + void killProcessByChildProcess(happyProcess).finally(() => { + void cleanupCopiedCodexConfig('webhook-timeout'); + }); + } else { + void cleanupCopiedCodexConfig('webhook-timeout'); } // If this was a worktree session, the worktree can only be @@ -910,6 +937,7 @@ export async function startRunner(options: { workspaceRoots?: string[] } = {}): } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); logger.debug('[RUNNER RUN] Failed to spawn session:', error); + await cleanupCopiedCodexConfig('exception'); await maybeCleanupWorktree('exception'); reportSpawnOutcomeToHub?.({ type: 'error',