fix(codex): handle remote request_user_input approvals (#382)

2026-08-05 06:24:37 +00:00 · 2026-04-07 12:04:52 +08:00
parent 3ef7b8c7cd
commit 56925f8b98
5 changed files with 237 additions and 19 deletions
@@ -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<string, unknown> : {};
                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) => {
@@ -0,0 +1,80 @@
+import { describe, expect, it, vi } from 'vitest';
+import { registerAppServerPermissionHandlers } from './appServerPermissionAdapter';
+
+type UserInputHandler = NonNullable<Parameters<typeof registerAppServerPermissionHandlers>[0]['onUserInputRequest']>;
+
+function createClient() {
+    const handlers = new Map<string, (params: unknown) => Promise<unknown> | unknown>();
+    return {
+        client: {
+            registerRequestHandler(method: string, handler: (params: unknown) => Promise<unknown> | 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'
+        });
+    });
+});
@@ -37,7 +37,10 @@ function mapDecision(decision: PermissionDecision): { decision: string } {
 export function registerAppServerPermissionHandlers(args: {
    client: CodexAppServerClient;
    permissionHandler: CodexPermissionHandler;
-    onUserInputRequest?: (request: unknown) => Promise<Record<string, string[]>>;
+    onUserInputRequest?: (request: { id: string; input: unknown }) => Promise<
+        | { decision: 'accept'; answers: Record<string, string[]> | Record<string, { answers: string[] }> }
+        | { 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;
    });
 }
@@ -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
+            }
+        });
+    });
 });
@@ -20,12 +20,19 @@ interface PermissionResponse {
    approved: boolean;
    decision?: 'approved' | 'approved_for_session' | 'denied' | 'abort';
    reason?: string;
+    answers?: Record<string, string[]> | Record<string, { answers: string[] }>;
 }

-interface PermissionResult {
+type ToolPermissionResult = {
    decision: 'approved' | 'approved_for_session' | 'denied' | 'abort';
    reason?: string;
-}
+};
+
+type UserInputResult = {
+    answers: Record<string, string[]> | Record<string, { answers: string[] }>;
+};
+
+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<string, string[]> | Record<string, { answers: string[] }>;
    }) => void;
 };

@@ -57,7 +65,7 @@ export class CodexPermissionHandler extends BasePermissionHandler<PermissionResp
        toolName: string,
        input: unknown,
        decision: AutoApprovalDecision
-    ): PermissionResult {
+    ): ToolPermissionResult {
        const timestamp = Date.now();

        this.options?.onRequest?.({ id, toolName, input });
@@ -100,7 +108,7 @@ export class CodexPermissionHandler extends BasePermissionHandler<PermissionResp
        toolCallId: string,
        toolName: string,
        input: unknown
-    ): Promise<PermissionResult> {
+    ): Promise<ToolPermissionResult> {
        const mode = this.getPermissionMode() ?? 'default';
        const autoDecision = this.resolveAutoApprovalDecision(mode, toolName, toolCallId);
        if (autoDecision) {
@@ -124,6 +132,26 @@ export class CodexPermissionHandler extends BasePermissionHandler<PermissionResp
            // );

            logger.debug(`[Codex] Permission request sent for tool: ${toolName} (${toolCallId})`);
+        }).then((result) => {
+            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<string, string[]> | Record<string, { answers: string[] }>> {
+        return new Promise<PermissionResult>((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<PermissionResp
        response: PermissionResponse,
        pending: PendingPermissionRequest<PermissionResult>
    ): Promise<PermissionCompletion> {
+        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<PermissionResp
            input: pending.input,
            approved: response.approved,
            decision: result.decision,
-            reason: result.reason
+            reason: result.reason,
+            answers: response.answers
        });

        return {