From b510d2ca15b33be21cb18b18b9e969f0d8d36d5b Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 7 Jul 2026 13:43:40 +0200 Subject: [PATCH 1/6] Support ACP form elicitation capability --- src/CodexAcpClient.ts | 7 +- src/CodexAcpServer.ts | 10 +- src/CodexAppServerClient.ts | 20 + src/CodexElicitationHandler.ts | 377 +++++++++++++++++- src/ElicitationCapabilities.ts | 28 ++ .../CodexACPAgent/elicitation-events.test.ts | 296 +++++++++++++- .../CodexACPAgent/initialize.test.ts | 33 ++ src/__tests__/acp-test-utils.ts | 15 +- 8 files changed, 765 insertions(+), 21 deletions(-) create mode 100644 src/ElicitationCapabilities.ts diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 2d5dbd64..fccddc91 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -22,6 +22,7 @@ import {AgentMode} from "./AgentMode"; import path from "node:path"; import {logger} from "./Logger"; import {sanitizeMcpServerName} from "./McpServerName"; +import {createAppServerInitializeCapabilities} from "./ElicitationCapabilities"; import type { AccountLoginCompletedNotification, AccountUpdatedNotification, @@ -69,7 +70,7 @@ export class CodexAcpClient { async initialize(request: acp.InitializeRequest): Promise { await this.codexClient.initialize({ - capabilities: null, + capabilities: createAppServerInitializeCapabilities(request.clientCapabilities), clientInfo: { name: request.clientInfo?.name ?? this.defaultClientInfo.name, version: request.clientInfo?.version ?? this.defaultClientInfo.version, @@ -534,6 +535,10 @@ export class CodexAcpClient { await this.waitForSessionNotifications(sessionId); return await elicitationHandler.handleElicitation(params); }, + handleUserInput: async (params) => { + await this.waitForSessionNotifications(sessionId); + return await elicitationHandler.handleUserInput(params); + }, }); } diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 6454ca86..58725426 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -144,6 +144,7 @@ export class CodexAcpServer { private readonly getRecentStderr: () => string; private readonly availableCommands: CodexCommands; private clientInfo: acp.Implementation | null; + private clientCapabilities: acp.ClientCapabilities | null; private terminalOutputMode: TerminalOutputMode; private booleanConfigOptionsSupported: boolean; @@ -175,6 +176,7 @@ export class CodexAcpServer { this.getExitCode = getExitCode ?? (() => null); this.getRecentStderr = getRecentStderr ?? (() => ""); this.clientInfo = null; + this.clientCapabilities = null; this.terminalOutputMode = "terminal_output_delta"; this.booleanConfigOptionsSupported = false; this.availableCommands = new CodexCommands( @@ -190,6 +192,7 @@ export class CodexAcpServer { ): Promise { logger.log("Initialize request received"); this.clientInfo = _params.clientInfo ?? null; + this.clientCapabilities = _params.clientCapabilities ?? null; this.terminalOutputMode = resolveTerminalOutputMode(_params.clientCapabilities); this.booleanConfigOptionsSupported = clientSupportsBooleanConfigOptions(_params.clientCapabilities); await this.runWithProcessCheck(() => this.codexAcpClient.initialize(_params)); @@ -1437,7 +1440,12 @@ export class CodexAcpServer { try { const eventHandler = new CodexEventHandler(this.connection, sessionState); const approvalHandler = new CodexApprovalHandler(this.connection, sessionState, activePrompt.signal); - const elicitationHandler = new CodexElicitationHandler(this.connection, sessionState, activePrompt.signal); + const elicitationHandler = new CodexElicitationHandler( + this.connection, + sessionState, + this.clientCapabilities, + activePrompt.signal, + ); await this.codexAcpClient.subscribeToSessionEvents(params.sessionId, (event) => { elicitationHandler.handleNotification(event); diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index d1bc210c..11d61a6e 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -51,6 +51,8 @@ import type { ThreadStartResponse, ThreadUnsubscribeParams, ThreadUnsubscribeResponse, + ToolRequestUserInputParams, + ToolRequestUserInputResponse, TurnCompletedNotification, TurnInterruptParams, TurnInterruptResponse, @@ -73,6 +75,7 @@ export interface ApprovalHandler { export interface ElicitationHandler { handleElicitation(params: McpServerElicitationRequestParams): Promise; + handleUserInput(params: ToolRequestUserInputParams): Promise; } export type McpStartupFailure = { @@ -110,6 +113,12 @@ const McpServerElicitationRequest = new RequestType< void >('mcpServer/elicitation/request'); +const ToolRequestUserInputRequest = new RequestType< + ToolRequestUserInputParams, + ToolRequestUserInputResponse, + void +>('item/tool/requestUserInput'); + const GOAL_RUNTIME_EFFECTS_GRACE_MS = 1_000; /** @@ -217,6 +226,17 @@ export class CodexAppServerClient { } return await handler.handleElicitation(params); }); + + this.connection.onRequest(ToolRequestUserInputRequest, async (params) => { + if (this.isStaleTurn(params.threadId, params.turnId)) { + return { answers: {} }; + } + const handler = this.elicitationHandlers.get(params.threadId); + if (!handler) { + return { answers: {} }; + } + return await handler.handleUserInput(params); + }); } onApprovalRequest(threadId: string, handler: ApprovalHandler): void { diff --git a/src/CodexElicitationHandler.ts b/src/CodexElicitationHandler.ts index 2a15df04..d24a68aa 100644 --- a/src/CodexElicitationHandler.ts +++ b/src/CodexElicitationHandler.ts @@ -2,15 +2,22 @@ import * as acp from "@agentclientprotocol/sdk"; import type { SessionState } from "./CodexAcpServer"; import type { ElicitationHandler } from "./CodexAppServerClient"; import type { ServerNotification } from "./app-server"; +import type {JsonValue} from "./app-server/serde_json/JsonValue"; import type { ItemCompletedNotification, ItemStartedNotification, McpServerElicitationRequestParams, McpServerElicitationRequestResponse, + ToolRequestUserInputParams, + ToolRequestUserInputResponse, } from "./app-server/v2"; import { logger } from "./Logger"; import { McpApprovalOptionId } from "./McpApprovalOptionId"; import type {AcpClientConnection} from "./ACPSessionConnection"; +import { + clientSupportsFormElicitation, + clientSupportsUrlElicitation, +} from "./ElicitationCapabilities"; // Standard elicitation options (non-tool-call approval). const ELICITATION_OPTIONS: acp.PermissionOption[] = [ @@ -19,6 +26,13 @@ const ELICITATION_OPTIONS: acp.PermissionOption[] = [ ]; type PersistValue = "session" | "always"; +type ToolApprovalPersistValue = PersistValue | "once"; + +type McpElicitationContext = { + isToolApproval: boolean; + persistOptions: Set; + correlatedCallId: string | undefined; +}; /** * Parses the `persist` field from the elicitation request `_meta`. @@ -48,6 +62,137 @@ function isMcpToolCallApproval(meta: unknown): boolean { ); } +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function normalizeJsonValue(value: unknown): JsonValue { + if (value === null || value === undefined) { + return null; + } + if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") { + return value; + } + if (typeof value === "bigint") { + return Number(value); + } + if (Array.isArray(value)) { + return value.map(normalizeJsonValue); + } + if (typeof value === "object") { + return Object.fromEntries( + Object.entries(value) + .filter(([, nested]) => nested !== undefined) + .map(([key, nested]) => [key, normalizeJsonValue(nested)]) + ); + } + return String(value); +} + +function normalizeElicitationSchema(value: unknown): acp.ElicitationSchema { + const normalized = normalizeElicitationSchemaValue(value); + if (!isRecord(normalized)) { + return { type: "object", properties: {} }; + } + + return { + ...normalized, + type: "object", + } as acp.ElicitationSchema; +} + +function normalizeElicitationSchemaValue(value: unknown): unknown { + if (typeof value === "bigint") { + return Number(value); + } + if (Array.isArray(value)) { + return value.map(normalizeElicitationSchemaValue); + } + if (!isRecord(value)) { + return value; + } + + const result: Record = Object.fromEntries( + Object.entries(value) + .filter(([, nested]) => nested !== undefined) + .map(([key, nested]) => [key, normalizeElicitationSchemaValue(nested)]) + ); + + if ( + result["type"] === "string" && + Array.isArray(result["enum"]) && + Array.isArray(result["enumNames"]) && + !Array.isArray(result["oneOf"]) + ) { + const values = result["enum"]; + const names = result["enumNames"]; + result["oneOf"] = values.map((value, index) => ({ + const: String(value), + title: String(names[index] ?? value), + })); + delete result["enum"]; + delete result["enumNames"]; + } + + return result; +} + +function metaRecord(meta: unknown): Record | null { + return isRecord(meta) ? meta : null; +} + +function persistChoiceOption(value: ToolApprovalPersistValue): acp.EnumOption { + switch (value) { + case "once": + return { const: "once", title: "Allow once" }; + case "session": + return { const: "session", title: "Allow for this session" }; + case "always": + return { const: "always", title: "Allow and don't ask again" }; + } +} + +function addPersistChoiceToSchema( + schema: acp.ElicitationSchema, + persistOptions: Set +): acp.ElicitationSchema { + if (persistOptions.size === 0) { + return schema; + } + + const choices: ToolApprovalPersistValue[] = ["once"]; + if (persistOptions.has("session")) choices.push("session"); + if (persistOptions.has("always")) choices.push("always"); + + return { + ...schema, + properties: { + ...schema.properties, + persist: { + type: "string", + title: "Approval scope", + oneOf: choices.map(persistChoiceOption), + default: "once", + }, + }, + required: Array.from(new Set([...(schema.required ?? []), "persist"])), + }; +} + +function contentRecord(content: unknown): Record { + return isRecord(content) ? content as Record : {}; +} + +function jsonObjectOrNull( + content: Record +): JsonValue | null { + const entries = Object.entries(content); + if (entries.length === 0) { + return null; + } + return Object.fromEntries(entries.map(([key, value]) => [key, normalizeJsonValue(value)])); +} + /** * Builds the ACP permission options for an MCP tool call approval elicitation. * Always includes "Allow Once"; adds session/always persist options when advertised. @@ -69,6 +214,7 @@ function buildToolApprovalOptions(persistOptions: Set): acp.Permis export class CodexElicitationHandler implements ElicitationHandler { private readonly connection: AcpClientConnection; private readonly sessionState: SessionState; + private readonly clientCapabilities: acp.ClientCapabilities | null; private readonly cancellationSignal: AbortSignal | undefined; // In Rust, the MCP elicitation handler receives ElicitationRequestEvent directly from the MCP // protocol layer, where id is set to "mcp_tool_call_approval_" — the call ID is extracted @@ -87,9 +233,15 @@ export class CodexElicitationHandler implements ElicitationHandler { // (threadId, serverName). private readonly pendingMcpApprovals = new Map(); - constructor(connection: AcpClientConnection, sessionState: SessionState, cancellationSignal?: AbortSignal) { + constructor( + connection: AcpClientConnection, + sessionState: SessionState, + clientCapabilities: acp.ClientCapabilities | null = null, + cancellationSignal?: AbortSignal + ) { this.connection = connection; this.sessionState = sessionState; + this.clientCapabilities = clientCapabilities; this.cancellationSignal = cancellationSignal; } @@ -113,7 +265,19 @@ export class CodexElicitationHandler implements ElicitationHandler { params: McpServerElicitationRequestParams ): Promise { try { - const { request, correlatedCallId } = this.buildPermissionRequest(params); + const context = this.createMcpElicitationContext(params); + if (this.shouldUseAcpElicitation(params)) { + const response = await this.connection.request( + acp.methods.client.elicitation.create, + this.buildElicitationRequest(params, context), + this.requestOptions(), + ); + const result = this.convertElicitationResponse(response, context); + await this.publishAcceptedMcpToolApproval(context, result.action === "accept"); + return result; + } + + const { request, correlatedCallId } = this.buildPermissionRequest(params, context); const response = await this.connection.request( acp.methods.client.session.requestPermission, request, @@ -128,19 +292,146 @@ export class CodexElicitationHandler implements ElicitationHandler { }); } } - return this.convertResponse(response); + return this.convertPermissionResponse(response); } catch (error) { logger.error("Error handling MCP elicitation request", error); return { action: "cancel", content: null, _meta: null }; } } + async handleUserInput(params: ToolRequestUserInputParams): Promise { + if (!clientSupportsFormElicitation(this.clientCapabilities)) { + return { answers: {} }; + } + + try { + const response = await this.connection.request( + acp.methods.client.elicitation.create, + this.buildUserInputRequest(params), + this.requestOptions(), + ); + return this.convertUserInputResponse(response); + } catch (error) { + logger.error("Error handling Codex user input request", error); + return { answers: {} }; + } + } + private requestOptions(): acp.SendRequestOptions | undefined { return this.cancellationSignal ? {cancellationSignal: this.cancellationSignal} : undefined; } + private createMcpElicitationContext(params: McpServerElicitationRequestParams): McpElicitationContext { + const isToolApproval = isMcpToolCallApproval(params._meta); + const persistOptions = parsePersistOptions(params._meta); + const correlatedCallId = isToolApproval && (params.mode === "form" || params.mode === "openai/form") + ? this.popPendingApproval(params.threadId, params.serverName) + : undefined; + return { isToolApproval, persistOptions, correlatedCallId }; + } + + private shouldUseAcpElicitation(params: McpServerElicitationRequestParams): boolean { + switch (params.mode) { + case "form": + case "openai/form": + return clientSupportsFormElicitation(this.clientCapabilities); + case "url": + return clientSupportsUrlElicitation(this.clientCapabilities); + } + } + + private buildElicitationRequest( + params: McpServerElicitationRequestParams, + context: McpElicitationContext + ): acp.CreateElicitationRequest { + const base = { + sessionId: this.sessionState.sessionId, + ...(context.correlatedCallId ? { toolCallId: context.correlatedCallId } : {}), + message: params.message, + _meta: metaRecord(params._meta), + }; + + switch (params.mode) { + case "form": + case "openai/form": { + const requestedSchema = context.isToolApproval + ? addPersistChoiceToSchema( + normalizeElicitationSchema(params.requestedSchema), + context.persistOptions, + ) + : normalizeElicitationSchema(params.requestedSchema); + return { + ...base, + mode: "form", + requestedSchema, + }; + } + case "url": + return { + ...base, + mode: "url", + url: params.url, + elicitationId: params.elicitationId, + }; + } + } + + private buildUserInputRequest(params: ToolRequestUserInputParams): acp.CreateElicitationRequest { + const properties: Record = {}; + const required: string[] = []; + + for (const question of params.questions) { + required.push(question.id); + const base = { + title: question.header || question.id, + description: question.question, + _meta: { + codex: { + isOther: question.isOther, + isSecret: question.isSecret, + }, + }, + }; + properties[question.id] = question.options && question.options.length > 0 + ? { + ...base, + type: "string", + oneOf: question.options.map(option => ({ + const: option.label, + title: option.label, + description: option.description, + })), + } + : { + ...base, + type: "string", + }; + } + + const firstQuestion = params.questions[0]; + return { + sessionId: this.sessionState.sessionId, + toolCallId: params.itemId, + mode: "form", + message: params.questions.length === 1 && firstQuestion + ? firstQuestion.question + : "Input requested", + requestedSchema: { + type: "object", + properties, + required, + }, + _meta: { + codex: { + autoResolutionMs: params.autoResolutionMs, + }, + }, + }; + } + private buildPermissionRequest( - params: McpServerElicitationRequestParams + params: McpServerElicitationRequestParams, + context: McpElicitationContext ): { request: acp.RequestPermissionRequest; correlatedCallId: string | undefined } { const sessionId = this.sessionState.sessionId; const messageContent: acp.ToolCallContent = { @@ -148,17 +439,12 @@ export class CodexElicitationHandler implements ElicitationHandler { content: { type: "text", text: params.message }, }; - const meta = params._meta; - const isToolApproval = isMcpToolCallApproval(meta); - const options = isToolApproval - ? buildToolApprovalOptions(parsePersistOptions(meta)) + const options = context.isToolApproval + ? buildToolApprovalOptions(context.persistOptions) : ELICITATION_OPTIONS; if (params.mode === "form" || params.mode === "openai/form") { - const correlatedCallId = isToolApproval - ? this.popPendingApproval(params.threadId, params.serverName) - : undefined; - if (correlatedCallId !== undefined) { + if (context.correlatedCallId !== undefined) { // The tool call item is already visible in the IDE conversation history because // item/started was emitted before the elicitation request. Sending content or // rawInput here would duplicate that information in the approval widget. @@ -166,7 +452,7 @@ export class CodexElicitationHandler implements ElicitationHandler { request: { sessionId, toolCall: { - toolCallId: correlatedCallId, + toolCallId: context.correlatedCallId, kind: "execute", status: "pending", // content: [messageContent], — omitted: already rendered via item/started @@ -175,7 +461,7 @@ export class CodexElicitationHandler implements ElicitationHandler { _meta: { is_mcp_tool_approval: true }, options, }, - correlatedCallId, + correlatedCallId: context.correlatedCallId, }; } return { @@ -183,12 +469,12 @@ export class CodexElicitationHandler implements ElicitationHandler { sessionId, toolCall: { toolCallId: `elicitation-${params.serverName}`, - kind: isToolApproval ? "execute" : "other", + kind: context.isToolApproval ? "execute" : "other", status: "pending", content: [messageContent], rawInput: { serverName: params.serverName, schema: params.requestedSchema }, }, - ...(isToolApproval ? { _meta: { is_mcp_tool_approval: true } } : {}), + ...(context.isToolApproval ? { _meta: { is_mcp_tool_approval: true } } : {}), options, }, correlatedCallId: undefined, @@ -211,7 +497,7 @@ export class CodexElicitationHandler implements ElicitationHandler { } } - private convertResponse( + private convertPermissionResponse( response: acp.RequestPermissionResponse ): McpServerElicitationRequestResponse { if (response.outcome.outcome === "cancelled") { @@ -231,6 +517,63 @@ export class CodexElicitationHandler implements ElicitationHandler { return { action: "decline", content: null, _meta: null }; } + private convertElicitationResponse( + response: acp.CreateElicitationResponse, + context: McpElicitationContext + ): McpServerElicitationRequestResponse { + switch (response.action) { + case "accept": { + const content = contentRecord(response.content); + const persist = context.isToolApproval ? content["persist"] : undefined; + if (persist === "session" || persist === "always" || persist === "once") { + delete content["persist"]; + } + return { + action: "accept", + content: jsonObjectOrNull(content), + _meta: persist === "session" || persist === "always" + ? { persist } + : null, + }; + } + case "decline": + return { action: "decline", content: null, _meta: null }; + case "cancel": + return { action: "cancel", content: null, _meta: null }; + default: + return { action: "cancel", content: null, _meta: null }; + } + } + + private convertUserInputResponse(response: acp.CreateElicitationResponse): ToolRequestUserInputResponse { + if (response.action !== "accept") { + return { answers: {} }; + } + + const answers: ToolRequestUserInputResponse["answers"] = {}; + for (const [id, value] of Object.entries(contentRecord(response.content))) { + answers[id] = { + answers: Array.isArray(value) + ? value.map(String) + : [String(value)], + }; + } + return { answers }; + } + + private async publishAcceptedMcpToolApproval( + context: McpElicitationContext, + accepted: boolean + ): Promise { + if (!accepted || context.correlatedCallId === undefined) { + return; + } + await this.connection.notify(acp.methods.client.session.update, { + sessionId: this.sessionState.sessionId, + update: { sessionUpdate: "tool_call_update", toolCallId: context.correlatedCallId, status: "in_progress" }, + }); + } + private handleItemStarted(event: ItemStartedNotification): void { if (event.item.type !== "mcpToolCall") { return; diff --git a/src/ElicitationCapabilities.ts b/src/ElicitationCapabilities.ts new file mode 100644 index 00000000..63efc665 --- /dev/null +++ b/src/ElicitationCapabilities.ts @@ -0,0 +1,28 @@ +import type * as acp from "@agentclientprotocol/sdk"; +import type {InitializeCapabilities} from "./app-server"; + +export function clientSupportsFormElicitation( + clientCapabilities?: acp.ClientCapabilities | null +): boolean { + return clientCapabilities?.elicitation?.form != null; +} + +export function clientSupportsUrlElicitation( + clientCapabilities?: acp.ClientCapabilities | null +): boolean { + return clientCapabilities?.elicitation?.url != null; +} + +export function createAppServerInitializeCapabilities( + clientCapabilities?: acp.ClientCapabilities | null +): InitializeCapabilities | null { + if (!clientSupportsFormElicitation(clientCapabilities)) { + return null; + } + + return { + experimentalApi: true, + requestAttestation: false, + mcpServerOpenaiFormElicitation: true, + }; +} diff --git a/src/__tests__/CodexACPAgent/elicitation-events.test.ts b/src/__tests__/CodexACPAgent/elicitation-events.test.ts index e796d8ba..e912c3d5 100644 --- a/src/__tests__/CodexACPAgent/elicitation-events.test.ts +++ b/src/__tests__/CodexACPAgent/elicitation-events.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import type { McpServerElicitationRequestParams } from '../../app-server/v2'; +import * as acp from "@agentclientprotocol/sdk"; +import type { McpServerElicitationRequestParams, ToolRequestUserInputParams } from '../../app-server/v2'; import { createCodexMockTestFixture, createTestSessionState, type CodexMockTestFixture } from '../acp-test-utils'; import type { SessionState } from '../../CodexAcpServer'; import { AgentMode } from "../../AgentMode"; @@ -49,7 +50,93 @@ describe('Elicitation Events', () => { }; } + async function setupSessionWithPendingPromptAndCapabilities(clientCapabilities: acp.ClientCapabilities) { + await fixture.getCodexAcpAgent().initialize({ + protocolVersion: acp.PROTOCOL_VERSION, + clientCapabilities, + }); + return setupSessionWithPendingPrompt(); + } + describe('Form mode elicitation', () => { + it('should use ACP form elicitation when the client supports it', async () => { + const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ + elicitation: { form: {} }, + }); + fixture.setElicitationResponse({ + action: 'accept', + content: { username: 'octocat' }, + }); + + const params: McpServerElicitationRequestParams = { + threadId: sessionId, turnId: 'turn-1', serverName: 'test-server', + mode: 'form', _meta: null, message: 'Please provide your username', + requestedSchema: { type: 'object', properties: { username: { type: 'string' } }, required: ['username'] }, + }; + + const response = await fixture.sendServerRequest('mcpServer/elicitation/request', params); + expect(response).toEqual({ action: 'accept', content: { username: 'octocat' }, _meta: null }); + + const [elicitationEvent] = fixture.getAcpConnectionEvents([]); + expect(elicitationEvent).toEqual({ + method: 'createElicitation', + args: [{ + sessionId, + mode: 'form', + message: 'Please provide your username', + requestedSchema: { + type: 'object', + properties: { username: { type: 'string' } }, + required: ['username'], + }, + _meta: null, + }], + }); + + completeTurn(); + await promptPromise; + }); + + it('should normalize legacy enumNames schemas for ACP form elicitation', async () => { + const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ + elicitation: { form: {} }, + }); + fixture.setElicitationResponse({ + action: 'accept', + content: { color: 'blue' }, + }); + + const params: McpServerElicitationRequestParams = { + threadId: sessionId, turnId: 'turn-1', serverName: 'test-server', + mode: 'form', _meta: null, message: 'Pick a color', + requestedSchema: { + type: 'object', + properties: { + color: { + type: 'string', + enum: ['red', 'blue'], + enumNames: ['Red', 'Blue'], + }, + }, + required: ['color'], + }, + }; + + await fixture.sendServerRequest('mcpServer/elicitation/request', params); + + const [elicitationEvent] = fixture.getAcpConnectionEvents([]); + expect(elicitationEvent?.args[0].requestedSchema.properties.color).toEqual({ + type: 'string', + oneOf: [ + { const: 'red', title: 'Red' }, + { const: 'blue', title: 'Blue' }, + ], + }); + + completeTurn(); + await promptPromise; + }); + it('should map accept to accept', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'accept' } }); @@ -131,6 +218,77 @@ describe('Elicitation Events', () => { }); describe('MCP tool call approval elicitation', () => { + it('should use ACP form elicitation for MCP tool approval when supported', async () => { + const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ + elicitation: { form: {} }, + }); + fixture.setElicitationResponse({ + action: 'accept', + content: { persist: 'always' }, + }); + + fixture.sendServerNotification({ + method: 'item/started', + params: { + threadId: sessionId, + turnId: 'turn-1', + startedAtMs: 0, + item: { + type: "mcpToolCall", + id: "call-id", + server: "tool-server", + tool: "tool-name", + status: "inProgress", + arguments: { argument: "example" }, + appContext: null, + pluginId: null, + result: null, + error: null, + durationMs: null, + }, + }, + }); + await fixture.getCodexAcpClient().waitForSessionNotifications(sessionId); + fixture.clearAcpConnectionDump(); + + const params: McpServerElicitationRequestParams = { + threadId: sessionId, turnId: 'turn-1', serverName: 'tool-server', + mode: 'form', + _meta: { codex_approval_kind: 'mcp_tool_call', persist: ['session', 'always'] }, + message: 'Allow tool call?', + requestedSchema: { type: 'object', properties: {} }, + }; + + const response = await fixture.sendServerRequest('mcpServer/elicitation/request', params); + expect(response).toEqual({ action: 'accept', content: null, _meta: { persist: 'always' } }); + + const events = fixture.getAcpConnectionEvents(['_meta']); + expect(events[0]).toMatchObject({ + method: 'createElicitation', + args: [{ + sessionId, + toolCallId: 'call-id', + mode: 'form', + message: 'Allow tool call?', + }], + }); + expect(events[0]!.args[0].requestedSchema.properties.persist.oneOf).toEqual([ + { const: 'once', title: 'Allow once' }, + { const: 'session', title: 'Allow for this session' }, + { const: 'always', title: "Allow and don't ask again" }, + ]); + expect(events[1]).toEqual({ + method: 'sessionUpdate', + args: [{ + sessionId, + update: { sessionUpdate: 'tool_call_update', toolCallId: 'call-id', status: 'in_progress' }, + }], + }); + + completeTurn(); + await promptPromise; + }); + it('should show Allow/session/always/Decline options when all persist values advertised', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: McpApprovalOptionId.AllowOnce } }); @@ -373,6 +531,38 @@ describe('Elicitation Events', () => { }); describe('URL mode elicitation', () => { + it('should use ACP URL elicitation when the client supports it', async () => { + const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ + elicitation: { url: {} }, + }); + fixture.setElicitationResponse({ action: 'accept' }); + + const params: McpServerElicitationRequestParams = { + threadId: sessionId, turnId: 'turn-1', serverName: 'auth-server', + mode: 'url', _meta: null, message: 'Please authorize access', + url: 'https://example.com/authorize', elicitationId: 'elicit-123', + }; + + const response = await fixture.sendServerRequest('mcpServer/elicitation/request', params); + expect(response).toEqual({ action: 'accept', content: null, _meta: null }); + + const [elicitationEvent] = fixture.getAcpConnectionEvents([]); + expect(elicitationEvent).toEqual({ + method: 'createElicitation', + args: [{ + sessionId, + mode: 'url', + message: 'Please authorize access', + url: 'https://example.com/authorize', + elicitationId: 'elicit-123', + _meta: null, + }], + }); + + completeTurn(); + await promptPromise; + }); + it('should map accept to accept for URL mode', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'accept' } }); @@ -426,4 +616,108 @@ describe('Elicitation Events', () => { await promptPromise; }); }); + + describe('Codex request_user_input', () => { + it('should use ACP form elicitation for request_user_input when supported', async () => { + const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ + elicitation: { form: {} }, + }); + fixture.setElicitationResponse({ + action: 'accept', + content: { + next_step: 'Run tests', + notes: 'Focus auth', + }, + }); + + const params: ToolRequestUserInputParams = { + threadId: sessionId, + turnId: 'turn-1', + itemId: 'request-user-input-1', + autoResolutionMs: 60000, + questions: [ + { + id: 'next_step', + header: 'Next step', + question: 'What should I do next?', + isOther: true, + isSecret: false, + options: [ + { label: 'Run tests', description: 'Run the focused test suite.' }, + { label: 'Stop', description: 'Stop and report current status.' }, + ], + }, + { + id: 'notes', + header: 'Notes', + question: 'Any extra instructions?', + isOther: false, + isSecret: false, + options: null, + }, + ], + }; + + const response = await fixture.sendServerRequest('item/tool/requestUserInput', params); + expect(response).toEqual({ + answers: { + next_step: { answers: ['Run tests'] }, + notes: { answers: ['Focus auth'] }, + }, + }); + + const [elicitationEvent] = fixture.getAcpConnectionEvents(['_meta']); + expect(elicitationEvent).toMatchObject({ + method: 'createElicitation', + args: [{ + sessionId, + toolCallId: 'request-user-input-1', + mode: 'form', + message: 'Input requested', + requestedSchema: { + type: 'object', + required: ['next_step', 'notes'], + }, + }], + }); + expect(elicitationEvent!.args[0].requestedSchema.properties.next_step.oneOf).toEqual([ + { const: 'Run tests', title: 'Run tests', description: 'Run the focused test suite.' }, + { const: 'Stop', title: 'Stop', description: 'Stop and report current status.' }, + ]); + expect(elicitationEvent!.args[0].requestedSchema.properties.notes).toMatchObject({ + type: 'string', + title: 'Notes', + description: 'Any extra instructions?', + }); + + completeTurn(); + await promptPromise; + }); + + it('should not call ACP elicitation for request_user_input without form support', async () => { + const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); + + const params: ToolRequestUserInputParams = { + threadId: sessionId, + turnId: 'turn-1', + itemId: 'request-user-input-1', + autoResolutionMs: null, + questions: [{ + id: 'next_step', + header: 'Next step', + question: 'What should I do next?', + isOther: false, + isSecret: false, + options: null, + }], + }; + + const response = await fixture.sendServerRequest('item/tool/requestUserInput', params); + expect(response).toEqual({ answers: {} }); + expect(fixture.getAcpConnectionEvents(['_meta'])).toEqual([]); + + completeTurn(); + await promptPromise; + }); + }); }); diff --git a/src/__tests__/CodexACPAgent/initialize.test.ts b/src/__tests__/CodexACPAgent/initialize.test.ts index e7050101..ee03ea34 100644 --- a/src/__tests__/CodexACPAgent/initialize.test.ts +++ b/src/__tests__/CodexACPAgent/initialize.test.ts @@ -84,6 +84,39 @@ describe('CodexACPAgent - initialize', () => { ])); }); + it('should opt into app-server form elicitation only when the client supports ACP form elicitation', async () => { + await agent.initialize({ + protocolVersion: acp.PROTOCOL_VERSION, + clientCapabilities: { + elicitation: { + form: {}, + }, + }, + }); + + expect(mockCodexConnection.sendRequest).toHaveBeenCalledWith("initialize", expect.objectContaining({ + capabilities: { + experimentalApi: true, + requestAttestation: false, + mcpServerOpenaiFormElicitation: true, + }, + })); + + vi.clearAllMocks(); + await agent.initialize({ + protocolVersion: acp.PROTOCOL_VERSION, + clientCapabilities: { + elicitation: { + url: {}, + }, + }, + }); + + expect(mockCodexConnection.sendRequest).toHaveBeenCalledWith("initialize", expect.objectContaining({ + capabilities: null, + })); + }); + it('should advertise API key auth with the legacy metadata method', () => { expect(getCodexAuthMethods()).toEqual(expect.arrayContaining([ expect.objectContaining({ diff --git a/src/__tests__/acp-test-utils.ts b/src/__tests__/acp-test-utils.ts index 2f50d49d..cddcbf53 100644 --- a/src/__tests__/acp-test-utils.ts +++ b/src/__tests__/acp-test-utils.ts @@ -1,5 +1,5 @@ import * as acp from "@agentclientprotocol/sdk"; -import type {McpServerStdio, RequestPermissionResponse} from "@agentclientprotocol/sdk"; +import type {CreateElicitationResponse, McpServerStdio, RequestPermissionResponse} from "@agentclientprotocol/sdk"; import {CodexAcpClient} from '../CodexAcpClient'; import {CodexAppServerClient, type CodexConnectionEvent} from '../CodexAppServerClient'; import {startCodexConnection} from "../CodexJsonRpcConnection"; @@ -42,6 +42,9 @@ function normalizeAcpConnectionEvent(event: MethodCallEvent): MethodCallEvent { if (event.method === "request" && event.args[0] === acp.methods.client.session.requestPermission) { return {method: "requestPermission", args: [event.args[1]]}; } + if (event.method === "request" && event.args[0] === acp.methods.client.elicitation.create) { + return {method: "createElicitation", args: [event.args[1]]}; + } if (event.method === "notify" && event.args[0] === acp.methods.client.session.update) { return {method: "sessionUpdate", args: [event.args[1]]}; } @@ -238,6 +241,7 @@ export interface CodexMockTestFixture extends TestFixture { sendServerNotification(notification: ServerNotification | Record): void, sendServerRequest(method: string, params: unknown): Promise, setPermissionResponse(response: RequestPermissionResponse): void, + setElicitationResponse(response: CreateElicitationResponse): void, } /** @@ -255,6 +259,9 @@ export function createCodexMockTestFixture(): CodexMockTestFixture { const permissionState: { response: RequestPermissionResponse } = { response: { outcome: { outcome: 'cancelled' } } }; + const elicitationState: { response: CreateElicitationResponse } = { + response: { action: 'cancel' } + }; const mockCodexConnection = { sendRequest: () => Promise.resolve(undefined), @@ -276,6 +283,9 @@ export function createCodexMockTestFixture(): CodexMockTestFixture { if (args[0] === acp.methods.client.session.requestPermission) { return permissionState.response; } + if (args[0] === acp.methods.client.elicitation.create) { + return elicitationState.response; + } return { mock: "Mocked return" }; }); returnValues.set('requestPermission', () => permissionState.response); @@ -313,6 +323,9 @@ export function createCodexMockTestFixture(): CodexMockTestFixture { setPermissionResponse(response: RequestPermissionResponse): void { permissionState.response = response; }, + setElicitationResponse(response: CreateElicitationResponse): void { + elicitationState.response = response; + }, }; } From 8e6a6f9b7b431f8735ca7cec744a3dc27fb8b065 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 7 Jul 2026 13:50:58 +0200 Subject: [PATCH 2/6] Auto-resolve stale user input elicitations --- src/CodexElicitationHandler.ts | 94 ++++++++++++++++--- .../CodexACPAgent/elicitation-events.test.ts | 48 +++++++++- src/__tests__/acp-test-utils.ts | 6 +- 3 files changed, 129 insertions(+), 19 deletions(-) diff --git a/src/CodexElicitationHandler.ts b/src/CodexElicitationHandler.ts index d24a68aa..815a9010 100644 --- a/src/CodexElicitationHandler.ts +++ b/src/CodexElicitationHandler.ts @@ -89,6 +89,14 @@ function normalizeJsonValue(value: unknown): JsonValue { return String(value); } +function normalizeJsonObject(value: Record): Record { + return Object.fromEntries( + Object.entries(value) + .filter(([, nested]) => nested !== undefined) + .map(([key, nested]) => [key, normalizeJsonValue(nested)]) + ); +} + function normalizeElicitationSchema(value: unknown): acp.ElicitationSchema { const normalized = normalizeElicitationSchemaValue(value); if (!isRecord(normalized)) { @@ -193,6 +201,22 @@ function jsonObjectOrNull( return Object.fromEntries(entries.map(([key, value]) => [key, normalizeJsonValue(value)])); } +function elicitationResponseMeta( + response: acp.CreateElicitationResponse, + context: McpElicitationContext, + persist: unknown = undefined +): JsonValue | null { + const responseMeta = metaRecord(response._meta); + const meta = responseMeta ? normalizeJsonObject(responseMeta) : {}; + if (context.isToolApproval) { + delete meta["persist"]; + } + if (persist === "session" || persist === "always") { + meta["persist"] = persist; + } + return Object.keys(meta).length === 0 ? null : meta; +} + /** * Builds the ACP permission options for an MCP tool call approval elicitation. * Always includes "Allow Once"; adds session/always persist options when advertised. @@ -305,11 +329,10 @@ export class CodexElicitationHandler implements ElicitationHandler { } try { - const response = await this.connection.request( - acp.methods.client.elicitation.create, - this.buildUserInputRequest(params), - this.requestOptions(), - ); + const response = await this.requestUserInputElicitation(params); + if (response === null) { + return { answers: {} }; + } return this.convertUserInputResponse(response); } catch (error) { logger.error("Error handling Codex user input request", error); @@ -317,8 +340,57 @@ export class CodexElicitationHandler implements ElicitationHandler { } } - private requestOptions(): acp.SendRequestOptions | undefined { - return this.cancellationSignal ? {cancellationSignal: this.cancellationSignal} : undefined; + private requestOptions(cancellationSignal: AbortSignal | undefined = this.cancellationSignal): acp.SendRequestOptions | undefined { + return cancellationSignal ? {cancellationSignal} : undefined; + } + + private async requestUserInputElicitation( + params: ToolRequestUserInputParams + ): Promise { + const request = this.buildUserInputRequest(params); + if (params.autoResolutionMs === null) { + return await this.connection.request( + acp.methods.client.elicitation.create, + request, + this.requestOptions(), + ); + } + + const abortController = new AbortController(); + let timeout: ReturnType | undefined; + let removeAbortListener: (() => void) | undefined; + const timeoutPromise = new Promise((resolve) => { + const resolveWithoutInput = () => { + abortController.abort(); + resolve(null); + }; + timeout = setTimeout(resolveWithoutInput, Math.max(0, params.autoResolutionMs ?? 0)); + if (this.cancellationSignal?.aborted) { + resolveWithoutInput(); + return; + } + if (this.cancellationSignal) { + this.cancellationSignal.addEventListener("abort", resolveWithoutInput, { once: true }); + removeAbortListener = () => { + this.cancellationSignal?.removeEventListener("abort", resolveWithoutInput); + }; + } + }); + const requestPromise = Promise.resolve(this.connection.request( + acp.methods.client.elicitation.create, + request, + this.requestOptions(abortController.signal), + )); + void requestPromise.catch(() => {}); + + try { + return await Promise.race([requestPromise, timeoutPromise]); + } finally { + if (timeout) { + clearTimeout(timeout); + } + removeAbortListener?.(); + } } private createMcpElicitationContext(params: McpServerElicitationRequestParams): McpElicitationContext { @@ -531,15 +603,13 @@ export class CodexElicitationHandler implements ElicitationHandler { return { action: "accept", content: jsonObjectOrNull(content), - _meta: persist === "session" || persist === "always" - ? { persist } - : null, + _meta: elicitationResponseMeta(response, context, persist), }; } case "decline": - return { action: "decline", content: null, _meta: null }; + return { action: "decline", content: null, _meta: elicitationResponseMeta(response, context) }; case "cancel": - return { action: "cancel", content: null, _meta: null }; + return { action: "cancel", content: null, _meta: elicitationResponseMeta(response, context) }; default: return { action: "cancel", content: null, _meta: null }; } diff --git a/src/__tests__/CodexACPAgent/elicitation-events.test.ts b/src/__tests__/CodexACPAgent/elicitation-events.test.ts index e912c3d5..6806cdfb 100644 --- a/src/__tests__/CodexACPAgent/elicitation-events.test.ts +++ b/src/__tests__/CodexACPAgent/elicitation-events.test.ts @@ -66,6 +66,7 @@ describe('Elicitation Events', () => { fixture.setElicitationResponse({ action: 'accept', content: { username: 'octocat' }, + _meta: { source: 'client' }, }); const params: McpServerElicitationRequestParams = { @@ -75,7 +76,7 @@ describe('Elicitation Events', () => { }; const response = await fixture.sendServerRequest('mcpServer/elicitation/request', params); - expect(response).toEqual({ action: 'accept', content: { username: 'octocat' }, _meta: null }); + expect(response).toEqual({ action: 'accept', content: { username: 'octocat' }, _meta: { source: 'client' } }); const [elicitationEvent] = fixture.getAcpConnectionEvents([]); expect(elicitationEvent).toEqual({ @@ -225,6 +226,7 @@ describe('Elicitation Events', () => { fixture.setElicitationResponse({ action: 'accept', content: { persist: 'always' }, + _meta: { source: 'client' }, }); fixture.sendServerNotification({ @@ -260,7 +262,7 @@ describe('Elicitation Events', () => { }; const response = await fixture.sendServerRequest('mcpServer/elicitation/request', params); - expect(response).toEqual({ action: 'accept', content: null, _meta: { persist: 'always' } }); + expect(response).toEqual({ action: 'accept', content: null, _meta: { source: 'client', persist: 'always' } }); const events = fixture.getAcpConnectionEvents(['_meta']); expect(events[0]).toMatchObject({ @@ -535,7 +537,7 @@ describe('Elicitation Events', () => { const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ elicitation: { url: {} }, }); - fixture.setElicitationResponse({ action: 'accept' }); + fixture.setElicitationResponse({ action: 'accept', _meta: { source: 'client' } }); const params: McpServerElicitationRequestParams = { threadId: sessionId, turnId: 'turn-1', serverName: 'auth-server', @@ -544,7 +546,7 @@ describe('Elicitation Events', () => { }; const response = await fixture.sendServerRequest('mcpServer/elicitation/request', params); - expect(response).toEqual({ action: 'accept', content: null, _meta: null }); + expect(response).toEqual({ action: 'accept', content: null, _meta: { source: 'client' } }); const [elicitationEvent] = fixture.getAcpConnectionEvents([]); expect(elicitationEvent).toEqual({ @@ -694,6 +696,44 @@ describe('Elicitation Events', () => { await promptPromise; }); + it('should auto-resolve request_user_input when the client does not answer in time', async () => { + const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ + elicitation: { form: {} }, + }); + fixture.setElicitationResponse(new Promise(() => {})); + + const params: ToolRequestUserInputParams = { + threadId: sessionId, + turnId: 'turn-1', + itemId: 'request-user-input-1', + autoResolutionMs: 1, + questions: [{ + id: 'next_step', + header: 'Next step', + question: 'What should I do next?', + isOther: false, + isSecret: false, + options: null, + }], + }; + + const response = await fixture.sendServerRequest('item/tool/requestUserInput', params); + expect(response).toEqual({ answers: {} }); + + const [elicitationEvent] = fixture.getAcpConnectionEvents(['_meta']); + expect(elicitationEvent).toMatchObject({ + method: 'createElicitation', + args: [{ + sessionId, + toolCallId: 'request-user-input-1', + mode: 'form', + }], + }); + + completeTurn(); + await promptPromise; + }); + it('should not call ACP elicitation for request_user_input without form support', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); diff --git a/src/__tests__/acp-test-utils.ts b/src/__tests__/acp-test-utils.ts index cddcbf53..660f3c96 100644 --- a/src/__tests__/acp-test-utils.ts +++ b/src/__tests__/acp-test-utils.ts @@ -241,7 +241,7 @@ export interface CodexMockTestFixture extends TestFixture { sendServerNotification(notification: ServerNotification | Record): void, sendServerRequest(method: string, params: unknown): Promise, setPermissionResponse(response: RequestPermissionResponse): void, - setElicitationResponse(response: CreateElicitationResponse): void, + setElicitationResponse(response: CreateElicitationResponse | Promise): void, } /** @@ -259,7 +259,7 @@ export function createCodexMockTestFixture(): CodexMockTestFixture { const permissionState: { response: RequestPermissionResponse } = { response: { outcome: { outcome: 'cancelled' } } }; - const elicitationState: { response: CreateElicitationResponse } = { + const elicitationState: { response: CreateElicitationResponse | Promise } = { response: { action: 'cancel' } }; @@ -323,7 +323,7 @@ export function createCodexMockTestFixture(): CodexMockTestFixture { setPermissionResponse(response: RequestPermissionResponse): void { permissionState.response = response; }, - setElicitationResponse(response: CreateElicitationResponse): void { + setElicitationResponse(response: CreateElicitationResponse | Promise): void { elicitationState.response = response; }, }; From 57048fe72b803fafb76972e5c086f682e0db715d Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 7 Jul 2026 14:06:26 +0200 Subject: [PATCH 3/6] Support Other answers in user input elicitations --- src/CodexElicitationHandler.ts | 75 +++++++++++++++++-- .../CodexACPAgent/elicitation-events.test.ts | 47 +++++++++++- 2 files changed, 114 insertions(+), 8 deletions(-) diff --git a/src/CodexElicitationHandler.ts b/src/CodexElicitationHandler.ts index 815a9010..82fa7231 100644 --- a/src/CodexElicitationHandler.ts +++ b/src/CodexElicitationHandler.ts @@ -34,6 +34,8 @@ type McpElicitationContext = { correlatedCallId: string | undefined; }; +const USER_INPUT_OTHER_FIELD_SUFFIX = "__other"; + /** * Parses the `persist` field from the elicitation request `_meta`. * Codex advertises which persistence options the client should show. @@ -217,6 +219,33 @@ function elicitationResponseMeta( return Object.keys(meta).length === 0 ? null : meta; } +function userInputOtherFieldId(questionId: string, questionIds: Set): string { + const base = `${questionId}${USER_INPUT_OTHER_FIELD_SUFFIX}`; + if (!questionIds.has(base)) { + return base; + } + + let index = 1; + while (questionIds.has(`${base}${index}`)) { + index += 1; + } + return `${base}${index}`; +} + +function userInputResponseValue( + content: Record, + fieldId: string +): acp.ElicitationContentValue | undefined { + const value = content[fieldId]; + if (typeof value === "string" && value.trim() === "") { + return undefined; + } + if (Array.isArray(value) && value.length === 0) { + return undefined; + } + return value; +} + /** * Builds the ACP permission options for an MCP tool call approval elicitation. * Always includes "Allow Once"; adds session/always persist options when advertised. @@ -333,7 +362,7 @@ export class CodexElicitationHandler implements ElicitationHandler { if (response === null) { return { answers: {} }; } - return this.convertUserInputResponse(response); + return this.convertUserInputResponse(response, params); } catch (error) { logger.error("Error handling Codex user input request", error); return { answers: {} }; @@ -451,9 +480,12 @@ export class CodexElicitationHandler implements ElicitationHandler { private buildUserInputRequest(params: ToolRequestUserInputParams): acp.CreateElicitationRequest { const properties: Record = {}; const required: string[] = []; + const questionIds = new Set(params.questions.map(question => question.id)); for (const question of params.questions) { - required.push(question.id); + const options = question.options ?? []; + const hasOptions = options.length > 0; + const hasOtherAnswer = question.isOther && hasOptions; const base = { title: question.header || question.id, description: question.question, @@ -464,11 +496,14 @@ export class CodexElicitationHandler implements ElicitationHandler { }, }, }; - properties[question.id] = question.options && question.options.length > 0 + if (!hasOtherAnswer) { + required.push(question.id); + } + properties[question.id] = hasOptions ? { ...base, type: "string", - oneOf: question.options.map(option => ({ + oneOf: options.map(option => ({ const: option.label, title: option.label, description: option.description, @@ -478,6 +513,20 @@ export class CodexElicitationHandler implements ElicitationHandler { ...base, type: "string", }; + if (hasOtherAnswer) { + properties[userInputOtherFieldId(question.id, questionIds)] = { + type: "string", + title: "Other", + description: "Type your own answer instead of choosing an option above.", + _meta: { + codex: { + questionId: question.id, + isOtherAnswer: true, + isSecret: question.isSecret, + }, + }, + }; + } } const firstQuestion = params.questions[0]; @@ -615,14 +664,26 @@ export class CodexElicitationHandler implements ElicitationHandler { } } - private convertUserInputResponse(response: acp.CreateElicitationResponse): ToolRequestUserInputResponse { + private convertUserInputResponse( + response: acp.CreateElicitationResponse, + params: ToolRequestUserInputParams + ): ToolRequestUserInputResponse { if (response.action !== "accept") { return { answers: {} }; } const answers: ToolRequestUserInputResponse["answers"] = {}; - for (const [id, value] of Object.entries(contentRecord(response.content))) { - answers[id] = { + const content = contentRecord(response.content); + const questionIds = new Set(params.questions.map(question => question.id)); + for (const question of params.questions) { + const value = question.isOther && question.options != null && question.options.length > 0 + ? userInputResponseValue(content, userInputOtherFieldId(question.id, questionIds)) + ?? userInputResponseValue(content, question.id) + : userInputResponseValue(content, question.id); + if (value === undefined) { + continue; + } + answers[question.id] = { answers: Array.isArray(value) ? value.map(String) : [String(value)], diff --git a/src/__tests__/CodexACPAgent/elicitation-events.test.ts b/src/__tests__/CodexACPAgent/elicitation-events.test.ts index 6806cdfb..783135ff 100644 --- a/src/__tests__/CodexACPAgent/elicitation-events.test.ts +++ b/src/__tests__/CodexACPAgent/elicitation-events.test.ts @@ -678,7 +678,7 @@ describe('Elicitation Events', () => { message: 'Input requested', requestedSchema: { type: 'object', - required: ['next_step', 'notes'], + required: ['notes'], }, }], }); @@ -686,6 +686,10 @@ describe('Elicitation Events', () => { { const: 'Run tests', title: 'Run tests', description: 'Run the focused test suite.' }, { const: 'Stop', title: 'Stop', description: 'Stop and report current status.' }, ]); + expect(elicitationEvent!.args[0].requestedSchema.properties.next_step__other).toMatchObject({ + type: 'string', + title: 'Other', + }); expect(elicitationEvent!.args[0].requestedSchema.properties.notes).toMatchObject({ type: 'string', title: 'Notes', @@ -696,6 +700,47 @@ describe('Elicitation Events', () => { await promptPromise; }); + it('should prefer free-form Other answers over fixed choices', async () => { + const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ + elicitation: { form: {} }, + }); + fixture.setElicitationResponse({ + action: 'accept', + content: { + next_step: 'Run tests', + next_step__other: 'Inspect flaky logs', + }, + }); + + const params: ToolRequestUserInputParams = { + threadId: sessionId, + turnId: 'turn-1', + itemId: 'request-user-input-1', + autoResolutionMs: null, + questions: [{ + id: 'next_step', + header: 'Next step', + question: 'What should I do next?', + isOther: true, + isSecret: false, + options: [ + { label: 'Run tests', description: 'Run the focused test suite.' }, + { label: 'Stop', description: 'Stop and report current status.' }, + ], + }], + }; + + const response = await fixture.sendServerRequest('item/tool/requestUserInput', params); + expect(response).toEqual({ + answers: { + next_step: { answers: ['Inspect flaky logs'] }, + }, + }); + + completeTurn(); + await promptPromise; + }); + it('should auto-resolve request_user_input when the client does not answer in time', async () => { const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ elicitation: { form: {} }, From 357cd53543c3486c750df5f53cda0a17191f78d8 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 7 Jul 2026 14:24:35 +0200 Subject: [PATCH 4/6] Complete accepted URL elicitations on resolution --- src/CodexAcpServer.ts | 4 +-- src/CodexElicitationHandler.ts | 31 ++++++++++++++++++- .../CodexACPAgent/elicitation-events.test.ts | 15 +++++++++ src/__tests__/acp-test-utils.ts | 3 ++ 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 58725426..2f33273a 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -1447,8 +1447,8 @@ export class CodexAcpServer { activePrompt.signal, ); await this.codexAcpClient.subscribeToSessionEvents(params.sessionId, - (event) => { - elicitationHandler.handleNotification(event); + async (event) => { + await elicitationHandler.handleNotification(event); return eventHandler.handleNotification(event); }, approvalHandler, diff --git a/src/CodexElicitationHandler.ts b/src/CodexElicitationHandler.ts index 82fa7231..6742ca2a 100644 --- a/src/CodexElicitationHandler.ts +++ b/src/CodexElicitationHandler.ts @@ -285,6 +285,9 @@ export class CodexElicitationHandler implements ElicitationHandler { // call's elicitation before starting the next, so there is at most one pending approval per // (threadId, serverName). private readonly pendingMcpApprovals = new Map(); + // The app-server handler exposes URL elicitationId, while serverRequest/resolved only exposes + // threadId here, so accepted URL elicitations are completed at thread scope. + private readonly pendingUrlElicitations = new Map>(); constructor( connection: AcpClientConnection, @@ -298,7 +301,7 @@ export class CodexElicitationHandler implements ElicitationHandler { this.cancellationSignal = cancellationSignal; } - handleNotification(notification: ServerNotification): void { + async handleNotification(notification: ServerNotification): Promise { switch (notification.method) { case "item/started": this.handleItemStarted(notification.params); @@ -308,6 +311,7 @@ export class CodexElicitationHandler implements ElicitationHandler { return; case "serverRequest/resolved": this.clearThread(notification.params.threadId); + await this.completeUrlElicitations(notification.params.threadId); return; default: return; @@ -326,6 +330,9 @@ export class CodexElicitationHandler implements ElicitationHandler { this.requestOptions(), ); const result = this.convertElicitationResponse(response, context); + if (params.mode === "url" && result.action === "accept") { + this.trackUrlElicitation(params.threadId, params.elicitationId); + } await this.publishAcceptedMcpToolApproval(context, result.action === "accept"); return result; } @@ -705,6 +712,28 @@ export class CodexElicitationHandler implements ElicitationHandler { }); } + private trackUrlElicitation(threadId: string, elicitationId: string): void { + const existing = this.pendingUrlElicitations.get(threadId); + if (existing) { + existing.add(elicitationId); + return; + } + this.pendingUrlElicitations.set(threadId, new Set([elicitationId])); + } + + private async completeUrlElicitations(threadId: string): Promise { + const elicitationIds = this.pendingUrlElicitations.get(threadId); + if (!elicitationIds) { + return; + } + this.pendingUrlElicitations.delete(threadId); + for (const elicitationId of elicitationIds) { + await this.connection.notify(acp.methods.client.elicitation.complete, { + elicitationId, + }); + } + } + private handleItemStarted(event: ItemStartedNotification): void { if (event.item.type !== "mcpToolCall") { return; diff --git a/src/__tests__/CodexACPAgent/elicitation-events.test.ts b/src/__tests__/CodexACPAgent/elicitation-events.test.ts index 783135ff..10e9485e 100644 --- a/src/__tests__/CodexACPAgent/elicitation-events.test.ts +++ b/src/__tests__/CodexACPAgent/elicitation-events.test.ts @@ -561,6 +561,21 @@ describe('Elicitation Events', () => { }], }); + fixture.sendServerNotification({ + method: 'serverRequest/resolved', + params: { + threadId: sessionId, + requestId: 'request-1', + }, + }); + await fixture.getCodexAcpClient().waitForSessionNotifications(sessionId); + + const [, completeElicitationEvent] = fixture.getAcpConnectionEvents([]); + expect(completeElicitationEvent).toEqual({ + method: 'completeElicitation', + args: [{ elicitationId: 'elicit-123' }], + }); + completeTurn(); await promptPromise; }); diff --git a/src/__tests__/acp-test-utils.ts b/src/__tests__/acp-test-utils.ts index 660f3c96..456701f0 100644 --- a/src/__tests__/acp-test-utils.ts +++ b/src/__tests__/acp-test-utils.ts @@ -45,6 +45,9 @@ function normalizeAcpConnectionEvent(event: MethodCallEvent): MethodCallEvent { if (event.method === "request" && event.args[0] === acp.methods.client.elicitation.create) { return {method: "createElicitation", args: [event.args[1]]}; } + if (event.method === "notify" && event.args[0] === acp.methods.client.elicitation.complete) { + return {method: "completeElicitation", args: [event.args[1]]}; + } if (event.method === "notify" && event.args[0] === acp.methods.client.session.update) { return {method: "sessionUpdate", args: [event.args[1]]}; } From e5a2a75a09f202531e90d2725dcd7785d5466f42 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 7 Jul 2026 16:02:41 +0200 Subject: [PATCH 5/6] Disable app-server elicitation opt-in --- src/ElicitationCapabilities.ts | 14 ++++------- .../CodexACPAgent/initialize.test.ts | 24 ++----------------- 2 files changed, 6 insertions(+), 32 deletions(-) diff --git a/src/ElicitationCapabilities.ts b/src/ElicitationCapabilities.ts index 63efc665..a90351b8 100644 --- a/src/ElicitationCapabilities.ts +++ b/src/ElicitationCapabilities.ts @@ -14,15 +14,9 @@ export function clientSupportsUrlElicitation( } export function createAppServerInitializeCapabilities( - clientCapabilities?: acp.ClientCapabilities | null + _clientCapabilities?: acp.ClientCapabilities | null ): InitializeCapabilities | null { - if (!clientSupportsFormElicitation(clientCapabilities)) { - return null; - } - - return { - experimentalApi: true, - requestAttestation: false, - mcpServerOpenaiFormElicitation: true, - }; + // Do not opt into app-server experimental APIs just because the ACP client supports elicitation. + // The handlers can stay in place, but this keeps request_user_input dormant until explicitly enabled. + return null; } diff --git a/src/__tests__/CodexACPAgent/initialize.test.ts b/src/__tests__/CodexACPAgent/initialize.test.ts index ee03ea34..0528629e 100644 --- a/src/__tests__/CodexACPAgent/initialize.test.ts +++ b/src/__tests__/CodexACPAgent/initialize.test.ts @@ -84,31 +84,11 @@ describe('CodexACPAgent - initialize', () => { ])); }); - it('should opt into app-server form elicitation only when the client supports ACP form elicitation', async () => { + it('should not opt into experimental app-server capabilities for ACP elicitation support', async () => { await agent.initialize({ protocolVersion: acp.PROTOCOL_VERSION, clientCapabilities: { - elicitation: { - form: {}, - }, - }, - }); - - expect(mockCodexConnection.sendRequest).toHaveBeenCalledWith("initialize", expect.objectContaining({ - capabilities: { - experimentalApi: true, - requestAttestation: false, - mcpServerOpenaiFormElicitation: true, - }, - })); - - vi.clearAllMocks(); - await agent.initialize({ - protocolVersion: acp.PROTOCOL_VERSION, - clientCapabilities: { - elicitation: { - url: {}, - }, + elicitation: { form: {}, url: {} }, }, }); From 57f1ca42a866434a60b9c539454d60746b80c780 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 7 Jul 2026 16:13:35 +0200 Subject: [PATCH 6/6] Remove request_user_input handling --- src/CodexAcpClient.ts | 7 +- src/CodexAppServerClient.ts | 20 -- src/CodexElicitationHandler.ts | 218 ++---------------- src/ElicitationCapabilities.ts | 9 - .../CodexACPAgent/elicitation-events.test.ts | 189 +-------------- 5 files changed, 15 insertions(+), 428 deletions(-) diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index fccddc91..2d5dbd64 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -22,7 +22,6 @@ import {AgentMode} from "./AgentMode"; import path from "node:path"; import {logger} from "./Logger"; import {sanitizeMcpServerName} from "./McpServerName"; -import {createAppServerInitializeCapabilities} from "./ElicitationCapabilities"; import type { AccountLoginCompletedNotification, AccountUpdatedNotification, @@ -70,7 +69,7 @@ export class CodexAcpClient { async initialize(request: acp.InitializeRequest): Promise { await this.codexClient.initialize({ - capabilities: createAppServerInitializeCapabilities(request.clientCapabilities), + capabilities: null, clientInfo: { name: request.clientInfo?.name ?? this.defaultClientInfo.name, version: request.clientInfo?.version ?? this.defaultClientInfo.version, @@ -535,10 +534,6 @@ export class CodexAcpClient { await this.waitForSessionNotifications(sessionId); return await elicitationHandler.handleElicitation(params); }, - handleUserInput: async (params) => { - await this.waitForSessionNotifications(sessionId); - return await elicitationHandler.handleUserInput(params); - }, }); } diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 11d61a6e..d1bc210c 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -51,8 +51,6 @@ import type { ThreadStartResponse, ThreadUnsubscribeParams, ThreadUnsubscribeResponse, - ToolRequestUserInputParams, - ToolRequestUserInputResponse, TurnCompletedNotification, TurnInterruptParams, TurnInterruptResponse, @@ -75,7 +73,6 @@ export interface ApprovalHandler { export interface ElicitationHandler { handleElicitation(params: McpServerElicitationRequestParams): Promise; - handleUserInput(params: ToolRequestUserInputParams): Promise; } export type McpStartupFailure = { @@ -113,12 +110,6 @@ const McpServerElicitationRequest = new RequestType< void >('mcpServer/elicitation/request'); -const ToolRequestUserInputRequest = new RequestType< - ToolRequestUserInputParams, - ToolRequestUserInputResponse, - void ->('item/tool/requestUserInput'); - const GOAL_RUNTIME_EFFECTS_GRACE_MS = 1_000; /** @@ -226,17 +217,6 @@ export class CodexAppServerClient { } return await handler.handleElicitation(params); }); - - this.connection.onRequest(ToolRequestUserInputRequest, async (params) => { - if (this.isStaleTurn(params.threadId, params.turnId)) { - return { answers: {} }; - } - const handler = this.elicitationHandlers.get(params.threadId); - if (!handler) { - return { answers: {} }; - } - return await handler.handleUserInput(params); - }); } onApprovalRequest(threadId: string, handler: ApprovalHandler): void { diff --git a/src/CodexElicitationHandler.ts b/src/CodexElicitationHandler.ts index 6742ca2a..3b4498e0 100644 --- a/src/CodexElicitationHandler.ts +++ b/src/CodexElicitationHandler.ts @@ -8,8 +8,6 @@ import type { ItemStartedNotification, McpServerElicitationRequestParams, McpServerElicitationRequestResponse, - ToolRequestUserInputParams, - ToolRequestUserInputResponse, } from "./app-server/v2"; import { logger } from "./Logger"; import { McpApprovalOptionId } from "./McpApprovalOptionId"; @@ -33,8 +31,10 @@ type McpElicitationContext = { persistOptions: Set; correlatedCallId: string | undefined; }; - -const USER_INPUT_OTHER_FIELD_SUFFIX = "__other"; +type AcpBackedMcpElicitationParams = Extract< + McpServerElicitationRequestParams, + { mode: "form" } | { mode: "url" } +>; /** * Parses the `persist` field from the elicitation request `_meta`. @@ -219,33 +219,6 @@ function elicitationResponseMeta( return Object.keys(meta).length === 0 ? null : meta; } -function userInputOtherFieldId(questionId: string, questionIds: Set): string { - const base = `${questionId}${USER_INPUT_OTHER_FIELD_SUFFIX}`; - if (!questionIds.has(base)) { - return base; - } - - let index = 1; - while (questionIds.has(`${base}${index}`)) { - index += 1; - } - return `${base}${index}`; -} - -function userInputResponseValue( - content: Record, - fieldId: string -): acp.ElicitationContentValue | undefined { - const value = content[fieldId]; - if (typeof value === "string" && value.trim() === "") { - return undefined; - } - if (Array.isArray(value) && value.length === 0) { - return undefined; - } - return value; -} - /** * Builds the ACP permission options for an MCP tool call approval elicitation. * Always includes "Allow Once"; adds session/always persist options when advertised. @@ -359,74 +332,8 @@ export class CodexElicitationHandler implements ElicitationHandler { } } - async handleUserInput(params: ToolRequestUserInputParams): Promise { - if (!clientSupportsFormElicitation(this.clientCapabilities)) { - return { answers: {} }; - } - - try { - const response = await this.requestUserInputElicitation(params); - if (response === null) { - return { answers: {} }; - } - return this.convertUserInputResponse(response, params); - } catch (error) { - logger.error("Error handling Codex user input request", error); - return { answers: {} }; - } - } - - private requestOptions(cancellationSignal: AbortSignal | undefined = this.cancellationSignal): acp.SendRequestOptions | undefined { - return cancellationSignal ? {cancellationSignal} : undefined; - } - - private async requestUserInputElicitation( - params: ToolRequestUserInputParams - ): Promise { - const request = this.buildUserInputRequest(params); - if (params.autoResolutionMs === null) { - return await this.connection.request( - acp.methods.client.elicitation.create, - request, - this.requestOptions(), - ); - } - - const abortController = new AbortController(); - let timeout: ReturnType | undefined; - let removeAbortListener: (() => void) | undefined; - const timeoutPromise = new Promise((resolve) => { - const resolveWithoutInput = () => { - abortController.abort(); - resolve(null); - }; - timeout = setTimeout(resolveWithoutInput, Math.max(0, params.autoResolutionMs ?? 0)); - if (this.cancellationSignal?.aborted) { - resolveWithoutInput(); - return; - } - if (this.cancellationSignal) { - this.cancellationSignal.addEventListener("abort", resolveWithoutInput, { once: true }); - removeAbortListener = () => { - this.cancellationSignal?.removeEventListener("abort", resolveWithoutInput); - }; - } - }); - const requestPromise = Promise.resolve(this.connection.request( - acp.methods.client.elicitation.create, - request, - this.requestOptions(abortController.signal), - )); - void requestPromise.catch(() => {}); - - try { - return await Promise.race([requestPromise, timeoutPromise]); - } finally { - if (timeout) { - clearTimeout(timeout); - } - removeAbortListener?.(); - } + private requestOptions(): acp.SendRequestOptions | undefined { + return this.cancellationSignal ? {cancellationSignal: this.cancellationSignal} : undefined; } private createMcpElicitationContext(params: McpServerElicitationRequestParams): McpElicitationContext { @@ -438,18 +345,21 @@ export class CodexElicitationHandler implements ElicitationHandler { return { isToolApproval, persistOptions, correlatedCallId }; } - private shouldUseAcpElicitation(params: McpServerElicitationRequestParams): boolean { + private shouldUseAcpElicitation( + params: McpServerElicitationRequestParams + ): params is AcpBackedMcpElicitationParams { switch (params.mode) { case "form": - case "openai/form": return clientSupportsFormElicitation(this.clientCapabilities); case "url": return clientSupportsUrlElicitation(this.clientCapabilities); + case "openai/form": + return false; } } private buildElicitationRequest( - params: McpServerElicitationRequestParams, + params: AcpBackedMcpElicitationParams, context: McpElicitationContext ): acp.CreateElicitationRequest { const base = { @@ -460,8 +370,7 @@ export class CodexElicitationHandler implements ElicitationHandler { }; switch (params.mode) { - case "form": - case "openai/form": { + case "form": { const requestedSchema = context.isToolApproval ? addPersistChoiceToSchema( normalizeElicitationSchema(params.requestedSchema), @@ -484,79 +393,6 @@ export class CodexElicitationHandler implements ElicitationHandler { } } - private buildUserInputRequest(params: ToolRequestUserInputParams): acp.CreateElicitationRequest { - const properties: Record = {}; - const required: string[] = []; - const questionIds = new Set(params.questions.map(question => question.id)); - - for (const question of params.questions) { - const options = question.options ?? []; - const hasOptions = options.length > 0; - const hasOtherAnswer = question.isOther && hasOptions; - const base = { - title: question.header || question.id, - description: question.question, - _meta: { - codex: { - isOther: question.isOther, - isSecret: question.isSecret, - }, - }, - }; - if (!hasOtherAnswer) { - required.push(question.id); - } - properties[question.id] = hasOptions - ? { - ...base, - type: "string", - oneOf: options.map(option => ({ - const: option.label, - title: option.label, - description: option.description, - })), - } - : { - ...base, - type: "string", - }; - if (hasOtherAnswer) { - properties[userInputOtherFieldId(question.id, questionIds)] = { - type: "string", - title: "Other", - description: "Type your own answer instead of choosing an option above.", - _meta: { - codex: { - questionId: question.id, - isOtherAnswer: true, - isSecret: question.isSecret, - }, - }, - }; - } - } - - const firstQuestion = params.questions[0]; - return { - sessionId: this.sessionState.sessionId, - toolCallId: params.itemId, - mode: "form", - message: params.questions.length === 1 && firstQuestion - ? firstQuestion.question - : "Input requested", - requestedSchema: { - type: "object", - properties, - required, - }, - _meta: { - codex: { - autoResolutionMs: params.autoResolutionMs, - }, - }, - }; - } - private buildPermissionRequest( params: McpServerElicitationRequestParams, context: McpElicitationContext @@ -671,34 +507,6 @@ export class CodexElicitationHandler implements ElicitationHandler { } } - private convertUserInputResponse( - response: acp.CreateElicitationResponse, - params: ToolRequestUserInputParams - ): ToolRequestUserInputResponse { - if (response.action !== "accept") { - return { answers: {} }; - } - - const answers: ToolRequestUserInputResponse["answers"] = {}; - const content = contentRecord(response.content); - const questionIds = new Set(params.questions.map(question => question.id)); - for (const question of params.questions) { - const value = question.isOther && question.options != null && question.options.length > 0 - ? userInputResponseValue(content, userInputOtherFieldId(question.id, questionIds)) - ?? userInputResponseValue(content, question.id) - : userInputResponseValue(content, question.id); - if (value === undefined) { - continue; - } - answers[question.id] = { - answers: Array.isArray(value) - ? value.map(String) - : [String(value)], - }; - } - return { answers }; - } - private async publishAcceptedMcpToolApproval( context: McpElicitationContext, accepted: boolean diff --git a/src/ElicitationCapabilities.ts b/src/ElicitationCapabilities.ts index a90351b8..51030b0d 100644 --- a/src/ElicitationCapabilities.ts +++ b/src/ElicitationCapabilities.ts @@ -1,5 +1,4 @@ import type * as acp from "@agentclientprotocol/sdk"; -import type {InitializeCapabilities} from "./app-server"; export function clientSupportsFormElicitation( clientCapabilities?: acp.ClientCapabilities | null @@ -12,11 +11,3 @@ export function clientSupportsUrlElicitation( ): boolean { return clientCapabilities?.elicitation?.url != null; } - -export function createAppServerInitializeCapabilities( - _clientCapabilities?: acp.ClientCapabilities | null -): InitializeCapabilities | null { - // Do not opt into app-server experimental APIs just because the ACP client supports elicitation. - // The handlers can stay in place, but this keeps request_user_input dormant until explicitly enabled. - return null; -} diff --git a/src/__tests__/CodexACPAgent/elicitation-events.test.ts b/src/__tests__/CodexACPAgent/elicitation-events.test.ts index 10e9485e..3dd84e1a 100644 --- a/src/__tests__/CodexACPAgent/elicitation-events.test.ts +++ b/src/__tests__/CodexACPAgent/elicitation-events.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import * as acp from "@agentclientprotocol/sdk"; -import type { McpServerElicitationRequestParams, ToolRequestUserInputParams } from '../../app-server/v2'; +import type { McpServerElicitationRequestParams } from '../../app-server/v2'; import { createCodexMockTestFixture, createTestSessionState, type CodexMockTestFixture } from '../acp-test-utils'; import type { SessionState } from '../../CodexAcpServer'; import { AgentMode } from "../../AgentMode"; @@ -633,191 +633,4 @@ describe('Elicitation Events', () => { await promptPromise; }); }); - - describe('Codex request_user_input', () => { - it('should use ACP form elicitation for request_user_input when supported', async () => { - const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ - elicitation: { form: {} }, - }); - fixture.setElicitationResponse({ - action: 'accept', - content: { - next_step: 'Run tests', - notes: 'Focus auth', - }, - }); - - const params: ToolRequestUserInputParams = { - threadId: sessionId, - turnId: 'turn-1', - itemId: 'request-user-input-1', - autoResolutionMs: 60000, - questions: [ - { - id: 'next_step', - header: 'Next step', - question: 'What should I do next?', - isOther: true, - isSecret: false, - options: [ - { label: 'Run tests', description: 'Run the focused test suite.' }, - { label: 'Stop', description: 'Stop and report current status.' }, - ], - }, - { - id: 'notes', - header: 'Notes', - question: 'Any extra instructions?', - isOther: false, - isSecret: false, - options: null, - }, - ], - }; - - const response = await fixture.sendServerRequest('item/tool/requestUserInput', params); - expect(response).toEqual({ - answers: { - next_step: { answers: ['Run tests'] }, - notes: { answers: ['Focus auth'] }, - }, - }); - - const [elicitationEvent] = fixture.getAcpConnectionEvents(['_meta']); - expect(elicitationEvent).toMatchObject({ - method: 'createElicitation', - args: [{ - sessionId, - toolCallId: 'request-user-input-1', - mode: 'form', - message: 'Input requested', - requestedSchema: { - type: 'object', - required: ['notes'], - }, - }], - }); - expect(elicitationEvent!.args[0].requestedSchema.properties.next_step.oneOf).toEqual([ - { const: 'Run tests', title: 'Run tests', description: 'Run the focused test suite.' }, - { const: 'Stop', title: 'Stop', description: 'Stop and report current status.' }, - ]); - expect(elicitationEvent!.args[0].requestedSchema.properties.next_step__other).toMatchObject({ - type: 'string', - title: 'Other', - }); - expect(elicitationEvent!.args[0].requestedSchema.properties.notes).toMatchObject({ - type: 'string', - title: 'Notes', - description: 'Any extra instructions?', - }); - - completeTurn(); - await promptPromise; - }); - - it('should prefer free-form Other answers over fixed choices', async () => { - const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ - elicitation: { form: {} }, - }); - fixture.setElicitationResponse({ - action: 'accept', - content: { - next_step: 'Run tests', - next_step__other: 'Inspect flaky logs', - }, - }); - - const params: ToolRequestUserInputParams = { - threadId: sessionId, - turnId: 'turn-1', - itemId: 'request-user-input-1', - autoResolutionMs: null, - questions: [{ - id: 'next_step', - header: 'Next step', - question: 'What should I do next?', - isOther: true, - isSecret: false, - options: [ - { label: 'Run tests', description: 'Run the focused test suite.' }, - { label: 'Stop', description: 'Stop and report current status.' }, - ], - }], - }; - - const response = await fixture.sendServerRequest('item/tool/requestUserInput', params); - expect(response).toEqual({ - answers: { - next_step: { answers: ['Inspect flaky logs'] }, - }, - }); - - completeTurn(); - await promptPromise; - }); - - it('should auto-resolve request_user_input when the client does not answer in time', async () => { - const { promptPromise, completeTurn } = await setupSessionWithPendingPromptAndCapabilities({ - elicitation: { form: {} }, - }); - fixture.setElicitationResponse(new Promise(() => {})); - - const params: ToolRequestUserInputParams = { - threadId: sessionId, - turnId: 'turn-1', - itemId: 'request-user-input-1', - autoResolutionMs: 1, - questions: [{ - id: 'next_step', - header: 'Next step', - question: 'What should I do next?', - isOther: false, - isSecret: false, - options: null, - }], - }; - - const response = await fixture.sendServerRequest('item/tool/requestUserInput', params); - expect(response).toEqual({ answers: {} }); - - const [elicitationEvent] = fixture.getAcpConnectionEvents(['_meta']); - expect(elicitationEvent).toMatchObject({ - method: 'createElicitation', - args: [{ - sessionId, - toolCallId: 'request-user-input-1', - mode: 'form', - }], - }); - - completeTurn(); - await promptPromise; - }); - - it('should not call ACP elicitation for request_user_input without form support', async () => { - const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); - - const params: ToolRequestUserInputParams = { - threadId: sessionId, - turnId: 'turn-1', - itemId: 'request-user-input-1', - autoResolutionMs: null, - questions: [{ - id: 'next_step', - header: 'Next step', - question: 'What should I do next?', - isOther: false, - isSecret: false, - options: null, - }], - }; - - const response = await fixture.sendServerRequest('item/tool/requestUserInput', params); - expect(response).toEqual({ answers: {} }); - expect(fixture.getAcpConnectionEvents(['_meta'])).toEqual([]); - - completeTurn(); - await promptPromise; - }); - }); });