From 318b26b77a2fad1ae79819eaa56c049ff7b3f916 Mon Sep 17 00:00:00 2001 From: Joshua Snyder Date: Mon, 6 Jul 2026 18:47:57 +0100 Subject: [PATCH] feat(codex)!: remove the codex-acp harness, app-server is the only codex adapter The native app-server adapter has been the default for codex sessions and codex-acp existed only as a flag-gated fallback. That fallback is a liability: when the selector's inputs wobble (missing binary, unresolved flag) sessions silently land on codex-acp, which deadlocks with current models instead of failing visibly. - delete the codex-acp adapter (CodexAcpAgent, its client, spawn, MCP server entries) and the acp-vs-app-server parity suite - move the shared CodexSettingsManager and model catalog into the codex-app-server directory - drop the codex-app-server feature flag, the useCodexAppServer plumbing (sessionService, workspace-server schemas, adapter-store pinning), and the POSTHOG_CODEX_USE_APP_SERVER / POSTHOG_CODEX_USE_ACP env overrides; a codex session always uses the app-server adapter and a missing native binary now fails loudly - stop downloading and staging the codex-acp binary (~80MB per platform); the bundle dir keeps its name so packaging config, CI verify steps, and older sandbox images stay compatible, and nativeCodexBinaryPath still accepts a sibling-binary hint from hosts that pass the old path Co-Authored-By: Claude Fable 5 --- apps/code/scripts/download-binaries.mjs | 28 - apps/code/vite-main-plugins.mts | 2 +- apps/code/vite.main.config.mts | 1 - packages/agent/e2e/config.ts | 1 - packages/agent/parity/harness.ts | 240 ---- packages/agent/parity/run.ts | 413 ------- .../agent/src/adapters/acp-connection.test.ts | 47 - packages/agent/src/adapters/acp-connection.ts | 98 +- .../adapters/codex-app-server/binary-path.ts | 17 +- .../codex-app-server-agent.ts | 9 +- .../codex-app-server/ext-notifications.ts | 4 +- .../local-tools-mcp-server.ts | 2 +- .../codex-app-server/local-tools-mcp.ts | 4 +- .../models.test.ts | 0 .../{codex => codex-app-server}/models.ts | 0 .../codex-app-server/session-config.ts | 2 +- .../settings.test.ts | 0 .../{codex => codex-app-server}/settings.ts | 2 +- .../src/adapters/codex-app-server/spawn.ts | 32 +- .../codex/codex-agent.refresh.test.ts | 289 ----- .../src/adapters/codex/codex-agent.test.ts | 728 ----------- .../agent/src/adapters/codex/codex-agent.ts | 1060 ----------------- .../src/adapters/codex/codex-client.test.ts | 338 ------ .../agent/src/adapters/codex/codex-client.ts | 307 ----- .../agent/src/adapters/codex/session-state.ts | 97 -- .../agent/src/adapters/codex/spawn.test.ts | 136 --- packages/agent/src/adapters/codex/spawn.ts | 212 ---- .../codex/structured-output-constants.ts | 9 - .../codex/structured-output-mcp-server.ts | 72 -- .../agent/src/adapters/reasoning-effort.ts | 2 +- packages/agent/src/agent.ts | 1 - packages/agent/src/server/agent-server.ts | 6 +- packages/agent/src/types.ts | 6 - packages/agent/tsup.config.ts | 5 +- packages/core/src/sessions/sessionService.ts | 79 +- packages/shared/src/sessions.ts | 4 +- .../sessions/hooks/useMessagingMode.ts | 2 +- .../features/sessions/sessionAdapterStore.ts | 24 +- .../features/sessions/sessionServiceHost.ts | 8 - .../src/services/agent/agent.ts | 12 +- .../src/services/agent/codex-home.ts | 2 +- .../src/services/agent/schemas.ts | 10 +- 42 files changed, 101 insertions(+), 4210 deletions(-) delete mode 100644 packages/agent/parity/harness.ts delete mode 100644 packages/agent/parity/run.ts delete mode 100644 packages/agent/src/adapters/acp-connection.test.ts rename packages/agent/src/adapters/{codex => codex-app-server}/local-tools-mcp-server.ts (96%) rename packages/agent/src/adapters/{codex => codex-app-server}/models.test.ts (100%) rename packages/agent/src/adapters/{codex => codex-app-server}/models.ts (100%) rename packages/agent/src/adapters/{codex => codex-app-server}/settings.test.ts (100%) rename packages/agent/src/adapters/{codex => codex-app-server}/settings.ts (98%) delete mode 100644 packages/agent/src/adapters/codex/codex-agent.refresh.test.ts delete mode 100644 packages/agent/src/adapters/codex/codex-agent.test.ts delete mode 100644 packages/agent/src/adapters/codex/codex-agent.ts delete mode 100644 packages/agent/src/adapters/codex/codex-client.test.ts delete mode 100644 packages/agent/src/adapters/codex/codex-client.ts delete mode 100644 packages/agent/src/adapters/codex/session-state.ts delete mode 100644 packages/agent/src/adapters/codex/spawn.test.ts delete mode 100644 packages/agent/src/adapters/codex/spawn.ts delete mode 100644 packages/agent/src/adapters/codex/structured-output-constants.ts delete mode 100644 packages/agent/src/adapters/codex/structured-output-mcp-server.ts diff --git a/apps/code/scripts/download-binaries.mjs b/apps/code/scripts/download-binaries.mjs index 4b1a049f2d..bec564d3ff 100644 --- a/apps/code/scripts/download-binaries.mjs +++ b/apps/code/scripts/download-binaries.mjs @@ -20,34 +20,6 @@ const __dirname = dirname(fileURLToPath(import.meta.url)); const DEST_DIR = join(__dirname, "..", "resources", "codex-acp"); const BINARIES = [ - { - name: "codex-acp", - version: "0.14.0", - getUrl: (version, target) => { - const ext = target.includes("windows") ? "zip" : "tar.gz"; - return `https://github.com/zed-industries/codex-acp/releases/download/v${version}/codex-acp-${version}-${target}.${ext}`; - }, - getTarget: () => { - const { platform, arch } = process; - const targets = { - darwin: { arm64: "aarch64-apple-darwin", x64: "x86_64-apple-darwin" }, - linux: { - arm64: "aarch64-unknown-linux-gnu", - x64: "x86_64-unknown-linux-gnu", - }, - win32: { - arm64: "aarch64-pc-windows-msvc", - x64: "x86_64-pc-windows-msvc", - }, - }; - const platformTargets = targets[platform]; - if (!platformTargets) - throw new Error(`Unsupported platform: ${platform}`); - const target = platformTargets[arch]; - if (!target) throw new Error(`Unsupported arch: ${arch}`); - return target; - }, - }, { name: "codex", version: "0.140.0", diff --git a/apps/code/vite-main-plugins.mts b/apps/code/vite-main-plugins.mts index bbc520d3cf..f83bea74b7 100644 --- a/apps/code/vite-main-plugins.mts +++ b/apps/code/vite-main-plugins.mts @@ -568,7 +568,7 @@ export function copyCodexAcpBinaries(): Plugin { const sourceDir = join(__dirname, "resources/codex-acp"); const binaries = [ - { name: "codex-acp", winName: "codex-acp.exe" }, + { name: "codex", winName: "codex.exe" }, // The native codex CLI must ship next to codex-acp: the app-server // sub-adapter resolves it as a sibling and silently falls back to // codex-acp when it's missing. diff --git a/apps/code/vite.main.config.mts b/apps/code/vite.main.config.mts index 6abd311c19..196400db7e 100644 --- a/apps/code/vite.main.config.mts +++ b/apps/code/vite.main.config.mts @@ -565,7 +565,6 @@ function copyCodexAcpBinaries(): Plugin { const sourceDir = join(__dirname, "resources/codex-acp"); const binaries = [ - { name: "codex-acp", winName: "codex-acp.exe" }, { name: "codex", winName: "codex.exe" }, { name: "rg", winName: "rg.exe" }, ]; diff --git a/packages/agent/e2e/config.ts b/packages/agent/e2e/config.ts index a1235d4036..29e563dd7f 100644 --- a/packages/agent/e2e/config.ts +++ b/packages/agent/e2e/config.ts @@ -90,7 +90,6 @@ export const E2E = { } process.env.OPENAI_BASE_URL = openAiBase(); process.env.OPENAI_API_KEY = TOKEN; - process.env.POSTHOG_CODEX_USE_APP_SERVER = "1"; }, /** The codexOptions the codex arm passes through `createAcpConnection`. */ diff --git a/packages/agent/parity/harness.ts b/packages/agent/parity/harness.ts deleted file mode 100644 index 83a197a34f..0000000000 --- a/packages/agent/parity/harness.ts +++ /dev/null @@ -1,240 +0,0 @@ -/** - * Differential parity harness for the two Codex adapters. - * - * Drives a scripted scenario (a stateful sequence of ACP client operations) - * through one codex adapter — selected by the POSTHOG_CODEX_USE_ACP env toggle — - * over the same in-process ACP transport the real host uses, and captures the - * full ACP stream (every sessionUpdate, every server→client requestPermission, - * and each call's response). Run the same scenario through both adapters and - * diff the captured streams to find parity gaps. No HTTP/JWT/Temporal. - */ -import { promises as fs } from "node:fs"; -import { resolve } from "node:path"; -// @ts-expect-error - resolved by tsx at runtime -import { ClientSideConnection, ndJsonStream } from "@agentclientprotocol/sdk"; -import { createAcpConnection } from "../src/adapters/acp-connection"; -import type { Logger } from "../src/utils/logger"; - -export type AdapterMode = "acp" | "app-server"; - -export interface CapturedEvent { - t: number; - kind: - | "step" - | "sessionUpdate" - | "requestPermission" - | "extNotification" - | "extMethod"; - op?: string; - sessionUpdate?: string; - data?: any; -} - -export interface CapturedRun { - adapter: AdapterMode; - scenario: string; - events: CapturedEvent[]; - stepResults: Array<{ op: string; ok: boolean; result?: any; error?: string }>; - fatalError?: string; -} - -export interface ScenarioCtx { - cwd: string; - model?: string; - /** Run one ACP operation, record it as a step boundary + its (redacted) result. */ - step(op: string, fn: () => Promise): Promise; -} - -export interface Scenario { - name: string; - run: (conn: any, ctx: ScenarioCtx) => Promise; -} - -export interface HarnessConfig { - cwd: string; - codexOptions: { - cwd: string; - binaryPath?: string; - apiBaseUrl?: string; - apiKey?: string; - model?: string; - reasoningEffort?: string; - }; - timeoutMs?: number; - logger?: Logger; -} - -/** Keep result shapes comparable: drop big/nondeterministic blobs, keep structure. */ -function redact(value: any): any { - if (!value || typeof value !== "object") return value; - const out: any = {}; - for (const [k, v] of Object.entries(value)) { - if (k === "sessionId") out[k] = ""; - else if (k === "configOptions" && Array.isArray(v)) { - out[k] = v.map((o: any) => ({ - id: o?.id, - category: o?.category, - value: o?.value, - options: (o?.options ?? []).map((x: any) => x?.id ?? x?.optionId), - })); - } else if (k === "modes") { - out[k] = { - currentModeId: (v as any)?.currentModeId, - availableModes: ((v as any)?.availableModes ?? []).map( - (m: any) => m?.id, - ), - }; - } else if (k === "usage" && v && typeof v === "object") { - out[k] = Object.fromEntries( - Object.entries(v).map(([uk, uv]) => [ - uk, - typeof uv === "number" ? (uv > 0 ? ">0" : 0) : uv, - ]), - ); - } else if (typeof v === "string" && v.length > 120) - out[k] = ``; - else out[k] = v; - } - return out; -} - -export async function runScenario( - mode: AdapterMode, - scenario: Scenario, - cfg: HarnessConfig, -): Promise { - // Select the adapter. Until the migration adds a passed-in option, the env - // toggle is the only lever: set => codex-acp, unset => native app-server. - if (mode === "acp") process.env.POSTHOG_CODEX_USE_ACP = "1"; - else delete process.env.POSTHOG_CODEX_USE_ACP; - - const captured: CapturedRun = { - adapter: mode, - scenario: scenario.name, - events: [], - stepResults: [], - }; - let ord = 0; - - const client = { - async sessionUpdate(p: any): Promise { - captured.events.push({ - t: ord++, - kind: "sessionUpdate", - sessionUpdate: p?.update?.sessionUpdate, - data: p?.update, - }); - }, - async requestPermission(p: any): Promise { - captured.events.push({ - t: ord++, - kind: "requestPermission", - data: { - title: p?.toolCall?.title, - kind: p?.toolCall?.kind, - options: (p?.options ?? []).map((o: any) => ({ - id: o?.optionId, - kind: o?.kind, - })), - }, - }); - const allow = - (p?.options ?? []).find( - (o: any) => o?.kind === "allow_once" || o?.kind === "allow_always", - ) ?? p?.options?.[0]; - return { - outcome: { outcome: "selected", optionId: allow?.optionId ?? "allow" }, - }; - }, - async readTextFile(p: any): Promise { - return { content: await fs.readFile(resolve(cfg.cwd, p.path), "utf8") }; - }, - async writeTextFile(p: any): Promise { - await fs.writeFile(resolve(cfg.cwd, p.path), p.content); - return {}; - }, - // PostHog ext-notifications (_posthog/usage_update, _posthog/turn_complete, - // _posthog/sdk_session, ...) are part of the parity surface and are sent - // outside sessionUpdate — capture them so the report covers them. - async extNotification(method: string, params: any): Promise { - captured.events.push({ - t: ord++, - kind: "extNotification", - op: method, - data: redact(params), - }); - }, - async extMethod(method: string, params: any): Promise { - captured.events.push({ - t: ord++, - kind: "extMethod", - op: method, - data: redact(params), - }); - return {}; - }, - }; - - const acp = createAcpConnection({ - adapter: "codex", - codexOptions: cfg.codexOptions as any, - logger: cfg.logger, - }); - const stream = ndJsonStream( - acp.clientStreams.writable, - acp.clientStreams.readable, - ); - const conn = new ClientSideConnection(() => client, stream); - - const ctx: ScenarioCtx = { - cwd: cfg.cwd, - model: cfg.codexOptions.model, - async step(op, fn) { - captured.events.push({ t: ord++, kind: "step", op }); - const started = Date.now(); - console.error(` [step] ${op} ...`); - try { - const result = await fn(); - console.error(` [step] ${op} ✓ (${Date.now() - started}ms)`); - captured.stepResults.push({ op, ok: true, result: redact(result) }); - return result; - } catch (e: any) { - console.error( - ` [step] ${op} ✗ (${Date.now() - started}ms): ${String(e?.message ?? e)}`, - ); - captured.stepResults.push({ - op, - ok: false, - error: String(e?.message ?? e), - }); - throw e; - } - }, - }; - - const timeout = new Promise((_, rej) => - setTimeout( - () => - rej(new Error(`scenario timeout after ${cfg.timeoutMs ?? 180000}ms`)), - cfg.timeoutMs ?? 180000, - ), - ); - try { - await ctx.step("initialize", () => - conn.initialize({ - protocolVersion: 1, - clientCapabilities: { fs: { readTextFile: true, writeTextFile: true } }, - }), - ); - await Promise.race([scenario.run(conn, ctx), timeout]); - } catch (e: any) { - captured.fatalError = String(e?.message ?? e); - } finally { - // Bounded: a wedged adapter cleanup must never hang the loop. - await Promise.race([ - acp.cleanup().catch(() => undefined), - new Promise((resolve) => setTimeout(resolve, 5000)), - ]); - } - return captured; -} diff --git a/packages/agent/parity/run.ts b/packages/agent/parity/run.ts deleted file mode 100644 index 0d89ae476e..0000000000 --- a/packages/agent/parity/run.ts +++ /dev/null @@ -1,413 +0,0 @@ -/** - * Parity runner: drive scenarios through both codex adapters, extract a - * normalized feature report from each ACP stream, and diff app-server vs - * codex-acp. Writes raw captures + parity-report.json to parity/out/. - * - * Usage (from packages/agent): - * PARITY_API_KEY= pnpm exec tsx parity/run.ts [--only acp|app-server] [--scenario name] - * Env: - * PARITY_GATEWAY_URL default http://localhost:3308/posthog_code/v1 - * PARITY_API_KEY PostHog token the local llm-gateway accepts (required for a live run) - * PARITY_MODEL default gpt-5.5 - */ -import { execFileSync } from "node:child_process"; -import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; -import { Logger } from "../src/utils/logger"; -import { - type AdapterMode, - type CapturedRun, - runScenario, - type Scenario, -} from "./harness"; - -const OUT_DIR = join(import.meta.dirname, "out"); -const RESOURCES = join( - import.meta.dirname, - "..", - "..", - "..", - "apps", - "code", - "resources", - "codex-acp", -); -const CODEX_ACP_BIN = join(RESOURCES, "codex-acp"); -const NATIVE_CODEX_BIN = join(RESOURCES, "codex"); -const GATEWAY = - process.env.PARITY_GATEWAY_URL ?? "http://localhost:3308/posthog_code/v1"; -const API_KEY = process.env.PARITY_API_KEY ?? ""; -const MODEL = process.env.PARITY_MODEL ?? "gpt-5.5"; -const REPO = "/tmp/codex-parity-repo"; - -const SCENARIOS: Scenario[] = [ - { - name: "basic-task", - async run(conn, ctx) { - const session = await ctx.step("newSession", () => - conn.newSession({ - cwd: ctx.cwd, - mcpServers: [], - _meta: { - sessionId: "parity", - systemPrompt: "You are a coding assistant in a tiny test repo.", - model: ctx.model, - permissionMode: "bypassPermissions", - }, - }), - ); - const sessionId = session.sessionId; - await ctx.step("prompt", () => - conn.prompt({ - sessionId, - prompt: [ - { - type: "text", - text: "Do exactly these steps and nothing else: 1) Read the file target.txt. 2) Edit it so the second line reads FOO instead of line2. 3) Run the shell command `cat target.txt`. 4) In one sentence confirm what you changed, then stop.", - }, - ], - }), - ); - }, - }, - { - name: "modes-and-resume", - async run(conn, ctx) { - const session = await ctx.step("newSession", () => - conn.newSession({ - cwd: ctx.cwd, - mcpServers: [], - _meta: { - sessionId: "parity2", - systemPrompt: "You are a coding assistant.", - model: ctx.model, - permissionMode: "auto", - }, - }), - ); - const sessionId = session.sessionId; - // Mode switch — codex-acp supports it; app-server gap until migration. - await ctx.step("setSessionConfigOption(mode)", () => - conn - .setSessionConfigOption({ - sessionId, - configId: "mode", - value: "read-only", - }) - .catch((e: any) => { - throw e; - }), - ); - await ctx.step("prompt", () => - conn.prompt({ - sessionId, - prompt: [ - { - type: "text", - text: "List the files in this repo with `ls`, then stop.", - }, - ], - }), - ); - // Resume in the same connection (host calls resumeSession on reconnect). - await ctx.step("resumeSession", () => - conn.resumeSession({ - sessionId, - cwd: ctx.cwd, - mcpServers: [], - _meta: { - systemPrompt: "You are a coding assistant.", - model: ctx.model, - }, - }), - ); - }, - }, -]; - -function extractFeatures(run: CapturedRun): Record { - const updateTypes = new Set(); - const toolKinds = new Set(); - const toolStatuses = new Set(); - let hasDiff = false; - let hasToolContent = false; - const approvals: string[] = []; - let usageFields = new Set(); - let modeUpdate = false; - const extNotifs = new Set(); - - for (const e of run.events) { - if (e.kind === "requestPermission") approvals.push(e.data?.kind ?? "?"); - if (e.kind === "extNotification") extNotifs.add(e.op ?? "?"); - if (e.kind !== "sessionUpdate") continue; - const u = e.sessionUpdate ?? "?"; - updateTypes.add(u); - const d = e.data ?? {}; - if (u === "tool_call") { - if (d.kind) toolKinds.add(d.kind); - if (d.status) toolStatuses.add(d.status); - } - if (u === "tool_call_update") { - if (d.status) toolStatuses.add(d.status); - const content = d.content ?? []; - if (Array.isArray(content)) { - for (const c of content) { - if (c?.type === "diff") hasDiff = true; - if (c?.type === "content") hasToolContent = true; - } - } - if ( - d.rawInput?.diff || - (typeof d.rawOutput === "string" && d.rawOutput.includes("diff")) - ) - hasDiff = true; - } - if (u === "current_mode_update" || u === "config_option_update") - modeUpdate = true; - if (u === "usage_update") - usageFields = new Set([ - ...usageFields, - ...Object.keys(d.usage ?? d ?? {}), - ]); - } - - // newSession response: configOptions / modes - const ns = run.stepResults.find((s) => s.op === "newSession")?.result ?? {}; - const configCategories = (ns.configOptions ?? []) - .map((o: any) => o.category) - .filter(Boolean); - const modes = ns.modes ?? null; - // prompt response usage / stopReason - const promptRes = run.stepResults - .filter((s) => s.op === "prompt") - .map((s) => s.result ?? {}); - const stopReasons = promptRes.map((r) => r.stopReason).filter(Boolean); - const promptUsage = promptRes.some( - (r) => r.usage && Object.keys(r.usage).length > 0, - ); - - return { - fatalError: run.fatalError ?? null, - updateTypes: [...updateTypes].sort(), - toolKinds: [...toolKinds].sort(), - toolStatuses: [...toolStatuses].sort(), - hasDiffContent: hasDiff, - hasToolContent: hasToolContent, - hasUsage: - promptUsage || - updateTypes.has("usage_update") || - extNotifs.has("_posthog/usage_update"), - usageFields: [...usageFields].sort(), - configOptionCategories: [...new Set(configCategories)].sort(), - modesPresent: !!modes, - modeChangeEmitted: modeUpdate, - approvalsRequested: approvals.length, - extNotifications: [...extNotifs].sort(), - stopReasons, - steps: run.stepResults.map((s) => ({ op: s.op, ok: s.ok, error: s.error })), - }; -} - -// Adapter-level features must match for parity. tool-rendering features depend -// on which tools the model chose (native codex edits via shell `execute`; -// codex-acp exposes Edit/Read) — a tool-surface difference, not an adapter bug — -// so they're reported as behavioral, not counted as parity gaps. -const ADAPTER_KEYS = [ - "fatalError", - "updateTypes", - "hasUsage", - "usageFields", - "configOptionCategories", - "modesPresent", - "modeChangeEmitted", - "extNotifications", - "stopReasons", -]; -const BEHAVIORAL_KEYS = [ - "toolKinds", - "toolStatuses", - "hasDiffContent", - "hasToolContent", -]; - -function diffFeatures( - acp: Record, - app: Record, -): Array<{ - feature: string; - acp: any; - appServer: any; - match: boolean; - behavioral: boolean; -}> { - const j = (v: any) => JSON.stringify(v); - const mk = (k: string, behavioral: boolean) => ({ - feature: k, - acp: acp[k], - appServer: app[k], - match: j(acp[k]) === j(app[k]), - behavioral, - }); - return [ - ...ADAPTER_KEYS.map((k) => mk(k, false)), - ...BEHAVIORAL_KEYS.map((k) => mk(k, true)), - ]; -} - -/** - * Recreate the repo from scratch. Runs before every (scenario, mode) pair so - * the second arm starts from the same pristine state as the first — a scenario - * edits target.txt, and comparing arms against different starting files would - * bias the exact diff this harness exists to produce. - */ -function setupRepo(): void { - rmSync(REPO, { recursive: true, force: true }); - mkdirSync(REPO, { recursive: true }); - execFileSync("git", ["init", "-q"], { cwd: REPO }); - writeFileSync(join(REPO, "target.txt"), "line1\nline2\nline3\n"); - execFileSync("git", ["add", "-A"], { cwd: REPO }); - // -c commit.gpgsign=false: ignore the user's global commit-signing config - // (e.g. 1Password SSH signer), which fails in this non-interactive context. - execFileSync( - "git", - [ - "-c", - "commit.gpgsign=false", - "-c", - "user.email=p@p.dev", - "-c", - "user.name=parity", - "commit", - "-qm", - "init", - ], - { cwd: REPO }, - ); -} - -async function main(): Promise { - const args = process.argv.slice(2); - const only = args.includes("--only") - ? (args[args.indexOf("--only") + 1] as AdapterMode) - : null; - const scenarioFilter = args.includes("--scenario") - ? args[args.indexOf("--scenario") + 1] - : null; - mkdirSync(OUT_DIR, { recursive: true }); - - const modes: AdapterMode[] = []; - if (!only || only === "acp") modes.push("acp"); - if ((!only || only === "app-server") && existsSync(NATIVE_CODEX_BIN)) - modes.push("app-server"); - else if (only === "app-server") - console.warn( - `native codex binary missing at ${NATIVE_CODEX_BIN}; app-server arm skipped`, - ); - - const scenarios = SCENARIOS.filter( - (s) => !scenarioFilter || s.name === scenarioFilter, - ); - const logger = new Logger({ - debug: !!process.env.PARITY_DEBUG, - prefix: "[parity]", - }); - const featuresByMode: Record> = {}; - - for (const scenario of scenarios) { - featuresByMode[scenario.name] = {}; - for (const mode of modes) { - console.log(`\n▶ ${scenario.name} via ${mode} ...`); - setupRepo(); - // codex spawns detached (own process group); a timed-out run orphans it - // holding a flock under ~/.codex/tmp, which wedges the next run. Kill any - // stragglers first — process death releases the flock, matched on THIS - // checkout's absolute resources path so unrelated runs are never killed. - // (Uses the default CODEX_HOME: an isolated empty home makes codex-acp - // crash at startup.) - try { - execFileSync("pkill", ["-9", "-f", RESOURCES], { - stdio: "ignore", - }); - } catch { - /* none running */ - } - const cfg = { - cwd: REPO, - codexOptions: { - cwd: REPO, - binaryPath: CODEX_ACP_BIN, - apiBaseUrl: GATEWAY, - apiKey: API_KEY, - model: MODEL, - }, - timeoutMs: 240000, - logger, - }; - const run = await runScenario(mode, scenario, cfg); - writeFileSync( - join(OUT_DIR, `${scenario.name}.${mode}.json`), - JSON.stringify(run, null, 2), - ); - const feats = extractFeatures(run); - featuresByMode[scenario.name][mode] = feats; - writeFileSync( - join(OUT_DIR, `${scenario.name}.${mode}.features.json`), - JSON.stringify(feats, null, 2), - ); - console.log( - ` steps: ${feats.steps.map((s: any) => `${s.op}${s.ok ? "✓" : "✗"}`).join(" ")}`, - ); - console.log( - ` updates: ${feats.updateTypes.join(",")} | tools: ${feats.toolKinds.join(",")} | usage:${feats.hasUsage} diff:${feats.hasDiffContent} stop:${feats.stopReasons.join(",")}`, - ); - if (feats.fatalError) console.log(` ⚠ fatalError: ${feats.fatalError}`); - } - } - - // Diff report (only meaningful when both arms ran) - const report: any = { gateway: GATEWAY, model: MODEL, scenarios: {} }; - let totalGaps = 0; - for (const scenario of scenarios) { - const acp = featuresByMode[scenario.name].acp; - const app = featuresByMode[scenario.name]["app-server"]; - if (acp && app) { - const diff = diffFeatures(acp, app); - const gaps = diff.filter((d) => !d.match && !d.behavioral); - const behavioral = diff.filter((d) => !d.match && d.behavioral); - totalGaps += gaps.length; - report.scenarios[scenario.name] = { - gaps, - behavioral, - allMatch: gaps.length === 0, - }; - console.log(`\n=== parity diff: ${scenario.name} ===`); - if (!gaps.length) console.log(" ✅ adapter parity"); - for (const g of gaps) - console.log( - ` ✗ ${g.feature}: acp=${JSON.stringify(g.acp)} app-server=${JSON.stringify(g.appServer)}`, - ); - for (const b of behavioral) - console.log( - ` · behavioral: ${b.feature} acp=${JSON.stringify(b.acp)} app-server=${JSON.stringify(b.appServer)}`, - ); - } else { - report.scenarios[scenario.name] = { - baselineOnly: acp ? "acp" : "app-server", - features: acp ?? app, - }; - } - } - writeFileSync( - join(OUT_DIR, "parity-report.json"), - JSON.stringify(report, null, 2), - ); - console.log( - `\nWrote ${join(OUT_DIR, "parity-report.json")} — ${totalGaps} parity gap(s).`, - ); - process.exit(totalGaps > 0 ? 1 : 0); -} - -main().catch((e) => { - console.error("parity runner failed:", e); - process.exit(2); -}); diff --git a/packages/agent/src/adapters/acp-connection.test.ts b/packages/agent/src/adapters/acp-connection.test.ts deleted file mode 100644 index b1b1d82833..0000000000 --- a/packages/agent/src/adapters/acp-connection.test.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { afterEach, describe, expect, it } from "vitest"; -import { resolveUseCodexAppServer } from "./acp-connection"; - -describe("resolveUseCodexAppServer", () => { - const saved = { - app: process.env.POSTHOG_CODEX_USE_APP_SERVER, - acp: process.env.POSTHOG_CODEX_USE_ACP, - }; - afterEach(() => { - if (saved.app === undefined) - delete process.env.POSTHOG_CODEX_USE_APP_SERVER; - else process.env.POSTHOG_CODEX_USE_APP_SERVER = saved.app; - if (saved.acp === undefined) delete process.env.POSTHOG_CODEX_USE_ACP; - else process.env.POSTHOG_CODEX_USE_ACP = saved.acp; - }); - - it("host flag wins over env and default", () => { - process.env.POSTHOG_CODEX_USE_ACP = "1"; - process.env.POSTHOG_CODEX_USE_APP_SERVER = "1"; - expect(resolveUseCodexAppServer({ useCodexAppServer: false })).toBe(false); - expect(resolveUseCodexAppServer({ useCodexAppServer: true })).toBe(true); - }); - - it("POSTHOG_CODEX_USE_APP_SERVER=1 forces app-server", () => { - delete process.env.POSTHOG_CODEX_USE_ACP; - process.env.POSTHOG_CODEX_USE_APP_SERVER = "1"; - expect(resolveUseCodexAppServer({})).toBe(true); - }); - - it("POSTHOG_CODEX_USE_ACP=1 forces codex-acp", () => { - delete process.env.POSTHOG_CODEX_USE_APP_SERVER; - process.env.POSTHOG_CODEX_USE_ACP = "1"; - expect(resolveUseCodexAppServer({})).toBe(false); - }); - - it("defaults to codex-acp when nothing is set (app-server is opt-in)", () => { - delete process.env.POSTHOG_CODEX_USE_APP_SERVER; - delete process.env.POSTHOG_CODEX_USE_ACP; - expect(resolveUseCodexAppServer({})).toBe(false); - }); - - it("host flag false beats POSTHOG_CODEX_USE_APP_SERVER=1", () => { - process.env.POSTHOG_CODEX_USE_APP_SERVER = "1"; - delete process.env.POSTHOG_CODEX_USE_ACP; - expect(resolveUseCodexAppServer({ useCodexAppServer: false })).toBe(false); - }); -}); diff --git a/packages/agent/src/adapters/acp-connection.ts b/packages/agent/src/adapters/acp-connection.ts index 35e197327d..f67aaeb477 100644 --- a/packages/agent/src/adapters/acp-connection.ts +++ b/packages/agent/src/adapters/acp-connection.ts @@ -9,10 +9,9 @@ import { } from "../utils/streams"; import { ClaudeAcpAgent } from "./claude/claude-agent"; import type { GatewayEnv } from "./claude/session/options"; -import { CodexAcpAgent } from "./codex/codex-agent"; -import type { CodexProcessOptions } from "./codex/spawn"; import { nativeCodexBinaryPath } from "./codex-app-server/binary-path"; import { CodexAppServerAgent } from "./codex-app-server/codex-app-server-agent"; +import type { CodexOptions } from "./codex-app-server/spawn"; type AgentAdapter = "claude" | "codex"; @@ -25,16 +24,8 @@ export type AcpConnectionConfig = { deviceType?: "local" | "cloud"; logger?: Logger; processCallbacks?: ProcessSpawnedCallback; - codexOptions?: CodexProcessOptions; + codexOptions?: CodexOptions; allowedModelIds?: Set; - /** - * Feature-flag lever for the codex sub-adapter, passed by the host from the - * `codex-app-server` PostHog flag (gradual rollout / kill-switch). `true` => - * native app-server, `false` => codex-acp. When undefined, falls back to env - * overrides then the default (codex-acp). Lets app-server roll out alongside - * codex-acp without a code change. - */ - useCodexAppServer?: boolean; /** Callback invoked when the agent calls the create_output tool for structured output */ onStructuredOutput?: (output: Record) => Promise; /** PostHog API config; when set, enables file-read enrichment unless disabled. */ @@ -78,24 +69,6 @@ function resolveEnricherApiConfig( return enabled ? config.posthogApiConfig : undefined; } -/** - * Resolves which codex sub-adapter to use. Precedence: host flag - * (`config.useCodexAppServer`, from the `codex-app-server` PostHog flag) > env - * overrides (`POSTHOG_CODEX_USE_APP_SERVER=1` / `POSTHOG_CODEX_USE_ACP=1`) > - * default (codex-acp, the proven fallback). The native app-server is opt-in: - * the host turns it on per-user via the flag (cloud passes the resolved env; - * desktop passes `useCodexAppServer`), so it can roll out alongside codex-acp - * without a code change and be killed instantly by flipping the flag off. - */ -export function resolveUseCodexAppServer(config: AcpConnectionConfig): boolean { - if (typeof config.useCodexAppServer === "boolean") { - return config.useCodexAppServer; - } - if (process.env.POSTHOG_CODEX_USE_APP_SERVER === "1") return true; - if (process.env.POSTHOG_CODEX_USE_ACP === "1") return false; - return false; -} - function createClaudeConnection(config: AcpConnectionConfig): AcpConnection { const logger = config.logger?.child("AcpConnection") ?? @@ -178,11 +151,9 @@ function createClaudeConnection(config: AcpConnectionConfig): AcpConnection { } /** - * Creates an ACP connection to codex-acp via an in-process proxy agent. - * - * The CodexAcpAgent implements the ACP Agent interface and delegates to - * the codex-acp binary over a ClientSideConnection. This replaces the - * previous raw stream transform approach and gives us proper interception + * Creates an ACP connection to the native codex app-server via an in-process + * proxy agent. CodexAppServerAgent implements the ACP Agent interface and + * delegates to `codex app-server` over JSON-RPC, giving us interception * points for PostHog-specific features. */ function createCodexConnection(config: AcpConnectionConfig): AcpConnection { @@ -231,48 +202,37 @@ function createCodexConnection(config: AcpConnectionConfig): AcpConnection { const agentStream = ndJsonStream(agentWritable, streams.agent.readable); - let agent: CodexAcpAgent | CodexAppServerAgent | null = null; + let agent: CodexAppServerAgent | null = null; const agentConnection = new AgentSideConnection((client) => { const codexOptions = config.codexOptions ?? {}; const nativeBinary = nativeCodexBinaryPath(codexOptions.binaryPath); - // Use the native app-server when its binary is bundled AND the host (flag) - // / env selects it. See resolveUseCodexAppServer for precedence. - const useAppServer = !!nativeBinary && resolveUseCodexAppServer(config); - logger.info( - `Codex sub-adapter selected: ${useAppServer ? "app-server (native codex)" : "codex-acp"}`, - { - useAppServer, - nativeBinaryFound: !!nativeBinary, - hostFlag: config.useCodexAppServer, - }, - ); - if (useAppServer) { - agent = new CodexAppServerAgent(client, { - processOptions: { - binaryPath: nativeBinary, - cwd: codexOptions.cwd, - apiBaseUrl: codexOptions.apiBaseUrl, - apiKey: codexOptions.apiKey, - codexHome: codexOptions.codexHome, - developerInstructions: codexOptions.developerInstructions, - configOverrides: codexOptions.configOverrides, - }, - model: codexOptions.model, - reasoningEffort: codexOptions.reasoningEffort, - processCallbacks: config.processCallbacks, - onStructuredOutput: config.onStructuredOutput, - logger: config.logger?.child("CodexAppServerAgent"), - }); - return agent; + // The native app-server is the only codex harness. A missing binary is a + // packaging bug — fail loudly instead of degrading. + if (!nativeBinary) { + throw new Error( + "native codex binary not found (looked next to " + + `${codexOptions.binaryPath ?? ""} and in @openai/codex). ` + + "Bundle the codex binary or install the @openai/codex dependency.", + ); } - - agent = new CodexAcpAgent(client, { - codexProcessOptions: { ...codexOptions, environment: config.deviceType }, + logger.info("Codex app-server selected", { nativeBinary }); + + agent = new CodexAppServerAgent(client, { + processOptions: { + binaryPath: nativeBinary, + cwd: codexOptions.cwd, + apiBaseUrl: codexOptions.apiBaseUrl, + apiKey: codexOptions.apiKey, + codexHome: codexOptions.codexHome, + developerInstructions: codexOptions.developerInstructions, + configOverrides: codexOptions.configOverrides, + }, + model: codexOptions.model, + reasoningEffort: codexOptions.reasoningEffort, processCallbacks: config.processCallbacks, - posthogApiConfig: resolveEnricherApiConfig(config), onStructuredOutput: config.onStructuredOutput, - logger: config.logger?.child("CodexAcpAgent"), + logger: config.logger?.child("CodexAppServerAgent"), }); return agent; }, agentStream); diff --git a/packages/agent/src/adapters/codex-app-server/binary-path.ts b/packages/agent/src/adapters/codex-app-server/binary-path.ts index 6af43597cb..a692bbbeb7 100644 --- a/packages/agent/src/adapters/codex-app-server/binary-path.ts +++ b/packages/agent/src/adapters/codex-app-server/binary-path.ts @@ -1,6 +1,6 @@ import { existsSync } from "node:fs"; import { createRequire } from "node:module"; -import { dirname, join } from "node:path"; +import { basename, dirname, join } from "node:path"; /** * Node `platform-arch` → codex target triple + `@openai/codex` platform sub-package @@ -67,15 +67,20 @@ function vendoredCodexBinary(): string | undefined { /** * Path to the native codex CLI (the one that exposes `app-server`), or undefined - * when unavailable. Two sources in order: bundled next to codex-acp, then vendored - * by the `@openai/codex` npm dependency. + * when unavailable. Sources in order: the hint itself when it already points at + * the codex binary, a `codex` sibling of the hint (older hosts pass another + * bundled binary's path in the same directory), then the binary vendored by the + * `@openai/codex` npm dependency. */ export function nativeCodexBinaryPath( - codexAcpPath?: string, + bundledHintPath?: string, ): string | undefined { const binaryName = process.platform === "win32" ? "codex.exe" : "codex"; - if (codexAcpPath) { - const candidate = join(dirname(codexAcpPath), binaryName); + if (bundledHintPath) { + const candidate = + basename(bundledHintPath) === binaryName + ? bundledHintPath + : join(dirname(bundledHintPath), binaryName); if (existsSync(candidate)) return candidate; } return vendoredCodexBinary(); diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts index 1c94abfeec..b5a4562b28 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts @@ -115,9 +115,8 @@ export interface CodexAppServerAgentOptions { } /** - * ACP Agent backed by the native Codex `app-server` JSON-RPC protocol. Presents the - * same ACP surface to PostHog Code as the codex-acp adapter, without the Zed - * translation layer, and stays at parity with it on the adapter surface. + * ACP Agent backed by the native Codex `app-server` JSON-RPC protocol, + * presenting the ACP surface PostHog Code expects. */ export class CodexAppServerAgent extends BaseAcpAgent { readonly adapterName = "codex"; @@ -455,7 +454,7 @@ export class CodexAppServerAgent extends BaseAcpAgent { return { configOptions: this.config.options }; } - /** codex-acp emits current_mode_update on mode change; mirror it for the host's mode cache. */ + /** Emit current_mode_update on mode change for the host's mode cache. */ private emitCurrentMode(modeId: string): void { if (!this.sessionId) return; void this.client @@ -781,7 +780,7 @@ export class CodexAppServerAgent extends BaseAcpAgent { .catch(() => undefined); } - /** Mirror codex-acp's `_posthog/usage_update` so the host's token/cost UI fills. */ + /** Emit `_posthog/usage_update` so the host's token/cost UI fills. */ private emitUsageExtNotification(params: unknown): void { if (!this.sessionId) return; const update = this.usage.ingest(params); diff --git a/packages/agent/src/adapters/codex-app-server/ext-notifications.ts b/packages/agent/src/adapters/codex-app-server/ext-notifications.ts index f6898c8fa2..a692eaa421 100644 --- a/packages/agent/src/adapters/codex-app-server/ext-notifications.ts +++ b/packages/agent/src/adapters/codex-app-server/ext-notifications.ts @@ -1,6 +1,6 @@ /** * Pure builders for the PostHog `_posthog/*` ext-notification params the app-server - * adapter emits, mirroring the codex-acp adapter so log consumers and the renderer + * adapter emits so log consumers and the renderer * see the same shapes. Param-only (no I/O) so each is unit-testable in isolation. */ @@ -60,7 +60,7 @@ export interface AccumulatedUsage { /** * `_posthog/turn_complete` — fired when a prompt turn finishes. `totalTokens` is the - * sum of all four component counts, matching the codex-acp adapter. + * sum of all four component counts. */ export function buildTurnCompleteParams( sessionId: string, diff --git a/packages/agent/src/adapters/codex/local-tools-mcp-server.ts b/packages/agent/src/adapters/codex-app-server/local-tools-mcp-server.ts similarity index 96% rename from packages/agent/src/adapters/codex/local-tools-mcp-server.ts rename to packages/agent/src/adapters/codex-app-server/local-tools-mcp-server.ts index cddf752754..7c65cfe560 100644 --- a/packages/agent/src/adapters/codex/local-tools-mcp-server.ts +++ b/packages/agent/src/adapters/codex-app-server/local-tools-mcp-server.ts @@ -1,6 +1,6 @@ /** * Standalone stdio MCP server exposing the general local tools to the Codex - * adapter. Spawned by codex-acp as an MCP server process. Reads its context + * app-server adapter, which spawns it as an MCP server process. Reads its context * (cwd, taskId, token) from POSTHOG_LOCAL_TOOLS_CTX and the set of tools to * register from POSTHOG_LOCAL_TOOLS_ENABLED (both set by the parent, which has * already evaluated each tool's gate) — then registers those registry tools, diff --git a/packages/agent/src/adapters/codex-app-server/local-tools-mcp.ts b/packages/agent/src/adapters/codex-app-server/local-tools-mcp.ts index 5461da1e8e..4461c1de46 100644 --- a/packages/agent/src/adapters/codex-app-server/local-tools-mcp.ts +++ b/packages/agent/src/adapters/codex-app-server/local-tools-mcp.ts @@ -1,6 +1,6 @@ /** * Builds the stdio local-tools MCP server config to inject into a Codex - * app-server thread's `config.mcp_servers`, ported from the codex-acp adapter. + * app-server thread's `config.mcp_servers`. * Returns the ACP `McpServerStdio` shape so the existing translation layer stays * the single owner of the ACP→Codex map. */ @@ -33,7 +33,7 @@ function toMcpServerStdio( enabledNames: string[], ): McpServerStdio { const scriptPath = resolveBundledMcpScript( - "adapters/codex/local-tools-mcp-server.js", + "adapters/codex-app-server/local-tools-mcp-server.js", ); const ctxBase64 = Buffer.from(JSON.stringify(ctx)).toString("base64"); const env = [ diff --git a/packages/agent/src/adapters/codex/models.test.ts b/packages/agent/src/adapters/codex-app-server/models.test.ts similarity index 100% rename from packages/agent/src/adapters/codex/models.test.ts rename to packages/agent/src/adapters/codex-app-server/models.test.ts diff --git a/packages/agent/src/adapters/codex/models.ts b/packages/agent/src/adapters/codex-app-server/models.ts similarity index 100% rename from packages/agent/src/adapters/codex/models.ts rename to packages/agent/src/adapters/codex-app-server/models.ts diff --git a/packages/agent/src/adapters/codex-app-server/session-config.ts b/packages/agent/src/adapters/codex-app-server/session-config.ts index d39353cbbb..1ef9b1edfd 100644 --- a/packages/agent/src/adapters/codex-app-server/session-config.ts +++ b/packages/agent/src/adapters/codex-app-server/session-config.ts @@ -1,7 +1,7 @@ import type { SessionConfigOption } from "@agentclientprotocol/sdk"; import { CODEX_MODE_PRESETS, type CodexModePreset } from "@posthog/shared"; import { type GatewayModel, isOpenAIModel } from "../../gateway-models"; -import { getReasoningEffortOptions } from "../codex/models"; +import { getReasoningEffortOptions } from "./models"; /** * Session config + mode synthesis for the codex app-server adapter. The native diff --git a/packages/agent/src/adapters/codex/settings.test.ts b/packages/agent/src/adapters/codex-app-server/settings.test.ts similarity index 100% rename from packages/agent/src/adapters/codex/settings.test.ts rename to packages/agent/src/adapters/codex-app-server/settings.test.ts diff --git a/packages/agent/src/adapters/codex/settings.ts b/packages/agent/src/adapters/codex-app-server/settings.ts similarity index 98% rename from packages/agent/src/adapters/codex/settings.ts rename to packages/agent/src/adapters/codex-app-server/settings.ts index ada2ae4afa..cfe8240e48 100644 --- a/packages/agent/src/adapters/codex/settings.ts +++ b/packages/agent/src/adapters/codex-app-server/settings.ts @@ -77,7 +77,7 @@ export class CodexSettingsManager { * describes the `env` field of server `foo`, not a separate server, so it must * collapse to `foo`. Treating it as its own server emits * `mcp_servers.foo.env.enabled=false`, which sets a boolean on the string-typed - * env map and makes codex-acp reject the whole config (it then crashes and the + * env map and makes codex reject the whole config (it then crashes and the * host silently falls back to Claude). Quoted segments (`"a.b"`) keep their dots. */ function firstMcpServerName(sectionPath: string): string | null { diff --git a/packages/agent/src/adapters/codex-app-server/spawn.ts b/packages/agent/src/adapters/codex-app-server/spawn.ts index d8ad7306a0..28d9d30516 100644 --- a/packages/agent/src/adapters/codex-app-server/spawn.ts +++ b/packages/agent/src/adapters/codex-app-server/spawn.ts @@ -5,7 +5,37 @@ import type { Readable, Writable } from "node:stream"; import type { ProcessSpawnedCallback } from "../../types"; import { Logger } from "../../utils/logger"; import { stripElectronNodeShimFromPath } from "../../utils/spawn-env"; -import { CodexSettingsManager } from "../codex/settings"; +import { CodexSettingsManager } from "./settings"; + +/** + * Host-facing codex options passed through `createAcpConnection`'s + * `codexOptions`. The connection layer maps these onto + * `CodexAppServerProcessOptions` plus the agent-level model settings. + */ +export interface CodexOptions { + cwd?: string; + apiBaseUrl?: string; + apiKey?: string; + model?: string; + reasoningEffort?: string; + /** Guidance appended on top of Codex's base prompt via `developer_instructions`. */ + developerInstructions?: string; + /** + * Bundled-binary hint: the native codex binary itself, or any file in the + * directory that contains it (see `nativeCodexBinaryPath`). + */ + binaryPath?: string; + codexHome?: string; + /** Extra codex `-c key=value` config overrides. */ + configOverrides?: Record; + /** + * Additional writable roots. Currently only honored per-thread via prompt + * params; accepted here so hosts can pass it uniformly. + */ + additionalDirectories?: string[]; + logger?: Logger; + processCallbacks?: ProcessSpawnedCallback; +} export interface CodexAppServerProcessOptions { /** Path to the native `codex` CLI binary (the one that exposes `app-server`). */ diff --git a/packages/agent/src/adapters/codex/codex-agent.refresh.test.ts b/packages/agent/src/adapters/codex/codex-agent.refresh.test.ts deleted file mode 100644 index ad70509724..0000000000 --- a/packages/agent/src/adapters/codex/codex-agent.refresh.test.ts +++ /dev/null @@ -1,289 +0,0 @@ -import { Readable, Writable } from "node:stream"; -import type { AgentSideConnection, McpServer } from "@agentclientprotocol/sdk"; -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { POSTHOG_METHODS } from "../../acp-extensions"; - -type MockCodexConnection = { - initialize: ReturnType; - newSession: ReturnType; - loadSession: ReturnType; - setSessionMode: ReturnType; - listSessions: ReturnType; - prompt: ReturnType; - setSessionConfigOption: ReturnType; - cancel: ReturnType; -}; - -type SpawnHandle = { - process: { pid: number }; - stdin: Writable; - stdout: Readable; - kill: ReturnType; -}; - -const hoisted = vi.hoisted(() => { - // Everything the mock factories depend on must live here — vi.mock() - // invocations are hoisted above any other top-level code. - const createdConnections: MockCodexConnection[] = []; - const spawnedProcesses: SpawnHandle[] = []; - - const makeConnection = (): MockCodexConnection => ({ - initialize: vi.fn().mockResolvedValue({ - protocolVersion: 1, - agentCapabilities: {}, - }), - newSession: vi.fn(), - loadSession: vi.fn().mockResolvedValue({ - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - }), - setSessionMode: vi.fn().mockResolvedValue({}), - listSessions: vi.fn(), - prompt: vi.fn(), - setSessionConfigOption: vi.fn(), - cancel: vi.fn().mockResolvedValue(undefined), - }); - - const clientSideConnectionCtor = class { - constructor() { - Object.assign(this, makeConnection()); - createdConnections.push(this as unknown as MockCodexConnection); - } - }; - - const spawnCodexProcessMock = vi.fn(() => { - const handle: SpawnHandle = { - process: { pid: 1000 + spawnedProcesses.length }, - stdin: new Writable({ - write(_chunk, _encoding, callback) { - callback(); - }, - }), - stdout: new Readable({ read() {} }), - kill: vi.fn(), - }; - spawnedProcesses.push(handle); - return handle; - }); - - return { - createdConnections, - spawnedProcesses, - clientSideConnectionCtor, - spawnCodexProcessMock, - }; -}); - -const createdConnections = hoisted.createdConnections; -const spawnedProcesses = hoisted.spawnedProcesses; - -vi.mock("@agentclientprotocol/sdk", async () => { - const actual = await vi.importActual("@agentclientprotocol/sdk"); - return { - ...actual, - ClientSideConnection: hoisted.clientSideConnectionCtor, - ndJsonStream: vi.fn(() => ({}) as object), - }; -}); - -vi.mock("./spawn", () => ({ - spawnCodexProcess: hoisted.spawnCodexProcessMock, -})); - -vi.mock("./settings", () => ({ - CodexSettingsManager: class { - constructor(private readonly cwd: string) {} - initialize = vi.fn().mockResolvedValue(undefined); - dispose = vi.fn(); - getCwd = () => this.cwd; - setCwd = vi.fn(); - getSettings = () => ({ mcpServerNames: [] }); - }, -})); - -import { CodexAcpAgent } from "./codex-agent"; - -type PrivateAgent = { - session: { - abortController: AbortController; - settingsManager: { dispose: ReturnType }; - notificationHistory: unknown[]; - promptRunning: boolean; - }; - sessionId: string; - sessionState: { - sessionId: string; - cwd: string; - accumulatedUsage: { - inputTokens: number; - outputTokens: number; - cachedReadTokens: number; - cachedWriteTokens: number; - }; - configOptions: unknown[]; - taskRunId?: string; - }; - codexProcess: SpawnHandle; - codexConnection: MockCodexConnection; - lastInitRequest?: { protocolVersion: number }; -}; - -function makeAgent(): CodexAcpAgent { - const client = { - extNotification: vi.fn().mockResolvedValue(undefined), - } as unknown as AgentSideConnection; - return new CodexAcpAgent(client, { - codexProcessOptions: { cwd: "/tmp/repo" }, - }); -} - -function primeSession( - agent: CodexAcpAgent, - sessionId: string, -): { - oldProcess: SpawnHandle; - oldConnection: MockCodexConnection; - priv: PrivateAgent; -} { - const priv = agent as unknown as PrivateAgent; - priv.sessionId = sessionId; - priv.sessionState = { - sessionId, - cwd: "/tmp/repo", - accumulatedUsage: { - inputTokens: 42, - outputTokens: 17, - cachedReadTokens: 0, - cachedWriteTokens: 0, - }, - configOptions: [{ id: "opt", value: "x" }], - taskRunId: "run-1", - }; - priv.session.notificationHistory = [{ foo: "bar" }]; - priv.lastInitRequest = { protocolVersion: 1 }; - return { - oldProcess: priv.codexProcess, - oldConnection: priv.codexConnection, - priv, - }; -} - -describe("CodexAcpAgent.extMethod refresh_session", () => { - beforeEach(() => { - spawnedProcesses.length = 0; - createdConnections.length = 0; - }); - - it("returns methodNotFound for unknown extension methods", async () => { - const agent = makeAgent(); - await expect(agent.extMethod("_posthog/nope", {})).rejects.toThrow( - /Method not found/i, - ); - }); - - it("rejects when mcpServers is missing", async () => { - const agent = makeAgent(); - await expect( - agent.extMethod(POSTHOG_METHODS.REFRESH_SESSION, {}), - ).rejects.toThrow(/at least one refreshable field/); - }); - - it("rejects when mcpServers is not an array", async () => { - const agent = makeAgent(); - await expect( - agent.extMethod(POSTHOG_METHODS.REFRESH_SESSION, { - mcpServers: "nope" as unknown, - }), - ).rejects.toThrow(/mcpServers must be an array/); - }); - - it("rejects refresh while a prompt is in flight", async () => { - const agent = makeAgent(); - const { priv } = primeSession(agent, "s-1"); - priv.session.promptRunning = true; - - await expect( - agent.extMethod(POSTHOG_METHODS.REFRESH_SESSION, { - mcpServers: [ - { name: "posthog", type: "http", url: "https://new", headers: [] }, - ], - }), - ).rejects.toThrow(/prompt turn is in flight/); - }); - - it("respawns the subprocess, re-initializes, and rehydrates with new MCP servers", async () => { - const agent = makeAgent(); - const { oldProcess, oldConnection, priv } = primeSession(agent, "s-2"); - const oldAbortController = priv.session.abortController; - const oldSettingsManager = priv.session.settingsManager; - - const mcpServers: McpServer[] = [ - { - name: "posthog", - type: "http", - url: "https://fresh", - headers: [{ name: "x-foo", value: "bar" }], - }, - ]; - - const result = await agent.extMethod(POSTHOG_METHODS.REFRESH_SESSION, { - mcpServers, - }); - - expect(result).toEqual({ refreshed: true }); - - // Old subprocess torn down, old connection cancelled. - expect(oldConnection.cancel).toHaveBeenCalledWith({ sessionId: "s-2" }); - expect(oldProcess.kill).toHaveBeenCalledTimes(1); - expect(oldAbortController.signal.aborted).toBe(true); - expect(oldSettingsManager.dispose).toHaveBeenCalledTimes(1); - - // A fresh subprocess was spawned and a new ClientSideConnection wired up. - expect(spawnedProcesses).toHaveLength(2); - expect(createdConnections).toHaveLength(2); - const newConnection = createdConnections[1]; - if (!newConnection) throw new Error("expected a second connection"); - - // ACP handshake replayed against the new subprocess. - expect(newConnection.initialize).toHaveBeenCalledWith({ - protocolVersion: 1, - }); - expect(newConnection.loadSession).toHaveBeenCalledWith({ - sessionId: "s-2", - cwd: "/tmp/repo", - mcpServers, - }); - - // References swapped to the new instances. - expect(priv.codexProcess).toBe(spawnedProcesses[1]); - expect(priv.codexConnection).toBe(newConnection); - expect(priv.session.abortController).not.toBe(oldAbortController); - expect(priv.session.settingsManager).not.toBe(oldSettingsManager); - - // Session-level state preserved across refresh. - expect(priv.sessionState.accumulatedUsage.inputTokens).toBe(42); - expect(priv.sessionState.accumulatedUsage.outputTokens).toBe(17); - expect(priv.sessionState.configOptions).toEqual([ - { id: "opt", value: "x" }, - ]); - expect(priv.sessionState.taskRunId).toBe("run-1"); - expect(priv.session.notificationHistory).toEqual([{ foo: "bar" }]); - }); - - it("does not fail refresh when cancel() throws on the stale connection", async () => { - const agent = makeAgent(); - const { oldConnection } = primeSession(agent, "s-3"); - oldConnection.cancel.mockRejectedValueOnce(new Error("already dead")); - - await expect( - agent.extMethod(POSTHOG_METHODS.REFRESH_SESSION, { - mcpServers: [ - { name: "posthog", type: "http", url: "https://x", headers: [] }, - ], - }), - ).resolves.toEqual({ refreshed: true }); - - expect(spawnedProcesses).toHaveLength(2); - expect(createdConnections[1]?.loadSession).toHaveBeenCalled(); - }); -}); diff --git a/packages/agent/src/adapters/codex/codex-agent.test.ts b/packages/agent/src/adapters/codex/codex-agent.test.ts deleted file mode 100644 index d1e1981109..0000000000 --- a/packages/agent/src/adapters/codex/codex-agent.test.ts +++ /dev/null @@ -1,728 +0,0 @@ -import { Readable, Writable } from "node:stream"; -import type { - AgentSideConnection, - LoadSessionResponse, - NewSessionResponse, -} from "@agentclientprotocol/sdk"; -import { beforeEach, describe, expect, it, vi } from "vitest"; - -const mockCodexConnection = { - initialize: vi.fn(), - newSession: vi.fn(), - loadSession: vi.fn(), - setSessionMode: vi.fn(), - listSessions: vi.fn(), - prompt: vi.fn(), - setSessionConfigOption: vi.fn(), -}; - -const mockKill = vi.fn(); - -vi.mock("@agentclientprotocol/sdk", async () => { - const actual = await vi.importActual("@agentclientprotocol/sdk"); - - return { - ...actual, - ClientSideConnection: class { - constructor() { - Object.assign(this, mockCodexConnection); - } - }, - ndJsonStream: vi.fn(() => ({}) as object), - }; -}); - -vi.mock("./spawn", () => ({ - spawnCodexProcess: vi.fn(() => ({ - process: { pid: 1234 }, - stdin: new Writable({ - write(_chunk, _encoding, callback) { - callback(); - }, - }), - stdout: new Readable({ - read() {}, - }), - kill: mockKill, - })), -})); - -vi.mock("./settings", () => ({ - CodexSettingsManager: class { - constructor(private readonly cwd: string) {} - initialize = vi.fn(); - dispose = vi.fn(); - getCwd = () => this.cwd; - setCwd = vi.fn(); - getSettings = () => ({}); - }, -})); - -vi.mock("node:fs", async (importActual) => { - const actual = await importActual(); - return { ...actual, existsSync: vi.fn(actual.existsSync) }; -}); - -import { CodexAcpAgent } from "./codex-agent"; - -describe("CodexAcpAgent", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - function createAgent( - overrides: Partial = {}, - agentOptions?: { - onStructuredOutput?: (output: Record) => Promise; - }, - ): { - agent: CodexAcpAgent; - client: AgentSideConnection & { - extNotification: ReturnType; - sessionUpdate: ReturnType; - }; - } { - const client = { - extNotification: vi.fn(), - sessionUpdate: vi.fn(), - ...overrides, - } as unknown as AgentSideConnection & { - extNotification: ReturnType; - sessionUpdate: ReturnType; - }; - - const agent = new CodexAcpAgent(client, { - codexProcessOptions: { - cwd: process.cwd(), - }, - onStructuredOutput: agentOptions?.onStructuredOutput, - }); - return { agent, client }; - } - - it("applies the requested initial mode for a new session", async () => { - const { agent } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - - const response = await agent.newSession({ - cwd: process.cwd(), - _meta: { permissionMode: "read-only" }, - } as never); - - expect(mockCodexConnection.setSessionMode).toHaveBeenCalledWith({ - sessionId: "session-1", - modeId: "read-only", - }); - expect( - (agent as unknown as { sessionState: { permissionMode: string } }) - .sessionState.permissionMode, - ).toBe("read-only"); - expect(response.modes?.currentModeId).toBe("read-only"); - }); - - it("returns the applied initial mode in config options", async () => { - const { agent } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "read-only", availableModes: [] }, - configOptions: [ - { - id: "mode", - name: "Mode", - type: "select", - category: "mode", - currentValue: "read-only", - options: [ - { value: "read-only", name: "Read Only" }, - { value: "auto", name: "Auto" }, - { value: "full-access", name: "Full Access" }, - ], - }, - ], - } satisfies Partial); - - const response = await agent.newSession({ - cwd: process.cwd(), - _meta: { permissionMode: "full-access" }, - } as never); - - expect(mockCodexConnection.setSessionMode).toHaveBeenCalledWith({ - sessionId: "session-1", - modeId: "full-access", - }); - expect(response.modes?.currentModeId).toBe("full-access"); - expect(response.configOptions?.find((o) => o.id === "mode")).toEqual( - expect.objectContaining({ currentValue: "full-access" }), - ); - expect( - (agent as unknown as { sessionState: { configOptions: unknown[] } }) - .sessionState.configOptions, - ).toEqual(response.configOptions); - }); - - it("propagates taskRunId and fires SDK_SESSION when loading a cloud session", async () => { - const { agent, client } = createAgent(); - mockCodexConnection.loadSession.mockResolvedValue({ - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - - await agent.loadSession({ - sessionId: "session-1", - cwd: process.cwd(), - _meta: { taskRunId: "run-1", taskId: "task-1" }, - } as never); - - expect( - (agent as unknown as { sessionState: { taskRunId?: string } }) - .sessionState.taskRunId, - ).toBe("run-1"); - expect(client.extNotification).toHaveBeenCalledWith( - "_posthog/sdk_session", - { - taskRunId: "run-1", - sessionId: "session-1", - adapter: "codex", - }, - ); - }); - - it("does not emit SDK_SESSION on loadSession when taskRunId is absent", async () => { - const { agent, client } = createAgent(); - mockCodexConnection.loadSession.mockResolvedValue({ - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - - await agent.loadSession({ - sessionId: "session-1", - cwd: process.cwd(), - } as never); - - expect(client.extNotification).not.toHaveBeenCalled(); - }); - - it("preserves the live session mode when loading an existing session", async () => { - const { agent } = createAgent(); - mockCodexConnection.loadSession.mockResolvedValue({ - modes: { currentModeId: "read-only", availableModes: [] }, - configOptions: [], - } satisfies Partial); - - await agent.loadSession({ - sessionId: "session-1", - cwd: process.cwd(), - _meta: { permissionMode: "auto" }, - } as never); - - expect(mockCodexConnection.setSessionMode).not.toHaveBeenCalled(); - expect( - (agent as unknown as { sessionState: { permissionMode: string } }) - .sessionState.permissionMode, - ).toBe("read-only"); - }); - - it("updates local permission state when changing codex mode config", async () => { - const { agent, client } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - mockCodexConnection.setSessionConfigOption.mockResolvedValue({ - configOptions: [ - { - id: "mode", - name: "Mode", - type: "select", - category: "mode", - currentValue: "full-access", - options: [ - { value: "read-only", name: "Read Only" }, - { value: "auto", name: "Auto" }, - { value: "full-access", name: "Full Access" }, - ], - }, - ], - }); - - await agent.newSession({ - cwd: process.cwd(), - _meta: { permissionMode: "auto" }, - } as never); - await agent.setSessionConfigOption({ - sessionId: "session-1", - configId: "mode", - value: "full-access", - }); - - expect( - (agent as unknown as { sessionState: { permissionMode: string } }) - .sessionState.permissionMode, - ).toBe("full-access"); - expect(client.sessionUpdate).toHaveBeenCalledWith({ - sessionId: "session-1", - update: { - sessionUpdate: "current_mode_update", - currentModeId: "full-access", - }, - }); - }); - - it("prepends _meta.prContext to the forwarded prompt but not to the broadcast", async () => { - const { agent, client } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - await agent.newSession({ - cwd: process.cwd(), - } as never); - - mockCodexConnection.prompt.mockResolvedValue({ stopReason: "end_turn" }); - - await agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "ship the fix" }], - _meta: { prContext: "PR #123 is open; review before editing." }, - } as never); - - // codex-acp receives the PR context prepended as a text block. - expect(mockCodexConnection.prompt).toHaveBeenCalledWith( - expect.objectContaining({ - prompt: [ - { type: "text", text: "PR #123 is open; review before editing." }, - { type: "text", text: "ship the fix" }, - ], - }), - ); - // The broadcast shows only the real user turn — the prContext prefix - // is internal routing and should not render as a user message. - expect(client.sessionUpdate).toHaveBeenCalledTimes(1); - expect(client.sessionUpdate).toHaveBeenCalledWith({ - sessionId: "session-1", - update: { - sessionUpdate: "user_message_chunk", - content: { type: "text", text: "ship the fix" }, - }, - }); - }); - - it("applies a local-skill invocation: drops the /command chunk, injects the skill context, strips the meta", async () => { - const { agent, client } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - await agent.newSession({ - cwd: process.cwd(), - } as never); - - mockCodexConnection.prompt.mockResolvedValue({ stopReason: "end_turn" }); - - await agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "/depparent run the readiness check" }], - _meta: { - localSkillContext: "SKILL INSTRUCTIONS: run the readiness check", - localSkillName: "depparent", - }, - } as never); - - // codex-acp must receive the resolved skill instructions as plain text — - // NOT the bare `/depparent` slash command it would reject — and the - // local-skill meta must not be forwarded. - expect(mockCodexConnection.prompt).toHaveBeenCalledTimes(1); - const forwarded = mockCodexConnection.prompt.mock.calls[0][0]; - expect(forwarded.prompt).toEqual([ - { type: "text", text: "SKILL INSTRUCTIONS: run the readiness check" }, - ]); - expect(forwarded._meta?.localSkillContext).toBeUndefined(); - expect(forwarded._meta?.localSkillName).toBeUndefined(); - // The broadcast still shows the real user turn (the typed command). - expect(client.sessionUpdate).toHaveBeenCalledWith({ - sessionId: "session-1", - update: { - sessionUpdate: "user_message_chunk", - content: { type: "text", text: "/depparent run the readiness check" }, - }, - }); - }); - - it.each([ - [ - "localSkillContext is set but localSkillName is missing", - { localSkillContext: "SKILL INSTRUCTIONS" }, - ], - ["there is no localSkillContext", {}], - ])("forwards the prompt unchanged when %s", async (_label, meta) => { - const { agent } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - await agent.newSession({ cwd: process.cwd() } as never); - mockCodexConnection.prompt.mockResolvedValue({ stopReason: "end_turn" }); - - await agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "/depparent run the readiness check" }], - _meta: meta, - } as never); - - // Without both fields we never inject context nor drop the chunk — the - // prompt must reach codex-acp exactly as given (no context + stray-command - // mix), so the original chunk is preserved verbatim. - const forwarded = mockCodexConnection.prompt.mock.calls[0][0]; - expect(forwarded.prompt).toEqual([ - { type: "text", text: "/depparent run the readiness check" }, - ]); - }); - - it("serializes concurrent prompts so usage accumulators are not wiped mid-turn", async () => { - const { agent } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - await agent.newSession({ - cwd: process.cwd(), - _meta: { taskRunId: "run-1" }, - } as never); - - const callOrder: string[] = []; - let releaseA: () => void; - const aStarted = new Promise((resolve) => { - releaseA = resolve; - }); - let allowAResolve: () => void = () => {}; - const aHold = new Promise((resolve) => { - allowAResolve = resolve; - }); - - mockCodexConnection.prompt.mockImplementationOnce(async () => { - callOrder.push("A:start"); - releaseA(); - await aHold; - callOrder.push("A:end"); - return { stopReason: "end_turn" }; - }); - mockCodexConnection.prompt.mockImplementationOnce(async () => { - callOrder.push("B:start"); - return { stopReason: "end_turn" }; - }); - - const promptA = agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "A" }], - } as never); - - await aStarted; - - const promptB = agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "B" }], - } as never); - - // B must not have started while A is still in-flight. - expect(callOrder).toEqual(["A:start"]); - - allowAResolve(); - await Promise.all([promptA, promptB]); - - expect(callOrder).toEqual(["A:start", "A:end", "B:start"]); - }); - - it("does not let a failing prompt block subsequent prompts", async () => { - const { agent } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - await agent.newSession({ - cwd: process.cwd(), - } as never); - - mockCodexConnection.prompt.mockRejectedValueOnce(new Error("boom")); - mockCodexConnection.prompt.mockResolvedValueOnce({ - stopReason: "end_turn", - }); - - await expect( - agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "A" }], - } as never), - ).rejects.toThrow("boom"); - - await expect( - agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "B" }], - } as never), - ).resolves.toEqual({ stopReason: "end_turn" }); - }); - - it.each([ - ["API Error: 429 rate_limit_error", "upstream_provider_failure"], - ["API Error: 503 internal_error", "upstream_provider_failure"], - ["API Error: 529 overloaded_error", "upstream_provider_failure"], - ["ordinary failure", undefined], - ] as const)( - "handles prompt failure %p", - async (message, expectedClassification) => { - const { agent } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - await agent.newSession({ - cwd: process.cwd(), - } as never); - - const promptError = new Error(message); - mockCodexConnection.prompt.mockRejectedValueOnce(promptError); - - let thrown: unknown; - try { - await agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "A" }], - } as never); - } catch (error) { - thrown = error; - } - - if (!expectedClassification) { - expect(thrown).toBe(promptError); - return; - } - - expect(thrown).toMatchObject({ - data: { - classification: expectedClassification, - result: message, - }, - }); - }, - ); - - it("does not let a classified failing prompt block subsequent prompts", async () => { - const { agent } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - await agent.newSession({ - cwd: process.cwd(), - } as never); - - mockCodexConnection.prompt.mockRejectedValueOnce( - new Error("API Error: 529 overloaded_error"), - ); - mockCodexConnection.prompt.mockResolvedValueOnce({ - stopReason: "end_turn", - }); - - await expect( - agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "A" }], - } as never), - ).rejects.toMatchObject({ - data: { - classification: "upstream_provider_failure", - result: "API Error: 529 overloaded_error", - }, - }); - - await expect( - agent.prompt({ - sessionId: "session-1", - prompt: [{ type: "text", text: "B" }], - } as never), - ).resolves.toEqual({ stopReason: "end_turn" }); - }); - - describe("structured output injection", () => { - const schema = { - type: "object", - properties: { answer: { type: "string" } }, - required: ["answer"], - } as const; - - beforeEach(async () => { - // The resolver checks existsSync to find the compiled MCP script. - // In unit tests the dist asset isn't on the walk-up path, so we - // make the first candidate succeed. Nothing in this test actually - // spawns the script — the agent only forwards the path to codex-acp. - const fs = await import("node:fs"); - vi.mocked(fs.existsSync).mockReturnValue(true); - }); - - it("injects the create_output MCP server and system-prompt note when jsonSchema and callback are present", async () => { - const { agent } = createAgent({}, { onStructuredOutput: vi.fn() }); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - - await agent.newSession({ - cwd: process.cwd(), - mcpServers: [{ name: "existing", command: "echo", args: [], env: [] }], - _meta: { jsonSchema: schema, systemPrompt: "be terse." }, - } as never); - - const forwarded = mockCodexConnection.newSession.mock.calls[0][0] as { - mcpServers: Array<{ name: string; command: string; env: unknown }>; - _meta: { systemPrompt: string }; - }; - - // Existing MCP server is preserved; ours is appended. - expect(forwarded.mcpServers).toHaveLength(2); - expect(forwarded.mcpServers[0].name).toBe("existing"); - expect(forwarded.mcpServers[1].name).toBe("posthog_output"); - expect(forwarded.mcpServers[1].command).toBe(process.execPath); - - // The schema is forwarded base64-encoded so codex-acp doesn't have - // to escape it through a shell. - const envEntry = ( - forwarded.mcpServers[1].env as Array<{ name: string; value: string }> - ).find((e) => e.name === "POSTHOG_OUTPUT_SCHEMA"); - expect(envEntry).toBeDefined(); - const decoded = JSON.parse( - Buffer.from(envEntry?.value ?? "", "base64").toString("utf-8"), - ); - expect(decoded).toEqual(schema); - - // Existing systemPrompt is preserved with the structured-output - // instruction appended (not overwritten). - expect(forwarded._meta.systemPrompt.startsWith("be terse.")).toBe(true); - expect(forwarded._meta.systemPrompt).toContain("create_output"); - }); - - it("is a no-op when jsonSchema is absent", async () => { - const { agent } = createAgent({}, { onStructuredOutput: vi.fn() }); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - - await agent.newSession({ - cwd: process.cwd(), - mcpServers: [], - } as never); - - const forwarded = mockCodexConnection.newSession.mock.calls[0][0] as { - mcpServers: unknown[]; - _meta?: { systemPrompt?: string }; - }; - expect(forwarded.mcpServers).toEqual([]); - expect(forwarded._meta?.systemPrompt).toBeUndefined(); - }); - - it("is a no-op when onStructuredOutput callback is not wired", async () => { - const { agent } = createAgent(); - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - - await agent.newSession({ - cwd: process.cwd(), - mcpServers: [], - _meta: { jsonSchema: schema }, - } as never); - - const forwarded = mockCodexConnection.newSession.mock.calls[0][0] as { - mcpServers: unknown[]; - }; - expect(forwarded.mcpServers).toEqual([]); - }); - - it("also injects on loadSession", async () => { - const { agent } = createAgent({}, { onStructuredOutput: vi.fn() }); - mockCodexConnection.loadSession.mockResolvedValue({ - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - - await agent.loadSession({ - sessionId: "session-1", - cwd: process.cwd(), - mcpServers: [], - _meta: { jsonSchema: schema }, - } as never); - - const forwarded = mockCodexConnection.loadSession.mock.calls[0][0] as { - mcpServers: Array<{ name: string }>; - }; - expect(forwarded.mcpServers.map((s) => s.name)).toContain( - "posthog_output", - ); - }); - }); - - it("broadcasts user prompt as user_message_chunk before delegating to codex-acp", async () => { - const { agent, client } = createAgent(); - // Seed an active session so prompt() has the state it expects. - mockCodexConnection.newSession.mockResolvedValue({ - sessionId: "session-1", - modes: { currentModeId: "auto", availableModes: [] }, - configOptions: [], - } satisfies Partial); - await agent.newSession({ - cwd: process.cwd(), - } as never); - - const callOrder: string[] = []; - client.sessionUpdate.mockImplementation(async () => { - callOrder.push("sessionUpdate"); - }); - mockCodexConnection.prompt.mockImplementation(async () => { - callOrder.push("prompt"); - return { stopReason: "end_turn" }; - }); - - await agent.prompt({ - sessionId: "session-1", - prompt: [ - { type: "text", text: "first chunk" }, - { type: "text", text: "second chunk" }, - ], - } as never); - - expect(client.sessionUpdate).toHaveBeenCalledTimes(2); - expect(client.sessionUpdate).toHaveBeenNthCalledWith(1, { - sessionId: "session-1", - update: { - sessionUpdate: "user_message_chunk", - content: { type: "text", text: "first chunk" }, - }, - }); - expect(client.sessionUpdate).toHaveBeenNthCalledWith(2, { - sessionId: "session-1", - update: { - sessionUpdate: "user_message_chunk", - content: { type: "text", text: "second chunk" }, - }, - }); - // Broadcast must land before the prompt reaches codex-acp so the user - // turn is persisted even if the underlying prompt fails. - expect(callOrder).toEqual(["sessionUpdate", "sessionUpdate", "prompt"]); - }); -}); diff --git a/packages/agent/src/adapters/codex/codex-agent.ts b/packages/agent/src/adapters/codex/codex-agent.ts deleted file mode 100644 index 696c05dd01..0000000000 --- a/packages/agent/src/adapters/codex/codex-agent.ts +++ /dev/null @@ -1,1060 +0,0 @@ -/** - * In-process ACP proxy agent for Codex. - * - * Implements the ACP Agent interface and delegates to the codex-acp binary - * via a ClientSideConnection. This gives us interception points for: - * - PostHog-specific notifications (sdk_session, usage_update, turn_complete) - * - Session resume/fork (not natively supported by codex-acp) - * - Usage accumulation - * - System prompt injection - */ - -import { - type AgentSideConnection, - type AuthenticateRequest, - ClientSideConnection, - type ForkSessionRequest, - type ForkSessionResponse, - type InitializeRequest, - type InitializeResponse, - type ListSessionsRequest, - type ListSessionsResponse, - type LoadSessionRequest, - type LoadSessionResponse, - type McpServer, - type McpServerStdio, - type NewSessionRequest, - type NewSessionResponse, - ndJsonStream, - type PromptRequest, - type PromptResponse, - RequestError, - type ResumeSessionRequest, - type ResumeSessionResponse, - type SessionConfigOption, - type SetSessionConfigOptionRequest, - type SetSessionConfigOptionResponse, - type SetSessionModeRequest, - type SetSessionModeResponse, -} from "@agentclientprotocol/sdk"; -import { ghTokenEnv } from "@posthog/git/signed-commit"; -import packageJson from "../../../package.json" with { type: "json" }; -import { - isMethod, - POSTHOG_METHODS, - POSTHOG_NOTIFICATIONS, -} from "../../acp-extensions"; -import { - createEnrichment, - type Enrichment, -} from "../../enrichment/file-enricher"; -import { - type CodeExecutionMode, - type CodexNativeMode, - isCodeExecutionMode, - isCodexNativeMode, - type PermissionMode, -} from "../../execution-mode"; -import type { PostHogAPIConfig, ProcessSpawnedCallback } from "../../types"; -import { isCloudRun } from "../../utils/common"; -import { resolveGithubToken } from "../../utils/github-token"; -import { Logger } from "../../utils/logger"; -import { resolveBundledMcpScript } from "../../utils/resolve-bundled-script"; -import { - nodeReadableToWebReadable, - nodeWritableToWebWritable, -} from "../../utils/streams"; -import { BaseAcpAgent, type BaseSession } from "../base-acp-agent"; -import { - buildBreakdown, - type ContextBreakdownBaseline, - emptyBaseline, - estimateTokens, -} from "../claude/context-breakdown"; -import { classifyAgentError } from "../error-classification"; -import { isLocalSkillCommandChunk } from "../local-skill"; -import { - enabledLocalTools, - LOCAL_TOOLS_MCP_NAME, - type LocalToolCtx, -} from "../local-tools"; -import { resolveTaskId } from "../session-meta"; -import { createCodexClient } from "./codex-client"; -import { - modelIdFromConfigOptions, - normalizeCodexConfigOptions, -} from "./models"; -import { - type CodexSessionState, - createSessionState, - resetSessionState, - resetUsage, -} from "./session-state"; -import { CodexSettingsManager } from "./settings"; -import { - type CodexProcess, - type CodexProcessOptions, - spawnCodexProcess, -} from "./spawn"; -import { - STRUCTURED_OUTPUT_MCP_NAME, - STRUCTURED_OUTPUT_TOOL_NAME, -} from "./structured-output-constants"; - -export { - STRUCTURED_OUTPUT_MCP_NAME, - STRUCTURED_OUTPUT_TOOL_NAME, -} from "./structured-output-constants"; - -interface NewSessionMeta { - taskRunId?: string; - taskId?: string; - environment?: "local" | "cloud"; - systemPrompt?: string; - permissionMode?: string; - model?: string; - baseBranch?: string; - persistence?: { taskId?: string; runId?: string; logUrl?: string }; - claudeCode?: { - options?: Record; - }; - additionalRoots?: string[]; - disableBuiltInTools?: boolean; - allowedDomains?: string[]; - jsonSchema?: Record | null; -} - -export interface CodexAcpAgentOptions { - codexProcessOptions: CodexProcessOptions; - processCallbacks?: ProcessSpawnedCallback; - posthogApiConfig?: PostHogAPIConfig; - onStructuredOutput?: (output: Record) => Promise; - /** - * Logger wired to the host log sink. Without it the codex-acp subprocess - * stderr, spawn failures and exit codes are written to a throwaway logger and - * never reach the exported logs, so a crash surfaces only as a generic - * "ACP connection closed" with no cause. - */ - logger?: Logger; -} - -type CodexSession = BaseSession & { - settingsManager: CodexSettingsManager; - promptRunning: boolean; -}; - -function toCodexPermissionMode(mode?: string): PermissionMode { - if (mode && (isCodexNativeMode(mode) || isCodeExecutionMode(mode))) { - return mode; - } - return "auto"; -} - -/** - * Prepend `_meta.prContext` (set by the agent-server on Slack-originated - * follow-up runs) to the prompt as a text block, mirroring Claude's - * `promptToClaude` behavior. Without this, codex cloud runs lose the - * PR-review context that follow-up flows rely on. - */ -function prependPrContext(params: PromptRequest): PromptRequest { - const prContext = (params._meta as Record | undefined) - ?.prContext; - if (typeof prContext !== "string" || prContext.length === 0) { - return params; - } - return { - ...params, - prompt: [{ type: "text", text: prContext }, ...params.prompt], - }; -} - -/** - * Apply an installed local-skill invocation for codex. Claude consumes - * `_meta.localSkillContext` in `promptToClaude`; codex has no such seam, so - * without this the resolved skill instructions are dropped and the bare - * `/` slash command reaches codex-acp, which rejects it as an unknown - * command ("Internal error"). Mirror Claude: drop the leading `/` chunk - * (the context already carries the user's args), prepend the skill - * instructions as text, and strip the local-skill `_meta` before forwarding. - */ -function prependLocalSkillContext(params: PromptRequest): PromptRequest { - const meta = params._meta as Record | undefined; - const localSkillContext = meta?.localSkillContext; - if (typeof localSkillContext !== "string" || localSkillContext.length === 0) { - return params; - } - const localSkillName = - typeof meta?.localSkillName === "string" ? meta.localSkillName : null; - // The agent-server always sets `localSkillContext` and `localSkillName` - // together. Without the name we can't identify the `/skill` chunk to drop, so - // injecting the context while leaving the bare command in place would forward - // exactly what codex-acp rejects — bail out rather than emit that broken mix. - if (!localSkillName) { - return params; - } - - let skipped = false; - const rest = params.prompt.filter((chunk) => { - if (!skipped && isLocalSkillCommandChunk(chunk, localSkillName)) { - skipped = true; - return false; - } - return true; - }); - - const { - localSkillContext: _ctx, - localSkillName: _name, - ...restMeta - } = meta ?? {}; - return { - ...params, - prompt: [{ type: "text", text: localSkillContext }, ...rest], - _meta: restMeta, - }; -} - -function classifyPromptError(error: unknown): unknown { - const message = error instanceof Error ? error.message : String(error ?? ""); - const classification = classifyAgentError(message); - if (classification === "agent_error") { - return error; - } - - return RequestError.internalError( - { classification, result: message }, - message, - ); -} - -// codex-rs/protocol/src/protocol.rs BASELINE_TOKENS — the always-resident -// floor (MCP schemas, skills, preset prompt) we can't attribute per-source. -const CODEX_BASELINE_TOKENS = 12000; - -function buildCodexBaseline( - meta: NewSessionMeta | undefined, -): ContextBreakdownBaseline { - const baseline = emptyBaseline(); - baseline.systemPrompt = - CODEX_BASELINE_TOKENS + estimateTokens(meta?.systemPrompt); - return baseline; -} - -const CODEX_NATIVE_MODE: Record = { - auto: "auto", - default: "auto", - acceptEdits: "auto", - plan: "read-only", - bypassPermissions: "full-access", -}; - -function toCodexNativeMode(mode?: string): CodexNativeMode { - if (mode && isCodexNativeMode(mode)) { - return mode; - } - if (mode && isCodeExecutionMode(mode)) { - return CODEX_NATIVE_MODE[mode]; - } - return "auto"; -} - -function getCurrentPermissionMode( - currentModeId?: string, - fallbackMode?: string, -): PermissionMode { - if (currentModeId && isCodexNativeMode(currentModeId)) { - return currentModeId; - } - - return toCodexPermissionMode(fallbackMode); -} - -function withCurrentMode( - configOptions: SessionConfigOption[] | null | undefined, - mode: CodexNativeMode, -): SessionConfigOption[] | null | undefined { - if (!configOptions) return configOptions; - return configOptions.map((option) => - option.category === "mode" && option.type === "select" - ? ({ ...option, currentValue: mode } as SessionConfigOption) - : option, - ); -} - -function syncInitialModeResponse( - response: NewSessionResponse | ForkSessionResponse, - mode: CodexNativeMode | undefined, -): void { - if (!mode) return; - if (response.modes) { - response.modes = { ...response.modes, currentModeId: mode }; - } - response.configOptions = withCurrentMode( - response.configOptions, - mode, - ) as typeof response.configOptions; -} - -const STRUCTURED_OUTPUT_INSTRUCTIONS = `\n\nWhen you have completed the task, call the \`${STRUCTURED_OUTPUT_TOOL_NAME}\` tool with the final structured result. The tool's input schema matches the required output format for this task. Do not describe the result in a plain message — submitting it via the tool is required for the task to be considered complete.`; - -/** - * Builds the stdio MCP server config that exposes the `create_output` tool. - * The child process validates tool input against the JSON schema with AJV. - * We pass the schema as a base64-encoded env var to avoid shell escaping. - */ -function buildStructuredOutputMcpServer( - jsonSchema: Record, -): McpServerStdio { - const scriptPath = resolveBundledMcpScript( - "adapters/codex/structured-output-mcp-server.js", - ); - const schemaBase64 = Buffer.from(JSON.stringify(jsonSchema)).toString( - "base64", - ); - return { - name: STRUCTURED_OUTPUT_MCP_NAME, - command: process.execPath, - args: [scriptPath], - env: [{ name: "POSTHOG_OUTPUT_SCHEMA", value: schemaBase64 }], - }; -} - -/** - * Builds the stdio MCP server config exposing the enabled local tools. Context - * (cwd, taskId, token) and the enabled tool names are passed base64/CSV-encoded - * so the child registers the same tools the Claude adapter exposes in-process. - */ -function buildLocalToolsMcpServer( - ctx: LocalToolCtx, - enabledNames: string[], -): McpServerStdio { - const scriptPath = resolveBundledMcpScript( - "adapters/codex/local-tools-mcp-server.js", - ); - const ctxBase64 = Buffer.from(JSON.stringify(ctx)).toString("base64"); - const env = [ - { name: "POSTHOG_LOCAL_TOOLS_CTX", value: ctxBase64 }, - { name: "POSTHOG_LOCAL_TOOLS_ENABLED", value: enabledNames.join(",") }, - ]; - if (ctx.token) { - // Token also on the child env so its own git remote ops (fetch/ls-remote) - // authenticate; the var names come from the single shared source. - env.push( - ...Object.entries(ghTokenEnv(ctx.token)).map(([name, value]) => ({ - name, - value, - })), - ); - } - return { - name: LOCAL_TOOLS_MCP_NAME, - command: process.execPath, - args: [scriptPath], - env, - }; -} - -export class CodexAcpAgent extends BaseAcpAgent { - readonly adapterName = "codex"; - declare session: CodexSession; - private codexProcess: CodexProcess; - private codexConnection: ClientSideConnection; - private sessionState: CodexSessionState; - /** - * FIFO serializer for prompt() calls. codex-acp and codex-rs themselves - * serialize submissions at the conversation level, but our adapter - * accumulates per-turn usage into sessionState.accumulatedUsage via the - * codex-client sessionUpdate handler. If two prompts ran concurrently on - * the JS side, the second's resetUsage() would wipe out the first's - * in-flight counters and both TURN_COMPLETE notifications would report - * garbled totals. Serializing on the JS side keeps the accumulator - * single-owner. - */ - private promptMutex: Promise = Promise.resolve(); - private readonly codexProcessOptions: CodexProcessOptions; - private readonly processCallbacks?: ProcessSpawnedCallback; - private readonly onStructuredOutput?: ( - output: Record, - ) => Promise; - // Snapshot of the initialize() request so refreshSession can replay the - // same handshake against a respawned codex-acp subprocess. - private lastInitRequest?: InitializeRequest; - private enrichment?: Enrichment; - - constructor(client: AgentSideConnection, options: CodexAcpAgentOptions) { - super(client); - this.logger = - options.logger ?? new Logger({ debug: true, prefix: "[CodexAcpAgent]" }); - - // Load user codex settings before spawning so spawnCodexProcess can - // filter out any [mcp_servers.*] entries from ~/.codex/config.toml. - const cwd = options.codexProcessOptions.cwd ?? process.cwd(); - const settingsManager = new CodexSettingsManager(cwd); - - this.codexProcessOptions = options.codexProcessOptions; - this.processCallbacks = options.processCallbacks; - this.onStructuredOutput = options.onStructuredOutput; - - // Spawn the codex-acp subprocess - this.codexProcess = spawnCodexProcess({ - ...options.codexProcessOptions, - settings: settingsManager.getSettings(), - logger: this.logger, - processCallbacks: options.processCallbacks, - }); - - // Create ACP connection to codex-acp over stdin/stdout - const codexReadable = nodeReadableToWebReadable(this.codexProcess.stdout); - const codexWritable = nodeWritableToWebWritable(this.codexProcess.stdin); - const codexStream = ndJsonStream(codexWritable, codexReadable); - - const abortController = new AbortController(); - this.session = { - abortController, - settingsManager, - notificationHistory: [], - cancelled: false, - promptRunning: false, - }; - - this.sessionState = createSessionState("", cwd); - - this.enrichment = createEnrichment(options.posthogApiConfig, this.logger); - - // Create the ClientSideConnection to codex-acp. - // The Client handler delegates all requests from codex-acp to the upstream - // PostHog Code client via our AgentSideConnection. - this.codexConnection = new ClientSideConnection( - (_agent) => - createCodexClient(this.client, this.logger, this.sessionState, { - enrichmentDeps: this.enrichment?.deps, - onStructuredOutput: this.onStructuredOutput, - }), - codexStream, - ); - } - - async initialize(request: InitializeRequest): Promise { - // Initialize settings - await this.session.settingsManager.initialize(); - - // Snapshot the handshake so refreshSession can replay it after respawn. - this.lastInitRequest = request; - - // Forward to codex-acp - const response = await this.codexConnection.initialize(request); - - // Merge our enhanced capabilities - return { - ...response, - agentCapabilities: { - ...response.agentCapabilities, - sessionCapabilities: { - ...response.agentCapabilities?.sessionCapabilities, - resume: {}, - fork: {}, - }, - _meta: { - posthog: { - resumeSession: true, - steering: "interrupt-resend", - }, - }, - }, - agentInfo: { - name: packageJson.name, - title: "Codex Agent", - version: packageJson.version, - }, - }; - } - - async newSession(params: NewSessionRequest): Promise { - const meta = params._meta as NewSessionMeta | undefined; - const requestedPermissionMode = toCodexPermissionMode(meta?.permissionMode); - - const injectedParams = this.applyLocalTools( - this.applyStructuredOutput(params, meta), - meta, - ); - const response = await this.codexConnection.newSession(injectedParams); - response.configOptions = normalizeCodexConfigOptions( - response.configOptions, - ); - - // Initialize session state. Mutate in place — codex-client closure- - // captured this object in the constructor and writes contextUsed/ - // accumulatedUsage to it on every upstream usage_update. - resetSessionState(this.sessionState, response.sessionId, params.cwd, { - taskRunId: meta?.taskRunId, - taskId: resolveTaskId(meta), - modeId: response.modes?.currentModeId ?? "auto", - modelId: modelIdFromConfigOptions(response.configOptions), - permissionMode: requestedPermissionMode, - }); - this.sessionId = response.sessionId; - this.sessionState.configOptions = response.configOptions ?? []; - this.sessionState.contextBreakdownBaseline = buildCodexBaseline(meta); - - const appliedMode = await this.applyInitialPermissionMode( - response.sessionId, - meta?.permissionMode, - response.modes?.currentModeId, - ); - syncInitialModeResponse(response, appliedMode); - if (appliedMode) { - this.sessionState.configOptions = response.configOptions ?? []; - } - - // Emit _posthog/sdk_session so the app can track the session - if (meta?.taskRunId) { - await this.client.extNotification(POSTHOG_NOTIFICATIONS.SDK_SESSION, { - taskRunId: meta.taskRunId, - sessionId: response.sessionId, - adapter: "codex", - }); - } - - this.logger.info("Codex session created", { - sessionId: response.sessionId, - taskRunId: meta?.taskRunId, - }); - - return response; - } - - async loadSession(params: LoadSessionRequest): Promise { - const meta = params._meta as NewSessionMeta | undefined; - const injectedParams = this.applyLocalTools( - this.applyStructuredOutput(params, meta), - meta, - ); - const response = await this.codexConnection.loadSession(injectedParams); - response.configOptions = normalizeCodexConfigOptions( - response.configOptions, - ); - const currentPermissionMode = getCurrentPermissionMode( - response.modes?.currentModeId, - meta?.permissionMode, - ); - - // Carry taskRunId/taskId across load so prompt() still emits cloud - // notifications (TURN_COMPLETE, USAGE_UPDATE) after a reload. newSession - // and resumeSession both do this; loadSession historically did - // not, which silently broke task-completion tracking on re-attach. - resetSessionState(this.sessionState, params.sessionId, params.cwd, { - taskRunId: meta?.taskRunId, - taskId: resolveTaskId(meta), - modeId: response.modes?.currentModeId ?? "auto", - permissionMode: currentPermissionMode, - }); - this.sessionId = params.sessionId; - this.sessionState.configOptions = response.configOptions ?? []; - this.sessionState.contextBreakdownBaseline = buildCodexBaseline(meta); - - if (meta?.taskRunId) { - await this.client.extNotification(POSTHOG_NOTIFICATIONS.SDK_SESSION, { - taskRunId: meta.taskRunId, - sessionId: params.sessionId, - adapter: "codex", - }); - } - - return response; - } - - async resumeSession( - params: ResumeSessionRequest, - ): Promise { - const meta = params._meta as NewSessionMeta | undefined; - const injectedParams = this.applyLocalTools( - this.applyStructuredOutput( - { - sessionId: params.sessionId, - cwd: params.cwd, - mcpServers: params.mcpServers ?? [], - _meta: params._meta, - }, - meta, - ), - meta, - ); - - // codex-acp doesn't support resume natively, use loadSession instead - const loadResponse = await this.codexConnection.loadSession(injectedParams); - loadResponse.configOptions = normalizeCodexConfigOptions( - loadResponse.configOptions, - ); - const currentPermissionMode = getCurrentPermissionMode( - loadResponse.modes?.currentModeId, - meta?.permissionMode, - ); - resetSessionState(this.sessionState, params.sessionId, params.cwd, { - taskRunId: meta?.taskRunId, - taskId: resolveTaskId(meta), - modeId: loadResponse.modes?.currentModeId ?? "auto", - permissionMode: currentPermissionMode, - }); - this.sessionId = params.sessionId; - this.sessionState.configOptions = loadResponse.configOptions ?? []; - this.sessionState.contextBreakdownBaseline = buildCodexBaseline(meta); - - if (meta?.taskRunId) { - await this.client.extNotification(POSTHOG_NOTIFICATIONS.SDK_SESSION, { - taskRunId: meta.taskRunId, - sessionId: params.sessionId, - adapter: "codex", - }); - } - - return { - modes: loadResponse.modes, - configOptions: loadResponse.configOptions, - }; - } - - async unstable_forkSession( - params: ForkSessionRequest, - ): Promise { - const meta = params._meta as NewSessionMeta | undefined; - const injectedParams = this.applyLocalTools( - this.applyStructuredOutput( - { - cwd: params.cwd, - mcpServers: params.mcpServers ?? [], - _meta: params._meta, - }, - meta, - ), - meta, - ); - - // Create a new session via codex-acp (fork isn't natively supported) - const newResponse = await this.codexConnection.newSession(injectedParams); - newResponse.configOptions = normalizeCodexConfigOptions( - newResponse.configOptions, - ); - - const requestedPermissionMode = toCodexPermissionMode(meta?.permissionMode); - resetSessionState(this.sessionState, newResponse.sessionId, params.cwd, { - taskRunId: meta?.taskRunId, - taskId: resolveTaskId(meta), - modeId: newResponse.modes?.currentModeId ?? "auto", - permissionMode: requestedPermissionMode, - }); - this.sessionId = newResponse.sessionId; - this.sessionState.configOptions = newResponse.configOptions ?? []; - this.sessionState.contextBreakdownBaseline = buildCodexBaseline(meta); - - const appliedMode = await this.applyInitialPermissionMode( - newResponse.sessionId, - meta?.permissionMode, - newResponse.modes?.currentModeId, - ); - syncInitialModeResponse(newResponse, appliedMode); - if (appliedMode) { - this.sessionState.configOptions = newResponse.configOptions ?? []; - } - - return newResponse; - } - - /** - * When the caller wires up `onStructuredOutput` and provides a JSON schema - * via `_meta.jsonSchema`, inject the stdio MCP server that exposes - * `create_output` and append instructions telling the model to use it. - * - * Codex has no native equivalent of Claude's `outputFormat`, so we lean on - * MCP tool-calling to get validated structured output back. - */ - private applyStructuredOutput< - T extends { mcpServers?: McpServer[]; _meta?: unknown }, - >(request: T, meta: NewSessionMeta | undefined): T { - if (!meta?.jsonSchema || !this.onStructuredOutput) { - return request; - } - - const mcpServer = buildStructuredOutputMcpServer(meta.jsonSchema); - const existingMeta = (request._meta ?? {}) as Record; - const existingSystemPrompt = - typeof existingMeta.systemPrompt === "string" - ? existingMeta.systemPrompt - : ""; - - return { - ...request, - mcpServers: [...(request.mcpServers ?? []), mcpServer], - _meta: { - ...existingMeta, - systemPrompt: existingSystemPrompt + STRUCTURED_OUTPUT_INSTRUCTIONS, - }, - }; - } - - /** - * Injects the stdio general local-tools MCP server. Tools self-gate via the - * registry (e.g. signed-commit is cloud-only and needs a GH token), so the - * server is only injected when at least one tool's gate passes. Their - * instructions already live in the shared cloud system prompt, so only the - * server needs injecting here. - */ - private applyLocalTools< - T extends { cwd?: string; mcpServers?: McpServer[]; _meta?: unknown }, - >(request: T, meta: NewSessionMeta | undefined): T { - const cwd = request.cwd; - if (!cwd) { - return request; - } - const ctx: LocalToolCtx = { - cwd, - token: resolveGithubToken(), - taskId: resolveTaskId(meta), - baseBranch: meta?.baseBranch, - }; - const tools = enabledLocalTools(ctx, meta); - if (tools.length === 0) { - if (isCloudRun(meta)) { - this.logger.warn( - "Cloud run registered no local tools — missing GH_TOKEN/GITHUB_TOKEN? signed commits unavailable", - ); - } - return request; - } - - const mcpServer = buildLocalToolsMcpServer( - ctx, - tools.map((t) => t.name), - ); - return { - ...request, - mcpServers: [...(request.mcpServers ?? []), mcpServer], - }; - } - - private async applyInitialPermissionMode( - sessionId: string, - permissionMode?: string, - currentModeId?: string, - ): Promise { - if (!permissionMode) { - return undefined; - } - - const nativeMode = toCodexNativeMode(permissionMode); - if (nativeMode === currentModeId) { - this.sessionState.modeId = nativeMode; - this.sessionState.permissionMode = toCodexPermissionMode(permissionMode); - return nativeMode; - } - - await this.codexConnection.setSessionMode({ - sessionId, - modeId: nativeMode, - }); - this.sessionState.modeId = nativeMode; - this.sessionState.permissionMode = toCodexPermissionMode(permissionMode); - return nativeMode; - } - - async listSessions( - params: ListSessionsRequest, - ): Promise { - return this.codexConnection.listSessions(params); - } - - async unstable_listSessions( - params: ListSessionsRequest, - ): Promise { - return this.listSessions(params); - } - - async prompt(params: PromptRequest): Promise { - const previous = this.promptMutex; - const next = previous.catch(() => {}).then(() => this.runPrompt(params)); - this.promptMutex = next; - return next; - } - - private async runPrompt(params: PromptRequest): Promise { - this.session.cancelled = false; - this.session.interruptReason = undefined; - resetUsage(this.sessionState); - - // codex-acp does not echo the user prompt back on the agent→client - // channel, so without this broadcast the tapped stream (persisted to S3 - // and rendered by the PostHog web UI) never sees a user turn and only - // the assistant reply shows up. Mirrors ClaudeAcpAgent.broadcastUserMessage. - // The original params (no _meta.prContext prefix) is broadcast so the - // injected PR context is not rendered as a user message. - await this.broadcastUserMessage(params); - - this.session.promptRunning = true; - let response: PromptResponse; - try { - response = await this.codexConnection.prompt( - prependPrContext(prependLocalSkillContext(params)), - ); - } catch (error) { - throw classifyPromptError(error); - } finally { - this.session.promptRunning = false; - } - - // Usage is already accumulated via sessionUpdate notifications in - // codex-client.ts. Do NOT also add response.usage here or tokens - // get double-counted. - - if (this.sessionState.taskRunId) { - const { accumulatedUsage } = this.sessionState; - - await this.client.extNotification(POSTHOG_NOTIFICATIONS.TURN_COMPLETE, { - sessionId: params.sessionId, - stopReason: response.stopReason ?? "end_turn", - usage: { - inputTokens: accumulatedUsage.inputTokens, - outputTokens: accumulatedUsage.outputTokens, - cachedReadTokens: accumulatedUsage.cachedReadTokens, - cachedWriteTokens: accumulatedUsage.cachedWriteTokens, - totalTokens: - accumulatedUsage.inputTokens + - accumulatedUsage.outputTokens + - accumulatedUsage.cachedReadTokens + - accumulatedUsage.cachedWriteTokens, - }, - }); - - if (response.usage) { - await this.client.extNotification(POSTHOG_NOTIFICATIONS.USAGE_UPDATE, { - sessionId: params.sessionId, - used: { - inputTokens: response.usage.inputTokens ?? 0, - outputTokens: response.usage.outputTokens ?? 0, - cachedReadTokens: response.usage.cachedReadTokens ?? 0, - cachedWriteTokens: response.usage.cachedWriteTokens ?? 0, - }, - cost: null, - }); - } - } - - // Emit the per-source breakdown so the renderer's ContextBreakdownPopover - // has data to show. This fires regardless of `taskRunId` (local sessions - // need it too) and regardless of `response.usage` (codex-acp doesn't - // populate it — context size comes from upstream usage_update events, - // tracked on sessionState.contextUsed). - if (this.sessionState.contextUsed !== undefined) { - await this.client.extNotification(POSTHOG_NOTIFICATIONS.USAGE_UPDATE, { - sessionId: params.sessionId, - breakdown: buildBreakdown( - this.sessionState.contextBreakdownBaseline ?? emptyBaseline(), - this.sessionState.contextUsed, - ), - }); - } - - return response; - } - - protected async interrupt(): Promise { - await this.codexConnection.cancel({ - sessionId: this.sessionId, - }); - } - - private async broadcastUserMessage(params: PromptRequest): Promise { - for (const chunk of params.prompt) { - const notification = { - sessionId: params.sessionId, - update: { - sessionUpdate: "user_message_chunk" as const, - content: chunk, - }, - }; - await this.client.sessionUpdate(notification); - this.appendNotification(params.sessionId, notification); - } - } - - /** - * Refresh the session between turns. Currently the only refreshable field - * is `mcpServers`. Unlike Claude (where we rebuild an in-process Query with - * `resume`), Codex runs as a `codex-acp` subprocess whose MCP set is bound - * at `newSession`/`loadSession` time and whose user-local MCPs are disabled - * via spawn-time `-c mcp_servers..enabled=false` CLI args. To - * guarantee the caller-supplied set fully wins, we respawn the subprocess - * and rehydrate the session via `loadSession` — codex-acp persists sessions - * to disk, so conversation history is preserved. - * - * This is an `extMethod` (request/response), not `extNotification`, so the - * caller can await completion before sending the next prompt. - * - * Caller contract: only call REFRESH_SESSION between turns (no prompt in flight). - */ - async extMethod( - method: string, - params: Record, - ): Promise> { - if (!isMethod(method, POSTHOG_METHODS.REFRESH_SESSION)) { - throw RequestError.methodNotFound(method); - } - - // Trust boundary: refresh is only safe when the caller is trusted infra - // (e.g. the sandbox agent-server). Do not route this method from - // untrusted clients — mcpServers contents are forwarded verbatim to - // codex-acp with no URL/command validation. - if (params.mcpServers === undefined) { - throw new RequestError( - -32602, - "refresh_session requires at least one refreshable field (e.g. mcpServers)", - ); - } - if (!Array.isArray(params.mcpServers)) { - throw new RequestError( - -32602, - "refresh_session: mcpServers must be an array", - ); - } - - await this.refreshSession(params.mcpServers as McpServer[]); - return { refreshed: true }; - } - - private async refreshSession(mcpServers: McpServer[]): Promise { - const prev = this.session; - if (prev.promptRunning) { - throw new RequestError( - -32002, - "Cannot refresh session while a prompt turn is in flight", - ); - } - - this.logger.info("Refreshing Codex session with fresh MCP servers", { - serverCount: mcpServers.length, - sessionId: this.sessionId, - }); - - // Abort FIRST so any stuck in-flight ACP request unblocks — otherwise - // cancel() can deadlock waiting on a codex-acp call that never returns. - prev.abortController.abort(); - try { - await this.codexConnection.cancel({ sessionId: this.sessionId }); - } catch (err) { - this.logger.warn("cancel() during refresh failed (non-fatal)", { - error: err, - }); - } - this.codexProcess.kill(); - - // Respawn with the same options and a fresh settings manager rooted at - // the current cwd (so the `mcp_servers..enabled=false` args are - // regenerated from the latest ~/.codex/config.toml). - const cwd = prev.settingsManager.getCwd(); - const newSettingsManager = new CodexSettingsManager(cwd); - await newSettingsManager.initialize(); - - const newProcess = spawnCodexProcess({ - ...this.codexProcessOptions, - cwd, - settings: newSettingsManager.getSettings(), - logger: this.logger, - processCallbacks: this.processCallbacks, - }); - - const codexReadable = nodeReadableToWebReadable(newProcess.stdout); - const codexWritable = nodeWritableToWebWritable(newProcess.stdin); - const codexStream = ndJsonStream(codexWritable, codexReadable); - - const newAbortController = new AbortController(); - const newConnection = new ClientSideConnection( - (_agent) => - createCodexClient(this.client, this.logger, this.sessionState, { - onStructuredOutput: this.onStructuredOutput, - }), - codexStream, - ); - - // Re-run ACP init on the new subprocess, then rehydrate the session with - // the new MCP set. loadSession is codex-acp's equivalent of Claude's - // `resume` — conversation history is restored from disk. - const initRequest: InitializeRequest = this.lastInitRequest ?? { - protocolVersion: 1, - }; - await newConnection.initialize(initRequest); - await newConnection.loadSession({ - sessionId: this.sessionId, - cwd: this.sessionState.cwd, - mcpServers, - }); - - // Swap everything at once so closeSession/prompt/cancel target the new - // subprocess going forward. Preserve sessionState (accumulatedUsage, - // taskRunId, configOptions) untouched. - this.codexProcess = newProcess; - this.codexConnection = newConnection; - prev.settingsManager.dispose(); - prev.settingsManager = newSettingsManager; - prev.abortController = newAbortController; - } - - async setSessionMode( - params: SetSessionModeRequest, - ): Promise { - const requestedMode = toCodexPermissionMode(params.modeId); - const nativeMode = toCodexNativeMode(params.modeId); - - const response = await this.codexConnection.setSessionMode({ - ...params, - modeId: nativeMode, - }); - - this.sessionState.modeId = nativeMode; - this.sessionState.permissionMode = requestedMode; - return response ?? {}; - } - - async setSessionConfigOption( - params: SetSessionConfigOptionRequest, - ): Promise { - const response = await this.codexConnection.setSessionConfigOption(params); - if (response.configOptions) { - response.configOptions = normalizeCodexConfigOptions( - response.configOptions, - ) as typeof response.configOptions; - this.sessionState.configOptions = response.configOptions; - } - if (params.configId === "mode" && typeof params.value === "string") { - this.sessionState.modeId = toCodexNativeMode(params.value); - this.sessionState.permissionMode = toCodexPermissionMode(params.value); - // Signal the mode change to agent-server so its session.permissionMode - // cache (used by shouldRelayPermissionToClient) stays in sync with the - // real Codex mode. Claude emits the same signal from its equivalent - // handler; without it, the agent-server's relay decisions for cloud - // runs would use a stale mode and silently auto-approve tool calls. - await this.client.sessionUpdate({ - sessionId: this.sessionId, - update: { - sessionUpdate: "current_mode_update", - currentModeId: params.value, - }, - }); - } - return response; - } - - async authenticate(_params: AuthenticateRequest): Promise { - // Auth handled externally - } - - async closeSession(): Promise { - this.logger.info("Closing Codex session", { sessionId: this.sessionId }); - this.session.abortController.abort(); - this.session.settingsManager.dispose(); - try { - this.codexProcess.kill(); - } catch (err) { - this.logger.warn("Failed to kill codex-acp process", { error: err }); - } - this.enrichment?.dispose(); - this.enrichment = undefined; - } -} diff --git a/packages/agent/src/adapters/codex/codex-client.test.ts b/packages/agent/src/adapters/codex/codex-client.test.ts deleted file mode 100644 index 56a8d8b1d0..0000000000 --- a/packages/agent/src/adapters/codex/codex-client.test.ts +++ /dev/null @@ -1,338 +0,0 @@ -import type { - AgentSideConnection, - ReadTextFileRequest, - ReadTextFileResponse, - SessionNotification, -} from "@agentclientprotocol/sdk"; -import { describe, expect, test, vi } from "vitest"; -import type { FileEnrichmentDeps } from "../../enrichment/file-enricher"; -import { Logger } from "../../utils/logger"; - -const enrichFileMock = vi.hoisted(() => vi.fn()); -vi.mock("../../enrichment/file-enricher", () => ({ - enrichFileForAgent: enrichFileMock, -})); - -import { createCodexClient } from "./codex-client"; -import { createSessionState, resetSessionState } from "./session-state"; - -function makeUpstream(response: ReadTextFileResponse): AgentSideConnection & { - readTextFile: ReturnType; -} { - const mock = { - readTextFile: vi.fn(async (_: ReadTextFileRequest) => response), - writeTextFile: vi.fn(), - requestPermission: vi.fn(), - sessionUpdate: vi.fn(), - createTerminal: vi.fn(), - terminalOutput: vi.fn(), - releaseTerminal: vi.fn(), - waitForTerminalExit: vi.fn(), - killTerminal: vi.fn(), - extMethod: vi.fn(), - extNotification: vi.fn(), - }; - return mock as unknown as AgentSideConnection & { - readTextFile: ReturnType; - }; -} - -describe("createCodexClient readTextFile", () => { - const logger = new Logger({ debug: false, prefix: "[test]" }); - const sessionState = createSessionState("", "/tmp"); - - test("returns upstream response unchanged when enrichmentDeps is absent", async () => { - enrichFileMock.mockReset(); - const upstream = makeUpstream({ content: "const x = 1;" }); - const client = createCodexClient(upstream, logger, sessionState); - - const result = await client.readTextFile?.({ - sessionId: "s", - path: "/tmp/a.ts", - }); - expect(result?.content).toBe("const x = 1;"); - expect(enrichFileMock).not.toHaveBeenCalled(); - }); - - test("returns enriched content when helper returns a string", async () => { - enrichFileMock.mockReset(); - enrichFileMock.mockResolvedValueOnce("const x = 1; // [PostHog] Flag ..."); - - const upstream = makeUpstream({ content: "const x = 1;" }); - const deps = {} as FileEnrichmentDeps; - const client = createCodexClient(upstream, logger, sessionState, { - enrichmentDeps: deps, - }); - - const result = await client.readTextFile?.({ - sessionId: "s", - path: "/tmp/a.ts", - }); - expect(result?.content).toBe("const x = 1; // [PostHog] Flag ..."); - expect(enrichFileMock).toHaveBeenCalledWith( - deps, - "/tmp/a.ts", - "const x = 1;", - ); - }); - - test("falls back to upstream response when helper returns null", async () => { - enrichFileMock.mockReset(); - enrichFileMock.mockResolvedValueOnce(null); - - const upstream = makeUpstream({ content: "no posthog here" }); - const client = createCodexClient(upstream, logger, sessionState, { - enrichmentDeps: {} as FileEnrichmentDeps, - }); - - const result = await client.readTextFile?.({ - sessionId: "s", - path: "/tmp/a.ts", - }); - expect(result?.content).toBe("no posthog here"); - }); - - test("calls upstream.readTextFile with original params (UI sees original)", async () => { - enrichFileMock.mockReset(); - enrichFileMock.mockResolvedValueOnce("enriched"); - - const upstream = makeUpstream({ content: "original" }); - const client = createCodexClient(upstream, logger, sessionState, { - enrichmentDeps: {} as FileEnrichmentDeps, - }); - - const params = { - sessionId: "s", - path: "/tmp/a.ts", - line: 10, - limit: 5, - }; - await client.readTextFile?.(params); - expect(upstream.readTextFile).toHaveBeenCalledWith(params); - }); -}); - -describe("createCodexClient onStructuredOutput", () => { - const logger = new Logger({ debug: false, prefix: "[test]" }); - const sessionState = createSessionState("sess", "/tmp"); - - function makeUpstream(): AgentSideConnection { - return { - sessionUpdate: vi.fn(async () => {}), - requestPermission: vi.fn(), - readTextFile: vi.fn(), - writeTextFile: vi.fn(), - createTerminal: vi.fn(), - terminalOutput: vi.fn(), - releaseTerminal: vi.fn(), - waitForTerminalExit: vi.fn(), - killTerminal: vi.fn(), - extMethod: vi.fn(), - extNotification: vi.fn(), - } as unknown as AgentSideConnection; - } - - function notification(update: Record): SessionNotification { - return { - sessionId: "sess", - update, - } as unknown as SessionNotification; - } - - test("fires once when create_output completes after rawInput arrived", async () => { - const onStructuredOutput = vi.fn(async () => {}); - const upstream = makeUpstream(); - const client = createCodexClient(upstream, logger, sessionState, { - onStructuredOutput, - }); - - await client.sessionUpdate?.( - notification({ - sessionUpdate: "tool_call", - toolCallId: "tc-1", - title: "create_output", - status: "in_progress", - rawInput: { result: "ok", count: 5 }, - }), - ); - expect(onStructuredOutput).not.toHaveBeenCalled(); - - await client.sessionUpdate?.( - notification({ - sessionUpdate: "tool_call_update", - toolCallId: "tc-1", - title: "create_output", - status: "completed", - }), - ); - - expect(onStructuredOutput).toHaveBeenCalledTimes(1); - expect(onStructuredOutput).toHaveBeenCalledWith({ result: "ok", count: 5 }); - }); - - test("matches mcp__-prefixed tool titles", async () => { - const onStructuredOutput = vi.fn(async () => {}); - const upstream = makeUpstream(); - const client = createCodexClient(upstream, logger, sessionState, { - onStructuredOutput, - }); - - await client.sessionUpdate?.( - notification({ - sessionUpdate: "tool_call", - toolCallId: "tc-1", - title: "mcp__posthog_output__create_output", - status: "completed", - rawInput: { ok: true }, - }), - ); - - expect(onStructuredOutput).toHaveBeenCalledWith({ ok: true }); - }); - - test("ignores tool calls that aren't create_output", async () => { - const onStructuredOutput = vi.fn(async () => {}); - const upstream = makeUpstream(); - const client = createCodexClient(upstream, logger, sessionState, { - onStructuredOutput, - }); - - await client.sessionUpdate?.( - notification({ - sessionUpdate: "tool_call", - toolCallId: "tc-1", - title: "Read", - status: "completed", - rawInput: { path: "/tmp/x" }, - }), - ); - - expect(onStructuredOutput).not.toHaveBeenCalled(); - }); - - test("does not fire when rawInput never arrived", async () => { - const onStructuredOutput = vi.fn(async () => {}); - const upstream = makeUpstream(); - const client = createCodexClient(upstream, logger, sessionState, { - onStructuredOutput, - }); - - await client.sessionUpdate?.( - notification({ - sessionUpdate: "tool_call", - toolCallId: "tc-1", - title: "create_output", - status: "completed", - }), - ); - - expect(onStructuredOutput).not.toHaveBeenCalled(); - }); - - test("does not fire twice if completed is re-emitted for the same tool call", async () => { - const onStructuredOutput = vi.fn(async () => {}); - const upstream = makeUpstream(); - const client = createCodexClient(upstream, logger, sessionState, { - onStructuredOutput, - }); - - const completed = notification({ - sessionUpdate: "tool_call", - toolCallId: "tc-1", - title: "create_output", - status: "completed", - rawInput: { final: 1 }, - }); - - await client.sessionUpdate?.(completed); - await client.sessionUpdate?.(completed); - - expect(onStructuredOutput).toHaveBeenCalledTimes(1); - }); - - test("forwards the notification upstream regardless of structured-output handling", async () => { - const onStructuredOutput = vi.fn(async () => {}); - const upstream = makeUpstream(); - const client = createCodexClient(upstream, logger, sessionState, { - onStructuredOutput, - }); - - const note = notification({ - sessionUpdate: "tool_call", - toolCallId: "tc-1", - title: "create_output", - status: "completed", - rawInput: { final: 1 }, - }); - await client.sessionUpdate?.(note); - - expect(upstream.sessionUpdate).toHaveBeenCalledWith(note); - }); - - test("does nothing when the callback is not wired", async () => { - const upstream = makeUpstream(); - const client = createCodexClient(upstream, logger, sessionState); - - // No onStructuredOutput configured — must not throw and must still - // forward upstream. - await client.sessionUpdate?.( - notification({ - sessionUpdate: "tool_call", - toolCallId: "tc-1", - title: "create_output", - status: "completed", - rawInput: { x: 1 }, - }), - ); - - expect(upstream.sessionUpdate).toHaveBeenCalledTimes(1); - }); -}); - -describe("createCodexClient usage_update propagation", () => { - const logger = new Logger({ debug: false, prefix: "[test]" }); - - function makeUpstream(): AgentSideConnection { - return { - sessionUpdate: vi.fn(async () => {}), - requestPermission: vi.fn(), - readTextFile: vi.fn(), - writeTextFile: vi.fn(), - createTerminal: vi.fn(), - terminalOutput: vi.fn(), - releaseTerminal: vi.fn(), - waitForTerminalExit: vi.fn(), - killTerminal: vi.fn(), - extMethod: vi.fn(), - extNotification: vi.fn(), - } as unknown as AgentSideConnection; - } - - // Regression: codex-client closure-captures the sessionState reference in - // its factory. CodexAcpAgent constructs the client once at startup with the - // initial "" sessionId state, then resetSessionState() mutates that same - // object on every newSession/loadSession/etc. If the agent ever reassigned - // `this.sessionState`, contextUsed writes would land on an orphan and the - // breakdown notification would never fire. - test("writes contextUsed to the same state object after resetSessionState", async () => { - const sessionState = createSessionState("", "/tmp"); - const upstream = makeUpstream(); - const client = createCodexClient(upstream, logger, sessionState); - - resetSessionState(sessionState, "real-session", "/tmp/repo", { - taskRunId: "run-1", - }); - - await client.sessionUpdate?.({ - sessionId: "real-session", - update: { - sessionUpdate: "usage_update", - used: 123_456, - size: 200_000, - }, - } as unknown as SessionNotification); - - expect(sessionState.contextUsed).toBe(123_456); - expect(sessionState.contextSize).toBe(200_000); - }); -}); diff --git a/packages/agent/src/adapters/codex/codex-client.ts b/packages/agent/src/adapters/codex/codex-client.ts deleted file mode 100644 index ebe05a4fe2..0000000000 --- a/packages/agent/src/adapters/codex/codex-client.ts +++ /dev/null @@ -1,307 +0,0 @@ -/** - * ACP Client implementation for communicating with codex-acp subprocess. - * - * This acts as the "client" from codex-acp's perspective: it receives - * permission requests, session updates, file I/O, and terminal operations - * from codex-acp and delegates them to the upstream PostHog Code client. - */ - -import type { - AgentSideConnection, - Client, - CreateTerminalRequest, - CreateTerminalResponse, - KillTerminalRequest, - KillTerminalResponse, - ReadTextFileRequest, - ReadTextFileResponse, - ReleaseTerminalRequest, - ReleaseTerminalResponse, - RequestPermissionRequest, - RequestPermissionResponse, - SessionNotification, - TerminalHandle, - TerminalOutputRequest, - TerminalOutputResponse, - ToolKind, - WaitForTerminalExitRequest, - WaitForTerminalExitResponse, - WriteTextFileRequest, - WriteTextFileResponse, -} from "@agentclientprotocol/sdk"; -import { - enrichFileForAgent, - type FileEnrichmentDeps, -} from "../../enrichment/file-enricher"; -import type { PermissionMode } from "../../execution-mode"; -import type { Logger } from "../../utils/logger"; -import type { CodexSessionState } from "./session-state"; -import { - STRUCTURED_OUTPUT_MCP_NAME, - STRUCTURED_OUTPUT_TOOL_NAME, -} from "./structured-output-constants"; - -export interface CodexClientCallbacks { - /** Called when a usage_update session notification is received */ - onUsageUpdate?: (update: Record) => void; - /** When set, Read responses are annotated with PostHog enrichment before reaching codex-acp. */ - enrichmentDeps?: FileEnrichmentDeps; - /** - * Called once per session when the agent completes the injected - * `create_output` MCP tool. Matches the Claude adapter's structured - * output delivery. - */ - onStructuredOutput?: (output: Record) => Promise; -} - -/** - * Tool calls for our injected MCP server surface in ACP `tool_call` / - * `tool_call_update` notifications. The `title` from codex-acp can be - * either the bare tool name or prefixed (`mcp____`); match - * both forms but require the server name on prefixed titles so an unrelated - * user tool happening to contain `create_output` doesn't trigger us. - */ -function isStructuredOutputToolCall(title: string | undefined | null): boolean { - if (!title) return false; - if (title === STRUCTURED_OUTPUT_TOOL_NAME) return true; - return ( - title.includes(STRUCTURED_OUTPUT_MCP_NAME) && - title.includes(STRUCTURED_OUTPUT_TOOL_NAME) - ); -} - -function toRecord(value: unknown): Record | null { - if (value && typeof value === "object" && !Array.isArray(value)) { - return value as Record; - } - return null; -} - -const AUTO_APPROVED_KINDS: Record> = { - default: new Set(["read", "search", "fetch", "think"]), - acceptEdits: new Set(["read", "edit", "search", "fetch", "think"]), - plan: new Set(["read", "search", "fetch", "think"]), - bypassPermissions: new Set([ - "read", - "edit", - "delete", - "move", - "search", - "execute", - "think", - "fetch", - "switch_mode", - "other", - ]), - auto: new Set(["read", "search", "fetch", "think"]), - "read-only": new Set(["read", "search", "fetch", "think"]), - "full-access": new Set([ - "read", - "edit", - "delete", - "move", - "search", - "execute", - "think", - "fetch", - "switch_mode", - "other", - ]), -}; - -function shouldAutoApprove( - mode: PermissionMode, - kind: ToolKind | null | undefined, -): boolean { - if (mode === "bypassPermissions" || mode === "full-access") return true; - if (!kind) return false; - return AUTO_APPROVED_KINDS[mode]?.has(kind) ?? false; -} - -/** - * Creates an ACP Client that delegates all requests from codex-acp - * to the upstream PostHog Code client (via AgentSideConnection). - */ -export function createCodexClient( - upstreamClient: AgentSideConnection, - logger: Logger, - sessionState: CodexSessionState, - callbacks?: CodexClientCallbacks, -): Client { - const terminalHandles = new Map(); - // Track rawInput across tool_call → tool_call_update → completed so we can - // fire onStructuredOutput exactly once per tool call id. Entries stay in - // the map after firing with `fired: true` so a re-emitted completion - // (if codex-acp ever resends one) is a no-op. - const structuredOutputState = new Map< - string, - { rawInput?: Record; fired: boolean } - >(); - - return { - async requestPermission( - params: RequestPermissionRequest, - ): Promise { - const kind = params.toolCall?.kind as ToolKind | null | undefined; - - if (shouldAutoApprove(sessionState.permissionMode, kind)) { - logger.debug("Auto-approving permission", { - mode: sessionState.permissionMode, - kind, - toolCallId: params.toolCall?.toolCallId, - }); - const allowOption = params.options?.find( - (o) => o.kind === "allow_once" || o.kind === "allow_always", - ); - return { - outcome: { - outcome: "selected", - optionId: allowOption?.optionId ?? "allow", - }, - }; - } - - return upstreamClient.requestPermission(params); - }, - - async sessionUpdate(params: SessionNotification): Promise { - const update = params.update as Record | undefined; - - if ( - callbacks?.onStructuredOutput && - (update?.sessionUpdate === "tool_call" || - update?.sessionUpdate === "tool_call_update") - ) { - const toolCallId = update.toolCallId as string | undefined; - const title = update.title as string | undefined; - if (toolCallId && isStructuredOutputToolCall(title)) { - const entry = structuredOutputState.get(toolCallId) ?? { - fired: false, - }; - const rawInput = toRecord(update.rawInput); - if (rawInput) entry.rawInput = rawInput; - structuredOutputState.set(toolCallId, entry); - - if (update.status === "completed" && !entry.fired && entry.rawInput) { - entry.fired = true; - try { - await callbacks.onStructuredOutput(entry.rawInput); - } catch (err) { - logger.warn("onStructuredOutput callback threw", { error: err }); - } - } - } - } - - if (update?.sessionUpdate === "usage_update") { - const used = update.used as number | undefined; - const size = update.size as number | undefined; - if (used !== undefined) sessionState.contextUsed = used; - if (size !== undefined) sessionState.contextSize = size; - - // Accumulate per-message token usage when available - const inputTokens = update.inputTokens as number | undefined; - const outputTokens = update.outputTokens as number | undefined; - if (inputTokens !== undefined) { - sessionState.accumulatedUsage.inputTokens += inputTokens; - } - if (outputTokens !== undefined) { - sessionState.accumulatedUsage.outputTokens += outputTokens; - } - const cachedRead = update.cachedReadTokens as number | undefined; - const cachedWrite = update.cachedWriteTokens as number | undefined; - if (cachedRead !== undefined) { - sessionState.accumulatedUsage.cachedReadTokens += cachedRead; - } - if (cachedWrite !== undefined) { - sessionState.accumulatedUsage.cachedWriteTokens += cachedWrite; - } - - callbacks?.onUsageUpdate?.(update); - } - - await upstreamClient.sessionUpdate(params); - }, - - async readTextFile( - params: ReadTextFileRequest, - ): Promise { - const response = await upstreamClient.readTextFile(params); - if (!callbacks?.enrichmentDeps) return response; - const enriched = await enrichFileForAgent( - callbacks.enrichmentDeps, - params.path, - response.content, - ); - return enriched ? { ...response, content: enriched } : response; - }, - - async writeTextFile( - params: WriteTextFileRequest, - ): Promise { - return upstreamClient.writeTextFile(params); - }, - - async createTerminal( - params: CreateTerminalRequest, - ): Promise { - const handle = await upstreamClient.createTerminal(params); - terminalHandles.set(handle.id, handle); - return { terminalId: handle.id }; - }, - - async terminalOutput( - params: TerminalOutputRequest, - ): Promise { - const handle = terminalHandles.get(params.terminalId); - if (!handle) { - return { output: "", truncated: false }; - } - return handle.currentOutput(); - }, - - async releaseTerminal( - params: ReleaseTerminalRequest, - ): Promise { - const handle = terminalHandles.get(params.terminalId); - if (handle) { - terminalHandles.delete(params.terminalId); - const result = await handle.release(); - return result ?? undefined; - } - }, - - async waitForTerminalExit( - params: WaitForTerminalExitRequest, - ): Promise { - const handle = terminalHandles.get(params.terminalId); - if (!handle) { - return { exitCode: 1 }; - } - return handle.waitForExit(); - }, - - async killTerminal( - params: KillTerminalRequest, - ): Promise { - const handle = terminalHandles.get(params.terminalId); - if (handle) { - return handle.kill(); - } - }, - - async extMethod( - method: string, - params: Record, - ): Promise> { - return upstreamClient.extMethod(method, params); - }, - - async extNotification( - method: string, - params: Record, - ): Promise { - return upstreamClient.extNotification(method, params); - }, - }; -} diff --git a/packages/agent/src/adapters/codex/session-state.ts b/packages/agent/src/adapters/codex/session-state.ts deleted file mode 100644 index 9aa8694a85..0000000000 --- a/packages/agent/src/adapters/codex/session-state.ts +++ /dev/null @@ -1,97 +0,0 @@ -import type { SessionConfigOption } from "@agentclientprotocol/sdk"; -import type { PermissionMode } from "../../execution-mode"; -import type { ContextBreakdownBaseline } from "../claude/context-breakdown"; - -export interface CodexUsage { - inputTokens: number; - outputTokens: number; - cachedReadTokens: number; - cachedWriteTokens: number; -} - -export interface CodexSessionState { - sessionId: string; - cwd: string; - modelId?: string; - modeId: string; - configOptions: SessionConfigOption[]; - accumulatedUsage: CodexUsage; - contextSize?: number; - contextUsed?: number; - contextBreakdownBaseline?: ContextBreakdownBaseline; - permissionMode: PermissionMode; - taskRunId?: string; - taskId?: string; -} - -export function createSessionState( - sessionId: string, - cwd: string, - opts?: { - taskRunId?: string; - taskId?: string; - modeId?: string; - modelId?: string; - permissionMode?: PermissionMode; - }, -): CodexSessionState { - return { - sessionId, - cwd, - modeId: opts?.modeId ?? "auto", - modelId: opts?.modelId, - configOptions: [], - accumulatedUsage: { - inputTokens: 0, - outputTokens: 0, - cachedReadTokens: 0, - cachedWriteTokens: 0, - }, - permissionMode: opts?.permissionMode ?? "auto", - taskRunId: opts?.taskRunId, - taskId: opts?.taskId, - }; -} - -// codex-client closure-captures the original sessionState reference, so we -// must mutate in place across newSession/loadSession/resumeSession/forkSession -// — reassigning would orphan it and silently break usage propagation. -export function resetSessionState( - state: CodexSessionState, - sessionId: string, - cwd: string, - opts?: { - taskRunId?: string; - taskId?: string; - modeId?: string; - modelId?: string; - permissionMode?: PermissionMode; - }, -): void { - state.sessionId = sessionId; - state.cwd = cwd; - state.modeId = opts?.modeId ?? "auto"; - state.modelId = opts?.modelId; - state.configOptions = []; - state.accumulatedUsage = { - inputTokens: 0, - outputTokens: 0, - cachedReadTokens: 0, - cachedWriteTokens: 0, - }; - state.contextSize = undefined; - state.contextUsed = undefined; - state.contextBreakdownBaseline = undefined; - state.permissionMode = opts?.permissionMode ?? "auto"; - state.taskRunId = opts?.taskRunId; - state.taskId = opts?.taskId; -} - -export function resetUsage(state: CodexSessionState): void { - state.accumulatedUsage = { - inputTokens: 0, - outputTokens: 0, - cachedReadTokens: 0, - cachedWriteTokens: 0, - }; -} diff --git a/packages/agent/src/adapters/codex/spawn.test.ts b/packages/agent/src/adapters/codex/spawn.test.ts deleted file mode 100644 index 2c448bd42b..0000000000 --- a/packages/agent/src/adapters/codex/spawn.test.ts +++ /dev/null @@ -1,136 +0,0 @@ -import { describe, expect, it, vi } from "vitest"; -import { Logger } from "../../utils/logger"; - -const spawnMock = vi.hoisted(() => vi.fn()); -vi.mock("node:child_process", () => ({ spawn: spawnMock })); - -const { spawnCodexProcess } = await import("./spawn"); - -function makeFakeChild() { - return { - stdin: { destroy: vi.fn() }, - stdout: { on: vi.fn(), destroy: vi.fn() }, - stderr: { on: vi.fn(), destroy: vi.fn() }, - on: vi.fn(), - kill: vi.fn(), - pid: 1234, - }; -} - -describe("spawnCodexProcess MCP disable args", () => { - it("disables bare-key servers but skips names codex's -c parser cannot express", () => { - spawnMock.mockReturnValue(makeFakeChild()); - - spawnCodexProcess({ - logger: new Logger({ debug: false }), - settings: { - mcpServerNames: ["simple", "with-dash", "my.server", "weird name"], - }, - }); - - const args: string[] = spawnMock.mock.calls[0][1]; - expect(args).toContain("mcp_servers.simple.enabled=false"); - expect(args).toContain("mcp_servers.with-dash.enabled=false"); - // A dotted or otherwise non-bare name would emit an override codex rejects, - // which crashes the whole session, so it is skipped (the server stays - // enabled, which is harmless). - expect(args.some((arg) => arg.includes("my.server"))).toBe(false); - expect(args.some((arg) => arg.includes("weird name"))).toBe(false); - }); -}); - -describe("spawnCodexProcess sandbox mode", () => { - it("disables codex's own sandbox only on cloud", () => { - spawnMock.mockClear(); - spawnMock.mockReturnValue(makeFakeChild()); - - spawnCodexProcess({ - logger: new Logger({ debug: false }), - environment: "cloud", - }); - - const args: string[] = spawnMock.mock.calls[0][1]; - expect(args).toContain('sandbox_mode="danger-full-access"'); - }); - - it.each([ - ["local", "local" as const], - ["unset", undefined], - ])( - "keeps codex's own sandbox as the OS-level backstop when environment is %s", - (_label, environment) => { - spawnMock.mockClear(); - spawnMock.mockReturnValue(makeFakeChild()); - - spawnCodexProcess({ - logger: new Logger({ debug: false }), - environment, - }); - - const args: string[] = spawnMock.mock.calls[0][1]; - expect(args.some((arg) => arg.includes("sandbox_mode"))).toBe(false); - }, - ); -}); - -describe("spawnCodexProcess developer instructions", () => { - it("passes guidance via developer_instructions to preserve the base prompt", () => { - spawnMock.mockClear(); - spawnMock.mockReturnValue(makeFakeChild()); - - spawnCodexProcess({ - logger: new Logger({ debug: false }), - developerInstructions: "Follow PostHog signed-commit rules.", - }); - - const args: string[] = spawnMock.mock.calls[0][1]; - expect(args).toContain( - 'developer_instructions="Follow PostHog signed-commit rules."', - ); - // The bare `instructions` and `model_instructions_file` keys replace Codex's - // model-optimized base prompt, so guidance must never go through them. - expect(args.some((arg) => arg.startsWith("instructions="))).toBe(false); - expect(args.some((arg) => arg.startsWith("model_instructions_file="))).toBe( - false, - ); - }); - - it("escapes backslashes, newlines and quotes in developer_instructions", () => { - spawnMock.mockClear(); - spawnMock.mockReturnValue(makeFakeChild()); - - spawnCodexProcess({ - logger: new Logger({ debug: false }), - developerInstructions: 'a\\b\n"c', - }); - - const args: string[] = spawnMock.mock.calls[0][1]; - expect(args).toContain('developer_instructions="a\\\\b\\n\\"c"'); - }); -}); - -describe("spawnCodexProcess codex home", () => { - it("sets CODEX_HOME on the child env when provided", () => { - spawnMock.mockClear(); - spawnMock.mockReturnValue(makeFakeChild()); - - spawnCodexProcess({ - logger: new Logger({ debug: false }), - codexHome: "/data/codex-home", - }); - - const env = spawnMock.mock.calls[0][2].env; - expect(env.CODEX_HOME).toBe("/data/codex-home"); - }); - - it("does not inject CODEX_HOME when not provided", () => { - spawnMock.mockClear(); - spawnMock.mockReturnValue(makeFakeChild()); - - spawnCodexProcess({ logger: new Logger({ debug: false }) }); - - // Inherits the ambient value (if any) without the adapter adding its own. - const env = spawnMock.mock.calls[0][2].env; - expect(env.CODEX_HOME).toBe(process.env.CODEX_HOME); - }); -}); diff --git a/packages/agent/src/adapters/codex/spawn.ts b/packages/agent/src/adapters/codex/spawn.ts deleted file mode 100644 index 38b7b57744..0000000000 --- a/packages/agent/src/adapters/codex/spawn.ts +++ /dev/null @@ -1,212 +0,0 @@ -import { type ChildProcess, spawn } from "node:child_process"; -import { existsSync } from "node:fs"; -import { delimiter, dirname } from "node:path"; -import type { Readable, Writable } from "node:stream"; -import type { ProcessSpawnedCallback } from "../../types"; -import { Logger } from "../../utils/logger"; -import { stripElectronNodeShimFromPath } from "../../utils/spawn-env"; -import type { CodexSettings } from "./settings"; - -export interface CodexProcessOptions { - cwd?: string; - apiBaseUrl?: string; - apiKey?: string; - model?: string; - reasoningEffort?: string; - /** - * Guidance appended on top of Codex's model-optimized base prompt via the - * `developer_instructions` config key. Unlike `instructions` / - * `model_instructions_file`, this does not replace the native base prompt. - */ - developerInstructions?: string; - binaryPath?: string; - codexHome?: string; - logger?: Logger; - processCallbacks?: ProcessSpawnedCallback; - settings?: CodexSettings; - /** Additional writable roots passed to Codex's workspace-write sandbox. */ - additionalDirectories?: string[]; - /** - * Extra codex `-c key=value` config overrides (app-server sub-adapter only). - * An escape hatch for config the adapter doesn't model — e.g. the e2e sets - * `auto_compact_token_limit` low to force a compaction. - */ - configOverrides?: Record; - /** Deployment environment; "cloud" disables codex's own OS sandbox (the enclosing sandbox isolates). */ - environment?: "local" | "cloud"; -} - -export interface CodexProcess { - process: ChildProcess; - stdin: Writable; - stdout: Readable; - kill: () => void; -} - -function buildConfigArgs(options: CodexProcessOptions): string[] { - const args: string[] = []; - - args.push("-c", `features.remote_models=false`); - - // On cloud the agent already runs inside PostHog's isolated sandbox (docker/Modal - // with agentsh egress + filesystem controls), so Codex's own OS-level sandbox is - // redundant — and its `linux-sandbox` launcher is unavailable inside that - // sandbox, so the default workspace-write mode panics ("sandbox launcher - // unavailable" → require_escalated) and wedges the session. Run Codex with no - // nested sandbox there; the enclosing sandbox provides the isolation. Local - // desktop sessions keep codex's own sandbox as the OS-level backstop. - if (options.environment === "cloud") { - args.push("-c", `sandbox_mode="danger-full-access"`); - } - - // Disable the user's local MCPs one-by-one so Codex only uses the MCPs we - // provide via ACP. We can't use `-c mcp_servers={}` because that makes Codex - // ignore MCPs entirely, including the ones we inject later. - // - // Only bare-key names are emitted: codex's `-c` parser rejects quoted key - // segments, so a name with a dot or other special character cannot be - // expressed as `mcp_servers..enabled=false` without producing an - // override that fails to load and crashes the whole codex session. Skipping - // such a name leaves that server enabled (harmless) instead of killing codex. - for (const name of options.settings?.mcpServerNames ?? []) { - if (!/^[A-Za-z0-9_-]+$/.test(name)) continue; - args.push("-c", `mcp_servers.${name}.enabled=false`); - } - - if (options.apiBaseUrl) { - args.push("-c", `model_provider="posthog"`); - args.push("-c", `model_providers.posthog.name="PostHog Gateway"`); - args.push("-c", `model_providers.posthog.base_url="${options.apiBaseUrl}"`); - args.push("-c", `model_providers.posthog.wire_api="responses"`); - args.push( - "-c", - `model_providers.posthog.env_key="POSTHOG_GATEWAY_API_KEY"`, - ); - } - - if (options.model) { - args.push("-c", `model="${options.model}"`); - } - - if (options.reasoningEffort) { - args.push("-c", `model_reasoning_effort="${options.reasoningEffort}"`); - } - - if (options.additionalDirectories?.length) { - const escaped = options.additionalDirectories - .map((p) => `"${p.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`) - .join(","); - args.push("-c", `sandbox_workspace_write.writable_roots=[${escaped}]`); - } - - if (options.developerInstructions) { - const escaped = options.developerInstructions - .replace(/\\/g, "\\\\") - .replace(/\n/g, "\\n") - .replace(/\r/g, "\\r") - .replace(/"/g, '\\"'); - args.push("-c", `developer_instructions="${escaped}"`); - } - - return args; -} - -function findCodexBinary(options: CodexProcessOptions): { - command: string; - args: string[]; -} { - const configArgs = buildConfigArgs(options); - - if (options.binaryPath && existsSync(options.binaryPath)) { - return { command: options.binaryPath, args: configArgs }; - } - - if (options.binaryPath) { - throw new Error( - `codex-acp binary not found at ${options.binaryPath}. Run "node apps/code/scripts/download-binaries.mjs" to download it.`, - ); - } - - return { command: "npx", args: ["@zed-industries/codex-acp", ...configArgs] }; -} - -export function spawnCodexProcess(options: CodexProcessOptions): CodexProcess { - const logger = - options.logger ?? new Logger({ debug: true, prefix: "[CodexSpawn]" }); - - const env: NodeJS.ProcessEnv = { ...process.env }; - - delete env.ELECTRON_RUN_AS_NODE; - delete env.ELECTRON_NO_ASAR; - - if (options.apiKey) { - env.POSTHOG_GATEWAY_API_KEY = options.apiKey; - } - - if (options.codexHome) { - env.CODEX_HOME = options.codexHome; - } - - const { command, args } = findCodexBinary(options); - - env.PATH = stripElectronNodeShimFromPath(env.PATH); - if (options.binaryPath && existsSync(options.binaryPath)) { - const binDir = dirname(options.binaryPath); - env.PATH = `${binDir}${delimiter}${env.PATH ?? ""}`; - } - - logger.info("Spawning codex-acp process", { - command, - args, - cwd: options.cwd, - hasApiBaseUrl: !!options.apiBaseUrl, - hasApiKey: !!options.apiKey, - binaryPath: options.binaryPath, - }); - - const child = spawn(command, args, { - cwd: options.cwd, - env, - stdio: ["pipe", "pipe", "pipe"], - detached: process.platform !== "win32", - }); - - child.stderr?.on("data", (data: Buffer) => { - logger.warn("codex-acp stderr:", data.toString()); - }); - - child.on("error", (err) => { - logger.error("codex-acp process error:", err); - }); - - child.on("exit", (code, signal) => { - logger.info("codex-acp process exited", { code, signal }); - if (child.pid && options.processCallbacks?.onProcessExited) { - options.processCallbacks.onProcessExited(child.pid); - } - }); - - if (!child.stdin || !child.stdout) { - throw new Error("Failed to get stdio streams from codex-acp process"); - } - - if (child.pid && options.processCallbacks?.onProcessSpawned) { - options.processCallbacks.onProcessSpawned({ - pid: child.pid, - command, - }); - } - - return { - process: child, - stdin: child.stdin, - stdout: child.stdout, - kill: () => { - logger.info("Killing codex-acp process", { pid: child.pid }); - child.stdin?.destroy(); - child.stdout?.destroy(); - child.stderr?.destroy(); - child.kill("SIGTERM"); - }, - }; -} diff --git a/packages/agent/src/adapters/codex/structured-output-constants.ts b/packages/agent/src/adapters/codex/structured-output-constants.ts deleted file mode 100644 index 739e2a775f..0000000000 --- a/packages/agent/src/adapters/codex/structured-output-constants.ts +++ /dev/null @@ -1,9 +0,0 @@ -/** - * Shared identifiers for the injected structured-output MCP server. - * Imported by codex-agent.ts (server config), codex-client.ts (tool-call - * matching), and structured-output-mcp-server.ts (tool registration) so the - * three stay in sync. - */ - -export const STRUCTURED_OUTPUT_MCP_NAME = "posthog_output"; -export const STRUCTURED_OUTPUT_TOOL_NAME = "create_output"; diff --git a/packages/agent/src/adapters/codex/structured-output-mcp-server.ts b/packages/agent/src/adapters/codex/structured-output-mcp-server.ts deleted file mode 100644 index 425b5ae08d..0000000000 --- a/packages/agent/src/adapters/codex/structured-output-mcp-server.ts +++ /dev/null @@ -1,72 +0,0 @@ -/** - * Standalone stdio MCP server for structured output in the Codex adapter. - * - * Spawned by codex-acp as an MCP server process. Reads the JSON schema - * from the POSTHOG_OUTPUT_SCHEMA env var (base64-encoded) and registers - * a tool whose Zod shape McpServer.tool() validates on each call. - * - * Usage: - * POSTHOG_OUTPUT_SCHEMA= node structured-output-mcp-server.js - */ - -import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; -import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; -import { z } from "zod"; -import { - STRUCTURED_OUTPUT_MCP_NAME, - STRUCTURED_OUTPUT_TOOL_NAME, -} from "./structured-output-constants"; - -function die(message: string): never { - process.stderr.write(`[structured-output-mcp-server] ${message}\n`); - process.exit(1); -} - -const schemaEnv = process.env.POSTHOG_OUTPUT_SCHEMA; -if (!schemaEnv) { - die("POSTHOG_OUTPUT_SCHEMA env var is required"); -} - -let jsonSchema: Record; -try { - jsonSchema = JSON.parse(Buffer.from(schemaEnv, "base64").toString("utf-8")); -} catch (err) { - die(`Failed to parse POSTHOG_OUTPUT_SCHEMA as base64-encoded JSON: ${err}`); -} - -const zodType = z.fromJSONSchema(jsonSchema); -if (!(zodType instanceof z.ZodObject)) { - die( - `POSTHOG_OUTPUT_SCHEMA must describe a JSON object schema (got ${zodType.constructor.name})`, - ); -} -// McpServer.tool() expects a mutable ZodRawShape -const zodShape = { ...zodType.shape } as z.ZodRawShape; - -const server = new McpServer({ - name: STRUCTURED_OUTPUT_MCP_NAME, - version: "1.0.0", -}); - -server.tool( - STRUCTURED_OUTPUT_TOOL_NAME, - "Submit the structured output for this task. Call this tool with the required fields to deliver your final result.", - zodShape, - async () => { - // McpServer.tool() validates `args` against `zodShape` before invoking - // this handler, so reaching this point means the input is valid. The - // parent process captures the validated output by intercepting the - // tool call in the ACP stream. - return { - content: [ - { - type: "text" as const, - text: "Output submitted successfully.", - }, - ], - }; - }, -); - -const transport = new StdioServerTransport(); -await server.connect(transport); diff --git a/packages/agent/src/adapters/reasoning-effort.ts b/packages/agent/src/adapters/reasoning-effort.ts index 2c031d024a..af1b1b85c8 100644 --- a/packages/agent/src/adapters/reasoning-effort.ts +++ b/packages/agent/src/adapters/reasoning-effort.ts @@ -1,5 +1,5 @@ import { getEffortOptions as getClaudeEffortOptions } from "./claude/session/models"; -import { getReasoningEffortOptions as getCodexReasoningEffortOptions } from "./codex/models"; +import { getReasoningEffortOptions as getCodexReasoningEffortOptions } from "./codex-app-server/models"; export type RuntimeAdapter = "claude" | "codex"; diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts index 85c78eab47..28d26d627c 100644 --- a/packages/agent/src/agent.ts +++ b/packages/agent/src/agent.ts @@ -129,7 +129,6 @@ export class Agent { logger: this.logger, processCallbacks: options.processCallbacks, onStructuredOutput: options.onStructuredOutput, - useCodexAppServer: options.useCodexAppServer, allowedModelIds, posthogApiConfig: this.posthogApiConfig, enricherEnabled: this.enricherEnabled, diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index 138fdfb5b5..e9adfa49e1 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -1203,10 +1203,10 @@ export class AgentServer { cwd: this.config.repositoryPath ?? "/tmp/workspace", apiBaseUrl: gatewayEnv.openaiBaseUrl, apiKey: this.config.apiKey, - // Path to the bundled codex-acp binary; the native app-server - // adapter derives `codex` from the same directory. Set in the + // Bundled-binary hint for the native codex CLI: the codex + // binary itself, or any file in its directory. Set in the // sandbox image (POSTHOG_CODEX_BINARY_PATH); when unset the - // adapter falls back to npx codex-acp. + // adapter uses the @openai/codex vendored binary. binaryPath: process.env.POSTHOG_CODEX_BINARY_PATH, model: this.config.model ?? DEFAULT_CODEX_MODEL, reasoningEffort: this.config.reasoningEffort, diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index d5c10e6169..0056590678 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -65,12 +65,6 @@ export interface TaskExecutionOptions { onStructuredOutput?: (output: Record) => Promise; /** Additional directories the agent process can access beyond cwd. */ additionalDirectories?: string[]; - /** - * Codex-only feature-flag lever: `true` selects the native app-server adapter, - * `false` codex-acp. The host evaluates a PostHog flag and passes the result; - * undefined falls back to env overrides then the bundled-binary default. - */ - useCodexAppServer?: boolean; } export type LogLevel = "debug" | "info" | "warn" | "error"; diff --git a/packages/agent/tsup.config.ts b/packages/agent/tsup.config.ts index bb5335eb1f..a169006d8f 100644 --- a/packages/agent/tsup.config.ts +++ b/packages/agent/tsup.config.ts @@ -120,10 +120,9 @@ export default defineConfig([ "src/adapters/claude/conversion/tool-use-to-acp.ts", "src/adapters/claude/session/jsonl-hydration.ts", "src/adapters/claude/session/models.ts", - "src/adapters/codex/models.ts", + "src/adapters/codex-app-server/models.ts", + "src/adapters/codex-app-server/local-tools-mcp-server.ts", "src/adapters/claude/mcp/tool-metadata.ts", - "src/adapters/codex/structured-output-mcp-server.ts", - "src/adapters/codex/local-tools-mcp-server.ts", "src/adapters/reasoning-effort.ts", "src/execution-mode.ts", "src/server/schemas.ts", diff --git a/packages/core/src/sessions/sessionService.ts b/packages/core/src/sessions/sessionService.ts index de156e5487..4c960bdca2 100644 --- a/packages/core/src/sessions/sessionService.ts +++ b/packages/core/src/sessions/sessionService.ts @@ -260,13 +260,6 @@ export interface SessionServiceHelpers { ) => Promise; } -/** - * PostHog flag gating the native codex app-server sub-adapter. When enabled for - * the user, a codex session uses the app-server adapter instead of codex-acp. - * Resolved at session start and passed to the agent as `useCodexAppServer`. - */ -export const CODEX_APP_SERVER_FLAG = "codex-app-server"; - export interface SessionServiceDeps { trpc: SessionTrpc; store: ISessionStore; @@ -282,12 +275,6 @@ export interface SessionServiceDeps { info: (msg: any, opts?: any) => unknown; }; track: (event: string, props?: Record) => void; - /** - * Evaluates a PostHog feature flag for the current user. Used to resolve - * {@link CODEX_APP_SERVER_FLAG} at session start. Optional so non-desktop - * hosts (stubbed web, tests) can omit it — absent is treated as "flag off". - */ - featureFlags?: { isEnabled(flagKey: string): boolean }; buildPermissionToolMetadata: (...args: any[]) => any; notifyPermissionRequest: (...args: any[]) => any; notifyPromptComplete: (...args: any[]) => any; @@ -307,9 +294,6 @@ export interface SessionServiceDeps { adapterStore: { getAdapter(taskRunId: string): Adapter | undefined; setAdapter(taskRunId: string, adapter: Adapter): void; - /** Codex sub-adapter pin: true = app-server, null = resolved undefined at creation. */ - setUseCodexAppServer(taskRunId: string, useAppServer: boolean | null): void; - getUseCodexAppServer(taskRunId: string): boolean | null | undefined; removeAdapter(taskRunId: string): void; }; readonly settings: { customInstructions?: string | null }; @@ -938,25 +922,6 @@ export class SessionService { this.d.adapterStore.setAdapter(taskRunId, resolvedAdapter); } - // Reuse the codex sub-adapter pinned at session creation so a rollout-flag - // flip cannot resume an app-server thread through codex-acp (or vice - // versa). Sessions from before pinning existed resolve live once, then get - // backfilled so later reconnects stay stable. - const pinnedUseCodexAppServer = - resolvedAdapter === "codex" - ? this.d.adapterStore.getUseCodexAppServer(taskRunId) - : undefined; - const useCodexAppServer = - pinnedUseCodexAppServer === undefined - ? this.resolveUseCodexAppServer(resolvedAdapter) - : (pinnedUseCodexAppServer ?? undefined); - if (resolvedAdapter === "codex" && pinnedUseCodexAppServer === undefined) { - this.d.adapterStore.setUseCodexAppServer( - taskRunId, - useCodexAppServer ?? null, - ); - } - if (previous) { session.optimisticItems = previous.optimisticItems; session.messageQueue = previous.messageQueue; @@ -1012,7 +977,6 @@ export class SessionService { logUrl, sessionId, adapter: resolvedAdapter, - useCodexAppServer, permissionMode: persistedMode, model: persistedModel, customInstructions: customInstructions || undefined, @@ -1305,30 +1269,6 @@ export class SessionService { ); } - /** - * Resolve the `codex-app-server` flag for a session. Only meaningful for the - * codex adapter (Claude ignores it), so returns undefined otherwise. - * - * One-way opt-in: when the flag is ON we force the app-server adapter (`true`). - * When off/unloaded (or no flags service on non-desktop hosts) we return - * `undefined` rather than `false`, so the agent falls through to its env - * override (`POSTHOG_CODEX_USE_APP_SERVER`) and then the codex-acp default — - * hard-passing `false` would shadow that env, since the host value has the - * highest precedence in resolveUseCodexAppServer. - * - * Consulted for NEW sessions (and once to backfill legacy ones); reconnects - * reuse the value pinned in the adapter store at creation, so a flag flip - * never resumes an existing thread through the other codex sub-adapter. - */ - private resolveUseCodexAppServer( - adapter: "claude" | "codex" | undefined, - ): boolean | undefined { - if (adapter !== "codex") return undefined; - return this.d.featureFlags?.isEnabled(CODEX_APP_SERVER_FLAG) - ? true - : undefined; - } - private async createNewLocalSession( taskId: string, taskTitle: string, @@ -1353,7 +1293,6 @@ export class SessionService { const { customInstructions: startCustomInstructions } = this.d.settings; const preferredModel = model ?? this.d.DEFAULT_GATEWAY_MODEL; - const useCodexAppServer = this.resolveUseCodexAppServer(adapter); const result = await this.d.trpc.agent.start.mutate({ taskId, taskRunId: taskRun.id, @@ -1362,7 +1301,6 @@ export class SessionService { projectId: auth.projectId, permissionMode: executionMode, adapter, - useCodexAppServer, customInstructions: startCustomInstructions || undefined, effort: effortLevelSchema.safeParse(reasoningLevel).success ? (reasoningLevel as EffortLevel) @@ -1408,16 +1346,9 @@ export class SessionService { this.d.setPersistedConfigOptions(taskRun.id, configOptions); } - // Persist the adapter, pinning the codex sub-adapter so reconnects keep - // using the one that created this thread even if the rollout flag flips. + // Persist the adapter so reconnects resume with the same one. if (adapter) { this.d.adapterStore.setAdapter(taskRun.id, adapter); - if (adapter === "codex") { - this.d.adapterStore.setUseCodexAppServer( - taskRun.id, - useCodexAppServer ?? null, - ); - } } // Store the initial prompt on the session so retry/reset flows can @@ -2258,10 +2189,10 @@ export class SessionService { // Steer: the user sent a message mid-turn and asked to fold it into the // running turn rather than queue it. Adapters that negotiated - // `steering: "native"` (Claude, codex app-server) inject at the next tool - // boundary; codex-acp ("interrupt-resend") and unknown adapters cancel and - // resend. Cloud has no real mid-turn steer (the backend only delivers - // messages between turns), so it falls through to the queue; compaction too. + // `steering: "native"` (Claude, codex) inject at the next tool boundary; + // unknown adapters cancel and resend. Cloud has no real mid-turn steer + // (the backend only delivers messages between turns), so it falls through + // to the queue; compaction too. if ( options?.steer && !session.isCloud && diff --git a/packages/shared/src/sessions.ts b/packages/shared/src/sessions.ts index 0774067852..19062f811d 100644 --- a/packages/shared/src/sessions.ts +++ b/packages/shared/src/sessions.ts @@ -71,7 +71,7 @@ export interface AgentSession { /** * Adapter's negotiated steering capability (`_meta.posthog.steering` from * initialize). "native" means a mid-turn message folds into the running turn - * (claude, codex app-server); "interrupt-resend" (codex-acp) or undefined + * (claude, codex); "interrupt-resend" (legacy) or undefined * means the host must cancel + resend. Drives the steer-vs-resend decision. */ steering?: string; @@ -195,7 +195,7 @@ export function resolveBypassRevertMode( * Whether a mid-turn message can be folded into the running turn (steered) * rather than interrupt-and-resent. Decided by the adapter's negotiated * `steering` capability: "native" folds (claude, codex app-server); - * "interrupt-resend" (codex-acp) does not. Cloud runs never steer locally. + * "interrupt-resend" (legacy) does not. Cloud runs never steer locally. * * Fallback: if `steering` is unset (a start path that predates capability * plumbing), Claude is still treated as native — it has always steered — so the diff --git a/packages/ui/src/features/sessions/hooks/useMessagingMode.ts b/packages/ui/src/features/sessions/hooks/useMessagingMode.ts index 1d00ceecdc..f4a93dd63e 100644 --- a/packages/ui/src/features/sessions/hooks/useMessagingMode.ts +++ b/packages/ui/src/features/sessions/hooks/useMessagingMode.ts @@ -19,7 +19,7 @@ export function useMessagingMode(taskId: string | undefined): MessagingMode { * Whether the task's session steers natively (folds a mid-turn message into the * running turn) versus falling back to interrupt-and-resend. Driven by the * adapter's negotiated `steering` capability — same decision as the host's - * sendPrompt gate — so Claude and codex app-server steer, codex-acp and cloud + * sendPrompt gate — so Claude and codex steer, while cloud * resend. Drives the steer label/tooltip, not whether steer is allowed. */ export function useSupportsNativeSteer(taskId: string | undefined): boolean { diff --git a/packages/ui/src/features/sessions/sessionAdapterStore.ts b/packages/ui/src/features/sessions/sessionAdapterStore.ts index f98bc4fac5..3c9c6f9507 100644 --- a/packages/ui/src/features/sessions/sessionAdapterStore.ts +++ b/packages/ui/src/features/sessions/sessionAdapterStore.ts @@ -6,17 +6,8 @@ type AdapterType = "claude" | "codex"; interface SessionAdapterState { adaptersByRunId: Record; - // Codex sub-adapter pinned at session creation: true = app-server, null = - // resolved undefined (codex-acp / env fallback). Keeps a resumed session on - // the sub-adapter that created its thread even if the rollout flag flips. - codexAppServerByRunId: Record; setAdapter: (taskRunId: string, adapter: AdapterType) => void; getAdapter: (taskRunId: string) => AdapterType | undefined; - setUseCodexAppServer: ( - taskRunId: string, - useAppServer: boolean | null, - ) => void; - getUseCodexAppServer: (taskRunId: string) => boolean | null | undefined; removeAdapter: (taskRunId: string) => void; } @@ -24,27 +15,15 @@ export const useSessionAdapterStore = create()( persist( (set, get) => ({ adaptersByRunId: {}, - codexAppServerByRunId: {}, setAdapter: (taskRunId, adapter) => set((state) => ({ adaptersByRunId: { ...state.adaptersByRunId, [taskRunId]: adapter }, })), getAdapter: (taskRunId) => get().adaptersByRunId[taskRunId], - setUseCodexAppServer: (taskRunId, useAppServer) => - set((state) => ({ - codexAppServerByRunId: { - ...state.codexAppServerByRunId, - [taskRunId]: useAppServer, - }, - })), - getUseCodexAppServer: (taskRunId) => - get().codexAppServerByRunId[taskRunId], removeAdapter: (taskRunId) => set((state) => { const { [taskRunId]: _removed, ...rest } = state.adaptersByRunId; - const { [taskRunId]: _removedPin, ...restPins } = - state.codexAppServerByRunId; - return { adaptersByRunId: rest, codexAppServerByRunId: restPins }; + return { adaptersByRunId: rest }; }), }), { @@ -52,7 +31,6 @@ export const useSessionAdapterStore = create()( storage: electronStorage, partialize: (state) => ({ adaptersByRunId: state.adaptersByRunId, - codexAppServerByRunId: state.codexAppServerByRunId, }), }, ), diff --git a/packages/ui/src/features/sessions/sessionServiceHost.ts b/packages/ui/src/features/sessions/sessionServiceHost.ts index 0c4d169a1f..9a57f913e7 100644 --- a/packages/ui/src/features/sessions/sessionServiceHost.ts +++ b/packages/ui/src/features/sessions/sessionServiceHost.ts @@ -37,7 +37,6 @@ import { WORKSPACE_QUERY_KEY } from "@posthog/ui/features/workspace/identifiers" import { toast } from "@posthog/ui/primitives/toast"; import { buildPermissionToolMetadata, - posthogFeatureFlags, track, } from "@posthog/ui/shell/posthogAnalyticsImpl"; import { logger } from "../../shell/logger"; @@ -81,7 +80,6 @@ function buildSessionServiceDeps(): SessionServiceDeps { ); }, buildPermissionToolMetadata, - featureFlags: posthogFeatureFlags, notifyPermissionRequest: (taskTitle, taskId) => resolveService(NotificationBus).notifyPermissionRequest( taskTitle, @@ -108,12 +106,6 @@ function buildSessionServiceDeps(): SessionServiceDeps { useSessionAdapterStore.getState().getAdapter(taskRunId), setAdapter: (taskRunId, adapter) => useSessionAdapterStore.getState().setAdapter(taskRunId, adapter), - setUseCodexAppServer: (taskRunId, useAppServer) => - useSessionAdapterStore - .getState() - .setUseCodexAppServer(taskRunId, useAppServer), - getUseCodexAppServer: (taskRunId) => - useSessionAdapterStore.getState().getUseCodexAppServer(taskRunId), removeAdapter: (taskRunId) => useSessionAdapterStore.getState().removeAdapter(taskRunId), }, diff --git a/packages/workspace-server/src/services/agent/agent.ts b/packages/workspace-server/src/services/agent/agent.ts index cd51029618..547a9c84ed 100644 --- a/packages/workspace-server/src/services/agent/agent.ts +++ b/packages/workspace-server/src/services/agent/agent.ts @@ -262,12 +262,6 @@ interface SessionConfig { /** The agent's session ID (for resume - SDK session ID for Claude, Codex's session ID for Codex) */ sessionId?: string; adapter?: "claude" | "codex"; - /** - * Resolved `codex-app-server` flag for the current user. When true and the - * adapter is codex, the agent uses the native app-server sub-adapter; when - * false/undefined it uses codex-acp. Ignored by the Claude adapter. - */ - useCodexAppServer?: boolean; /** Permission mode to use for the session */ permissionMode?: string; /** Custom instructions injected into the system prompt */ @@ -432,7 +426,7 @@ export class AgentService extends TypedEventEmitter { } private getCodexBinaryPath(): string { - const binary = process.platform === "win32" ? "codex-acp.exe" : "codex-acp"; + const binary = process.platform === "win32" ? "codex.exe" : "codex"; return this.bundledResources.resolve(`.vite/build/codex-acp/${binary}`); } @@ -698,7 +692,6 @@ If a repository IS genuinely required, attach one in this priority order: credentials, logUrl, adapter, - useCodexAppServer, permissionMode, customInstructions, systemPromptOverride, @@ -811,7 +804,6 @@ If a repository IS genuinely required, attach one in this priority order: const acpConnection = await agent.run(taskId, taskRunId, { adapter, - useCodexAppServer, gatewayUrl: proxyUrl, codexBinaryPath: adapter === "codex" ? this.getCodexBinaryPath() : undefined, @@ -1975,8 +1967,6 @@ For git operations while detached: logUrl: "logUrl" in params ? params.logUrl : undefined, sessionId: "sessionId" in params ? params.sessionId : undefined, adapter: "adapter" in params ? params.adapter : undefined, - useCodexAppServer: - "useCodexAppServer" in params ? params.useCodexAppServer : undefined, permissionMode: "permissionMode" in params ? params.permissionMode : undefined, customInstructions: diff --git a/packages/workspace-server/src/services/agent/codex-home.ts b/packages/workspace-server/src/services/agent/codex-home.ts index 723f367f73..520281520c 100644 --- a/packages/workspace-server/src/services/agent/codex-home.ts +++ b/packages/workspace-server/src/services/agent/codex-home.ts @@ -43,7 +43,7 @@ export async function cleanupCodexHome( * load the bundled PostHog catalog and the user's `~/.claude/skills` — without * ever writing into the shared cross-agent `~/.agents/skills`. * - * codex-acp scans `$CODEX_HOME/skills` plus `$HOME/.agents/skills`. By pointing + * codex scans `$CODEX_HOME/skills` plus `$HOME/.agents/skills`. By pointing * CODEX_HOME at this app-private dir we feed our skills through the former while * the user's own Codex skills still load from the latter (it is keyed off * `$HOME`, not `$CODEX_HOME`). The user's real `~/.codex/config.toml` is diff --git a/packages/workspace-server/src/services/agent/schemas.ts b/packages/workspace-server/src/services/agent/schemas.ts index 477630edfb..647f2e9e8e 100644 --- a/packages/workspace-server/src/services/agent/schemas.ts +++ b/packages/workspace-server/src/services/agent/schemas.ts @@ -52,12 +52,6 @@ export const startSessionInput = z.object({ autoProgress: z.boolean().optional(), runMode: z.enum(["local", "cloud"]).optional(), adapter: z.enum(["claude", "codex"]).optional(), - /** - * Resolved value of the `codex-app-server` PostHog flag (evaluated host-side - * for the current user). When true and adapter is "codex", the agent uses the - * native app-server sub-adapter instead of codex-acp. Ignored for Claude. - */ - useCodexAppServer: z.boolean().optional(), additionalDirectories: z.array(z.string()).optional(), customInstructions: z.string().max(2000).optional(), /** @@ -144,7 +138,7 @@ export const sessionResponseSchema = z.object({ configOptions: z.array(sessionConfigOptionSchema).optional(), // The adapter's negotiated steering capability from initialize // (`_meta.posthog.steering`): "native" folds a mid-turn message into the - // running turn; "interrupt-resend" (codex-acp) or absent means the host must + // running turn; "interrupt-resend" (legacy) or absent means the host must // cancel + resend instead. Drives the host's steer-vs-resend decision. steering: z.string().optional(), }); @@ -205,8 +199,6 @@ export const reconnectSessionInput = z.object({ logUrl: z.string().optional(), sessionId: z.string().optional(), adapter: z.enum(["claude", "codex"]).optional(), - /** See startSessionInput.useCodexAppServer — re-resolved on reconnect. */ - useCodexAppServer: z.boolean().optional(), /** Additional directories Claude can access beyond cwd (for worktree support) */ additionalDirectories: z.array(z.string()).optional(), permissionMode: z.string().optional(),