diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 6454ca86..2f33273a 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,10 +1440,15 @@ 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); + async (event) => { + await elicitationHandler.handleNotification(event); return eventHandler.handleNotification(event); }, approvalHandler, diff --git a/src/CodexElicitationHandler.ts b/src/CodexElicitationHandler.ts index 2a15df04..3b4498e0 100644 --- a/src/CodexElicitationHandler.ts +++ b/src/CodexElicitationHandler.ts @@ -2,6 +2,7 @@ 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, @@ -11,6 +12,10 @@ import type { 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 +24,17 @@ const ELICITATION_OPTIONS: acp.PermissionOption[] = [ ]; type PersistValue = "session" | "always"; +type ToolApprovalPersistValue = PersistValue | "once"; + +type McpElicitationContext = { + isToolApproval: boolean; + persistOptions: Set; + correlatedCallId: string | undefined; +}; +type AcpBackedMcpElicitationParams = Extract< + McpServerElicitationRequestParams, + { mode: "form" } | { mode: "url" } +>; /** * Parses the `persist` field from the elicitation request `_meta`. @@ -48,6 +64,161 @@ 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 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)) { + 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)])); +} + +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. @@ -69,6 +240,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 @@ -86,14 +258,23 @@ 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, 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; } - handleNotification(notification: ServerNotification): void { + async handleNotification(notification: ServerNotification): Promise { switch (notification.method) { case "item/started": this.handleItemStarted(notification.params); @@ -103,6 +284,7 @@ export class CodexElicitationHandler implements ElicitationHandler { return; case "serverRequest/resolved": this.clearThread(notification.params.threadId); + await this.completeUrlElicitations(notification.params.threadId); return; default: return; @@ -113,7 +295,22 @@ 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); + if (params.mode === "url" && result.action === "accept") { + this.trackUrlElicitation(params.threadId, params.elicitationId); + } + 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,7 +325,7 @@ 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 }; @@ -139,8 +336,66 @@ export class CodexElicitationHandler implements ElicitationHandler { return this.cancellationSignal ? {cancellationSignal: this.cancellationSignal} : undefined; } - private buildPermissionRequest( + 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 + ): params is AcpBackedMcpElicitationParams { + switch (params.mode) { + case "form": + return clientSupportsFormElicitation(this.clientCapabilities); + case "url": + return clientSupportsUrlElicitation(this.clientCapabilities); + case "openai/form": + return false; + } + } + + private buildElicitationRequest( + params: AcpBackedMcpElicitationParams, + 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": { + 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 buildPermissionRequest( + params: McpServerElicitationRequestParams, + context: McpElicitationContext ): { request: acp.RequestPermissionRequest; correlatedCallId: string | undefined } { const sessionId = this.sessionState.sessionId; const messageContent: acp.ToolCallContent = { @@ -148,17 +403,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 +416,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 +425,7 @@ export class CodexElicitationHandler implements ElicitationHandler { _meta: { is_mcp_tool_approval: true }, options, }, - correlatedCallId, + correlatedCallId: context.correlatedCallId, }; } return { @@ -183,12 +433,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 +461,7 @@ export class CodexElicitationHandler implements ElicitationHandler { } } - private convertResponse( + private convertPermissionResponse( response: acp.RequestPermissionResponse ): McpServerElicitationRequestResponse { if (response.outcome.outcome === "cancelled") { @@ -231,6 +481,67 @@ 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: elicitationResponseMeta(response, context, persist), + }; + } + case "decline": + return { action: "decline", content: null, _meta: elicitationResponseMeta(response, context) }; + case "cancel": + return { action: "cancel", content: null, _meta: elicitationResponseMeta(response, context) }; + default: + return { action: "cancel", content: null, _meta: null }; + } + } + + 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 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/ElicitationCapabilities.ts b/src/ElicitationCapabilities.ts new file mode 100644 index 00000000..51030b0d --- /dev/null +++ b/src/ElicitationCapabilities.ts @@ -0,0 +1,13 @@ +import type * as acp from "@agentclientprotocol/sdk"; + +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; +} diff --git a/src/__tests__/CodexACPAgent/elicitation-events.test.ts b/src/__tests__/CodexACPAgent/elicitation-events.test.ts index e796d8ba..3dd84e1a 100644 --- a/src/__tests__/CodexACPAgent/elicitation-events.test.ts +++ b/src/__tests__/CodexACPAgent/elicitation-events.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import * as acp from "@agentclientprotocol/sdk"; import type { McpServerElicitationRequestParams } from '../../app-server/v2'; import { createCodexMockTestFixture, createTestSessionState, type CodexMockTestFixture } from '../acp-test-utils'; import type { SessionState } from '../../CodexAcpServer'; @@ -49,7 +50,94 @@ 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' }, + _meta: { source: 'client' }, + }); + + 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: { source: 'client' } }); + + 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 +219,78 @@ 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' }, + _meta: { source: 'client' }, + }); + + 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: { source: 'client', 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 +533,53 @@ 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', _meta: { source: 'client' } }); + + 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: { source: 'client' } }); + + 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, + }], + }); + + 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; + }); + it('should map accept to accept for URL mode', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'accept' } }); diff --git a/src/__tests__/CodexACPAgent/initialize.test.ts b/src/__tests__/CodexACPAgent/initialize.test.ts index e7050101..0528629e 100644 --- a/src/__tests__/CodexACPAgent/initialize.test.ts +++ b/src/__tests__/CodexACPAgent/initialize.test.ts @@ -84,6 +84,19 @@ describe('CodexACPAgent - initialize', () => { ])); }); + it('should not opt into experimental app-server capabilities for ACP elicitation support', async () => { + await agent.initialize({ + protocolVersion: acp.PROTOCOL_VERSION, + clientCapabilities: { + elicitation: { form: {}, 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..456701f0 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,12 @@ 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.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]]}; } @@ -238,6 +244,7 @@ export interface CodexMockTestFixture extends TestFixture { sendServerNotification(notification: ServerNotification | Record): void, sendServerRequest(method: string, params: unknown): Promise, setPermissionResponse(response: RequestPermissionResponse): void, + setElicitationResponse(response: CreateElicitationResponse | Promise): void, } /** @@ -255,6 +262,9 @@ export function createCodexMockTestFixture(): CodexMockTestFixture { const permissionState: { response: RequestPermissionResponse } = { response: { outcome: { outcome: 'cancelled' } } }; + const elicitationState: { response: CreateElicitationResponse | Promise } = { + response: { action: 'cancel' } + }; const mockCodexConnection = { sendRequest: () => Promise.resolve(undefined), @@ -276,6 +286,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 +326,9 @@ export function createCodexMockTestFixture(): CodexMockTestFixture { setPermissionResponse(response: RequestPermissionResponse): void { permissionState.response = response; }, + setElicitationResponse(response: CreateElicitationResponse | Promise): void { + elicitationState.response = response; + }, }; }