diff --git a/src/CodexElicitationHandler.ts b/src/CodexElicitationHandler.ts index 732822b..68a4994 100644 --- a/src/CodexElicitationHandler.ts +++ b/src/CodexElicitationHandler.ts @@ -9,7 +9,7 @@ import type { McpServerElicitationRequestResponse, } from "./app-server/v2"; import { logger } from "./Logger"; -import { ApprovalOptionId } from "./ApprovalOptionId"; +import { McpApprovalOptionId } from "./McpApprovalOptionId"; // Standard elicitation options (non-tool-call approval). const ELICITATION_OPTIONS: acp.PermissionOption[] = [ @@ -17,9 +17,6 @@ const ELICITATION_OPTIONS: acp.PermissionOption[] = [ { optionId: "decline", name: "Decline", kind: "reject_once" }, ]; -// Option ID unique to elicitation persist choices — not part of the shared ApprovalOptionId set. -const OPTION_ALLOW_SESSION = "allow_session"; - type PersistValue = "session" | "always"; /** @@ -56,15 +53,15 @@ function isMcpToolCallApproval(meta: unknown): boolean { */ function buildToolApprovalOptions(persistOptions: Set): acp.PermissionOption[] { const options: acp.PermissionOption[] = [ - { optionId: ApprovalOptionId.AllowOnce, name: "Allow", kind: "allow_once" }, + { optionId: McpApprovalOptionId.AllowOnce, name: "Allow", kind: "allow_once" }, ]; if (persistOptions.has("session")) { - options.push({ optionId: OPTION_ALLOW_SESSION, name: "Allow for This Session", kind: "allow_always" }); + options.push({ optionId: McpApprovalOptionId.AllowSession, name: "Allow for This Session", kind: "allow_always" }); } if (persistOptions.has("always")) { - options.push({ optionId: ApprovalOptionId.AllowAlways, name: "Allow and Don't Ask Again", kind: "allow_always" }); + options.push({ optionId: McpApprovalOptionId.AllowAlways, name: "Allow and Don't Ask Again", kind: "allow_always" }); } - options.push({ optionId: "decline", name: "Decline", kind: "reject_once" }); + options.push({ optionId: McpApprovalOptionId.Decline, name: "Decline", kind: "reject_once" }); return options; } @@ -117,7 +114,7 @@ export class CodexElicitationHandler implements ElicitationHandler { const response = await this.connection.requestPermission(request); if (correlatedCallId !== undefined && response.outcome.outcome !== "cancelled") { const optionId = response.outcome.optionId; - if (optionId !== "decline") { + if (optionId !== McpApprovalOptionId.Decline) { await this.connection.sessionUpdate({ sessionId: this.sessionState.sessionId, update: { sessionUpdate: "tool_call_update", toolCallId: correlatedCallId, status: "in_progress" }, @@ -211,13 +208,13 @@ export class CodexElicitationHandler implements ElicitationHandler { } const optionId = response.outcome.optionId; - if (optionId === OPTION_ALLOW_SESSION) { + if (optionId === McpApprovalOptionId.AllowSession) { return { action: "accept", content: null, _meta: { persist: "session" } }; } - if (optionId === ApprovalOptionId.AllowAlways) { + if (optionId === McpApprovalOptionId.AllowAlways) { return { action: "accept", content: null, _meta: { persist: "always" } }; } - if (optionId === ApprovalOptionId.AllowOnce || optionId === "accept") { + if (optionId === McpApprovalOptionId.AllowOnce || optionId === "accept") { return { action: "accept", content: null, _meta: null }; } return { action: "decline", content: null, _meta: null }; diff --git a/src/McpApprovalOptionId.ts b/src/McpApprovalOptionId.ts new file mode 100644 index 0000000..3728145 --- /dev/null +++ b/src/McpApprovalOptionId.ts @@ -0,0 +1,8 @@ +export const McpApprovalOptionId = { + AllowOnce: "allow_once", + AllowSession: "allow_session", + AllowAlways: "allow_always", + Decline: "decline", +} as const; + +export type McpApprovalOptionId = typeof McpApprovalOptionId[keyof typeof McpApprovalOptionId]; diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts index 42123a7..e0ef576 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts @@ -1,25 +1,29 @@ import type * as acp from "@agentclientprotocol/sdk"; +import fs from "node:fs"; import path from "node:path"; import {afterEach, beforeEach, expect, it} from "vitest"; -import {ApprovalOptionId} from "../../../ApprovalOptionId"; +import {McpApprovalOptionId, type McpApprovalOptionId as McpApprovalOptionIdValue} from "../../../McpApprovalOptionId"; import { createAuthenticatedFixture, - createPermissionResponse, describeE2E, expectEndTurn, type PermissionResponder, type SpawnedAgentFixture, } from "./acp-e2e-test-utils"; +import os from "node:os"; const MCP_SERVER_NAME = "integration-mcp"; const MCP_ECHO_MESSAGE = "mcp approval e2e"; -function createMcpServer(): acp.McpServerStdio { +function createMcpServer(invocationMarkerPath: string): acp.McpServerStdio { return { name: MCP_SERVER_NAME, command: process.execPath, - args: [path.join(process.cwd(), "node_modules/mcp-hello-world/build/stdio.js")], - env: [], + args: [path.join(process.cwd(), "src/__tests__/CodexACPAgent/e2e/fixtures/invocation-aware-mcp-server.mjs")], + env: [{ + name: "MCP_TOOL_INVOCATION_MARKER_PATH", + value: invocationMarkerPath, + }], }; } @@ -27,42 +31,67 @@ function isMcpPermissionRequest(request: acp.RequestPermissionRequest): boolean return request.toolCall.kind === "execute" && request._meta?.["is_mcp_tool_approval"] === true; } -function createMcpPermissionResponder(optionId: ApprovalOptionId): PermissionResponder { - return (request) => createPermissionResponse(isMcpPermissionRequest(request) ? optionId : null); +function createMcpPermissionResponse(optionId: McpApprovalOptionIdValue | null): acp.RequestPermissionResponse { + if (optionId === null) { + return {outcome: {outcome: "cancelled"}}; + } + return {outcome: {outcome: "selected", optionId}}; +} + +function createMcpPermissionResponder(...optionIds: McpApprovalOptionIdValue[]): PermissionResponder { + const queue = [...optionIds]; + return (request) => createMcpPermissionResponse( + isMcpPermissionRequest(request) + ? queue.shift() ?? McpApprovalOptionId.Decline + : null, + ); +} + +async function expectEchoToolReply(fixture: SpawnedAgentFixture, sessionId: string, message: string): Promise { + await fixture.expectPromptText( + sessionId, + `Use the ${MCP_SERVER_NAME} MCP echo tool with message "${message}". Reply with exactly the tool result and no extra text.`, + (text) => expect(text).toContain(`You said: ${message}`), + ); +} + +function expectMcpPermissionRequestCount(fixture: SpawnedAgentFixture, sessionId: string, count: number): void { + const requests = fixture.readPermissionRequests(sessionId, "execute"); + expect(requests.length).toBe(count); + for (const request of requests) { + expect(isMcpPermissionRequest(request)).toBe(true); + } } -describeE2E("E2E MCP approval tests", () => { +describeE2E("E2E MCP approval tests (configured in session)", () => { let fixture: SpawnedAgentFixture; - let sessionId: string; beforeEach(async () => { fixture = await createAuthenticatedFixture(); - sessionId = (await fixture.createSession([createMcpServer()])).sessionId; }); afterEach(async () => { await fixture.dispose(); }); - function expectMcpToolPermissionRequest(): void { - const requests = fixture.readPermissionRequests(sessionId, "execute"); - expect(requests.length).toBe(1); - expect(isMcpPermissionRequest(requests[0]!)).toBe(true); + async function createMcpSession(): Promise<{ sessionId: string; invocationMarkerPath: string }> { + const invocationMarkerPath = path.join(fixture.workspaceDir, `mcp-tool-invocation-${crypto.randomUUID()}.txt`); + const sessionId = (await fixture.createSession([createMcpServer(invocationMarkerPath)])).sessionId; + return {sessionId, invocationMarkerPath}; } it("executes an approved MCP tool call", async () => { - fixture.setPermissionResponder(createMcpPermissionResponder(ApprovalOptionId.AllowOnce)); + fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.AllowOnce)); + const {sessionId, invocationMarkerPath} = await createMcpSession(); - await fixture.expectPromptText( - sessionId, - `Use the ${MCP_SERVER_NAME} MCP echo tool with message "${MCP_ECHO_MESSAGE}". Reply with exactly the tool result and no extra text.`, - (text) => expect(text).toContain(`You said: ${MCP_ECHO_MESSAGE}`), - ); - expectMcpToolPermissionRequest(); + await expectEchoToolReply(fixture, sessionId, MCP_ECHO_MESSAGE); + expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe(MCP_ECHO_MESSAGE); + expectMcpPermissionRequestCount(fixture, sessionId, 1); }); it("ends turn when MCP tool call is rejected", async () => { - fixture.setPermissionResponder(createMcpPermissionResponder(ApprovalOptionId.RejectOnce)); + fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.Decline)); + const {sessionId, invocationMarkerPath} = await createMcpSession(); expectEndTurn(await fixture.connection.prompt({ sessionId, @@ -71,6 +100,92 @@ describeE2E("E2E MCP approval tests", () => { text: `Use the ${MCP_SERVER_NAME} MCP echo tool with message "${MCP_ECHO_MESSAGE}". Stop if the tool call is rejected.`, }], })); - expectMcpToolPermissionRequest(); + expect(fs.existsSync(invocationMarkerPath)).toBe(false); + expectMcpPermissionRequestCount(fixture, sessionId, 1); + }); + + it("skips subsequent approvals in the same session when allow_session is selected", async () => { + fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.AllowSession)); + const {sessionId, invocationMarkerPath} = await createMcpSession(); + + await expectEchoToolReply(fixture, sessionId, "session approval first"); + await expectEchoToolReply(fixture, sessionId, "session approval second"); + + expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe("session approval second"); + expectMcpPermissionRequestCount(fixture, sessionId, 1); + }); + + it("requests subsequent approvals after session restart when allow_session is selected", async () => { + fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.AllowSession, McpApprovalOptionId.AllowOnce)); + const {sessionId, invocationMarkerPath} = await createMcpSession(); + + await expectEchoToolReply(fixture, sessionId, MCP_ECHO_MESSAGE); + expectMcpPermissionRequestCount(fixture, sessionId, 1); + + fixture = await fixture.restart(); + await fixture.connection.loadSession({ + sessionId, + cwd: fixture.workspaceDir, + mcpServers: [createMcpServer(invocationMarkerPath)], + }); + + await expectEchoToolReply(fixture, sessionId, MCP_ECHO_MESSAGE); + expectMcpPermissionRequestCount(fixture, sessionId, 2); + }); +}); + +describeE2E("E2E MCP approval tests (configured in toml)", () => { + let invocationMarkerPath: string; + let fixture: SpawnedAgentFixture; + + beforeEach(async () => { + fixture = await createAuthenticatedFixture(); + invocationMarkerPath = path.join(os.tmpdir(), `mcp-tool-invocation-${crypto.randomUUID()}.txt`) + }); + + afterEach(async () => { + await fixture.dispose(); + fs.rmSync(invocationMarkerPath, { force: true }); + }); + + + beforeEach(async () => { + await fixture.dispose(); + fixture = await createAuthenticatedFixture(undefined, [createMcpServer(invocationMarkerPath)]); + }); + + it("skips subsequent approvals in the same session when allow_always is selected", async () => { + fixture.setPermissionResponder( + createMcpPermissionResponder(McpApprovalOptionId.AllowAlways), + ); + const sessionId = (await fixture.createSession()).sessionId; + + await expectEchoToolReply(fixture, sessionId, "always approval first"); + await expectEchoToolReply(fixture, sessionId, "always approval second"); + + expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe("always approval second"); + expectMcpPermissionRequestCount(fixture, sessionId, 1); + }); + + it.skip("skips subsequent approvals after session restart when allow_always is selected", async () => { + fixture.setPermissionResponder( + createMcpPermissionResponder(McpApprovalOptionId.AllowAlways), + ); + const firstSessionId = (await fixture.createSession()).sessionId; + + await expectEchoToolReply(fixture, firstSessionId, "always approval first"); + + fixture = await fixture.restart(); + fixture.setPermissionResponder((request) => { + if (isMcpPermissionRequest(request)) { + throw new Error("unexpected MCP approval after allow_always restart"); + } + return createMcpPermissionResponse(null); + }); + const newSessionId = (await fixture.createSession()).sessionId; + await expectEchoToolReply(fixture, newSessionId, "always approval second"); + + expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe("always approval second"); + expect(fixture.readPermissionRequests(newSessionId, "execute").length).toBe(0); }); }); diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts index 429e79d..5b44be0 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts @@ -1,7 +1,7 @@ import * as acp from "@agentclientprotocol/sdk"; import {describe, expect} from "vitest"; import {AgentMode} from "../../../AgentMode"; -import {createSpawnedAgentFixture, type SpawnedAgentFixture} from "./spawned-agent-fixture"; +import {createSpawnedAgentFixture, type SpawnedAgentFixture,} from "./spawned-agent-fixture"; export { createPermissionResponder, @@ -46,7 +46,7 @@ export function expectNoPermissionRequests(fixture: SpawnedAgentFixture, session expectPermissionRequests(fixture, sessionId, { edit: 0, execute: 0 }); } -export async function createAuthenticatedFixture(initialMode?: AgentMode): Promise { +export async function createAuthenticatedFixture(initialMode?: AgentMode, mcpServers?: acp.McpServerStdio[]): Promise { const apiKey = requireLiveApiKey(); const extraEnv = initialMode ? {INITIAL_AGENT_MODE: initialMode.id} : undefined; return await createSpawnedFixture(async (connection, authMethods) => { @@ -67,7 +67,7 @@ export async function createAuthenticatedFixture(initialMode?: AgentMode): Promi if (authenticationStatus["type"] !== "api-key") { throw new Error(`Unexpected authentication status: ${JSON.stringify(authenticationStatus)}`); } - }, extraEnv); + }, extraEnv, mcpServers); } export async function createGatewayFixture( @@ -119,6 +119,7 @@ type Authenticator = (connection: acp.ClientSideConnection, authMethods: acp.Aut async function createSpawnedFixture( authenticate: Authenticator, extraEnv?: NodeJS.ProcessEnv, + mcpServers?: acp.McpServerStdio[], ): Promise { return await createSpawnedAgentFixture(async (connection) => { const initializeResponse = await connection.initialize({ @@ -135,7 +136,7 @@ async function createSpawnedFixture( } await authenticate(connection, initializeResponse.authMethods ?? []); - }, extraEnv); + }, extraEnv, mcpServers); } export function requireLiveApiKey(): string { diff --git a/src/__tests__/CodexACPAgent/e2e/fixtures/invocation-aware-mcp-server.mjs b/src/__tests__/CodexACPAgent/e2e/fixtures/invocation-aware-mcp-server.mjs new file mode 100644 index 0000000..828986c --- /dev/null +++ b/src/__tests__/CodexACPAgent/e2e/fixtures/invocation-aware-mcp-server.mjs @@ -0,0 +1,32 @@ +import fs from "node:fs/promises"; +import {McpServer} from "@modelcontextprotocol/sdk/server/mcp.js"; +import {StdioServerTransport} from "@modelcontextprotocol/sdk/server/stdio.js"; +import {z} from "zod"; + +const markerPath = process.env["MCP_TOOL_INVOCATION_MARKER_PATH"]; + +if (!markerPath) { + throw new Error("MCP_TOOL_INVOCATION_MARKER_PATH is required."); +} + +const server = new McpServer({ + name: "integration-mcp", + version: "1.0.0", +}); + +server.tool( + "echo", + "Echoes back a message with a side effect for test assertions.", + {message: z.string().describe("The message to echo")}, + async ({message}) => { + await fs.writeFile(markerPath, message, "utf8"); + return { + content: [{ + type: "text", + text: `You said: ${message}`, + }], + }; + }, +); + +await server.connect(new StdioServerTransport()); diff --git a/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts b/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts index 326fd04..7ec42d8 100644 --- a/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts +++ b/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts @@ -43,25 +43,37 @@ type ConnectionInitializer = (connection: acp.ClientSideConnection) => Promise { + const resolvedPaths = paths ?? RuntimePaths.createTemporary(); + const configuredMcpServers = mcpServers ?? []; + writeCodexHomeConfig(resolvedPaths.codexHome, { + model: DEFAULT_TEST_MODEL_ID.model, + model_reasoning_effort: DEFAULT_TEST_MODEL_ID.effort, + web_search: "disabled", + }, configuredMcpServers); + + const resolvedClient = client ?? new RecordingClient(); const agentProcess = spawn("npm", ["run", "--silent", "start"], { cwd: process.cwd(), env: { ...process.env, - CODEX_HOME: paths.codexHome, - APP_SERVER_LOGS: paths.appServerLogsDir, + CODEX_HOME: resolvedPaths.codexHome, + APP_SERVER_LOGS: resolvedPaths.appServerLogsDir, ...extraEnv, }, stdio: ["pipe", "pipe", "pipe"], }); const fixture = new SpawnedAgentFixtureImpl( - new RecordingClient(), + resolvedClient, agentProcess, - paths, + resolvedPaths, initializeConnection, - extraEnv, + extraEnv ?? {}, + configuredMcpServers, ); await initializeConnection(fixture.connection); return fixture; @@ -84,11 +96,6 @@ class RuntimePaths { for (const dir of [paths.rootDir, paths.codexHome, paths.workspaceDir, paths.appServerLogsDir]) { fs.mkdirSync(dir, {recursive: true}); } - writeCodexHomeConfig(paths.codexHome, { - model: DEFAULT_TEST_MODEL_ID.model, - model_reasoning_effort: DEFAULT_TEST_MODEL_ID.effort, - web_search: "disabled", - }); return paths; } } @@ -145,7 +152,8 @@ class SpawnedAgentFixtureImpl implements SpawnedAgentFixture { private readonly agentProcess: ChildProcessWithoutNullStreams, private readonly paths: RuntimePaths, private readonly initializeConnection: ConnectionInitializer, - private readonly extraEnv?: NodeJS.ProcessEnv, + private readonly extraEnv: NodeJS.ProcessEnv, + private readonly mcpServers: acp.McpServerStdio[], ) { const output = Readable.toWeb(agentProcess.stdout) as ReadableStream; this.connection = new acp.ClientSideConnection( @@ -167,7 +175,7 @@ class SpawnedAgentFixtureImpl implements SpawnedAgentFixture { async restart(): Promise { await this.stopProcess(false); - return await createSpawnedAgentFixture(this.initializeConnection, this.extraEnv, this.paths); + return await createSpawnedAgentFixture(this.initializeConnection, this.extraEnv, this.mcpServers, this.paths, this.client); } writeSkill(skill: TestSkill, rootDir?: string): void { diff --git a/src/__tests__/CodexACPAgent/elicitation-events.test.ts b/src/__tests__/CodexACPAgent/elicitation-events.test.ts index 32548be..82d14a0 100644 --- a/src/__tests__/CodexACPAgent/elicitation-events.test.ts +++ b/src/__tests__/CodexACPAgent/elicitation-events.test.ts @@ -3,6 +3,7 @@ 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"; +import { McpApprovalOptionId } from "../../McpApprovalOptionId"; import type { ServerNotification } from "../../app-server"; describe('Elicitation Events', () => { @@ -132,7 +133,7 @@ describe('Elicitation Events', () => { describe('MCP tool call approval elicitation', () => { it('should show Allow/session/always/Decline options when all persist values advertised', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); - fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } }); + fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: McpApprovalOptionId.AllowOnce } }); const params: McpServerElicitationRequestParams = { threadId: sessionId, turnId: 'turn-1', serverName: 'tool-server', @@ -151,7 +152,7 @@ describe('Elicitation Events', () => { it('should map allow_once to accept with null meta', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); - fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } }); + fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: McpApprovalOptionId.AllowOnce } }); const params: McpServerElicitationRequestParams = { threadId: sessionId, turnId: 'turn-1', serverName: 'tool-server', @@ -170,7 +171,7 @@ describe('Elicitation Events', () => { it('should map allow_session to accept with persist:session meta', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); - fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_session' } }); + fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: McpApprovalOptionId.AllowSession } }); const params: McpServerElicitationRequestParams = { threadId: sessionId, turnId: 'turn-1', serverName: 'tool-server', @@ -189,7 +190,7 @@ describe('Elicitation Events', () => { it('should map allow_always to accept with persist:always meta', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); - fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_always' } }); + fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: McpApprovalOptionId.AllowAlways } }); const params: McpServerElicitationRequestParams = { threadId: sessionId, turnId: 'turn-1', serverName: 'tool-server', @@ -208,7 +209,7 @@ describe('Elicitation Events', () => { it('should only show session option when persist is "session"', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); - fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } }); + fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: McpApprovalOptionId.AllowOnce } }); const params: McpServerElicitationRequestParams = { threadId: sessionId, turnId: 'turn-1', serverName: 'tool-server', @@ -227,7 +228,7 @@ describe('Elicitation Events', () => { it('should show only Allow and Decline when no persist options', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); - fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } }); + fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: McpApprovalOptionId.AllowOnce } }); const params: McpServerElicitationRequestParams = { threadId: sessionId, turnId: 'turn-1', serverName: 'tool-server', @@ -246,7 +247,7 @@ describe('Elicitation Events', () => { it('should not reuse a completed auto-approved call id for a later approval request', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); - fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } }); + fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: McpApprovalOptionId.AllowOnce } }); const startedNotification: ServerNotification = { method: 'item/started', @@ -309,7 +310,7 @@ describe('Elicitation Events', () => { it('should not reuse a stale call id after serverRequest/resolved clears interrupted approval state', async () => { const { promptPromise, completeTurn } = setupSessionWithPendingPrompt(); - fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: 'allow_once' } }); + fixture.setPermissionResponse({ outcome: { outcome: 'selected', optionId: McpApprovalOptionId.AllowOnce } }); const startedNotification: ServerNotification = { method: 'item/started', diff --git a/src/__tests__/acp-test-utils.ts b/src/__tests__/acp-test-utils.ts index ec510dc..9f82f16 100644 --- a/src/__tests__/acp-test-utils.ts +++ b/src/__tests__/acp-test-utils.ts @@ -1,8 +1,8 @@ +import type {AgentSideConnection, McpServerStdio, RequestPermissionResponse} from "@agentclientprotocol/sdk"; import {CodexAcpClient} from '../CodexAcpClient'; -import {type CodexConnectionEvent, CodexAppServerClient} from '../CodexAppServerClient'; +import {CodexAppServerClient, type CodexConnectionEvent} from '../CodexAppServerClient'; import {startCodexConnection} from "../CodexJsonRpcConnection"; import {CodexAcpServer, type SessionState} from "../CodexAcpServer"; -import type {AgentSideConnection, RequestPermissionResponse} from "@agentclientprotocol/sdk"; import type {ServerNotification} from "../app-server"; import type {MessageConnection} from "vscode-jsonrpc/node"; import path from "node:path"; @@ -176,17 +176,37 @@ function createTestCodexHome(): string { return codexHome; } -export function writeCodexHomeConfig(codexHome: string, extras: Record = {}): void { +export function writeCodexHomeConfig( + codexHome: string, + extras: Record = {}, + mcpServers: McpServerStdio[] = [], +): void { const entries: Record = { cli_auth_credentials_store: "file", ...extras, }; - const body = Object.entries(entries) + let body = Object.entries(entries) .map(([key, value]) => `${key} = "${value}"`) .join("\n"); + for (const server of mcpServers) { + body += `\n\n[mcp_servers."${escapeTOML(server.name)}"]`; + body += `\ncommand = "${escapeTOML(server.command)}"`; + const argsToml = server.args.map(a => `"${escapeTOML(a)}"`).join(", "); + body += `\nargs = [${argsToml}]`; + if (server.env && server.env.length > 0) { + const envPairs = server.env + .map(e => `${e.name} = "${escapeTOML(e.value)}"`) + .join(", "); + body += `\nenv = {${envPairs}}`; + } + } fs.writeFileSync(path.join(codexHome, "config.toml"), body, "utf8"); } +function escapeTOML(value: string): string { + return value.replace(/\\/g, "\\\\").replace(/"/g, '\\"'); +} + export function removeDirectoryWithRetry(directory: string): void { for (let attempt = 0; attempt < 5; attempt += 1) { try {