diff --git a/codex-cli/Dockerfile b/codex-cli/Dockerfile index 4ed3089bbb..ab41c5eaf9 100644 --- a/codex-cli/Dockerfile +++ b/codex-cli/Dockerfile @@ -46,6 +46,10 @@ RUN npm install -g codex.tgz \ && rm -rf /usr/local/share/npm-global/lib/node_modules/codex-cli/tests \ && rm -rf /usr/local/share/npm-global/lib/node_modules/codex-cli/docs +# Inside the container we consider the environment already sufficiently locked +# down, therefore instruct Codex CLI to allow running without sandboxing. +ENV CODEX_UNSAFE_ALLOW_NO_SANDBOX=1 + # Copy and set up firewall script as root. USER root COPY scripts/init_firewall.sh /usr/local/bin/ diff --git a/codex-cli/src/cli.tsx b/codex-cli/src/cli.tsx index 14b6b0fa68..4ddd7d4914 100644 --- a/codex-cli/src/cli.tsx +++ b/codex-cli/src/cli.tsx @@ -184,6 +184,10 @@ const cli = meow( }, ); +// --------------------------------------------------------------------------- +// Global flag handling +// --------------------------------------------------------------------------- + // Handle 'completion' subcommand before any prompting or API calls if (cli.input[0] === "completion") { const shell = cli.input[1] || "bash"; diff --git a/codex-cli/src/utils/agent/agent-loop.ts b/codex-cli/src/utils/agent/agent-loop.ts index b56c3efc8e..5fca001617 100644 --- a/codex-cli/src/utils/agent/agent-loop.ts +++ b/codex-cli/src/utils/agent/agent-loop.ts @@ -46,6 +46,7 @@ export type CommandConfirmation = { }; const alreadyProcessedResponses = new Set(); +const alreadyStagedItemIds = new Set(); type AgentLoopParams = { model: string; @@ -562,6 +563,12 @@ export class AgentLoop { return; } + // Skip items we've already processed to avoid staging duplicates + if (item.id && alreadyStagedItemIds.has(item.id)) { + return; + } + alreadyStagedItemIds.add(item.id); + // Store the item so the final flush can still operate on a complete list. // We'll nil out entries once they're delivered. const idx = staged.push(item) - 1; diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts index 85d6869192..6cb48016ad 100644 --- a/codex-cli/src/utils/agent/handle-exec-command.ts +++ b/codex-cli/src/utils/agent/handle-exec-command.ts @@ -1,17 +1,18 @@ import type { CommandConfirmation } from "./agent-loop.js"; -import type { AppConfig } from "../config.js"; -import type { ExecInput } from "./sandbox/interface.js"; import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; +import type { ExecInput } from "./sandbox/interface.js"; import type { ResponseInputItem } from "openai/resources/responses/responses.mjs"; -import { exec, execApplyPatch } from "./exec.js"; -import { ReviewDecision } from "./review.js"; -import { FullAutoErrorMode } from "../auto-approval-mode.js"; -import { SandboxType } from "./sandbox/interface.js"; import { canAutoApprove } from "../../approvals.js"; import { formatCommandForDisplay } from "../../format-command.js"; +import { FullAutoErrorMode } from "../auto-approval-mode.js"; +import { CODEX_UNSAFE_ALLOW_NO_SANDBOX, type AppConfig } from "../config.js"; +import { exec, execApplyPatch } from "./exec.js"; +import { ReviewDecision } from "./review.js"; import { isLoggingEnabled, log } from "../logger/log.js"; +import { SandboxType } from "./sandbox/interface.js"; import { access } from "fs/promises"; +import { execFile } from "node:child_process"; // --------------------------------------------------------------------------- // Session‑level cache of commands that the user has chosen to always approve. @@ -270,30 +271,44 @@ async function execCommand( }; } -const isInLinux = async (): Promise => { - try { - await access("/proc/1/cgroup"); - return true; - } catch { - return false; - } -}; +/** + * Return `true` if the `sandbox-exec` binary can be located. This intentionally does **not** + * spawn the binary – we only care about its presence. + */ +export const isSandboxExecAvailable = (): Promise => + new Promise((res) => + execFile( + "command", + ["-v", "sandbox-exec"], + { signal: AbortSignal.timeout(200) }, + (err) => res(!err), // exit 0 ⇒ found + ), + ); async function getSandbox(runInSandbox: boolean): Promise { if (runInSandbox) { if (process.platform === "darwin") { - return SandboxType.MACOS_SEATBELT; - } else if (await isInLinux()) { - return SandboxType.NONE; - } else if (process.platform === "win32") { - // On Windows, we don't have a sandbox implementation yet, so we fall back to NONE - // instead of throwing an error, which would crash the application - log( - "WARNING: Sandbox was requested but is not available on Windows. Continuing without sandbox.", - ); + // On macOS we rely on the system-provided `sandbox-exec` binary to + // enforce the Seatbelt profile. However, starting with macOS 14 the + // executable may be removed from the default installation or the user + // might be running the CLI on a stripped-down environment (for + // instance, inside certain CI images). Attempting to spawn a missing + // binary makes Node.js throw an *uncaught* `ENOENT` error further down + // the stack which crashes the whole CLI. + if (await isSandboxExecAvailable()) { + return SandboxType.MACOS_SEATBELT; + } else { + throw new Error( + "Sandbox was mandated, but 'sandbox-exec' was not found in PATH!", + ); + } + } else if (CODEX_UNSAFE_ALLOW_NO_SANDBOX) { + // Allow running without a sandbox if the user has explicitly marked the + // environment as already being sufficiently locked-down. return SandboxType.NONE; } - // For other platforms, still throw an error as before + + // For all else, we hard fail if the user has requested a sandbox and none is available. throw new Error("Sandbox was mandated, but no sandbox is available!"); } else { return SandboxType.NONE; diff --git a/codex-cli/src/utils/config.ts b/codex-cli/src/utils/config.ts index 7bd6052e52..6d7d682bb4 100644 --- a/codex-cli/src/utils/config.ts +++ b/codex-cli/src/utils/config.ts @@ -11,11 +11,37 @@ import type { FullAutoErrorMode } from "./auto-approval-mode.js"; import { AutoApprovalMode } from "./auto-approval-mode.js"; import { log } from "./logger/log.js"; import { providers } from "./providers.js"; +import { config as loadDotenv } from "dotenv"; import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; import { load as loadYaml, dump as dumpYaml } from "js-yaml"; import { homedir } from "os"; import { dirname, join, extname, resolve as resolvePath } from "path"; +// --------------------------------------------------------------------------- +// User‑wide environment config (~/.codex.env) +// --------------------------------------------------------------------------- + +// Load a user‑level dotenv file **after** process.env and any project‑local +// .env file (loaded via "dotenv/config" in cli.tsx) are in place. We rely on +// dotenv's default behaviour of *not* overriding existing variables so that +// the precedence order becomes: +// 1. Explicit environment variables +// 2. Project‑local .env (handled in cli.tsx) +// 3. User‑wide ~/.codex.env (loaded here) +// This guarantees that users can still override the global key on a per‑project +// basis while enjoying the convenience of a persistent default. + +// Skip when running inside Vitest to avoid interfering with the FS mocks used +// by tests that stub out `fs` *after* importing this module. +const USER_WIDE_CONFIG_PATH = join(homedir(), ".codex.env"); + +const isVitest = + typeof (globalThis as { vitest?: unknown }).vitest !== "undefined"; + +if (!isVitest) { + loadDotenv({ path: USER_WIDE_CONFIG_PATH }); +} + export const DEFAULT_AGENTIC_MODEL = "o4-mini"; export const DEFAULT_FULL_CONTEXT_MODEL = "gpt-4.1"; export const DEFAULT_APPROVAL_MODE = AutoApprovalMode.SUGGEST; @@ -39,6 +65,12 @@ export let OPENAI_API_KEY = process.env["OPENAI_API_KEY"] || ""; export const OPENAI_ORGANIZATION = process.env["OPENAI_ORGANIZATION"] || ""; export const OPENAI_PROJECT = process.env["OPENAI_PROJECT"] || ""; +// Can be set `true` when Codex is running in an environment that is marked as already +// considered sufficiently locked-down so that we allow running wihtout an explicit sandbox. +export const CODEX_UNSAFE_ALLOW_NO_SANDBOX = Boolean( + process.env["CODEX_UNSAFE_ALLOW_NO_SANDBOX"] || "", +); + export function setApiKey(apiKey: string): void { OPENAI_API_KEY = apiKey; } @@ -117,7 +149,7 @@ export type StoredConfig = { // propagating to existing users until they explicitly set a model. export const EMPTY_STORED_CONFIG: StoredConfig = { model: "" }; -// Pre‑stringified JSON variant so we don’t stringify repeatedly. +// Pre‑stringified JSON variant so we don't stringify repeatedly. const EMPTY_CONFIG_JSON = JSON.stringify(EMPTY_STORED_CONFIG, null, 2) + "\n"; export type MemoryConfig = { diff --git a/codex-cli/src/utils/get-diff.ts b/codex-cli/src/utils/get-diff.ts index 348ee42c78..9ac7844d2a 100644 --- a/codex-cli/src/utils/get-diff.ts +++ b/codex-cli/src/utils/get-diff.ts @@ -1,5 +1,26 @@ import { execSync } from "node:child_process"; +// The objects thrown by `child_process.execSync()` are `Error` instances that +// include additional, undocumented properties such as `status` (exit code) and +// `stdout` (captured standard output). Declare a minimal interface that captures +// just the fields we need so that we can avoid the use of `any` while keeping +// the checks type-safe. +interface ExecSyncError extends Error { + // Exit status code. When a diff is produced, git exits with code 1 which we + // treat as a non-error signal. + status?: number; + // Captured stdout. We rely on this to obtain the diff output when git exits + // with status 1. + stdout?: string; +} + +// Type-guard that narrows an unknown value to `ExecSyncError`. +function isExecSyncError(err: unknown): err is ExecSyncError { + return ( + typeof err === "object" && err != null && "status" in err && "stdout" in err + ); +} + /** * Returns the current Git diff for the working directory. If the current * working directory is not inside a Git repository, `isGitRepo` will be @@ -15,13 +36,86 @@ export function getGitDiff(): { execSync("git rev-parse --is-inside-work-tree", { stdio: "ignore" }); // If the above call didn’t throw, we are inside a git repo. Retrieve the - // diff including color codes so that the overlay can render them. - const output = execSync("git diff --color", { - encoding: "utf8", - maxBuffer: 10 * 1024 * 1024, // 10 MB ought to be enough for now - }); + // diff for tracked files **and** include any untracked files so that the + // `/diff` overlay shows a complete picture of the working tree state. - return { isGitRepo: true, diff: output }; + // 1. Diff for tracked files (unchanged behaviour) + let trackedDiff = ""; + try { + trackedDiff = execSync("git diff --color", { + encoding: "utf8", + maxBuffer: 10 * 1024 * 1024, // 10 MB ought to be enough for now + }); + } catch (err) { + // Exit status 1 simply means that differences were found. Capture the + // diff from stdout in that case. Re-throw for any other status codes. + if ( + isExecSyncError(err) && + err.status === 1 && + typeof err.stdout === "string" + ) { + trackedDiff = err.stdout; + } else { + throw err; + } + } + + // 2. Determine untracked files. + // We use `git ls-files --others --exclude-standard` which outputs paths + // relative to the repository root, one per line. These are files that + // are not tracked *and* are not ignored by .gitignore. + const untrackedOutput = execSync( + "git ls-files --others --exclude-standard", + { + encoding: "utf8", + maxBuffer: 10 * 1024 * 1024, + }, + ); + + const untrackedFiles = untrackedOutput + .split("\n") + .map((p) => p.trim()) + .filter(Boolean); + + let untrackedDiff = ""; + + const nullDevice = process.platform === "win32" ? "NUL" : "/dev/null"; + + for (const file of untrackedFiles) { + try { + // `git diff --no-index` produces a diff even outside the index by + // comparing two paths. We compare the file against /dev/null so that + // the file is treated as "new". + // + // `git diff --color --no-index /dev/null ` exits with status 1 + // when differences are found, so we capture stdout from the thrown + // error object instead of letting it propagate. + execSync(`git diff --color --no-index -- "${nullDevice}" "${file}"`, { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + maxBuffer: 10 * 1024 * 1024, + }); + } catch (err) { + if ( + isExecSyncError(err) && + // Exit status 1 simply means that the two inputs differ, which is + // exactly what we expect here. Any other status code indicates a + // real error (e.g. the file disappeared between the ls-files and + // diff calls), so re-throw those. + err.status === 1 && + typeof err.stdout === "string" + ) { + untrackedDiff += err.stdout; + } else { + throw err; + } + } + } + + // Concatenate tracked and untracked diffs. + const combinedDiff = `${trackedDiff}${untrackedDiff}`; + + return { isGitRepo: true, diff: combinedDiff }; } catch { // Either git is not installed or we’re not inside a repository. return { isGitRepo: false, diff: "" }; diff --git a/codex-cli/tests/agent-dedupe-items.test.ts b/codex-cli/tests/agent-dedupe-items.test.ts new file mode 100644 index 0000000000..148aa7440f --- /dev/null +++ b/codex-cli/tests/agent-dedupe-items.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect, vi } from "vitest"; + +// --------------------------------------------------------------------------- +// This regression test ensures that AgentLoop only surfaces each response item +// once even when the same item appears multiple times in the OpenAI streaming +// response (e.g. as an early `response.output_item.done` event *and* again in +// the final `response.completed` payload). +// --------------------------------------------------------------------------- + +// Fake OpenAI stream that emits the *same* message twice: first as an +// incremental output event and then again in the turn completion payload. +class FakeStream { + public controller = { abort: vi.fn() }; + + async *[Symbol.asyncIterator]() { + // 1) Early incremental item. + yield { + type: "response.output_item.done", + item: { + type: "message", + id: "call-dedupe-1", + role: "assistant", + content: [{ type: "input_text", text: "Hello!" }], + }, + } as any; + + // 2) Turn completion containing the *same* item again. + yield { + type: "response.completed", + response: { + id: "resp-dedupe-1", + status: "completed", + output: [ + { + type: "message", + id: "call-dedupe-1", + role: "assistant", + content: [{ type: "input_text", text: "Hello!" }], + }, + ], + }, + } as any; + } +} + +// Intercept the OpenAI SDK used inside AgentLoop so we can inject our fake +// streaming implementation. +vi.mock("openai", () => { + class FakeOpenAI { + public responses = { + create: async () => new FakeStream(), + }; + } + + class APIConnectionTimeoutError extends Error {} + + return { __esModule: true, default: FakeOpenAI, APIConnectionTimeoutError }; +}); + +// Stub approvals / formatting helpers – not relevant here. +vi.mock("../src/approvals.js", () => ({ + __esModule: true, + alwaysApprovedCommands: new Set(), + canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false }) as any, + isSafeCommand: () => null, +})); + +vi.mock("../src/format-command.js", () => ({ + __esModule: true, + formatCommandForDisplay: (cmd: Array) => cmd.join(" "), +})); + +vi.mock("../src/utils/agent/log.js", () => ({ + __esModule: true, + log: () => {}, + isLoggingEnabled: () => false, +})); + +// After the dependency mocks we can import the module under test. +import { AgentLoop } from "../src/utils/agent/agent-loop.js"; + +describe("AgentLoop deduplicates output items", () => { + it("invokes onItem exactly once for duplicate items with the same id", async () => { + const received: Array = []; + + const agent = new AgentLoop({ + model: "any", + instructions: "", + config: { model: "any", instructions: "", notify: false }, + approvalPolicy: { mode: "auto" } as any, + additionalWritableRoots: [], + onItem: (item) => received.push(item), + onLoading: () => {}, + getCommandConfirmation: async () => ({ review: "yes" }) as any, + onLastResponseId: () => {}, + }); + + const userMsg = [ + { + type: "message", + role: "user", + content: [{ type: "input_text", text: "hi" }], + }, + ]; + + await agent.run(userMsg as any); + + // Give the setTimeout(3ms) inside AgentLoop.stageItem a chance to fire. + await new Promise((r) => setTimeout(r, 20)); + + // Count how many times the duplicate item surfaced. + const appearances = received.filter((i) => i.id === "call-dedupe-1").length; + expect(appearances).toBe(1); + }); +}); diff --git a/codex-cli/tests/user-config-env.test.ts b/codex-cli/tests/user-config-env.test.ts new file mode 100644 index 0000000000..76cdcbacd7 --- /dev/null +++ b/codex-cli/tests/user-config-env.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, writeFileSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; + +/** + * Verifies that ~/.codex.env is parsed (lowest‑priority) when present. + */ + +describe("user‑wide ~/.codex.env support", () => { + const ORIGINAL_HOME = process.env["HOME"]; + const ORIGINAL_API_KEY = process.env["OPENAI_API_KEY"]; + + let tempHome: string; + + beforeEach(() => { + // Create an isolated fake $HOME directory. + tempHome = mkdtempSync(join(tmpdir(), "codex-home-")); + process.env["HOME"] = tempHome; + + // Ensure the env var is unset so that the file value is picked up. + delete process.env["OPENAI_API_KEY"]; + + // Write ~/.codex.env with a dummy key. + writeFileSync( + join(tempHome, ".codex.env"), + "OPENAI_API_KEY=my-home-key\n", + { + encoding: "utf8", + }, + ); + }); + + afterEach(() => { + // Cleanup temp directory. + try { + rmSync(tempHome, { recursive: true, force: true }); + } catch { + // ignore + } + + // Restore original env. + if (ORIGINAL_HOME !== undefined) { + process.env["HOME"] = ORIGINAL_HOME; + } else { + delete process.env["HOME"]; + } + + if (ORIGINAL_API_KEY !== undefined) { + process.env["OPENAI_API_KEY"] = ORIGINAL_API_KEY; + } else { + delete process.env["OPENAI_API_KEY"]; + } + }); + + it("loads the API key from ~/.codex.env when not set elsewhere", async () => { + // Import the config module AFTER setting up the fake env. + const { getApiKey } = await import("../src/utils/config.js"); + + expect(getApiKey("openai")).toBe("my-home-key"); + }); +}); diff --git a/codex-rs/tui/src/conversation_history_widget.rs b/codex-rs/tui/src/conversation_history_widget.rs index de1dbba963..5d374794e9 100644 --- a/codex-rs/tui/src/conversation_history_widget.rs +++ b/codex-rs/tui/src/conversation_history_widget.rs @@ -238,7 +238,7 @@ impl WidgetRef for ConversationHistoryWidget { fn render_ref(&self, area: Rect, buf: &mut Buffer) { let (title, border_style) = if self.has_input_focus { ( - "Messages (↑/↓ or j/k = line, b/u = PgUp, space/d = PgDn)", + "Messages (↑/↓ or j/k = line, b/space = page)", Style::default().fg(Color::LightYellow), ) } else {