Skip to content

Commit 725aa41

Browse files
authored
🤖 Remove per-line byte limit for IPC bash execution (#348)
Removes the 1KB per-line byte limit for executeBash over IPC to support code review operations with long lines (e.g., minified files, generated code). **Changes:** - IPC mode (truncate): No line limit, no per-line limit, 1MB total byte limit - AI mode (tmpfile): Unchanged - 300 line limit, 1KB per-line limit, 16KB display limit This fixes "Line exceeded per-line limit" errors when viewing diffs with long lines. _Generated with `cmux`_
1 parent 40899f9 commit 725aa41

File tree

3 files changed

+92
-23
lines changed

3 files changed

+92
-23
lines changed

src/constants/toolLimits.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ export const BASH_MAX_TOTAL_BYTES = 16 * 1024; // 16KB total output to show agen
77
export const BASH_MAX_FILE_BYTES = 100 * 1024; // 100KB max to save to temp file
88

99
// truncate policy limits (IPC - generous for UI features like code review)
10-
// No line limit for IPC - only byte limit applies
10+
// No line limit or per-line byte limit for IPC - only total byte limit applies
1111
export const BASH_TRUNCATE_MAX_TOTAL_BYTES = 1024 * 1024; // 1MB total output
1212
export const BASH_TRUNCATE_MAX_FILE_BYTES = 1024 * 1024; // 1MB file limit (same as total for IPC)
1313

14-
// Shared limits
15-
export const BASH_MAX_LINE_BYTES = 1024; // 1KB per line (shared across both policies)
14+
// tmpfile policy limits (AI agent only)
15+
export const BASH_MAX_LINE_BYTES = 1024; // 1KB per line for AI agent
1616

1717
export const MAX_TODOS = 7; // Maximum number of TODO items in a list

src/services/tools/bash.test.ts

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ describe("bash tool", () => {
177177
expect(result.success).toBe(true);
178178
expect(result.truncated).toBeDefined();
179179
if (result.truncated) {
180-
expect(result.truncated.reason).toContain("exceeded");
180+
expect(result.truncated.reason).toContain("exceed");
181181
// Should collect lines up to ~1MB (around 1150-1170 lines with 900 bytes each)
182182
expect(result.truncated.totalLines).toBeGreaterThan(1000);
183183
expect(result.truncated.totalLines).toBeLessThan(1300);
@@ -195,6 +195,72 @@ describe("bash tool", () => {
195195
tempDir[Symbol.dispose]();
196196
});
197197

198+
it("should reject single overlong line before storing it (IPC mode)", async () => {
199+
const tempDir = new TestTempDir("test-bash-overlong-line");
200+
const tool = createBashTool({
201+
cwd: process.cwd(),
202+
tempDir: tempDir.path,
203+
overflow_policy: "truncate",
204+
});
205+
206+
const args: BashToolArgs = {
207+
// Generate a single 2MB line (exceeds 1MB total limit)
208+
script: 'perl -e \'print "A" x 2000000 . "\\n"\'',
209+
timeout_secs: 5,
210+
};
211+
212+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
213+
214+
// Should succeed but with truncation before storing the overlong line
215+
expect(result.success).toBe(true);
216+
expect(result.truncated).toBeDefined();
217+
if (result.truncated) {
218+
expect(result.truncated.reason).toContain("would exceed file preservation limit");
219+
// Should have 0 lines collected since the first line was too long
220+
expect(result.truncated.totalLines).toBe(0);
221+
}
222+
223+
// CRITICAL: Output must NOT contain the 2MB line - should be empty or nearly empty
224+
expect(result.output?.length ?? 0).toBeLessThan(100);
225+
226+
tempDir[Symbol.dispose]();
227+
});
228+
229+
it("should reject overlong line at boundary (IPC mode)", async () => {
230+
const tempDir = new TestTempDir("test-bash-boundary");
231+
const tool = createBashTool({
232+
cwd: process.cwd(),
233+
tempDir: tempDir.path,
234+
overflow_policy: "truncate",
235+
});
236+
237+
const args: BashToolArgs = {
238+
// First line: 500KB (within limit)
239+
// Second line: 600KB (would exceed 1MB when added)
240+
script: 'perl -e \'print "A" x 500000 . "\\n"; print "B" x 600000 . "\\n"\'',
241+
timeout_secs: 5,
242+
};
243+
244+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
245+
246+
expect(result.success).toBe(true);
247+
expect(result.truncated).toBeDefined();
248+
if (result.truncated) {
249+
expect(result.truncated.reason).toContain("would exceed");
250+
// Should have collected exactly 1 line (the 500KB line)
251+
expect(result.truncated.totalLines).toBe(1);
252+
}
253+
254+
// Output should contain only the first line (~500KB), not the second line
255+
expect(result.output?.length).toBeGreaterThanOrEqual(500000);
256+
expect(result.output?.length).toBeLessThan(600000);
257+
// Verify content is only 'A's, not 'B's
258+
expect(result.output).toContain("AAAA");
259+
expect(result.output).not.toContain("BBBB");
260+
261+
tempDir[Symbol.dispose]();
262+
});
263+
198264
it("should use tmpfile policy by default when overflow_policy not specified", async () => {
199265
const tempDir = new TestTempDir("test-bash-default");
200266
const tool = createBashTool({

src/services/tools/bash.ts

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,15 @@ class DisposableProcess implements Disposable {
6262
*/
6363
export const createBashTool: ToolFactory = (config: ToolConfiguration) => {
6464
// Select limits based on overflow policy
65-
// truncate = IPC calls (generous limits for UI features, no line limit)
65+
// truncate = IPC calls (generous limits for UI features, no line limit, no per-line limit)
6666
// tmpfile = AI agent calls (conservative limits for LLM context)
6767
const overflowPolicy = config.overflow_policy ?? "tmpfile";
6868
const maxTotalBytes =
6969
overflowPolicy === "truncate" ? BASH_TRUNCATE_MAX_TOTAL_BYTES : BASH_MAX_TOTAL_BYTES;
7070
const maxFileBytes =
7171
overflowPolicy === "truncate" ? BASH_TRUNCATE_MAX_FILE_BYTES : BASH_MAX_FILE_BYTES;
7272
const maxLines = overflowPolicy === "truncate" ? Infinity : BASH_HARD_MAX_LINES;
73+
const maxLineBytes = overflowPolicy === "truncate" ? Infinity : BASH_MAX_LINE_BYTES;
7374

7475
return tool({
7576
description: TOOL_DEFINITIONS.bash.description + "\nRuns in " + config.cwd + " - no cd needed",
@@ -237,25 +238,26 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => {
237238
const lineBytes = Buffer.byteLength(line, "utf-8");
238239

239240
// Check if line exceeds per-line limit (hard stop - this is likely corrupt data)
240-
if (lineBytes > BASH_MAX_LINE_BYTES) {
241+
if (lineBytes > maxLineBytes) {
241242
triggerFileTruncation(
242-
`Line ${lines.length + 1} exceeded per-line limit: ${lineBytes} bytes > ${BASH_MAX_LINE_BYTES} bytes`
243+
`Line ${lines.length + 1} exceeded per-line limit: ${lineBytes} bytes > ${maxLineBytes} bytes`
243244
);
244245
return;
245246
}
246247

247-
// Collect this line (even if display is truncated, keep for file)
248-
lines.push(line);
249-
totalBytesAccumulated += lineBytes + 1; // +1 for newline
250-
251-
// Check file limit first (hard stop)
252-
if (totalBytesAccumulated > maxFileBytes) {
248+
// Check file limit BEFORE adding line to prevent overlong lines from being returned
249+
const bytesAfterLine = totalBytesAccumulated + lineBytes + 1; // +1 for newline
250+
if (bytesAfterLine > maxFileBytes) {
253251
triggerFileTruncation(
254-
`Total output exceeded file preservation limit: ${totalBytesAccumulated} bytes > ${maxFileBytes} bytes (at line ${lines.length})`
252+
`Total output would exceed file preservation limit: ${bytesAfterLine} bytes > ${maxFileBytes} bytes (at line ${lines.length + 1})`
255253
);
256254
return;
257255
}
258256

257+
// Collect this line (even if display is truncated, keep for file)
258+
lines.push(line);
259+
totalBytesAccumulated = bytesAfterLine;
260+
259261
// Check display limits (soft stop - keep collecting for file)
260262
if (!displayTruncated) {
261263
if (totalBytesAccumulated > maxTotalBytes) {
@@ -279,25 +281,26 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => {
279281
const lineBytes = Buffer.byteLength(line, "utf-8");
280282

281283
// Check if line exceeds per-line limit (hard stop - this is likely corrupt data)
282-
if (lineBytes > BASH_MAX_LINE_BYTES) {
284+
if (lineBytes > maxLineBytes) {
283285
triggerFileTruncation(
284-
`Line ${lines.length + 1} exceeded per-line limit: ${lineBytes} bytes > ${BASH_MAX_LINE_BYTES} bytes`
286+
`Line ${lines.length + 1} exceeded per-line limit: ${lineBytes} bytes > ${maxLineBytes} bytes`
285287
);
286288
return;
287289
}
288290

289-
// Collect this line (even if display is truncated, keep for file)
290-
lines.push(line);
291-
totalBytesAccumulated += lineBytes + 1; // +1 for newline
292-
293-
// Check file limit first (hard stop)
294-
if (totalBytesAccumulated > maxFileBytes) {
291+
// Check file limit BEFORE adding line to prevent overlong lines from being returned
292+
const bytesAfterLine = totalBytesAccumulated + lineBytes + 1; // +1 for newline
293+
if (bytesAfterLine > maxFileBytes) {
295294
triggerFileTruncation(
296-
`Total output exceeded file preservation limit: ${totalBytesAccumulated} bytes > ${maxFileBytes} bytes (at line ${lines.length})`
295+
`Total output would exceed file preservation limit: ${bytesAfterLine} bytes > ${maxFileBytes} bytes (at line ${lines.length + 1})`
297296
);
298297
return;
299298
}
300299

300+
// Collect this line (even if display is truncated, keep for file)
301+
lines.push(line);
302+
totalBytesAccumulated = bytesAfterLine;
303+
301304
// Check display limits (soft stop - keep collecting for file)
302305
if (!displayTruncated) {
303306
if (totalBytesAccumulated > maxTotalBytes) {

0 commit comments

Comments
 (0)