diff --git a/packages/happy-cli/src/codex/__tests__/codexMcpClient.test.ts b/packages/happy-cli/src/codex/__tests__/codexMcpClient.test.ts index 5070dacb14..b08733096f 100644 --- a/packages/happy-cli/src/codex/__tests__/codexMcpClient.test.ts +++ b/packages/happy-cli/src/codex/__tests__/codexMcpClient.test.ts @@ -9,7 +9,9 @@ const { mockSandboxCleanup, mockClientConnect, mockClientClose, + mockClientSetRequestHandler, mockStdioCtor, + mockClients, } = vi.hoisted(() => ({ mockExecSync: vi.fn(), mockInitializeSandbox: vi.fn(), @@ -17,7 +19,9 @@ const { mockSandboxCleanup: vi.fn(), mockClientConnect: vi.fn(), mockClientClose: vi.fn(), + mockClientSetRequestHandler: vi.fn(), mockStdioCtor: vi.fn(), + mockClients: [] as any[], })); vi.mock('child_process', () => ({ @@ -40,11 +44,14 @@ vi.mock('@/ui/logger', () => ({ vi.mock('@modelcontextprotocol/sdk/client/index.js', () => ({ Client: class MockClient { setNotificationHandler = vi.fn(); - setRequestHandler = vi.fn(); + setRequestHandler = mockClientSetRequestHandler; connect = mockClientConnect; close = mockClientClose; callTool = vi.fn(); - constructor() {} + _requestHandlers = new Map(); + constructor() { + mockClients.push(this); + } }, })); @@ -77,6 +84,7 @@ describe('CodexMcpClient sandbox integration', () => { beforeEach(() => { vi.clearAllMocks(); + mockClients.length = 0; process.env.RUST_LOG = originalRustLog; mockExecSync.mockReturnValue('codex-cli 0.43.0'); mockClientConnect.mockResolvedValue(undefined); @@ -152,4 +160,53 @@ describe('CodexMcpClient sandbox integration', () => { }), ); }); + + it('registers a raw elicitation handler for Codex approvals', async () => { + const client = new CodexMcpClient(sandboxConfig); + + await client.connect(); + + const rawHandler = mockClients[0]?._requestHandlers.get('elicitation/create'); + expect(typeof rawHandler).toBe('function'); + expect(mockClientSetRequestHandler).not.toHaveBeenCalled(); + }); + + it('normalizes approved_for_session to approved for Codex approvals', async () => { + const client = new CodexMcpClient(sandboxConfig); + client.setPermissionHandler({ + handleToolCall: vi.fn().mockResolvedValue({ decision: 'approved_for_session' }), + } as any); + + await client.connect(); + + const rawHandler = mockClients[0]?._requestHandlers.get('elicitation/create'); + const response = await rawHandler({ + params: { + codex_call_id: 'call_123', + codex_command: ['mkdir', '-p', '../test'], + codex_cwd: '/tmp/project', + }, + }); + + expect(response).toEqual({ decision: 'approved' }); + }); + + it('aborts approvals when Codex call id is missing', async () => { + const client = new CodexMcpClient(sandboxConfig); + client.setPermissionHandler({ + handleToolCall: vi.fn(), + } as any); + + await client.connect(); + + const rawHandler = mockClients[0]?._requestHandlers.get('elicitation/create'); + const response = await rawHandler({ + params: { + codex_command: ['pwd'], + codex_cwd: '/tmp/project', + }, + }); + + expect(response).toEqual({ decision: 'abort' }); + }); }); diff --git a/packages/happy-cli/src/codex/codexMcpClient.ts b/packages/happy-cli/src/codex/codexMcpClient.ts index 25f2e6854d..07c90d4c83 100644 --- a/packages/happy-cli/src/codex/codexMcpClient.ts +++ b/packages/happy-cli/src/codex/codexMcpClient.ts @@ -7,7 +7,6 @@ import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js' import { logger } from '@/ui/logger'; import type { CodexSessionConfig, CodexToolResponse } from './types'; import { z } from 'zod'; -import { ElicitRequestSchema } from '@modelcontextprotocol/sdk/types.js'; import { CodexPermissionHandler } from './utils/permissionHandler'; import { execSync } from 'child_process'; import type { SandboxConfig } from '@/persistence'; @@ -15,6 +14,25 @@ import { initializeSandbox, wrapForMcpTransport } from '@/sandbox/manager'; const DEFAULT_TIMEOUT = 14 * 24 * 60 * 60 * 1000; // 14 days, which is the half of the maximum possible timeout (~28 days for int32 value in NodeJS) +type CodexApprovalDecision = 'approved' | 'abort'; + +type CodexElicitationParams = { + message?: string; + codex_elicitation?: string; + codex_mcp_tool_call_id?: string; + codex_event_id?: string; + codex_call_id?: string; + codex_command?: string[] | string; + codex_cwd?: string; +}; + +type CodexRawRequestClient = { + _requestHandlers?: Map< + string, + (request: { params: CodexElicitationParams }) => Promise<{ decision: CodexApprovalDecision }> + >; +}; + /** * Get the correct MCP subcommand based on installed codex version * Versions >= 0.43.0-alpha.5 use 'mcp-server', older versions use 'mcp' @@ -186,56 +204,53 @@ export class CodexMcpClient { } private registerPermissionHandlers(): void { - // Register handler for exec command approval requests - this.client.setRequestHandler( - ElicitRequestSchema, - async (request) => { - console.log('[CodexMCP] Received elicitation request:', request.params); - - // Load params - const params = request.params as unknown as { - message: string, - codex_elicitation: string, - codex_mcp_tool_call_id: string, - codex_event_id: string, - codex_call_id: string, - codex_command: string[], - codex_cwd: string - } - const toolName = 'CodexBash'; - - // If no permission handler set, deny by default - if (!this.permissionHandler) { - logger.debug('[CodexMCP] No permission handler set, denying by default'); - return { - decision: 'denied' as const, - }; - } + const rawClient = this.client as unknown as CodexRawRequestClient; + const requestHandlers = rawClient._requestHandlers; - try { - // Request permission through the handler - const result = await this.permissionHandler.handleToolCall( - params.codex_call_id, - toolName, - { - command: params.codex_command, - cwd: params.codex_cwd - } - ); + if (!(requestHandlers instanceof Map)) { + throw new Error('Codex MCP client does not expose raw request handlers'); + } - logger.debug('[CodexMCP] Permission result:', result); - return { - decision: result.decision - } - } catch (error) { - logger.debug('[CodexMCP] Error handling permission request:', error); - return { - decision: 'denied' as const, - reason: error instanceof Error ? error.message : 'Permission request failed' - }; - } + // Codex expects a non-standard top-level `{ decision }` approval payload. + requestHandlers.set('elicitation/create', async (request: { params: CodexElicitationParams }) => { + logger.debug('[CodexMCP] Received elicitation request:', request.params); + + const params = request.params ?? {}; + const toolName = 'CodexBash'; + const toolCallId = params.codex_call_id; + + if (!toolCallId) { + logger.debug('[CodexMCP] Missing codex_call_id, aborting request'); + return { decision: 'abort' }; } - ); + + if (!this.permissionHandler) { + logger.debug('[CodexMCP] No permission handler set, aborting by default'); + return { decision: 'abort' }; + } + + try { + const result = await this.permissionHandler.handleToolCall( + toolCallId, + toolName, + { + command: params.codex_command, + cwd: params.codex_cwd, + }, + ); + + const decision: CodexApprovalDecision = + result.decision === 'approved' || result.decision === 'approved_for_session' + ? 'approved' + : 'abort'; + + logger.debug('[CodexMCP] Elicitation response:', { decision }); + return { decision }; + } catch (error) { + logger.debug('[CodexMCP] Error handling permission request:', error); + return { decision: 'abort' }; + } + }); logger.debug('[CodexMCP] Permission handlers registered'); }