From 34f931ef59ed7bbcfa565d1f3b616242250e125c Mon Sep 17 00:00:00 2001 From: weishu Date: Mon, 16 Mar 2026 21:50:09 +0800 Subject: [PATCH] remove codex mcp backend --- cli/src/codex/codexMcpClient.ts | 451 ------------------ cli/src/codex/codexRemoteLauncher.test.ts | 2 - cli/src/codex/codexRemoteLauncher.ts | 376 +++++---------- cli/src/codex/runCodex.ts | 9 +- cli/src/codex/types.ts | 25 - cli/src/codex/utils/codexStartConfig.test.ts | 60 --- cli/src/codex/utils/codexStartConfig.ts | 50 -- .../codex/utils/terminalEventGuard.test.ts | 18 - cli/src/codex/utils/terminalEventGuard.ts | 5 - hub/src/web/routes/sessions.test.ts | 41 +- hub/src/web/routes/sessions.ts | 3 - shared/src/schemas.ts | 1 - web/src/components/SessionChat.tsx | 3 +- web/src/hooks/mutations/useSessionActions.ts | 2 +- 14 files changed, 154 insertions(+), 892 deletions(-) delete mode 100644 cli/src/codex/codexMcpClient.ts delete mode 100644 cli/src/codex/types.ts delete mode 100644 cli/src/codex/utils/codexStartConfig.test.ts delete mode 100644 cli/src/codex/utils/codexStartConfig.ts diff --git a/cli/src/codex/codexMcpClient.ts b/cli/src/codex/codexMcpClient.ts deleted file mode 100644 index ffad694c..00000000 --- a/cli/src/codex/codexMcpClient.ts +++ /dev/null @@ -1,451 +0,0 @@ -/** - * Codex MCP Client - Simple wrapper for Codex tools - */ - -import { Client } from '@modelcontextprotocol/sdk/client/index.js'; -import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; -import { isObject } from '@hapi/protocol'; -import { logger } from '@/ui/logger'; -import { isProcessAlive, killProcess } from '@/utils/process'; -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 { randomUUID } from 'node:crypto'; - -type ElicitResponseValue = string | number | boolean | string[]; -type ElicitRequestedSchema = { - type?: string; - properties?: Record; - required?: string[]; -}; - -function extractRequestedSchema(params: Record): ElicitRequestedSchema | null { - const raw = params.requestedSchema; - if (!isObject(raw)) return null; - const properties = isObject(raw.properties) ? (raw.properties as Record) : undefined; - const required = Array.isArray(raw.required) ? raw.required.filter((item) => typeof item === 'string') : undefined; - const type = typeof raw.type === 'string' ? raw.type : undefined; - return { type, properties, required }; -} - -function extractToolCallId(params: Record): string | null { - const candidateKeys = [ - 'codex_call_id', - 'codex_mcp_tool_call_id', - 'codex_event_id', - 'call_id', - 'tool_call_id', - 'toolCallId', - 'mcp_tool_call_id', - 'mcpToolCallId', - 'id' - ]; - - for (const key of candidateKeys) { - const value = params[key]; - if (typeof value === 'string' && value.length > 0) { - return value; - } - } - - return null; -} - -function extractCommand(params: Record): string[] | null { - const command = params.codex_command ?? params.command ?? params.cmd; - if (Array.isArray(command) && command.every((item) => typeof item === 'string')) { - return command as string[]; - } - if (typeof command === 'string' && command.length > 0) { - return [command]; - } - return null; -} - -function extractCwd(params: Record): string | null { - const cwd = params.codex_cwd ?? params.cwd; - return typeof cwd === 'string' && cwd.length > 0 ? cwd : null; -} - -function buildElicitationResult( - decision: 'approved' | 'approved_for_session' | 'denied' | 'abort', - requestedSchema: ElicitRequestedSchema | null, - reason?: string -): { - action: 'accept' | 'decline' | 'cancel'; - content?: Record; - decision?: string; - reason?: string; -} { - const action: 'accept' | 'decline' | 'cancel' = - decision === 'approved' || decision === 'approved_for_session' - ? 'accept' - : decision === 'abort' - ? 'cancel' - : 'decline'; - - if (!requestedSchema?.properties || Object.keys(requestedSchema.properties).length === 0) { - return reason ? { action, decision, reason } : { action, decision }; - } - - if (action !== 'accept') { - return reason ? { action, decision, reason } : { action, decision }; - } - - const properties = requestedSchema?.properties ?? null; - const content: Record = {}; - - if (properties && Object.keys(properties).length > 0) { - const approved = decision === 'approved' || decision === 'approved_for_session'; - - if (Object.prototype.hasOwnProperty.call(properties, 'decision')) { - content.decision = decision; - } - if (Object.prototype.hasOwnProperty.call(properties, 'approved')) { - content.approved = approved; - } - if (Object.prototype.hasOwnProperty.call(properties, 'allow')) { - content.allow = approved; - } - if (reason && Object.prototype.hasOwnProperty.call(properties, 'reason')) { - content.reason = reason; - } - - if (Object.keys(content).length === 0) { - const [fallbackKey] = Object.keys(properties); - if (fallbackKey) { - content[fallbackKey] = decision; - } - } - } else { - content.decision = decision; - if (reason) { - content.reason = reason; - } - } - - return reason ? { action, content, decision, reason } : { action, content, decision }; -} - -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) - -/** - * Get the correct MCP subcommand based on installed codex version - * Versions >= 0.43.0-alpha.5 use 'mcp-server', older versions use 'mcp' - */ -function getCodexMcpCommand(): string { - try { - const version = execSync('codex --version', { encoding: 'utf8' }).trim(); - const match = version.match(/codex-cli\s+(\d+\.\d+\.\d+(?:-alpha\.\d+)?)/); - if (!match) return 'mcp-server'; // Default to newer command if we can't parse - - const versionStr = match[1]; - const [major, minor, patch] = versionStr.split(/[-.]/).map(Number); - - // Version >= 0.43.0-alpha.5 has mcp-server - if (major > 0 || minor > 43) return 'mcp-server'; - if (minor === 43 && patch === 0) { - // Check for alpha version - if (versionStr.includes('-alpha.')) { - const alphaNum = parseInt(versionStr.split('-alpha.')[1]); - return alphaNum >= 5 ? 'mcp-server' : 'mcp'; - } - return 'mcp-server'; // 0.43.0 stable has mcp-server - } - return 'mcp'; // Older versions use mcp - } catch (error) { - logger.debug('[CodexMCP] Error detecting codex version, defaulting to mcp-server:', error); - return 'mcp-server'; // Default to newer command - } -} - -export class CodexMcpClient { - private client: Client; - private transport: StdioClientTransport | null = null; - private connected: boolean = false; - private sessionId: string | null = null; - private conversationId: string | null = null; - private handler: ((event: any) => void) | null = null; - private permissionHandler: CodexPermissionHandler | null = null; - - constructor() { - this.client = new Client( - { name: 'hapi-codex-client', version: '1.0.0' }, - { capabilities: { elicitation: {} } } - ); - - // Avoid TS instantiation depth issues by widening the schema type. - const codexNotificationSchema: z.ZodTypeAny = z.object({ - method: z.literal('codex/event'), - params: z.object({ - msg: z.any() - }) - }); - - const setNotificationHandler = - this.client.setNotificationHandler.bind(this.client) as ( - schema: unknown, - handler: (notification: { params: { msg: any } }) => void - ) => void; - - setNotificationHandler(codexNotificationSchema, (data) => { - const msg = data.params.msg; - this.updateIdentifiersFromEvent(msg); - this.handler?.(msg); - }); - } - - setHandler(handler: ((event: any) => void) | null): void { - this.handler = handler; - } - - /** - * Set the permission handler for tool approval - */ - setPermissionHandler(handler: CodexPermissionHandler): void { - this.permissionHandler = handler; - } - - async connect(): Promise { - if (this.connected) return; - - const mcpCommand = getCodexMcpCommand(); - logger.debug(`[CodexMCP] Connecting to Codex MCP server using command: codex ${mcpCommand}`); - - this.transport = new StdioClientTransport({ - command: 'codex', - args: [mcpCommand], - env: Object.keys(process.env).reduce((acc, key) => { - const value = process.env[key]; - if (typeof value === 'string') acc[key] = value; - return acc; - }, {} as Record) - }); - - // Register request handlers for Codex permission methods - this.registerPermissionHandlers(); - - await this.client.connect(this.transport); - this.connected = true; - - logger.debug('[CodexMCP] Connected to Codex'); - } - - private registerPermissionHandlers(): void { - // Register handler for exec command approval requests - this.client.setRequestHandler( - ElicitRequestSchema, - async (request) => { - const params = request.params as Record; - const requestedSchema = extractRequestedSchema(params); - - // Load params - const toolCallId = extractToolCallId(params) ?? randomUUID(); - const command = extractCommand(params); - const cwd = extractCwd(params); - const toolName = 'CodexPermission'; - - // If no permission handler set, deny by default - if (!this.permissionHandler) { - logger.debug('[CodexMCP] No permission handler set, denying by default'); - return buildElicitationResult('denied', requestedSchema, 'Permission handler not configured'); - } - - try { - // Request permission through the handler - const result = await this.permissionHandler.handleToolCall( - toolCallId, - toolName, - { - message: typeof params.message === 'string' ? params.message : undefined, - command: command ?? undefined, - cwd: cwd ?? undefined - } - ); - - logger.debug('[CodexMCP] Permission result:', result); - return buildElicitationResult(result.decision, requestedSchema, result.reason); - } catch (error) { - logger.debug('[CodexMCP] Error handling permission request:', error); - const reason = error instanceof Error ? error.message : 'Permission request failed'; - return buildElicitationResult('denied', requestedSchema, reason); - } - } - ); - - logger.debug('[CodexMCP] Permission handlers registered'); - } - - async startSession(config: CodexSessionConfig, options?: { signal?: AbortSignal }): Promise { - if (!this.connected) await this.connect(); - - logger.debug('[CodexMCP] Starting Codex session:', config); - - const response = await this.client.callTool({ - name: 'codex', - arguments: config as any - }, undefined, { - signal: options?.signal, - timeout: DEFAULT_TIMEOUT, - // maxTotalTimeout: 10000000000 - }); - - logger.debug('[CodexMCP] startSession response:', response); - - // Extract session / conversation identifiers from response if present - this.extractIdentifiers(response); - - return response as CodexToolResponse; - } - - async continueSession(prompt: string, options?: { signal?: AbortSignal }): Promise { - if (!this.connected) await this.connect(); - - if (!this.sessionId) { - throw new Error('No active session. Call startSession first.'); - } - - if (!this.conversationId) { - // Some Codex deployments reuse the session ID as the conversation identifier - this.conversationId = this.sessionId; - logger.debug('[CodexMCP] conversationId missing, defaulting to sessionId:', this.conversationId); - } - - const args = { sessionId: this.sessionId, conversationId: this.conversationId, prompt }; - logger.debug('[CodexMCP] Continuing Codex session:', args); - - const response = await this.client.callTool({ - name: 'codex-reply', - arguments: args - }, undefined, { - signal: options?.signal, - timeout: DEFAULT_TIMEOUT - }); - - logger.debug('[CodexMCP] continueSession response:', response); - this.extractIdentifiers(response); - - return response as CodexToolResponse; - } - - - private updateIdentifiersFromEvent(event: any): void { - if (!event || typeof event !== 'object') { - return; - } - - const candidates: any[] = [event]; - if (event.data && typeof event.data === 'object') { - candidates.push(event.data); - } - - for (const candidate of candidates) { - const sessionId = candidate.session_id ?? candidate.sessionId; - if (sessionId) { - this.sessionId = sessionId; - logger.debug('[CodexMCP] Session ID extracted from event:', this.sessionId); - } - - const conversationId = candidate.conversation_id ?? candidate.conversationId; - if (conversationId) { - this.conversationId = conversationId; - logger.debug('[CodexMCP] Conversation ID extracted from event:', this.conversationId); - } - } - } - private extractIdentifiers(response: any): void { - const meta = response?.meta || {}; - if (meta.sessionId) { - this.sessionId = meta.sessionId; - logger.debug('[CodexMCP] Session ID extracted:', this.sessionId); - } else if (response?.sessionId) { - this.sessionId = response.sessionId; - logger.debug('[CodexMCP] Session ID extracted:', this.sessionId); - } - - if (meta.conversationId) { - this.conversationId = meta.conversationId; - logger.debug('[CodexMCP] Conversation ID extracted:', this.conversationId); - } else if (response?.conversationId) { - this.conversationId = response.conversationId; - logger.debug('[CodexMCP] Conversation ID extracted:', this.conversationId); - } - - const content = response?.content; - if (Array.isArray(content)) { - for (const item of content) { - if (!this.sessionId && item?.sessionId) { - this.sessionId = item.sessionId; - logger.debug('[CodexMCP] Session ID extracted from content:', this.sessionId); - } - if (!this.conversationId && item && typeof item === 'object' && 'conversationId' in item && item.conversationId) { - this.conversationId = item.conversationId; - logger.debug('[CodexMCP] Conversation ID extracted from content:', this.conversationId); - } - } - } - } - - getSessionId(): string | null { - return this.sessionId; - } - - hasActiveSession(): boolean { - return this.sessionId !== null; - } - - clearSession(): void { - // Store the previous session ID before clearing for potential resume - const previousSessionId = this.sessionId; - this.sessionId = null; - this.conversationId = null; - logger.debug('[CodexMCP] Session cleared, previous sessionId:', previousSessionId); - } - - /** - * Store the current session ID without clearing it, useful for abort handling - */ - storeSessionForResume(): string | null { - logger.debug('[CodexMCP] Storing session for potential resume:', this.sessionId); - return this.sessionId; - } - - async disconnect(): Promise { - if (!this.connected) return; - - // Capture pid in case we need to force-kill - const pid = this.transport?.pid ?? null; - logger.debug(`[CodexMCP] Disconnecting; child pid=${pid ?? 'none'}`); - - try { - // Ask client to close the transport - logger.debug('[CodexMCP] client.close begin'); - await this.client.close(); - logger.debug('[CodexMCP] client.close done'); - } catch (e) { - logger.debug('[CodexMCP] Error closing client, attempting transport close directly', e); - try { - logger.debug('[CodexMCP] transport.close begin'); - await this.transport?.close?.(); - logger.debug('[CodexMCP] transport.close done'); - } catch {} - } - - // As a last resort, if child still exists, send SIGKILL - if (pid) { - if (isProcessAlive(pid)) { - logger.debug('[CodexMCP] Child still alive, sending SIGKILL'); - await killProcess(pid, true); - } - } - - this.transport = null; - this.connected = false; - this.sessionId = null; - this.conversationId = null; - - logger.debug('[CodexMCP] Disconnected'); - } -} diff --git a/cli/src/codex/codexRemoteLauncher.test.ts b/cli/src/codex/codexRemoteLauncher.test.ts index 6d9d6fe6..799b5c75 100644 --- a/cli/src/codex/codexRemoteLauncher.test.ts +++ b/cli/src/codex/codexRemoteLauncher.test.ts @@ -168,11 +168,9 @@ describe('codexRemoteLauncher', () => { harness.notifications = []; harness.registerRequestCalls = []; harness.initializeCalls = []; - delete process.env.CODEX_USE_MCP_SERVER; }); it('finishes a turn and emits ready when task lifecycle events omit turn_id', async () => { - delete process.env.CODEX_USE_MCP_SERVER; const { session, sessionEvents, diff --git a/cli/src/codex/codexRemoteLauncher.ts b/cli/src/codex/codexRemoteLauncher.ts index 705809f1..eee4eb03 100644 --- a/cli/src/codex/codexRemoteLauncher.ts +++ b/cli/src/codex/codexRemoteLauncher.ts @@ -1,20 +1,17 @@ import React from 'react'; import { randomUUID } from 'node:crypto'; -import { CodexMcpClient } from './codexMcpClient'; import { CodexAppServerClient } from './codexAppServerClient'; import { CodexPermissionHandler } from './utils/permissionHandler'; import { ReasoningProcessor } from './utils/reasoningProcessor'; import { DiffProcessor } from './utils/diffProcessor'; import { logger } from '@/ui/logger'; import { CodexDisplay } from '@/ui/ink/CodexDisplay'; -import type { CodexSessionConfig } from './types'; import { buildHapiMcpBridge } from './utils/buildHapiMcpBridge'; import { emitReadyIfIdle } from './utils/emitReadyIfIdle'; import type { CodexSession } from './session'; import type { EnhancedMode } from './loop'; import { hasCodexCliOverrides } from './utils/codexCliOverrides'; -import { buildCodexStartConfig } from './utils/codexStartConfig'; import { AppServerEventConverter } from './utils/appServerEventConverter'; import { registerAppServerPermissionHandlers } from './utils/appServerPermissionAdapter'; import { buildThreadStartParams, buildTurnStartParams } from './utils/appServerConfig'; @@ -26,17 +23,11 @@ import { } from '@/modules/common/remote/RemoteLauncherBase'; type HappyServer = Awaited>['server']; - -function shouldUseAppServer(): boolean { - const useMcpServer = process.env.CODEX_USE_MCP_SERVER === '1'; - return !useMcpServer; -} +type QueuedMessage = { message: string; mode: EnhancedMode; isolate: boolean; hash: string }; class CodexRemoteLauncher extends RemoteLauncherBase { private readonly session: CodexSession; - private readonly useAppServer: boolean; - private readonly mcpClient: CodexMcpClient | null; - private readonly appServerClient: CodexAppServerClient | null; + private readonly appServerClient: CodexAppServerClient; private permissionHandler: CodexPermissionHandler | null = null; private reasoningProcessor: ReasoningProcessor | null = null; private diffProcessor: DiffProcessor | null = null; @@ -48,9 +39,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { constructor(session: CodexSession) { super(process.env.DEBUG ? session.logPath : undefined); this.session = session; - this.useAppServer = shouldUseAppServer(); - this.mcpClient = this.useAppServer ? null : new CodexMcpClient(); - this.appServerClient = this.useAppServer ? new CodexAppServerClient() : null; + this.appServerClient = new CodexAppServerClient(); } protected createDisplay(context: RemoteLauncherDisplayContext): React.ReactElement { @@ -60,20 +49,17 @@ class CodexRemoteLauncher extends RemoteLauncherBase { private async handleAbort(): Promise { logger.debug('[Codex] Abort requested - stopping current task'); try { - if (this.useAppServer && this.appServerClient) { - if (this.currentThreadId && this.currentTurnId) { - try { - await this.appServerClient.interruptTurn({ - threadId: this.currentThreadId, - turnId: this.currentTurnId - }); - } catch (error) { - logger.debug('[Codex] Error interrupting app-server turn:', error); - } + if (this.currentThreadId && this.currentTurnId) { + try { + await this.appServerClient.interruptTurn({ + threadId: this.currentThreadId, + turnId: this.currentTurnId + }); + } catch (error) { + logger.debug('[Codex] Error interrupting app-server turn:', error); } - - this.currentTurnId = null; } + this.currentTurnId = null; this.abortController.abort(); this.session.queue.reset(); @@ -128,10 +114,8 @@ class CodexRemoteLauncher extends RemoteLauncherBase { protected async runMainLoop(): Promise { const session = this.session; const messageBuffer = this.messageBuffer; - const useAppServer = this.useAppServer; - const mcpClient = this.mcpClient; const appServerClient = this.appServerClient; - const appServerEventConverter = useAppServer ? new AppServerEventConverter() : null; + const appServerEventConverter = new AppServerEventConverter(); const normalizeCommand = (value: unknown): string | undefined => { if (typeof value === 'string') { @@ -265,14 +249,13 @@ class CodexRemoteLauncher extends RemoteLauncherBase { if (turnId) { this.currentTurnId = turnId; allowAnonymousTerminalEvent = false; - } else if (useAppServer && !this.currentTurnId) { + } else if (!this.currentTurnId) { allowAnonymousTerminalEvent = true; } } if (isTerminalEvent) { if (shouldIgnoreTerminalEvent({ - useAppServer, eventTurnId, currentTurnId: this.currentTurnId, turnInFlight, @@ -289,16 +272,6 @@ class CodexRemoteLauncher extends RemoteLauncherBase { allowAnonymousTerminalEvent = false; } - if (!useAppServer) { - logger.debug(`[Codex] MCP message: ${JSON.stringify(msg)}`); - - if (msgType === 'event_msg' || msgType === 'response_item' || msgType === 'session_meta') { - const payload = asRecord(msg.payload); - const payloadType = asString(payload?.type); - logger.debug(`[Codex] MCP wrapper event type: ${msgType}${payloadType ? ` (payload=${payloadType})` : ''}`); - } - } - if (msgType === 'agent_message') { const message = asString(msg.message); if (message) { @@ -324,29 +297,18 @@ class CodexRemoteLauncher extends RemoteLauncherBase { messageBuffer.addMessage('Starting task...', 'status'); } else if (msgType === 'task_complete') { messageBuffer.addMessage('Task completed', 'status'); - if (!useAppServer) { - sendReady(); - } } else if (msgType === 'turn_aborted') { messageBuffer.addMessage('Turn aborted', 'status'); - if (!useAppServer) { - sendReady(); - } } else if (msgType === 'task_failed') { const error = asString(msg.error); messageBuffer.addMessage(error ? `Task failed: ${error}` : 'Task failed', 'status'); - if (!useAppServer) { - sendReady(); - } } if (msgType === 'task_started') { clearReadyAfterTurnTimer?.(); - if (useAppServer) { - turnInFlight = true; - if (!eventTurnId && !this.currentTurnId) { - allowAnonymousTerminalEvent = true; - } + turnInFlight = true; + if (!eventTurnId && !this.currentTurnId) { + allowAnonymousTerminalEvent = true; } if (!session.thinking) { logger.debug('thinking started'); @@ -354,24 +316,20 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } } if (isTerminalEvent) { - if (useAppServer) { - turnInFlight = false; - allowAnonymousTerminalEvent = false; - } + turnInFlight = false; + allowAnonymousTerminalEvent = false; if (session.thinking) { logger.debug('thinking completed'); session.onThinkingChange(false); } diffProcessor.reset(); - appServerEventConverter?.reset(); + appServerEventConverter.reset(); } - if (useAppServer) { - if (isTerminalEvent && !turnInFlight) { - scheduleReadyAfterTurn?.(); - } else if (readyAfterTurnTimer && msgType !== 'task_started') { - scheduleReadyAfterTurn?.(); - } + if (isTerminalEvent && !turnInFlight) { + scheduleReadyAfterTurn?.(); + } else if (readyAfterTurnTimer && msgType !== 'task_started') { + scheduleReadyAfterTurn?.(); } if (msgType === 'agent_reasoning_section_break') { @@ -535,26 +493,18 @@ class CodexRemoteLauncher extends RemoteLauncherBase { } }; - if (useAppServer && appServerClient && appServerEventConverter) { - registerAppServerPermissionHandlers({ - client: appServerClient, - permissionHandler - }); + registerAppServerPermissionHandlers({ + client: appServerClient, + permissionHandler + }); - appServerClient.setNotificationHandler((method, params) => { - const events = appServerEventConverter.handleNotification(method, params); - for (const event of events) { - const eventRecord = asRecord(event) ?? { type: undefined }; - handleCodexEvent(eventRecord); - } - }); - } else if (mcpClient) { - mcpClient.setPermissionHandler(permissionHandler); - mcpClient.setHandler((msg) => { - const eventRecord = asRecord(msg) ?? { type: undefined }; + appServerClient.setNotificationHandler((method, params) => { + const events = appServerEventConverter.handleNotification(method, params); + for (const event of events) { + const eventRecord = asRecord(event) ?? { type: undefined }; handleCodexEvent(eventRecord); - }); - } + } + }); const { server: happyServer, mcpServers } = await buildHapiMcpBridge(session.client); this.happyServer = happyServer; @@ -580,33 +530,19 @@ class CodexRemoteLauncher extends RemoteLauncherBase { session.sendSessionEvent({ type: 'ready' }); }; - const syncSessionId = () => { - if (!mcpClient) return; - const clientSessionId = mcpClient.getSessionId(); - if (clientSessionId && clientSessionId !== session.sessionId) { - session.onSessionFound(clientSessionId); + await appServerClient.connect(); + await appServerClient.initialize({ + clientInfo: { + name: 'hapi-codex-client', + version: '1.0.0' + }, + capabilities: { + experimentalApi: true } - }; + }); - if (useAppServer && appServerClient) { - await appServerClient.connect(); - await appServerClient.initialize({ - clientInfo: { - name: 'hapi-codex-client', - version: '1.0.0' - }, - capabilities: { - experimentalApi: true - } - }); - } else if (mcpClient) { - await mcpClient.connect(); - } - - let wasCreated = false; - let currentModeHash: string | null = null; - let pending: { message: string; mode: EnhancedMode; isolate: boolean; hash: string } | null = null; - let first = true; + let hasThread = false; + let pending: QueuedMessage | null = null; clearReadyAfterTurnTimer = () => { if (!readyAfterTurnTimer) { @@ -632,7 +568,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { while (!this.shouldExit) { logActiveHandles('loop-top'); - let message: { message: string; mode: EnhancedMode; isolate: boolean; hash: string } | null = pending; + let message: QueuedMessage | null = pending; pending = null; if (!message) { const waitSignal = this.abortController.signal; @@ -652,178 +588,111 @@ class CodexRemoteLauncher extends RemoteLauncherBase { break; } - if (!useAppServer && wasCreated && currentModeHash && message.hash !== currentModeHash) { - logger.debug('[Codex] Mode changed – restarting Codex session'); - messageBuffer.addMessage('═'.repeat(40), 'status'); - messageBuffer.addMessage('Starting new Codex session (mode changed)...', 'status'); - mcpClient?.clearSession(); - wasCreated = false; - currentModeHash = null; - pending = message; - permissionHandler.reset(); - reasoningProcessor.abort(); - diffProcessor.reset(); - session.onThinkingChange(false); - continue; - } - messageBuffer.addMessage(message.message, 'user'); - currentModeHash = message.hash; try { - if (!wasCreated) { - if (useAppServer && appServerClient) { - const threadParams = buildThreadStartParams({ - mode: message.mode, - mcpServers, - cliOverrides: session.codexCliOverrides - }); + if (!hasThread) { + const threadParams = buildThreadStartParams({ + mode: message.mode, + mcpServers, + cliOverrides: session.codexCliOverrides + }); - const resumeCandidate = session.sessionId; - let threadId: string | null = null; + const resumeCandidate = session.sessionId; + let threadId: string | null = null; - if (resumeCandidate) { - try { - const resumeResponse = await appServerClient.resumeThread({ - threadId: resumeCandidate, - ...threadParams - }, { - signal: this.abortController.signal - }); - const resumeRecord = asRecord(resumeResponse); - const resumeThread = resumeRecord ? asRecord(resumeRecord.thread) : null; - threadId = asString(resumeThread?.id) ?? resumeCandidate; - applyResolvedModel(resumeRecord?.model); - logger.debug(`[Codex] Resumed app-server thread ${threadId}`); - } catch (error) { - logger.warn(`[Codex] Failed to resume app-server thread ${resumeCandidate}, starting new thread`, error); - } - } - - if (!threadId) { - const threadResponse = await appServerClient.startThread(threadParams, { + if (resumeCandidate) { + try { + const resumeResponse = await appServerClient.resumeThread({ + threadId: resumeCandidate, + ...threadParams + }, { signal: this.abortController.signal }); - const threadRecord = asRecord(threadResponse); - const thread = threadRecord ? asRecord(threadRecord.thread) : null; - threadId = asString(thread?.id); - applyResolvedModel(threadRecord?.model); - if (!threadId) { - throw new Error('app-server thread/start did not return thread.id'); - } + const resumeRecord = asRecord(resumeResponse); + const resumeThread = resumeRecord ? asRecord(resumeRecord.thread) : null; + threadId = asString(resumeThread?.id) ?? resumeCandidate; + applyResolvedModel(resumeRecord?.model); + logger.debug(`[Codex] Resumed app-server thread ${threadId}`); + } catch (error) { + logger.warn(`[Codex] Failed to resume app-server thread ${resumeCandidate}, starting new thread`, error); } - - if (!threadId) { - throw new Error('app-server resume did not return thread.id'); - } - - this.currentThreadId = threadId; - session.onSessionFound(threadId); - - const turnParams = buildTurnStartParams({ - threadId, - message: message.message, - mode: { - ...message.mode, - model: session.getModel() ?? message.mode.model - }, - cliOverrides: session.codexCliOverrides - }); - turnInFlight = true; - allowAnonymousTerminalEvent = false; - const turnResponse = await appServerClient.startTurn(turnParams, { - signal: this.abortController.signal - }); - const turnRecord = asRecord(turnResponse); - const turn = turnRecord ? asRecord(turnRecord.turn) : null; - const turnId = asString(turn?.id); - if (turnId) { - this.currentTurnId = turnId; - } else if (!this.currentTurnId) { - allowAnonymousTerminalEvent = true; - } - } else if (mcpClient) { - const startConfig: CodexSessionConfig = buildCodexStartConfig({ - message: message.message, - mode: message.mode, - first, - mcpServers, - cliOverrides: session.codexCliOverrides - }); - - await mcpClient.startSession(startConfig, { signal: this.abortController.signal }); - syncSessionId(); } - wasCreated = true; - first = false; - } else if (useAppServer && appServerClient) { + if (!threadId) { + const threadResponse = await appServerClient.startThread(threadParams, { + signal: this.abortController.signal + }); + const threadRecord = asRecord(threadResponse); + const thread = threadRecord ? asRecord(threadRecord.thread) : null; + threadId = asString(thread?.id); + applyResolvedModel(threadRecord?.model); + if (!threadId) { + throw new Error('app-server thread/start did not return thread.id'); + } + } + + if (!threadId) { + throw new Error('app-server resume did not return thread.id'); + } + + this.currentThreadId = threadId; + session.onSessionFound(threadId); + hasThread = true; + } else { if (!this.currentThreadId) { logger.debug('[Codex] Missing thread id; restarting app-server thread'); - wasCreated = false; + hasThread = false; pending = message; continue; } + } - const turnParams = buildTurnStartParams({ - threadId: this.currentThreadId, - message: message.message, - mode: { - ...message.mode, - model: session.getModel() ?? message.mode.model - }, - cliOverrides: session.codexCliOverrides - }); - turnInFlight = true; - allowAnonymousTerminalEvent = false; - const turnResponse = await appServerClient.startTurn(turnParams, { - signal: this.abortController.signal - }); - const turnRecord = asRecord(turnResponse); - const turn = turnRecord ? asRecord(turnRecord.turn) : null; - const turnId = asString(turn?.id); - if (turnId) { - this.currentTurnId = turnId; - } else if (!this.currentTurnId) { - allowAnonymousTerminalEvent = true; - } - } else if (mcpClient) { - await mcpClient.continueSession(message.message, { signal: this.abortController.signal }); - syncSessionId(); + const turnParams = buildTurnStartParams({ + threadId: this.currentThreadId, + message: message.message, + mode: { + ...message.mode, + model: session.getModel() ?? message.mode.model + }, + cliOverrides: session.codexCliOverrides + }); + turnInFlight = true; + allowAnonymousTerminalEvent = false; + const turnResponse = await appServerClient.startTurn(turnParams, { + signal: this.abortController.signal + }); + const turnRecord = asRecord(turnResponse); + const turn = turnRecord ? asRecord(turnRecord.turn) : null; + const turnId = asString(turn?.id); + if (turnId) { + this.currentTurnId = turnId; + } else if (!this.currentTurnId) { + allowAnonymousTerminalEvent = true; } } catch (error) { logger.warn('Error in codex session:', error); const isAbortError = error instanceof Error && error.name === 'AbortError'; - if (useAppServer) { - turnInFlight = false; - allowAnonymousTerminalEvent = false; - this.currentTurnId = null; - } + turnInFlight = false; + allowAnonymousTerminalEvent = false; + this.currentTurnId = null; if (isAbortError) { messageBuffer.addMessage('Aborted by user', 'status'); session.sendSessionEvent({ type: 'message', message: 'Aborted by user' }); - if (!useAppServer) { - wasCreated = false; - currentModeHash = null; - logger.debug('[Codex] Marked session as not created after abort for proper resume'); - } } else { messageBuffer.addMessage('Process exited unexpectedly', 'status'); session.sendSessionEvent({ type: 'message', message: 'Process exited unexpectedly' }); - if (useAppServer) { - this.currentTurnId = null; - this.currentThreadId = null; - wasCreated = false; - } + this.currentTurnId = null; + this.currentThreadId = null; + hasThread = false; } } finally { - const shouldFinalizeTurnState = !useAppServer || !turnInFlight; - if (shouldFinalizeTurnState) { + if (!turnInFlight) { permissionHandler.reset(); reasoningProcessor.abort(); diffProcessor.reset(); - appServerEventConverter?.reset(); + appServerEventConverter.reset(); session.onThinkingChange(false); clearReadyAfterTurnTimer?.(); emitReadyIfIdle({ @@ -841,12 +710,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { protected async cleanup(): Promise { logger.debug('[codex-remote]: cleanup start'); try { - if (this.appServerClient) { - await this.appServerClient.disconnect(); - } - if (this.mcpClient) { - await this.mcpClient.disconnect(); - } + await this.appServerClient.disconnect(); } catch (error) { logger.debug('[codex-remote]: Error disconnecting client', error); } diff --git a/cli/src/codex/runCodex.ts b/cli/src/codex/runCodex.ts index 4cbf4bc4..bab692fb 100644 --- a/cli/src/codex/runCodex.ts +++ b/cli/src/codex/runCodex.ts @@ -23,7 +23,6 @@ export async function runCodex(opts: { }): Promise { const workingDirectory = process.cwd(); const startedBy = opts.startedBy ?? 'terminal'; - const codexRemoteBackend = process.env.CODEX_USE_MCP_SERVER === '1' ? 'mcp-server' : 'app-server'; logger.debug(`[codex] Starting with options: startedBy=${startedBy}`); @@ -35,10 +34,7 @@ export async function runCodex(opts: { startedBy, workingDirectory, agentState: state, - model: opts.model, - metadataOverrides: { - codexRemoteBackend - } + model: opts.model }); const startingMode: 'local' | 'remote' = startedBy === 'runner' ? 'remote' : 'local'; @@ -152,9 +148,6 @@ export async function runCodex(opts: { } if (config.collaborationMode !== undefined) { - if (codexRemoteBackend !== 'app-server') { - throw new Error('Collaboration mode is only supported for Codex app-server remote sessions'); - } currentCollaborationMode = resolveCollaborationMode(config.collaborationMode); } diff --git a/cli/src/codex/types.ts b/cli/src/codex/types.ts deleted file mode 100644 index efaffb67..00000000 --- a/cli/src/codex/types.ts +++ /dev/null @@ -1,25 +0,0 @@ -/** - * Type definitions for Codex MCP integration - */ - -export interface CodexSessionConfig { - prompt: string; - 'approval-policy'?: 'untrusted' | 'on-failure' | 'on-request' | 'never'; - 'base-instructions'?: string; - config?: Record; - cwd?: string; - 'include-plan-tool'?: boolean; - model?: string; - profile?: string; - sandbox?: 'read-only' | 'workspace-write' | 'danger-full-access'; -} - -export interface CodexToolResponse { - content: Array<{ - type: 'text' | 'image' | 'resource'; - text?: string; - data?: any; - mimeType?: string; - }>; - isError?: boolean; -} diff --git a/cli/src/codex/utils/codexStartConfig.test.ts b/cli/src/codex/utils/codexStartConfig.test.ts deleted file mode 100644 index 37beb877..00000000 --- a/cli/src/codex/utils/codexStartConfig.test.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { buildCodexStartConfig } from './codexStartConfig'; -import { codexSystemPrompt } from './systemPrompt'; - -describe('buildCodexStartConfig', () => { - const mcpServers = { hapi: { command: 'node', args: ['mcp'] } }; - - it('applies CLI overrides when permission mode is default', () => { - const config = buildCodexStartConfig({ - message: 'hello', - mode: { permissionMode: 'default', collaborationMode: 'default' }, - first: true, - mcpServers, - cliOverrides: { sandbox: 'danger-full-access', approvalPolicy: 'never' } - }); - - expect(config.sandbox).toBe('danger-full-access'); - expect(config['approval-policy']).toBe('never'); - expect(config.config).toEqual({ - mcp_servers: mcpServers, - developer_instructions: codexSystemPrompt - }); - }); - - it('ignores CLI overrides when permission mode is not default', () => { - const config = buildCodexStartConfig({ - message: 'hello', - mode: { permissionMode: 'yolo', collaborationMode: 'default' }, - first: false, - mcpServers, - cliOverrides: { sandbox: 'read-only', approvalPolicy: 'never' } - }); - - expect(config.sandbox).toBe('danger-full-access'); - expect(config['approval-policy']).toBe('never'); - }); - - it('keeps on-failure approvals for safe-yolo', () => { - const config = buildCodexStartConfig({ - message: 'hello', - mode: { permissionMode: 'safe-yolo', collaborationMode: 'default' }, - first: false, - mcpServers - }); - - expect(config.sandbox).toBe('workspace-write'); - expect(config['approval-policy']).toBe('on-failure'); - }); - - it('passes model when provided', () => { - const config = buildCodexStartConfig({ - message: 'hello', - mode: { permissionMode: 'default', model: 'o3', collaborationMode: 'default' }, - first: false, - mcpServers - }); - - expect(config.model).toBe('o3'); - }); -}); diff --git a/cli/src/codex/utils/codexStartConfig.ts b/cli/src/codex/utils/codexStartConfig.ts deleted file mode 100644 index 7ba9556f..00000000 --- a/cli/src/codex/utils/codexStartConfig.ts +++ /dev/null @@ -1,50 +0,0 @@ -import type { CodexSessionConfig } from '../types'; -import type { EnhancedMode } from '../loop'; -import type { CodexCliOverrides } from './codexCliOverrides'; -import { codexSystemPrompt } from './systemPrompt'; -import { resolveCodexPermissionModeConfig } from './permissionModeConfig'; - -function resolveApprovalPolicy(mode: EnhancedMode): CodexSessionConfig['approval-policy'] { - return resolveCodexPermissionModeConfig(mode.permissionMode).approvalPolicy; -} - -function resolveSandbox(mode: EnhancedMode): CodexSessionConfig['sandbox'] { - return resolveCodexPermissionModeConfig(mode.permissionMode).sandbox; -} - -export function buildCodexStartConfig(args: { - message: string; - mode: EnhancedMode; - first: boolean; - mcpServers: Record; - cliOverrides?: CodexCliOverrides; - developerInstructions?: string; -}): CodexSessionConfig { - const approvalPolicy = resolveApprovalPolicy(args.mode); - const sandbox = resolveSandbox(args.mode); - const allowCliOverrides = args.mode.permissionMode === 'default'; - const cliOverrides = allowCliOverrides ? args.cliOverrides : undefined; - const resolvedApprovalPolicy = cliOverrides?.approvalPolicy ?? approvalPolicy; - const resolvedSandbox = cliOverrides?.sandbox ?? sandbox; - - const prompt = args.message; - const baseInstructions = codexSystemPrompt; - const config: Record = { - mcp_servers: args.mcpServers, - developer_instructions: args.developerInstructions - ? `${baseInstructions}\n\n${args.developerInstructions}` - : baseInstructions - }; - const startConfig: CodexSessionConfig = { - prompt, - sandbox: resolvedSandbox, - 'approval-policy': resolvedApprovalPolicy, - config - }; - - if (args.mode.model) { - startConfig.model = args.mode.model; - } - - return startConfig; -} diff --git a/cli/src/codex/utils/terminalEventGuard.test.ts b/cli/src/codex/utils/terminalEventGuard.test.ts index c9c9947f..5896b11e 100644 --- a/cli/src/codex/utils/terminalEventGuard.test.ts +++ b/cli/src/codex/utils/terminalEventGuard.test.ts @@ -2,20 +2,8 @@ import { describe, expect, it } from 'vitest'; import { shouldIgnoreTerminalEvent } from './terminalEventGuard'; describe('shouldIgnoreTerminalEvent', () => { - it('returns false for non app-server mode', () => { - const ignored = shouldIgnoreTerminalEvent({ - useAppServer: false, - eventTurnId: null, - currentTurnId: 'turn-1', - turnInFlight: true - }); - - expect(ignored).toBe(false); - }); - it('ignores terminal events without turn_id when current turn id exists', () => { const ignored = shouldIgnoreTerminalEvent({ - useAppServer: true, eventTurnId: null, currentTurnId: 'turn-1', turnInFlight: true @@ -26,7 +14,6 @@ describe('shouldIgnoreTerminalEvent', () => { it('ignores terminal events without turn_id while a turn is still in flight', () => { const ignored = shouldIgnoreTerminalEvent({ - useAppServer: true, eventTurnId: null, currentTurnId: null, turnInFlight: true @@ -37,7 +24,6 @@ describe('shouldIgnoreTerminalEvent', () => { it('accepts terminal events without turn_id when anonymous terminal is explicitly allowed', () => { const ignored = shouldIgnoreTerminalEvent({ - useAppServer: true, eventTurnId: null, currentTurnId: null, turnInFlight: true, @@ -49,7 +35,6 @@ describe('shouldIgnoreTerminalEvent', () => { it('still ignores terminal events without turn_id when current turn id exists', () => { const ignored = shouldIgnoreTerminalEvent({ - useAppServer: true, eventTurnId: null, currentTurnId: 'turn-1', turnInFlight: true, @@ -61,7 +46,6 @@ describe('shouldIgnoreTerminalEvent', () => { it('ignores stale terminal events from another turn', () => { const ignored = shouldIgnoreTerminalEvent({ - useAppServer: true, eventTurnId: 'turn-old', currentTurnId: 'turn-current', turnInFlight: true @@ -72,7 +56,6 @@ describe('shouldIgnoreTerminalEvent', () => { it('accepts terminal events that match the current turn id', () => { const ignored = shouldIgnoreTerminalEvent({ - useAppServer: true, eventTurnId: 'turn-current', currentTurnId: 'turn-current', turnInFlight: true @@ -83,7 +66,6 @@ describe('shouldIgnoreTerminalEvent', () => { it('accepts terminal events without turn_id when no turn is active', () => { const ignored = shouldIgnoreTerminalEvent({ - useAppServer: true, eventTurnId: null, currentTurnId: null, turnInFlight: false diff --git a/cli/src/codex/utils/terminalEventGuard.ts b/cli/src/codex/utils/terminalEventGuard.ts index 0319a7f0..a4b76b01 100644 --- a/cli/src/codex/utils/terminalEventGuard.ts +++ b/cli/src/codex/utils/terminalEventGuard.ts @@ -1,5 +1,4 @@ export type TerminalEventGuardInput = { - useAppServer: boolean; eventTurnId: string | null; currentTurnId: string | null; turnInFlight: boolean; @@ -9,10 +8,6 @@ export type TerminalEventGuardInput = { export function shouldIgnoreTerminalEvent(input: TerminalEventGuardInput): boolean { const allowAnonymousTerminalEvent = input.allowAnonymousTerminalEvent === true; - if (!input.useAppServer) { - return false; - } - if (input.eventTurnId) { return Boolean(input.currentTurnId && input.eventTurnId !== input.currentTurnId); } diff --git a/hub/src/web/routes/sessions.test.ts b/hub/src/web/routes/sessions.test.ts index c4ff06a1..bb56a06c 100644 --- a/hub/src/web/routes/sessions.test.ts +++ b/hub/src/web/routes/sessions.test.ts @@ -8,8 +8,7 @@ function createSession(overrides?: Partial): Session { const baseMetadata = { path: '/tmp/project', host: 'localhost', - flavor: 'codex' as const, - codexRemoteBackend: 'app-server' as const + flavor: 'codex' as const } const base: Session = { id: 'session-1', @@ -70,13 +69,12 @@ function createApp(session: Session) { } describe('sessions routes', () => { - it('rejects collaboration mode changes for MCP-backed Codex sessions', async () => { + it('rejects collaboration mode changes for local Codex sessions', async () => { const session = createSession({ - metadata: { - path: '/tmp/project', - host: 'localhost', - flavor: 'codex', - codexRemoteBackend: 'mcp-server' + agentState: { + controlledByUser: true, + requests: {}, + completedRequests: {} } }) const { app, applySessionConfigCalls } = createApp(session) @@ -89,12 +87,35 @@ describe('sessions routes', () => { expect(response.status).toBe(409) expect(await response.json()).toEqual({ - error: 'Collaboration mode is only supported for Codex app-server remote sessions' + error: 'Collaboration mode can only be changed for remote Codex sessions' }) expect(applySessionConfigCalls).toEqual([]) }) - it('applies collaboration mode changes for app-server Codex sessions', async () => { + it('rejects collaboration mode changes for non-Codex sessions', async () => { + const session = createSession({ + metadata: { + path: '/tmp/project', + host: 'localhost', + flavor: 'claude' + } + }) + const { app, applySessionConfigCalls } = createApp(session) + + const response = await app.request('/api/sessions/session-1/collaboration-mode', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ mode: 'plan' }) + }) + + expect(response.status).toBe(400) + expect(await response.json()).toEqual({ + error: 'Collaboration mode is only supported for Codex sessions' + }) + expect(applySessionConfigCalls).toEqual([]) + }) + + it('applies collaboration mode changes for remote Codex sessions', async () => { const { app, applySessionConfigCalls } = createApp(createSession()) const response = await app.request('/api/sessions/session-1/collaboration-mode', { diff --git a/hub/src/web/routes/sessions.ts b/hub/src/web/routes/sessions.ts index d17d0b84..054528ab 100644 --- a/hub/src/web/routes/sessions.ts +++ b/hub/src/web/routes/sessions.ts @@ -278,9 +278,6 @@ export function createSessionsRoutes(getSyncEngine: () => SyncEngine | null): Ho if (sessionResult.session.agentState?.controlledByUser === true) { return c.json({ error: 'Collaboration mode can only be changed for remote Codex sessions' }, 409) } - if (sessionResult.session.metadata?.codexRemoteBackend !== 'app-server') { - return c.json({ error: 'Collaboration mode is only supported for Codex app-server remote sessions' }, 409) - } const body = await c.req.json().catch(() => null) const parsed = collaborationModeSchema.safeParse(body) diff --git a/shared/src/schemas.ts b/shared/src/schemas.ts index ae96ce7a..55e8cea6 100644 --- a/shared/src/schemas.ts +++ b/shared/src/schemas.ts @@ -46,7 +46,6 @@ export const MetadataSchema = z.object({ archivedBy: z.string().optional(), archiveReason: z.string().optional(), flavor: z.string().nullish(), - codexRemoteBackend: z.enum(['app-server', 'mcp-server']).optional(), worktree: WorktreeMetadataSchema.optional() }) diff --git a/web/src/components/SessionChat.tsx b/web/src/components/SessionChat.tsx index c9820890..c435447c 100644 --- a/web/src/components/SessionChat.tsx +++ b/web/src/components/SessionChat.tsx @@ -46,9 +46,8 @@ export function SessionChat(props: { const blocksByIdRef = useRef>(new Map()) const [forceScrollToken, setForceScrollToken] = useState(0) const agentFlavor = props.session.metadata?.flavor ?? null - const codexCollaborationModeSupported = agentFlavor === 'codex' - && props.session.metadata?.codexRemoteBackend === 'app-server' const controlledByUser = props.session.agentState?.controlledByUser === true + const codexCollaborationModeSupported = agentFlavor === 'codex' && !controlledByUser const { abortSession, switchSession, setPermissionMode, setCollaborationMode, setModel } = useSessionActions( props.api, props.session.id, diff --git a/web/src/hooks/mutations/useSessionActions.ts b/web/src/hooks/mutations/useSessionActions.ts index 1910d667..aaed3661 100644 --- a/web/src/hooks/mutations/useSessionActions.ts +++ b/web/src/hooks/mutations/useSessionActions.ts @@ -82,7 +82,7 @@ export function useSessionActions( throw new Error('Collaboration mode is only supported for Codex sessions') } if (!codexCollaborationModeSupported) { - throw new Error('Collaboration mode is only supported for Codex app-server remote sessions') + throw new Error('Collaboration mode is only supported for remote Codex sessions') } await api.setCollaborationMode(sessionId, mode) },