diff --git a/cli/src/codex/codexRemoteLauncher.ts b/cli/src/codex/codexRemoteLauncher.ts index be648d65d..53024deec 100644 --- a/cli/src/codex/codexRemoteLauncher.ts +++ b/cli/src/codex/codexRemoteLauncher.ts @@ -177,6 +177,17 @@ class CodexRemoteLauncher extends RemoteLauncherBase { : undefined; }, { onRequest: ({ id, toolName, input }) => { + if (toolName === 'request_user_input') { + session.sendAgentMessage({ + type: 'tool-call', + name: 'request_user_input', + callId: id, + input, + id: randomUUID() + }); + return; + } + const inputRecord = input && typeof input === 'object' ? input as Record : {}; const message = typeof inputRecord.message === 'string' ? inputRecord.message : undefined; const rawCommand = inputRecord.command; @@ -201,14 +212,16 @@ class CodexRemoteLauncher extends RemoteLauncherBase { id: randomUUID() }); }, - onComplete: ({ id, decision, reason, approved }) => { + onComplete: ({ id, toolName, decision, reason, approved, answers }) => { session.sendAgentMessage({ type: 'tool-call-result', callId: id, - output: { - decision, - reason - }, + output: toolName === 'request_user_input' + ? { answers } + : { + decision, + reason + }, is_error: !approved, id: randomUUID() }); @@ -495,7 +508,22 @@ class CodexRemoteLauncher extends RemoteLauncherBase { registerAppServerPermissionHandlers({ client: appServerClient, - permissionHandler + permissionHandler, + onUserInputRequest: async ({ id, input }) => { + try { + const answers = await permissionHandler.handleUserInputRequest(id, input); + return { + decision: 'accept', + answers + }; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.debug(`[Codex] request_user_input failed: ${message}`); + return { + decision: 'cancel' + }; + } + } }); appServerClient.setNotificationHandler((method, params) => { diff --git a/cli/src/codex/utils/appServerPermissionAdapter.test.ts b/cli/src/codex/utils/appServerPermissionAdapter.test.ts new file mode 100644 index 000000000..f899111e0 --- /dev/null +++ b/cli/src/codex/utils/appServerPermissionAdapter.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it, vi } from 'vitest'; +import { registerAppServerPermissionHandlers } from './appServerPermissionAdapter'; + +type UserInputHandler = NonNullable[0]['onUserInputRequest']>; + +function createClient() { + const handlers = new Map Promise | unknown>(); + return { + client: { + registerRequestHandler(method: string, handler: (params: unknown) => Promise | unknown) { + handlers.set(method, handler); + } + }, + handlers + }; +} + +describe('registerAppServerPermissionHandlers', () => { + it('forwards request_user_input answers through the callback', async () => { + const { client, handlers } = createClient(); + const permissionHandler = { + handleToolCall: vi.fn() + }; + const onUserInputRequest: UserInputHandler = async ({ id, input }) => { + expect(id).toBe('tool-123'); + expect(input).toEqual({ + itemId: 'tool-123', + questions: [{ id: 'approve_nav', question: 'Approve app tool call?' }] + }); + return { + decision: 'accept', + answers: { + approve_nav: { + answers: ['Allow'] + } + } + }; + }; + + registerAppServerPermissionHandlers({ + client: client as never, + permissionHandler: permissionHandler as never, + onUserInputRequest: vi.fn(onUserInputRequest) + }); + + const handler = handlers.get('item/tool/requestUserInput'); + expect(handler).toBeTypeOf('function'); + + await expect(handler?.({ + itemId: 'tool-123', + questions: [{ id: 'approve_nav', question: 'Approve app tool call?' }] + })).resolves.toEqual({ + decision: 'accept', + answers: { + approve_nav: { + answers: ['Allow'] + } + } + }); + }); + + it('cancels request_user_input when no callback is registered', async () => { + const { client, handlers } = createClient(); + const permissionHandler = { + handleToolCall: vi.fn() + }; + + registerAppServerPermissionHandlers({ + client: client as never, + permissionHandler: permissionHandler as never + }); + + const handler = handlers.get('item/tool/requestUserInput'); + expect(handler).toBeTypeOf('function'); + + await expect(handler?.({ itemId: 'tool-123' })).resolves.toEqual({ + decision: 'cancel' + }); + }); +}); diff --git a/cli/src/codex/utils/appServerPermissionAdapter.ts b/cli/src/codex/utils/appServerPermissionAdapter.ts index 73c409293..78fe9f1f5 100644 --- a/cli/src/codex/utils/appServerPermissionAdapter.ts +++ b/cli/src/codex/utils/appServerPermissionAdapter.ts @@ -37,7 +37,10 @@ function mapDecision(decision: PermissionDecision): { decision: string } { export function registerAppServerPermissionHandlers(args: { client: CodexAppServerClient; permissionHandler: CodexPermissionHandler; - onUserInputRequest?: (request: unknown) => Promise>; + onUserInputRequest?: (request: { id: string; input: unknown }) => Promise< + | { decision: 'accept'; answers: Record | Record } + | { decision: 'decline' | 'cancel' } + >; }): void { const { client, permissionHandler, onUserInputRequest } = args; @@ -80,15 +83,23 @@ export function registerAppServerPermissionHandlers(args: { }); client.registerRequestHandler('item/tool/requestUserInput', async (params) => { + const record = asRecord(params) ?? {}; + const requestId = asString(record.itemId) ?? randomUUID(); + if (!onUserInputRequest) { logger.debug('[CodexAppServer] No user-input handler registered; cancelling request'); return { decision: 'cancel' }; } - const answers = await onUserInputRequest(params); - return { - decision: 'accept', - answers - }; + const result = await onUserInputRequest({ + id: requestId, + input: params + }); + + if (result.decision !== 'accept') { + return { decision: result.decision }; + } + + return result; }); } diff --git a/cli/src/codex/utils/permissionHandler.test.ts b/cli/src/codex/utils/permissionHandler.test.ts index f51ec3f22..8de0adca4 100644 --- a/cli/src/codex/utils/permissionHandler.test.ts +++ b/cli/src/codex/utils/permissionHandler.test.ts @@ -133,4 +133,43 @@ describe('CodexPermissionHandler', () => { handler.reset(); await expect(patchPromise).rejects.toThrow('Session reset'); }); + + it('keeps request_user_input pending until answers arrive and stores nested answers', async () => { + const { handler, rpcHandlers, getAgentState } = createHarness('default'); + const resultPromise = handler.handleUserInputRequest('input-1', { + questions: [{ id: 'approve_nav', question: 'Approve app tool call?' }] + }); + + expect(getAgentState().requests).toMatchObject({ + 'input-1': { + tool: 'request_user_input' + } + }); + + const permissionRpc = rpcHandlers.get('permission'); + expect(permissionRpc).toBeTypeOf('function'); + + const answers = { + approve_nav: { + answers: ['Allow'] + } + }; + + await permissionRpc?.({ + id: 'input-1', + approved: true, + answers + }); + + await expect(resultPromise).resolves.toEqual(answers); + + expect(getAgentState().requests).toEqual({}); + expect(getAgentState().completedRequests).toMatchObject({ + 'input-1': { + tool: 'request_user_input', + status: 'approved', + answers + } + }); + }); }); diff --git a/cli/src/codex/utils/permissionHandler.ts b/cli/src/codex/utils/permissionHandler.ts index 2b50decfb..89e2d73ae 100644 --- a/cli/src/codex/utils/permissionHandler.ts +++ b/cli/src/codex/utils/permissionHandler.ts @@ -20,12 +20,19 @@ interface PermissionResponse { approved: boolean; decision?: 'approved' | 'approved_for_session' | 'denied' | 'abort'; reason?: string; + answers?: Record | Record; } -interface PermissionResult { +type ToolPermissionResult = { decision: 'approved' | 'approved_for_session' | 'denied' | 'abort'; reason?: string; -} +}; + +type UserInputResult = { + answers: Record | Record; +}; + +type PermissionResult = ToolPermissionResult | UserInputResult; type CodexPermissionHandlerOptions = { onRequest?: (request: { id: string; toolName: string; input: unknown }) => void; @@ -34,8 +41,9 @@ type CodexPermissionHandlerOptions = { toolName: string; input: unknown; approved: boolean; - decision: PermissionResult['decision']; + decision?: ToolPermissionResult['decision']; reason?: string; + answers?: Record | Record; }) => void; }; @@ -57,7 +65,7 @@ export class CodexPermissionHandler extends BasePermissionHandler { + ): Promise { const mode = this.getPermissionMode() ?? 'default'; const autoDecision = this.resolveAutoApprovalDecision(mode, toolName, toolCallId); if (autoDecision) { @@ -124,6 +132,26 @@ export class CodexPermissionHandler extends BasePermissionHandler { + if ('answers' in result) { + throw new Error(`Expected permission decision for ${toolName}, received request_user_input answers`); + } + return result; + }); + } + + async handleUserInputRequest( + toolCallId: string, + input: unknown + ): Promise | Record> { + return new Promise((resolve, reject) => { + this.addPendingRequest(toolCallId, 'request_user_input', input, { resolve, reject }); + logger.debug(`[Codex] User-input request sent (${toolCallId})`); + }).then((result) => { + if (!('answers' in result)) { + throw new Error(`Expected request_user_input answers for ${toolCallId}, received permission decision`); + } + return result.answers; }); } @@ -134,8 +162,39 @@ export class CodexPermissionHandler extends BasePermissionHandler ): Promise { + if (pending.toolName === 'request_user_input') { + const answers = response.answers ?? {}; + + if (!response.approved || Object.keys(answers).length === 0) { + pending.reject(new Error(response.reason || 'No answers were provided.')); + logger.debug('[Codex] User-input request denied or missing answers'); + return { + status: response.approved ? 'denied' : 'canceled', + reason: response.reason || 'No answers were provided.', + decision: response.decision ?? (response.approved ? 'denied' : 'abort'), + answers + }; + } + + pending.resolve({ answers }); + logger.debug('[Codex] User-input request approved'); + + this.options?.onComplete?.({ + id: response.id, + toolName: pending.toolName, + input: pending.input, + approved: true, + answers + }); + + return { + status: 'approved', + answers + }; + } + const reason = typeof response.reason === 'string' ? response.reason : undefined; - const result: PermissionResult = response.approved + const result: ToolPermissionResult = response.approved ? { decision: response.decision === 'approved_for_session' ? 'approved_for_session' : 'approved', reason @@ -154,7 +213,8 @@ export class CodexPermissionHandler extends BasePermissionHandler