Add more e2e tests

src/ApprovalOptionId.ts

@@ -0,0 +1,7 @@
+export const ApprovalOptionId = {
+    AllowOnce: "allow_once",
+    AllowAlways: "allow_always",
+    RejectOnce: "reject_once",
+} as const;
+
+export type ApprovalOptionId = typeof ApprovalOptionId[keyof typeof ApprovalOptionId];

src/CodexApprovalHandler.ts

@@ -10,11 +10,12 @@ import type {
 import type {ToolCallContent} from "@agentclientprotocol/sdk/dist/schema/types.gen";
 import {logger} from "./Logger";
 import {stripShellPrefix} from "./CodexEventHandler";
+import {ApprovalOptionId} from "./ApprovalOptionId";
 
 const APPROVAL_OPTIONS: acp.PermissionOption[] = [
-    { optionId: "allow_once", name: "Allow Once", kind: "allow_once" },
-    { optionId: "allow_always", name: "Allow for Session", kind: "allow_always" },
-    { optionId: "reject_once", name: "Reject", kind: "reject_once" },
+    { optionId: ApprovalOptionId.AllowOnce, name: "Allow Once", kind: "allow_once" },
+    { optionId: ApprovalOptionId.AllowAlways, name: "Allow for Session", kind: "allow_always" },
+    { optionId: ApprovalOptionId.RejectOnce, name: "Reject", kind: "reject_once" },
 ];
 
 export class CodexApprovalHandler implements ApprovalHandler {
@@ -113,9 +114,9 @@ export class CodexApprovalHandler implements ApprovalHandler {
         }
 
         const optionId = response.outcome.optionId;
-        if (optionId === "allow_once") {
+        if (optionId === ApprovalOptionId.AllowOnce) {
             return { decision: "accept" };
-        } else if (optionId === "allow_always") {
+        } else if (optionId === ApprovalOptionId.AllowAlways) {
             return { decision: "acceptForSession" };
         } else {
             return { decision: "decline" };
@@ -130,9 +131,9 @@ export class CodexApprovalHandler implements ApprovalHandler {
         }
 
         const optionId = response.outcome.optionId;
-        if (optionId === "allow_once") {
+        if (optionId === ApprovalOptionId.AllowOnce) {
             return { decision: "accept" };
-        } else if (optionId === "allow_always") {
+        } else if (optionId === ApprovalOptionId.AllowAlways) {
             return { decision: "acceptForSession" };
         } else {
             return { decision: "cancel" };

src/CodexElicitationHandler.ts

@@ -9,17 +9,16 @@ import type {
     McpServerElicitationRequestResponse,
 } from "./app-server/v2";
 import { logger } from "./Logger";
+import { ApprovalOptionId } from "./ApprovalOptionId";
 
 // Standard elicitation options (non-tool-call approval).
 const ELICITATION_OPTIONS: acp.PermissionOption[] = [
     { optionId: "accept", name: "Accept", kind: "allow_once" },
     { optionId: "decline", name: "Decline", kind: "reject_once" },
 ];
 
-// Option IDs used for MCP tool call approval persist choices.
-const OPTION_ALLOW_ONCE = "allow_once";
+// Option ID unique to elicitation persist choices — not part of the shared ApprovalOptionId set.
 const OPTION_ALLOW_SESSION = "allow_session";
-const OPTION_ALLOW_ALWAYS = "allow_always";
 
 type PersistValue = "session" | "always";
 
@@ -57,13 +56,13 @@ function isMcpToolCallApproval(meta: unknown): boolean {
  */
 function buildToolApprovalOptions(persistOptions: Set<PersistValue>): acp.PermissionOption[] {
     const options: acp.PermissionOption[] = [
-        { optionId: OPTION_ALLOW_ONCE, name: "Allow", kind: "allow_once" },
+        { optionId: ApprovalOptionId.AllowOnce, name: "Allow", kind: "allow_once" },
     ];
     if (persistOptions.has("session")) {
         options.push({ optionId: OPTION_ALLOW_SESSION, name: "Allow for This Session", kind: "allow_always" });
     }
     if (persistOptions.has("always")) {
-        options.push({ optionId: OPTION_ALLOW_ALWAYS, name: "Allow and Don't Ask Again", kind: "allow_always" });
+        options.push({ optionId: ApprovalOptionId.AllowAlways, name: "Allow and Don't Ask Again", kind: "allow_always" });
     }
     options.push({ optionId: "decline", name: "Decline", kind: "reject_once" });
     return options;
@@ -215,10 +214,10 @@ export class CodexElicitationHandler implements ElicitationHandler {
         if (optionId === OPTION_ALLOW_SESSION) {
             return { action: "accept", content: null, _meta: { persist: "session" } };
         }
-        if (optionId === OPTION_ALLOW_ALWAYS) {
+        if (optionId === ApprovalOptionId.AllowAlways) {
             return { action: "accept", content: null, _meta: { persist: "always" } };
         }
-        if (optionId === OPTION_ALLOW_ONCE || optionId === "accept") {
+        if (optionId === ApprovalOptionId.AllowOnce || optionId === "accept") {
             return { action: "accept", content: null, _meta: null };
         }
         return { action: "decline", content: null, _meta: null };

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

@@ -0,0 +1,56 @@
+import fs from "node:fs";
+import path from "node:path";
+import {afterEach, beforeEach, expect, it} from "vitest";
+import {ApprovalOptionId} from "../../../ApprovalOptionId";
+import {
+    createPermissionResponder,
+    createReadOnlyFixture,
+    describeE2E,
+    type SpawnedAgentFixture,
+} from "./acp-e2e-test-utils";
+
+const FILE_NAME = "approval-file.txt";
+const FILE_CONTENT = "file approval e2e";
+
+describeE2E("E2E file approval tests", () => {
+    let fixture: SpawnedAgentFixture;
+    let sessionId: string;
+
+    beforeEach(async () => {
+        fixture = await createReadOnlyFixture();
+        sessionId = (await fixture.createSession()).sessionId;
+    });
+
+    afterEach(async () => {
+        await fixture.dispose();
+    });
+
+    async function expectFileApproval(
+        optionId: ApprovalOptionId,
+        expectedStopReason: "end_turn" | "cancelled",
+    ): Promise<void> {
+        fixture.setPermissionResponder(createPermissionResponder("edit", optionId));
+        const response = await fixture.connection.prompt({
+            sessionId,
+            prompt: [{
+                type: "text",
+                text: `Create ${FILE_NAME} by editing files directly. Content must be exactly: ${FILE_CONTENT}. Do not use shell commands, and stop if the edit is rejected.`,
+            }],
+        });
+        expect(response.stopReason).toBe(expectedStopReason);
+        expect(fixture.readPermissionRequests(sessionId, "edit").length).toBe(1);
+        expect(fixture.readPermissionRequests(sessionId, "execute").length).toBe(0);
+    }
+
+    it("applies approved file edits", async () => {
+        await expectFileApproval(ApprovalOptionId.AllowOnce, "end_turn");
+        const filePath = path.join(fixture.workspaceDir, FILE_NAME);
+        expect(fs.existsSync(filePath)).toBe(true);
+        expect(fs.readFileSync(filePath, "utf8").trim()).toBe(FILE_CONTENT);
+    });
+
+    it("does not apply rejected file edits", async () => {
+        await expectFileApproval(ApprovalOptionId.RejectOnce, "cancelled");
+        expect(fs.existsSync(path.join(fixture.workspaceDir, FILE_NAME))).toBe(false);
+    });
+});

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

@@ -0,0 +1,71 @@
+import fs from "node:fs";
+import path from "node:path";
+import {afterEach, beforeEach, expect, it} from "vitest";
+import {ApprovalOptionId} from "../../../ApprovalOptionId";
+import {
+    createPermissionResponse,
+    createPermissionResponder,
+    createReadOnlyFixture,
+    describeE2E,
+    expectEndTurn,
+    type SpawnedAgentFixture,
+} from "./acp-e2e-test-utils";
+
+const FIRST_FILE_NAME = "approval-first.txt";
+const SECOND_FILE_NAME = "approval-second.txt";
+const COMMAND = `if [ -e ${FIRST_FILE_NAME} ]; then touch ${SECOND_FILE_NAME}; else touch ${FIRST_FILE_NAME}; fi`;
+
+describeE2E("E2E shell approval tests", () => {
+    let fixture: SpawnedAgentFixture;
+    let sessionId: string;
+
+    beforeEach(async () => {
+        fixture = await createReadOnlyFixture();
+        sessionId = (await fixture.createSession()).sessionId;
+    });
+
+    afterEach(async () => {
+        await fixture.dispose();
+    });
+
+    async function promptShellCommandTwice(): Promise<void> {
+        for (const text of [
+            `Use your shell tool to run exactly \`${COMMAND}\`.`,
+            `Use your shell tool to run exactly the same command again: \`${COMMAND}\`.`,
+        ]) {
+            expectEndTurn(await fixture.connection.prompt({
+                sessionId,
+                prompt: [{type: "text", text}],
+            }));
+        }
+    }
+
+    it("prompts for every command when allow_once is selected", async () => {
+        const responses = [ApprovalOptionId.AllowOnce, ApprovalOptionId.RejectOnce];
+        fixture.setPermissionResponder((request) => createPermissionResponse(
+            request.toolCall.kind === "execute"
+                ? responses.shift() ?? ApprovalOptionId.RejectOnce
+                : null
+        ));
+        await promptShellCommandTwice();
+        expect(fs.existsSync(path.join(fixture.workspaceDir, FIRST_FILE_NAME))).toBe(true);
+        expect(fs.existsSync(path.join(fixture.workspaceDir, SECOND_FILE_NAME))).toBe(false);
+        expect(fixture.readPermissionRequests(sessionId, "execute").length).toBe(2);
+    });
+
+    it("skips subsequent approvals when allow_always is selected", async () => {
+        fixture.setPermissionResponder(createPermissionResponder("execute", ApprovalOptionId.AllowAlways));
+        await promptShellCommandTwice();
+        expect(fs.existsSync(path.join(fixture.workspaceDir, FIRST_FILE_NAME))).toBe(true);
+        expect(fs.existsSync(path.join(fixture.workspaceDir, SECOND_FILE_NAME))).toBe(true);
+        expect(fixture.readPermissionRequests(sessionId, "execute").length).toBe(1);
+    });
+
+    it("prompts for every command when reject_once is selected", async () => {
+        fixture.setPermissionResponder(createPermissionResponder("execute", ApprovalOptionId.RejectOnce));
+        await promptShellCommandTwice();
+        expect(fs.existsSync(path.join(fixture.workspaceDir, FIRST_FILE_NAME))).toBe(false);
+        expect(fs.existsSync(path.join(fixture.workspaceDir, SECOND_FILE_NAME))).toBe(false);
+        expect(fixture.readPermissionRequests(sessionId, "execute").length).toBe(2);
+    });
+});