From d508ca374ba38aae966206feb611d9611a916e3b Mon Sep 17 00:00:00 2001 From: "Ilia.Shulgin" Date: Fri, 24 Apr 2026 11:39:29 +0200 Subject: [PATCH 1/6] Add e2e test for gateway auth type --- .../CodexACPAgent/e2e/acp-e2e-test-utils.ts | 78 ++++++++++++++----- .../CodexACPAgent/e2e/acp-e2e.test.ts | 14 ++++ 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts index 9f74d65e..eb30762e 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts @@ -71,6 +71,64 @@ export async function createAuthenticatedFixture( runtimePaths = createTemporaryRuntimePaths() ): Promise { const apiKey = requireLiveApiKey(); + return await createSpawnedFixture(async (connection, authMethods) => { + if (!authMethods.some((method) => method.id === "api-key")) { + throw new Error("API key authentication is not available."); + } + + await connection.authenticate({ + methodId: "api-key", + _meta: { + "api-key": { + apiKey, + }, + }, + }); + + const authenticationStatus = await getAuthenticationStatus(connection); + if (authenticationStatus["type"] !== "api-key") { + throw new Error(`Unexpected authentication status: ${JSON.stringify(authenticationStatus)}`); + } + }, runtimePaths); +} + +export interface GatewayFixtureOptions { + readonly baseUrl: string; + readonly headers?: Record; +} + +export async function createGatewayFixture( + options: GatewayFixtureOptions, + runtimePaths = createTemporaryRuntimePaths(), +): Promise { + return await createSpawnedFixture(async (connection, authMethods) => { + if (!authMethods.some((method) => method.id === "gateway")) { + throw new Error("Gateway authentication is not available."); + } + + await connection.authenticate({ + methodId: "gateway", + _meta: { + gateway: { + baseUrl: options.baseUrl, + headers: options.headers ?? {}, + }, + }, + }); + + const authenticationStatus = await getAuthenticationStatus(connection); + if (authenticationStatus["type"] !== "gateway" || authenticationStatus["name"] !== "custom-gateway") { + throw new Error(`Unexpected authentication status: ${JSON.stringify(authenticationStatus)}`); + } + }, runtimePaths); +} + +type Authenticator = (connection: acp.ClientSideConnection, authMethods: acp.AuthMethod[]) => Promise; + +async function createSpawnedFixture( + authenticate: Authenticator, + runtimePaths: RuntimePaths, +): Promise { const agentProcess = spawn("npm", ["run", "--silent", "start"], { cwd: process.cwd(), env: { @@ -101,23 +159,7 @@ export async function createAuthenticatedFixture( throw new Error(`Unexpected protocol version: ${initializeResponse.protocolVersion}`); } - if (!initializeResponse.authMethods?.some((method) => method.id === "api-key")) { - throw new Error("API key authentication is not available."); - } - - await connection.authenticate({ - methodId: "api-key", - _meta: { - "api-key": { - apiKey, - }, - }, - }); - - const authenticationStatus = await getAuthenticationStatus(connection); - if (authenticationStatus["type"] !== "api-key") { - throw new Error(`Unexpected authentication status: ${JSON.stringify(authenticationStatus)}`); - } + await authenticate(connection, initializeResponse.authMethods ?? []); const createSession = async (): Promise => { const newSessionResponse = await connection.newSession({ @@ -153,7 +195,7 @@ export async function createAuthenticatedFixture( }; } -function requireLiveApiKey(): string { +export function requireLiveApiKey(): string { const apiKey = process.env["CODEX_API_KEY"] ?? process.env["OPENAI_API_KEY"]; if (!apiKey) { throw new Error("Live integration test requires CODEX_API_KEY or OPENAI_API_KEY."); diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts index 19d7dfd8..409f8783 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts @@ -2,7 +2,9 @@ import {afterEach, expect, it} from "vitest"; import { createAuthenticatedFixture, createFixtureWithSkill, + createGatewayFixture, describeE2E, + requireLiveApiKey, type SpawnedAgentFixture, } from "./acp-e2e-test-utils"; @@ -24,6 +26,18 @@ describeE2E("E2E tests", () => { }); }); + it('returns model response when authenticated via gateway', async () => { + const apiKey = requireLiveApiKey(); + fixture = await createGatewayFixture({ + baseUrl: "https://api.openai.com/v1", + headers: {Authorization: `Bearer ${apiKey}`}, + }); + const session = await fixture.createSession(); + await session.expectPromptText("Reply with exactly integration-ok and nothing else.", (text) => { + expect(text.toLowerCase()).toContain("integration-ok"); + }); + }); + it("uses the selected session model for subsequent prompts", async () => { fixture = await createAuthenticatedFixture(); const session = await fixture.createSession(); From 5a763b2cfc200d4c65c7d310e46b801091a1281c Mon Sep 17 00:00:00 2001 From: "Ilia.Shulgin" Date: Fri, 24 Apr 2026 12:26:48 +0200 Subject: [PATCH 2/6] Add e2e tests for session modes and refactor status checks --- .../CodexACPAgent/e2e/acp-e2e-test-utils.ts | 20 +++++++-- .../CodexACPAgent/e2e/acp-e2e.test.ts | 41 ++++++++++++++++++- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts index eb30762e..cce9e92c 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts @@ -4,7 +4,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import {Readable, Writable} from "node:stream"; -import {describe, vi} from "vitest"; +import {describe, expect, vi} from "vitest"; import {removeDirectoryWithRetry} from "../../acp-test-utils"; export const RUN_E2E_TESTS = process.env["RUN_E2E_TESTS"] === "true"; @@ -25,6 +25,17 @@ export function describeE2E(name: string, factory: () => void, timeoutMs = DEFAU describe.skipIf(!RUN_E2E_TESTS)(name, {timeout: timeoutMs}, factory); } +export async function expectStatus( + session: SpawnedSessionFixture, + fields: Record, +): Promise { + await session.expectPromptText("/status", (text) => { + for (const [field, value] of Object.entries(fields)) { + expect(text).toContain(`**${field}:** ${String(value)}`); + } + }); +} + interface TestSkill { readonly name: string; readonly description: string; @@ -68,7 +79,8 @@ export async function createFixtureWithSkill(skill: TestSkill): Promise { const apiKey = requireLiveApiKey(); return await createSpawnedFixture(async (connection, authMethods) => { @@ -89,7 +101,7 @@ export async function createAuthenticatedFixture( if (authenticationStatus["type"] !== "api-key") { throw new Error(`Unexpected authentication status: ${JSON.stringify(authenticationStatus)}`); } - }, runtimePaths); + }, runtimePaths, extraEnv); } export interface GatewayFixtureOptions { @@ -128,6 +140,7 @@ type Authenticator = (connection: acp.ClientSideConnection, authMethods: acp.Aut async function createSpawnedFixture( authenticate: Authenticator, runtimePaths: RuntimePaths, + extraEnv?: NodeJS.ProcessEnv, ): Promise { const agentProcess = spawn("npm", ["run", "--silent", "start"], { cwd: process.cwd(), @@ -135,6 +148,7 @@ async function createSpawnedFixture( ...process.env, CODEX_HOME: runtimePaths.codexHome, APP_SERVER_LOGS: runtimePaths.appServerLogsDir, + ...extraEnv, }, stdio: ["pipe", "pipe", "pipe"], }); diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts index 409f8783..0a765b99 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts @@ -1,9 +1,11 @@ import {afterEach, expect, it} from "vitest"; +import {AgentMode} from "../../../AgentMode"; import { createAuthenticatedFixture, createFixtureWithSkill, createGatewayFixture, describeE2E, + expectStatus, requireLiveApiKey, type SpawnedAgentFixture, } from "./acp-e2e-test-utils"; @@ -60,8 +62,43 @@ describeE2E("E2E tests", () => { sessionId: session.response.sessionId, modelId: selectedModelId, }); - await session.expectPromptText("/status", (text) => { - expect(text).toContain(`**Model:** ${selectedModelId}`); + await expectStatus(session, {Model: selectedModelId}); + }); + + it("changes session mode via setSessionMode and reflects it in /status", async () => { + fixture = await createAuthenticatedFixture(); + const session = await fixture.createSession(); + + const modes = session.response.modes; + expect(modes?.currentModeId).toBe(AgentMode.DEFAULT_AGENT_MODE.id); + expect(modes?.availableModes.map((mode) => mode.id)).toEqual( + AgentMode.all().map((mode) => mode.id), + ); + + const targetMode = AgentMode.AgentFullAccess; + await fixture.connection.setSessionMode({ + sessionId: session.response.sessionId, + modeId: targetMode.id, + }); + + await expectStatus(session, { + Approval: targetMode.approvalPolicy, + Sandbox: targetMode.sandboxMode, + }); + }); + + it("respects INITIAL_AGENT_MODE when seeding the initial session mode", async () => { + const initialMode = AgentMode.ReadOnly; + fixture = await createAuthenticatedFixture(undefined, { + INITIAL_AGENT_MODE: initialMode.id, + }); + const session = await fixture.createSession(); + + expect(session.response.modes?.currentModeId).toBe(initialMode.id); + + await expectStatus(session, { + Approval: initialMode.approvalPolicy, + Sandbox: initialMode.sandboxMode, }); }); From 3a572af7b8a2f502845981ad5b658cc54848c0bb Mon Sep 17 00:00:00 2001 From: "Ilia.Shulgin" Date: Fri, 24 Apr 2026 17:30:55 +0200 Subject: [PATCH 3/6] Use minimal model in e2e tests --- .../CodexACPAgent/e2e/acp-e2e-test-utils.ts | 8 ++++++-- src/__tests__/acp-test-utils.ts | 13 ++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts index cce9e92c..c2307e95 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts @@ -5,7 +5,7 @@ import os from "node:os"; import path from "node:path"; import {Readable, Writable} from "node:stream"; import {describe, expect, vi} from "vitest"; -import {removeDirectoryWithRetry} from "../../acp-test-utils"; +import {removeDirectoryWithRetry, writeCodexHomeConfig} from "../../acp-test-utils"; export const RUN_E2E_TESTS = process.env["RUN_E2E_TESTS"] === "true"; const DEFAULT_E2E_SUITE_TIMEOUT_MS = 60_000; @@ -226,7 +226,11 @@ function createTemporaryRuntimePaths(): RuntimePaths { fs.mkdirSync(codexHome, {recursive: true}); fs.mkdirSync(workspaceDir, {recursive: true}); fs.mkdirSync(appServerLogsDir, {recursive: true}); - fs.writeFileSync(path.join(codexHome, "config.toml"), 'cli_auth_credentials_store = "file"\n', "utf8"); + writeCodexHomeConfig(codexHome, { + model: "gpt-5.2", + model_reasoning_effort: "none", + web_search: "disabled", + }); return { rootDir, diff --git a/src/__tests__/acp-test-utils.ts b/src/__tests__/acp-test-utils.ts index d702f0d2..a1629214 100644 --- a/src/__tests__/acp-test-utils.ts +++ b/src/__tests__/acp-test-utils.ts @@ -171,10 +171,21 @@ export function createTestFixture(): TestFixture { 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"); + writeCodexHomeConfig(codexHome); return codexHome; } +export function writeCodexHomeConfig(codexHome: string, extras: Record = {}): void { + const entries: Record = { + cli_auth_credentials_store: "file", + ...extras, + }; + const body = Object.entries(entries) + .map(([key, value]) => `${key} = "${value}"`) + .join("\n"); + fs.writeFileSync(path.join(codexHome, "config.toml"), body, "utf8"); +} + export function removeDirectoryWithRetry(directory: string): void { for (let attempt = 0; attempt < 5; attempt += 1) { try { From dffdd2908a2caf5ff46ed281d0f1637effcc0f46 Mon Sep 17 00:00:00 2001 From: "Ilia.Shulgin" Date: Tue, 28 Apr 2026 14:54:58 +0200 Subject: [PATCH 4/6] Add e2e for files/commands approval --- src/ApprovalOptionId.ts | 7 ++ src/CodexApprovalHandler.ts | 15 ++-- src/CodexElicitationHandler.ts | 13 ++-- .../e2e/acp-e2e-file-approval.test.ts | 57 +++++++++++++++ .../e2e/acp-e2e-shell-approval.test.ts | 72 +++++++++++++++++++ .../CodexACPAgent/e2e/acp-e2e-test-utils.ts | 72 +++++++++++++++++-- 6 files changed, 215 insertions(+), 21 deletions(-) create mode 100644 src/ApprovalOptionId.ts create mode 100644 src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts create mode 100644 src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts diff --git a/src/ApprovalOptionId.ts b/src/ApprovalOptionId.ts new file mode 100644 index 00000000..ccec90ed --- /dev/null +++ b/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]; diff --git a/src/CodexApprovalHandler.ts b/src/CodexApprovalHandler.ts index 3b0643c0..0eddf3ac 100644 --- a/src/CodexApprovalHandler.ts +++ b/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" }; diff --git a/src/CodexElicitationHandler.ts b/src/CodexElicitationHandler.ts index 1b90bcf2..732822bd 100644 --- a/src/CodexElicitationHandler.ts +++ b/src/CodexElicitationHandler.ts @@ -9,6 +9,7 @@ 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[] = [ @@ -16,10 +17,8 @@ const ELICITATION_OPTIONS: acp.PermissionOption[] = [ { 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): 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 }; diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts new file mode 100644 index 00000000..1ff2ab21 --- /dev/null +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts @@ -0,0 +1,57 @@ +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, + type SpawnedSessionFixture, +} 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 session: SpawnedSessionFixture; + + beforeEach(async () => { + fixture = await createReadOnlyFixture(); + session = await fixture.createSession(); + }); + + afterEach(async () => { + await fixture.dispose(); + }); + + async function expectFileApproval( + optionId: ApprovalOptionId, + expectedStopReason: "end_turn" | "cancelled", + ) { + fixture.setPermissionResponder(createPermissionResponder("edit", optionId)); + const response = await fixture.connection.prompt({ + sessionId: session.response.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(session.readPermissionRequests("edit").length).toBe(1); + expect(session.readPermissionRequests("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); + }); +}); diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts new file mode 100644 index 00000000..5aeb62e0 --- /dev/null +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts @@ -0,0 +1,72 @@ +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, + type SpawnedSessionFixture, +} 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 session: SpawnedSessionFixture; + + beforeEach(async () => { + fixture = await createReadOnlyFixture(); + session = await fixture.createSession(); + }); + + afterEach(async () => { + await fixture.dispose(); + }); + + async function promptShellCommandTwice() { + 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: session.response.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(session.readPermissionRequests("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(session.readPermissionRequests("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(session.readPermissionRequests("execute").length).toBe(2); + }); +}); diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts index c2307e95..d593bd1f 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts @@ -5,6 +5,8 @@ import os from "node:os"; import path from "node:path"; import {Readable, Writable} from "node:stream"; import {describe, expect, vi} from "vitest"; +import {AgentMode} from "../../../AgentMode"; +import {ApprovalOptionId} from "../../../ApprovalOptionId"; import {removeDirectoryWithRetry, writeCodexHomeConfig} from "../../acp-test-utils"; export const RUN_E2E_TESTS = process.env["RUN_E2E_TESTS"] === "true"; @@ -13,11 +15,38 @@ const DEFAULT_E2E_SUITE_TIMEOUT_MS = 60_000; export interface SpawnedSessionFixture { readonly response: acp.NewSessionResponse; expectPromptText(promptText: string, assertText: (text: string) => void, timeoutMs?: number): Promise; + readPermissionRequests(toolCallKind: acp.ToolKind): acp.RequestPermissionRequest[]; +} + +export function expectEndTurn(response: acp.PromptResponse): void { + expect(response.stopReason).toBe("end_turn"); +} + +export type PermissionResponder = ( + params: acp.RequestPermissionRequest, +) => acp.RequestPermissionResponse; + +export function createPermissionResponder( + expectedToolCallKind: acp.ToolKind, + optionId: ApprovalOptionId, +): PermissionResponder { + return (request) => createPermissionResponse( + request.toolCall.kind === expectedToolCallKind ? optionId : null + ); +} + +export function createPermissionResponse(optionId: ApprovalOptionId | null): acp.RequestPermissionResponse { + if (optionId === null) { + return {outcome: {outcome: "cancelled"}}; + } + return {outcome: {outcome: "selected", optionId}}; } export interface SpawnedAgentFixture { readonly connection: acp.ClientSideConnection; + readonly workspaceDir: string; createSession(): Promise; + setPermissionResponder(responder: PermissionResponder): void; dispose(): Promise; } @@ -51,11 +80,23 @@ interface RuntimePaths { class RecordingClient implements acp.Client { private readonly textBySessionId = new Map(); + private readonly permissionRequestsBySessionId = new Map(); + private permissionResponder: PermissionResponder = () => ({ + outcome: {outcome: "cancelled"}, + }); - async requestPermission(_params: acp.RequestPermissionRequest): Promise { - return { - outcome: {outcome: "cancelled"}, - }; + setPermissionResponder(responder: PermissionResponder): void { + this.permissionResponder = responder; + } + + async requestPermission(params: acp.RequestPermissionRequest): Promise { + let requests = this.permissionRequestsBySessionId.get(params.sessionId); + if (!requests) { + requests = []; + this.permissionRequestsBySessionId.set(params.sessionId, requests); + } + requests.push(params); + return this.permissionResponder(params); } async sessionUpdate(params: acp.SessionNotification): Promise { @@ -70,6 +111,14 @@ class RecordingClient implements acp.Client { readText(sessionId: string): string { return this.textBySessionId.get(sessionId) ?? ""; } + + readPermissionRequests( + sessionId: string, + toolCallKind: acp.ToolKind, + ): acp.RequestPermissionRequest[] { + const requests = this.permissionRequestsBySessionId.get(sessionId) ?? []; + return requests.filter((request) => request.toolCall.kind === toolCallKind); + } } export async function createFixtureWithSkill(skill: TestSkill): Promise { @@ -104,6 +153,10 @@ export async function createAuthenticatedFixture( }, runtimePaths, extraEnv); } +export async function createReadOnlyFixture(): Promise { + return await createAuthenticatedFixture(undefined, {INITIAL_AGENT_MODE: AgentMode.ReadOnly.id}); +} + export interface GatewayFixtureOptions { readonly baseUrl: string; readonly headers?: Record; @@ -186,12 +239,19 @@ async function createSpawnedFixture( async expectPromptText(promptText: string, assertText: (text: string) => void, timeoutMs = 30_000): Promise { await expectPromptTextForSession(connection, client, newSessionResponse.sessionId, promptText, assertText, timeoutMs); }, + readPermissionRequests(toolCallKind: acp.ToolKind): acp.RequestPermissionRequest[] { + return client.readPermissionRequests(newSessionResponse.sessionId, toolCallKind); + }, }; }; return { connection, + workspaceDir: runtimePaths.workspaceDir, createSession, + setPermissionResponder(responder: PermissionResponder): void { + client.setPermissionResponder(responder); + }, async dispose(): Promise { if (!agentProcess.stdin.destroyed && !agentProcess.stdin.writableEnded) { agentProcess.stdin.end(); @@ -281,9 +341,7 @@ async function expectPromptTextForSession( }], }); - if (promptResponse.stopReason !== "end_turn") { - throw new Error(`Unexpected stop reason: ${promptResponse.stopReason}`); - } + expectEndTurn(promptResponse); await vi.waitFor(() => { const sessionText = client.readText(sessionId); From 241febc11120266934e1e46ed199d440cd2a8c30 Mon Sep 17 00:00:00 2001 From: "Ilia.Shulgin" Date: Wed, 29 Apr 2026 10:47:07 +0200 Subject: [PATCH 5/6] Refactor e2e tests helpers 1. Extract abstractions from acp-e2e-test-utils.ts 2. Get rid of SpawnedSessionFixture, keeping SpawnedAgentFixture only 3. General cleanup and members re-ordering --- .../e2e/acp-e2e-file-approval.test.ts | 13 +- .../e2e/acp-e2e-shell-approval.test.ts | 15 +- .../CodexACPAgent/e2e/acp-e2e-test-utils.ts | 322 ++---------------- .../CodexACPAgent/e2e/acp-e2e.test.ts | 40 +-- .../e2e/permission-responders.ts | 22 ++ .../e2e/spawned-agent-fixture.ts | 260 ++++++++++++++ 6 files changed, 338 insertions(+), 334 deletions(-) create mode 100644 src/__tests__/CodexACPAgent/e2e/permission-responders.ts create mode 100644 src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts index 1ff2ab21..8e2484ff 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-file-approval.test.ts @@ -7,7 +7,6 @@ import { createReadOnlyFixture, describeE2E, type SpawnedAgentFixture, - type SpawnedSessionFixture, } from "./acp-e2e-test-utils"; const FILE_NAME = "approval-file.txt"; @@ -15,11 +14,11 @@ const FILE_CONTENT = "file approval e2e"; describeE2E("E2E file approval tests", () => { let fixture: SpawnedAgentFixture; - let session: SpawnedSessionFixture; + let sessionId: string; beforeEach(async () => { fixture = await createReadOnlyFixture(); - session = await fixture.createSession(); + sessionId = (await fixture.createSession()).sessionId; }); afterEach(async () => { @@ -29,18 +28,18 @@ describeE2E("E2E file approval tests", () => { async function expectFileApproval( optionId: ApprovalOptionId, expectedStopReason: "end_turn" | "cancelled", - ) { + ): Promise { fixture.setPermissionResponder(createPermissionResponder("edit", optionId)); const response = await fixture.connection.prompt({ - sessionId: session.response.sessionId, + 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(session.readPermissionRequests("edit").length).toBe(1); - expect(session.readPermissionRequests("execute").length).toBe(0); + expect(fixture.readPermissionRequests(sessionId, "edit").length).toBe(1); + expect(fixture.readPermissionRequests(sessionId, "execute").length).toBe(0); } it("applies approved file edits", async () => { diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts index 5aeb62e0..da026f52 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-shell-approval.test.ts @@ -9,7 +9,6 @@ import { describeE2E, expectEndTurn, type SpawnedAgentFixture, - type SpawnedSessionFixture, } from "./acp-e2e-test-utils"; const FIRST_FILE_NAME = "approval-first.txt"; @@ -18,24 +17,24 @@ const COMMAND = `if [ -e ${FIRST_FILE_NAME} ]; then touch ${SECOND_FILE_NAME}; e describeE2E("E2E shell approval tests", () => { let fixture: SpawnedAgentFixture; - let session: SpawnedSessionFixture; + let sessionId: string; beforeEach(async () => { fixture = await createReadOnlyFixture(); - session = await fixture.createSession(); + sessionId = (await fixture.createSession()).sessionId; }); afterEach(async () => { await fixture.dispose(); }); - async function promptShellCommandTwice() { + async function promptShellCommandTwice(): Promise { 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: session.response.sessionId, + sessionId, prompt: [{type: "text", text}], })); } @@ -51,7 +50,7 @@ describeE2E("E2E shell approval tests", () => { 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(session.readPermissionRequests("execute").length).toBe(2); + expect(fixture.readPermissionRequests(sessionId, "execute").length).toBe(2); }); it("skips subsequent approvals when allow_always is selected", async () => { @@ -59,7 +58,7 @@ describeE2E("E2E shell approval tests", () => { 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(session.readPermissionRequests("execute").length).toBe(1); + expect(fixture.readPermissionRequests(sessionId, "execute").length).toBe(1); }); it("prompts for every command when reject_once is selected", async () => { @@ -67,6 +66,6 @@ describeE2E("E2E shell approval tests", () => { 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(session.readPermissionRequests("execute").length).toBe(2); + expect(fixture.readPermissionRequests(sessionId, "execute").length).toBe(2); }); }); diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts index d593bd1f..da3a9220 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts @@ -1,136 +1,30 @@ import * as acp from "@agentclientprotocol/sdk"; -import {type ChildProcessWithoutNullStreams, spawn} from "node:child_process"; -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; -import {Readable, Writable} from "node:stream"; -import {describe, expect, vi} from "vitest"; +import {describe, expect} from "vitest"; import {AgentMode} from "../../../AgentMode"; -import {ApprovalOptionId} from "../../../ApprovalOptionId"; -import {removeDirectoryWithRetry, writeCodexHomeConfig} from "../../acp-test-utils"; +import {createSpawnedAgentFixture, type SpawnedAgentFixture} from "./spawned-agent-fixture"; + +export { + createPermissionResponder, + createPermissionResponse, + type PermissionResponder, +} from "./permission-responders"; +export { + type SpawnedAgentFixture, + type TestSkill, +} from "./spawned-agent-fixture"; export const RUN_E2E_TESTS = process.env["RUN_E2E_TESTS"] === "true"; const DEFAULT_E2E_SUITE_TIMEOUT_MS = 60_000; -export interface SpawnedSessionFixture { - readonly response: acp.NewSessionResponse; - expectPromptText(promptText: string, assertText: (text: string) => void, timeoutMs?: number): Promise; - readPermissionRequests(toolCallKind: acp.ToolKind): acp.RequestPermissionRequest[]; -} - -export function expectEndTurn(response: acp.PromptResponse): void { - expect(response.stopReason).toBe("end_turn"); -} - -export type PermissionResponder = ( - params: acp.RequestPermissionRequest, -) => acp.RequestPermissionResponse; - -export function createPermissionResponder( - expectedToolCallKind: acp.ToolKind, - optionId: ApprovalOptionId, -): PermissionResponder { - return (request) => createPermissionResponse( - request.toolCall.kind === expectedToolCallKind ? optionId : null - ); -} - -export function createPermissionResponse(optionId: ApprovalOptionId | null): acp.RequestPermissionResponse { - if (optionId === null) { - return {outcome: {outcome: "cancelled"}}; - } - return {outcome: {outcome: "selected", optionId}}; -} - -export interface SpawnedAgentFixture { - readonly connection: acp.ClientSideConnection; - readonly workspaceDir: string; - createSession(): Promise; - setPermissionResponder(responder: PermissionResponder): void; - dispose(): Promise; -} - export function describeE2E(name: string, factory: () => void, timeoutMs = DEFAULT_E2E_SUITE_TIMEOUT_MS): void { describe.skipIf(!RUN_E2E_TESTS)(name, {timeout: timeoutMs}, factory); } -export async function expectStatus( - session: SpawnedSessionFixture, - fields: Record, -): Promise { - await session.expectPromptText("/status", (text) => { - for (const [field, value] of Object.entries(fields)) { - expect(text).toContain(`**${field}:** ${String(value)}`); - } - }); -} - -interface TestSkill { - readonly name: string; - readonly description: string; - readonly body: string; -} - -interface RuntimePaths { - readonly rootDir: string; - readonly codexHome: string; - readonly workspaceDir: string; - readonly appServerLogsDir: string; -} - -class RecordingClient implements acp.Client { - private readonly textBySessionId = new Map(); - private readonly permissionRequestsBySessionId = new Map(); - private permissionResponder: PermissionResponder = () => ({ - outcome: {outcome: "cancelled"}, - }); - - setPermissionResponder(responder: PermissionResponder): void { - this.permissionResponder = responder; - } - - async requestPermission(params: acp.RequestPermissionRequest): Promise { - let requests = this.permissionRequestsBySessionId.get(params.sessionId); - if (!requests) { - requests = []; - this.permissionRequestsBySessionId.set(params.sessionId, requests); - } - requests.push(params); - return this.permissionResponder(params); - } - - async sessionUpdate(params: acp.SessionNotification): Promise { - if (params.update.sessionUpdate !== "agent_message_chunk" || params.update.content.type !== "text") { - return; - } - - const nextText = `${this.textBySessionId.get(params.sessionId) ?? ""}${params.update.content.text}`; - this.textBySessionId.set(params.sessionId, nextText); - } - - readText(sessionId: string): string { - return this.textBySessionId.get(sessionId) ?? ""; - } - - readPermissionRequests( - sessionId: string, - toolCallKind: acp.ToolKind, - ): acp.RequestPermissionRequest[] { - const requests = this.permissionRequestsBySessionId.get(sessionId) ?? []; - return requests.filter((request) => request.toolCall.kind === toolCallKind); - } -} - -export async function createFixtureWithSkill(skill: TestSkill): Promise { - const runtimePaths = createTemporaryRuntimePaths(); - writeSkill(runtimePaths.codexHome, skill); - return await createAuthenticatedFixture(runtimePaths); +export function expectEndTurn(response: acp.PromptResponse): void { + expect(response.stopReason).toBe("end_turn"); } -export async function createAuthenticatedFixture( - runtimePaths = createTemporaryRuntimePaths(), - extraEnv?: NodeJS.ProcessEnv, -): Promise { +export async function createAuthenticatedFixture(extraEnv?: NodeJS.ProcessEnv): Promise { const apiKey = requireLiveApiKey(); return await createSpawnedFixture(async (connection, authMethods) => { if (!authMethods.some((method) => method.id === "api-key")) { @@ -150,21 +44,16 @@ export async function createAuthenticatedFixture( if (authenticationStatus["type"] !== "api-key") { throw new Error(`Unexpected authentication status: ${JSON.stringify(authenticationStatus)}`); } - }, runtimePaths, extraEnv); + }, extraEnv); } export async function createReadOnlyFixture(): Promise { - return await createAuthenticatedFixture(undefined, {INITIAL_AGENT_MODE: AgentMode.ReadOnly.id}); -} - -export interface GatewayFixtureOptions { - readonly baseUrl: string; - readonly headers?: Record; + return await createAuthenticatedFixture({INITIAL_AGENT_MODE: AgentMode.ReadOnly.id}); } export async function createGatewayFixture( - options: GatewayFixtureOptions, - runtimePaths = createTemporaryRuntimePaths(), + baseUrl: string, + headers: Record, ): Promise { return await createSpawnedFixture(async (connection, authMethods) => { if (!authMethods.some((method) => method.id === "gateway")) { @@ -175,8 +64,8 @@ export async function createGatewayFixture( methodId: "gateway", _meta: { gateway: { - baseUrl: options.baseUrl, - headers: options.headers ?? {}, + baseUrl, + headers, }, }, }); @@ -185,33 +74,17 @@ export async function createGatewayFixture( if (authenticationStatus["type"] !== "gateway" || authenticationStatus["name"] !== "custom-gateway") { throw new Error(`Unexpected authentication status: ${JSON.stringify(authenticationStatus)}`); } - }, runtimePaths); + }); } type Authenticator = (connection: acp.ClientSideConnection, authMethods: acp.AuthMethod[]) => Promise; async function createSpawnedFixture( authenticate: Authenticator, - runtimePaths: RuntimePaths, extraEnv?: NodeJS.ProcessEnv, ): Promise { - const agentProcess = spawn("npm", ["run", "--silent", "start"], { - cwd: process.cwd(), - env: { - ...process.env, - CODEX_HOME: runtimePaths.codexHome, - APP_SERVER_LOGS: runtimePaths.appServerLogsDir, - ...extraEnv, - }, - stdio: ["pipe", "pipe", "pipe"], - }); - - const client = new RecordingClient(); - const output = Readable.toWeb(agentProcess.stdout) as ReadableStream; - const connection = new acp.ClientSideConnection( - () => client, - acp.ndJsonStream(Writable.toWeb(agentProcess.stdin), output) - ); + const fixture = createSpawnedAgentFixture(extraEnv); + const connection = fixture.connection; const initializeResponse = await connection.initialize({ protocolVersion: acp.PROTOCOL_VERSION, @@ -227,46 +100,7 @@ async function createSpawnedFixture( } await authenticate(connection, initializeResponse.authMethods ?? []); - - const createSession = async (): Promise => { - const newSessionResponse = await connection.newSession({ - cwd: runtimePaths.workspaceDir, - mcpServers: [], - }); - - return { - response: newSessionResponse, - async expectPromptText(promptText: string, assertText: (text: string) => void, timeoutMs = 30_000): Promise { - await expectPromptTextForSession(connection, client, newSessionResponse.sessionId, promptText, assertText, timeoutMs); - }, - readPermissionRequests(toolCallKind: acp.ToolKind): acp.RequestPermissionRequest[] { - return client.readPermissionRequests(newSessionResponse.sessionId, toolCallKind); - }, - }; - }; - - return { - connection, - workspaceDir: runtimePaths.workspaceDir, - createSession, - setPermissionResponder(responder: PermissionResponder): void { - client.setPermissionResponder(responder); - }, - async dispose(): Promise { - if (!agentProcess.stdin.destroyed && !agentProcess.stdin.writableEnded) { - agentProcess.stdin.end(); - } - - const exitedAfterStdinClose = await waitForProcessExit(agentProcess, 4_000); - if (!exitedAfterStdinClose && !agentProcess.killed) { - agentProcess.kill(); - await waitForProcessExit(agentProcess, 4_000); - } - - printLogDirectory(runtimePaths.appServerLogsDir); - removeDirectoryWithRetry(runtimePaths.rootDir); - }, - }; + return fixture; } export function requireLiveApiKey(): string { @@ -277,112 +111,6 @@ export function requireLiveApiKey(): string { return apiKey; } -function createTemporaryRuntimePaths(): RuntimePaths { - const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "codex-acp-integration-")); - const codexHome = path.join(rootDir, "codex-home"); - const workspaceDir = path.join(rootDir, "workspace"); - const appServerLogsDir = path.join(rootDir, "logs"); - - fs.mkdirSync(codexHome, {recursive: true}); - fs.mkdirSync(workspaceDir, {recursive: true}); - fs.mkdirSync(appServerLogsDir, {recursive: true}); - writeCodexHomeConfig(codexHome, { - model: "gpt-5.2", - model_reasoning_effort: "none", - web_search: "disabled", - }); - - return { - rootDir, - codexHome, - workspaceDir, - appServerLogsDir, - }; -} - -function writeSkill(codexHome: string, skill: TestSkill): void { - const skillDirectory = path.join(codexHome, "skills", skill.name); - fs.mkdirSync(skillDirectory, {recursive: true}); - fs.writeFileSync( - path.join(skillDirectory, "SKILL.md"), - [ - "---", - `name: ${skill.name}`, - `description: ${skill.description}`, - "metadata:", - ` short-description: ${skill.description}`, - "---", - "", - skill.body, - "", - ].join("\n"), - "utf8", - ); -} - async function getAuthenticationStatus(connection: acp.ClientSideConnection): Promise> { return await connection.extMethod("authentication/status", {}); } - -async function expectPromptTextForSession( - connection: acp.ClientSideConnection, - client: RecordingClient, - sessionId: string, - promptText: string, - assertText: (text: string) => void, - timeoutMs: number, -): Promise { - const previousText = client.readText(sessionId); - const promptResponse = await connection.prompt({ - sessionId, - prompt: [{ - type: "text", - text: promptText, - }], - }); - - expectEndTurn(promptResponse); - - await vi.waitFor(() => { - const sessionText = client.readText(sessionId); - const nextText = sessionText.slice(previousText.length); - assertText(nextText); - }, {timeout: timeoutMs}); -} - -function printLogDirectory(logDirectory: string): void { - fs.readdirSync(logDirectory, {withFileTypes: true}) - .filter((entry) => entry.isFile()) - .forEach((entry) => { - const logFilePath = path.join(logDirectory, entry.name); - const content = 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("------"); - }); -} - -async function waitForProcessExit(proc: ChildProcessWithoutNullStreams, timeoutMs: number): Promise { - if (proc.exitCode !== null || proc.signalCode !== null) { - return true; - } - - return await new Promise((resolve) => { - const timeout = setTimeout(() => { - cleanup(); - resolve(false); - }, timeoutMs); - - const cleanup = () => { - clearTimeout(timeout); - proc.off("exit", handleExit); - }; - - const handleExit = () => { - cleanup(); - resolve(true); - }; - - proc.once("exit", handleExit); - }); -} diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts index 0a765b99..97511e7c 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts @@ -2,10 +2,8 @@ import {afterEach, expect, it} from "vitest"; import {AgentMode} from "../../../AgentMode"; import { createAuthenticatedFixture, - createFixtureWithSkill, createGatewayFixture, describeE2E, - expectStatus, requireLiveApiKey, type SpawnedAgentFixture, } from "./acp-e2e-test-utils"; @@ -20,22 +18,19 @@ describeE2E("E2E tests", () => { } }); - it('returns model response', async () => { + it("returns model response", async () => { fixture = await createAuthenticatedFixture(); const session = await fixture.createSession(); - await session.expectPromptText("Reply with exactly integration-ok and nothing else.", (text) => { + await fixture.expectPromptText(session.sessionId, "Reply with exactly integration-ok and nothing else.", (text) => { expect(text.toLowerCase()).toContain("integration-ok"); }); }); - it('returns model response when authenticated via gateway', async () => { + it("returns model response when authenticated via gateway", async () => { const apiKey = requireLiveApiKey(); - fixture = await createGatewayFixture({ - baseUrl: "https://api.openai.com/v1", - headers: {Authorization: `Bearer ${apiKey}`}, - }); + fixture = await createGatewayFixture("https://api.openai.com/v1", {Authorization: `Bearer ${apiKey}`}); const session = await fixture.createSession(); - await session.expectPromptText("Reply with exactly integration-ok and nothing else.", (text) => { + await fixture.expectPromptText(session.sessionId, "Reply with exactly integration-ok and nothing else.", (text) => { expect(text.toLowerCase()).toContain("integration-ok"); }); }); @@ -44,7 +39,7 @@ describeE2E("E2E tests", () => { fixture = await createAuthenticatedFixture(); const session = await fixture.createSession(); - const models = session.response.models; + const models = session.models; if (!models) { throw new Error("Agent did not return initial model state."); } @@ -59,17 +54,17 @@ describeE2E("E2E tests", () => { } await fixture.connection.unstable_setSessionModel({ - sessionId: session.response.sessionId, + sessionId: session.sessionId, modelId: selectedModelId, }); - await expectStatus(session, {Model: selectedModelId}); + await fixture.expectStatus(session.sessionId, {Model: selectedModelId}); }); it("changes session mode via setSessionMode and reflects it in /status", async () => { fixture = await createAuthenticatedFixture(); const session = await fixture.createSession(); - const modes = session.response.modes; + const modes = session.modes; expect(modes?.currentModeId).toBe(AgentMode.DEFAULT_AGENT_MODE.id); expect(modes?.availableModes.map((mode) => mode.id)).toEqual( AgentMode.all().map((mode) => mode.id), @@ -77,11 +72,11 @@ describeE2E("E2E tests", () => { const targetMode = AgentMode.AgentFullAccess; await fixture.connection.setSessionMode({ - sessionId: session.response.sessionId, + sessionId: session.sessionId, modeId: targetMode.id, }); - await expectStatus(session, { + await fixture.expectStatus(session.sessionId, { Approval: targetMode.approvalPolicy, Sandbox: targetMode.sandboxMode, }); @@ -89,27 +84,28 @@ describeE2E("E2E tests", () => { it("respects INITIAL_AGENT_MODE when seeding the initial session mode", async () => { const initialMode = AgentMode.ReadOnly; - fixture = await createAuthenticatedFixture(undefined, { + fixture = await createAuthenticatedFixture({ INITIAL_AGENT_MODE: initialMode.id, }); const session = await fixture.createSession(); - expect(session.response.modes?.currentModeId).toBe(initialMode.id); + expect(session.modes?.currentModeId).toBe(initialMode.id); - await expectStatus(session, { + await fixture.expectStatus(session.sessionId, { Approval: initialMode.approvalPolicy, Sandbox: initialMode.sandboxMode, }); }); - it('lists a user skill from the wrapped CODEX_HOME', async () => { - fixture = await createFixtureWithSkill({ + it("lists a user skill from the wrapped CODEX_HOME", async () => { + fixture = await createAuthenticatedFixture(); + fixture.writeSkill({ name: "integration-skill", description: "Integration skill", body: "This skill exists only for integration testing.", }); const session = await fixture.createSession(); - await session.expectPromptText("/skills", (text) => { + await fixture.expectPromptText(session.sessionId, "/skills", (text) => { expect(text).toContain("Available skills:"); expect(text).toContain("- integration-skill: Integration skill"); }); diff --git a/src/__tests__/CodexACPAgent/e2e/permission-responders.ts b/src/__tests__/CodexACPAgent/e2e/permission-responders.ts new file mode 100644 index 00000000..79822a81 --- /dev/null +++ b/src/__tests__/CodexACPAgent/e2e/permission-responders.ts @@ -0,0 +1,22 @@ +import * as acp from "@agentclientprotocol/sdk"; +import {ApprovalOptionId} from "../../../ApprovalOptionId"; + +export type PermissionResponder = ( + params: acp.RequestPermissionRequest, +) => acp.RequestPermissionResponse; + +export function createPermissionResponder( + expectedToolCallKind: acp.ToolKind, + optionId: ApprovalOptionId, +): PermissionResponder { + return (request) => createPermissionResponse( + request.toolCall.kind === expectedToolCallKind ? optionId : null + ); +} + +export function createPermissionResponse(optionId: ApprovalOptionId | null): acp.RequestPermissionResponse { + if (optionId === null) { + return {outcome: {outcome: "cancelled"}}; + } + return {outcome: {outcome: "selected", optionId}}; +} diff --git a/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts b/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts new file mode 100644 index 00000000..78347e43 --- /dev/null +++ b/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts @@ -0,0 +1,260 @@ +import * as acp from "@agentclientprotocol/sdk"; +import {type ChildProcessWithoutNullStreams, spawn} from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import {Readable, Writable} from "node:stream"; +import {expect, vi} from "vitest"; +import {removeDirectoryWithRetry, writeCodexHomeConfig} from "../../acp-test-utils"; +import type {PermissionResponder} from "./permission-responders"; + +export interface TestSkill { + readonly name: string; + readonly description: string; + readonly body: string; +} + +export interface SpawnedAgentFixture { + readonly connection: acp.ClientSideConnection; + readonly workspaceDir: string; + createSession(): Promise; + writeSkill(skill: TestSkill): void; + setPermissionResponder(responder: PermissionResponder): void; + expectPromptText( + sessionId: string, + promptText: string, + assertText: (text: string) => void, + timeoutMs?: number, + ): Promise; + expectStatus(sessionId: string, fields: Record): Promise; + readPermissionRequests( + sessionId: string, + toolCallKind: acp.ToolKind, + ): acp.RequestPermissionRequest[]; + dispose(): Promise; +} + +export function createSpawnedAgentFixture( + extraEnv?: NodeJS.ProcessEnv, +): SpawnedAgentFixture { + const paths = RuntimePaths.createTemporary(); + writeCodexHomeConfig(paths.codexHome, { + model: "gpt-5.2", + model_reasoning_effort: "none", + web_search: "disabled", + }); + + const agentProcess = spawn("npm", ["run", "--silent", "start"], { + cwd: process.cwd(), + env: { + ...process.env, + CODEX_HOME: paths.codexHome, + APP_SERVER_LOGS: paths.appServerLogsDir, + ...extraEnv, + }, + stdio: ["pipe", "pipe", "pipe"], + }); + + return new SpawnedAgentFixtureImpl(new RecordingClient(), agentProcess, paths); +} + +class RuntimePaths { + readonly codexHome: string; + readonly workspaceDir: string; + readonly appServerLogsDir: string; + + constructor(readonly rootDir: string) { + this.codexHome = path.join(rootDir, "codex-home"); + this.workspaceDir = path.join(rootDir, "workspace"); + this.appServerLogsDir = path.join(rootDir, "logs"); + } + + static createTemporary(): RuntimePaths { + const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "codex-acp-integration-")); + const paths = new RuntimePaths(rootDir); + for (const dir of [paths.codexHome, paths.workspaceDir, paths.appServerLogsDir]) { + fs.mkdirSync(dir, {recursive: true}); + } + return paths; + } +} + +class RecordingClient implements acp.Client { + private readonly textBySessionId = new Map(); + private readonly permissionRequestsBySessionId = new Map(); + private permissionResponder: PermissionResponder = () => ({ + outcome: {outcome: "cancelled"}, + }); + + setPermissionResponder(responder: PermissionResponder): void { + this.permissionResponder = responder; + } + + async requestPermission(params: acp.RequestPermissionRequest): Promise { + let requests = this.permissionRequestsBySessionId.get(params.sessionId); + if (!requests) { + requests = []; + this.permissionRequestsBySessionId.set(params.sessionId, requests); + } + requests.push(params); + return this.permissionResponder(params); + } + + async sessionUpdate(params: acp.SessionNotification): Promise { + if (params.update.sessionUpdate !== "agent_message_chunk" || params.update.content.type !== "text") { + return; + } + + const nextText = `${this.textBySessionId.get(params.sessionId) ?? ""}${params.update.content.text}`; + this.textBySessionId.set(params.sessionId, nextText); + } + + readText(sessionId: string): string { + return this.textBySessionId.get(sessionId) ?? ""; + } + + readPermissionRequests( + sessionId: string, + toolCallKind: acp.ToolKind, + ): acp.RequestPermissionRequest[] { + const requests = this.permissionRequestsBySessionId.get(sessionId) ?? []; + return requests.filter((request) => request.toolCall.kind === toolCallKind); + } +} + +class SpawnedAgentFixtureImpl implements SpawnedAgentFixture { + readonly connection: acp.ClientSideConnection; + + constructor( + private readonly client: RecordingClient, + private readonly agentProcess: ChildProcessWithoutNullStreams, + private readonly paths: RuntimePaths, + ) { + const output = Readable.toWeb(agentProcess.stdout) as ReadableStream; + this.connection = new acp.ClientSideConnection( + () => client, + acp.ndJsonStream(Writable.toWeb(agentProcess.stdin), output) + ); + } + + get workspaceDir(): string { + return this.paths.workspaceDir; + } + + async createSession(): Promise { + return await this.connection.newSession({ + cwd: this.workspaceDir, + mcpServers: [], + }); + } + + writeSkill(skill: TestSkill): void { + const skillDirectory = path.join(this.paths.codexHome, "skills", skill.name); + fs.mkdirSync(skillDirectory, {recursive: true}); + fs.writeFileSync( + path.join(skillDirectory, "SKILL.md"), + [ + "---", + `name: ${skill.name}`, + `description: ${skill.description}`, + "metadata:", + ` short-description: ${skill.description}`, + "---", + "", + skill.body, + "", + ].join("\n"), + "utf8", + ); + } + + setPermissionResponder(responder: PermissionResponder): void { + this.client.setPermissionResponder(responder); + } + + async expectPromptText( + sessionId: string, + promptText: string, + assertText: (text: string) => void, + timeoutMs = 30_000, + ): Promise { + const previousText = this.client.readText(sessionId); + const promptResponse = await this.connection.prompt({ + sessionId, + prompt: [{type: "text", text: promptText}], + }); + expect(promptResponse.stopReason).toBe("end_turn"); + + await vi.waitFor(() => { + const sessionText = this.client.readText(sessionId); + assertText(sessionText.slice(previousText.length)); + }, {timeout: timeoutMs}); + } + + async expectStatus(sessionId: string, fields: Record): Promise { + await this.expectPromptText(sessionId, "/status", (text) => { + for (const [field, value] of Object.entries(fields)) { + expect(text).toContain(`**${field}:** ${String(value)}`); + } + }); + } + + readPermissionRequests( + sessionId: string, + toolCallKind: acp.ToolKind, + ): acp.RequestPermissionRequest[] { + return this.client.readPermissionRequests(sessionId, toolCallKind); + } + + async dispose(): Promise { + if (!this.agentProcess.stdin.destroyed && !this.agentProcess.stdin.writableEnded) { + this.agentProcess.stdin.end(); + } + + const exitedAfterStdinClose = await waitForProcessExit(this.agentProcess, 4_000); + if (!exitedAfterStdinClose && !this.agentProcess.killed) { + this.agentProcess.kill(); + await waitForProcessExit(this.agentProcess, 4_000); + } + + printLogDirectory(this.paths.appServerLogsDir); + removeDirectoryWithRetry(this.paths.rootDir); + } +} + +function printLogDirectory(logDirectory: string): void { + fs.readdirSync(logDirectory, {withFileTypes: true}) + .filter((entry) => entry.isFile()) + .forEach((entry) => { + const logFilePath = path.join(logDirectory, entry.name); + const content = 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("------"); + }); +} + +async function waitForProcessExit(proc: ChildProcessWithoutNullStreams, timeoutMs: number): Promise { + if (proc.exitCode !== null || proc.signalCode !== null) { + return true; + } + + return await new Promise((resolve) => { + const timeout = setTimeout(() => { + cleanup(); + resolve(false); + }, timeoutMs); + + const cleanup = () => { + clearTimeout(timeout); + proc.off("exit", handleExit); + }; + + const handleExit = () => { + cleanup(); + resolve(true); + }; + + proc.once("exit", handleExit); + }); +} From 349cfd8cae5ebaf48fee05ea83d0207f97a1cfcb Mon Sep 17 00:00:00 2001 From: "Ilia.Shulgin" Date: Wed, 29 Apr 2026 10:56:49 +0200 Subject: [PATCH 6/6] Specify client capabilities for e2e tests --- .../CodexACPAgent/e2e/acp-e2e-test-utils.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts index da3a9220..92d094d2 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts @@ -77,6 +77,24 @@ export async function createGatewayFixture( }); } +function buildClientCapabilities(): acp.ClientCapabilities { + return { + fs: { + readTextFile: true, + writeTextFile: true, + }, + terminal: true, + auth: { + _meta: { + gateway: true, + }, + }, + _meta: { + "terminal-auth": true, + }, + }; +} + type Authenticator = (connection: acp.ClientSideConnection, authMethods: acp.AuthMethod[]) => Promise; async function createSpawnedFixture( @@ -88,7 +106,7 @@ async function createSpawnedFixture( const initializeResponse = await connection.initialize({ protocolVersion: acp.PROTOCOL_VERSION, - clientCapabilities: {}, + clientCapabilities: buildClientCapabilities(), clientInfo: { name: "vitest", version: "1.0.0",