From b3b195351edcc71a6842465628c0306b8e24766c Mon Sep 17 00:00:00 2001
From: Fouad Matin <169186268+fouad-openai@users.noreply.github.com>
Date: Sat, 19 Apr 2025 16:23:27 -0700
Subject: [PATCH 1/6] feat: `/diff` command to view git diff (#426)

Adds `/diff` command to view git diff
---
 .../components/chat/terminal-chat-input.tsx   |  9 ++
 .../chat/terminal-chat-new-input.tsx          |  9 ++
 .../src/components/chat/terminal-chat.tsx     | 39 +++++++-
 codex-cli/src/components/diff-overlay.tsx     | 93 +++++++++++++++++++
 codex-cli/src/components/help-overlay.tsx     |  3 +
 .../src/utils/extract-applied-patches.ts      | 36 +++++++
 codex-cli/src/utils/get-diff.ts               | 29 ++++++
 codex-cli/src/utils/slash-commands.ts         |  5 +
 codex-cli/tests/slash-commands.test.ts        |  1 +
 .../terminal-chat-input-compact.test.tsx      |  1 +
 package.json                                  |  4 +-
 pnpm-workspace.yaml                           | 10 +-
 12 files changed, 232 insertions(+), 7 deletions(-)
 create mode 100644 codex-cli/src/components/diff-overlay.tsx
 create mode 100644 codex-cli/src/utils/extract-applied-patches.ts
 create mode 100644 codex-cli/src/utils/get-diff.ts

diff --git a/codex-cli/src/components/chat/terminal-chat-input.tsx b/codex-cli/src/components/chat/terminal-chat-input.tsx
index 59265221d4..e1bcbf9ea4 100644
--- a/codex-cli/src/components/chat/terminal-chat-input.tsx
+++ b/codex-cli/src/components/chat/terminal-chat-input.tsx
@@ -42,6 +42,7 @@ export default function TerminalChatInput({
   openModelOverlay,
   openApprovalOverlay,
   openHelpOverlay,
+  openDiffOverlay,
   onCompact,
   interruptAgent,
   active,
@@ -64,6 +65,7 @@ export default function TerminalChatInput({
   openModelOverlay: () => void;
   openApprovalOverlay: () => void;
   openHelpOverlay: () => void;
+  openDiffOverlay: () => void;
   onCompact: () => void;
   interruptAgent: () => void;
   active: boolean;
@@ -270,6 +272,12 @@ export default function TerminalChatInput({
         return;
       }
 
+      if (inputValue === "/diff") {
+        setInput("");
+        openDiffOverlay();
+        return;
+      }
+
       if (inputValue === "/compact") {
         setInput("");
         onCompact();
@@ -494,6 +502,7 @@ export default function TerminalChatInput({
       openApprovalOverlay,
       openModelOverlay,
       openHelpOverlay,
+      openDiffOverlay,
       history,
       onCompact,
       skipNextSubmit,
diff --git a/codex-cli/src/components/chat/terminal-chat-new-input.tsx b/codex-cli/src/components/chat/terminal-chat-new-input.tsx
index 9ceb4bbccc..57acba3625 100644
--- a/codex-cli/src/components/chat/terminal-chat-new-input.tsx
+++ b/codex-cli/src/components/chat/terminal-chat-new-input.tsx
@@ -52,6 +52,7 @@ export default function TerminalChatInput({
   openModelOverlay,
   openApprovalOverlay,
   openHelpOverlay,
+  openDiffOverlay,
   interruptAgent,
   active,
   thinkingSeconds,
@@ -72,6 +73,7 @@ export default function TerminalChatInput({
   openModelOverlay: () => void;
   openApprovalOverlay: () => void;
   openHelpOverlay: () => void;
+  openDiffOverlay: () => void;
   interruptAgent: () => void;
   active: boolean;
   thinkingSeconds: number;
@@ -230,6 +232,12 @@ export default function TerminalChatInput({
         return;
       }
 
+      if (inputValue === "/diff") {
+        setInput("");
+        openDiffOverlay();
+        return;
+      }
+
       if (inputValue.startsWith("/model")) {
         setInput("");
         openModelOverlay();
@@ -337,6 +345,7 @@ export default function TerminalChatInput({
       openApprovalOverlay,
       openModelOverlay,
       openHelpOverlay,
+      openDiffOverlay,
       history, // Add history to the dependency array
     ],
   );
diff --git a/codex-cli/src/components/chat/terminal-chat.tsx b/codex-cli/src/components/chat/terminal-chat.tsx
index e341cdfbbb..26112f1492 100644
--- a/codex-cli/src/components/chat/terminal-chat.tsx
+++ b/codex-cli/src/components/chat/terminal-chat.tsx
@@ -19,12 +19,15 @@ import { isLoggingEnabled, log } from "../../utils/agent/log.js";
 import { ReviewDecision } from "../../utils/agent/review.js";
 import { generateCompactSummary } from "../../utils/compact-summary.js";
 import { OPENAI_BASE_URL } from "../../utils/config.js";
+import { extractAppliedPatches as _extractAppliedPatches } from "../../utils/extract-applied-patches.js";
+import { getGitDiff } from "../../utils/get-diff.js";
 import { createInputItem } from "../../utils/input-utils.js";
 import { getAvailableModels } from "../../utils/model-utils.js";
 import { CLI_VERSION } from "../../utils/session.js";
 import { shortCwd } from "../../utils/short-path.js";
 import { saveRollout } from "../../utils/storage/save-rollout.js";
 import ApprovalModeOverlay from "../approval-mode-overlay.js";
+import DiffOverlay from "../diff-overlay.js";
 import HelpOverlay from "../help-overlay.js";
 import HistoryOverlay from "../history-overlay.js";
 import ModelOverlay from "../model-overlay.js";
@@ -180,9 +183,16 @@ export default function TerminalChat({
     submitConfirmation,
   } = useConfirmation();
   const [overlayMode, setOverlayMode] = useState<
-    "none" | "history" | "model" | "approval" | "help"
+    "none" | "history" | "model" | "approval" | "help" | "diff"
   >("none");
 
+  // Store the diff text when opening the diff overlay so the view isn’t
+  // recomputed on every re‑render while it is open.
+  // diffText is passed down to the DiffOverlay component. The setter is
+  // currently unused but retained for potential future updates. Prefix with
+  // an underscore so eslint ignores the unused variable.
+  const [diffText, _setDiffText] = useState<string>("");
+
   const [initialPrompt, setInitialPrompt] = useState(_initialPrompt);
   const [initialImagePaths, setInitialImagePaths] =
     useState(_initialImagePaths);
@@ -497,6 +507,26 @@ export default function TerminalChat({
             openModelOverlay={() => setOverlayMode("model")}
             openApprovalOverlay={() => setOverlayMode("approval")}
             openHelpOverlay={() => setOverlayMode("help")}
+            openDiffOverlay={() => {
+              const { isGitRepo, diff } = getGitDiff();
+              let text: string;
+              if (isGitRepo) {
+                text = diff;
+              } else {
+                text = "`/diff` — _not inside a git repository_";
+              }
+              setItems((prev) => [
+                ...prev,
+                {
+                  id: `diff-${Date.now()}`,
+                  type: "message",
+                  role: "system",
+                  content: [{ type: "input_text", text }],
+                },
+              ]);
+              // Ensure no overlay is shown.
+              setOverlayMode("none");
+            }}
             onCompact={handleCompact}
             active={overlayMode === "none"}
             interruptAgent={() => {
@@ -622,6 +652,13 @@ export default function TerminalChat({
         {overlayMode === "help" && (
           <HelpOverlay onExit={() => setOverlayMode("none")} />
         )}
+
+        {overlayMode === "diff" && (
+          <DiffOverlay
+            diffText={diffText}
+            onExit={() => setOverlayMode("none")}
+          />
+        )}
       </Box>
     </Box>
   );
diff --git a/codex-cli/src/components/diff-overlay.tsx b/codex-cli/src/components/diff-overlay.tsx
new file mode 100644
index 0000000000..8de85b87d5
--- /dev/null
+++ b/codex-cli/src/components/diff-overlay.tsx
@@ -0,0 +1,93 @@
+import { Box, Text, useInput } from "ink";
+import React, { useState } from "react";
+
+/**
+ * Simple scrollable view for displaying a diff.
+ * The component is intentionally lightweight and mirrors the UX of
+ * HistoryOverlay: Up/Down or j/k to scroll, PgUp/PgDn for paging and Esc to
+ * close. The caller is responsible for computing the diff text.
+ */
+export default function DiffOverlay({
+  diffText,
+  onExit,
+}: {
+  diffText: string;
+  onExit: () => void;
+}): JSX.Element {
+  const lines = diffText.length > 0 ? diffText.split("\n") : ["(no changes)"];
+
+  const [cursor, setCursor] = useState(0);
+
+  // Determine how many rows we can display – similar to HistoryOverlay.
+  const rows = process.stdout.rows || 24;
+  const headerRows = 2;
+  const footerRows = 1;
+  const maxVisible = Math.max(4, rows - headerRows - footerRows);
+
+  useInput((input, key) => {
+    if (key.escape || input === "q") {
+      onExit();
+      return;
+    }
+
+    if (key.downArrow || input === "j") {
+      setCursor((c) => Math.min(lines.length - 1, c + 1));
+    } else if (key.upArrow || input === "k") {
+      setCursor((c) => Math.max(0, c - 1));
+    } else if (key.pageDown) {
+      setCursor((c) => Math.min(lines.length - 1, c + maxVisible));
+    } else if (key.pageUp) {
+      setCursor((c) => Math.max(0, c - maxVisible));
+    } else if (input === "g") {
+      setCursor(0);
+    } else if (input === "G") {
+      setCursor(lines.length - 1);
+    }
+  });
+
+  const firstVisible = Math.min(
+    Math.max(0, cursor - Math.floor(maxVisible / 2)),
+    Math.max(0, lines.length - maxVisible),
+  );
+  const visible = lines.slice(firstVisible, firstVisible + maxVisible);
+
+  // Very small helper to colorize diff lines in a basic way.
+  function renderLine(line: string, idx: number): JSX.Element {
+    let color: "green" | "red" | "cyan" | undefined = undefined;
+    if (line.startsWith("+")) {
+      color = "green";
+    } else if (line.startsWith("-")) {
+      color = "red";
+    } else if (line.startsWith("@@") || line.startsWith("diff --git")) {
+      color = "cyan";
+    }
+    return (
+      <Text key={idx} color={color} wrap="truncate-end">
+        {line === "" ? " " : line}
+      </Text>
+    );
+  }
+
+  return (
+    <Box
+      flexDirection="column"
+      borderStyle="round"
+      borderColor="gray"
+      width={Math.min(120, process.stdout.columns || 120)}
+    >
+      <Box paddingX={1}>
+        <Text bold>Working tree diff ({lines.length} lines)</Text>
+      </Box>
+
+      <Box flexDirection="column" paddingX={1}>
+        {visible.map((line, idx) => {
+          return renderLine(line, firstVisible + idx);
+        })}
+      </Box>
+
+      <Box paddingX={1}>
+        <Text dimColor>esc Close ↑↓ Scroll PgUp/PgDn g/G First/Last</Text>
+      </Box>
+    </Box>
+  );
+}
diff --git a/codex-cli/src/components/help-overlay.tsx b/codex-cli/src/components/help-overlay.tsx
index 132add8307..6eeffb9efb 100644
--- a/codex-cli/src/components/help-overlay.tsx
+++ b/codex-cli/src/components/help-overlay.tsx
@@ -55,6 +55,9 @@ export default function HelpOverlay({
         <Text>
           <Text color="cyan">/bug</Text> – file a bug report with session log
         </Text>
+        <Text>
+          <Text color="cyan">/diff</Text> – view working tree git diff
+        </Text>
         <Text>
           <Text color="cyan">/compact</Text> – condense context into a summary
         </Text>
diff --git a/codex-cli/src/utils/extract-applied-patches.ts b/codex-cli/src/utils/extract-applied-patches.ts
new file mode 100644
index 0000000000..3e9bc10479
--- /dev/null
+++ b/codex-cli/src/utils/extract-applied-patches.ts
@@ -0,0 +1,36 @@
+import type { ResponseItem } from "openai/resources/responses/responses.mjs";
+
+/**
+ * Extracts the patch texts of all `apply_patch` tool calls from the given
+ * message history. Returns an empty string when none are found.
+ */
+export function extractAppliedPatches(items: Array<ResponseItem>): string {
+  const patches: Array<string> = [];
+
+  for (const item of items) {
+    if (item.type !== "function_call") {
+      continue;
+    }
+
+    const { name: toolName, arguments: argsString } = item as unknown as {
+      name: unknown;
+      arguments: unknown;
+    };
+
+    if (toolName !== "apply_patch" || typeof argsString !== "string") {
+      continue;
+    }
+
+    try {
+      const args = JSON.parse(argsString) as { patch?: string };
+      if (typeof args.patch === "string" && args.patch.length > 0) {
+        patches.push(args.patch.trim());
+      }
+    } catch {
+      // Ignore malformed JSON – we never want to crash the overlay.
+      continue;
+    }
+  }
+
+  return patches.join("\n\n");
+}
diff --git a/codex-cli/src/utils/get-diff.ts b/codex-cli/src/utils/get-diff.ts
new file mode 100644
index 0000000000..348ee42c78
--- /dev/null
+++ b/codex-cli/src/utils/get-diff.ts
@@ -0,0 +1,29 @@
+import { execSync } from "node:child_process";
+
+/**
+ * Returns the current Git diff for the working directory. If the current
+ * working directory is not inside a Git repository, `isGitRepo` will be
+ * false and `diff` will be an empty string.
+ */
+export function getGitDiff(): {
+  isGitRepo: boolean;
+  diff: string;
+} {
+  try {
+    // First check whether we are inside a git repository. `rev‑parse` exits
+    // with a non‑zero status code if not.
+    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
+    });
+
+    return { isGitRepo: true, diff: output };
+  } catch {
+    // Either git is not installed or we’re not inside a repository.
+    return { isGitRepo: false, diff: "" };
+  }
+}
diff --git a/codex-cli/src/utils/slash-commands.ts b/codex-cli/src/utils/slash-commands.ts
index 720941a98a..b276c49135 100644
--- a/codex-cli/src/utils/slash-commands.ts
+++ b/codex-cli/src/utils/slash-commands.ts
@@ -24,4 +24,9 @@ export const SLASH_COMMANDS: Array<SlashCommand> = [
   { command: "/model", description: "Open model selection panel" },
   { command: "/approval", description: "Open approval mode selection panel" },
   { command: "/bug", description: "Generate a prefilled GitHub bug report" },
+  {
+    command: "/diff",
+    description:
+      "Show git diff of the working directory (or applied patches if not in git)",
+  },
 ];
diff --git a/codex-cli/tests/slash-commands.test.ts b/codex-cli/tests/slash-commands.test.ts
index 4864aa26aa..b10a484f3b 100644
--- a/codex-cli/tests/slash-commands.test.ts
+++ b/codex-cli/tests/slash-commands.test.ts
@@ -10,6 +10,7 @@ test("SLASH_COMMANDS includes expected commands", () => {
   expect(commands).toContain("/model");
   expect(commands).toContain("/approval");
   expect(commands).toContain("/clearhistory");
+  expect(commands).toContain("/diff");
 });
 
 test("filters slash commands by prefix", () => {
diff --git a/codex-cli/tests/terminal-chat-input-compact.test.tsx b/codex-cli/tests/terminal-chat-input-compact.test.tsx
index d93a07abdf..2120aab49d 100644
--- a/codex-cli/tests/terminal-chat-input-compact.test.tsx
+++ b/codex-cli/tests/terminal-chat-input-compact.test.tsx
@@ -17,6 +17,7 @@ describe("TerminalChatInput compact command", () => {
       setItems: () => {},
       contextLeftPercent: 10,
       openOverlay: () => {},
+      openDiffOverlay: () => {},
       openModelOverlay: () => {},
       openApprovalOverlay: () => {},
       openHelpOverlay: () => {},
diff --git a/package.json b/package.json
index a54db4ada4..215616bec1 100644
--- a/package.json
+++ b/package.json
@@ -37,8 +37,8 @@
     "*.md": "prettier --write",
     ".github/workflows/*.yml": "prettier --write",
     "**/*.{js,ts,tsx}": [
-      "pnpm --filter @openai/codex run lint",
-      "pnpm --filter @openai/codex run typecheck"
+      "cd codex-cli && pnpm run lint",
+      "cd codex-cli && pnpm run typecheck"
     ]
   },
   "packageManager": "pnpm@10.8.1"
diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml
index 6aaf86b654..d3ac856082 100644
--- a/pnpm-workspace.yaml
+++ b/pnpm-workspace.yaml
@@ -1,5 +1,7 @@
 packages:
-  - 'codex-cli'
-  - 'docs'
-  # For future packages
-  - 'packages/*'
+  - codex-cli
+  - docs
+  - packages/*
+
+ignoredBuiltDependencies:
+  - esbuild

From a3889f92e4ce5ee1f8d1bbe9bea8ba91a8997607 Mon Sep 17 00:00:00 2001
From: Tomas Cupr <work@tomcupr.com>
Date: Sun, 20 Apr 2025 02:00:33 +0200
Subject: [PATCH 2/6] fix: `full-auto` support in quiet mode (#374)

Fixes https://github.com/openai/codex/issues/292

---------

Co-authored-by: Thibault Sottiaux <tibo@openai.com>
---
 codex-cli/src/cli.tsx | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/codex-cli/src/cli.tsx b/codex-cli/src/cli.tsx
index 169ead945c..08b95cb8b8 100644
--- a/codex-cli/src/cli.tsx
+++ b/codex-cli/src/cli.tsx
@@ -327,9 +327,6 @@ const additionalWritableRoots: ReadonlyArray<string> = (
 
 // If we are running in --quiet mode, do that and exit.
 const quietMode = Boolean(cli.flags.quiet);
-const autoApproveEverything = Boolean(
-  cli.flags.dangerouslyAutoApproveEverything,
-);
 const fullStdout = Boolean(cli.flags.fullStdout);
 
 if (quietMode) {
@@ -341,12 +338,19 @@ if (quietMode) {
     );
     process.exit(1);
   }
+
+  // Determine approval policy for quiet mode based on flags
+  const quietApprovalPolicy: ApprovalPolicy =
+    cli.flags.fullAuto || cli.flags.approvalMode === "full-auto"
+      ? AutoApprovalMode.FULL_AUTO
+      : cli.flags.autoEdit || cli.flags.approvalMode === "auto-edit"
+      ? AutoApprovalMode.AUTO_EDIT
+      : config.approvalMode || AutoApprovalMode.SUGGEST;
+
   await runQuietMode({
     prompt: prompt as string,
     imagePaths: imagePaths || [],
-    approvalPolicy: autoApproveEverything
-      ? AutoApprovalMode.FULL_AUTO
-      : config.approvalMode || AutoApprovalMode.SUGGEST,
+    approvalPolicy: quietApprovalPolicy,
     additionalWritableRoots,
     config,
   });
@@ -470,7 +474,12 @@ async function runQuietMode({
     getCommandConfirmation: (
       _command: Array<string>,
     ): Promise<CommandConfirmation> => {
-      return Promise.resolve({ review: ReviewDecision.NO_CONTINUE });
+      // In quiet mode, default to NO_CONTINUE, except when in full-auto mode
+      const reviewDecision =
+        approvalPolicy === AutoApprovalMode.FULL_AUTO
+          ? ReviewDecision.YES
+          : ReviewDecision.NO_CONTINUE;
+      return Promise.resolve({ review: reviewDecision });
     },
     onLastResponseId: () => {
       /* intentionally ignored in quiet mode */

From 63c99e7d8286f756d5bb41d8dd2856946ca41980 Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Sat, 19 Apr 2025 18:29:00 -0700
Subject: [PATCH 3/6] use spawn instead of exec to avoid injection
 vulnerability (#416)

https://github.com/openai/codex/pull/160 introduced a call to `exec()`
that takes a format string as an argument, but it is not clear that the
expansions within the format string are escaped safely. As written, it
is possible a carefully crafted command (e.g., if `cwd` were `"; && rm
-rf` or something...) could run arbitrary code.

Moving to `spawn()` makes this a bit better, as now at least `spawn()`
itself won't run an arbitrary process, though I suppose `osascript`
itself still could if the value passed to `-e` were abused. I'm not
clear on the escaping rules for AppleScript to ensure that `safePreview`
and `cwd` are injected safely.

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/416).
* #423
* #420
* #419
* __->__ #416
---
 codex-cli/src/components/chat/terminal-chat.tsx | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/codex-cli/src/components/chat/terminal-chat.tsx b/codex-cli/src/components/chat/terminal-chat.tsx
index 26112f1492..fd4cff5df5 100644
--- a/codex-cli/src/components/chat/terminal-chat.tsx
+++ b/codex-cli/src/components/chat/terminal-chat.tsx
@@ -32,7 +32,7 @@ import HelpOverlay from "../help-overlay.js";
 import HistoryOverlay from "../history-overlay.js";
 import ModelOverlay from "../model-overlay.js";
 import { Box, Text } from "ink";
-import { exec } from "node:child_process";
+import { spawn } from "node:child_process";
 import OpenAI from "openai";
 import React, { useEffect, useMemo, useRef, useState } from "react";
 import { inspect } from "util";
@@ -374,9 +374,10 @@ export default function TerminalChat({
           const safePreview = preview.replace(/"/g, '\\"');
           const title = "Codex CLI";
           const cwd = PWD;
-          exec(
-            `osascript -e 'display notification "${safePreview}" with title "${title}" subtitle "${cwd}" sound name "Ping"'`,
-          );
+          spawn("osascript", [
+            "-e",
+            `display notification "${safePreview}" with title "${title}" subtitle "${cwd}" sound name "Ping"`,
+          ]);
         }
       }
     }

From a7a4a69ccc5527bf691b2f0d1d230745790f1cdd Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Sat, 19 Apr 2025 18:29:11 -0700
Subject: [PATCH 4/6] CONFIG_DIR should not be in the list of writable roots by
 default

---
 .../src/utils/agent/sandbox/macos-seatbelt.ts     | 15 +++++++--------
 codex-cli/src/utils/agent/sandbox/raw-exec.ts     |  2 +-
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts b/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts
index 0317458261..760ba63d8c 100644
--- a/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts
+++ b/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts
@@ -3,11 +3,9 @@ import type { SpawnOptions } from "child_process";
 
 import { exec } from "./raw-exec.js";
 import { log } from "../log.js";
-import { CONFIG_DIR } from "src/utils/config.js";
 
 function getCommonRoots() {
   return [
-    CONFIG_DIR,
     // Without this root, it'll cause:
     // pyenv: cannot rehash: $HOME/.pyenv/shims isn't writable
     `${process.env["HOME"]}/.pyenv`,
@@ -17,16 +15,17 @@ function getCommonRoots() {
 export function execWithSeatbelt(
   cmd: Array<string>,
   opts: SpawnOptions,
-  writableRoots: Array<string>,
+  writableRoots: ReadonlyArray<string>,
   abortSignal?: AbortSignal,
 ): Promise<ExecResult> {
   let scopedWritePolicy: string;
   let policyTemplateParams: Array<string>;
-  if (writableRoots.length > 0) {
-    // Add `~/.codex` to the list of writable roots
-    // (if there's any already, not in read-only mode)
-    getCommonRoots().map((root) => writableRoots.push(root));
-    const { policies, params } = writableRoots
+
+  const fullWritableRoots = [...writableRoots, ...getCommonRoots()];
+  // In practice, fullWritableRoots will be non-empty, but we check just in
+  // case the logic to build up fullWritableRoots changes.
+  if (fullWritableRoots.length > 0) {
+    const { policies, params } = fullWritableRoots
       .map((root, index) => ({
         policy: `(subpath (param "WRITABLE_ROOT_${index}"))`,
         param: `-DWRITABLE_ROOT_${index}=${root}`,
diff --git a/codex-cli/src/utils/agent/sandbox/raw-exec.ts b/codex-cli/src/utils/agent/sandbox/raw-exec.ts
index 6cfb304731..e7bcdb2a13 100644
--- a/codex-cli/src/utils/agent/sandbox/raw-exec.ts
+++ b/codex-cli/src/utils/agent/sandbox/raw-exec.ts
@@ -21,7 +21,7 @@ const MAX_BUFFER = 1024 * 100; // 100 KB
 export function exec(
   command: Array<string>,
   options: SpawnOptions,
-  _writableRoots: Array<string>,
+  _writableRoots: ReadonlyArray<string>,
   abortSignal?: AbortSignal,
 ): Promise<ExecResult> {
   // Adapt command for the current platform (e.g., convert 'ls' to 'dir' on Windows)

From 4025cb6909e2cc90234918dfdfeb07cebf2f6365 Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Sat, 19 Apr 2025 18:29:11 -0700
Subject: [PATCH 5/6] remove unnecessary isLoggingEnabled() checks

---
 .../chat/terminal-chat-input-thinking.tsx     | 18 ++---
 .../components/chat/terminal-chat-input.tsx   | 18 ++---
 .../chat/terminal-chat-new-input.tsx          | 18 ++---
 .../src/components/chat/terminal-chat.tsx     | 65 +++++++------------
 codex-cli/src/utils/agent/agent-loop.ts       | 58 +++++++----------
 .../src/utils/agent/handle-exec-command.ts    | 33 +++++-----
 codex-cli/src/utils/agent/log.ts              |  8 +++
 .../src/utils/agent/platform-commands.ts      | 10 +--
 codex-cli/src/utils/agent/sandbox/raw-exec.ts | 19 ++----
 codex-cli/src/utils/config.ts                 | 14 ++--
 10 files changed, 103 insertions(+), 158 deletions(-)

diff --git a/codex-cli/src/components/chat/terminal-chat-input-thinking.tsx b/codex-cli/src/components/chat/terminal-chat-input-thinking.tsx
index 213dd8c9af..fdc8bd218c 100644
--- a/codex-cli/src/components/chat/terminal-chat-input-thinking.tsx
+++ b/codex-cli/src/components/chat/terminal-chat-input-thinking.tsx
@@ -1,4 +1,4 @@
-import { log, isLoggingEnabled } from "../../utils/agent/log.js";
+import { log } from "../../utils/agent/log.js";
 import { Box, Text, useInput, useStdin } from "ink";
 import React, { useState } from "react";
 import { useInterval } from "use-interval";
@@ -40,11 +40,9 @@ export default function TerminalChatInputThinking({
 
       const str = Buffer.isBuffer(data) ? data.toString("utf8") : data;
       if (str === "\x1b\x1b") {
-        if (isLoggingEnabled()) {
-          log(
-            "raw stdin: received collapsed ESC ESC – starting confirmation timer",
-          );
-        }
+        log(
+          "raw stdin: received collapsed ESC ESC – starting confirmation timer",
+        );
         setAwaitingConfirm(true);
         setTimeout(() => setAwaitingConfirm(false), 1500);
       }
@@ -65,15 +63,11 @@ export default function TerminalChatInputThinking({
       }
 
       if (awaitingConfirm) {
-        if (isLoggingEnabled()) {
-          log("useInput: second ESC detected – triggering onInterrupt()");
-        }
+        log("useInput: second ESC detected – triggering onInterrupt()");
         onInterrupt();
         setAwaitingConfirm(false);
       } else {
-        if (isLoggingEnabled()) {
-          log("useInput: first ESC detected – waiting for confirmation");
-        }
+        log("useInput: first ESC detected – waiting for confirmation");
         setAwaitingConfirm(true);
         setTimeout(() => setAwaitingConfirm(false), 1500);
       }
diff --git a/codex-cli/src/components/chat/terminal-chat-input.tsx b/codex-cli/src/components/chat/terminal-chat-input.tsx
index e1bcbf9ea4..525bbd9c23 100644
--- a/codex-cli/src/components/chat/terminal-chat-input.tsx
+++ b/codex-cli/src/components/chat/terminal-chat-input.tsx
@@ -6,7 +6,7 @@ import type {
 } from "openai/resources/responses/responses.mjs";
 
 import { TerminalChatCommandReview } from "./terminal-chat-command-review.js";
-import { log, isLoggingEnabled } from "../../utils/agent/log.js";
+import { log } from "../../utils/agent/log.js";
 import { loadConfig } from "../../utils/config.js";
 import { createInputItem } from "../../utils/input-utils.js";
 import { setSessionId } from "../../utils/session.js";
@@ -692,11 +692,9 @@ function TerminalChatInputThinking({
       const str = Buffer.isBuffer(data) ? data.toString("utf8") : data;
       if (str === "\x1b\x1b") {
         // Treat as the first Escape press – prompt the user for confirmation.
-        if (isLoggingEnabled()) {
-          log(
-            "raw stdin: received collapsed ESC ESC – starting confirmation timer",
-          );
-        }
+        log(
+          "raw stdin: received collapsed ESC ESC – starting confirmation timer",
+        );
         setAwaitingConfirm(true);
         setTimeout(() => setAwaitingConfirm(false), 1500);
       }
@@ -721,15 +719,11 @@ function TerminalChatInputThinking({
       }
 
       if (awaitingConfirm) {
-        if (isLoggingEnabled()) {
-          log("useInput: second ESC detected – triggering onInterrupt()");
-        }
+        log("useInput: second ESC detected – triggering onInterrupt()");
         onInterrupt();
         setAwaitingConfirm(false);
       } else {
-        if (isLoggingEnabled()) {
-          log("useInput: first ESC detected – waiting for confirmation");
-        }
+        log("useInput: first ESC detected – waiting for confirmation");
         setAwaitingConfirm(true);
         setTimeout(() => setAwaitingConfirm(false), 1500);
       }
diff --git a/codex-cli/src/components/chat/terminal-chat-new-input.tsx b/codex-cli/src/components/chat/terminal-chat-new-input.tsx
index 57acba3625..7dbe130e2c 100644
--- a/codex-cli/src/components/chat/terminal-chat-new-input.tsx
+++ b/codex-cli/src/components/chat/terminal-chat-new-input.tsx
@@ -8,7 +8,7 @@ import type {
 
 import MultilineTextEditor from "./multiline-editor";
 import { TerminalChatCommandReview } from "./terminal-chat-command-review.js";
-import { log, isLoggingEnabled } from "../../utils/agent/log.js";
+import { log } from "../../utils/agent/log.js";
 import { loadConfig } from "../../utils/config.js";
 import { createInputItem } from "../../utils/input-utils.js";
 import { setSessionId } from "../../utils/session.js";
@@ -505,11 +505,9 @@ function TerminalChatInputThinking({
       const str = Buffer.isBuffer(data) ? data.toString("utf8") : data;
       if (str === "\x1b\x1b") {
         // Treat as the first Escape press – prompt the user for confirmation.
-        if (isLoggingEnabled()) {
-          log(
-            "raw stdin: received collapsed ESC ESC – starting confirmation timer",
-          );
-        }
+        log(
+          "raw stdin: received collapsed ESC ESC – starting confirmation timer",
+        );
         setAwaitingConfirm(true);
         setTimeout(() => setAwaitingConfirm(false), 1500);
       }
@@ -531,15 +529,11 @@ function TerminalChatInputThinking({
       }
 
       if (awaitingConfirm) {
-        if (isLoggingEnabled()) {
-          log("useInput: second ESC detected – triggering onInterrupt()");
-        }
+        log("useInput: second ESC detected – triggering onInterrupt()");
         onInterrupt();
         setAwaitingConfirm(false);
       } else {
-        if (isLoggingEnabled()) {
-          log("useInput: first ESC detected – waiting for confirmation");
-        }
+        log("useInput: first ESC detected – waiting for confirmation");
         setAwaitingConfirm(true);
         setTimeout(() => setAwaitingConfirm(false), 1500);
       }
diff --git a/codex-cli/src/components/chat/terminal-chat.tsx b/codex-cli/src/components/chat/terminal-chat.tsx
index fd4cff5df5..825d0b9a92 100644
--- a/codex-cli/src/components/chat/terminal-chat.tsx
+++ b/codex-cli/src/components/chat/terminal-chat.tsx
@@ -15,7 +15,7 @@ import { formatCommandForDisplay } from "../../format-command.js";
 import { useConfirmation } from "../../hooks/use-confirmation.js";
 import { useTerminalSize } from "../../hooks/use-terminal-size.js";
 import { AgentLoop } from "../../utils/agent/agent-loop.js";
-import { isLoggingEnabled, log } from "../../utils/agent/log.js";
+import { log } from "../../utils/agent/log.js";
 import { ReviewDecision } from "../../utils/agent/review.js";
 import { generateCompactSummary } from "../../utils/compact-summary.js";
 import { OPENAI_BASE_URL } from "../../utils/config.js";
@@ -207,30 +207,25 @@ export default function TerminalChat({
   // ────────────────────────────────────────────────────────────────
   // DEBUG: log every render w/ key bits of state
   // ────────────────────────────────────────────────────────────────
-  if (isLoggingEnabled()) {
-    log(
-      `render – agent? ${Boolean(agentRef.current)} loading=${loading} items=${
-        items.length
-      }`,
-    );
-  }
+  log(
+    `render – agent? ${Boolean(agentRef.current)} loading=${loading} items=${
+      items.length
+    }`,
+  );
 
   useEffect(() => {
     // Skip recreating the agent if awaiting a decision on a pending confirmation
     if (confirmationPrompt != null) {
-      if (isLoggingEnabled()) {
-        log("skip AgentLoop recreation due to pending confirmationPrompt");
-      }
+      log("skip AgentLoop recreation due to pending confirmationPrompt");
       return;
     }
-    if (isLoggingEnabled()) {
-      log("creating NEW AgentLoop");
-      log(
-        `model=${model} instructions=${Boolean(
-          config.instructions,
-        )} approvalPolicy=${approvalPolicy}`,
-      );
-    }
+
+    log("creating NEW AgentLoop");
+    log(
+      `model=${model} instructions=${Boolean(
+        config.instructions,
+      )} approvalPolicy=${approvalPolicy}`,
+    );
 
     // Tear down any existing loop before creating a new one
     agentRef.current?.terminate();
@@ -298,14 +293,10 @@ export default function TerminalChat({
     // force a render so JSX below can "see" the freshly created agent
     forceUpdate();
 
-    if (isLoggingEnabled()) {
-      log(`AgentLoop created: ${inspect(agentRef.current, { depth: 1 })}`);
-    }
+    log(`AgentLoop created: ${inspect(agentRef.current, { depth: 1 })}`);
 
     return () => {
-      if (isLoggingEnabled()) {
-        log("terminating AgentLoop");
-      }
+      log("terminating AgentLoop");
       agentRef.current?.terminate();
       agentRef.current = undefined;
       forceUpdate(); // re‑render after teardown too
@@ -387,9 +378,7 @@ export default function TerminalChat({
   // Let's also track whenever the ref becomes available
   const agent = agentRef.current;
   useEffect(() => {
-    if (isLoggingEnabled()) {
-      log(`agentRef.current is now ${Boolean(agent)}`);
-    }
+    log(`agentRef.current is now ${Boolean(agent)}`);
   }, [agent]);
 
   // ---------------------------------------------------------------------
@@ -534,11 +523,9 @@ export default function TerminalChat({
               if (!agent) {
                 return;
               }
-              if (isLoggingEnabled()) {
-                log(
-                  "TerminalChat: interruptAgent invoked – calling agent.cancel()",
-                );
-              }
+              log(
+                "TerminalChat: interruptAgent invoked – calling agent.cancel()",
+              );
               agent.cancel();
               setLoading(false);
 
@@ -574,13 +561,11 @@ export default function TerminalChat({
             currentModel={model}
             hasLastResponse={Boolean(lastResponseId)}
             onSelect={(newModel) => {
-              if (isLoggingEnabled()) {
-                log(
-                  "TerminalChat: interruptAgent invoked – calling agent.cancel()",
-                );
-                if (!agent) {
-                  log("TerminalChat: agent is not ready yet");
-                }
+              log(
+                "TerminalChat: interruptAgent invoked – calling agent.cancel()",
+              );
+              if (!agent) {
+                log("TerminalChat: agent is not ready yet");
               }
               agent?.cancel();
               setLoading(false);
diff --git a/codex-cli/src/utils/agent/agent-loop.ts b/codex-cli/src/utils/agent/agent-loop.ts
index cbe3c56960..172ba67295 100644
--- a/codex-cli/src/utils/agent/agent-loop.ts
+++ b/codex-cli/src/utils/agent/agent-loop.ts
@@ -8,7 +8,7 @@ import type {
 } from "openai/resources/responses/responses.mjs";
 import type { Reasoning } from "openai/resources.mjs";
 
-import { log, isLoggingEnabled } from "./log.js";
+import { log } from "./log.js";
 import { OPENAI_BASE_URL, OPENAI_TIMEOUT_MS } from "../config.js";
 import { parseToolCallArguments } from "../parsers.js";
 import {
@@ -116,15 +116,13 @@ export class AgentLoop {
 
     // Reset the current stream to allow new requests
     this.currentStream = null;
-    if (isLoggingEnabled()) {
-      log(
-        `AgentLoop.cancel() invoked – currentStream=${Boolean(
-          this.currentStream,
-        )} execAbortController=${Boolean(
-          this.execAbortController,
-        )} generation=${this.generation}`,
-      );
-    }
+    log(
+      `AgentLoop.cancel() invoked – currentStream=${Boolean(
+        this.currentStream,
+      )} execAbortController=${Boolean(this.execAbortController)} generation=${
+        this.generation
+      }`,
+    );
     (
       this.currentStream as { controller?: { abort?: () => void } } | null
     )?.controller?.abort?.();
@@ -136,9 +134,7 @@ export class AgentLoop {
 
     // Create a new abort controller for future tool calls
     this.execAbortController = new AbortController();
-    if (isLoggingEnabled()) {
-      log("AgentLoop.cancel(): execAbortController.abort() called");
-    }
+    log("AgentLoop.cancel(): execAbortController.abort() called");
 
     // NOTE: We intentionally do *not* clear `lastResponseId` here.  If the
     // stream produced a `function_call` before the user cancelled, OpenAI now
@@ -174,9 +170,7 @@ export class AgentLoop {
     // this.onItem(cancelNotice);
 
     this.generation += 1;
-    if (isLoggingEnabled()) {
-      log(`AgentLoop.cancel(): generation bumped to ${this.generation}`);
-    }
+    log(`AgentLoop.cancel(): generation bumped to ${this.generation}`);
   }
 
   /**
@@ -315,13 +309,11 @@ export class AgentLoop {
     const callId: string = (item as any).call_id ?? (item as any).id;
 
     const args = parseToolCallArguments(rawArguments ?? "{}");
-    if (isLoggingEnabled()) {
-      log(
-        `handleFunctionCall(): name=${
-          name ?? "undefined"
-        } callId=${callId} args=${rawArguments}`,
-      );
-    }
+    log(
+      `handleFunctionCall(): name=${
+        name ?? "undefined"
+      } callId=${callId} args=${rawArguments}`,
+    );
 
     if (args == null) {
       const outputItem: ResponseInputItem.FunctionCallOutput = {
@@ -407,11 +399,9 @@ export class AgentLoop {
       // Create a fresh AbortController for this run so that tool calls from a
       // previous run do not accidentally get signalled.
       this.execAbortController = new AbortController();
-      if (isLoggingEnabled()) {
-        log(
-          `AgentLoop.run(): new execAbortController created (${this.execAbortController.signal}) for generation ${this.generation}`,
-        );
-      }
+      log(
+        `AgentLoop.run(): new execAbortController created (${this.execAbortController.signal}) for generation ${this.generation}`,
+      );
       // NOTE: We no longer (re‑)attach an `abort` listener to `hardAbort` here.
       // A single listener that forwards the `abort` to the current
       // `execAbortController` is installed once in the constructor. Re‑adding a
@@ -502,11 +492,9 @@ export class AgentLoop {
             const mergedInstructions = [prefix, this.instructions]
               .filter(Boolean)
               .join("\n");
-            if (isLoggingEnabled()) {
-              log(
-                `instructions (length ${mergedInstructions.length}): ${mergedInstructions}`,
-              );
-            }
+            log(
+              `instructions (length ${mergedInstructions.length}): ${mergedInstructions}`,
+            );
             // eslint-disable-next-line no-await-in-loop
             stream = await this.oai.responses.create({
               model: this.model,
@@ -733,9 +721,7 @@ export class AgentLoop {
         try {
           // eslint-disable-next-line no-await-in-loop
           for await (const event of stream) {
-            if (isLoggingEnabled()) {
-              log(`AgentLoop.run(): response event ${event.type}`);
-            }
+            log(`AgentLoop.run(): response event ${event.type}`);
 
             // process and surface each item (no‑op until we can depend on streaming events)
             if (event.type === "response.output_item.done") {
diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts
index 1af390e226..97b78c0817 100644
--- a/codex-cli/src/utils/agent/handle-exec-command.ts
+++ b/codex-cli/src/utils/agent/handle-exec-command.ts
@@ -223,23 +223,22 @@ async function execCommand(
       workdir = process.cwd();
     }
   }
-  if (isLoggingEnabled()) {
-    if (applyPatchCommand != null) {
-      log("EXEC running apply_patch command");
-    } else {
-      const { cmd, timeoutInMillis } = execInput;
-      // Seconds are a bit easier to read in log messages and most timeouts
-      // are specified as multiples of 1000, anyway.
-      const timeout =
-        timeoutInMillis != null
-          ? Math.round(timeoutInMillis / 1000).toString()
-          : "undefined";
-      log(
-        `EXEC running \`${formatCommandForDisplay(
-          cmd,
-        )}\` in workdir=${workdir} with timeout=${timeout}s`,
-      );
-    }
+
+  if (applyPatchCommand != null) {
+    log("EXEC running apply_patch command");
+  } else if (isLoggingEnabled()) {
+    const { cmd, timeoutInMillis } = execInput;
+    // Seconds are a bit easier to read in log messages and most timeouts
+    // are specified as multiples of 1000, anyway.
+    const timeout =
+      timeoutInMillis != null
+        ? Math.round(timeoutInMillis / 1000).toString()
+        : "undefined";
+    log(
+      `EXEC running \`${formatCommandForDisplay(
+        cmd,
+      )}\` in workdir=${workdir} with timeout=${timeout}s`,
+    );
   }
 
   // Note execApplyPatch() and exec() are coded defensively and should not
diff --git a/codex-cli/src/utils/agent/log.ts b/codex-cli/src/utils/agent/log.ts
index e804386566..e4aca06c79 100644
--- a/codex-cli/src/utils/agent/log.ts
+++ b/codex-cli/src/utils/agent/log.ts
@@ -124,6 +124,14 @@ export function log(message: string): void {
   (logger ?? initLogger()).log(message);
 }
 
+/**
+ * USE SPARINGLY! This function should only be used to guard a call to log() if
+ * the log message is large and you want to avoid constructing it if logging is
+ * disabled.
+ *
+ * `log()` is already a no-op if DEBUG is not set, so an extra
+ * `isLoggingEnabled()` check is unnecessary.
+ */
 export function isLoggingEnabled(): boolean {
   return (logger ?? initLogger()).isLoggingEnabled();
 }
diff --git a/codex-cli/src/utils/agent/platform-commands.ts b/codex-cli/src/utils/agent/platform-commands.ts
index 7be02c7ac5..085c575d44 100644
--- a/codex-cli/src/utils/agent/platform-commands.ts
+++ b/codex-cli/src/utils/agent/platform-commands.ts
@@ -2,7 +2,7 @@
  * Utility functions for handling platform-specific commands
  */
 
-import { log, isLoggingEnabled } from "./log.js";
+import { log } from "./log.js";
 
 /**
  * Map of Unix commands to their Windows equivalents
@@ -59,9 +59,7 @@ export function adaptCommandForPlatform(command: Array<string>): Array<string> {
     return command;
   }
 
-  if (isLoggingEnabled()) {
-    log(`Adapting command '${cmd}' for Windows platform`);
-  }
+  log(`Adapting command '${cmd}' for Windows platform`);
 
   // Create a new command array with the adapted command
   const adaptedCommand = [...command];
@@ -78,9 +76,7 @@ export function adaptCommandForPlatform(command: Array<string>): Array<string> {
     }
   }
 
-  if (isLoggingEnabled()) {
-    log(`Adapted command: ${adaptedCommand.join(" ")}`);
-  }
+  log(`Adapted command: ${adaptedCommand.join(" ")}`);
 
   return adaptedCommand;
 }
diff --git a/codex-cli/src/utils/agent/sandbox/raw-exec.ts b/codex-cli/src/utils/agent/sandbox/raw-exec.ts
index e7bcdb2a13..35ae8a4a92 100644
--- a/codex-cli/src/utils/agent/sandbox/raw-exec.ts
+++ b/codex-cli/src/utils/agent/sandbox/raw-exec.ts
@@ -7,7 +7,7 @@ import type {
   StdioPipe,
 } from "child_process";
 
-import { log, isLoggingEnabled } from "../log.js";
+import { log } from "../log.js";
 import { adaptCommandForPlatform } from "../platform-commands.js";
 import { spawn } from "child_process";
 import * as os from "os";
@@ -27,10 +27,7 @@ export function exec(
   // Adapt command for the current platform (e.g., convert 'ls' to 'dir' on Windows)
   const adaptedCommand = adaptCommandForPlatform(command);
 
-  if (
-    isLoggingEnabled() &&
-    JSON.stringify(adaptedCommand) !== JSON.stringify(command)
-  ) {
+  if (JSON.stringify(adaptedCommand) !== JSON.stringify(command)) {
     log(
       `Command adapted for platform: ${command.join(
         " ",
@@ -95,9 +92,7 @@ export function exec(
   // timely fashion.
   if (abortSignal) {
     const abortHandler = () => {
-      if (isLoggingEnabled()) {
-        log(`raw-exec: abort signal received – killing child ${child.pid}`);
-      }
+      log(`raw-exec: abort signal received – killing child ${child.pid}`);
       const killTarget = (signal: NodeJS.Signals) => {
         if (!child.pid) {
           return;
@@ -194,11 +189,9 @@ export function exec(
         exitCode = 1;
       }
 
-      if (isLoggingEnabled()) {
-        log(
-          `raw-exec: child ${child.pid} exited code=${exitCode} signal=${signal}`,
-        );
-      }
+      log(
+        `raw-exec: child ${child.pid} exited code=${exitCode} signal=${signal}`,
+      );
       resolve({
         stdout,
         stderr,
diff --git a/codex-cli/src/utils/config.ts b/codex-cli/src/utils/config.ts
index 8467653580..3be77059e0 100644
--- a/codex-cli/src/utils/config.ts
+++ b/codex-cli/src/utils/config.ts
@@ -8,7 +8,7 @@
 
 import type { FullAutoErrorMode } from "./auto-approval-mode.js";
 
-import { log, isLoggingEnabled } from "./agent/log.js";
+import { log } from "./agent/log.js";
 import { AutoApprovalMode } from "./auto-approval-mode.js";
 import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs";
 import { load as loadYaml, dump as dumpYaml } from "js-yaml";
@@ -245,15 +245,11 @@ export const loadConfig = (
       ? resolvePath(cwd, options.projectDocPath)
       : discoverProjectDocPath(cwd);
     if (projectDocPath) {
-      if (isLoggingEnabled()) {
-        log(
-          `[codex] Loaded project doc from ${projectDocPath} (${projectDoc.length} bytes)`,
-        );
-      }
+      log(
+        `[codex] Loaded project doc from ${projectDocPath} (${projectDoc.length} bytes)`,
+      );
     } else {
-      if (isLoggingEnabled()) {
-        log(`[codex] No project doc found in ${cwd}`);
-      }
+      log(`[codex] No project doc found in ${cwd}`);
     }
   }
 

From 698381bae42bc5c93167af819de46caac9aab911 Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Sat, 19 Apr 2025 18:29:11 -0700
Subject: [PATCH 6/6] reduce max output of ExecResult

---
 codex-cli/src/utils/agent/sandbox/raw-exec.ts | 130 +++++++++++++-----
 1 file changed, 96 insertions(+), 34 deletions(-)

diff --git a/codex-cli/src/utils/agent/sandbox/raw-exec.ts b/codex-cli/src/utils/agent/sandbox/raw-exec.ts
index 35ae8a4a92..22add83ed8 100644
--- a/codex-cli/src/utils/agent/sandbox/raw-exec.ts
+++ b/codex-cli/src/utils/agent/sandbox/raw-exec.ts
@@ -12,7 +12,10 @@ import { adaptCommandForPlatform } from "../platform-commands.js";
 import { spawn } from "child_process";
 import * as os from "os";
 
-const MAX_BUFFER = 1024 * 100; // 100 KB
+// Maximum output cap: either MAX_OUTPUT_LINES lines or MAX_OUTPUT_BYTES bytes,
+// whichever limit is reached first.
+const MAX_OUTPUT_BYTES = 1024 * 10; // 10 KB
+const MAX_OUTPUT_LINES = 256;
 
 /**
  * This function should never return a rejected promise: errors should be
@@ -143,37 +146,14 @@ export function exec(
   // resolve the promise and translate the failure into a regular
   // ExecResult object so the rest of the agent loop can carry on gracefully.
 
-  const stdoutChunks: Array<Buffer> = [];
-  const stderrChunks: Array<Buffer> = [];
-  let numStdoutBytes = 0;
-  let numStderrBytes = 0;
-  let hitMaxStdout = false;
-  let hitMaxStderr = false;
-
   return new Promise<ExecResult>((resolve) => {
-    child.stdout?.on("data", (data: Buffer) => {
-      if (!hitMaxStdout) {
-        numStdoutBytes += data.length;
-        if (numStdoutBytes <= MAX_BUFFER) {
-          stdoutChunks.push(data);
-        } else {
-          hitMaxStdout = true;
-        }
-      }
-    });
-    child.stderr?.on("data", (data: Buffer) => {
-      if (!hitMaxStderr) {
-        numStderrBytes += data.length;
-        if (numStderrBytes <= MAX_BUFFER) {
-          stderrChunks.push(data);
-        } else {
-          hitMaxStderr = true;
-        }
-      }
-    });
+    // Collect stdout and stderr up to configured limits.
+    const stdoutCollector = createTruncatingCollector(child.stdout!);
+    const stderrCollector = createTruncatingCollector(child.stderr!);
+
     child.on("exit", (code, signal) => {
-      const stdout = Buffer.concat(stdoutChunks).toString("utf8");
-      const stderr = Buffer.concat(stderrChunks).toString("utf8");
+      const stdout = stdoutCollector.getString();
+      const stderr = stderrCollector.getString();
 
       // Map (code, signal) to an exit code. We expect exactly one of the two
       // values to be non-null, but we code defensively to handle the case where
@@ -192,19 +172,101 @@ export function exec(
       log(
         `raw-exec: child ${child.pid} exited code=${exitCode} signal=${signal}`,
       );
-      resolve({
+
+      const execResult = {
         stdout,
         stderr,
         exitCode,
-      });
+      };
+      resolve(
+        addTruncationWarningsIfNecessary(
+          execResult,
+          stdoutCollector.hit,
+          stderrCollector.hit,
+        ),
+      );
     });
 
     child.on("error", (err) => {
-      resolve({
+      const execResult = {
         stdout: "",
         stderr: String(err),
         exitCode: 1,
-      });
+      };
+      resolve(
+        addTruncationWarningsIfNecessary(
+          execResult,
+          stdoutCollector.hit,
+          stderrCollector.hit,
+        ),
+      );
     });
   });
 }
+
+/**
+ * Creates a collector that accumulates data Buffers from a stream up to
+ * specified byte and line limits. After either limit is exceeded, further
+ * data is ignored.
+ */
+function createTruncatingCollector(
+  stream: NodeJS.ReadableStream,
+  byteLimit: number = MAX_OUTPUT_BYTES,
+  lineLimit: number = MAX_OUTPUT_LINES,
+) {
+  const chunks: Array<Buffer> = [];
+  let totalBytes = 0;
+  let totalLines = 0;
+  let hitLimit = false;
+
+  stream?.on("data", (data: Buffer) => {
+    if (hitLimit) {
+      return;
+    }
+    totalBytes += data.length;
+    for (let i = 0; i < data.length; i++) {
+      if (data[i] === 0x0a) {
+        totalLines++;
+      }
+    }
+    if (totalBytes <= byteLimit && totalLines <= lineLimit) {
+      chunks.push(data);
+    } else {
+      hitLimit = true;
+    }
+  });
+
+  return {
+    getString() {
+      return Buffer.concat(chunks).toString("utf8");
+    },
+    /** True if either byte or line limit was exceeded */
+    get hit(): boolean {
+      return hitLimit;
+    },
+  };
+}
+
+/**
+ * Adds a truncation warnings to stdout and stderr, if appropriate.
+ */
+function addTruncationWarningsIfNecessary(
+  execResult: ExecResult,
+  hitMaxStdout: boolean,
+  hitMaxStderr: boolean,
+): ExecResult {
+  if (!hitMaxStdout && !hitMaxStderr) {
+    return execResult;
+  } else {
+    const { stdout, stderr, exitCode } = execResult;
+    return {
+      stdout: hitMaxStdout
+        ? stdout + "\n\n[Output truncated: too many lines or bytes]"
+        : stdout,
+      stderr: hitMaxStderr
+        ? stderr + "\n\n[Output truncated: too many lines or bytes]"
+        : stderr,
+      exitCode,
+    };
+  }
+}