-
-
Notifications
You must be signed in to change notification settings - Fork 349
fix(codex): Handle mcpServer/elicitation/request #381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 7 commits
1bb7cc5
6b684ac
0fe8747
8239d80
36c71b5
d63c4e3
9f19cb4
c985e7d
daa2bab
bd7d0fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import { buildHapiMcpBridge } from './utils/buildHapiMcpBridge'; | |
| import { emitReadyIfIdle } from './utils/emitReadyIfIdle'; | ||
| import type { CodexSession } from './session'; | ||
| import type { EnhancedMode } from './loop'; | ||
| import type { McpServerElicitationRequestParams, McpServerElicitationResponse } from './appServerTypes'; | ||
| import { hasCodexCliOverrides } from './utils/codexCliOverrides'; | ||
| import { AppServerEventConverter } from './utils/appServerEventConverter'; | ||
| import { registerAppServerPermissionHandlers } from './utils/appServerPermissionAdapter'; | ||
|
|
@@ -24,6 +25,14 @@ import { | |
|
|
||
| type HappyServer = Awaited<ReturnType<typeof buildHapiMcpBridge>>['server']; | ||
| type QueuedMessage = { message: string; mode: EnhancedMode; isolate: boolean; hash: string }; | ||
| type McpElicitationRpcResponse = { | ||
| id: string; | ||
| action: 'accept' | 'decline' | 'cancel'; | ||
| content?: unknown | null; | ||
| }; | ||
| type PendingMcpElicitationRequest = { | ||
| resolve: (response: McpServerElicitationResponse) => void; | ||
| }; | ||
|
|
||
| class CodexRemoteLauncher extends RemoteLauncherBase { | ||
| private readonly session: CodexSession; | ||
|
|
@@ -35,6 +44,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { | |
| private abortController: AbortController = new AbortController(); | ||
| private currentThreadId: string | null = null; | ||
| private currentTurnId: string | null = null; | ||
| private readonly pendingMcpElicitationRequests = new Map<string, PendingMcpElicitationRequest>(); | ||
|
|
||
| constructor(session: CodexSession) { | ||
| super(process.env.DEBUG ? session.logPath : undefined); | ||
|
|
@@ -64,6 +74,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { | |
| this.abortController.abort(); | ||
| this.session.queue.reset(); | ||
| this.permissionHandler?.reset(); | ||
| this.cancelPendingMcpElicitationRequests(); | ||
| this.reasoningProcessor?.abort(); | ||
| this.diffProcessor?.reset(); | ||
| logger.debug('[Codex] Abort completed - session remains active'); | ||
|
|
@@ -74,6 +85,16 @@ class CodexRemoteLauncher extends RemoteLauncherBase { | |
| } | ||
| } | ||
|
|
||
| private cancelPendingMcpElicitationRequests(): void { | ||
| for (const [requestId, pending] of this.pendingMcpElicitationRequests.entries()) { | ||
| pending.resolve({ | ||
| action: 'cancel', | ||
| content: null | ||
| }); | ||
| this.pendingMcpElicitationRequests.delete(requestId); | ||
| } | ||
| } | ||
|
|
||
| private async handleExitFromUi(): Promise<void> { | ||
| logger.debug('[codex-remote]: Exiting agent via Ctrl-C'); | ||
| this.exitReason = 'exit'; | ||
|
|
@@ -229,6 +250,107 @@ class CodexRemoteLauncher extends RemoteLauncherBase { | |
| let turnInFlight = false; | ||
| let allowAnonymousTerminalEvent = false; | ||
|
|
||
| const toMcpElicitationResponse = (response: McpElicitationRpcResponse): McpServerElicitationResponse => ({ | ||
| action: response.action, | ||
| content: response.action === 'accept' ? response.content ?? null : null | ||
| }); | ||
|
|
||
| const parseMcpElicitationRequest = (params: McpServerElicitationRequestParams) => { | ||
| const paramsRecord = asRecord(params) ?? {}; | ||
| const mode = asString(paramsRecord.mode); | ||
| const message = asString(paramsRecord.message) ?? ''; | ||
| const requestedSchema = asRecord(paramsRecord.requestedSchema); | ||
| const url = asString(paramsRecord.url); | ||
| const elicitationId = asString(paramsRecord.elicitationId); | ||
|
|
||
| if (mode !== 'form' && mode !== 'url') { | ||
| throw new Error('Invalid MCP elicitation request: missing mode'); | ||
| } | ||
|
|
||
| if (mode === 'form' && !requestedSchema) { | ||
| throw new Error('Invalid MCP elicitation form request: missing requestedSchema'); | ||
| } | ||
|
|
||
| if (mode === 'url' && !url) { | ||
| throw new Error('Invalid MCP elicitation URL request: missing url'); | ||
| } | ||
|
|
||
| const requestId = mode === 'url' ? (elicitationId ?? randomUUID()) : randomUUID(); | ||
|
|
||
| return { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MINOR] The web side now reads MCP tool titles/descriptions from Suggested fix: const meta = asRecord(requestRecord._meta)
return {
requestId,
threadId: params.threadId,
turnId: params.turnId,
serverName: params.serverName,
mode,
message,
requestedSchema: mode === 'form' ? requestedSchema : undefined,
url: mode === 'url' ? url : undefined,
elicitationId: mode === 'url' ? elicitationId : undefined,
_meta: meta ?? undefined
} |
||
| requestId, | ||
| threadId: params.threadId, | ||
| turnId: params.turnId, | ||
| serverName: params.serverName, | ||
| mode, | ||
| message, | ||
| requestedSchema: mode === 'form' ? requestedSchema : undefined, | ||
| url: mode === 'url' ? url : undefined, | ||
| elicitationId: mode === 'url' ? elicitationId : undefined | ||
| }; | ||
| }; | ||
|
|
||
| const waitForMcpElicitationResponse = async (requestId: string): Promise<McpServerElicitationResponse> => { | ||
| return await new Promise<McpServerElicitationResponse>((resolve) => { | ||
| this.pendingMcpElicitationRequests.set(requestId, { resolve }); | ||
| }); | ||
| }; | ||
|
|
||
| const handleMcpElicitationResponse = async (response: unknown): Promise<void> => { | ||
| const record = asRecord(response) ?? {}; | ||
| const requestId = asString(record.id); | ||
| const action = asString(record.action); | ||
| if (!requestId || (action !== 'accept' && action !== 'decline' && action !== 'cancel')) { | ||
| logger.debug('[Codex] Ignoring invalid MCP elicitation response payload', response); | ||
| return; | ||
| } | ||
|
|
||
| const pending = this.pendingMcpElicitationRequests.get(requestId); | ||
| if (!pending) { | ||
| logger.debug(`[Codex] No pending MCP elicitation request for id ${requestId}`); | ||
| return; | ||
| } | ||
|
|
||
| this.pendingMcpElicitationRequests.delete(requestId); | ||
| pending.resolve(toMcpElicitationResponse({ | ||
| id: requestId, | ||
| action, | ||
| content: record.content | ||
| })); | ||
| }; | ||
|
|
||
| const handleMcpElicitationRequest = async ( | ||
| params: McpServerElicitationRequestParams | ||
| ): Promise<McpServerElicitationResponse> => { | ||
| const request = parseMcpElicitationRequest(params); | ||
|
|
||
| logger.debug('[Codex] Bridging MCP elicitation request', { | ||
| requestId: request.requestId, | ||
| mode: request.mode, | ||
| serverName: request.serverName | ||
| }); | ||
|
|
||
| session.sendAgentMessage({ | ||
| type: 'tool-call', | ||
| name: 'CodexMcpElicitation', | ||
| callId: request.requestId, | ||
| input: request, | ||
| id: randomUUID() | ||
| }); | ||
|
|
||
| const result = await waitForMcpElicitationResponse(request.requestId); | ||
|
|
||
| session.sendAgentMessage({ | ||
| type: 'tool-call-result', | ||
| callId: request.requestId, | ||
| output: result, | ||
| is_error: result.action !== 'accept', | ||
| id: randomUUID() | ||
| }); | ||
|
|
||
| return result; | ||
| }; | ||
|
|
||
| const handleCodexEvent = (msg: Record<string, unknown>) => { | ||
| const msgType = asString(msg.type); | ||
| if (!msgType) return; | ||
|
|
@@ -495,7 +617,8 @@ class CodexRemoteLauncher extends RemoteLauncherBase { | |
|
|
||
| registerAppServerPermissionHandlers({ | ||
| client: appServerClient, | ||
| permissionHandler | ||
| permissionHandler, | ||
| onMcpElicitationRequest: handleMcpElicitationRequest | ||
| }); | ||
|
|
||
| appServerClient.setNotificationHandler((method, params) => { | ||
|
|
@@ -513,6 +636,10 @@ class CodexRemoteLauncher extends RemoteLauncherBase { | |
| onAbort: () => this.handleAbort(), | ||
| onSwitch: () => this.handleSwitchRequest() | ||
| }); | ||
| session.client.rpcHandlerManager.registerHandler<McpElicitationRpcResponse, void>( | ||
| 'mcp-elicitation-response', | ||
| handleMcpElicitationResponse | ||
| ); | ||
|
|
||
| function logActiveHandles(tag: string) { | ||
| if (!process.env.DEBUG) return; | ||
|
|
@@ -725,6 +852,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { | |
| } | ||
|
|
||
| this.permissionHandler?.reset(); | ||
| this.cancelPendingMcpElicitationRequests(); | ||
| this.reasoningProcessor?.abort(); | ||
| this.diffProcessor?.reset(); | ||
| this.permissionHandler = null; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MAJOR]
McpServerElicitationRequestParamsputs the actual elicitation payload underparams.request, but this parser readsmode/message/requestedSchema/urlfrom the top level. With the typed app-server shape,modestaysnullhere and the request is rejected before it ever reaches the hub/web bridge.Suggested fix: