From 8beb2ec1246b5af8d32f555116898efb01eb3863 Mon Sep 17 00:00:00 2001 From: Zero Liu Date: Fri, 10 Jul 2026 16:37:18 -0700 Subject: [PATCH] feat(agent-mode): adopt current Codex ACP runtime --- .../backends/codex/CodexBackend.test.ts | 246 ++++++++++-------- src/agentMode/backends/codex/CodexBackend.ts | 114 +++----- .../backends/codex/descriptor.test.ts | 208 +++++++++++++++ src/agentMode/backends/codex/descriptor.ts | 106 +++++--- 4 files changed, 452 insertions(+), 222 deletions(-) create mode 100644 src/agentMode/backends/codex/descriptor.test.ts diff --git a/src/agentMode/backends/codex/CodexBackend.test.ts b/src/agentMode/backends/codex/CodexBackend.test.ts index fb513081..ed99bf8c 100644 --- a/src/agentMode/backends/codex/CodexBackend.test.ts +++ b/src/agentMode/backends/codex/CodexBackend.test.ts @@ -6,7 +6,7 @@ import { updateCachedSystemPrompts, } from "@/system-prompts/state"; import type { UserSystemPrompt } from "@/system-prompts/type"; -import { CodexBackend, toTomlBasicString } from "./CodexBackend"; +import { CodexBackend } from "./CodexBackend"; jest.mock("@/logger", () => ({ logInfo: jest.fn(), @@ -14,18 +14,6 @@ jest.mock("@/logger", () => ({ logError: jest.fn(), })); -function makeSystemPrompt(title: string, content: string): UserSystemPrompt { - return { title, content, createdMs: 0, modifiedMs: 0, lastUsedMs: 0 }; -} - -/** The system-prompt jotai store is module-global — reset it between tests. */ -function resetPromptState(): void { - setDisableBuiltinSystemPrompt(false); - setSelectedPromptTitle(""); - setDefaultSystemPromptTitle(""); - updateCachedSystemPrompts([]); -} - jest.mock("@/agentMode/skills", () => { const actual = jest.requireActual("@/agentMode/skills"); return { @@ -43,6 +31,21 @@ jest.mock("@/agentMode/skills", () => { }; }); +function makeSystemPrompt(title: string, content: string): UserSystemPrompt { + return { title, content, createdMs: 0, modifiedMs: 0, lastUsedMs: 0 }; +} + +function resetPromptState(): void { + setDisableBuiltinSystemPrompt(false); + setSelectedPromptTitle(""); + setDefaultSystemPromptTitle(""); + updateCachedSystemPrompts([]); +} + +function codexConfig(env: NodeJS.ProcessEnv): Record { + return JSON.parse(env.CODEX_CONFIG ?? "") as Record; +} + describe("CodexBackend.buildSpawnDescriptor", () => { beforeEach(() => { resetSettings(); @@ -62,53 +65,47 @@ describe("CodexBackend.buildSpawnDescriptor", () => { }); }); - it("forwards the Copilot base prompt + pill-syntax directive via -c developer_instructions", async () => { - const backend = new CodexBackend(); - const desc = await backend.buildSpawnDescriptor({ vaultBasePath: "/vault" }); + it("transports the composed Copilot instructions through CODEX_CONFIG", async () => { + const desc = await new CodexBackend().buildSpawnDescriptor({ vaultBasePath: "/vault" }); + const instructions = codexConfig(desc.env).developer_instructions; + expect(desc.command).toBe("/usr/local/bin/codex-acp"); - const cIdx = desc.args.indexOf("-c"); - expect(cIdx).toBeGreaterThanOrEqual(0); - const value = desc.args[cIdx + 1]; - expect(value.startsWith("developer_instructions=")).toBe(true); - // Base Obsidian-vault framing reaches Codex (decode the TOML basic string). - expect(value).toContain("Obsidian Copilot"); - expect(value).toContain("NOT a software-engineering agent or CLI coding tool"); - // Pill-syntax directive. - expect(value).toContain("{folder_name}"); - expect(value).toContain("{activeNote}"); - // Skill discovery is automatic from `.agents/skills/`, so the directive - // never templates in SKILL.md authoring instructions. - expect(value).not.toContain("metadata.copilot-enabled-agents"); - expect(value).not.toContain("copilot/skills//SKILL.md"); + expect(desc.args).toEqual([]); + expect(instructions).toEqual(expect.any(String)); + expect(instructions).toContain("Obsidian Copilot"); + expect(instructions).toContain("NOT a software-engineering agent or CLI coding tool"); + expect(instructions).toContain("{folder_name}"); + expect(instructions).toContain("{activeNote}"); + expect(instructions).not.toContain("metadata.copilot-enabled-agents"); + expect(instructions).not.toContain("copilot/skills//SKILL.md"); }); - it("appends the user's selected custom prompt to developer_instructions", async () => { + it("preserves the selected custom prompt in developer_instructions", async () => { updateCachedSystemPrompts([makeSystemPrompt("Haiku", "respond in haiku")]); setSelectedPromptTitle("Haiku"); - const backend = new CodexBackend(); - const desc = await backend.buildSpawnDescriptor({ vaultBasePath: "/vault" }); - const value = desc.args[desc.args.indexOf("-c") + 1]; - expect(value).toContain("Obsidian Copilot"); - // The TOML basic string escapes newlines as \n, so match the wrapper + - // content rather than the literal multi-line block. - expect(value).toContain(""); - expect(value).toContain("respond in haiku"); + + const desc = await new CodexBackend().buildSpawnDescriptor({ vaultBasePath: "/vault" }); + const instructions = codexConfig(desc.env).developer_instructions; + + expect(instructions).toContain("Obsidian Copilot"); + expect(instructions).toContain(""); + expect(instructions).toContain("respond in haiku"); }); - it("suppresses the base prompt when 'disable builtin' is on, keeping the user prompt + pill directive", async () => { + it("keeps the user prompt and pill directive when the builtin prompt is disabled", async () => { updateCachedSystemPrompts([makeSystemPrompt("Haiku", "respond in haiku")]); setSelectedPromptTitle("Haiku"); setDisableBuiltinSystemPrompt(true); - const backend = new CodexBackend(); - const desc = await backend.buildSpawnDescriptor({ vaultBasePath: "/vault" }); - const value = desc.args[desc.args.indexOf("-c") + 1]; - expect(value).not.toContain("Obsidian Copilot"); - expect(value).toContain("respond in haiku"); - // Pill directive is functional wiring, not builtin framing — always sent. - expect(value).toContain("{folder_name}"); + + const desc = await new CodexBackend().buildSpawnDescriptor({ vaultBasePath: "/vault" }); + const instructions = codexConfig(desc.env).developer_instructions; + + expect(instructions).not.toContain("Obsidian Copilot"); + expect(instructions).toContain("respond in haiku"); + expect(instructions).toContain("{folder_name}"); }); - it("does not template a skills folder into developer_instructions", async () => { + it("merges user Codex config while keeping Copilot-owned instructions and initial mode", async () => { setSettings({ agentMode: { byok: {}, @@ -116,81 +113,102 @@ describe("CodexBackend.buildSpawnDescriptor", () => { activeBackend: "codex", debugFullFrames: false, welcomeDismissed: false, - skills: { folder: "team-skills" }, - backends: { codex: { binaryPath: "/usr/local/bin/codex-acp" } }, + skills: { folder: "copilot/skills" }, + backends: { + codex: { + binaryPath: "/usr/local/bin/codex-acp", + envOverrides: { + CODEX_CONFIG: JSON.stringify({ model: "gpt-custom", developer_instructions: "old" }), + INITIAL_AGENT_MODE: "read-only", + OPENAI_API_KEY: "test-key", + }, + }, + }, }, }); - const backend = new CodexBackend(); - const desc = await backend.buildSpawnDescriptor({ vaultBasePath: "/vault" }); - const cIdx = desc.args.indexOf("-c"); - const value = desc.args[cIdx + 1]; - // The pill directive doesn't reference the skills folder at all. - expect(value).not.toContain("team-skills"); - expect(value).not.toContain("copilot/skills"); + + const desc = await new CodexBackend().buildSpawnDescriptor({ vaultBasePath: "/vault" }); + + expect(codexConfig(desc.env)).toMatchObject({ + model: "gpt-custom", + developer_instructions: expect.stringContaining("Obsidian Copilot"), + }); + expect(desc.env.INITIAL_AGENT_MODE).toBe("agent"); + expect(desc.env.OPENAI_API_KEY).toBe("test-key"); }); - it("escapes embedded double quotes and backslashes for TOML safety", async () => { - // Folders can't contain quotes in practice (validateSkillsFolder - // strips them), but the escape logic should still be airtight — the - // resulting -c value is consumed by a TOML parser, so an unescaped - // quote would terminate the basic-string literal and break - // codex-acp's startup. - const backend = new CodexBackend(); - const desc = await backend.buildSpawnDescriptor({ vaultBasePath: "/vault" }); - const cIdx = desc.args.indexOf("-c"); - const value = desc.args[cIdx + 1]; - // The value is wrapped in unescaped outer quotes; any inner double - // quote must be `\"` and every newline `\n` (no raw newlines, which - // would also break TOML basic strings). - expect(value).not.toMatch(/\n/); - // Confirm the outer literal is well-formed: starts with `key="…` and - // ends with `…"` (the closing quote of the TOML string). - expect(value.startsWith('developer_instructions="')).toBe(true); - expect(value.endsWith('"')).toBe(true); + it("launches a Windows npm JavaScript entry through the resolver-detected Node path", async () => { + const entry = "C:\\npm\\node_modules\\@agentclientprotocol\\codex-acp\\dist\\index.js"; + setSettings({ + agentMode: { + byok: {}, + mcpServers: [], + activeBackend: "codex", + debugFullFrames: false, + welcomeDismissed: false, + skills: { folder: "copilot/skills" }, + backends: { codex: { binaryPath: entry } }, + }, + }); + + const desc = await new CodexBackend({ + platform: "win32", + nodePath: "C:\\Program Files\\nodejs\\node.exe", + }).buildSpawnDescriptor({ vaultBasePath: "C:\\vault" }); + + expect(desc.command).toBe("C:\\Program Files\\nodejs\\node.exe"); + expect(desc.args).toEqual([entry]); }); - it("escapes the full TOML basic-string control set", () => { - // Named escapes per the TOML 1.0 spec. - expect(toTomlBasicString("a\bb\tc\nd\fe\rf")).toBe('"a\\bb\\tc\\nd\\fe\\rf"'); - // Backslash + double-quote. - expect(toTomlBasicString('back\\slash"quote')).toBe('"back\\\\slash\\"quote"'); - // Other controls fall through as \\uXXXX. Build the input from char - // codes so the source file stays plain ASCII (and copies/pastes cleanly). - const controls = - String.fromCharCode(0x01) + String.fromCharCode(0x1f) + String.fromCharCode(0x7f); - expect(toTomlBasicString(controls)).toBe('"\\u0001\\u001f\\u007f"'); - // Non-ASCII passes through unescaped. - expect(toTomlBasicString("über — café")).toBe('"über — café"'); + it("rejects a Windows JavaScript entry when Node was not resolver-detected", async () => { + setSettings({ + agentMode: { + byok: {}, + mcpServers: [], + activeBackend: "codex", + debugFullFrames: false, + welcomeDismissed: false, + skills: { folder: "copilot/skills" }, + backends: { codex: { binaryPath: "C:\\npm\\dist\\index.js" } }, + }, + }); + + await expect( + new CodexBackend({ platform: "win32" }).buildSpawnDescriptor({ vaultBasePath: "C:\\vault" }) + ).rejects.toThrow("Node executable is required"); }); - it("pins spawn-time approval_policy + sandbox_mode to canonical 'auto' preset", async () => { - // Without these overrides codex-acp derives the initial mode from - // ~/.codex/config.toml, which can land on read-only and surface as - // "Plan" in our picker for a brief moment before the post-spawn - // coerce kicks in. The TOML strings need outer quotes — codex parses - // the value portion of `-c key=value` as TOML. - const backend = new CodexBackend(); - const desc = await backend.buildSpawnDescriptor({ vaultBasePath: "/vault" }); - expect(desc.args).toEqual( - expect.arrayContaining([ - "-c", - 'approval_policy="on-request"', - "-c", - 'sandbox_mode="workspace-write"', - ]) - ); + it("throws when CODEX_CONFIG is not a JSON object", async () => { + setSettings({ + agentMode: { + byok: {}, + mcpServers: [], + activeBackend: "codex", + debugFullFrames: false, + welcomeDismissed: false, + skills: { folder: "copilot/skills" }, + backends: { + codex: { + binaryPath: "/usr/local/bin/codex-acp", + envOverrides: { CODEX_CONFIG: "[]" }, + }, + }, + }, + }); + + await expect( + new CodexBackend().buildSpawnDescriptor({ vaultBasePath: "/vault" }) + ).rejects.toThrow("CODEX_CONFIG must be a JSON object"); }); - it("does not add a project.md fallback to the codex spawn args", async () => { - // Session-start ensureAgentsMirror supersedes the spawn-level fallback for project scopes; - // omitting it also prevents a GLOBAL session from treating a vault-root project.md note as - // codex instructions (the spawn descriptor has no scope to gate on). - const backend = new CodexBackend(); - const desc = await backend.buildSpawnDescriptor({ vaultBasePath: "/vault" }); + it("does not add legacy -c arguments or a project.md fallback", async () => { + const desc = await new CodexBackend().buildSpawnDescriptor({ vaultBasePath: "/vault" }); + + expect(desc.args).not.toContain("-c"); expect(desc.args).not.toContainEqual(expect.stringContaining("project_doc_fallback_filenames")); }); - it("throws when the codex binary path is unset", async () => { + it("throws when the Codex binary path is unset", async () => { setSettings({ agentMode: { byok: {}, @@ -202,9 +220,9 @@ describe("CodexBackend.buildSpawnDescriptor", () => { backends: {}, }, }); - const backend = new CodexBackend(); - await expect(backend.buildSpawnDescriptor({ vaultBasePath: "/vault" })).rejects.toThrow( - /Codex binary path not configured/ - ); + + await expect( + new CodexBackend().buildSpawnDescriptor({ vaultBasePath: "/vault" }) + ).rejects.toThrow(/Codex binary path not configured/); }); }); diff --git a/src/agentMode/backends/codex/CodexBackend.ts b/src/agentMode/backends/codex/CodexBackend.ts index 5610b252..06443c65 100644 --- a/src/agentMode/backends/codex/CodexBackend.ts +++ b/src/agentMode/backends/codex/CodexBackend.ts @@ -1,94 +1,64 @@ -import { getSettings } from "@/settings/model"; import { AcpBackend, AcpSpawnDescriptor } from "@/agentMode/acp/types"; -import { buildSimpleSpawnDescriptor } from "@/agentMode/backends/shared/simpleBinaryBackend"; import { buildAgentSystemPrompt } from "@/agentMode/backends/shared/agentSystemPrompt"; import { buildCopilotPlusEnv } from "@/agentMode/backends/shared/copilotPlusEnv"; +import { buildSimpleSpawnDescriptor } from "@/agentMode/backends/shared/simpleBinaryBackend"; +import { getSettings } from "@/settings/model"; +import { launcherForConfiguredPath } from "./CodexBinaryManager"; + +const DEFAULT_AGENT_MODE = "agent"; + +export interface CodexBackendDependencies { + platform?: NodeJS.Platform; + nodePath?: string; +} /** - * Spawns the user-provided `codex-acp` binary - * (`@zed-industries/codex-acp`). The package wraps the local `codex` CLI - * and exposes it as an ACP server over stdio. Authentication is inherited - * from the user's existing `codex login` (`~/.codex/auth.json`) or - * `OPENAI_API_KEY` / `CODEX_API_KEY` exported in the user's shell — we - * deliberately do not inject keys so ChatGPT-login subscriptions work - * transparently. + * Spawns the user-provided `@agentclientprotocol/codex-acp` launcher. The + * adapter inherits ChatGPT login and API-key authentication from Codex's + * normal environment; Copilot only supplies its session configuration and + * initial permission preset. */ export class CodexBackend implements AcpBackend { readonly id = "codex" as const; readonly displayName = "Codex"; + private readonly platform: NodeJS.Platform; + private readonly nodePath: string | undefined; + + constructor(deps: CodexBackendDependencies = {}) { + this.platform = deps.platform ?? process.platform; + this.nodePath = deps.nodePath; + } + async buildSpawnDescriptor(_ctx: { vaultBasePath: string }): Promise { + const settings = getSettings().agentMode?.backends?.codex; const descriptor = buildSimpleSpawnDescriptor( - getSettings().agentMode?.backends?.codex?.binaryPath, + settings?.binaryPath, "Codex binary path not configured. Open Agent Mode settings and set the path to codex-acp.", - getSettings().agentMode?.backends?.codex?.envOverrides, - // Builtin Copilot Plus skill scripts read the license from the env. + settings?.envOverrides, await buildCopilotPlusEnv() ); - // Forward the shared composed system prompt — the Copilot base framing - // (unless the user disabled it), the pill-syntax directive, and the user's - // custom prompt — via codex's `developer_instructions` config field as a - // TOML 1.0 basic string. codex appends `developer_instructions` to its own - // base prompt, so this adds the Obsidian-vault framing on top. Read at - // spawn time; the host restarts codex on prompt changes via - // `restartOnSystemPromptChange`. - const directive = buildAgentSystemPrompt(); - descriptor.args = [ - ...descriptor.args, - "-c", - `developer_instructions=${toTomlBasicString(directive)}`, - // Pin spawn-time approval/sandbox so codex-acp's first - // `currentModeId` report matches the canonical `auto` preset - // (workspace-write + on-request), which Agent Mode surfaces as - // canonical `default` (ask mode). Without this, codex-acp derives - // the initial mode from the user's `~/.codex/config.toml` defaults - // (often `read-only` for untrusted projects), causing the picker - // to briefly show "Plan" before our post-spawn coerce switches it - // — see the matching `auto` preset in codex-utils-approval-presets - // and `Thread::modes()` in codex-acp/src/thread.rs. - "-c", - 'approval_policy="on-request"', - "-c", - 'sandbox_mode="workspace-write"', - ]; - // DESIGN NOTE: deliberately no `project_doc_fallback_filenames=["project.md"]`. - // Post-Phase-2 the session-start `ensureAgentsMirror` (AgentSessionManager, run before - // `resolveSessionCwd` for codex/opencode project sessions) guarantees the marker'd - // `AGENTS.md` mirror exists in the project cwd, so a `project.md` fallback is redundant. - // This descriptor only knows `vaultBasePath`, not the session scope: a spawn-level fallback - // would also apply to GLOBAL sessions and let codex read a user's vault-root `project.md` - // note as instructions. On the rare ensure failure a project session gets no instructions - // (ensure never throws and re-runs next session) rather than the frontmatter-laden source. + const launcher = launcherForConfiguredPath(descriptor.command, this.platform, this.nodePath); + + descriptor.command = launcher.command; + descriptor.args = launcher.args; + descriptor.env.CODEX_CONFIG = buildCodexConfig( + descriptor.env.CODEX_CONFIG, + buildAgentSystemPrompt() + ); + descriptor.env.INITIAL_AGENT_MODE = DEFAULT_AGENT_MODE; return descriptor; } } -/** - * Encode `value` as a TOML 1.0 basic string (double-quoted). Escapes: - * - `\` and `"` - * - named escapes `\b \t \n \f \r` - * - any other byte in 0x00–0x1F and 0x7F as `\uXXXX` - * - * Non-ASCII characters above 0x7F are valid in basic strings and pass - * through unescaped. Exported for unit testing. - */ -export function toTomlBasicString(value: string): string { - let out = '"'; - for (let i = 0; i < value.length; i++) { - const ch = value.charCodeAt(i); - if (ch === 0x5c) out += "\\\\"; - else if (ch === 0x22) out += '\\"'; - else if (ch === 0x08) out += "\\b"; - else if (ch === 0x09) out += "\\t"; - else if (ch === 0x0a) out += "\\n"; - else if (ch === 0x0c) out += "\\f"; - else if (ch === 0x0d) out += "\\r"; - else if (ch < 0x20 || ch === 0x7f) { - out += "\\u" + ch.toString(16).padStart(4, "0"); - } else { - out += value[i]; +function buildCodexConfig(existing: string | undefined, developerInstructions: string): string { + let config: Record = {}; + if (existing) { + const parsed: unknown = JSON.parse(existing); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw new Error("CODEX_CONFIG must be a JSON object."); } + config = parsed as Record; } - out += '"'; - return out; + return JSON.stringify({ ...config, developer_instructions: developerInstructions }); } diff --git a/src/agentMode/backends/codex/descriptor.test.ts b/src/agentMode/backends/codex/descriptor.test.ts new file mode 100644 index 00000000..82015aec --- /dev/null +++ b/src/agentMode/backends/codex/descriptor.test.ts @@ -0,0 +1,208 @@ +import { + CODEX_ACP_INSTALL_COMMAND, + CODEX_ACP_MIGRATION_COMMAND, + CODEX_ACP_MIN_VERSION, + CODEX_CLI_MIN_VERSION, +} from "@/constants"; +import { resetSettings, setSettings, type CodexProbeMetadata } from "@/settings/model"; +import { CodexBinaryManager, codexProbeSettingsFingerprint } from "./CodexBinaryManager"; +import { + CODEX_INSTALL_COMMAND, + CODEX_MIGRATION_COMMAND, + CodexBackendDescriptor, + resolveCodexNodePath, + subscribeCodexInstallState, +} from "./descriptor"; + +jest.mock("@/logger", () => ({ + logInfo: jest.fn(), + logWarn: jest.fn(), + logError: jest.fn(), +})); + +const BINARY_PATH = "/usr/local/bin/codex-acp"; + +function supportedProbe(): CodexProbeMetadata { + const settings = { binaryPath: BINARY_PATH }; + return { + kind: "supported", + launcherPath: BINARY_PATH, + launcherKind: "executable", + settingsFingerprint: codexProbeSettingsFingerprint(settings), + probedAt: "2026-07-10T12:00:00.000Z", + adapterVersion: CODEX_ACP_MIN_VERSION, + cliVersion: CODEX_CLI_MIN_VERSION, + cliSource: "bundled", + }; +} + +function setCodexSettings(probe?: CodexProbeMetadata): void { + setSettings({ + agentMode: { + byok: {}, + mcpServers: [], + activeBackend: "codex", + debugFullFrames: false, + welcomeDismissed: false, + skills: { folder: "copilot/skills" }, + backends: { codex: { binaryPath: BINARY_PATH, probe } }, + }, + }); +} + +describe("CodexBackendDescriptor installation contract", () => { + beforeEach(() => { + resetSettings(); + }); + + it("uses the replacement package install and migration commands", () => { + expect(CODEX_INSTALL_COMMAND).toBe(CODEX_ACP_INSTALL_COMMAND); + expect(CODEX_MIGRATION_COMMAND).toBe(CODEX_ACP_MIGRATION_COMMAND); + expect(CODEX_INSTALL_COMMAND).toContain("@agentclientprotocol/codex-acp"); + expect(CODEX_INSTALL_COMMAND).not.toContain("@zed-industries/codex-acp"); + }); + + it("reports supported adapter and CLI versions from persisted health", () => { + const probe = supportedProbe(); + const settings = { + agentMode: { backends: { codex: { binaryPath: BINARY_PATH, probe } } }, + } as never; + + expect(CodexBackendDescriptor.getInstallState(settings)).toEqual({ + kind: "ready", + source: "custom", + details: { + adapterVersion: CODEX_ACP_MIN_VERSION, + cliVersion: CODEX_CLI_MIN_VERSION, + cliSource: "bundled", + }, + }); + }); + + it("never reports a legacy probe as ready", () => { + const base = { binaryPath: BINARY_PATH }; + const settings = { + agentMode: { + backends: { + codex: { + ...base, + probe: { + kind: "legacy", + launcherPath: BINARY_PATH, + settingsFingerprint: codexProbeSettingsFingerprint(base), + probedAt: "2026-07-10T12:00:00.000Z", + adapterVersion: "0.8.1", + reason: "Legacy adapter", + }, + }, + }, + }, + } as never; + + expect(CodexBackendDescriptor.getInstallState(settings)).toMatchObject({ + kind: "blocked", + remediation: CODEX_ACP_MIGRATION_COMMAND, + }); + }); + + it("refreshes changed launcher inputs and notifies after probe health changes", () => { + setCodexSettings(supportedProbe()); + const callback = jest.fn(); + const refreshInstallState = jest.fn().mockResolvedValue({}); + const unsubscribe = subscribeCodexInstallState(() => ({ refreshInstallState }), callback); + + setSettings((current) => ({ + agentMode: { + ...current.agentMode, + backends: { + ...current.agentMode.backends, + codex: { + ...current.agentMode.backends.codex, + enabledModels: ["gpt-5"], + }, + }, + }, + })); + expect(callback).not.toHaveBeenCalled(); + expect(refreshInstallState).not.toHaveBeenCalled(); + + setSettings((current) => ({ + agentMode: { + ...current.agentMode, + backends: { + ...current.agentMode.backends, + codex: { + ...current.agentMode.backends.codex, + envOverrides: { CODEX_PATH: "/custom/codex" }, + }, + }, + }, + })); + expect(refreshInstallState).toHaveBeenCalledTimes(1); + expect(callback).not.toHaveBeenCalled(); + + setSettings((current) => ({ + agentMode: { + ...current.agentMode, + backends: { + ...current.agentMode.backends, + codex: { + ...current.agentMode.backends.codex, + probe: { + kind: "absent", + launcherPath: BINARY_PATH, + settingsFingerprint: codexProbeSettingsFingerprint(current.agentMode.backends.codex), + probedAt: "2026-07-10T12:01:00.000Z", + }, + }, + }, + }, + })); + expect(callback).toHaveBeenCalledTimes(1); + unsubscribe(); + }); + + it("refreshes manager health on plugin load", async () => { + const refresh = jest + .spyOn(CodexBinaryManager.prototype, "refreshInstallState") + .mockResolvedValue(supportedProbe()); + + await CodexBackendDescriptor.onPluginLoad?.({} as never); + + expect(refresh).toHaveBeenCalledTimes(1); + refresh.mockRestore(); + }); +}); + +describe("CodexBackendDescriptor modes and Windows launcher", () => { + it("maps Ask, Plan, and Auto to replacement adapter mode ids", () => { + expect(CodexBackendDescriptor.getModeMapping?.(null, null)).toEqual({ + kind: "setMode", + canonical: { default: "agent", plan: "read-only", auto: "agent-full-access" }, + readOnlyModeId: "read-only", + }); + }); + + it("uses the resolver-detected Node executable for the Windows npm entry", () => { + const entry = + "C:\\Users\\me\\AppData\\Roaming\\npm\\node_modules\\@agentclientprotocol\\codex-acp\\dist\\index.js"; + const nodePath = "C:\\Program Files\\nodejs\\node.exe"; + const existing = new Set([entry, nodePath]); + + expect( + resolveCodexNodePath({ + homeDir: "C:\\Users\\me", + platform: "win32", + env: { + APPDATA: "C:\\Users\\me\\AppData\\Roaming", + Path: "C:\\Program Files\\nodejs", + }, + fs: { + existsSync: (candidate) => existing.has(candidate), + readFileSync: () => "", + readdirSync: () => [], + }, + }) + ).toBe(nodePath); + }); +}); diff --git a/src/agentMode/backends/codex/descriptor.ts b/src/agentMode/backends/codex/descriptor.ts index 6c2ef5e2..0baf8d5a 100644 --- a/src/agentMode/backends/codex/descriptor.ts +++ b/src/agentMode/backends/codex/descriptor.ts @@ -1,5 +1,7 @@ import * as fs from "node:fs"; import * as os from "node:os"; +import { CODEX_ACP_INSTALL_COMMAND, CODEX_ACP_MIGRATION_COMMAND } from "@/constants"; +import { logError } from "@/logger"; import type CopilotPlugin from "@/main"; import { subscribeToSettingsChange, @@ -13,10 +15,7 @@ import CodexLogo from "./logo.svg"; import { CodexSettingsPanel } from "./CodexSettingsPanel"; import type { AgentSession } from "@/agentMode/session/AgentSession"; import { agentOriginEnabledModelEntries } from "@/agentMode/backends/shared/agentEnabledModels"; -import { - binaryPathInstallState, - simpleBinaryBackendProcess, -} from "@/agentMode/backends/shared/simpleBinaryBackend"; +import { simpleBinaryBackendProcess } from "@/agentMode/backends/shared/simpleBinaryBackend"; import type { EnabledModelEntry, ModeMapping, @@ -25,13 +24,20 @@ import type { } from "@/agentMode/session/types"; import type { BackendDescriptor, BackendProcess, InstallState } from "@/agentMode/session/types"; import { detectBinary } from "@/utils/detectBinary"; -import { codexAcpSearchDirs, resolveCodexAcpBinary } from "./codexBinaryResolver"; +import { CodexBinaryManager, codexProbeSettingsFingerprint } from "./CodexBinaryManager"; +import { + codexAcpSearchDirs, + resolveCodexAcpBinary, + resolveCodexAcpLauncher, + type CodexAcpBinaryResolverInput, +} from "./codexBinaryResolver"; export const CODEX_BINARY_NAME = "codex-acp"; -export const CODEX_INSTALL_COMMAND = - process.platform === "win32" - ? "irm https://gist.githubusercontent.com/logancyang/380ef4dbf9f98900771da76eca3d21e6/raw/install-codex-agent-mode-windows.ps1 | iex" - : "npm install -g @zed-industries/codex-acp"; +export const CODEX_INSTALL_COMMAND = CODEX_ACP_INSTALL_COMMAND; +export const CODEX_MIGRATION_COMMAND = CODEX_ACP_MIGRATION_COMMAND; + +let managerRef: CodexBinaryManager | null = null; +let managerNodePath: string | undefined; /** * Vocabulary mirrors codex-acp's advertised efforts. `minimal` is included @@ -44,7 +50,7 @@ export function updateCodexFields(partial: Partial): void updateAgentModeBackendFields("codex", partial); } -function codexAcpResolverEnv(): Parameters[0] { +function codexAcpResolverEnv(): CodexAcpBinaryResolverInput { return { homeDir: os.homedir(), platform: process.platform, @@ -57,9 +63,45 @@ function codexAcpResolverEnv(): Parameters[0] { }; } +export function resolveCodexNodePath(input: CodexAcpBinaryResolverInput): string | undefined { + const launcher = resolveCodexAcpLauncher(input); + return launcher?.kind === "node" ? launcher.command : undefined; +} + +export function getCodexBinaryManager(): CodexBinaryManager { + const resolverEnv = codexAcpResolverEnv(); + const nodePath = resolveCodexNodePath(resolverEnv); + if (!managerRef || managerNodePath !== nodePath) { + managerRef = new CodexBinaryManager({ + platform: resolverEnv.platform, + nodePath, + }); + managerNodePath = nodePath; + } + return managerRef; +} + +export function subscribeCodexInstallState( + getManager: () => Pick, + cb: () => void +): () => void { + return subscribeToSettingsChange((prev, next) => { + const previous = prev.agentMode?.backends?.codex; + const current = next.agentMode?.backends?.codex; + if (codexProbeSettingsFingerprint(previous) !== codexProbeSettingsFingerprint(current)) { + void getManager() + .refreshInstallState(current) + .catch((error) => logError("[AgentMode] Codex install-state refresh failed", error)); + return; + } + if (previous?.probe !== current?.probe) cb(); + }); +} + export async function detectCodexAcpPath(): Promise { const fromResolver = resolveCodexAcpBinary(codexAcpResolverEnv()); if (fromResolver) return fromResolver; + if (process.platform === "win32") return null; return detectBinary(CODEX_BINARY_NAME); } @@ -92,7 +134,7 @@ const codexWire: ModelWireCodec = { }; /** - * Codex backend — wraps `@zed-industries/codex-acp`, which inherits auth + * Codex backend — wraps `@agentclientprotocol/codex-acp`, which inherits auth * from the local `codex` CLI login. Auth is CLI-owned (no Copilot-side keys), * so the candidate models come entirely from the CLI's live `availableModels` * (active session or preloader cache); curation is the model-management @@ -135,7 +177,7 @@ export const CodexBackendDescriptor: BackendDescriptor = { }, getInstallState(settings: CopilotSettings): InstallState { - return binaryPathInstallState(settings.agentMode?.backends?.codex?.binaryPath); + return getCodexBinaryManager().getInstallState(settings.agentMode?.backends?.codex); }, getResolvedBinaryPath(settings: CopilotSettings): string | null { @@ -143,13 +185,7 @@ export const CodexBackendDescriptor: BackendDescriptor = { }, subscribeInstallState(_plugin: CopilotPlugin, cb: () => void): () => void { - return subscribeToSettingsChange((prev, next) => { - if ( - prev.agentMode?.backends?.codex?.binaryPath !== next.agentMode?.backends?.codex?.binaryPath - ) { - cb(); - } - }); + return subscribeCodexInstallState(getCodexBinaryManager, cb); }, openInstallUI(plugin: CopilotPlugin): void { @@ -165,32 +201,30 @@ export const CodexBackendDescriptor: BackendDescriptor = { // symlink. The per-agent toggle drives whether the symlink exists; no // deny synthesis is needed because Codex does not cross-discover from // `.claude/skills/` or `.opencode/skills/`. - return simpleBinaryBackendProcess(args, new CodexBackend()); + return simpleBinaryBackendProcess( + args, + new CodexBackend({ + platform: process.platform, + nodePath: resolveCodexNodePath(codexAcpResolverEnv()), + }) + ); }, SettingsPanel: CodexSettingsPanel, + async onPluginLoad(_plugin: CopilotPlugin): Promise { + await getCodexBinaryManager().refreshInstallState(); + }, + /** - * Codex exposes sandbox/approval presets via ACP setMode: `read-only`, - * `auto`, and `full-access`. We surface all three: - * - build → "auto" (workspace-write, on-request approvals) - * - plan → "read-only" (no writes, no exec; closest ACP analog) - * - auto-build → "full-access" (no sandbox, no approvals) - * - * Note: this is a sandbox restriction, not Codex CLI's real `ModeKind::Plan` - * (which would draft a plan artifact). That mode lives behind the app-server - * `turn/start.collaborationMode` field, which `@zed-industries/codex-acp` - * does not forward — it translates ACP modes to - * `Op::OverrideTurnContext { approval_policy, sandbox_policy }` only. - * Read-only is the closest available analog: the agent can read and reason - * but cannot mutate the vault, which matches user intent for "Plan". + * The replacement adapter exposes three approval/sandbox presets via ACP + * setMode. Read-only remains the closest available Plan behavior; the + * adapter's mode controls permissions rather than Codex collaboration mode. */ getModeMapping(): ModeMapping { return { kind: "setMode", - canonical: { default: "auto", plan: "read-only", auto: "full-access" }, - // Codex's `read-only` preset is a genuine no-write/no-exec sandbox, so it - // doubles as the fan-out read-only sandbox mode. + canonical: { default: "agent", plan: "read-only", auto: "agent-full-access" }, readOnlyModeId: "read-only", }; },