Add e2e tests for mcp approval/rejection

Author: ishulgin

src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts

@@ -0,0 +1,76 @@
+import type * as acp from "@agentclientprotocol/sdk";
+import path from "node:path";
+import {afterEach, beforeEach, expect, it} from "vitest";
+import {ApprovalOptionId} from "../../../ApprovalOptionId";
+import {
+    createAuthenticatedFixture,
+    createPermissionResponse,
+    describeE2E,
+    expectEndTurn,
+    type PermissionResponder,
+    type SpawnedAgentFixture,
+} from "./acp-e2e-test-utils";
+
+const MCP_SERVER_NAME = "integration-mcp";
+const MCP_ECHO_MESSAGE = "mcp approval e2e";
+
+function createMcpServer(): acp.McpServerStdio {
+    return {
+        name: MCP_SERVER_NAME,
+        command: process.execPath,
+        args: [path.join(process.cwd(), "node_modules/mcp-hello-world/build/stdio.js")],
+        env: [],
+    };
+}
+
+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);
+}
+
+describeE2E("E2E MCP approval tests", () => {
+    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);
+    }
+
+    it("executes an approved MCP tool call", async () => {
+        fixture.setPermissionResponder(createMcpPermissionResponder(ApprovalOptionId.AllowOnce));
+
+        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();
+    });
+
+    it("ends turn when MCP tool call is rejected", async () => {
+        fixture.setPermissionResponder(createMcpPermissionResponder(ApprovalOptionId.RejectOnce));
+
+        expectEndTurn(await fixture.connection.prompt({
+            sessionId,
+            prompt: [{
+                type: "text",
+                text: `Use the ${MCP_SERVER_NAME} MCP echo tool with message "${MCP_ECHO_MESSAGE}". Stop if the tool call is rejected.`,
+            }],
+        }));
+        expectMcpToolPermissionRequest();
+    });
+});

src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts

@@ -21,7 +21,7 @@ export interface TestSkill {
 export interface SpawnedAgentFixture {
     readonly connection: acp.ClientSideConnection;
     readonly workspaceDir: string;
-    createSession(): Promise<acp.NewSessionResponse>;
+    createSession(mcpServers?: acp.McpServer[]): Promise<acp.NewSessionResponse>;
     restart(): Promise<SpawnedAgentFixture>;
     writeSkill(skill: TestSkill): void;
     setPermissionResponder(responder: PermissionResponder): void;
@@ -159,10 +159,10 @@ class SpawnedAgentFixtureImpl implements SpawnedAgentFixture {
         return this.paths.workspaceDir;
     }
 
-    async createSession(): Promise<acp.NewSessionResponse> {
+    async createSession(mcpServers: acp.McpServer[] = []): Promise<acp.NewSessionResponse> {
         return await this.connection.newSession({
             cwd: this.workspaceDir,
-            mcpServers: [],
+            mcpServers,
         });
     }
 
@@ -263,13 +263,20 @@ function printLogDirectory(logDirectory: string): void {
         .filter((entry) => entry.isFile())
         .forEach((entry) => {
             const logFilePath = path.join(logDirectory, entry.name);
-            const content = fs.readFileSync(logFilePath, "utf8").trim();
+            const content = redactLogSecrets(fs.readFileSync(logFilePath, "utf8").trim());
             console.log(`[APP_SERVER_LOGS] Logs from ${logFilePath}:`);
             console.log(content.length > 0 ? content : "[APP_SERVER_LOGS] Log file is empty");
             console.log("------");
         });
 }
 
+function redactLogSecrets(content: string): string {
+    return content
+        .replace(/("apiKey"\s*:\s*")[^"]+(")/g, "$1[REDACTED]$2")
+        .replace(/("Authorization"\s*:\s*")Bearer [^"]+(")/gi, "$1Bearer [REDACTED]$2")
+        .replace(/(Incorrect API key provided: )[^.,\s]+/g, "$1[REDACTED]");
+}
+
 async function waitForProcessExit(proc: ChildProcessWithoutNullStreams, timeoutMs: number): Promise<boolean> {
     if (proc.exitCode !== null || proc.signalCode !== null) {
         return true;