fix: increase output limits for truncating collector (#575)
This Pull Request addresses an issue where the output of commands executed in the raw-exec utility was being truncated due to restrictive limits on the number of lines and bytes collected. The truncation caused the message [Output truncated: too many lines or bytes] to appear when processing large outputs, which could hinder the functionality of the CLI. Changes Made Increased the maximum output limits in the [createTruncatingCollector](https://github.com/openai/codex/pull/575) utility: Bytes: Increased from 10 KB to 100 KB. Lines: Increased from 256 lines to 1024 lines. Installed the @types/node package to resolve missing type definitions for [NodeJS](https://github.com/openai/codex/pull/575) and [Buffer](https://github.com/openai/codex/pull/575). Verified and fixed any related errors in the [createTruncatingCollector](https://github.com/openai/codex/pull/575) implementation. Issue Solved: This PR ensures that larger outputs can be processed without truncation, improving the usability of the CLI for commands that generate extensive output. https://github.com/openai/codex/issues/509 --------- Co-authored-by: Michael Bolin <bolinfest@gmail.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import type { AppConfig } from "../config.js";
|
||||
import type { ExecInput, ExecResult } from "./sandbox/interface.js";
|
||||
import type { SpawnOptions } from "child_process";
|
||||
import type { ParseEntry } from "shell-quote";
|
||||
@@ -41,6 +42,7 @@ export function exec(
|
||||
additionalWritableRoots,
|
||||
}: ExecInput & { additionalWritableRoots: ReadonlyArray<string> },
|
||||
sandbox: SandboxType,
|
||||
config: AppConfig,
|
||||
abortSignal?: AbortSignal,
|
||||
): Promise<ExecResult> {
|
||||
const opts: SpawnOptions = {
|
||||
@@ -52,7 +54,7 @@ export function exec(
|
||||
switch (sandbox) {
|
||||
case SandboxType.NONE: {
|
||||
// SandboxType.NONE uses the raw exec implementation.
|
||||
return rawExec(cmd, opts, abortSignal);
|
||||
return rawExec(cmd, opts, config, abortSignal);
|
||||
}
|
||||
case SandboxType.MACOS_SEATBELT: {
|
||||
// Merge default writable roots with any user-specified ones.
|
||||
@@ -61,10 +63,16 @@ export function exec(
|
||||
os.tmpdir(),
|
||||
...additionalWritableRoots,
|
||||
];
|
||||
return execWithSeatbelt(cmd, opts, writableRoots, abortSignal);
|
||||
return execWithSeatbelt(cmd, opts, writableRoots, config, abortSignal);
|
||||
}
|
||||
case SandboxType.LINUX_LANDLOCK: {
|
||||
return execWithLandlock(cmd, opts, additionalWritableRoots, abortSignal);
|
||||
return execWithLandlock(
|
||||
cmd,
|
||||
opts,
|
||||
additionalWritableRoots,
|
||||
config,
|
||||
abortSignal,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -94,6 +94,7 @@ export async function handleExecCommand(
|
||||
/* applyPatch */ undefined,
|
||||
/* runInSandbox */ false,
|
||||
additionalWritableRoots,
|
||||
config,
|
||||
abortSignal,
|
||||
).then(convertSummaryToResult);
|
||||
}
|
||||
@@ -142,6 +143,7 @@ export async function handleExecCommand(
|
||||
applyPatch,
|
||||
runInSandbox,
|
||||
additionalWritableRoots,
|
||||
config,
|
||||
abortSignal,
|
||||
);
|
||||
// If the operation was aborted in the meantime, propagate the cancellation
|
||||
@@ -179,6 +181,7 @@ export async function handleExecCommand(
|
||||
applyPatch,
|
||||
false,
|
||||
additionalWritableRoots,
|
||||
config,
|
||||
abortSignal,
|
||||
);
|
||||
return convertSummaryToResult(summary);
|
||||
@@ -213,6 +216,7 @@ async function execCommand(
|
||||
applyPatchCommand: ApplyPatchCommand | undefined,
|
||||
runInSandbox: boolean,
|
||||
additionalWritableRoots: ReadonlyArray<string>,
|
||||
config: AppConfig,
|
||||
abortSignal?: AbortSignal,
|
||||
): Promise<ExecCommandSummary> {
|
||||
let { workdir } = execInput;
|
||||
@@ -252,6 +256,7 @@ async function execCommand(
|
||||
: await exec(
|
||||
{ ...execInput, additionalWritableRoots },
|
||||
await getSandbox(runInSandbox),
|
||||
config,
|
||||
abortSignal,
|
||||
);
|
||||
const duration = Date.now() - start;
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
// Maximum output cap: either MAX_OUTPUT_LINES lines or MAX_OUTPUT_BYTES bytes,
|
||||
// whichever limit is reached first.
|
||||
const MAX_OUTPUT_BYTES = 1024 * 10; // 10 KB
|
||||
const MAX_OUTPUT_LINES = 256;
|
||||
import { DEFAULT_SHELL_MAX_BYTES, DEFAULT_SHELL_MAX_LINES } from "../../config";
|
||||
|
||||
/**
|
||||
* Creates a collector that accumulates data Buffers from a stream up to
|
||||
@@ -10,8 +9,8 @@ const MAX_OUTPUT_LINES = 256;
|
||||
*/
|
||||
export function createTruncatingCollector(
|
||||
stream: NodeJS.ReadableStream,
|
||||
byteLimit: number = MAX_OUTPUT_BYTES,
|
||||
lineLimit: number = MAX_OUTPUT_LINES,
|
||||
byteLimit: number = DEFAULT_SHELL_MAX_BYTES,
|
||||
lineLimit: number = DEFAULT_SHELL_MAX_LINES,
|
||||
): {
|
||||
getString: () => string;
|
||||
hit: boolean;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { ExecResult } from "./interface.js";
|
||||
import type { AppConfig } from "../../config.js";
|
||||
import type { SpawnOptions } from "child_process";
|
||||
|
||||
import { exec } from "./raw-exec.js";
|
||||
@@ -19,6 +20,7 @@ export async function execWithLandlock(
|
||||
cmd: Array<string>,
|
||||
opts: SpawnOptions,
|
||||
userProvidedWritableRoots: ReadonlyArray<string>,
|
||||
config: AppConfig,
|
||||
abortSignal?: AbortSignal,
|
||||
): Promise<ExecResult> {
|
||||
const sandboxExecutable = await getSandboxExecutable();
|
||||
@@ -44,7 +46,7 @@ export async function execWithLandlock(
|
||||
...cmd,
|
||||
];
|
||||
|
||||
return exec(fullCommand, opts, abortSignal);
|
||||
return exec(fullCommand, opts, config, abortSignal);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { ExecResult } from "./interface.js";
|
||||
import type { AppConfig } from "../../config.js";
|
||||
import type { SpawnOptions } from "child_process";
|
||||
|
||||
import { exec } from "./raw-exec.js";
|
||||
@@ -24,6 +25,7 @@ export function execWithSeatbelt(
|
||||
cmd: Array<string>,
|
||||
opts: SpawnOptions,
|
||||
writableRoots: ReadonlyArray<string>,
|
||||
config: AppConfig,
|
||||
abortSignal?: AbortSignal,
|
||||
): Promise<ExecResult> {
|
||||
let scopedWritePolicy: string;
|
||||
@@ -72,7 +74,7 @@ export function execWithSeatbelt(
|
||||
"--",
|
||||
...cmd,
|
||||
];
|
||||
return exec(fullCommand, opts, abortSignal);
|
||||
return exec(fullCommand, opts, config, abortSignal);
|
||||
}
|
||||
|
||||
const READ_ONLY_SEATBELT_POLICY = `
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { ExecResult } from "./interface";
|
||||
import type { AppConfig } from "../../config";
|
||||
import type {
|
||||
ChildProcess,
|
||||
SpawnOptions,
|
||||
@@ -20,6 +21,7 @@ import * as os from "os";
|
||||
export function exec(
|
||||
command: Array<string>,
|
||||
options: SpawnOptions,
|
||||
config: AppConfig,
|
||||
abortSignal?: AbortSignal,
|
||||
): Promise<ExecResult> {
|
||||
// Adapt command for the current platform (e.g., convert 'ls' to 'dir' on Windows)
|
||||
@@ -142,9 +144,21 @@ export function exec(
|
||||
// ExecResult object so the rest of the agent loop can carry on gracefully.
|
||||
|
||||
return new Promise<ExecResult>((resolve) => {
|
||||
// Get shell output limits from config if available
|
||||
const maxBytes = config?.tools?.shell?.maxBytes;
|
||||
const maxLines = config?.tools?.shell?.maxLines;
|
||||
|
||||
// Collect stdout and stderr up to configured limits.
|
||||
const stdoutCollector = createTruncatingCollector(child.stdout!);
|
||||
const stderrCollector = createTruncatingCollector(child.stderr!);
|
||||
const stdoutCollector = createTruncatingCollector(
|
||||
child.stdout!,
|
||||
maxBytes,
|
||||
maxLines,
|
||||
);
|
||||
const stderrCollector = createTruncatingCollector(
|
||||
child.stderr!,
|
||||
maxBytes,
|
||||
maxLines,
|
||||
);
|
||||
|
||||
child.on("exit", (code, signal) => {
|
||||
const stdout = stdoutCollector.getString();
|
||||
|
||||
@@ -48,6 +48,10 @@ export const DEFAULT_FULL_CONTEXT_MODEL = "gpt-4.1";
|
||||
export const DEFAULT_APPROVAL_MODE = AutoApprovalMode.SUGGEST;
|
||||
export const DEFAULT_INSTRUCTIONS = "";
|
||||
|
||||
// Default shell output limits
|
||||
export const DEFAULT_SHELL_MAX_BYTES = 1024 * 10; // 10 KB
|
||||
export const DEFAULT_SHELL_MAX_LINES = 256;
|
||||
|
||||
export const CONFIG_DIR = join(homedir(), ".codex");
|
||||
export const CONFIG_JSON_FILEPATH = join(CONFIG_DIR, "config.json");
|
||||
export const CONFIG_YAML_FILEPATH = join(CONFIG_DIR, "config.yaml");
|
||||
@@ -145,6 +149,12 @@ export type StoredConfig = {
|
||||
saveHistory?: boolean;
|
||||
sensitivePatterns?: Array<string>;
|
||||
};
|
||||
tools?: {
|
||||
shell?: {
|
||||
maxBytes?: number;
|
||||
maxLines?: number;
|
||||
};
|
||||
};
|
||||
/** User-defined safe commands */
|
||||
safeCommands?: Array<string>;
|
||||
reasoningEffort?: ReasoningEffort;
|
||||
@@ -186,6 +196,12 @@ export type AppConfig = {
|
||||
saveHistory: boolean;
|
||||
sensitivePatterns: Array<string>;
|
||||
};
|
||||
tools?: {
|
||||
shell?: {
|
||||
maxBytes: number;
|
||||
maxLines: number;
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
// Formatting (quiet mode-only).
|
||||
@@ -388,6 +404,14 @@ export const loadConfig = (
|
||||
instructions: combinedInstructions,
|
||||
notify: storedConfig.notify === true,
|
||||
approvalMode: storedConfig.approvalMode,
|
||||
tools: {
|
||||
shell: {
|
||||
maxBytes:
|
||||
storedConfig.tools?.shell?.maxBytes ?? DEFAULT_SHELL_MAX_BYTES,
|
||||
maxLines:
|
||||
storedConfig.tools?.shell?.maxLines ?? DEFAULT_SHELL_MAX_LINES,
|
||||
},
|
||||
},
|
||||
disableResponseStorage: storedConfig.disableResponseStorage === true,
|
||||
reasoningEffort: storedConfig.reasoningEffort,
|
||||
};
|
||||
@@ -517,6 +541,18 @@ export const saveConfig = (
|
||||
};
|
||||
}
|
||||
|
||||
// Add tools settings if they exist
|
||||
if (config.tools) {
|
||||
configToSave.tools = {
|
||||
shell: config.tools.shell
|
||||
? {
|
||||
maxBytes: config.tools.shell.maxBytes,
|
||||
maxLines: config.tools.shell.maxLines,
|
||||
}
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
|
||||
if (ext === ".yaml" || ext === ".yml") {
|
||||
writeFileSync(targetPath, dumpYaml(configToSave), "utf-8");
|
||||
} else {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { exec as rawExec } from "../src/utils/agent/sandbox/raw-exec.js";
|
||||
import { describe, it, expect } from "vitest";
|
||||
import type { AppConfig } from "src/utils/config.js";
|
||||
|
||||
// Import the low‑level exec implementation so we can verify that AbortSignal
|
||||
// correctly terminates a spawned process. We bypass the higher‑level wrappers
|
||||
@@ -12,9 +13,13 @@ describe("exec cancellation", () => {
|
||||
// Spawn a node process that would normally run for 5 seconds before
|
||||
// printing anything. We should abort long before that happens.
|
||||
const cmd = ["node", "-e", "setTimeout(() => console.log('late'), 5000);"];
|
||||
|
||||
const config: AppConfig = {
|
||||
model: "test-model",
|
||||
instructions: "test-instructions",
|
||||
};
|
||||
const start = Date.now();
|
||||
const promise = rawExec(cmd, {}, abortController.signal);
|
||||
|
||||
const promise = rawExec(cmd, {}, config, abortController.signal);
|
||||
|
||||
// Abort almost immediately.
|
||||
abortController.abort();
|
||||
@@ -36,9 +41,14 @@ describe("exec cancellation", () => {
|
||||
it("allows the process to finish when not aborted", async () => {
|
||||
const abortController = new AbortController();
|
||||
|
||||
const config: AppConfig = {
|
||||
model: "test-model",
|
||||
instructions: "test-instructions",
|
||||
};
|
||||
|
||||
const cmd = ["node", "-e", "console.log('finished')"];
|
||||
|
||||
const result = await rawExec(cmd, {}, abortController.signal);
|
||||
const result = await rawExec(cmd, {}, config, abortController.signal);
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stdout.trim()).toBe("finished");
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import type * as fsType from "fs";
|
||||
|
||||
import { loadConfig, saveConfig } from "../src/utils/config.js"; // parent import first
|
||||
import {
|
||||
loadConfig,
|
||||
saveConfig,
|
||||
DEFAULT_SHELL_MAX_BYTES,
|
||||
DEFAULT_SHELL_MAX_LINES,
|
||||
} from "../src/utils/config.js";
|
||||
import { AutoApprovalMode } from "../src/utils/auto-approval-mode.js";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
@@ -275,3 +280,84 @@ test("handles empty user instructions when saving with project doc separator", (
|
||||
});
|
||||
expect(loadedConfig.instructions).toBe("");
|
||||
});
|
||||
|
||||
test("loads default shell config when not specified", () => {
|
||||
// Setup config without shell settings
|
||||
memfs[testConfigPath] = JSON.stringify(
|
||||
{
|
||||
model: "mymodel",
|
||||
},
|
||||
null,
|
||||
2,
|
||||
);
|
||||
memfs[testInstructionsPath] = "test instructions";
|
||||
|
||||
// Load config and verify default shell settings
|
||||
const loadedConfig = loadConfig(testConfigPath, testInstructionsPath, {
|
||||
disableProjectDoc: true,
|
||||
});
|
||||
|
||||
// Check shell settings were loaded with defaults
|
||||
expect(loadedConfig.tools).toBeDefined();
|
||||
expect(loadedConfig.tools?.shell).toBeDefined();
|
||||
expect(loadedConfig.tools?.shell?.maxBytes).toBe(DEFAULT_SHELL_MAX_BYTES);
|
||||
expect(loadedConfig.tools?.shell?.maxLines).toBe(DEFAULT_SHELL_MAX_LINES);
|
||||
});
|
||||
|
||||
test("loads and saves custom shell config", () => {
|
||||
// Setup config with custom shell settings
|
||||
const customMaxBytes = 12_410;
|
||||
const customMaxLines = 500;
|
||||
|
||||
memfs[testConfigPath] = JSON.stringify(
|
||||
{
|
||||
model: "mymodel",
|
||||
tools: {
|
||||
shell: {
|
||||
maxBytes: customMaxBytes,
|
||||
maxLines: customMaxLines,
|
||||
},
|
||||
},
|
||||
},
|
||||
null,
|
||||
2,
|
||||
);
|
||||
memfs[testInstructionsPath] = "test instructions";
|
||||
|
||||
// Load config and verify custom shell settings
|
||||
const loadedConfig = loadConfig(testConfigPath, testInstructionsPath, {
|
||||
disableProjectDoc: true,
|
||||
});
|
||||
|
||||
// Check shell settings were loaded correctly
|
||||
expect(loadedConfig.tools?.shell?.maxBytes).toBe(customMaxBytes);
|
||||
expect(loadedConfig.tools?.shell?.maxLines).toBe(customMaxLines);
|
||||
|
||||
// Modify shell settings and save
|
||||
const updatedMaxBytes = 20_000;
|
||||
const updatedMaxLines = 1_000;
|
||||
|
||||
const updatedConfig = {
|
||||
...loadedConfig,
|
||||
tools: {
|
||||
shell: {
|
||||
maxBytes: updatedMaxBytes,
|
||||
maxLines: updatedMaxLines,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
saveConfig(updatedConfig, testConfigPath, testInstructionsPath);
|
||||
|
||||
// Verify saved config contains updated shell settings
|
||||
expect(memfs[testConfigPath]).toContain(`"maxBytes": ${updatedMaxBytes}`);
|
||||
expect(memfs[testConfigPath]).toContain(`"maxLines": ${updatedMaxLines}`);
|
||||
|
||||
// Load again and verify updated values
|
||||
const reloadedConfig = loadConfig(testConfigPath, testInstructionsPath, {
|
||||
disableProjectDoc: true,
|
||||
});
|
||||
|
||||
expect(reloadedConfig.tools?.shell?.maxBytes).toBe(updatedMaxBytes);
|
||||
expect(reloadedConfig.tools?.shell?.maxLines).toBe(updatedMaxLines);
|
||||
});
|
||||
|
||||
@@ -5,12 +5,12 @@ import { describe, it, expect, vi } from "vitest";
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
import { exec as rawExec } from "../src/utils/agent/sandbox/raw-exec.js";
|
||||
|
||||
import type { AppConfig } from "../src/utils/config.js";
|
||||
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 config = { model: "any", instructions: "" } as AppConfig;
|
||||
const result = await rawExec(cmd, {}, config);
|
||||
|
||||
expect(result.exitCode).not.toBe(0);
|
||||
expect(result.stderr.length).toBeGreaterThan(0);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { exec as rawExec } from "../src/utils/agent/sandbox/raw-exec.js";
|
||||
import type { AppConfig } from "src/utils/config.js";
|
||||
|
||||
// Regression test: When cancelling an in‑flight `rawExec()` the implementation
|
||||
// must terminate *all* processes that belong to the spawned command – not just
|
||||
@@ -27,13 +28,17 @@ describe("rawExec – abort kills entire process group", () => {
|
||||
// Bash script: spawn `sleep 30` in background, print its PID, then wait.
|
||||
const script = "sleep 30 & pid=$!; echo $pid; wait $pid";
|
||||
const cmd = ["bash", "-c", script];
|
||||
const config: AppConfig = {
|
||||
model: "test-model",
|
||||
instructions: "test-instructions",
|
||||
};
|
||||
|
||||
// Start a bash shell that:
|
||||
// - spawns a background `sleep 30`
|
||||
// - 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, {}, config, abortController.signal);
|
||||
|
||||
// Give Bash a tiny bit of time to start and print the PID.
|
||||
await new Promise((r) => setTimeout(r, 100));
|
||||
|
||||
Reference in New Issue
Block a user