From 033d379ecad723fd9ef9d67e24d08ec66f6343ff Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 30 Apr 2025 14:08:27 -0700 Subject: [PATCH] fix: remove unused _writableRoots arg to exec() function (#762) I suspect this was done originally so that `execForSandbox()` had a consistent signature for both the `SandboxType.NONE` and `SandboxType.MACOS_SEATBELT` cases, but that is not really necessary and turns out to make the upcoming Landlock support a bit more complicated to implement, so I had Codex remove it and clean up the call sites. --- codex-cli/src/utils/agent/exec.ts | 10 ++++++---- codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts | 2 +- codex-cli/src/utils/agent/sandbox/raw-exec.ts | 1 - codex-cli/tests/cancel-exec.test.ts | 4 ++-- codex-cli/tests/invalid-command-handling.test.ts | 2 +- codex-cli/tests/raw-exec-process-group.test.ts | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/codex-cli/src/utils/agent/exec.ts b/codex-cli/src/utils/agent/exec.ts index 9c763ef5..3a0e653d 100644 --- a/codex-cli/src/utils/agent/exec.ts +++ b/codex-cli/src/utils/agent/exec.ts @@ -45,9 +45,6 @@ export function exec( // This is a temporary measure to understand what are the common base commands // until we start persisting and uploading rollouts - const execForSandbox = - sandbox === SandboxType.MACOS_SEATBELT ? execWithSeatbelt : rawExec; - const opts: SpawnOptions = { timeout: timeoutInMillis || DEFAULT_TIMEOUT_MS, ...(requiresShell(cmd) ? { shell: true } : {}), @@ -59,7 +56,12 @@ export function exec( os.tmpdir(), ...additionalWritableRoots, ]; - return execForSandbox(cmd, opts, writableRoots, abortSignal); + if (sandbox === SandboxType.MACOS_SEATBELT) { + return execWithSeatbelt(cmd, opts, writableRoots, abortSignal); + } + + // SandboxType.NONE (or any other) falls back to the raw exec implementation + return rawExec(cmd, opts, abortSignal); } export function execApplyPatch( diff --git a/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts b/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts index a01e2c63..af6664b1 100644 --- a/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts +++ b/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts @@ -72,7 +72,7 @@ export function execWithSeatbelt( "--", ...cmd, ]; - return exec(fullCommand, opts, writableRoots, abortSignal); + return exec(fullCommand, opts, abortSignal); } const READ_ONLY_SEATBELT_POLICY = ` diff --git a/codex-cli/src/utils/agent/sandbox/raw-exec.ts b/codex-cli/src/utils/agent/sandbox/raw-exec.ts index b33feb85..02d3768f 100644 --- a/codex-cli/src/utils/agent/sandbox/raw-exec.ts +++ b/codex-cli/src/utils/agent/sandbox/raw-exec.ts @@ -20,7 +20,6 @@ import * as os from "os"; export function exec( command: Array, options: SpawnOptions, - _writableRoots: ReadonlyArray, abortSignal?: AbortSignal, ): Promise { // Adapt command for the current platform (e.g., convert 'ls' to 'dir' on Windows) diff --git a/codex-cli/tests/cancel-exec.test.ts b/codex-cli/tests/cancel-exec.test.ts index 021e889e..c65b1bbc 100644 --- a/codex-cli/tests/cancel-exec.test.ts +++ b/codex-cli/tests/cancel-exec.test.ts @@ -14,7 +14,7 @@ describe("exec cancellation", () => { const cmd = ["node", "-e", "setTimeout(() => console.log('late'), 5000);"]; const start = Date.now(); - const promise = rawExec(cmd, {}, [], abortController.signal); + const promise = rawExec(cmd, {}, abortController.signal); // Abort almost immediately. abortController.abort(); @@ -38,7 +38,7 @@ describe("exec cancellation", () => { const cmd = ["node", "-e", "console.log('finished')"]; - const result = await rawExec(cmd, {}, [], abortController.signal); + const result = await rawExec(cmd, {}, abortController.signal); expect(result.exitCode).toBe(0); expect(result.stdout.trim()).toBe("finished"); diff --git a/codex-cli/tests/invalid-command-handling.test.ts b/codex-cli/tests/invalid-command-handling.test.ts index 556d7023..65b084de 100644 --- a/codex-cli/tests/invalid-command-handling.test.ts +++ b/codex-cli/tests/invalid-command-handling.test.ts @@ -10,7 +10,7 @@ describe("rawExec – invalid command handling", () => { it("resolves with non‑zero exit code when executable is missing", async () => { const cmd = ["definitely-not-a-command-1234567890"]; - const result = await rawExec(cmd, {}, []); + const result = await rawExec(cmd, {}); expect(result.exitCode).not.toBe(0); expect(result.stderr.length).toBeGreaterThan(0); diff --git a/codex-cli/tests/raw-exec-process-group.test.ts b/codex-cli/tests/raw-exec-process-group.test.ts index 8dfc2821..8aa18432 100644 --- a/codex-cli/tests/raw-exec-process-group.test.ts +++ b/codex-cli/tests/raw-exec-process-group.test.ts @@ -33,7 +33,7 @@ describe("rawExec – abort kills entire process group", () => { // - prints the PID of the `sleep` // - waits for `sleep` to exit const { stdout, exitCode } = await (async () => { - const p = rawExec(cmd, {}, [], abortController.signal); + const p = rawExec(cmd, {}, abortController.signal); // Give Bash a tiny bit of time to start and print the PID. await new Promise((r) => setTimeout(r, 100));