Isolate tests with temporary CODEX_HOME

ishulgin · ishulgin · commit 2881b8eae8f8 · 2026-04-20T12:21:18.000+02:00
diff --git a/src/CodexJsonRpcConnection.ts b/src/CodexJsonRpcConnection.ts
@@ -11,10 +11,11 @@ export interface CodexConnection {
     readonly process: ChildProcessWithoutNullStreams;
 }
 
-export function startCodexConnection(codexPath: string): CodexConnection {
+export function startCodexConnection(codexPath: string, env?: NodeJS.ProcessEnv): CodexConnection {
+    const spawnEnv = env ?? process.env;
     const codex: ChildProcessWithoutNullStreams = process.platform === 'win32'
-        ? spawn(`"${codexPath}" app-server`, { shell: true })
-        : spawn(codexPath, ['app-server']);
+        ? spawn(`"${codexPath}" app-server`, { shell: true, env: spawnEnv })
+        : spawn(codexPath, ['app-server'], { env: spawnEnv });
 
     attachLogs(codex);
 
diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts
@@ -1,9 +1,6 @@
 // noinspection ES6RedundantAwait
 
 import {describe, expect, it, vi, beforeEach} from 'vitest';
-import fs from "node:fs";
-import os from "node:os";
-import path from "node:path";
 import type {CodexAuthRequest} from "../../CodexAuthMethod";
 import type * as acp from "@agentclientprotocol/sdk";
 import {createTestFixture, createCodexMockTestFixture, createTestSessionState, type TestFixture} from "../acp-test-utils";
@@ -14,46 +11,6 @@ import type {ListMcpServerStatusResponse, Model, SkillsListResponse} from "../..
 import type {RateLimitsMap} from "../../RateLimitsMap";
 import {ModelId} from "../../ModelId";
 
-const CODEX_HOME_ENV = "CODEX_HOME";
-
-async function overrideCodexHome<T>(configToml: string, run: () => Promise<T>): Promise<T> {
-    const previousCodexHome = process.env[CODEX_HOME_ENV];
-    const codexHome = fs.mkdtempSync(path.join(os.tmpdir(), "codex-acp-codex-home-"));
-    fs.writeFileSync(path.join(codexHome, "config.toml"), configToml, "utf8");
-    process.env[CODEX_HOME_ENV] = codexHome;
-
-    try {
-        return await run();
-    } finally {
-        if (previousCodexHome === undefined) {
-            delete process.env[CODEX_HOME_ENV];
-        } else {
-            process.env[CODEX_HOME_ENV] = previousCodexHome;
-        }
-        await removeDirectoryWithRetry(codexHome);
-    }
-}
-
-async function removeDirectoryWithRetry(directory: string): Promise<void> {
-    let lastError: NodeJS.ErrnoException | null = null;
-    for (let attempt = 0; attempt < 5; attempt += 1) {
-        try {
-            fs.rmSync(directory, { recursive: true, force: true });
-            return;
-        } catch (error) {
-            const err = error as NodeJS.ErrnoException;
-            if (err.code !== "ENOTEMPTY" && err.code !== "EBUSY") {
-                throw err;
-            }
-            lastError = err;
-            await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1)));
-        }
-    }
-    if (lastError) {
-        throw lastError;
-    }
-}
-
 describe('ACP server test', { timeout: 40_000 }, () => {
 
     let fixture: TestFixture;
@@ -65,126 +22,119 @@ describe('ACP server test', { timeout: 40_000 }, () => {
     const ignoredFields = ["thread", "cwd", "id", "createdAt", "path", "threadId", "userAgent", "sandbox",  "conversationId", "origins", "supportedReasoningEfforts", "reasoningEffort", "model", "readOnlyAccess", "approvalsReviewer"];
 
     it('should throw error without authentication', async () => {
-        await overrideCodexHome('cli_auth_credentials_store = "file"', async () => {
-            const authFixture = createTestFixture();
-            const codexAcpAgent = authFixture.getCodexAcpAgent();
+        const authFixture = createTestFixture();
+        const codexAcpAgent = authFixture.getCodexAcpAgent();
 
-            await codexAcpAgent.initialize({protocolVersion: 1});
-            await authFixture.getCodexAcpClient().logout();
-            authFixture.clearCodexConnectionDump();
+        await codexAcpAgent.initialize({protocolVersion: 1});
+        await authFixture.getCodexAcpClient().logout();
+        authFixture.clearCodexConnectionDump();
 
-            await expect(
-                codexAcpAgent.newSession({cwd: "", mcpServers: []})
-            ).rejects.toThrow("Authentication required");
+        await expect(
+            codexAcpAgent.newSession({cwd: "", mcpServers: []})
+        ).rejects.toThrow("Authentication required");
 
-            const transportDump = authFixture.getCodexConnectionDump(ignoredFields);
-            await expect(transportDump).toMatchFileSnapshot("data/auth-failed.json");
-        });
+        const transportDump = authFixture.getCodexConnectionDump(ignoredFields);
+        await expect(transportDump).toMatchFileSnapshot("data/auth-failed.json");
     });
 
     it('should authenticate with key', async () => {
-        // In sandboxed environments Codex may fail when trying to write to the OS keychain (`Operation not permitted`).
-        await overrideCodexHome('cli_auth_credentials_store = "file"', async () => {
-            const keyFixture = createTestFixture();
-            const codexAcpAgent = keyFixture.getCodexAcpAgent();
-
-            await codexAcpAgent.initialize({protocolVersion: 1});
-            await keyFixture.getCodexAcpClient().logout();
-
-
-            const unauthenticatedResponse = await keyFixture.getCodexAcpAgent().extMethod("authentication/status", {});
-            expect(unauthenticatedResponse).toEqual({type: "unauthenticated"});
-
-            keyFixture.clearCodexConnectionDump();
-
-            const authRequest: CodexAuthRequest = { methodId: "api-key", _meta: { "api-key": { apiKey: "TOKEN" }}}
-            await codexAcpAgent.authenticate(authRequest);
-            const newSessionResponse = await codexAcpAgent.newSession({cwd: "", mcpServers: []});
-            expect(newSessionResponse.sessionId).toBeDefined()
-
-            const transportEvents = keyFixture.getCodexConnectionEvents([...ignoredFields, "upgrade"]);
-            const transportMethods = transportEvents.flatMap(event => "method" in event ? [event.method] : []);
-            const loginRequest = transportEvents.find(event =>
-                event.eventType === "request" &&
-                "method" in event &&
-                event.method === "account/login/start"
-            );
-            const loginResponse = transportEvents.find(event =>
-                event.eventType === "response" &&
-                "type" in event &&
-                event.type === "apiKey"
-            );
-            const threadStartResponse = transportEvents.find(event =>
-                event.eventType === "response" &&
-                "modelProvider" in event &&
-                "approvalPolicy" in event
-            );
-            expect(transportMethods).toEqual([
-                "account/login/start",
-                "account/read",
-                "account/updated",
-                "thread/start",
-                "model/list",
-                "thread/started",
-                "account/read",
-                "skills/list",
-            ]);
-            expect(loginRequest).toEqual({
-                eventType: "request",
-                method: "account/login/start",
-                params: {
-                    type: "apiKey",
-                    apiKey: "TOKEN",
-                }
-            });
-            expect(loginResponse).toEqual({
-                eventType: "response",
+        const keyFixture = createTestFixture();
+        const codexAcpAgent = keyFixture.getCodexAcpAgent();
+
+        await codexAcpAgent.initialize({protocolVersion: 1});
+        await keyFixture.getCodexAcpClient().logout();
+
+
+        const unauthenticatedResponse = await keyFixture.getCodexAcpAgent().extMethod("authentication/status", {});
+        expect(unauthenticatedResponse).toEqual({type: "unauthenticated"});
+
+        keyFixture.clearCodexConnectionDump();
+
+        const authRequest: CodexAuthRequest = { methodId: "api-key", _meta: { "api-key": { apiKey: "TOKEN" }}}
+        await codexAcpAgent.authenticate(authRequest);
+        const newSessionResponse = await codexAcpAgent.newSession({cwd: "", mcpServers: []});
+        expect(newSessionResponse.sessionId).toBeDefined()
+
+        const transportEvents = keyFixture.getCodexConnectionEvents([...ignoredFields, "upgrade"]);
+        const transportMethods = transportEvents.flatMap(event => "method" in event ? [event.method] : []);
+        const loginRequest = transportEvents.find(event =>
+            event.eventType === "request" &&
+            "method" in event &&
+            event.method === "account/login/start"
+        );
+        const loginResponse = transportEvents.find(event =>
+            event.eventType === "response" &&
+            "type" in event &&
+            event.type === "apiKey"
+        );
+        const threadStartResponse = transportEvents.find(event =>
+            event.eventType === "response" &&
+            "modelProvider" in event &&
+            "approvalPolicy" in event
+        );
+        expect(transportMethods).toEqual([
+            "account/login/start",
+            "account/read",
+            "account/updated",
+            "thread/start",
+            "model/list",
+            "thread/started",
+            "account/read",
+            "skills/list",
+        ]);
+        expect(loginRequest).toEqual({
+            eventType: "request",
+            method: "account/login/start",
+            params: {
                 type: "apiKey",
-            });
-            expect(threadStartResponse).toMatchObject({
-                eventType: "response",
-                modelProvider: "openai",
-                approvalPolicy: "on-request",
-                approvalsReviewer: "approvalsReviewer",
-            });
-            const authenticatedResponse = await keyFixture.getCodexAcpAgent().extMethod("authentication/status", {});
-            expect(authenticatedResponse).toEqual({type: "api-key"});
-
-            await keyFixture.getCodexAcpAgent().extMethod("authentication/logout", {});
-            const logoutResponse = await keyFixture.getCodexAcpAgent().extMethod("authentication/status", {});
-            expect(logoutResponse).toEqual({type: "unauthenticated"});
+                apiKey: "TOKEN",
+            }
         });
+        expect(loginResponse).toEqual({
+            eventType: "response",
+            type: "apiKey",
+        });
+        expect(threadStartResponse).toMatchObject({
+            eventType: "response",
+            modelProvider: "openai",
+            approvalPolicy: "on-request",
+            approvalsReviewer: "approvalsReviewer",
+        });
+        const authenticatedResponse = await keyFixture.getCodexAcpAgent().extMethod("authentication/status", {});
+        expect(authenticatedResponse).toEqual({type: "api-key"});
+
+        await keyFixture.getCodexAcpAgent().extMethod("authentication/logout", {});
+        const logoutResponse = await keyFixture.getCodexAcpAgent().extMethod("authentication/status", {});
+        expect(logoutResponse).toEqual({type: "unauthenticated"});
     });
 
     it('should authenticate with a gateway', async () => {
-        await overrideCodexHome('cli_auth_credentials_store = "file"', async () => {
-            const gatewayFixture = createTestFixture();
-            const codexAcpAgent = gatewayFixture.getCodexAcpAgent();
-
-            await codexAcpAgent.initialize({protocolVersion: 1});
-            await gatewayFixture.getCodexAcpClient().logout();
-
-            const authRequest: CodexAuthRequest = {
-                methodId: "gateway",
-                _meta: {
-                    "gateway": {
-                        baseUrl: "https://www.example.com",
-                        headers: {
-                            "Custom-Auth-Header": "TOKEN"
-                        }
+        const gatewayFixture = createTestFixture();
+        const codexAcpAgent = gatewayFixture.getCodexAcpAgent();
+
+        await codexAcpAgent.initialize({protocolVersion: 1});
+        await gatewayFixture.getCodexAcpClient().logout();
+
+        const authRequest: CodexAuthRequest = {
+            methodId: "gateway",
+            _meta: {
+                "gateway": {
+                    baseUrl: "https://www.example.com",
+                    headers: {
+                        "Custom-Auth-Header": "TOKEN"
                     }
                 }
-            };
+            }
+        };
 
-            await codexAcpAgent.authenticate(authRequest);
-            expect(await gatewayFixture.getCodexAcpClient().authRequired()).toBe(false);
+        await codexAcpAgent.authenticate(authRequest);
+        expect(await gatewayFixture.getCodexAcpClient().authRequired()).toBe(false);
 
-            const authenticatedResponse = await gatewayFixture.getCodexAcpAgent().extMethod("authentication/status", {});
-            expect(authenticatedResponse).toEqual({type: "gateway", name: "custom-gateway"});
+        const authenticatedResponse = await gatewayFixture.getCodexAcpAgent().extMethod("authentication/status", {});
+        expect(authenticatedResponse).toEqual({type: "gateway", name: "custom-gateway"});
 
-            const newSessionResponse = await codexAcpAgent.newSession({cwd: "", mcpServers: []});
-            expect(newSessionResponse.sessionId).toBeDefined()
-        });
+        const newSessionResponse = await codexAcpAgent.newSession({cwd: "", mcpServers: []});
+        expect(newSessionResponse.sessionId).toBeDefined()
     })
 
     it('prefetches session additional skill roots before thread start', async () => {
diff --git a/src/__tests__/acp-test-utils.ts b/src/__tests__/acp-test-utils.ts
@@ -7,6 +7,7 @@ import type {ServerNotification} from "../app-server";
 import type {MessageConnection} from "vscode-jsonrpc/node";
 import path from "node:path";
 import fs from "node:fs";
+import os from "node:os";
 import {AgentMode} from "../AgentMode";
 import {expect, vi} from "vitest";
 
@@ -153,14 +154,41 @@ export function createTestFixture(): TestFixture {
         throw new Error(`Codex binary not found at ${pathToCodex}. Did you run 'npm install'?`);
     }
 
-    const codexConnection = startCodexConnection(pathToCodex);
+    const codexHome = createTestCodexHome();
+    const codexConnection = startCodexConnection(pathToCodex, {
+        ...process.env,
+        CODEX_HOME: codexHome,
+    });
+    codexConnection.process.on("exit", () => {
+        removeDirectoryWithRetry(codexHome);
+    });
 
     return createBaseTestFixture({
         connection: codexConnection.connection,
         getExitCode: () => codexConnection.process.exitCode
     });
 }
 
+function createTestCodexHome(): string {
+    const codexHome = fs.mkdtempSync(path.join(os.tmpdir(), "codex-acp-codex-home-"));
+    fs.writeFileSync(path.join(codexHome, "config.toml"), 'cli_auth_credentials_store = "file"\n', "utf8");
+    return codexHome;
+}
+
+function removeDirectoryWithRetry(directory: string): void {
+    for (let attempt = 0; attempt < 5; attempt += 1) {
+        try {
+            fs.rmSync(directory, { recursive: true, force: true });
+            return;
+        } catch (error) {
+            const err = error as NodeJS.ErrnoException;
+            if (err.code !== "ENOTEMPTY" && err.code !== "EBUSY") {
+                return;
+            }
+        }
+    }
+}
+
 export interface CodexMockTestFixture extends TestFixture {
     sendServerNotification(notification: ServerNotification | Record<string, unknown>): void,
     sendServerRequest<T>(method: string, params: unknown): Promise<T>,
