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..8e2484ff --- /dev/null +++ b/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 { + 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); + }); +}); 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..da026f52 --- /dev/null +++ b/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 { + 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); + }); +}); diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts index 9f74d65e..92d094d2 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-test-utils.ts @@ -1,96 +1,112 @@ 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, vi} from "vitest"; -import {removeDirectoryWithRetry} from "../../acp-test-utils"; +import {describe, expect} from "vitest"; +import {AgentMode} from "../../../AgentMode"; +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; -} - -export interface SpawnedAgentFixture { - readonly connection: acp.ClientSideConnection; - createSession(): Promise; - 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); } -interface TestSkill { - readonly name: string; - readonly description: string; - readonly body: string; +export function expectEndTurn(response: acp.PromptResponse): void { + expect(response.stopReason).toBe("end_turn"); } -interface RuntimePaths { - readonly rootDir: string; - readonly codexHome: string; - readonly workspaceDir: string; - readonly appServerLogsDir: string; -} +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")) { + throw new Error("API key authentication is not available."); + } -class RecordingClient implements acp.Client { - private readonly textBySessionId = new Map(); + await connection.authenticate({ + methodId: "api-key", + _meta: { + "api-key": { + apiKey, + }, + }, + }); - async requestPermission(_params: acp.RequestPermissionRequest): Promise { - return { - outcome: {outcome: "cancelled"}, - }; - } + const authenticationStatus = await getAuthenticationStatus(connection); + if (authenticationStatus["type"] !== "api-key") { + throw new Error(`Unexpected authentication status: ${JSON.stringify(authenticationStatus)}`); + } + }, extraEnv); +} - async sessionUpdate(params: acp.SessionNotification): Promise { - if (params.update.sessionUpdate !== "agent_message_chunk" || params.update.content.type !== "text") { - return; +export async function createReadOnlyFixture(): Promise { + return await createAuthenticatedFixture({INITIAL_AGENT_MODE: AgentMode.ReadOnly.id}); +} + +export async function createGatewayFixture( + baseUrl: string, + headers: Record, +): Promise { + return await createSpawnedFixture(async (connection, authMethods) => { + if (!authMethods.some((method) => method.id === "gateway")) { + throw new Error("Gateway authentication is not available."); } - const nextText = `${this.textBySessionId.get(params.sessionId) ?? ""}${params.update.content.text}`; - this.textBySessionId.set(params.sessionId, nextText); - } + await connection.authenticate({ + methodId: "gateway", + _meta: { + gateway: { + baseUrl, + headers, + }, + }, + }); - readText(sessionId: string): string { - return this.textBySessionId.get(sessionId) ?? ""; - } + const authenticationStatus = await getAuthenticationStatus(connection); + if (authenticationStatus["type"] !== "gateway" || authenticationStatus["name"] !== "custom-gateway") { + throw new Error(`Unexpected authentication status: ${JSON.stringify(authenticationStatus)}`); + } + }); } -export async function createFixtureWithSkill(skill: TestSkill): Promise { - const runtimePaths = createTemporaryRuntimePaths(); - writeSkill(runtimePaths.codexHome, skill); - return await createAuthenticatedFixture(runtimePaths); +function buildClientCapabilities(): acp.ClientCapabilities { + return { + fs: { + readTextFile: true, + writeTextFile: true, + }, + terminal: true, + auth: { + _meta: { + gateway: true, + }, + }, + _meta: { + "terminal-auth": true, + }, + }; } -export async function createAuthenticatedFixture( - runtimePaths = createTemporaryRuntimePaths() -): Promise { - const apiKey = requireLiveApiKey(); - const agentProcess = spawn("npm", ["run", "--silent", "start"], { - cwd: process.cwd(), - env: { - ...process.env, - CODEX_HOME: runtimePaths.codexHome, - APP_SERVER_LOGS: runtimePaths.appServerLogsDir, - }, - stdio: ["pipe", "pipe", "pipe"], - }); +type Authenticator = (connection: acp.ClientSideConnection, authMethods: acp.AuthMethod[]) => Promise; - 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) - ); +async function createSpawnedFixture( + authenticate: Authenticator, + extraEnv?: NodeJS.ProcessEnv, +): Promise { + const fixture = createSpawnedAgentFixture(extraEnv); + const connection = fixture.connection; const initializeResponse = await connection.initialize({ protocolVersion: acp.PROTOCOL_VERSION, - clientCapabilities: {}, + clientCapabilities: buildClientCapabilities(), clientInfo: { name: "vitest", version: "1.0.0", @@ -101,59 +117,11 @@ 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)}`); - } - - 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); - }, - }; - }; - - return { - connection, - createSession, - 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); - }, - }; + await authenticate(connection, initializeResponse.authMethods ?? []); + return fixture; } -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."); @@ -161,110 +129,6 @@ 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}); - fs.writeFileSync(path.join(codexHome, "config.toml"), 'cli_auth_credentials_store = "file"\n', "utf8"); - - 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, - }], - }); - - if (promptResponse.stopReason !== "end_turn") { - throw new Error(`Unexpected stop reason: ${promptResponse.stopReason}`); - } - - 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 19d7dfd8..97511e7c 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts @@ -1,8 +1,10 @@ import {afterEach, expect, it} from "vitest"; +import {AgentMode} from "../../../AgentMode"; import { createAuthenticatedFixture, - createFixtureWithSkill, + createGatewayFixture, describeE2E, + requireLiveApiKey, type SpawnedAgentFixture, } from "./acp-e2e-test-utils"; @@ -16,10 +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 () => { + const apiKey = requireLiveApiKey(); + fixture = await createGatewayFixture("https://api.openai.com/v1", {Authorization: `Bearer ${apiKey}`}); + const session = await fixture.createSession(); + await fixture.expectPromptText(session.sessionId, "Reply with exactly integration-ok and nothing else.", (text) => { expect(text.toLowerCase()).toContain("integration-ok"); }); }); @@ -28,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."); } @@ -43,22 +54,58 @@ describeE2E("E2E tests", () => { } await fixture.connection.unstable_setSessionModel({ - sessionId: session.response.sessionId, + sessionId: session.sessionId, modelId: selectedModelId, }); - await session.expectPromptText("/status", (text) => { - expect(text).toContain(`**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.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.sessionId, + modeId: targetMode.id, + }); + + await fixture.expectStatus(session.sessionId, { + 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({ + INITIAL_AGENT_MODE: initialMode.id, + }); + const session = await fixture.createSession(); + + expect(session.modes?.currentModeId).toBe(initialMode.id); + + 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); + }); +} 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 {