From fb16eab4fb5ec3844a7cf4942c04316806189bac Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Apr 2025 13:48:34 -0700 Subject: [PATCH 01/14] Back out @lib indirection in tsconfig.json --- codex-cli/package-lock.json | 4 +- codex-cli/src/app.tsx | 2 +- codex-cli/src/{lib => }/approvals.ts | 2 +- codex-cli/src/cli.tsx | 4 +- .../src/components/chat/multiline-editor.tsx | 2 +- .../chat/terminal-chat-tool-call-item.tsx | 2 +- .../src/components/chat/terminal-chat.tsx | 4 +- codex-cli/src/{lib => }/format-command.ts | 0 codex-cli/src/{lib => }/parse-apply-patch.ts | 0 codex-cli/src/{lib => }/text-buffer.ts | 0 codex-cli/src/utils/agent/agent-loop.ts | 2 +- codex-cli/src/utils/agent/exec.ts | 2 +- .../src/utils/agent/handle-exec-command.ts | 6 +- codex-cli/src/utils/agent/review.ts | 2 +- codex-cli/src/utils/parsers.ts | 55 ++----------------- codex-cli/tests/agent-cancel-early.test.ts | 4 +- .../tests/agent-cancel-prev-response.test.ts | 4 +- codex-cli/tests/agent-cancel-race.test.ts | 4 +- codex-cli/tests/agent-cancel.test.ts | 4 +- .../tests/agent-function-call-id.test.ts | 4 +- .../tests/agent-generic-network-error.test.ts | 4 +- .../tests/agent-invalid-request-error.test.ts | 4 +- .../tests/agent-max-tokens-error.test.ts | 4 +- codex-cli/tests/agent-network-errors.test.ts | 4 +- codex-cli/tests/agent-project-doc.test.ts | 4 +- .../tests/agent-rate-limit-error.test.ts | 4 +- codex-cli/tests/agent-server-retry.test.ts | 4 +- codex-cli/tests/agent-terminate.test.ts | 4 +- codex-cli/tests/agent-thinking-time.test.ts | 4 +- codex-cli/tests/approvals.test.ts | 4 +- codex-cli/tests/external-editor.test.ts | 2 +- codex-cli/tests/format-command.test.ts | 2 +- .../tests/invalid-command-handling.test.ts | 4 +- ...ultiline-external-editor-shortcut.test.tsx | 2 +- .../tests/multiline-history-behavior.test.tsx | 6 +- codex-cli/tests/parse-apply-patch.test.ts | 2 +- .../tests/text-buffer-copy-paste.test.ts | 2 +- codex-cli/tests/text-buffer-crlf.test.ts | 2 +- codex-cli/tests/text-buffer-gaps.test.ts | 2 +- codex-cli/tests/text-buffer-word.test.ts | 2 +- codex-cli/tests/text-buffer.test.ts | 2 +- codex-cli/tsconfig.json | 3 - 42 files changed, 65 insertions(+), 113 deletions(-) rename codex-cli/src/{lib => }/approvals.ts (99%) rename codex-cli/src/{lib => }/format-command.ts (100%) rename codex-cli/src/{lib => }/parse-apply-patch.ts (100%) rename codex-cli/src/{lib => }/text-buffer.ts (100%) diff --git a/codex-cli/package-lock.json b/codex-cli/package-lock.json index d29561b53a..589ee59659 100644 --- a/codex-cli/package-lock.json +++ b/codex-cli/package-lock.json @@ -1,12 +1,12 @@ { "name": "@openai/codex", - "version": "0.1.04160940", + "version": "0.1.04161241", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@openai/codex", - "version": "0.1.04160940", + "version": "0.1.04161241", "license": "Apache-2.0", "dependencies": { "@inkjs/ui": "^2.0.0", diff --git a/codex-cli/src/app.tsx b/codex-cli/src/app.tsx index dbb0cdedc8..c0b8c6f4e3 100644 --- a/codex-cli/src/app.tsx +++ b/codex-cli/src/app.tsx @@ -1,5 +1,5 @@ +import type { ApprovalPolicy } from "./approvals"; import type { AppConfig } from "./utils/config"; -import type { ApprovalPolicy } from "@lib/approvals"; import type { ResponseItem } from "openai/resources/responses/responses"; import TerminalChat from "./components/chat/terminal-chat"; diff --git a/codex-cli/src/lib/approvals.ts b/codex-cli/src/approvals.ts similarity index 99% rename from codex-cli/src/lib/approvals.ts rename to codex-cli/src/approvals.ts index 8985939a43..0cf3703b54 100644 --- a/codex-cli/src/lib/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -3,7 +3,7 @@ import type { ParseEntry, ControlOperator } from "shell-quote"; import { identify_files_added, identify_files_needed, -} from "../utils/agent/apply-patch"; +} from "./utils/agent/apply-patch"; import * as path from "path"; import { parse } from "shell-quote"; diff --git a/codex-cli/src/cli.tsx b/codex-cli/src/cli.tsx index 5e43dff1f3..0af421a431 100644 --- a/codex-cli/src/cli.tsx +++ b/codex-cli/src/cli.tsx @@ -1,9 +1,9 @@ #!/usr/bin/env node import type { AppRollout } from "./app"; +import type { ApprovalPolicy } from "./approvals"; import type { CommandConfirmation } from "./utils/agent/agent-loop"; import type { AppConfig } from "./utils/config"; -import type { ApprovalPolicy } from "@lib/approvals"; import type { ResponseItem } from "openai/resources/responses/responses"; import App from "./app"; @@ -124,7 +124,7 @@ const cli = meow( fullContext: { type: "boolean", aliases: ["f"], - description: `Run in full-context editing approach. The model is given the whole code + description: `Run in full-context editing approach. The model is given the whole code directory as context and performs changes in one go without acting.`, }, }, diff --git a/codex-cli/src/components/chat/multiline-editor.tsx b/codex-cli/src/components/chat/multiline-editor.tsx index c18555447e..c99961bbcd 100644 --- a/codex-cli/src/components/chat/multiline-editor.tsx +++ b/codex-cli/src/components/chat/multiline-editor.tsx @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { useTerminalSize } from "../../hooks/use-terminal-size"; -import TextBuffer from "../../lib/text-buffer.js"; +import TextBuffer from "../../text-buffer.js"; import chalk from "chalk"; import { Box, Text, useInput, useStdin } from "ink"; import { EventEmitter } from "node:events"; diff --git a/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx b/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx index 1aeb7d7e98..5853460884 100644 --- a/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx +++ b/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx @@ -1,5 +1,5 @@ +import { parseApplyPatch } from "../../parse-apply-patch"; import { shortenPath } from "../../utils/short-path"; -import { parseApplyPatch } from "@lib/parse-apply-patch"; import chalk from "chalk"; import { Text } from "ink"; import React from "react"; diff --git a/codex-cli/src/components/chat/terminal-chat.tsx b/codex-cli/src/components/chat/terminal-chat.tsx index bbc9bb052a..35fecec564 100644 --- a/codex-cli/src/components/chat/terminal-chat.tsx +++ b/codex-cli/src/components/chat/terminal-chat.tsx @@ -1,6 +1,6 @@ +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { CommandConfirmation } from "../../utils/agent/agent-loop.js"; import type { AppConfig } from "../../utils/config.js"; -import type { ApplyPatchCommand, ApprovalPolicy } from "@lib/approvals.js"; import type { ColorName } from "chalk"; import type { ResponseItem } from "openai/resources/responses/responses.mjs"; import type { ReviewDecision } from "src/utils/agent/review.ts"; @@ -12,6 +12,7 @@ import { uniqueById, } from "./terminal-chat-utils.js"; import TerminalMessageHistory from "./terminal-message-history.js"; +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"; @@ -25,7 +26,6 @@ import ApprovalModeOverlay from "../approval-mode-overlay.js"; import HelpOverlay from "../help-overlay.js"; import HistoryOverlay from "../history-overlay.js"; import ModelOverlay from "../model-overlay.js"; -import { formatCommandForDisplay } from "@lib/format-command.js"; import { Box, Text } from "ink"; import React, { useEffect, useMemo, useState } from "react"; import { inspect } from "util"; diff --git a/codex-cli/src/lib/format-command.ts b/codex-cli/src/format-command.ts similarity index 100% rename from codex-cli/src/lib/format-command.ts rename to codex-cli/src/format-command.ts diff --git a/codex-cli/src/lib/parse-apply-patch.ts b/codex-cli/src/parse-apply-patch.ts similarity index 100% rename from codex-cli/src/lib/parse-apply-patch.ts rename to codex-cli/src/parse-apply-patch.ts diff --git a/codex-cli/src/lib/text-buffer.ts b/codex-cli/src/text-buffer.ts similarity index 100% rename from codex-cli/src/lib/text-buffer.ts rename to codex-cli/src/text-buffer.ts diff --git a/codex-cli/src/utils/agent/agent-loop.ts b/codex-cli/src/utils/agent/agent-loop.ts index 65127fd85c..d0db6f125d 100644 --- a/codex-cli/src/utils/agent/agent-loop.ts +++ b/codex-cli/src/utils/agent/agent-loop.ts @@ -1,6 +1,6 @@ import type { ReviewDecision } from "./review.js"; +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { AppConfig } from "../config.js"; -import type { ApplyPatchCommand, ApprovalPolicy } from "@lib/approvals.js"; import type { ResponseFunctionToolCall, ResponseInputItem, diff --git a/codex-cli/src/utils/agent/exec.ts b/codex-cli/src/utils/agent/exec.ts index aade68a860..a441f192f8 100644 --- a/codex-cli/src/utils/agent/exec.ts +++ b/codex-cli/src/utils/agent/exec.ts @@ -5,7 +5,7 @@ import { process_patch } from "./apply-patch.js"; import { SandboxType } from "./sandbox/interface.js"; import { execWithSeatbelt } from "./sandbox/macos-seatbelt.js"; import { exec as rawExec } from "./sandbox/raw-exec.js"; -import { formatCommandForDisplay } from "@lib/format-command.js"; +import { formatCommandForDisplay } from "../../format-command.js"; import fs from "fs"; import os from "os"; diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts index 5eaa11d660..41b7abbcd4 100644 --- a/codex-cli/src/utils/agent/handle-exec-command.ts +++ b/codex-cli/src/utils/agent/handle-exec-command.ts @@ -1,7 +1,7 @@ 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 "@lib/approvals.js"; +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { ResponseInputItem } from "openai/resources/responses/responses.mjs"; import { exec, execApplyPatch } from "./exec.js"; @@ -9,8 +9,8 @@ import { isLoggingEnabled, log } from "./log.js"; import { ReviewDecision } from "./review.js"; import { FullAutoErrorMode } from "../auto-approval-mode.js"; import { SandboxType } from "./sandbox/interface.js"; -import { canAutoApprove } from "@lib/approvals.js"; -import { formatCommandForDisplay } from "@lib/format-command.js"; +import { canAutoApprove } from "../../approvals.js"; +import { formatCommandForDisplay } from "../../format-command.js"; import { access } from "fs/promises"; // --------------------------------------------------------------------------- diff --git a/codex-cli/src/utils/agent/review.ts b/codex-cli/src/utils/agent/review.ts index ed2af0ce17..a370388569 100644 --- a/codex-cli/src/utils/agent/review.ts +++ b/codex-cli/src/utils/agent/review.ts @@ -1,4 +1,4 @@ -import type { SafeCommandReason } from "@lib/approvals"; +import type { SafeCommandReason } from "../../approvals"; export type CommandReviewDetails = { cmd: Array; diff --git a/codex-cli/src/utils/parsers.ts b/codex-cli/src/utils/parsers.ts index cb477c1d7d..815e7b2f00 100644 --- a/codex-cli/src/utils/parsers.ts +++ b/codex-cli/src/utils/parsers.ts @@ -3,11 +3,13 @@ import type { ExecInput, ExecOutputMetadata, } from "./agent/sandbox/interface.js"; -import type { SafeCommandReason } from "@lib/approvals.js"; import type { ResponseFunctionToolCall } from "openai/resources/responses/responses.mjs"; +import { isSafeCommand, type SafeCommandReason } from "../approvals.js"; import { log } from "node:console"; import process from "process"; +import { parse } from "shell-quote"; +import { formatCommandForDisplay } from "src/format-command.js"; // The console utility import is intentionally explicit to avoid bundlers from // including the entire `console` module when only the `log` function is @@ -23,52 +25,6 @@ const SAFE_SHELL_OPERATORS: ReadonlySet = new Set([ ";", ]); -// Lazily resolve heavy dependencies at runtime to avoid test environments -// (which might not have the @lib alias configured) from failing at import -// time. If the modules cannot be loaded we fall back to permissive stub -// implementations so that basic functionality – like unit‑testing small UI -// helpers – continues to work without the full codex‑lib dependency tree. - -let isSafeCommand: (cmd: Array) => SafeCommandReason | null = () => - null; -let shellQuoteParse: - | ((cmd: string, env?: Record) => Array) - | undefined; -let formatCommandForDisplay: (cmd: Array) => string = (cmd) => - cmd.join(" "); - -async function loadLibs(): Promise { - try { - const approvals = await import("@lib/approvals.js"); - if (typeof approvals.isSafeCommand === "function") { - isSafeCommand = approvals.isSafeCommand; - } - } catch { - // ignore – keep stub - } - try { - const fmt = await import("@lib/format-command.js"); - if (typeof fmt.formatCommandForDisplay === "function") { - formatCommandForDisplay = fmt.formatCommandForDisplay; - } - } catch { - // ignore – keep stub - } - try { - const sq = await import("shell-quote"); - if (typeof sq.parse === "function") { - shellQuoteParse = sq.parse as typeof shellQuoteParse; - } - } catch { - // ignore – keep stub - } -} - -// Trigger the dynamic import in the background; callers that need the real -// implementation should await the returned promise (parsers currently does not -// require this for correctness during tests). -void loadLibs(); - export function parseToolCallOutput(toolCallOutput: string): { output: string; metadata: ExecOutputMetadata; @@ -175,10 +131,9 @@ function computeAutoApproval(cmd: Array): SafeCommandReason | null { cmd.length === 3 && cmd[0] === "bash" && cmd[1] === "-lc" && - typeof cmd[2] === "string" && - shellQuoteParse + typeof cmd[2] === "string" ) { - const parsed = shellQuoteParse(cmd[2], process.env ?? {}); + const parsed = parse(cmd[2], process.env ?? {}); if (parsed.length === 0) { return null; } diff --git a/codex-cli/tests/agent-cancel-early.test.ts b/codex-cli/tests/agent-cancel-early.test.ts index b7b5fdcae7..b235a6d6bc 100644 --- a/codex-cli/tests/agent-cancel-early.test.ts +++ b/codex-cli/tests/agent-cancel-early.test.ts @@ -64,13 +64,13 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-cancel-prev-response.test.ts b/codex-cli/tests/agent-cancel-prev-response.test.ts index 4047f88452..fe73c338cc 100644 --- a/codex-cli/tests/agent-cancel-prev-response.test.ts +++ b/codex-cli/tests/agent-cancel-prev-response.test.ts @@ -71,13 +71,13 @@ vi.mock("openai", () => { }); // Stub helpers not relevant for this test. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-cancel-race.test.ts b/codex-cli/tests/agent-cancel-race.test.ts index c9c1845d8e..89e7cca744 100644 --- a/codex-cli/tests/agent-cancel-race.test.ts +++ b/codex-cli/tests/agent-cancel-race.test.ts @@ -67,11 +67,11 @@ vi.mock("openai", () => { }); // Stubs for external helpers referenced indirectly. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-cancel.test.ts b/codex-cli/tests/agent-cancel.test.ts index 93ff5ba317..69c17f7f7e 100644 --- a/codex-cli/tests/agent-cancel.test.ts +++ b/codex-cli/tests/agent-cancel.test.ts @@ -47,7 +47,7 @@ vi.mock("openai", () => { }); // Mock the approvals and formatCommand helpers referenced by handle‑exec‑command. -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, alwaysApprovedCommands: new Set(), @@ -57,7 +57,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/agent-function-call-id.test.ts b/codex-cli/tests/agent-function-call-id.test.ts index 7d0a55d122..d50c08eea4 100644 --- a/codex-cli/tests/agent-function-call-id.test.ts +++ b/codex-cli/tests/agent-function-call-id.test.ts @@ -88,14 +88,14 @@ vi.mock("openai", () => { }); // Stub approvals & command formatting – not relevant for this test. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-generic-network-error.test.ts b/codex-cli/tests/agent-generic-network-error.test.ts index 8c636fbda3..942adff668 100644 --- a/codex-cli/tests/agent-generic-network-error.test.ts +++ b/codex-cli/tests/agent-generic-network-error.test.ts @@ -23,14 +23,14 @@ vi.mock("openai", () => { }); // Stub approvals / formatting helpers – unrelated to network handling. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-invalid-request-error.test.ts b/codex-cli/tests/agent-invalid-request-error.test.ts index 631d451944..090d0b52d9 100644 --- a/codex-cli/tests/agent-invalid-request-error.test.ts +++ b/codex-cli/tests/agent-invalid-request-error.test.ts @@ -22,14 +22,14 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-max-tokens-error.test.ts b/codex-cli/tests/agent-max-tokens-error.test.ts index 5e01229454..de4fd17026 100644 --- a/codex-cli/tests/agent-max-tokens-error.test.ts +++ b/codex-cli/tests/agent-max-tokens-error.test.ts @@ -22,14 +22,14 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-network-errors.test.ts b/codex-cli/tests/agent-network-errors.test.ts index e01b08e918..f98ea5bf19 100644 --- a/codex-cli/tests/agent-network-errors.test.ts +++ b/codex-cli/tests/agent-network-errors.test.ts @@ -42,14 +42,14 @@ vi.mock("openai", () => { }); // Stub approvals / formatting helpers – not relevant here. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-project-doc.test.ts b/codex-cli/tests/agent-project-doc.test.ts index 97ad9c8c34..d3050f3953 100644 --- a/codex-cli/tests/agent-project-doc.test.ts +++ b/codex-cli/tests/agent-project-doc.test.ts @@ -51,7 +51,7 @@ vi.mock("openai", () => { // The AgentLoop pulls these helpers in order to decide whether a command can // be auto‑approved. None of that matters for this test, so we stub the module // with minimal no‑op implementations. -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, alwaysApprovedCommands: new Set(), @@ -61,7 +61,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/agent-rate-limit-error.test.ts b/codex-cli/tests/agent-rate-limit-error.test.ts index 318b52f348..abe91171c4 100644 --- a/codex-cli/tests/agent-rate-limit-error.test.ts +++ b/codex-cli/tests/agent-rate-limit-error.test.ts @@ -52,14 +52,14 @@ vi.mock("openai", () => { }); // Stub approvals / formatting helpers – not relevant to rate‑limit handling. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-server-retry.test.ts b/codex-cli/tests/agent-server-retry.test.ts index df98e69e95..e851dd9778 100644 --- a/codex-cli/tests/agent-server-retry.test.ts +++ b/codex-cli/tests/agent-server-retry.test.ts @@ -32,14 +32,14 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-terminate.test.ts b/codex-cli/tests/agent-terminate.test.ts index 7a60456518..bce77437af 100644 --- a/codex-cli/tests/agent-terminate.test.ts +++ b/codex-cli/tests/agent-terminate.test.ts @@ -49,7 +49,7 @@ vi.mock("openai", () => { // --- Helpers referenced by handle‑exec‑command ----------------------------- -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, alwaysApprovedCommands: new Set(), @@ -59,7 +59,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/agent-thinking-time.test.ts b/codex-cli/tests/agent-thinking-time.test.ts index c94d8a5e5e..7132070084 100644 --- a/codex-cli/tests/agent-thinking-time.test.ts +++ b/codex-cli/tests/agent-thinking-time.test.ts @@ -74,12 +74,12 @@ vi.mock("openai", () => { }); // Stub helpers referenced indirectly so we do not pull in real FS/network -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/approvals.test.ts b/codex-cli/tests/approvals.test.ts index e06db94092..7cb0bd3d3e 100644 --- a/codex-cli/tests/approvals.test.ts +++ b/codex-cli/tests/approvals.test.ts @@ -1,6 +1,6 @@ -import type { SafetyAssessment } from "../src/lib/approvals"; +import type { SafetyAssessment } from "../src/approvals"; -import { canAutoApprove } from "../src/lib/approvals"; +import { canAutoApprove } from "../src/approvals"; import { describe, test, expect } from "vitest"; describe("canAutoApprove()", () => { diff --git a/codex-cli/tests/external-editor.test.ts b/codex-cli/tests/external-editor.test.ts index d530be5ecc..77041c2870 100644 --- a/codex-cli/tests/external-editor.test.ts +++ b/codex-cli/tests/external-editor.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer"; +import TextBuffer from "../src/text-buffer"; import { describe, it, expect, vi } from "vitest"; /* ------------------------------------------------------------------------- diff --git a/codex-cli/tests/format-command.test.ts b/codex-cli/tests/format-command.test.ts index 3981d9b106..7de417308a 100644 --- a/codex-cli/tests/format-command.test.ts +++ b/codex-cli/tests/format-command.test.ts @@ -1,4 +1,4 @@ -import { formatCommandForDisplay } from "../src/lib/format-command"; +import { formatCommandForDisplay } from "../src/format-command"; import { describe, test, expect } from "vitest"; describe("formatCommandForDisplay()", () => { diff --git a/codex-cli/tests/invalid-command-handling.test.ts b/codex-cli/tests/invalid-command-handling.test.ts index 6619b4f240..a3f87a7251 100644 --- a/codex-cli/tests/invalid-command-handling.test.ts +++ b/codex-cli/tests/invalid-command-handling.test.ts @@ -22,7 +22,7 @@ describe("rawExec – invalid command handling", () => { // --------------------------------------------------------------------------- // Mock approvals and logging helpers so the test focuses on execution flow. -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, canAutoApprove: () => @@ -31,7 +31,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/multiline-external-editor-shortcut.test.tsx b/codex-cli/tests/multiline-external-editor-shortcut.test.tsx index 158a5e655d..9b2e2f25e5 100644 --- a/codex-cli/tests/multiline-external-editor-shortcut.test.tsx +++ b/codex-cli/tests/multiline-external-editor-shortcut.test.tsx @@ -1,6 +1,6 @@ import { renderTui } from "./ui-test-helpers.js"; import MultilineTextEditor from "../src/components/chat/multiline-editor.js"; -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import * as React from "react"; import { describe, it, expect, vi } from "vitest"; diff --git a/codex-cli/tests/multiline-history-behavior.test.tsx b/codex-cli/tests/multiline-history-behavior.test.tsx index 5c906837ee..cada52ddab 100644 --- a/codex-cli/tests/multiline-history-behavior.test.tsx +++ b/codex-cli/tests/multiline-history-behavior.test.tsx @@ -34,12 +34,12 @@ vi.mock("../src/utils/input-utils.js", () => ({ })), })); -// Mock the optional @lib/* dependencies so the dynamic import in parsers.ts +// Mock the optional ../src/* dependencies so the dynamic import in parsers.ts // does not fail during the test environment where the alias isn't configured. -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ formatCommandForDisplay: (cmd: Array) => cmd.join(" "), })); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ isSafeCommand: (_cmd: Array) => null, })); diff --git a/codex-cli/tests/parse-apply-patch.test.ts b/codex-cli/tests/parse-apply-patch.test.ts index 0195542e56..53aa119093 100644 --- a/codex-cli/tests/parse-apply-patch.test.ts +++ b/codex-cli/tests/parse-apply-patch.test.ts @@ -1,4 +1,4 @@ -import { parseApplyPatch } from "../src/lib/parse-apply-patch"; +import { parseApplyPatch } from "../src/parse-apply-patch"; import { expect, test, describe } from "vitest"; // Helper function to unwrap a non‑null result in tests that expect success. diff --git a/codex-cli/tests/text-buffer-copy-paste.test.ts b/codex-cli/tests/text-buffer-copy-paste.test.ts index 311b2b9aa6..cc1fd119e5 100644 --- a/codex-cli/tests/text-buffer-copy-paste.test.ts +++ b/codex-cli/tests/text-buffer-copy-paste.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import { describe, it, expect } from "vitest"; // These tests ensure that the TextBuffer copy‑&‑paste logic keeps parity with diff --git a/codex-cli/tests/text-buffer-crlf.test.ts b/codex-cli/tests/text-buffer-crlf.test.ts index 4b33b498b5..736c22a27d 100644 --- a/codex-cli/tests/text-buffer-crlf.test.ts +++ b/codex-cli/tests/text-buffer-crlf.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import { describe, it, expect } from "vitest"; describe("TextBuffer – newline normalisation", () => { diff --git a/codex-cli/tests/text-buffer-gaps.test.ts b/codex-cli/tests/text-buffer-gaps.test.ts index 986ad37eed..046d468e8a 100644 --- a/codex-cli/tests/text-buffer-gaps.test.ts +++ b/codex-cli/tests/text-buffer-gaps.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer"; +import TextBuffer from "../src/text-buffer"; import { describe, it, expect } from "vitest"; // The purpose of this test‑suite is NOT to make the implementation green today diff --git a/codex-cli/tests/text-buffer-word.test.ts b/codex-cli/tests/text-buffer-word.test.ts index 009786a28c..4ea7679450 100644 --- a/codex-cli/tests/text-buffer-word.test.ts +++ b/codex-cli/tests/text-buffer-word.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import { describe, test, expect } from "vitest"; describe("TextBuffer – word‑wise navigation & deletion", () => { diff --git a/codex-cli/tests/text-buffer.test.ts b/codex-cli/tests/text-buffer.test.ts index ae78f29409..c3f33d0fa1 100644 --- a/codex-cli/tests/text-buffer.test.ts +++ b/codex-cli/tests/text-buffer.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer"; +import TextBuffer from "../src/text-buffer"; import { describe, it, expect } from "vitest"; describe("TextBuffer – basic editing parity with Rust suite", () => { diff --git a/codex-cli/tsconfig.json b/codex-cli/tsconfig.json index 626fc5dbf4..d1dacc9149 100644 --- a/codex-cli/tsconfig.json +++ b/codex-cli/tsconfig.json @@ -11,9 +11,6 @@ ], "types": ["node"], "baseUrl": "./", - "paths": { - "@lib/*": ["./src/lib/*"] - }, "resolveJsonModule": false, // ESM doesn't yet support JSON modules. "jsx": "react", "declaration": true, From 66904fc9256815c2603f4c06e772f3f978b1b730 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Apr 2025 13:51:46 -0700 Subject: [PATCH 02/14] Back out @lib indirection in tsconfig.json --- codex-cli/src/app.tsx | 2 +- codex-cli/src/{lib => }/approvals.ts | 2 +- codex-cli/src/cli.tsx | 4 +- .../src/components/chat/multiline-editor.tsx | 2 +- .../chat/terminal-chat-tool-call-item.tsx | 2 +- .../src/components/chat/terminal-chat.tsx | 4 +- codex-cli/src/{lib => }/format-command.ts | 0 codex-cli/src/{lib => }/parse-apply-patch.ts | 0 codex-cli/src/{lib => }/text-buffer.ts | 0 codex-cli/src/utils/agent/agent-loop.ts | 2 +- codex-cli/src/utils/agent/exec.ts | 2 +- .../src/utils/agent/handle-exec-command.ts | 6 +- codex-cli/src/utils/agent/review.ts | 2 +- codex-cli/src/utils/parsers.ts | 55 ++----------------- codex-cli/tests/agent-cancel-early.test.ts | 4 +- .../tests/agent-cancel-prev-response.test.ts | 4 +- codex-cli/tests/agent-cancel-race.test.ts | 4 +- codex-cli/tests/agent-cancel.test.ts | 4 +- .../tests/agent-function-call-id.test.ts | 4 +- .../tests/agent-generic-network-error.test.ts | 4 +- .../tests/agent-invalid-request-error.test.ts | 4 +- .../tests/agent-max-tokens-error.test.ts | 4 +- codex-cli/tests/agent-network-errors.test.ts | 4 +- codex-cli/tests/agent-project-doc.test.ts | 4 +- .../tests/agent-rate-limit-error.test.ts | 4 +- codex-cli/tests/agent-server-retry.test.ts | 4 +- codex-cli/tests/agent-terminate.test.ts | 4 +- codex-cli/tests/agent-thinking-time.test.ts | 4 +- codex-cli/tests/approvals.test.ts | 4 +- codex-cli/tests/external-editor.test.ts | 2 +- codex-cli/tests/format-command.test.ts | 2 +- .../tests/invalid-command-handling.test.ts | 4 +- ...ultiline-external-editor-shortcut.test.tsx | 2 +- .../tests/multiline-history-behavior.test.tsx | 6 +- codex-cli/tests/parse-apply-patch.test.ts | 2 +- .../tests/text-buffer-copy-paste.test.ts | 2 +- codex-cli/tests/text-buffer-crlf.test.ts | 2 +- codex-cli/tests/text-buffer-gaps.test.ts | 2 +- codex-cli/tests/text-buffer-word.test.ts | 2 +- codex-cli/tests/text-buffer.test.ts | 2 +- codex-cli/tsconfig.json | 3 - 41 files changed, 63 insertions(+), 111 deletions(-) rename codex-cli/src/{lib => }/approvals.ts (99%) rename codex-cli/src/{lib => }/format-command.ts (100%) rename codex-cli/src/{lib => }/parse-apply-patch.ts (100%) rename codex-cli/src/{lib => }/text-buffer.ts (100%) diff --git a/codex-cli/src/app.tsx b/codex-cli/src/app.tsx index dbb0cdedc8..c0b8c6f4e3 100644 --- a/codex-cli/src/app.tsx +++ b/codex-cli/src/app.tsx @@ -1,5 +1,5 @@ +import type { ApprovalPolicy } from "./approvals"; import type { AppConfig } from "./utils/config"; -import type { ApprovalPolicy } from "@lib/approvals"; import type { ResponseItem } from "openai/resources/responses/responses"; import TerminalChat from "./components/chat/terminal-chat"; diff --git a/codex-cli/src/lib/approvals.ts b/codex-cli/src/approvals.ts similarity index 99% rename from codex-cli/src/lib/approvals.ts rename to codex-cli/src/approvals.ts index 8985939a43..0cf3703b54 100644 --- a/codex-cli/src/lib/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -3,7 +3,7 @@ import type { ParseEntry, ControlOperator } from "shell-quote"; import { identify_files_added, identify_files_needed, -} from "../utils/agent/apply-patch"; +} from "./utils/agent/apply-patch"; import * as path from "path"; import { parse } from "shell-quote"; diff --git a/codex-cli/src/cli.tsx b/codex-cli/src/cli.tsx index 5e43dff1f3..0af421a431 100644 --- a/codex-cli/src/cli.tsx +++ b/codex-cli/src/cli.tsx @@ -1,9 +1,9 @@ #!/usr/bin/env node import type { AppRollout } from "./app"; +import type { ApprovalPolicy } from "./approvals"; import type { CommandConfirmation } from "./utils/agent/agent-loop"; import type { AppConfig } from "./utils/config"; -import type { ApprovalPolicy } from "@lib/approvals"; import type { ResponseItem } from "openai/resources/responses/responses"; import App from "./app"; @@ -124,7 +124,7 @@ const cli = meow( fullContext: { type: "boolean", aliases: ["f"], - description: `Run in full-context editing approach. The model is given the whole code + description: `Run in full-context editing approach. The model is given the whole code directory as context and performs changes in one go without acting.`, }, }, diff --git a/codex-cli/src/components/chat/multiline-editor.tsx b/codex-cli/src/components/chat/multiline-editor.tsx index c18555447e..c99961bbcd 100644 --- a/codex-cli/src/components/chat/multiline-editor.tsx +++ b/codex-cli/src/components/chat/multiline-editor.tsx @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { useTerminalSize } from "../../hooks/use-terminal-size"; -import TextBuffer from "../../lib/text-buffer.js"; +import TextBuffer from "../../text-buffer.js"; import chalk from "chalk"; import { Box, Text, useInput, useStdin } from "ink"; import { EventEmitter } from "node:events"; diff --git a/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx b/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx index 1aeb7d7e98..5853460884 100644 --- a/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx +++ b/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx @@ -1,5 +1,5 @@ +import { parseApplyPatch } from "../../parse-apply-patch"; import { shortenPath } from "../../utils/short-path"; -import { parseApplyPatch } from "@lib/parse-apply-patch"; import chalk from "chalk"; import { Text } from "ink"; import React from "react"; diff --git a/codex-cli/src/components/chat/terminal-chat.tsx b/codex-cli/src/components/chat/terminal-chat.tsx index bbc9bb052a..35fecec564 100644 --- a/codex-cli/src/components/chat/terminal-chat.tsx +++ b/codex-cli/src/components/chat/terminal-chat.tsx @@ -1,6 +1,6 @@ +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { CommandConfirmation } from "../../utils/agent/agent-loop.js"; import type { AppConfig } from "../../utils/config.js"; -import type { ApplyPatchCommand, ApprovalPolicy } from "@lib/approvals.js"; import type { ColorName } from "chalk"; import type { ResponseItem } from "openai/resources/responses/responses.mjs"; import type { ReviewDecision } from "src/utils/agent/review.ts"; @@ -12,6 +12,7 @@ import { uniqueById, } from "./terminal-chat-utils.js"; import TerminalMessageHistory from "./terminal-message-history.js"; +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"; @@ -25,7 +26,6 @@ import ApprovalModeOverlay from "../approval-mode-overlay.js"; import HelpOverlay from "../help-overlay.js"; import HistoryOverlay from "../history-overlay.js"; import ModelOverlay from "../model-overlay.js"; -import { formatCommandForDisplay } from "@lib/format-command.js"; import { Box, Text } from "ink"; import React, { useEffect, useMemo, useState } from "react"; import { inspect } from "util"; diff --git a/codex-cli/src/lib/format-command.ts b/codex-cli/src/format-command.ts similarity index 100% rename from codex-cli/src/lib/format-command.ts rename to codex-cli/src/format-command.ts diff --git a/codex-cli/src/lib/parse-apply-patch.ts b/codex-cli/src/parse-apply-patch.ts similarity index 100% rename from codex-cli/src/lib/parse-apply-patch.ts rename to codex-cli/src/parse-apply-patch.ts diff --git a/codex-cli/src/lib/text-buffer.ts b/codex-cli/src/text-buffer.ts similarity index 100% rename from codex-cli/src/lib/text-buffer.ts rename to codex-cli/src/text-buffer.ts diff --git a/codex-cli/src/utils/agent/agent-loop.ts b/codex-cli/src/utils/agent/agent-loop.ts index e2b465733d..d88604c471 100644 --- a/codex-cli/src/utils/agent/agent-loop.ts +++ b/codex-cli/src/utils/agent/agent-loop.ts @@ -1,6 +1,6 @@ import type { ReviewDecision } from "./review.js"; +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { AppConfig } from "../config.js"; -import type { ApplyPatchCommand, ApprovalPolicy } from "@lib/approvals.js"; import type { ResponseFunctionToolCall, ResponseInputItem, diff --git a/codex-cli/src/utils/agent/exec.ts b/codex-cli/src/utils/agent/exec.ts index aade68a860..a441f192f8 100644 --- a/codex-cli/src/utils/agent/exec.ts +++ b/codex-cli/src/utils/agent/exec.ts @@ -5,7 +5,7 @@ import { process_patch } from "./apply-patch.js"; import { SandboxType } from "./sandbox/interface.js"; import { execWithSeatbelt } from "./sandbox/macos-seatbelt.js"; import { exec as rawExec } from "./sandbox/raw-exec.js"; -import { formatCommandForDisplay } from "@lib/format-command.js"; +import { formatCommandForDisplay } from "../../format-command.js"; import fs from "fs"; import os from "os"; diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts index 5eaa11d660..41b7abbcd4 100644 --- a/codex-cli/src/utils/agent/handle-exec-command.ts +++ b/codex-cli/src/utils/agent/handle-exec-command.ts @@ -1,7 +1,7 @@ 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 "@lib/approvals.js"; +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { ResponseInputItem } from "openai/resources/responses/responses.mjs"; import { exec, execApplyPatch } from "./exec.js"; @@ -9,8 +9,8 @@ import { isLoggingEnabled, log } from "./log.js"; import { ReviewDecision } from "./review.js"; import { FullAutoErrorMode } from "../auto-approval-mode.js"; import { SandboxType } from "./sandbox/interface.js"; -import { canAutoApprove } from "@lib/approvals.js"; -import { formatCommandForDisplay } from "@lib/format-command.js"; +import { canAutoApprove } from "../../approvals.js"; +import { formatCommandForDisplay } from "../../format-command.js"; import { access } from "fs/promises"; // --------------------------------------------------------------------------- diff --git a/codex-cli/src/utils/agent/review.ts b/codex-cli/src/utils/agent/review.ts index ed2af0ce17..a370388569 100644 --- a/codex-cli/src/utils/agent/review.ts +++ b/codex-cli/src/utils/agent/review.ts @@ -1,4 +1,4 @@ -import type { SafeCommandReason } from "@lib/approvals"; +import type { SafeCommandReason } from "../../approvals"; export type CommandReviewDetails = { cmd: Array; diff --git a/codex-cli/src/utils/parsers.ts b/codex-cli/src/utils/parsers.ts index cb477c1d7d..815e7b2f00 100644 --- a/codex-cli/src/utils/parsers.ts +++ b/codex-cli/src/utils/parsers.ts @@ -3,11 +3,13 @@ import type { ExecInput, ExecOutputMetadata, } from "./agent/sandbox/interface.js"; -import type { SafeCommandReason } from "@lib/approvals.js"; import type { ResponseFunctionToolCall } from "openai/resources/responses/responses.mjs"; +import { isSafeCommand, type SafeCommandReason } from "../approvals.js"; import { log } from "node:console"; import process from "process"; +import { parse } from "shell-quote"; +import { formatCommandForDisplay } from "src/format-command.js"; // The console utility import is intentionally explicit to avoid bundlers from // including the entire `console` module when only the `log` function is @@ -23,52 +25,6 @@ const SAFE_SHELL_OPERATORS: ReadonlySet = new Set([ ";", ]); -// Lazily resolve heavy dependencies at runtime to avoid test environments -// (which might not have the @lib alias configured) from failing at import -// time. If the modules cannot be loaded we fall back to permissive stub -// implementations so that basic functionality – like unit‑testing small UI -// helpers – continues to work without the full codex‑lib dependency tree. - -let isSafeCommand: (cmd: Array) => SafeCommandReason | null = () => - null; -let shellQuoteParse: - | ((cmd: string, env?: Record) => Array) - | undefined; -let formatCommandForDisplay: (cmd: Array) => string = (cmd) => - cmd.join(" "); - -async function loadLibs(): Promise { - try { - const approvals = await import("@lib/approvals.js"); - if (typeof approvals.isSafeCommand === "function") { - isSafeCommand = approvals.isSafeCommand; - } - } catch { - // ignore – keep stub - } - try { - const fmt = await import("@lib/format-command.js"); - if (typeof fmt.formatCommandForDisplay === "function") { - formatCommandForDisplay = fmt.formatCommandForDisplay; - } - } catch { - // ignore – keep stub - } - try { - const sq = await import("shell-quote"); - if (typeof sq.parse === "function") { - shellQuoteParse = sq.parse as typeof shellQuoteParse; - } - } catch { - // ignore – keep stub - } -} - -// Trigger the dynamic import in the background; callers that need the real -// implementation should await the returned promise (parsers currently does not -// require this for correctness during tests). -void loadLibs(); - export function parseToolCallOutput(toolCallOutput: string): { output: string; metadata: ExecOutputMetadata; @@ -175,10 +131,9 @@ function computeAutoApproval(cmd: Array): SafeCommandReason | null { cmd.length === 3 && cmd[0] === "bash" && cmd[1] === "-lc" && - typeof cmd[2] === "string" && - shellQuoteParse + typeof cmd[2] === "string" ) { - const parsed = shellQuoteParse(cmd[2], process.env ?? {}); + const parsed = parse(cmd[2], process.env ?? {}); if (parsed.length === 0) { return null; } diff --git a/codex-cli/tests/agent-cancel-early.test.ts b/codex-cli/tests/agent-cancel-early.test.ts index b7b5fdcae7..b235a6d6bc 100644 --- a/codex-cli/tests/agent-cancel-early.test.ts +++ b/codex-cli/tests/agent-cancel-early.test.ts @@ -64,13 +64,13 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-cancel-prev-response.test.ts b/codex-cli/tests/agent-cancel-prev-response.test.ts index 4047f88452..fe73c338cc 100644 --- a/codex-cli/tests/agent-cancel-prev-response.test.ts +++ b/codex-cli/tests/agent-cancel-prev-response.test.ts @@ -71,13 +71,13 @@ vi.mock("openai", () => { }); // Stub helpers not relevant for this test. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-cancel-race.test.ts b/codex-cli/tests/agent-cancel-race.test.ts index c9c1845d8e..89e7cca744 100644 --- a/codex-cli/tests/agent-cancel-race.test.ts +++ b/codex-cli/tests/agent-cancel-race.test.ts @@ -67,11 +67,11 @@ vi.mock("openai", () => { }); // Stubs for external helpers referenced indirectly. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-cancel.test.ts b/codex-cli/tests/agent-cancel.test.ts index 93ff5ba317..69c17f7f7e 100644 --- a/codex-cli/tests/agent-cancel.test.ts +++ b/codex-cli/tests/agent-cancel.test.ts @@ -47,7 +47,7 @@ vi.mock("openai", () => { }); // Mock the approvals and formatCommand helpers referenced by handle‑exec‑command. -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, alwaysApprovedCommands: new Set(), @@ -57,7 +57,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/agent-function-call-id.test.ts b/codex-cli/tests/agent-function-call-id.test.ts index 7d0a55d122..d50c08eea4 100644 --- a/codex-cli/tests/agent-function-call-id.test.ts +++ b/codex-cli/tests/agent-function-call-id.test.ts @@ -88,14 +88,14 @@ vi.mock("openai", () => { }); // Stub approvals & command formatting – not relevant for this test. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-generic-network-error.test.ts b/codex-cli/tests/agent-generic-network-error.test.ts index 8c636fbda3..942adff668 100644 --- a/codex-cli/tests/agent-generic-network-error.test.ts +++ b/codex-cli/tests/agent-generic-network-error.test.ts @@ -23,14 +23,14 @@ vi.mock("openai", () => { }); // Stub approvals / formatting helpers – unrelated to network handling. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-invalid-request-error.test.ts b/codex-cli/tests/agent-invalid-request-error.test.ts index 631d451944..090d0b52d9 100644 --- a/codex-cli/tests/agent-invalid-request-error.test.ts +++ b/codex-cli/tests/agent-invalid-request-error.test.ts @@ -22,14 +22,14 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-max-tokens-error.test.ts b/codex-cli/tests/agent-max-tokens-error.test.ts index 5e01229454..de4fd17026 100644 --- a/codex-cli/tests/agent-max-tokens-error.test.ts +++ b/codex-cli/tests/agent-max-tokens-error.test.ts @@ -22,14 +22,14 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-network-errors.test.ts b/codex-cli/tests/agent-network-errors.test.ts index e01b08e918..f98ea5bf19 100644 --- a/codex-cli/tests/agent-network-errors.test.ts +++ b/codex-cli/tests/agent-network-errors.test.ts @@ -42,14 +42,14 @@ vi.mock("openai", () => { }); // Stub approvals / formatting helpers – not relevant here. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-project-doc.test.ts b/codex-cli/tests/agent-project-doc.test.ts index 97ad9c8c34..d3050f3953 100644 --- a/codex-cli/tests/agent-project-doc.test.ts +++ b/codex-cli/tests/agent-project-doc.test.ts @@ -51,7 +51,7 @@ vi.mock("openai", () => { // The AgentLoop pulls these helpers in order to decide whether a command can // be auto‑approved. None of that matters for this test, so we stub the module // with minimal no‑op implementations. -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, alwaysApprovedCommands: new Set(), @@ -61,7 +61,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/agent-rate-limit-error.test.ts b/codex-cli/tests/agent-rate-limit-error.test.ts index 18779450aa..9782744679 100644 --- a/codex-cli/tests/agent-rate-limit-error.test.ts +++ b/codex-cli/tests/agent-rate-limit-error.test.ts @@ -34,14 +34,14 @@ vi.mock("openai", () => { // Stub helpers that the agent indirectly imports so it does not attempt any // file‑system access or real approvals logic during the test. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-server-retry.test.ts b/codex-cli/tests/agent-server-retry.test.ts index 9ec4eb5aac..09278f2ceb 100644 --- a/codex-cli/tests/agent-server-retry.test.ts +++ b/codex-cli/tests/agent-server-retry.test.ts @@ -32,14 +32,14 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-terminate.test.ts b/codex-cli/tests/agent-terminate.test.ts index 7a60456518..bce77437af 100644 --- a/codex-cli/tests/agent-terminate.test.ts +++ b/codex-cli/tests/agent-terminate.test.ts @@ -49,7 +49,7 @@ vi.mock("openai", () => { // --- Helpers referenced by handle‑exec‑command ----------------------------- -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, alwaysApprovedCommands: new Set(), @@ -59,7 +59,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/agent-thinking-time.test.ts b/codex-cli/tests/agent-thinking-time.test.ts index c94d8a5e5e..7132070084 100644 --- a/codex-cli/tests/agent-thinking-time.test.ts +++ b/codex-cli/tests/agent-thinking-time.test.ts @@ -74,12 +74,12 @@ vi.mock("openai", () => { }); // Stub helpers referenced indirectly so we do not pull in real FS/network -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/approvals.test.ts b/codex-cli/tests/approvals.test.ts index e06db94092..7cb0bd3d3e 100644 --- a/codex-cli/tests/approvals.test.ts +++ b/codex-cli/tests/approvals.test.ts @@ -1,6 +1,6 @@ -import type { SafetyAssessment } from "../src/lib/approvals"; +import type { SafetyAssessment } from "../src/approvals"; -import { canAutoApprove } from "../src/lib/approvals"; +import { canAutoApprove } from "../src/approvals"; import { describe, test, expect } from "vitest"; describe("canAutoApprove()", () => { diff --git a/codex-cli/tests/external-editor.test.ts b/codex-cli/tests/external-editor.test.ts index d530be5ecc..77041c2870 100644 --- a/codex-cli/tests/external-editor.test.ts +++ b/codex-cli/tests/external-editor.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer"; +import TextBuffer from "../src/text-buffer"; import { describe, it, expect, vi } from "vitest"; /* ------------------------------------------------------------------------- diff --git a/codex-cli/tests/format-command.test.ts b/codex-cli/tests/format-command.test.ts index 3981d9b106..7de417308a 100644 --- a/codex-cli/tests/format-command.test.ts +++ b/codex-cli/tests/format-command.test.ts @@ -1,4 +1,4 @@ -import { formatCommandForDisplay } from "../src/lib/format-command"; +import { formatCommandForDisplay } from "../src/format-command"; import { describe, test, expect } from "vitest"; describe("formatCommandForDisplay()", () => { diff --git a/codex-cli/tests/invalid-command-handling.test.ts b/codex-cli/tests/invalid-command-handling.test.ts index 6619b4f240..a3f87a7251 100644 --- a/codex-cli/tests/invalid-command-handling.test.ts +++ b/codex-cli/tests/invalid-command-handling.test.ts @@ -22,7 +22,7 @@ describe("rawExec – invalid command handling", () => { // --------------------------------------------------------------------------- // Mock approvals and logging helpers so the test focuses on execution flow. -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, canAutoApprove: () => @@ -31,7 +31,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/multiline-external-editor-shortcut.test.tsx b/codex-cli/tests/multiline-external-editor-shortcut.test.tsx index 158a5e655d..9b2e2f25e5 100644 --- a/codex-cli/tests/multiline-external-editor-shortcut.test.tsx +++ b/codex-cli/tests/multiline-external-editor-shortcut.test.tsx @@ -1,6 +1,6 @@ import { renderTui } from "./ui-test-helpers.js"; import MultilineTextEditor from "../src/components/chat/multiline-editor.js"; -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import * as React from "react"; import { describe, it, expect, vi } from "vitest"; diff --git a/codex-cli/tests/multiline-history-behavior.test.tsx b/codex-cli/tests/multiline-history-behavior.test.tsx index 5c906837ee..cada52ddab 100644 --- a/codex-cli/tests/multiline-history-behavior.test.tsx +++ b/codex-cli/tests/multiline-history-behavior.test.tsx @@ -34,12 +34,12 @@ vi.mock("../src/utils/input-utils.js", () => ({ })), })); -// Mock the optional @lib/* dependencies so the dynamic import in parsers.ts +// Mock the optional ../src/* dependencies so the dynamic import in parsers.ts // does not fail during the test environment where the alias isn't configured. -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ formatCommandForDisplay: (cmd: Array) => cmd.join(" "), })); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ isSafeCommand: (_cmd: Array) => null, })); diff --git a/codex-cli/tests/parse-apply-patch.test.ts b/codex-cli/tests/parse-apply-patch.test.ts index 0195542e56..53aa119093 100644 --- a/codex-cli/tests/parse-apply-patch.test.ts +++ b/codex-cli/tests/parse-apply-patch.test.ts @@ -1,4 +1,4 @@ -import { parseApplyPatch } from "../src/lib/parse-apply-patch"; +import { parseApplyPatch } from "../src/parse-apply-patch"; import { expect, test, describe } from "vitest"; // Helper function to unwrap a non‑null result in tests that expect success. diff --git a/codex-cli/tests/text-buffer-copy-paste.test.ts b/codex-cli/tests/text-buffer-copy-paste.test.ts index 311b2b9aa6..cc1fd119e5 100644 --- a/codex-cli/tests/text-buffer-copy-paste.test.ts +++ b/codex-cli/tests/text-buffer-copy-paste.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import { describe, it, expect } from "vitest"; // These tests ensure that the TextBuffer copy‑&‑paste logic keeps parity with diff --git a/codex-cli/tests/text-buffer-crlf.test.ts b/codex-cli/tests/text-buffer-crlf.test.ts index 4b33b498b5..736c22a27d 100644 --- a/codex-cli/tests/text-buffer-crlf.test.ts +++ b/codex-cli/tests/text-buffer-crlf.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import { describe, it, expect } from "vitest"; describe("TextBuffer – newline normalisation", () => { diff --git a/codex-cli/tests/text-buffer-gaps.test.ts b/codex-cli/tests/text-buffer-gaps.test.ts index 986ad37eed..046d468e8a 100644 --- a/codex-cli/tests/text-buffer-gaps.test.ts +++ b/codex-cli/tests/text-buffer-gaps.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer"; +import TextBuffer from "../src/text-buffer"; import { describe, it, expect } from "vitest"; // The purpose of this test‑suite is NOT to make the implementation green today diff --git a/codex-cli/tests/text-buffer-word.test.ts b/codex-cli/tests/text-buffer-word.test.ts index 009786a28c..4ea7679450 100644 --- a/codex-cli/tests/text-buffer-word.test.ts +++ b/codex-cli/tests/text-buffer-word.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import { describe, test, expect } from "vitest"; describe("TextBuffer – word‑wise navigation & deletion", () => { diff --git a/codex-cli/tests/text-buffer.test.ts b/codex-cli/tests/text-buffer.test.ts index ae78f29409..c3f33d0fa1 100644 --- a/codex-cli/tests/text-buffer.test.ts +++ b/codex-cli/tests/text-buffer.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer"; +import TextBuffer from "../src/text-buffer"; import { describe, it, expect } from "vitest"; describe("TextBuffer – basic editing parity with Rust suite", () => { diff --git a/codex-cli/tsconfig.json b/codex-cli/tsconfig.json index 626fc5dbf4..d1dacc9149 100644 --- a/codex-cli/tsconfig.json +++ b/codex-cli/tsconfig.json @@ -11,9 +11,6 @@ ], "types": ["node"], "baseUrl": "./", - "paths": { - "@lib/*": ["./src/lib/*"] - }, "resolveJsonModule": false, // ESM doesn't yet support JSON modules. "jsx": "react", "declaration": true, From 2cafcf2ec061552d195a762123025d21f0f00097 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Apr 2025 14:15:00 -0700 Subject: [PATCH 03/14] Back out @lib indirection in tsconfig.json --- codex-cli/src/app.tsx | 2 +- codex-cli/src/{lib => }/approvals.ts | 2 +- codex-cli/src/cli.tsx | 4 +- .../src/components/chat/multiline-editor.tsx | 2 +- .../chat/terminal-chat-tool-call-item.tsx | 2 +- .../src/components/chat/terminal-chat.tsx | 4 +- codex-cli/src/{lib => }/format-command.ts | 0 codex-cli/src/{lib => }/parse-apply-patch.ts | 0 codex-cli/src/{lib => }/text-buffer.ts | 0 codex-cli/src/utils/agent/agent-loop.ts | 2 +- codex-cli/src/utils/agent/exec.ts | 2 +- .../src/utils/agent/handle-exec-command.ts | 6 +- codex-cli/src/utils/agent/review.ts | 2 +- codex-cli/src/utils/parsers.ts | 55 ++----------------- codex-cli/tests/agent-cancel-early.test.ts | 4 +- .../tests/agent-cancel-prev-response.test.ts | 4 +- codex-cli/tests/agent-cancel-race.test.ts | 4 +- codex-cli/tests/agent-cancel.test.ts | 4 +- .../tests/agent-function-call-id.test.ts | 4 +- .../tests/agent-generic-network-error.test.ts | 4 +- .../tests/agent-invalid-request-error.test.ts | 4 +- .../tests/agent-max-tokens-error.test.ts | 4 +- codex-cli/tests/agent-network-errors.test.ts | 4 +- codex-cli/tests/agent-project-doc.test.ts | 4 +- .../tests/agent-rate-limit-error.test.ts | 4 +- codex-cli/tests/agent-server-retry.test.ts | 4 +- codex-cli/tests/agent-terminate.test.ts | 4 +- codex-cli/tests/agent-thinking-time.test.ts | 4 +- codex-cli/tests/approvals.test.ts | 4 +- codex-cli/tests/external-editor.test.ts | 2 +- codex-cli/tests/format-command.test.ts | 2 +- .../tests/invalid-command-handling.test.ts | 4 +- ...ultiline-external-editor-shortcut.test.tsx | 2 +- .../tests/multiline-history-behavior.test.tsx | 6 +- codex-cli/tests/parse-apply-patch.test.ts | 2 +- .../tests/text-buffer-copy-paste.test.ts | 2 +- codex-cli/tests/text-buffer-crlf.test.ts | 2 +- codex-cli/tests/text-buffer-gaps.test.ts | 2 +- codex-cli/tests/text-buffer-word.test.ts | 2 +- codex-cli/tests/text-buffer.test.ts | 2 +- codex-cli/tsconfig.json | 3 - 41 files changed, 63 insertions(+), 111 deletions(-) rename codex-cli/src/{lib => }/approvals.ts (99%) rename codex-cli/src/{lib => }/format-command.ts (100%) rename codex-cli/src/{lib => }/parse-apply-patch.ts (100%) rename codex-cli/src/{lib => }/text-buffer.ts (100%) diff --git a/codex-cli/src/app.tsx b/codex-cli/src/app.tsx index dbb0cdedc8..c0b8c6f4e3 100644 --- a/codex-cli/src/app.tsx +++ b/codex-cli/src/app.tsx @@ -1,5 +1,5 @@ +import type { ApprovalPolicy } from "./approvals"; import type { AppConfig } from "./utils/config"; -import type { ApprovalPolicy } from "@lib/approvals"; import type { ResponseItem } from "openai/resources/responses/responses"; import TerminalChat from "./components/chat/terminal-chat"; diff --git a/codex-cli/src/lib/approvals.ts b/codex-cli/src/approvals.ts similarity index 99% rename from codex-cli/src/lib/approvals.ts rename to codex-cli/src/approvals.ts index 8985939a43..0cf3703b54 100644 --- a/codex-cli/src/lib/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -3,7 +3,7 @@ import type { ParseEntry, ControlOperator } from "shell-quote"; import { identify_files_added, identify_files_needed, -} from "../utils/agent/apply-patch"; +} from "./utils/agent/apply-patch"; import * as path from "path"; import { parse } from "shell-quote"; diff --git a/codex-cli/src/cli.tsx b/codex-cli/src/cli.tsx index 5e43dff1f3..0af421a431 100644 --- a/codex-cli/src/cli.tsx +++ b/codex-cli/src/cli.tsx @@ -1,9 +1,9 @@ #!/usr/bin/env node import type { AppRollout } from "./app"; +import type { ApprovalPolicy } from "./approvals"; import type { CommandConfirmation } from "./utils/agent/agent-loop"; import type { AppConfig } from "./utils/config"; -import type { ApprovalPolicy } from "@lib/approvals"; import type { ResponseItem } from "openai/resources/responses/responses"; import App from "./app"; @@ -124,7 +124,7 @@ const cli = meow( fullContext: { type: "boolean", aliases: ["f"], - description: `Run in full-context editing approach. The model is given the whole code + description: `Run in full-context editing approach. The model is given the whole code directory as context and performs changes in one go without acting.`, }, }, diff --git a/codex-cli/src/components/chat/multiline-editor.tsx b/codex-cli/src/components/chat/multiline-editor.tsx index c18555447e..c99961bbcd 100644 --- a/codex-cli/src/components/chat/multiline-editor.tsx +++ b/codex-cli/src/components/chat/multiline-editor.tsx @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { useTerminalSize } from "../../hooks/use-terminal-size"; -import TextBuffer from "../../lib/text-buffer.js"; +import TextBuffer from "../../text-buffer.js"; import chalk from "chalk"; import { Box, Text, useInput, useStdin } from "ink"; import { EventEmitter } from "node:events"; diff --git a/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx b/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx index 1aeb7d7e98..5853460884 100644 --- a/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx +++ b/codex-cli/src/components/chat/terminal-chat-tool-call-item.tsx @@ -1,5 +1,5 @@ +import { parseApplyPatch } from "../../parse-apply-patch"; import { shortenPath } from "../../utils/short-path"; -import { parseApplyPatch } from "@lib/parse-apply-patch"; import chalk from "chalk"; import { Text } from "ink"; import React from "react"; diff --git a/codex-cli/src/components/chat/terminal-chat.tsx b/codex-cli/src/components/chat/terminal-chat.tsx index bbc9bb052a..35fecec564 100644 --- a/codex-cli/src/components/chat/terminal-chat.tsx +++ b/codex-cli/src/components/chat/terminal-chat.tsx @@ -1,6 +1,6 @@ +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { CommandConfirmation } from "../../utils/agent/agent-loop.js"; import type { AppConfig } from "../../utils/config.js"; -import type { ApplyPatchCommand, ApprovalPolicy } from "@lib/approvals.js"; import type { ColorName } from "chalk"; import type { ResponseItem } from "openai/resources/responses/responses.mjs"; import type { ReviewDecision } from "src/utils/agent/review.ts"; @@ -12,6 +12,7 @@ import { uniqueById, } from "./terminal-chat-utils.js"; import TerminalMessageHistory from "./terminal-message-history.js"; +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"; @@ -25,7 +26,6 @@ import ApprovalModeOverlay from "../approval-mode-overlay.js"; import HelpOverlay from "../help-overlay.js"; import HistoryOverlay from "../history-overlay.js"; import ModelOverlay from "../model-overlay.js"; -import { formatCommandForDisplay } from "@lib/format-command.js"; import { Box, Text } from "ink"; import React, { useEffect, useMemo, useState } from "react"; import { inspect } from "util"; diff --git a/codex-cli/src/lib/format-command.ts b/codex-cli/src/format-command.ts similarity index 100% rename from codex-cli/src/lib/format-command.ts rename to codex-cli/src/format-command.ts diff --git a/codex-cli/src/lib/parse-apply-patch.ts b/codex-cli/src/parse-apply-patch.ts similarity index 100% rename from codex-cli/src/lib/parse-apply-patch.ts rename to codex-cli/src/parse-apply-patch.ts diff --git a/codex-cli/src/lib/text-buffer.ts b/codex-cli/src/text-buffer.ts similarity index 100% rename from codex-cli/src/lib/text-buffer.ts rename to codex-cli/src/text-buffer.ts diff --git a/codex-cli/src/utils/agent/agent-loop.ts b/codex-cli/src/utils/agent/agent-loop.ts index e2b465733d..d88604c471 100644 --- a/codex-cli/src/utils/agent/agent-loop.ts +++ b/codex-cli/src/utils/agent/agent-loop.ts @@ -1,6 +1,6 @@ import type { ReviewDecision } from "./review.js"; +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { AppConfig } from "../config.js"; -import type { ApplyPatchCommand, ApprovalPolicy } from "@lib/approvals.js"; import type { ResponseFunctionToolCall, ResponseInputItem, diff --git a/codex-cli/src/utils/agent/exec.ts b/codex-cli/src/utils/agent/exec.ts index aade68a860..a441f192f8 100644 --- a/codex-cli/src/utils/agent/exec.ts +++ b/codex-cli/src/utils/agent/exec.ts @@ -5,7 +5,7 @@ import { process_patch } from "./apply-patch.js"; import { SandboxType } from "./sandbox/interface.js"; import { execWithSeatbelt } from "./sandbox/macos-seatbelt.js"; import { exec as rawExec } from "./sandbox/raw-exec.js"; -import { formatCommandForDisplay } from "@lib/format-command.js"; +import { formatCommandForDisplay } from "../../format-command.js"; import fs from "fs"; import os from "os"; diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts index 5eaa11d660..41b7abbcd4 100644 --- a/codex-cli/src/utils/agent/handle-exec-command.ts +++ b/codex-cli/src/utils/agent/handle-exec-command.ts @@ -1,7 +1,7 @@ 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 "@lib/approvals.js"; +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { ResponseInputItem } from "openai/resources/responses/responses.mjs"; import { exec, execApplyPatch } from "./exec.js"; @@ -9,8 +9,8 @@ import { isLoggingEnabled, log } from "./log.js"; import { ReviewDecision } from "./review.js"; import { FullAutoErrorMode } from "../auto-approval-mode.js"; import { SandboxType } from "./sandbox/interface.js"; -import { canAutoApprove } from "@lib/approvals.js"; -import { formatCommandForDisplay } from "@lib/format-command.js"; +import { canAutoApprove } from "../../approvals.js"; +import { formatCommandForDisplay } from "../../format-command.js"; import { access } from "fs/promises"; // --------------------------------------------------------------------------- diff --git a/codex-cli/src/utils/agent/review.ts b/codex-cli/src/utils/agent/review.ts index ed2af0ce17..a370388569 100644 --- a/codex-cli/src/utils/agent/review.ts +++ b/codex-cli/src/utils/agent/review.ts @@ -1,4 +1,4 @@ -import type { SafeCommandReason } from "@lib/approvals"; +import type { SafeCommandReason } from "../../approvals"; export type CommandReviewDetails = { cmd: Array; diff --git a/codex-cli/src/utils/parsers.ts b/codex-cli/src/utils/parsers.ts index cb477c1d7d..815e7b2f00 100644 --- a/codex-cli/src/utils/parsers.ts +++ b/codex-cli/src/utils/parsers.ts @@ -3,11 +3,13 @@ import type { ExecInput, ExecOutputMetadata, } from "./agent/sandbox/interface.js"; -import type { SafeCommandReason } from "@lib/approvals.js"; import type { ResponseFunctionToolCall } from "openai/resources/responses/responses.mjs"; +import { isSafeCommand, type SafeCommandReason } from "../approvals.js"; import { log } from "node:console"; import process from "process"; +import { parse } from "shell-quote"; +import { formatCommandForDisplay } from "src/format-command.js"; // The console utility import is intentionally explicit to avoid bundlers from // including the entire `console` module when only the `log` function is @@ -23,52 +25,6 @@ const SAFE_SHELL_OPERATORS: ReadonlySet = new Set([ ";", ]); -// Lazily resolve heavy dependencies at runtime to avoid test environments -// (which might not have the @lib alias configured) from failing at import -// time. If the modules cannot be loaded we fall back to permissive stub -// implementations so that basic functionality – like unit‑testing small UI -// helpers – continues to work without the full codex‑lib dependency tree. - -let isSafeCommand: (cmd: Array) => SafeCommandReason | null = () => - null; -let shellQuoteParse: - | ((cmd: string, env?: Record) => Array) - | undefined; -let formatCommandForDisplay: (cmd: Array) => string = (cmd) => - cmd.join(" "); - -async function loadLibs(): Promise { - try { - const approvals = await import("@lib/approvals.js"); - if (typeof approvals.isSafeCommand === "function") { - isSafeCommand = approvals.isSafeCommand; - } - } catch { - // ignore – keep stub - } - try { - const fmt = await import("@lib/format-command.js"); - if (typeof fmt.formatCommandForDisplay === "function") { - formatCommandForDisplay = fmt.formatCommandForDisplay; - } - } catch { - // ignore – keep stub - } - try { - const sq = await import("shell-quote"); - if (typeof sq.parse === "function") { - shellQuoteParse = sq.parse as typeof shellQuoteParse; - } - } catch { - // ignore – keep stub - } -} - -// Trigger the dynamic import in the background; callers that need the real -// implementation should await the returned promise (parsers currently does not -// require this for correctness during tests). -void loadLibs(); - export function parseToolCallOutput(toolCallOutput: string): { output: string; metadata: ExecOutputMetadata; @@ -175,10 +131,9 @@ function computeAutoApproval(cmd: Array): SafeCommandReason | null { cmd.length === 3 && cmd[0] === "bash" && cmd[1] === "-lc" && - typeof cmd[2] === "string" && - shellQuoteParse + typeof cmd[2] === "string" ) { - const parsed = shellQuoteParse(cmd[2], process.env ?? {}); + const parsed = parse(cmd[2], process.env ?? {}); if (parsed.length === 0) { return null; } diff --git a/codex-cli/tests/agent-cancel-early.test.ts b/codex-cli/tests/agent-cancel-early.test.ts index b7b5fdcae7..b235a6d6bc 100644 --- a/codex-cli/tests/agent-cancel-early.test.ts +++ b/codex-cli/tests/agent-cancel-early.test.ts @@ -64,13 +64,13 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-cancel-prev-response.test.ts b/codex-cli/tests/agent-cancel-prev-response.test.ts index 4047f88452..fe73c338cc 100644 --- a/codex-cli/tests/agent-cancel-prev-response.test.ts +++ b/codex-cli/tests/agent-cancel-prev-response.test.ts @@ -71,13 +71,13 @@ vi.mock("openai", () => { }); // Stub helpers not relevant for this test. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-cancel-race.test.ts b/codex-cli/tests/agent-cancel-race.test.ts index c9c1845d8e..89e7cca744 100644 --- a/codex-cli/tests/agent-cancel-race.test.ts +++ b/codex-cli/tests/agent-cancel-race.test.ts @@ -67,11 +67,11 @@ vi.mock("openai", () => { }); // Stubs for external helpers referenced indirectly. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-cancel.test.ts b/codex-cli/tests/agent-cancel.test.ts index 93ff5ba317..69c17f7f7e 100644 --- a/codex-cli/tests/agent-cancel.test.ts +++ b/codex-cli/tests/agent-cancel.test.ts @@ -47,7 +47,7 @@ vi.mock("openai", () => { }); // Mock the approvals and formatCommand helpers referenced by handle‑exec‑command. -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, alwaysApprovedCommands: new Set(), @@ -57,7 +57,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/agent-function-call-id.test.ts b/codex-cli/tests/agent-function-call-id.test.ts index 7d0a55d122..d50c08eea4 100644 --- a/codex-cli/tests/agent-function-call-id.test.ts +++ b/codex-cli/tests/agent-function-call-id.test.ts @@ -88,14 +88,14 @@ vi.mock("openai", () => { }); // Stub approvals & command formatting – not relevant for this test. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-generic-network-error.test.ts b/codex-cli/tests/agent-generic-network-error.test.ts index 8c636fbda3..942adff668 100644 --- a/codex-cli/tests/agent-generic-network-error.test.ts +++ b/codex-cli/tests/agent-generic-network-error.test.ts @@ -23,14 +23,14 @@ vi.mock("openai", () => { }); // Stub approvals / formatting helpers – unrelated to network handling. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-invalid-request-error.test.ts b/codex-cli/tests/agent-invalid-request-error.test.ts index 631d451944..090d0b52d9 100644 --- a/codex-cli/tests/agent-invalid-request-error.test.ts +++ b/codex-cli/tests/agent-invalid-request-error.test.ts @@ -22,14 +22,14 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-max-tokens-error.test.ts b/codex-cli/tests/agent-max-tokens-error.test.ts index 5e01229454..de4fd17026 100644 --- a/codex-cli/tests/agent-max-tokens-error.test.ts +++ b/codex-cli/tests/agent-max-tokens-error.test.ts @@ -22,14 +22,14 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-network-errors.test.ts b/codex-cli/tests/agent-network-errors.test.ts index e01b08e918..f98ea5bf19 100644 --- a/codex-cli/tests/agent-network-errors.test.ts +++ b/codex-cli/tests/agent-network-errors.test.ts @@ -42,14 +42,14 @@ vi.mock("openai", () => { }); // Stub approvals / formatting helpers – not relevant here. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-project-doc.test.ts b/codex-cli/tests/agent-project-doc.test.ts index 97ad9c8c34..d3050f3953 100644 --- a/codex-cli/tests/agent-project-doc.test.ts +++ b/codex-cli/tests/agent-project-doc.test.ts @@ -51,7 +51,7 @@ vi.mock("openai", () => { // The AgentLoop pulls these helpers in order to decide whether a command can // be auto‑approved. None of that matters for this test, so we stub the module // with minimal no‑op implementations. -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, alwaysApprovedCommands: new Set(), @@ -61,7 +61,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/agent-rate-limit-error.test.ts b/codex-cli/tests/agent-rate-limit-error.test.ts index 18779450aa..9782744679 100644 --- a/codex-cli/tests/agent-rate-limit-error.test.ts +++ b/codex-cli/tests/agent-rate-limit-error.test.ts @@ -34,14 +34,14 @@ vi.mock("openai", () => { // Stub helpers that the agent indirectly imports so it does not attempt any // file‑system access or real approvals logic during the test. -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-server-retry.test.ts b/codex-cli/tests/agent-server-retry.test.ts index 9ec4eb5aac..09278f2ceb 100644 --- a/codex-cli/tests/agent-server-retry.test.ts +++ b/codex-cli/tests/agent-server-retry.test.ts @@ -32,14 +32,14 @@ vi.mock("openai", () => { }; }); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, alwaysApprovedCommands: new Set(), canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false } as any), isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/agent-terminate.test.ts b/codex-cli/tests/agent-terminate.test.ts index 7a60456518..bce77437af 100644 --- a/codex-cli/tests/agent-terminate.test.ts +++ b/codex-cli/tests/agent-terminate.test.ts @@ -49,7 +49,7 @@ vi.mock("openai", () => { // --- Helpers referenced by handle‑exec‑command ----------------------------- -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, alwaysApprovedCommands: new Set(), @@ -59,7 +59,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/agent-thinking-time.test.ts b/codex-cli/tests/agent-thinking-time.test.ts index c94d8a5e5e..7132070084 100644 --- a/codex-cli/tests/agent-thinking-time.test.ts +++ b/codex-cli/tests/agent-thinking-time.test.ts @@ -74,12 +74,12 @@ vi.mock("openai", () => { }); // Stub helpers referenced indirectly so we do not pull in real FS/network -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ __esModule: true, isSafeCommand: () => null, })); -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ __esModule: true, formatCommandForDisplay: (c: Array) => c.join(" "), })); diff --git a/codex-cli/tests/approvals.test.ts b/codex-cli/tests/approvals.test.ts index e06db94092..7cb0bd3d3e 100644 --- a/codex-cli/tests/approvals.test.ts +++ b/codex-cli/tests/approvals.test.ts @@ -1,6 +1,6 @@ -import type { SafetyAssessment } from "../src/lib/approvals"; +import type { SafetyAssessment } from "../src/approvals"; -import { canAutoApprove } from "../src/lib/approvals"; +import { canAutoApprove } from "../src/approvals"; import { describe, test, expect } from "vitest"; describe("canAutoApprove()", () => { diff --git a/codex-cli/tests/external-editor.test.ts b/codex-cli/tests/external-editor.test.ts index d530be5ecc..77041c2870 100644 --- a/codex-cli/tests/external-editor.test.ts +++ b/codex-cli/tests/external-editor.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer"; +import TextBuffer from "../src/text-buffer"; import { describe, it, expect, vi } from "vitest"; /* ------------------------------------------------------------------------- diff --git a/codex-cli/tests/format-command.test.ts b/codex-cli/tests/format-command.test.ts index 3981d9b106..7de417308a 100644 --- a/codex-cli/tests/format-command.test.ts +++ b/codex-cli/tests/format-command.test.ts @@ -1,4 +1,4 @@ -import { formatCommandForDisplay } from "../src/lib/format-command"; +import { formatCommandForDisplay } from "../src/format-command"; import { describe, test, expect } from "vitest"; describe("formatCommandForDisplay()", () => { diff --git a/codex-cli/tests/invalid-command-handling.test.ts b/codex-cli/tests/invalid-command-handling.test.ts index 6619b4f240..a3f87a7251 100644 --- a/codex-cli/tests/invalid-command-handling.test.ts +++ b/codex-cli/tests/invalid-command-handling.test.ts @@ -22,7 +22,7 @@ describe("rawExec – invalid command handling", () => { // --------------------------------------------------------------------------- // Mock approvals and logging helpers so the test focuses on execution flow. -vi.mock("@lib/approvals.js", () => { +vi.mock("../src/approvals.js", () => { return { __esModule: true, canAutoApprove: () => @@ -31,7 +31,7 @@ vi.mock("@lib/approvals.js", () => { }; }); -vi.mock("@lib/format-command.js", () => { +vi.mock("../src/format-command.js", () => { return { __esModule: true, formatCommandForDisplay: (cmd: Array) => cmd.join(" "), diff --git a/codex-cli/tests/multiline-external-editor-shortcut.test.tsx b/codex-cli/tests/multiline-external-editor-shortcut.test.tsx index 158a5e655d..9b2e2f25e5 100644 --- a/codex-cli/tests/multiline-external-editor-shortcut.test.tsx +++ b/codex-cli/tests/multiline-external-editor-shortcut.test.tsx @@ -1,6 +1,6 @@ import { renderTui } from "./ui-test-helpers.js"; import MultilineTextEditor from "../src/components/chat/multiline-editor.js"; -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import * as React from "react"; import { describe, it, expect, vi } from "vitest"; diff --git a/codex-cli/tests/multiline-history-behavior.test.tsx b/codex-cli/tests/multiline-history-behavior.test.tsx index 5c906837ee..cada52ddab 100644 --- a/codex-cli/tests/multiline-history-behavior.test.tsx +++ b/codex-cli/tests/multiline-history-behavior.test.tsx @@ -34,12 +34,12 @@ vi.mock("../src/utils/input-utils.js", () => ({ })), })); -// Mock the optional @lib/* dependencies so the dynamic import in parsers.ts +// Mock the optional ../src/* dependencies so the dynamic import in parsers.ts // does not fail during the test environment where the alias isn't configured. -vi.mock("@lib/format-command.js", () => ({ +vi.mock("../src/format-command.js", () => ({ formatCommandForDisplay: (cmd: Array) => cmd.join(" "), })); -vi.mock("@lib/approvals.js", () => ({ +vi.mock("../src/approvals.js", () => ({ isSafeCommand: (_cmd: Array) => null, })); diff --git a/codex-cli/tests/parse-apply-patch.test.ts b/codex-cli/tests/parse-apply-patch.test.ts index 0195542e56..53aa119093 100644 --- a/codex-cli/tests/parse-apply-patch.test.ts +++ b/codex-cli/tests/parse-apply-patch.test.ts @@ -1,4 +1,4 @@ -import { parseApplyPatch } from "../src/lib/parse-apply-patch"; +import { parseApplyPatch } from "../src/parse-apply-patch"; import { expect, test, describe } from "vitest"; // Helper function to unwrap a non‑null result in tests that expect success. diff --git a/codex-cli/tests/text-buffer-copy-paste.test.ts b/codex-cli/tests/text-buffer-copy-paste.test.ts index 311b2b9aa6..cc1fd119e5 100644 --- a/codex-cli/tests/text-buffer-copy-paste.test.ts +++ b/codex-cli/tests/text-buffer-copy-paste.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import { describe, it, expect } from "vitest"; // These tests ensure that the TextBuffer copy‑&‑paste logic keeps parity with diff --git a/codex-cli/tests/text-buffer-crlf.test.ts b/codex-cli/tests/text-buffer-crlf.test.ts index 4b33b498b5..736c22a27d 100644 --- a/codex-cli/tests/text-buffer-crlf.test.ts +++ b/codex-cli/tests/text-buffer-crlf.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import { describe, it, expect } from "vitest"; describe("TextBuffer – newline normalisation", () => { diff --git a/codex-cli/tests/text-buffer-gaps.test.ts b/codex-cli/tests/text-buffer-gaps.test.ts index 986ad37eed..046d468e8a 100644 --- a/codex-cli/tests/text-buffer-gaps.test.ts +++ b/codex-cli/tests/text-buffer-gaps.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer"; +import TextBuffer from "../src/text-buffer"; import { describe, it, expect } from "vitest"; // The purpose of this test‑suite is NOT to make the implementation green today diff --git a/codex-cli/tests/text-buffer-word.test.ts b/codex-cli/tests/text-buffer-word.test.ts index 009786a28c..4ea7679450 100644 --- a/codex-cli/tests/text-buffer-word.test.ts +++ b/codex-cli/tests/text-buffer-word.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer.js"; +import TextBuffer from "../src/text-buffer.js"; import { describe, test, expect } from "vitest"; describe("TextBuffer – word‑wise navigation & deletion", () => { diff --git a/codex-cli/tests/text-buffer.test.ts b/codex-cli/tests/text-buffer.test.ts index ae78f29409..c3f33d0fa1 100644 --- a/codex-cli/tests/text-buffer.test.ts +++ b/codex-cli/tests/text-buffer.test.ts @@ -1,4 +1,4 @@ -import TextBuffer from "../src/lib/text-buffer"; +import TextBuffer from "../src/text-buffer"; import { describe, it, expect } from "vitest"; describe("TextBuffer – basic editing parity with Rust suite", () => { diff --git a/codex-cli/tsconfig.json b/codex-cli/tsconfig.json index 626fc5dbf4..d1dacc9149 100644 --- a/codex-cli/tsconfig.json +++ b/codex-cli/tsconfig.json @@ -11,9 +11,6 @@ ], "types": ["node"], "baseUrl": "./", - "paths": { - "@lib/*": ["./src/lib/*"] - }, "resolveJsonModule": false, // ESM doesn't yet support JSON modules. "jsx": "react", "declaration": true, From 54ba90c4b80cc820ca3267e93a2dd8dbf681c52f Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Apr 2025 15:09:12 -0700 Subject: [PATCH 04/14] Removes computeAutoApproval() and tightens up canAutoApprove() as the source of truth --- codex-cli/src/approvals.ts | 127 ++++++++++-------- .../components/chat/use-message-grouping.ts | 72 ---------- codex-cli/src/utils/agent/review.ts | 8 -- codex-cli/src/utils/parsers.ts | 111 ++------------- 4 files changed, 80 insertions(+), 238 deletions(-) diff --git a/codex-cli/src/approvals.ts b/codex-cli/src/approvals.ts index 0cf3703b54..8a670b01ca 100644 --- a/codex-cli/src/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -75,19 +75,72 @@ export function canAutoApprove( writableRoots: ReadonlyArray, env: NodeJS.ProcessEnv = process.env, ): SafetyAssessment { - try { - if (command[0] === "apply_patch") { - return command.length === 2 && typeof command[1] === "string" - ? canAutoApproveApplyPatch(command[1], writableRoots, policy) - : { - type: "reject", - reason: "Invalid apply_patch command", - }; + if (command[0] === "apply_patch") { + return command.length === 2 && typeof command[1] === "string" + ? canAutoApproveApplyPatch(command[1], writableRoots, policy) + : { + type: "reject", + reason: "Invalid apply_patch command", + }; + } + + const isSafe = isSafeCommand(command); + if (isSafe != null) { + const { reason, group } = isSafe; + return { + type: "auto-approve", + reason, + group, + runInSandbox: false, + }; + } + + if ( + command[0] === "bash" && + command[1] === "-lc" && + typeof command[2] === "string" && + command.length === 3 + ) { + const applyPatchArg = tryParseApplyPatch(command[2]); + if (applyPatchArg != null) { + return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); } - const isSafe = isSafeCommand(command); - if (isSafe != null) { - const { reason, group } = isSafe; + let bashCmd; + try { + bashCmd = parse(command[2], env); + } catch (e) { + // In practice, there seem to be syntactically valid shell commands that + // shell-quote cannot parse, so we should not reject, but ask the user. + switch (policy) { + case "full-auto": + // In full-auto, we still run the command automatically, but must + // restrict it to the sandbox. + return { + type: "auto-approve", + reason: "Full auto mode", + group: "Running commands", + runInSandbox: true, + }; + case "suggest": + case "auto-edit": + // In all other modes, since we cannot reason about the command, we + // should ask the user. + return { + type: "ask-user", + }; + } + } + + // bashCmd could be a mix of strings and operators, e.g.: + // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] + // We try to ensure that *every* command segment is deemed safe and that + // all operators belong to an allow‑list. If so, the entire expression is + // considered auto‑approvable. + + const shellSafe = isEntireShellExpressionSafe(bashCmd); + if (shellSafe != null) { + const { reason, group } = shellSafe; return { type: "auto-approve", reason, @@ -95,58 +148,16 @@ export function canAutoApprove( runInSandbox: false, }; } + } - if ( - command[0] === "bash" && - command[1] === "-lc" && - typeof command[2] === "string" && - command.length === 3 - ) { - const applyPatchArg = tryParseApplyPatch(command[2]); - if (applyPatchArg != null) { - return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); - } - - const bashCmd = parse(command[2], env); - - // bashCmd could be a mix of strings and operators, e.g.: - // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] - // We try to ensure that *every* command segment is deemed safe and that - // all operators belong to an allow‑list. If so, the entire expression is - // considered auto‑approvable. - - const shellSafe = isEntireShellExpressionSafe(bashCmd); - if (shellSafe != null) { - const { reason, group } = shellSafe; - return { - type: "auto-approve", - reason, - group, - runInSandbox: false, - }; - } - } - - return policy === "full-auto" - ? { - type: "auto-approve", - reason: "Full auto mode", - group: "Running commands", - runInSandbox: true, - } - : { type: "ask-user" }; - } catch (err) { - if (policy === "full-auto") { - return { + return policy === "full-auto" + ? { type: "auto-approve", reason: "Full auto mode", group: "Running commands", runInSandbox: true, - }; - } else { - return { type: "ask-user" }; - } - } + } + : { type: "ask-user" }; } function canAutoApproveApplyPatch( diff --git a/codex-cli/src/components/chat/use-message-grouping.ts b/codex-cli/src/components/chat/use-message-grouping.ts index 75e51ac682..1e526821d0 100644 --- a/codex-cli/src/components/chat/use-message-grouping.ts +++ b/codex-cli/src/components/chat/use-message-grouping.ts @@ -1,8 +1,5 @@ import type { ResponseItem } from "openai/resources/responses/responses.mjs"; -import { parseToolCall } from "../../utils/parsers.js"; -import { useMemo } from "react"; - /** * Represents a grouped sequence of response items (e.g., function call batches). */ @@ -10,72 +7,3 @@ export type GroupedResponseItem = { label: string; items: Array; }; - -/** - * Custom hook to group recent response items for display batching. - * Returns counts of auto-approved tool call groups, the latest batch, - * and the count of user messages in the visible window. - */ -export function useMessageGrouping(visibleItems: Array): { - groupCounts: Record; - batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }>; - userMsgCount: number; -} { - return useMemo(() => { - // The grouping logic only depends on the subset of messages that are - // currently rendered (visibleItems). Using that as the sole dependency - // keeps recomputations to a minimum and avoids unnecessary work when the - // full list of `items` changes outside of the visible window. - let userMsgCount = 0; - const groupCounts: Record = {}; - visibleItems.forEach((m) => { - if (m.type === "function_call") { - const toolCall = parseToolCall(m); - if (toolCall?.autoApproval) { - const group = toolCall.autoApproval.group; - groupCounts[group] = (groupCounts[group] || 0) + 1; - } - } - if (m.type === "message" && m.role === "user") { - userMsgCount++; - } - }); - const lastFew = visibleItems.slice(-3); - const batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }> = - []; - if (lastFew[0]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[0]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew, - }, - }); - if (lastFew[2]?.type === "message") { - batch.push({ item: lastFew[2] }); - } - } else if (lastFew[1]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[1]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew.slice(1), - }, - }); - } else if (lastFew[2]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[2]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: [lastFew[2]], - }, - }); - } else { - lastFew.forEach((item) => batch.push({ item })); - } - return { groupCounts, batch, userMsgCount }; - // `items` is stable across renders while `visibleItems` changes based on - // the scroll window. Including only `visibleItems` avoids unnecessary - // recomputations while still producing correct results. - }, [visibleItems]); -} diff --git a/codex-cli/src/utils/agent/review.ts b/codex-cli/src/utils/agent/review.ts index a370388569..9a5e66a4ff 100644 --- a/codex-cli/src/utils/agent/review.ts +++ b/codex-cli/src/utils/agent/review.ts @@ -1,11 +1,3 @@ -import type { SafeCommandReason } from "../../approvals"; - -export type CommandReviewDetails = { - cmd: Array; - cmdReadableText: string; - autoApproval: SafeCommandReason | null; -}; - export enum ReviewDecision { YES = "yes", NO_CONTINUE = "no-continue", diff --git a/codex-cli/src/utils/parsers.ts b/codex-cli/src/utils/parsers.ts index 815e7b2f00..e46db3b34d 100644 --- a/codex-cli/src/utils/parsers.ts +++ b/codex-cli/src/utils/parsers.ts @@ -5,26 +5,13 @@ import type { } from "./agent/sandbox/interface.js"; import type { ResponseFunctionToolCall } from "openai/resources/responses/responses.mjs"; -import { isSafeCommand, type SafeCommandReason } from "../approvals.js"; import { log } from "node:console"; -import process from "process"; -import { parse } from "shell-quote"; import { formatCommandForDisplay } from "src/format-command.js"; // The console utility import is intentionally explicit to avoid bundlers from // including the entire `console` module when only the `log` function is // required. -// Allowed shell operators that we consider "safe" as they do not introduce -// side‑effects on their own (unlike redirections). Parentheses and braces for -// grouping are excluded for simplicity. -const SAFE_SHELL_OPERATORS: ReadonlySet = new Set([ - "&&", - "||", - "|", - ";", -]); - export function parseToolCallOutput(toolCallOutput: string): { output: string; metadata: ExecOutputMetadata; @@ -46,6 +33,17 @@ export function parseToolCallOutput(toolCallOutput: string): { } } +export type CommandReviewDetails = { + cmd: Array; + cmdReadableText: string; +}; + +/** + * Tries to parse a tool call and, if successful, returns an object that has + * both: + * - an array of strings to use with `ExecInput` and `canAutoApprove()` + * - a human-readable string to display to the user + */ export function parseToolCall( toolCall: ResponseFunctionToolCall, ): CommandReviewDetails | undefined { @@ -57,12 +55,9 @@ export function parseToolCall( const { cmd } = toolCallArgs; const cmdReadableText = formatCommandForDisplay(cmd); - const autoApproval = computeAutoApproval(cmd); - return { cmd, cmdReadableText, - autoApproval, }; } @@ -109,87 +104,3 @@ function toStringArray(obj: unknown): Array | undefined { return undefined; } } - -// ---------------- safe‑command helpers ---------------- - -/** - * Attempts to determine whether `cmd` is composed exclusively of safe - * sub‑commands combined using only operators from the SAFE_SHELL_OPERATORS - * allow‑list. Returns the `SafeCommandReason` (taken from the first sub‑command) - * if the whole expression is safe; otherwise returns `null`. - */ -function computeAutoApproval(cmd: Array): SafeCommandReason | null { - // Fast path: a simple command with no shell processing. - const direct = isSafeCommand(cmd); - if (direct != null) { - return direct; - } - - // For expressions like ["bash", "-lc", "ls && pwd"] break down the inner - // string and verify each segment. - if ( - cmd.length === 3 && - cmd[0] === "bash" && - cmd[1] === "-lc" && - typeof cmd[2] === "string" - ) { - const parsed = parse(cmd[2], process.env ?? {}); - if (parsed.length === 0) { - return null; - } - - let current: Array = []; - let first: SafeCommandReason | null = null; - - const flush = (): boolean => { - if (current.length === 0) { - return true; - } - const safe = isSafeCommand(current); - if (safe == null) { - return false; - } - if (!first) { - first = safe; - } - current = []; - return true; - }; - - for (const part of parsed) { - if (typeof part === "string") { - // Simple word/argument token. - if (part === "(" || part === ")" || part === "{" || part === "}") { - // We treat explicit grouping tokens as unsafe because their - // semantics depend on the shell evaluation environment. - return null; - } - current.push(part); - } else if (part && typeof part === "object") { - const opToken = part as { op?: string }; - if (typeof opToken.op === "string") { - if (!flush()) { - return null; - } - if (!SAFE_SHELL_OPERATORS.has(opToken.op)) { - return null; - } - } else { - // Unknown object token kind (e.g. redirection) – treat as unsafe. - return null; - } - } else { - // Token types such as numbers / booleans are unexpected – treat as unsafe. - return null; - } - } - - if (!flush()) { - return null; - } - - return first; - } - - return null; -} From c56f84c5f59ddd36cb92a46bf184773f1bc5c318 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Apr 2025 15:09:12 -0700 Subject: [PATCH 05/14] Removes computeAutoApproval() and tightens up canAutoApprove() as the source of truth --- codex-cli/src/approvals.ts | 127 ++++++++++-------- .../components/chat/use-message-grouping.ts | 72 ---------- codex-cli/src/utils/agent/review.ts | 8 -- codex-cli/src/utils/parsers.ts | 112 ++------------- 4 files changed, 80 insertions(+), 239 deletions(-) diff --git a/codex-cli/src/approvals.ts b/codex-cli/src/approvals.ts index 0cf3703b54..8a670b01ca 100644 --- a/codex-cli/src/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -75,19 +75,72 @@ export function canAutoApprove( writableRoots: ReadonlyArray, env: NodeJS.ProcessEnv = process.env, ): SafetyAssessment { - try { - if (command[0] === "apply_patch") { - return command.length === 2 && typeof command[1] === "string" - ? canAutoApproveApplyPatch(command[1], writableRoots, policy) - : { - type: "reject", - reason: "Invalid apply_patch command", - }; + if (command[0] === "apply_patch") { + return command.length === 2 && typeof command[1] === "string" + ? canAutoApproveApplyPatch(command[1], writableRoots, policy) + : { + type: "reject", + reason: "Invalid apply_patch command", + }; + } + + const isSafe = isSafeCommand(command); + if (isSafe != null) { + const { reason, group } = isSafe; + return { + type: "auto-approve", + reason, + group, + runInSandbox: false, + }; + } + + if ( + command[0] === "bash" && + command[1] === "-lc" && + typeof command[2] === "string" && + command.length === 3 + ) { + const applyPatchArg = tryParseApplyPatch(command[2]); + if (applyPatchArg != null) { + return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); } - const isSafe = isSafeCommand(command); - if (isSafe != null) { - const { reason, group } = isSafe; + let bashCmd; + try { + bashCmd = parse(command[2], env); + } catch (e) { + // In practice, there seem to be syntactically valid shell commands that + // shell-quote cannot parse, so we should not reject, but ask the user. + switch (policy) { + case "full-auto": + // In full-auto, we still run the command automatically, but must + // restrict it to the sandbox. + return { + type: "auto-approve", + reason: "Full auto mode", + group: "Running commands", + runInSandbox: true, + }; + case "suggest": + case "auto-edit": + // In all other modes, since we cannot reason about the command, we + // should ask the user. + return { + type: "ask-user", + }; + } + } + + // bashCmd could be a mix of strings and operators, e.g.: + // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] + // We try to ensure that *every* command segment is deemed safe and that + // all operators belong to an allow‑list. If so, the entire expression is + // considered auto‑approvable. + + const shellSafe = isEntireShellExpressionSafe(bashCmd); + if (shellSafe != null) { + const { reason, group } = shellSafe; return { type: "auto-approve", reason, @@ -95,58 +148,16 @@ export function canAutoApprove( runInSandbox: false, }; } + } - if ( - command[0] === "bash" && - command[1] === "-lc" && - typeof command[2] === "string" && - command.length === 3 - ) { - const applyPatchArg = tryParseApplyPatch(command[2]); - if (applyPatchArg != null) { - return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); - } - - const bashCmd = parse(command[2], env); - - // bashCmd could be a mix of strings and operators, e.g.: - // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] - // We try to ensure that *every* command segment is deemed safe and that - // all operators belong to an allow‑list. If so, the entire expression is - // considered auto‑approvable. - - const shellSafe = isEntireShellExpressionSafe(bashCmd); - if (shellSafe != null) { - const { reason, group } = shellSafe; - return { - type: "auto-approve", - reason, - group, - runInSandbox: false, - }; - } - } - - return policy === "full-auto" - ? { - type: "auto-approve", - reason: "Full auto mode", - group: "Running commands", - runInSandbox: true, - } - : { type: "ask-user" }; - } catch (err) { - if (policy === "full-auto") { - return { + return policy === "full-auto" + ? { type: "auto-approve", reason: "Full auto mode", group: "Running commands", runInSandbox: true, - }; - } else { - return { type: "ask-user" }; - } - } + } + : { type: "ask-user" }; } function canAutoApproveApplyPatch( diff --git a/codex-cli/src/components/chat/use-message-grouping.ts b/codex-cli/src/components/chat/use-message-grouping.ts index 75e51ac682..1e526821d0 100644 --- a/codex-cli/src/components/chat/use-message-grouping.ts +++ b/codex-cli/src/components/chat/use-message-grouping.ts @@ -1,8 +1,5 @@ import type { ResponseItem } from "openai/resources/responses/responses.mjs"; -import { parseToolCall } from "../../utils/parsers.js"; -import { useMemo } from "react"; - /** * Represents a grouped sequence of response items (e.g., function call batches). */ @@ -10,72 +7,3 @@ export type GroupedResponseItem = { label: string; items: Array; }; - -/** - * Custom hook to group recent response items for display batching. - * Returns counts of auto-approved tool call groups, the latest batch, - * and the count of user messages in the visible window. - */ -export function useMessageGrouping(visibleItems: Array): { - groupCounts: Record; - batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }>; - userMsgCount: number; -} { - return useMemo(() => { - // The grouping logic only depends on the subset of messages that are - // currently rendered (visibleItems). Using that as the sole dependency - // keeps recomputations to a minimum and avoids unnecessary work when the - // full list of `items` changes outside of the visible window. - let userMsgCount = 0; - const groupCounts: Record = {}; - visibleItems.forEach((m) => { - if (m.type === "function_call") { - const toolCall = parseToolCall(m); - if (toolCall?.autoApproval) { - const group = toolCall.autoApproval.group; - groupCounts[group] = (groupCounts[group] || 0) + 1; - } - } - if (m.type === "message" && m.role === "user") { - userMsgCount++; - } - }); - const lastFew = visibleItems.slice(-3); - const batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }> = - []; - if (lastFew[0]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[0]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew, - }, - }); - if (lastFew[2]?.type === "message") { - batch.push({ item: lastFew[2] }); - } - } else if (lastFew[1]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[1]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew.slice(1), - }, - }); - } else if (lastFew[2]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[2]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: [lastFew[2]], - }, - }); - } else { - lastFew.forEach((item) => batch.push({ item })); - } - return { groupCounts, batch, userMsgCount }; - // `items` is stable across renders while `visibleItems` changes based on - // the scroll window. Including only `visibleItems` avoids unnecessary - // recomputations while still producing correct results. - }, [visibleItems]); -} diff --git a/codex-cli/src/utils/agent/review.ts b/codex-cli/src/utils/agent/review.ts index a370388569..9a5e66a4ff 100644 --- a/codex-cli/src/utils/agent/review.ts +++ b/codex-cli/src/utils/agent/review.ts @@ -1,11 +1,3 @@ -import type { SafeCommandReason } from "../../approvals"; - -export type CommandReviewDetails = { - cmd: Array; - cmdReadableText: string; - autoApproval: SafeCommandReason | null; -}; - export enum ReviewDecision { YES = "yes", NO_CONTINUE = "no-continue", diff --git a/codex-cli/src/utils/parsers.ts b/codex-cli/src/utils/parsers.ts index 815e7b2f00..4461379c02 100644 --- a/codex-cli/src/utils/parsers.ts +++ b/codex-cli/src/utils/parsers.ts @@ -1,30 +1,16 @@ -import type { CommandReviewDetails } from "./agent/review.js"; import type { ExecInput, ExecOutputMetadata, } from "./agent/sandbox/interface.js"; import type { ResponseFunctionToolCall } from "openai/resources/responses/responses.mjs"; -import { isSafeCommand, type SafeCommandReason } from "../approvals.js"; import { log } from "node:console"; -import process from "process"; -import { parse } from "shell-quote"; import { formatCommandForDisplay } from "src/format-command.js"; // The console utility import is intentionally explicit to avoid bundlers from // including the entire `console` module when only the `log` function is // required. -// Allowed shell operators that we consider "safe" as they do not introduce -// side‑effects on their own (unlike redirections). Parentheses and braces for -// grouping are excluded for simplicity. -const SAFE_SHELL_OPERATORS: ReadonlySet = new Set([ - "&&", - "||", - "|", - ";", -]); - export function parseToolCallOutput(toolCallOutput: string): { output: string; metadata: ExecOutputMetadata; @@ -46,6 +32,17 @@ export function parseToolCallOutput(toolCallOutput: string): { } } +export type CommandReviewDetails = { + cmd: Array; + cmdReadableText: string; +}; + +/** + * Tries to parse a tool call and, if successful, returns an object that has + * both: + * - an array of strings to use with `ExecInput` and `canAutoApprove()` + * - a human-readable string to display to the user + */ export function parseToolCall( toolCall: ResponseFunctionToolCall, ): CommandReviewDetails | undefined { @@ -57,12 +54,9 @@ export function parseToolCall( const { cmd } = toolCallArgs; const cmdReadableText = formatCommandForDisplay(cmd); - const autoApproval = computeAutoApproval(cmd); - return { cmd, cmdReadableText, - autoApproval, }; } @@ -109,87 +103,3 @@ function toStringArray(obj: unknown): Array | undefined { return undefined; } } - -// ---------------- safe‑command helpers ---------------- - -/** - * Attempts to determine whether `cmd` is composed exclusively of safe - * sub‑commands combined using only operators from the SAFE_SHELL_OPERATORS - * allow‑list. Returns the `SafeCommandReason` (taken from the first sub‑command) - * if the whole expression is safe; otherwise returns `null`. - */ -function computeAutoApproval(cmd: Array): SafeCommandReason | null { - // Fast path: a simple command with no shell processing. - const direct = isSafeCommand(cmd); - if (direct != null) { - return direct; - } - - // For expressions like ["bash", "-lc", "ls && pwd"] break down the inner - // string and verify each segment. - if ( - cmd.length === 3 && - cmd[0] === "bash" && - cmd[1] === "-lc" && - typeof cmd[2] === "string" - ) { - const parsed = parse(cmd[2], process.env ?? {}); - if (parsed.length === 0) { - return null; - } - - let current: Array = []; - let first: SafeCommandReason | null = null; - - const flush = (): boolean => { - if (current.length === 0) { - return true; - } - const safe = isSafeCommand(current); - if (safe == null) { - return false; - } - if (!first) { - first = safe; - } - current = []; - return true; - }; - - for (const part of parsed) { - if (typeof part === "string") { - // Simple word/argument token. - if (part === "(" || part === ")" || part === "{" || part === "}") { - // We treat explicit grouping tokens as unsafe because their - // semantics depend on the shell evaluation environment. - return null; - } - current.push(part); - } else if (part && typeof part === "object") { - const opToken = part as { op?: string }; - if (typeof opToken.op === "string") { - if (!flush()) { - return null; - } - if (!SAFE_SHELL_OPERATORS.has(opToken.op)) { - return null; - } - } else { - // Unknown object token kind (e.g. redirection) – treat as unsafe. - return null; - } - } else { - // Token types such as numbers / booleans are unexpected – treat as unsafe. - return null; - } - } - - if (!flush()) { - return null; - } - - return first; - } - - return null; -} From db9c62b85d40fd1fc44cadc0b3337da2c3e12c17 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Apr 2025 15:14:02 -0700 Subject: [PATCH 06/14] Removes computeAutoApproval() and tightens up canAutoApprove() as the source of truth --- codex-cli/src/approvals.ts | 127 ++++++++++-------- .../components/chat/use-message-grouping.ts | 72 ---------- codex-cli/src/utils/agent/review.ts | 8 -- codex-cli/src/utils/parsers.ts | 112 ++------------- 4 files changed, 80 insertions(+), 239 deletions(-) diff --git a/codex-cli/src/approvals.ts b/codex-cli/src/approvals.ts index 0cf3703b54..8a670b01ca 100644 --- a/codex-cli/src/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -75,19 +75,72 @@ export function canAutoApprove( writableRoots: ReadonlyArray, env: NodeJS.ProcessEnv = process.env, ): SafetyAssessment { - try { - if (command[0] === "apply_patch") { - return command.length === 2 && typeof command[1] === "string" - ? canAutoApproveApplyPatch(command[1], writableRoots, policy) - : { - type: "reject", - reason: "Invalid apply_patch command", - }; + if (command[0] === "apply_patch") { + return command.length === 2 && typeof command[1] === "string" + ? canAutoApproveApplyPatch(command[1], writableRoots, policy) + : { + type: "reject", + reason: "Invalid apply_patch command", + }; + } + + const isSafe = isSafeCommand(command); + if (isSafe != null) { + const { reason, group } = isSafe; + return { + type: "auto-approve", + reason, + group, + runInSandbox: false, + }; + } + + if ( + command[0] === "bash" && + command[1] === "-lc" && + typeof command[2] === "string" && + command.length === 3 + ) { + const applyPatchArg = tryParseApplyPatch(command[2]); + if (applyPatchArg != null) { + return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); } - const isSafe = isSafeCommand(command); - if (isSafe != null) { - const { reason, group } = isSafe; + let bashCmd; + try { + bashCmd = parse(command[2], env); + } catch (e) { + // In practice, there seem to be syntactically valid shell commands that + // shell-quote cannot parse, so we should not reject, but ask the user. + switch (policy) { + case "full-auto": + // In full-auto, we still run the command automatically, but must + // restrict it to the sandbox. + return { + type: "auto-approve", + reason: "Full auto mode", + group: "Running commands", + runInSandbox: true, + }; + case "suggest": + case "auto-edit": + // In all other modes, since we cannot reason about the command, we + // should ask the user. + return { + type: "ask-user", + }; + } + } + + // bashCmd could be a mix of strings and operators, e.g.: + // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] + // We try to ensure that *every* command segment is deemed safe and that + // all operators belong to an allow‑list. If so, the entire expression is + // considered auto‑approvable. + + const shellSafe = isEntireShellExpressionSafe(bashCmd); + if (shellSafe != null) { + const { reason, group } = shellSafe; return { type: "auto-approve", reason, @@ -95,58 +148,16 @@ export function canAutoApprove( runInSandbox: false, }; } + } - if ( - command[0] === "bash" && - command[1] === "-lc" && - typeof command[2] === "string" && - command.length === 3 - ) { - const applyPatchArg = tryParseApplyPatch(command[2]); - if (applyPatchArg != null) { - return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); - } - - const bashCmd = parse(command[2], env); - - // bashCmd could be a mix of strings and operators, e.g.: - // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] - // We try to ensure that *every* command segment is deemed safe and that - // all operators belong to an allow‑list. If so, the entire expression is - // considered auto‑approvable. - - const shellSafe = isEntireShellExpressionSafe(bashCmd); - if (shellSafe != null) { - const { reason, group } = shellSafe; - return { - type: "auto-approve", - reason, - group, - runInSandbox: false, - }; - } - } - - return policy === "full-auto" - ? { - type: "auto-approve", - reason: "Full auto mode", - group: "Running commands", - runInSandbox: true, - } - : { type: "ask-user" }; - } catch (err) { - if (policy === "full-auto") { - return { + return policy === "full-auto" + ? { type: "auto-approve", reason: "Full auto mode", group: "Running commands", runInSandbox: true, - }; - } else { - return { type: "ask-user" }; - } - } + } + : { type: "ask-user" }; } function canAutoApproveApplyPatch( diff --git a/codex-cli/src/components/chat/use-message-grouping.ts b/codex-cli/src/components/chat/use-message-grouping.ts index 75e51ac682..1e526821d0 100644 --- a/codex-cli/src/components/chat/use-message-grouping.ts +++ b/codex-cli/src/components/chat/use-message-grouping.ts @@ -1,8 +1,5 @@ import type { ResponseItem } from "openai/resources/responses/responses.mjs"; -import { parseToolCall } from "../../utils/parsers.js"; -import { useMemo } from "react"; - /** * Represents a grouped sequence of response items (e.g., function call batches). */ @@ -10,72 +7,3 @@ export type GroupedResponseItem = { label: string; items: Array; }; - -/** - * Custom hook to group recent response items for display batching. - * Returns counts of auto-approved tool call groups, the latest batch, - * and the count of user messages in the visible window. - */ -export function useMessageGrouping(visibleItems: Array): { - groupCounts: Record; - batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }>; - userMsgCount: number; -} { - return useMemo(() => { - // The grouping logic only depends on the subset of messages that are - // currently rendered (visibleItems). Using that as the sole dependency - // keeps recomputations to a minimum and avoids unnecessary work when the - // full list of `items` changes outside of the visible window. - let userMsgCount = 0; - const groupCounts: Record = {}; - visibleItems.forEach((m) => { - if (m.type === "function_call") { - const toolCall = parseToolCall(m); - if (toolCall?.autoApproval) { - const group = toolCall.autoApproval.group; - groupCounts[group] = (groupCounts[group] || 0) + 1; - } - } - if (m.type === "message" && m.role === "user") { - userMsgCount++; - } - }); - const lastFew = visibleItems.slice(-3); - const batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }> = - []; - if (lastFew[0]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[0]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew, - }, - }); - if (lastFew[2]?.type === "message") { - batch.push({ item: lastFew[2] }); - } - } else if (lastFew[1]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[1]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew.slice(1), - }, - }); - } else if (lastFew[2]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[2]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: [lastFew[2]], - }, - }); - } else { - lastFew.forEach((item) => batch.push({ item })); - } - return { groupCounts, batch, userMsgCount }; - // `items` is stable across renders while `visibleItems` changes based on - // the scroll window. Including only `visibleItems` avoids unnecessary - // recomputations while still producing correct results. - }, [visibleItems]); -} diff --git a/codex-cli/src/utils/agent/review.ts b/codex-cli/src/utils/agent/review.ts index a370388569..9a5e66a4ff 100644 --- a/codex-cli/src/utils/agent/review.ts +++ b/codex-cli/src/utils/agent/review.ts @@ -1,11 +1,3 @@ -import type { SafeCommandReason } from "../../approvals"; - -export type CommandReviewDetails = { - cmd: Array; - cmdReadableText: string; - autoApproval: SafeCommandReason | null; -}; - export enum ReviewDecision { YES = "yes", NO_CONTINUE = "no-continue", diff --git a/codex-cli/src/utils/parsers.ts b/codex-cli/src/utils/parsers.ts index 815e7b2f00..4461379c02 100644 --- a/codex-cli/src/utils/parsers.ts +++ b/codex-cli/src/utils/parsers.ts @@ -1,30 +1,16 @@ -import type { CommandReviewDetails } from "./agent/review.js"; import type { ExecInput, ExecOutputMetadata, } from "./agent/sandbox/interface.js"; import type { ResponseFunctionToolCall } from "openai/resources/responses/responses.mjs"; -import { isSafeCommand, type SafeCommandReason } from "../approvals.js"; import { log } from "node:console"; -import process from "process"; -import { parse } from "shell-quote"; import { formatCommandForDisplay } from "src/format-command.js"; // The console utility import is intentionally explicit to avoid bundlers from // including the entire `console` module when only the `log` function is // required. -// Allowed shell operators that we consider "safe" as they do not introduce -// side‑effects on their own (unlike redirections). Parentheses and braces for -// grouping are excluded for simplicity. -const SAFE_SHELL_OPERATORS: ReadonlySet = new Set([ - "&&", - "||", - "|", - ";", -]); - export function parseToolCallOutput(toolCallOutput: string): { output: string; metadata: ExecOutputMetadata; @@ -46,6 +32,17 @@ export function parseToolCallOutput(toolCallOutput: string): { } } +export type CommandReviewDetails = { + cmd: Array; + cmdReadableText: string; +}; + +/** + * Tries to parse a tool call and, if successful, returns an object that has + * both: + * - an array of strings to use with `ExecInput` and `canAutoApprove()` + * - a human-readable string to display to the user + */ export function parseToolCall( toolCall: ResponseFunctionToolCall, ): CommandReviewDetails | undefined { @@ -57,12 +54,9 @@ export function parseToolCall( const { cmd } = toolCallArgs; const cmdReadableText = formatCommandForDisplay(cmd); - const autoApproval = computeAutoApproval(cmd); - return { cmd, cmdReadableText, - autoApproval, }; } @@ -109,87 +103,3 @@ function toStringArray(obj: unknown): Array | undefined { return undefined; } } - -// ---------------- safe‑command helpers ---------------- - -/** - * Attempts to determine whether `cmd` is composed exclusively of safe - * sub‑commands combined using only operators from the SAFE_SHELL_OPERATORS - * allow‑list. Returns the `SafeCommandReason` (taken from the first sub‑command) - * if the whole expression is safe; otherwise returns `null`. - */ -function computeAutoApproval(cmd: Array): SafeCommandReason | null { - // Fast path: a simple command with no shell processing. - const direct = isSafeCommand(cmd); - if (direct != null) { - return direct; - } - - // For expressions like ["bash", "-lc", "ls && pwd"] break down the inner - // string and verify each segment. - if ( - cmd.length === 3 && - cmd[0] === "bash" && - cmd[1] === "-lc" && - typeof cmd[2] === "string" - ) { - const parsed = parse(cmd[2], process.env ?? {}); - if (parsed.length === 0) { - return null; - } - - let current: Array = []; - let first: SafeCommandReason | null = null; - - const flush = (): boolean => { - if (current.length === 0) { - return true; - } - const safe = isSafeCommand(current); - if (safe == null) { - return false; - } - if (!first) { - first = safe; - } - current = []; - return true; - }; - - for (const part of parsed) { - if (typeof part === "string") { - // Simple word/argument token. - if (part === "(" || part === ")" || part === "{" || part === "}") { - // We treat explicit grouping tokens as unsafe because their - // semantics depend on the shell evaluation environment. - return null; - } - current.push(part); - } else if (part && typeof part === "object") { - const opToken = part as { op?: string }; - if (typeof opToken.op === "string") { - if (!flush()) { - return null; - } - if (!SAFE_SHELL_OPERATORS.has(opToken.op)) { - return null; - } - } else { - // Unknown object token kind (e.g. redirection) – treat as unsafe. - return null; - } - } else { - // Token types such as numbers / booleans are unexpected – treat as unsafe. - return null; - } - } - - if (!flush()) { - return null; - } - - return first; - } - - return null; -} From 7199511a34c1e11006263b5fca9b9cfb8a4fc172 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Apr 2025 15:16:51 -0700 Subject: [PATCH 07/14] Removes computeAutoApproval() and tightens up canAutoApprove() as the source of truth Signed-off-by: Michael Bolin --- codex-cli/src/approvals.ts | 127 ++++++++++-------- .../components/chat/use-message-grouping.ts | 72 ---------- codex-cli/src/utils/agent/review.ts | 8 -- codex-cli/src/utils/parsers.ts | 112 ++------------- 4 files changed, 80 insertions(+), 239 deletions(-) diff --git a/codex-cli/src/approvals.ts b/codex-cli/src/approvals.ts index 0cf3703b54..8a670b01ca 100644 --- a/codex-cli/src/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -75,19 +75,72 @@ export function canAutoApprove( writableRoots: ReadonlyArray, env: NodeJS.ProcessEnv = process.env, ): SafetyAssessment { - try { - if (command[0] === "apply_patch") { - return command.length === 2 && typeof command[1] === "string" - ? canAutoApproveApplyPatch(command[1], writableRoots, policy) - : { - type: "reject", - reason: "Invalid apply_patch command", - }; + if (command[0] === "apply_patch") { + return command.length === 2 && typeof command[1] === "string" + ? canAutoApproveApplyPatch(command[1], writableRoots, policy) + : { + type: "reject", + reason: "Invalid apply_patch command", + }; + } + + const isSafe = isSafeCommand(command); + if (isSafe != null) { + const { reason, group } = isSafe; + return { + type: "auto-approve", + reason, + group, + runInSandbox: false, + }; + } + + if ( + command[0] === "bash" && + command[1] === "-lc" && + typeof command[2] === "string" && + command.length === 3 + ) { + const applyPatchArg = tryParseApplyPatch(command[2]); + if (applyPatchArg != null) { + return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); } - const isSafe = isSafeCommand(command); - if (isSafe != null) { - const { reason, group } = isSafe; + let bashCmd; + try { + bashCmd = parse(command[2], env); + } catch (e) { + // In practice, there seem to be syntactically valid shell commands that + // shell-quote cannot parse, so we should not reject, but ask the user. + switch (policy) { + case "full-auto": + // In full-auto, we still run the command automatically, but must + // restrict it to the sandbox. + return { + type: "auto-approve", + reason: "Full auto mode", + group: "Running commands", + runInSandbox: true, + }; + case "suggest": + case "auto-edit": + // In all other modes, since we cannot reason about the command, we + // should ask the user. + return { + type: "ask-user", + }; + } + } + + // bashCmd could be a mix of strings and operators, e.g.: + // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] + // We try to ensure that *every* command segment is deemed safe and that + // all operators belong to an allow‑list. If so, the entire expression is + // considered auto‑approvable. + + const shellSafe = isEntireShellExpressionSafe(bashCmd); + if (shellSafe != null) { + const { reason, group } = shellSafe; return { type: "auto-approve", reason, @@ -95,58 +148,16 @@ export function canAutoApprove( runInSandbox: false, }; } + } - if ( - command[0] === "bash" && - command[1] === "-lc" && - typeof command[2] === "string" && - command.length === 3 - ) { - const applyPatchArg = tryParseApplyPatch(command[2]); - if (applyPatchArg != null) { - return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); - } - - const bashCmd = parse(command[2], env); - - // bashCmd could be a mix of strings and operators, e.g.: - // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] - // We try to ensure that *every* command segment is deemed safe and that - // all operators belong to an allow‑list. If so, the entire expression is - // considered auto‑approvable. - - const shellSafe = isEntireShellExpressionSafe(bashCmd); - if (shellSafe != null) { - const { reason, group } = shellSafe; - return { - type: "auto-approve", - reason, - group, - runInSandbox: false, - }; - } - } - - return policy === "full-auto" - ? { - type: "auto-approve", - reason: "Full auto mode", - group: "Running commands", - runInSandbox: true, - } - : { type: "ask-user" }; - } catch (err) { - if (policy === "full-auto") { - return { + return policy === "full-auto" + ? { type: "auto-approve", reason: "Full auto mode", group: "Running commands", runInSandbox: true, - }; - } else { - return { type: "ask-user" }; - } - } + } + : { type: "ask-user" }; } function canAutoApproveApplyPatch( diff --git a/codex-cli/src/components/chat/use-message-grouping.ts b/codex-cli/src/components/chat/use-message-grouping.ts index 75e51ac682..1e526821d0 100644 --- a/codex-cli/src/components/chat/use-message-grouping.ts +++ b/codex-cli/src/components/chat/use-message-grouping.ts @@ -1,8 +1,5 @@ import type { ResponseItem } from "openai/resources/responses/responses.mjs"; -import { parseToolCall } from "../../utils/parsers.js"; -import { useMemo } from "react"; - /** * Represents a grouped sequence of response items (e.g., function call batches). */ @@ -10,72 +7,3 @@ export type GroupedResponseItem = { label: string; items: Array; }; - -/** - * Custom hook to group recent response items for display batching. - * Returns counts of auto-approved tool call groups, the latest batch, - * and the count of user messages in the visible window. - */ -export function useMessageGrouping(visibleItems: Array): { - groupCounts: Record; - batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }>; - userMsgCount: number; -} { - return useMemo(() => { - // The grouping logic only depends on the subset of messages that are - // currently rendered (visibleItems). Using that as the sole dependency - // keeps recomputations to a minimum and avoids unnecessary work when the - // full list of `items` changes outside of the visible window. - let userMsgCount = 0; - const groupCounts: Record = {}; - visibleItems.forEach((m) => { - if (m.type === "function_call") { - const toolCall = parseToolCall(m); - if (toolCall?.autoApproval) { - const group = toolCall.autoApproval.group; - groupCounts[group] = (groupCounts[group] || 0) + 1; - } - } - if (m.type === "message" && m.role === "user") { - userMsgCount++; - } - }); - const lastFew = visibleItems.slice(-3); - const batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }> = - []; - if (lastFew[0]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[0]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew, - }, - }); - if (lastFew[2]?.type === "message") { - batch.push({ item: lastFew[2] }); - } - } else if (lastFew[1]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[1]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew.slice(1), - }, - }); - } else if (lastFew[2]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[2]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: [lastFew[2]], - }, - }); - } else { - lastFew.forEach((item) => batch.push({ item })); - } - return { groupCounts, batch, userMsgCount }; - // `items` is stable across renders while `visibleItems` changes based on - // the scroll window. Including only `visibleItems` avoids unnecessary - // recomputations while still producing correct results. - }, [visibleItems]); -} diff --git a/codex-cli/src/utils/agent/review.ts b/codex-cli/src/utils/agent/review.ts index a370388569..9a5e66a4ff 100644 --- a/codex-cli/src/utils/agent/review.ts +++ b/codex-cli/src/utils/agent/review.ts @@ -1,11 +1,3 @@ -import type { SafeCommandReason } from "../../approvals"; - -export type CommandReviewDetails = { - cmd: Array; - cmdReadableText: string; - autoApproval: SafeCommandReason | null; -}; - export enum ReviewDecision { YES = "yes", NO_CONTINUE = "no-continue", diff --git a/codex-cli/src/utils/parsers.ts b/codex-cli/src/utils/parsers.ts index 815e7b2f00..4461379c02 100644 --- a/codex-cli/src/utils/parsers.ts +++ b/codex-cli/src/utils/parsers.ts @@ -1,30 +1,16 @@ -import type { CommandReviewDetails } from "./agent/review.js"; import type { ExecInput, ExecOutputMetadata, } from "./agent/sandbox/interface.js"; import type { ResponseFunctionToolCall } from "openai/resources/responses/responses.mjs"; -import { isSafeCommand, type SafeCommandReason } from "../approvals.js"; import { log } from "node:console"; -import process from "process"; -import { parse } from "shell-quote"; import { formatCommandForDisplay } from "src/format-command.js"; // The console utility import is intentionally explicit to avoid bundlers from // including the entire `console` module when only the `log` function is // required. -// Allowed shell operators that we consider "safe" as they do not introduce -// side‑effects on their own (unlike redirections). Parentheses and braces for -// grouping are excluded for simplicity. -const SAFE_SHELL_OPERATORS: ReadonlySet = new Set([ - "&&", - "||", - "|", - ";", -]); - export function parseToolCallOutput(toolCallOutput: string): { output: string; metadata: ExecOutputMetadata; @@ -46,6 +32,17 @@ export function parseToolCallOutput(toolCallOutput: string): { } } +export type CommandReviewDetails = { + cmd: Array; + cmdReadableText: string; +}; + +/** + * Tries to parse a tool call and, if successful, returns an object that has + * both: + * - an array of strings to use with `ExecInput` and `canAutoApprove()` + * - a human-readable string to display to the user + */ export function parseToolCall( toolCall: ResponseFunctionToolCall, ): CommandReviewDetails | undefined { @@ -57,12 +54,9 @@ export function parseToolCall( const { cmd } = toolCallArgs; const cmdReadableText = formatCommandForDisplay(cmd); - const autoApproval = computeAutoApproval(cmd); - return { cmd, cmdReadableText, - autoApproval, }; } @@ -109,87 +103,3 @@ function toStringArray(obj: unknown): Array | undefined { return undefined; } } - -// ---------------- safe‑command helpers ---------------- - -/** - * Attempts to determine whether `cmd` is composed exclusively of safe - * sub‑commands combined using only operators from the SAFE_SHELL_OPERATORS - * allow‑list. Returns the `SafeCommandReason` (taken from the first sub‑command) - * if the whole expression is safe; otherwise returns `null`. - */ -function computeAutoApproval(cmd: Array): SafeCommandReason | null { - // Fast path: a simple command with no shell processing. - const direct = isSafeCommand(cmd); - if (direct != null) { - return direct; - } - - // For expressions like ["bash", "-lc", "ls && pwd"] break down the inner - // string and verify each segment. - if ( - cmd.length === 3 && - cmd[0] === "bash" && - cmd[1] === "-lc" && - typeof cmd[2] === "string" - ) { - const parsed = parse(cmd[2], process.env ?? {}); - if (parsed.length === 0) { - return null; - } - - let current: Array = []; - let first: SafeCommandReason | null = null; - - const flush = (): boolean => { - if (current.length === 0) { - return true; - } - const safe = isSafeCommand(current); - if (safe == null) { - return false; - } - if (!first) { - first = safe; - } - current = []; - return true; - }; - - for (const part of parsed) { - if (typeof part === "string") { - // Simple word/argument token. - if (part === "(" || part === ")" || part === "{" || part === "}") { - // We treat explicit grouping tokens as unsafe because their - // semantics depend on the shell evaluation environment. - return null; - } - current.push(part); - } else if (part && typeof part === "object") { - const opToken = part as { op?: string }; - if (typeof opToken.op === "string") { - if (!flush()) { - return null; - } - if (!SAFE_SHELL_OPERATORS.has(opToken.op)) { - return null; - } - } else { - // Unknown object token kind (e.g. redirection) – treat as unsafe. - return null; - } - } else { - // Token types such as numbers / booleans are unexpected – treat as unsafe. - return null; - } - } - - if (!flush()) { - return null; - } - - return first; - } - - return null; -} From 08d209f8fa49242bfa7979b3861ff6302550b6f3 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Apr 2025 15:36:25 -0700 Subject: [PATCH 08/14] Removes computeAutoApproval() and tightens up canAutoApprove() as the source of truth Signed-off-by: Michael Bolin --- codex-cli/src/approvals.ts | 127 ++++++++++-------- .../components/chat/use-message-grouping.ts | 72 ---------- codex-cli/src/utils/agent/review.ts | 8 -- codex-cli/src/utils/parsers.ts | 112 ++------------- 4 files changed, 80 insertions(+), 239 deletions(-) diff --git a/codex-cli/src/approvals.ts b/codex-cli/src/approvals.ts index 0cf3703b54..8a670b01ca 100644 --- a/codex-cli/src/approvals.ts +++ b/codex-cli/src/approvals.ts @@ -75,19 +75,72 @@ export function canAutoApprove( writableRoots: ReadonlyArray, env: NodeJS.ProcessEnv = process.env, ): SafetyAssessment { - try { - if (command[0] === "apply_patch") { - return command.length === 2 && typeof command[1] === "string" - ? canAutoApproveApplyPatch(command[1], writableRoots, policy) - : { - type: "reject", - reason: "Invalid apply_patch command", - }; + if (command[0] === "apply_patch") { + return command.length === 2 && typeof command[1] === "string" + ? canAutoApproveApplyPatch(command[1], writableRoots, policy) + : { + type: "reject", + reason: "Invalid apply_patch command", + }; + } + + const isSafe = isSafeCommand(command); + if (isSafe != null) { + const { reason, group } = isSafe; + return { + type: "auto-approve", + reason, + group, + runInSandbox: false, + }; + } + + if ( + command[0] === "bash" && + command[1] === "-lc" && + typeof command[2] === "string" && + command.length === 3 + ) { + const applyPatchArg = tryParseApplyPatch(command[2]); + if (applyPatchArg != null) { + return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); } - const isSafe = isSafeCommand(command); - if (isSafe != null) { - const { reason, group } = isSafe; + let bashCmd; + try { + bashCmd = parse(command[2], env); + } catch (e) { + // In practice, there seem to be syntactically valid shell commands that + // shell-quote cannot parse, so we should not reject, but ask the user. + switch (policy) { + case "full-auto": + // In full-auto, we still run the command automatically, but must + // restrict it to the sandbox. + return { + type: "auto-approve", + reason: "Full auto mode", + group: "Running commands", + runInSandbox: true, + }; + case "suggest": + case "auto-edit": + // In all other modes, since we cannot reason about the command, we + // should ask the user. + return { + type: "ask-user", + }; + } + } + + // bashCmd could be a mix of strings and operators, e.g.: + // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] + // We try to ensure that *every* command segment is deemed safe and that + // all operators belong to an allow‑list. If so, the entire expression is + // considered auto‑approvable. + + const shellSafe = isEntireShellExpressionSafe(bashCmd); + if (shellSafe != null) { + const { reason, group } = shellSafe; return { type: "auto-approve", reason, @@ -95,58 +148,16 @@ export function canAutoApprove( runInSandbox: false, }; } + } - if ( - command[0] === "bash" && - command[1] === "-lc" && - typeof command[2] === "string" && - command.length === 3 - ) { - const applyPatchArg = tryParseApplyPatch(command[2]); - if (applyPatchArg != null) { - return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy); - } - - const bashCmd = parse(command[2], env); - - // bashCmd could be a mix of strings and operators, e.g.: - // "ls || (true && pwd)" => [ 'ls', { op: '||' }, '(', 'true', { op: '&&' }, 'pwd', ')' ] - // We try to ensure that *every* command segment is deemed safe and that - // all operators belong to an allow‑list. If so, the entire expression is - // considered auto‑approvable. - - const shellSafe = isEntireShellExpressionSafe(bashCmd); - if (shellSafe != null) { - const { reason, group } = shellSafe; - return { - type: "auto-approve", - reason, - group, - runInSandbox: false, - }; - } - } - - return policy === "full-auto" - ? { - type: "auto-approve", - reason: "Full auto mode", - group: "Running commands", - runInSandbox: true, - } - : { type: "ask-user" }; - } catch (err) { - if (policy === "full-auto") { - return { + return policy === "full-auto" + ? { type: "auto-approve", reason: "Full auto mode", group: "Running commands", runInSandbox: true, - }; - } else { - return { type: "ask-user" }; - } - } + } + : { type: "ask-user" }; } function canAutoApproveApplyPatch( diff --git a/codex-cli/src/components/chat/use-message-grouping.ts b/codex-cli/src/components/chat/use-message-grouping.ts index 75e51ac682..1e526821d0 100644 --- a/codex-cli/src/components/chat/use-message-grouping.ts +++ b/codex-cli/src/components/chat/use-message-grouping.ts @@ -1,8 +1,5 @@ import type { ResponseItem } from "openai/resources/responses/responses.mjs"; -import { parseToolCall } from "../../utils/parsers.js"; -import { useMemo } from "react"; - /** * Represents a grouped sequence of response items (e.g., function call batches). */ @@ -10,72 +7,3 @@ export type GroupedResponseItem = { label: string; items: Array; }; - -/** - * Custom hook to group recent response items for display batching. - * Returns counts of auto-approved tool call groups, the latest batch, - * and the count of user messages in the visible window. - */ -export function useMessageGrouping(visibleItems: Array): { - groupCounts: Record; - batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }>; - userMsgCount: number; -} { - return useMemo(() => { - // The grouping logic only depends on the subset of messages that are - // currently rendered (visibleItems). Using that as the sole dependency - // keeps recomputations to a minimum and avoids unnecessary work when the - // full list of `items` changes outside of the visible window. - let userMsgCount = 0; - const groupCounts: Record = {}; - visibleItems.forEach((m) => { - if (m.type === "function_call") { - const toolCall = parseToolCall(m); - if (toolCall?.autoApproval) { - const group = toolCall.autoApproval.group; - groupCounts[group] = (groupCounts[group] || 0) + 1; - } - } - if (m.type === "message" && m.role === "user") { - userMsgCount++; - } - }); - const lastFew = visibleItems.slice(-3); - const batch: Array<{ item?: ResponseItem; group?: GroupedResponseItem }> = - []; - if (lastFew[0]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[0]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew, - }, - }); - if (lastFew[2]?.type === "message") { - batch.push({ item: lastFew[2] }); - } - } else if (lastFew[1]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[1]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: lastFew.slice(1), - }, - }); - } else if (lastFew[2]?.type === "function_call") { - const toolCall = parseToolCall(lastFew[2]); - batch.push({ - group: { - label: toolCall?.autoApproval?.group || "Running command", - items: [lastFew[2]], - }, - }); - } else { - lastFew.forEach((item) => batch.push({ item })); - } - return { groupCounts, batch, userMsgCount }; - // `items` is stable across renders while `visibleItems` changes based on - // the scroll window. Including only `visibleItems` avoids unnecessary - // recomputations while still producing correct results. - }, [visibleItems]); -} diff --git a/codex-cli/src/utils/agent/review.ts b/codex-cli/src/utils/agent/review.ts index a370388569..9a5e66a4ff 100644 --- a/codex-cli/src/utils/agent/review.ts +++ b/codex-cli/src/utils/agent/review.ts @@ -1,11 +1,3 @@ -import type { SafeCommandReason } from "../../approvals"; - -export type CommandReviewDetails = { - cmd: Array; - cmdReadableText: string; - autoApproval: SafeCommandReason | null; -}; - export enum ReviewDecision { YES = "yes", NO_CONTINUE = "no-continue", diff --git a/codex-cli/src/utils/parsers.ts b/codex-cli/src/utils/parsers.ts index 815e7b2f00..4461379c02 100644 --- a/codex-cli/src/utils/parsers.ts +++ b/codex-cli/src/utils/parsers.ts @@ -1,30 +1,16 @@ -import type { CommandReviewDetails } from "./agent/review.js"; import type { ExecInput, ExecOutputMetadata, } from "./agent/sandbox/interface.js"; import type { ResponseFunctionToolCall } from "openai/resources/responses/responses.mjs"; -import { isSafeCommand, type SafeCommandReason } from "../approvals.js"; import { log } from "node:console"; -import process from "process"; -import { parse } from "shell-quote"; import { formatCommandForDisplay } from "src/format-command.js"; // The console utility import is intentionally explicit to avoid bundlers from // including the entire `console` module when only the `log` function is // required. -// Allowed shell operators that we consider "safe" as they do not introduce -// side‑effects on their own (unlike redirections). Parentheses and braces for -// grouping are excluded for simplicity. -const SAFE_SHELL_OPERATORS: ReadonlySet = new Set([ - "&&", - "||", - "|", - ";", -]); - export function parseToolCallOutput(toolCallOutput: string): { output: string; metadata: ExecOutputMetadata; @@ -46,6 +32,17 @@ export function parseToolCallOutput(toolCallOutput: string): { } } +export type CommandReviewDetails = { + cmd: Array; + cmdReadableText: string; +}; + +/** + * Tries to parse a tool call and, if successful, returns an object that has + * both: + * - an array of strings to use with `ExecInput` and `canAutoApprove()` + * - a human-readable string to display to the user + */ export function parseToolCall( toolCall: ResponseFunctionToolCall, ): CommandReviewDetails | undefined { @@ -57,12 +54,9 @@ export function parseToolCall( const { cmd } = toolCallArgs; const cmdReadableText = formatCommandForDisplay(cmd); - const autoApproval = computeAutoApproval(cmd); - return { cmd, cmdReadableText, - autoApproval, }; } @@ -109,87 +103,3 @@ function toStringArray(obj: unknown): Array | undefined { return undefined; } } - -// ---------------- safe‑command helpers ---------------- - -/** - * Attempts to determine whether `cmd` is composed exclusively of safe - * sub‑commands combined using only operators from the SAFE_SHELL_OPERATORS - * allow‑list. Returns the `SafeCommandReason` (taken from the first sub‑command) - * if the whole expression is safe; otherwise returns `null`. - */ -function computeAutoApproval(cmd: Array): SafeCommandReason | null { - // Fast path: a simple command with no shell processing. - const direct = isSafeCommand(cmd); - if (direct != null) { - return direct; - } - - // For expressions like ["bash", "-lc", "ls && pwd"] break down the inner - // string and verify each segment. - if ( - cmd.length === 3 && - cmd[0] === "bash" && - cmd[1] === "-lc" && - typeof cmd[2] === "string" - ) { - const parsed = parse(cmd[2], process.env ?? {}); - if (parsed.length === 0) { - return null; - } - - let current: Array = []; - let first: SafeCommandReason | null = null; - - const flush = (): boolean => { - if (current.length === 0) { - return true; - } - const safe = isSafeCommand(current); - if (safe == null) { - return false; - } - if (!first) { - first = safe; - } - current = []; - return true; - }; - - for (const part of parsed) { - if (typeof part === "string") { - // Simple word/argument token. - if (part === "(" || part === ")" || part === "{" || part === "}") { - // We treat explicit grouping tokens as unsafe because their - // semantics depend on the shell evaluation environment. - return null; - } - current.push(part); - } else if (part && typeof part === "object") { - const opToken = part as { op?: string }; - if (typeof opToken.op === "string") { - if (!flush()) { - return null; - } - if (!SAFE_SHELL_OPERATORS.has(opToken.op)) { - return null; - } - } else { - // Unknown object token kind (e.g. redirection) – treat as unsafe. - return null; - } - } else { - // Token types such as numbers / booleans are unexpected – treat as unsafe. - return null; - } - } - - if (!flush()) { - return null; - } - - return first; - } - - return null; -} From 832b98a5577d08106f242668faea4fd9c3192f5e Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 19 Apr 2025 09:52:49 -0700 Subject: [PATCH 09/14] use spawn instead of exec to avoid injection vulnerability --- .../src/components/chat/terminal-chat.tsx | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/codex-cli/src/components/chat/terminal-chat.tsx b/codex-cli/src/components/chat/terminal-chat.tsx index 298d9208e2..f228ed19c4 100644 --- a/codex-cli/src/components/chat/terminal-chat.tsx +++ b/codex-cli/src/components/chat/terminal-chat.tsx @@ -29,7 +29,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"; @@ -300,15 +300,10 @@ export default function TerminalChat({ agentRef.current = undefined; forceUpdate(); // re‑render after teardown too }; - // We intentionally omit 'approvalPolicy' and 'confirmationPrompt' from the deps - // so switching modes or showing confirmation dialogs doesn’t tear down the loop. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ - model, - config, - requestConfirmation, - additionalWritableRoots, - ]); + // We intentionally omit 'approvalPolicy' and 'confirmationPrompt' from the deps + // so switching modes or showing confirmation dialogs doesn’t tear down the loop. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [model, config, requestConfirmation, additionalWritableRoots]); // whenever loading starts/stops, reset or start a timer — but pause the // timer while a confirmation overlay is displayed so we don't trigger a @@ -369,9 +364,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"`, + ]); } } } @@ -597,7 +593,11 @@ export default function TerminalChat({ setApprovalPolicy(newMode as ApprovalPolicy); // update existing AgentLoop instance if (agentRef.current) { - (agentRef.current as unknown as { approvalPolicy: ApprovalPolicy }).approvalPolicy = newMode as ApprovalPolicy; + ( + agentRef.current as unknown as { + approvalPolicy: ApprovalPolicy; + } + ).approvalPolicy = newMode as ApprovalPolicy; } setItems((prev) => [ ...prev, From 66ae9fd234f3d5448ec74a8a381e67736f61af44 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 19 Apr 2025 10:01:18 -0700 Subject: [PATCH 10/14] re-enable Prettier check for codex-cli in CI --- .github/workflows/ci.yml | 6 +- .../chat/terminal-chat-command-review.tsx | 79 ++++++++++--------- codex-cli/tests/input-utils.test.ts | 10 ++- 3 files changed, 52 insertions(+), 43 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 695190c4c9..378aea50c6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,7 +44,11 @@ jobs: # Run all tasks using workspace filters - - name: Check formatting + - name: Check TypeScript code formatting. + working-directory: codex-cli + run: pnpm run format + + - name: Check Markdown and config file formatting. run: pnpm run format - name: Run tests diff --git a/codex-cli/src/components/chat/terminal-chat-command-review.tsx b/codex-cli/src/components/chat/terminal-chat-command-review.tsx index eadb9071ae..912af97961 100644 --- a/codex-cli/src/components/chat/terminal-chat-command-review.tsx +++ b/codex-cli/src/components/chat/terminal-chat-command-review.tsx @@ -121,46 +121,47 @@ export function TerminalChatCommandReview({ useInput( (input, key) => { - if (mode === "select") { - if (input === "y") { - onReviewCommand(ReviewDecision.YES); - } else if (input === "x") { - onReviewCommand(ReviewDecision.EXPLAIN); - } else if (input === "e") { - setMode("input"); - } else if (input === "n") { - onReviewCommand( - ReviewDecision.NO_CONTINUE, - "Don't do that, keep going though", - ); - } else if (input === "a" && showAlwaysApprove) { - onReviewCommand(ReviewDecision.ALWAYS); - } else if (input === "s") { - // switch approval mode - onSwitchApprovalMode(); - } else if (key.escape) { - onReviewCommand(ReviewDecision.NO_EXIT); + if (mode === "select") { + if (input === "y") { + onReviewCommand(ReviewDecision.YES); + } else if (input === "x") { + onReviewCommand(ReviewDecision.EXPLAIN); + } else if (input === "e") { + setMode("input"); + } else if (input === "n") { + onReviewCommand( + ReviewDecision.NO_CONTINUE, + "Don't do that, keep going though", + ); + } else if (input === "a" && showAlwaysApprove) { + onReviewCommand(ReviewDecision.ALWAYS); + } else if (input === "s") { + // switch approval mode + onSwitchApprovalMode(); + } else if (key.escape) { + onReviewCommand(ReviewDecision.NO_EXIT); + } + } else if (mode === "explanation") { + // When in explanation mode, any key returns to select mode + if (key.return || key.escape || input === "x") { + setMode("select"); + } + } else { + // text entry mode + if (key.return) { + // if user hit enter on empty msg, fall back to DEFAULT_DENY_MESSAGE + const custom = msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg; + onReviewCommand(ReviewDecision.NO_CONTINUE, custom); + } else if (key.escape) { + // treat escape as denial with default message as well + onReviewCommand( + ReviewDecision.NO_CONTINUE, + msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg, + ); + } } - } else if (mode === "explanation") { - // When in explanation mode, any key returns to select mode - if (key.return || key.escape || input === "x") { - setMode("select"); - } - } else { - // text entry mode - if (key.return) { - // if user hit enter on empty msg, fall back to DEFAULT_DENY_MESSAGE - const custom = msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg; - onReviewCommand(ReviewDecision.NO_CONTINUE, custom); - } else if (key.escape) { - // treat escape as denial with default message as well - onReviewCommand( - ReviewDecision.NO_CONTINUE, - msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg, - ); - } - } - }, { isActive } + }, + { isActive }, ); return ( diff --git a/codex-cli/tests/input-utils.test.ts b/codex-cli/tests/input-utils.test.ts index 5290e55488..780a384240 100644 --- a/codex-cli/tests/input-utils.test.ts +++ b/codex-cli/tests/input-utils.test.ts @@ -14,9 +14,13 @@ describe("createInputItem", () => { it("includes image content for existing file", async () => { const fakeBuffer = Buffer.from("fake image content"); - const readSpy = vi.spyOn(fs, "readFile").mockResolvedValue(fakeBuffer as any); + const readSpy = vi + .spyOn(fs, "readFile") + .mockResolvedValue(fakeBuffer as any); const result = await createInputItem("hello", ["dummy-path"]); - const expectedUrl = `data:application/octet-stream;base64,${fakeBuffer.toString("base64")}`; + const expectedUrl = `data:application/octet-stream;base64,${fakeBuffer.toString( + "base64", + )}`; expect(result.role).toBe("user"); expect(result.type).toBe("message"); expect(result.content.length).toBe(2); @@ -40,4 +44,4 @@ describe("createInputItem", () => { text: "[missing image: does-not-exist.png]", }); }); -}); \ No newline at end of file +}); From ede10c12476349cf070b07aeecb239db7e81d646 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 19 Apr 2025 10:01:41 -0700 Subject: [PATCH 11/14] re-enable Prettier check for codex-cli in CI --- .github/workflows/ci.yml | 6 +- .../chat/terminal-chat-command-review.tsx | 79 ++++++++++--------- codex-cli/tests/input-utils.test.ts | 10 ++- 3 files changed, 52 insertions(+), 43 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 695190c4c9..378aea50c6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,7 +44,11 @@ jobs: # Run all tasks using workspace filters - - name: Check formatting + - name: Check TypeScript code formatting. + working-directory: codex-cli + run: pnpm run format + + - name: Check Markdown and config file formatting. run: pnpm run format - name: Run tests diff --git a/codex-cli/src/components/chat/terminal-chat-command-review.tsx b/codex-cli/src/components/chat/terminal-chat-command-review.tsx index eadb9071ae..912af97961 100644 --- a/codex-cli/src/components/chat/terminal-chat-command-review.tsx +++ b/codex-cli/src/components/chat/terminal-chat-command-review.tsx @@ -121,46 +121,47 @@ export function TerminalChatCommandReview({ useInput( (input, key) => { - if (mode === "select") { - if (input === "y") { - onReviewCommand(ReviewDecision.YES); - } else if (input === "x") { - onReviewCommand(ReviewDecision.EXPLAIN); - } else if (input === "e") { - setMode("input"); - } else if (input === "n") { - onReviewCommand( - ReviewDecision.NO_CONTINUE, - "Don't do that, keep going though", - ); - } else if (input === "a" && showAlwaysApprove) { - onReviewCommand(ReviewDecision.ALWAYS); - } else if (input === "s") { - // switch approval mode - onSwitchApprovalMode(); - } else if (key.escape) { - onReviewCommand(ReviewDecision.NO_EXIT); + if (mode === "select") { + if (input === "y") { + onReviewCommand(ReviewDecision.YES); + } else if (input === "x") { + onReviewCommand(ReviewDecision.EXPLAIN); + } else if (input === "e") { + setMode("input"); + } else if (input === "n") { + onReviewCommand( + ReviewDecision.NO_CONTINUE, + "Don't do that, keep going though", + ); + } else if (input === "a" && showAlwaysApprove) { + onReviewCommand(ReviewDecision.ALWAYS); + } else if (input === "s") { + // switch approval mode + onSwitchApprovalMode(); + } else if (key.escape) { + onReviewCommand(ReviewDecision.NO_EXIT); + } + } else if (mode === "explanation") { + // When in explanation mode, any key returns to select mode + if (key.return || key.escape || input === "x") { + setMode("select"); + } + } else { + // text entry mode + if (key.return) { + // if user hit enter on empty msg, fall back to DEFAULT_DENY_MESSAGE + const custom = msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg; + onReviewCommand(ReviewDecision.NO_CONTINUE, custom); + } else if (key.escape) { + // treat escape as denial with default message as well + onReviewCommand( + ReviewDecision.NO_CONTINUE, + msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg, + ); + } } - } else if (mode === "explanation") { - // When in explanation mode, any key returns to select mode - if (key.return || key.escape || input === "x") { - setMode("select"); - } - } else { - // text entry mode - if (key.return) { - // if user hit enter on empty msg, fall back to DEFAULT_DENY_MESSAGE - const custom = msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg; - onReviewCommand(ReviewDecision.NO_CONTINUE, custom); - } else if (key.escape) { - // treat escape as denial with default message as well - onReviewCommand( - ReviewDecision.NO_CONTINUE, - msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg, - ); - } - } - }, { isActive } + }, + { isActive }, ); return ( diff --git a/codex-cli/tests/input-utils.test.ts b/codex-cli/tests/input-utils.test.ts index 5290e55488..780a384240 100644 --- a/codex-cli/tests/input-utils.test.ts +++ b/codex-cli/tests/input-utils.test.ts @@ -14,9 +14,13 @@ describe("createInputItem", () => { it("includes image content for existing file", async () => { const fakeBuffer = Buffer.from("fake image content"); - const readSpy = vi.spyOn(fs, "readFile").mockResolvedValue(fakeBuffer as any); + const readSpy = vi + .spyOn(fs, "readFile") + .mockResolvedValue(fakeBuffer as any); const result = await createInputItem("hello", ["dummy-path"]); - const expectedUrl = `data:application/octet-stream;base64,${fakeBuffer.toString("base64")}`; + const expectedUrl = `data:application/octet-stream;base64,${fakeBuffer.toString( + "base64", + )}`; expect(result.role).toBe("user"); expect(result.type).toBe("message"); expect(result.content.length).toBe(2); @@ -40,4 +44,4 @@ describe("createInputItem", () => { text: "[missing image: does-not-exist.png]", }); }); -}); \ No newline at end of file +}); From be9d216aabc0cb62e1d965b428996f50d81ca7db Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 19 Apr 2025 10:01:41 -0700 Subject: [PATCH 12/14] re-enable Prettier check for codex-cli in CI --- .github/workflows/ci.yml | 6 +- .../chat/terminal-chat-command-review.tsx | 79 ++++++++++--------- .../src/components/chat/terminal-chat.tsx | 19 +++-- codex-cli/tests/input-utils.test.ts | 10 ++- 4 files changed, 61 insertions(+), 53 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 695190c4c9..378aea50c6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,7 +44,11 @@ jobs: # Run all tasks using workspace filters - - name: Check formatting + - name: Check TypeScript code formatting. + working-directory: codex-cli + run: pnpm run format + + - name: Check Markdown and config file formatting. run: pnpm run format - name: Run tests diff --git a/codex-cli/src/components/chat/terminal-chat-command-review.tsx b/codex-cli/src/components/chat/terminal-chat-command-review.tsx index eadb9071ae..912af97961 100644 --- a/codex-cli/src/components/chat/terminal-chat-command-review.tsx +++ b/codex-cli/src/components/chat/terminal-chat-command-review.tsx @@ -121,46 +121,47 @@ export function TerminalChatCommandReview({ useInput( (input, key) => { - if (mode === "select") { - if (input === "y") { - onReviewCommand(ReviewDecision.YES); - } else if (input === "x") { - onReviewCommand(ReviewDecision.EXPLAIN); - } else if (input === "e") { - setMode("input"); - } else if (input === "n") { - onReviewCommand( - ReviewDecision.NO_CONTINUE, - "Don't do that, keep going though", - ); - } else if (input === "a" && showAlwaysApprove) { - onReviewCommand(ReviewDecision.ALWAYS); - } else if (input === "s") { - // switch approval mode - onSwitchApprovalMode(); - } else if (key.escape) { - onReviewCommand(ReviewDecision.NO_EXIT); + if (mode === "select") { + if (input === "y") { + onReviewCommand(ReviewDecision.YES); + } else if (input === "x") { + onReviewCommand(ReviewDecision.EXPLAIN); + } else if (input === "e") { + setMode("input"); + } else if (input === "n") { + onReviewCommand( + ReviewDecision.NO_CONTINUE, + "Don't do that, keep going though", + ); + } else if (input === "a" && showAlwaysApprove) { + onReviewCommand(ReviewDecision.ALWAYS); + } else if (input === "s") { + // switch approval mode + onSwitchApprovalMode(); + } else if (key.escape) { + onReviewCommand(ReviewDecision.NO_EXIT); + } + } else if (mode === "explanation") { + // When in explanation mode, any key returns to select mode + if (key.return || key.escape || input === "x") { + setMode("select"); + } + } else { + // text entry mode + if (key.return) { + // if user hit enter on empty msg, fall back to DEFAULT_DENY_MESSAGE + const custom = msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg; + onReviewCommand(ReviewDecision.NO_CONTINUE, custom); + } else if (key.escape) { + // treat escape as denial with default message as well + onReviewCommand( + ReviewDecision.NO_CONTINUE, + msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg, + ); + } } - } else if (mode === "explanation") { - // When in explanation mode, any key returns to select mode - if (key.return || key.escape || input === "x") { - setMode("select"); - } - } else { - // text entry mode - if (key.return) { - // if user hit enter on empty msg, fall back to DEFAULT_DENY_MESSAGE - const custom = msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg; - onReviewCommand(ReviewDecision.NO_CONTINUE, custom); - } else if (key.escape) { - // treat escape as denial with default message as well - onReviewCommand( - ReviewDecision.NO_CONTINUE, - msg.trim() === "" ? DEFAULT_DENY_MESSAGE : msg, - ); - } - } - }, { isActive } + }, + { isActive }, ); return ( diff --git a/codex-cli/src/components/chat/terminal-chat.tsx b/codex-cli/src/components/chat/terminal-chat.tsx index 298d9208e2..e341cdfbbb 100644 --- a/codex-cli/src/components/chat/terminal-chat.tsx +++ b/codex-cli/src/components/chat/terminal-chat.tsx @@ -300,15 +300,10 @@ export default function TerminalChat({ agentRef.current = undefined; forceUpdate(); // re‑render after teardown too }; - // We intentionally omit 'approvalPolicy' and 'confirmationPrompt' from the deps - // so switching modes or showing confirmation dialogs doesn’t tear down the loop. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [ - model, - config, - requestConfirmation, - additionalWritableRoots, - ]); + // We intentionally omit 'approvalPolicy' and 'confirmationPrompt' from the deps + // so switching modes or showing confirmation dialogs doesn’t tear down the loop. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [model, config, requestConfirmation, additionalWritableRoots]); // whenever loading starts/stops, reset or start a timer — but pause the // timer while a confirmation overlay is displayed so we don't trigger a @@ -597,7 +592,11 @@ export default function TerminalChat({ setApprovalPolicy(newMode as ApprovalPolicy); // update existing AgentLoop instance if (agentRef.current) { - (agentRef.current as unknown as { approvalPolicy: ApprovalPolicy }).approvalPolicy = newMode as ApprovalPolicy; + ( + agentRef.current as unknown as { + approvalPolicy: ApprovalPolicy; + } + ).approvalPolicy = newMode as ApprovalPolicy; } setItems((prev) => [ ...prev, diff --git a/codex-cli/tests/input-utils.test.ts b/codex-cli/tests/input-utils.test.ts index 5290e55488..780a384240 100644 --- a/codex-cli/tests/input-utils.test.ts +++ b/codex-cli/tests/input-utils.test.ts @@ -14,9 +14,13 @@ describe("createInputItem", () => { it("includes image content for existing file", async () => { const fakeBuffer = Buffer.from("fake image content"); - const readSpy = vi.spyOn(fs, "readFile").mockResolvedValue(fakeBuffer as any); + const readSpy = vi + .spyOn(fs, "readFile") + .mockResolvedValue(fakeBuffer as any); const result = await createInputItem("hello", ["dummy-path"]); - const expectedUrl = `data:application/octet-stream;base64,${fakeBuffer.toString("base64")}`; + const expectedUrl = `data:application/octet-stream;base64,${fakeBuffer.toString( + "base64", + )}`; expect(result.role).toBe("user"); expect(result.type).toBe("message"); expect(result.content.length).toBe(2); @@ -40,4 +44,4 @@ describe("createInputItem", () => { text: "[missing image: does-not-exist.png]", }); }); -}); \ No newline at end of file +}); From 19a43ad15b679721d3c33aa01f4f04dd3bf2797c Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 19 Apr 2025 10:03:17 -0700 Subject: [PATCH 13/14] use spawn instead of exec to avoid injection vulnerability --- 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 e341cdfbbb..f228ed19c4 100644 --- a/codex-cli/src/components/chat/terminal-chat.tsx +++ b/codex-cli/src/components/chat/terminal-chat.tsx @@ -29,7 +29,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"; @@ -364,9 +364,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 3bbeba28ce0617a5b2d0e53eb96792fe93a5d3f6 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 19 Apr 2025 10:26:23 -0700 Subject: [PATCH 14/14] CONFIG_DIR should not be in the list of writable roots by default --- .../src/utils/agent/sandbox/macos-seatbelt.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 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, opts: SpawnOptions, - writableRoots: Array, + writableRoots: ReadonlyArray, abortSignal?: AbortSignal, ): Promise { let scopedWritePolicy: string; let policyTemplateParams: Array; - 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}`,