Skip to content

fix(codex): handle remote request_user_input approvals #382

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.

Sign up for GitHub

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

Open
lwc-alex wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(codex): handle remote request_user_input approvals #382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions cli/src/codex/codexRemoteLauncher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()
});
Expand Down Expand Up @@ -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'
Copy link
Copy Markdown

@github-actions github-actions bot Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] Explicit deny is being collapsed to cancel here. handleUserInputRequest() rejects every non-accept outcome, and this catch hard-codes { decision: 'cancel' }, so Telegram or any other denyPermission() caller never returns decline to Codex. The same path is then finalized as canceled instead of denied in cli/src/codex/utils/permissionHandler.ts:168, which changes the behavior of real deny actions rather than only fixing the hang.

Suggested fix:

type UserInputApproval =
    | { decision: 'accept'; answers: RequestUserInputAnswers }
    | { decision: 'decline' | 'cancel'; reason?: string }

const result = await permissionHandler.handleUserInputRequest(id, input)
return result.decision === 'accept'
    ? result
    : { decision: result.decision }

};
}
}
});

appServerClient.setNotificationHandler((method, params) => {
Expand Down
80 changes: 80 additions & 0 deletions cli/src/codex/utils/appServerPermissionAdapter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'
});
});
});
23 changes: 17 additions & 6 deletions cli/src/codex/utils/appServerPermissionAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
});
}
39 changes: 39 additions & 0 deletions cli/src/codex/utils/permissionHandler.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
});
});
});
74 changes: 67 additions & 7 deletions cli/src/codex/utils/permissionHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
};

Expand All @@ -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 });
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
});
}

Expand All @@ -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
Expand All @@ -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 {
Expand Down
Loading