Add more e2e mcp approval tests

Author: ishulgin

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

@@ -37,8 +37,13 @@ function createMcpPermissionResponse(optionId: McpApprovalOptionIdValue | null):
     return {outcome: {outcome: "selected", optionId}};
 }
 
-function createMcpPermissionResponder(optionId: McpApprovalOptionIdValue): PermissionResponder {
-    return (request) => createMcpPermissionResponse(isMcpPermissionRequest(request) ? optionId : null);
+function createMcpPermissionResponder(...optionIds: McpApprovalOptionIdValue[]): PermissionResponder {
+    const queue = [...optionIds];
+    return (request) => createMcpPermissionResponse(
+        isMcpPermissionRequest(request)
+            ? queue.shift() ?? McpApprovalOptionId.Decline
+            : null,
+    );
 }
 
 describeE2E("E2E MCP approval tests", () => {
@@ -52,30 +57,40 @@ describeE2E("E2E MCP approval tests", () => {
         await fixture.dispose();
     });
 
-    function expectMcpToolPermissionRequest(sessionId: string): 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(McpApprovalOptionId.AllowOnce));
+    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};
+    }
 
+    async function expectEchoToolReply(sessionId: string, message: string): Promise<void> {
         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}`),
+            `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(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);
+        }
+    }
+
+    it("executes an approved MCP tool call", async () => {
+        fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.AllowOnce));
+        const {sessionId, invocationMarkerPath} = await createMcpSession();
+
+        await expectEchoToolReply(sessionId, MCP_ECHO_MESSAGE);
         expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe(MCP_ECHO_MESSAGE);
-        expectMcpToolPermissionRequest(sessionId);
+        expectMcpPermissionRequestCount(sessionId, 1);
     });
 
     it("ends turn when MCP tool call is rejected", async () => {
         fixture.setPermissionResponder(createMcpPermissionResponder(McpApprovalOptionId.Decline));
-        const invocationMarkerPath = path.join(fixture.workspaceDir, `mcp-tool-invocation-${crypto.randomUUID()}.txt`);
-        const sessionId = (await fixture.createSession([createMcpServer(invocationMarkerPath)])).sessionId;
+        const {sessionId, invocationMarkerPath} = await createMcpSession();
 
         expectEndTurn(await fixture.connection.prompt({
             sessionId,
@@ -85,6 +100,34 @@ describeE2E("E2E MCP approval tests", () => {
             }],
         }));
         expect(fs.existsSync(invocationMarkerPath)).toBe(false);
-        expectMcpToolPermissionRequest(sessionId);
+        expectMcpPermissionRequestCount(sessionId, 1);
+    });
+
+    it("skips subsequent approvals in the same session when allow_session is selected", async () => {
+        fixture.setPermissionResponder(
+            createMcpPermissionResponder(McpApprovalOptionId.AllowSession, McpApprovalOptionId.Decline),
+        );
+        const {sessionId, invocationMarkerPath} = await createMcpSession();
+
+        await expectEchoToolReply(sessionId, "session approval first");
+        await expectEchoToolReply(sessionId, "session approval second");
+
+        expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe("session approval second");
+        expectMcpPermissionRequestCount(sessionId, 1);
+    });
+
+    // This case only asserts observed same-session approval.
+    // codex-acp adapter produces persist metadata for MCP tool approvals, but its persistence behavior depends on upstream.
+    it("skips subsequent approvals in the same session when allow_always is selected", async () => {
+        fixture.setPermissionResponder(
+            createMcpPermissionResponder(McpApprovalOptionId.AllowAlways, McpApprovalOptionId.Decline),
+        );
+        const {sessionId, invocationMarkerPath} = await createMcpSession();
+
+        await expectEchoToolReply(sessionId, "always approval first");
+        await expectEchoToolReply(sessionId, "always approval second");
+
+        expect(fs.readFileSync(invocationMarkerPath, "utf8")).toBe("always approval second");
+        expectMcpPermissionRequestCount(sessionId, 1);
     });
 });