Merge 07c1f65e65 into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2025-04-28 09:21:56 -07:00
committed by GitHub
9 changed files with 365 additions and 32 deletions
+4
View File
@@ -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/
+4
View File
@@ -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";
+7
View File
@@ -46,6 +46,7 @@ export type CommandConfirmation = {
};
const alreadyProcessedResponses = new Set();
const alreadyStagedItemIds = new Set<string>();
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;
@@ -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<boolean> => {
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<boolean> =>
new Promise((res) =>
execFile(
"command",
["-v", "sandbox-exec"],
{ signal: AbortSignal.timeout(200) },
(err) => res(!err), // exit 0 ⇒ found
),
);
async function getSandbox(runInSandbox: boolean): Promise<SandboxType> {
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;
+33 -1
View File
@@ -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 = {
+100 -6
View File
@@ -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 <file>` 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: "" };
+115
View File
@@ -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<string>(),
canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false }) as any,
isSafeCommand: () => null,
}));
vi.mock("../src/format-command.js", () => ({
__esModule: true,
formatCommandForDisplay: (cmd: Array<string>) => 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<any> = [];
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);
});
});
+62
View File
@@ -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");
});
});
@@ -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 {