From 7d16e3e1d9ec4a1cfd2a2d4f6bd1c4fe1037df58 Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 14 Oct 2025 18:49:38 -0500 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A4=96=20Add=20overflow=5Fpolicy=20to?= =?UTF-8?q?=20bash=20tool=20for=20git=20status=20operations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `overflow_policy: 'truncate' | 'tmpfile'` option to bash execution that prevents temp file spam from background git status polling. ## Problem GitStatusStore and GitStatusIndicator poll git status every 3 seconds. When output exceeds 16KB limit, bash tool writes full output to temp files, causing console spam: ``` [gitStatus] Script failed: [OUTPUT OVERFLOW - ...] Full output saved to /var/folders/.../bash-ffe12c1a.txt ``` Background operations don't need full output - they just fail silently. ## Solution Add `overflow_policy` config option (not exposed to AI): - `'truncate'` - Return first 50 lines inline, no temp file - `'tmpfile'` - Current behavior (default for AI operations) ## Changes 1. **Core implementation** (bash.ts): - Check `config.overflow_policy ?? 'tmpfile'` in truncation handler - Truncate: return first 50 lines with error message - Tmpfile: write full output to temp file (default behavior) 2. **Type plumbing**: - Add `overflow_policy?` to ToolConfiguration interface - Pass through IPC types and ipcMain handler - NOT exposed in AI tool schema (verified) 3. **Usage**: - GitStatusStore: use `overflow_policy: 'truncate'` for status checks - GitStatusStore: use `overflow_policy: 'truncate'` for git fetch - GitStatusIndicator: use `overflow_policy: 'truncate'` for popover ## Testing Added 2 new tests: - Verify truncate policy returns first 50 lines, no temp file - Verify default (no policy) uses tmpfile behavior All 504 tests pass. Typecheck passes. _Generated with `cmux`_ --- src/components/GitStatusIndicator.tsx | 1 + src/services/ipcMain.ts | 7 ++- src/services/tools/bash.test.ts | 70 +++++++++++++++++++++++++++ src/services/tools/bash.ts | 68 ++++++++++++++++---------- src/stores/GitStatusStore.ts | 2 + src/types/ipc.ts | 6 ++- src/utils/tools/tools.ts | 2 + 7 files changed, 129 insertions(+), 27 deletions(-) diff --git a/src/components/GitStatusIndicator.tsx b/src/components/GitStatusIndicator.tsx index 515ca55d6..1d4ba265f 100644 --- a/src/components/GitStatusIndicator.tsx +++ b/src/components/GitStatusIndicator.tsx @@ -324,6 +324,7 @@ ${getDirtyFiles} const result = await window.api.workspace.executeBash(workspaceId, script, { timeout_secs: 5, niceness: 19, // Lowest priority - don't interfere with user operations + overflow_policy: "truncate", // Don't create temp files for git status popover }); if (!result.success) { diff --git a/src/services/ipcMain.ts b/src/services/ipcMain.ts index febc8079f..09f3f25b5 100644 --- a/src/services/ipcMain.ts +++ b/src/services/ipcMain.ts @@ -595,7 +595,11 @@ export class IpcMain { _event, workspaceId: string, script: string, - options?: { timeout_secs?: number; niceness?: number } + options?: { + timeout_secs?: number; + niceness?: number; + overflow_policy?: "truncate" | "tmpfile"; + } ) => { try { // Get workspace metadata to find workspacePath @@ -621,6 +625,7 @@ export class IpcMain { secrets: secretsToRecord(projectSecrets), niceness: options?.niceness, tempDir: tempDir.path, + overflow_policy: options?.overflow_policy, }); // Execute the script with provided options diff --git a/src/services/tools/bash.test.ts b/src/services/tools/bash.test.ts index d5fa4fdc0..e7d345b1e 100644 --- a/src/services/tools/bash.test.ts +++ b/src/services/tools/bash.test.ts @@ -157,6 +157,76 @@ describe("bash tool", () => { } }); + it("should truncate overflow output when overflow_policy is 'truncate'", async () => { + const tempDir = new TestTempDir("test-bash-truncate"); + const tool = createBashTool({ + cwd: process.cwd(), + tempDir: tempDir.path, + overflow_policy: "truncate", + }); + + const args: BashToolArgs = { + script: "for i in {1..400}; do echo line$i; done", // Exceeds 300 line hard cap + timeout_secs: 5, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + // Should contain truncation notice + expect(result.error).toContain("[OUTPUT TRUNCATED"); + expect(result.error).toContain("Showing first 50 of"); + expect(result.error).toContain("lines:"); + + // Should contain first 50 lines + expect(result.error).toContain("line1"); + expect(result.error).toContain("line50"); + + // Should NOT contain line 51 or beyond + expect(result.error).not.toContain("line51"); + expect(result.error).not.toContain("line100"); + + // Should NOT create temp file + const files = fs.readdirSync(tempDir.path); + const bashFiles = files.filter((f) => f.startsWith("bash-")); + expect(bashFiles.length).toBe(0); + } + + tempDir[Symbol.dispose](); + }); + + it("should use tmpfile policy by default when overflow_policy not specified", async () => { + const tempDir = new TestTempDir("test-bash-default"); + const tool = createBashTool({ + cwd: process.cwd(), + tempDir: tempDir.path, + // overflow_policy not specified - should default to tmpfile + }); + + const args: BashToolArgs = { + script: "for i in {1..400}; do echo line$i; done", + timeout_secs: 5, + }; + + const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; + + expect(result.success).toBe(false); + if (!result.success) { + // Should use tmpfile behavior + expect(result.error).toContain("[OUTPUT OVERFLOW"); + expect(result.error).toContain("saved to"); + expect(result.error).not.toContain("[OUTPUT TRUNCATED"); + + // Verify temp file was created + const files = fs.readdirSync(tempDir.path); + const bashFiles = files.filter((f) => f.startsWith("bash-")); + expect(bashFiles.length).toBe(1); + } + + tempDir[Symbol.dispose](); + }); + it("should interleave stdout and stderr", async () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; diff --git a/src/services/tools/bash.ts b/src/services/tools/bash.ts index 6c919002a..cb90960d0 100644 --- a/src/services/tools/bash.ts +++ b/src/services/tools/bash.ts @@ -320,38 +320,56 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { wall_duration_ms, }); } else if (truncated) { - // Save overflow output to temp file instead of returning an error - // We don't show ANY of the actual output to avoid overwhelming context. - // Instead, save it to a temp file and encourage the agent to use filtering tools. - try { - // Use 8 hex characters for short, memorable temp file IDs - const fileId = Math.random().toString(16).substring(2, 10); - const overflowPath = path.join(config.tempDir, `bash-${fileId}.txt`); - const fullOutput = lines.join("\n"); - fs.writeFileSync(overflowPath, fullOutput, "utf-8"); - - const output = `[OUTPUT OVERFLOW - ${overflowReason ?? "unknown reason"}] + // Handle overflow based on policy + const overflowPolicy = config.overflow_policy ?? "tmpfile"; -Full output (${lines.length} lines) saved to ${overflowPath} - -Use selective filtering tools (e.g. grep) to extract relevant information and continue your task - -File will be automatically cleaned up when stream ends.`; + if (overflowPolicy === "truncate") { + // Return truncated output with first 50 lines + const maxTruncateLines = 50; + const truncatedLines = lines.slice(0, maxTruncateLines); + const truncatedOutput = truncatedLines.join("\n"); + const errorMessage = `[OUTPUT TRUNCATED - ${overflowReason ?? "unknown reason"}]\n\nShowing first ${maxTruncateLines} of ${lines.length} lines:\n\n${truncatedOutput}`; resolveOnce({ success: false, - error: output, - exitCode: -1, - wall_duration_ms, - }); - } catch (err) { - // If temp file creation fails, fall back to original error - resolveOnce({ - success: false, - error: `Command output overflow: ${overflowReason ?? "unknown reason"}. Failed to save overflow to temp file: ${String(err)}`, + error: errorMessage, exitCode: -1, wall_duration_ms, }); + } else { + // tmpfile policy: Save overflow output to temp file instead of returning an error + // We don't show ANY of the actual output to avoid overwhelming context. + // Instead, save it to a temp file and encourage the agent to use filtering tools. + try { + // Use 8 hex characters for short, memorable temp file IDs + const fileId = Math.random().toString(16).substring(2, 10); + const overflowPath = path.join(config.tempDir, `bash-${fileId}.txt`); + const fullOutput = lines.join("\n"); + fs.writeFileSync(overflowPath, fullOutput, "utf-8"); + + const output = `[OUTPUT OVERFLOW - ${overflowReason ?? "unknown reason"}] + +Full output (${lines.length} lines) saved to ${overflowPath} + +Use selective filtering tools (e.g. grep) to extract relevant information and continue your task + +File will be automatically cleaned up when stream ends.`; + + resolveOnce({ + success: false, + error: output, + exitCode: -1, + wall_duration_ms, + }); + } catch (err) { + // If temp file creation fails, fall back to original error + resolveOnce({ + success: false, + error: `Command output overflow: ${overflowReason ?? "unknown reason"}. Failed to save overflow to temp file: ${String(err)}`, + exitCode: -1, + wall_duration_ms, + }); + } } } else if (exitCode === 0 || exitCode === null) { resolveOnce({ diff --git a/src/stores/GitStatusStore.ts b/src/stores/GitStatusStore.ts index 98244c656..cdae9d917 100644 --- a/src/stores/GitStatusStore.ts +++ b/src/stores/GitStatusStore.ts @@ -206,6 +206,7 @@ export class GitStatusStore { const result = await window.api.workspace.executeBash(metadata.id, GIT_STATUS_SCRIPT, { timeout_secs: 5, niceness: 19, // Lowest priority - don't interfere with user operations + overflow_policy: "truncate", // Don't create temp files for background git status checks }); if (!result.success) { @@ -328,6 +329,7 @@ export class GitStatusStore { const result = await window.api.workspace.executeBash(workspaceId, GIT_FETCH_SCRIPT, { timeout_secs: 30, niceness: 19, // Lowest priority - don't interfere with user operations + overflow_policy: "truncate", // Don't create temp files for background git fetch }); if (!result.success) { diff --git a/src/types/ipc.ts b/src/types/ipc.ts index 9bba03829..d9b7a7308 100644 --- a/src/types/ipc.ts +++ b/src/types/ipc.ts @@ -210,7 +210,11 @@ export interface IPCApi { executeBash( workspaceId: string, script: string, - options?: { timeout_secs?: number; niceness?: number } + options?: { + timeout_secs?: number; + niceness?: number; + overflow_policy?: "truncate" | "tmpfile"; + } ): Promise>; openTerminal(workspacePath: string): Promise; diff --git a/src/utils/tools/tools.ts b/src/utils/tools/tools.ts index a2130ea25..923ccfb6b 100644 --- a/src/utils/tools/tools.ts +++ b/src/utils/tools/tools.ts @@ -21,6 +21,8 @@ export interface ToolConfiguration { niceness?: number; /** Temporary directory for tool outputs (required) */ tempDir: string; + /** Overflow policy for bash tool output (optional, not exposed to AI) */ + overflow_policy?: "truncate" | "tmpfile"; } /** From 4bfee0304ef7eb33067e32014aae1de0c4f8c65d Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 14 Oct 2025 19:32:57 -0500 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=A4=96=20Simplify:=20hardcode=20trunc?= =?UTF-8?q?ate=20policy=20in=20ipcMain?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All executeBash IPC calls are from UI (background operations). AI uses bash tool directly (not through IPC), so always gets tmpfile. Simplifications: - Remove overflow_policy from IPC types - Hardcode overflow_policy: 'truncate' in ipcMain handler - Remove overflow_policy from all call sites (GitStatusStore, GitStatusIndicator) Result: Same behavior, less configuration surface area. _Generated with `cmux`_ --- src/components/GitStatusIndicator.tsx | 1 - src/services/ipcMain.ts | 4 ++-- src/stores/GitStatusStore.ts | 2 -- src/types/ipc.ts | 1 - 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/components/GitStatusIndicator.tsx b/src/components/GitStatusIndicator.tsx index 1d4ba265f..515ca55d6 100644 --- a/src/components/GitStatusIndicator.tsx +++ b/src/components/GitStatusIndicator.tsx @@ -324,7 +324,6 @@ ${getDirtyFiles} const result = await window.api.workspace.executeBash(workspaceId, script, { timeout_secs: 5, niceness: 19, // Lowest priority - don't interfere with user operations - overflow_policy: "truncate", // Don't create temp files for git status popover }); if (!result.success) { diff --git a/src/services/ipcMain.ts b/src/services/ipcMain.ts index 09f3f25b5..1dcf47f80 100644 --- a/src/services/ipcMain.ts +++ b/src/services/ipcMain.ts @@ -598,7 +598,6 @@ export class IpcMain { options?: { timeout_secs?: number; niceness?: number; - overflow_policy?: "truncate" | "tmpfile"; } ) => { try { @@ -620,12 +619,13 @@ export class IpcMain { using tempDir = new DisposableTempDir("cmux-ipc-bash"); // Create bash tool with workspace's cwd and secrets + // All IPC bash calls are from UI (background operations) - use truncate to avoid temp file spam const bashTool = createBashTool({ cwd: workspacePath, secrets: secretsToRecord(projectSecrets), niceness: options?.niceness, tempDir: tempDir.path, - overflow_policy: options?.overflow_policy, + overflow_policy: "truncate", }); // Execute the script with provided options diff --git a/src/stores/GitStatusStore.ts b/src/stores/GitStatusStore.ts index cdae9d917..98244c656 100644 --- a/src/stores/GitStatusStore.ts +++ b/src/stores/GitStatusStore.ts @@ -206,7 +206,6 @@ export class GitStatusStore { const result = await window.api.workspace.executeBash(metadata.id, GIT_STATUS_SCRIPT, { timeout_secs: 5, niceness: 19, // Lowest priority - don't interfere with user operations - overflow_policy: "truncate", // Don't create temp files for background git status checks }); if (!result.success) { @@ -329,7 +328,6 @@ export class GitStatusStore { const result = await window.api.workspace.executeBash(workspaceId, GIT_FETCH_SCRIPT, { timeout_secs: 30, niceness: 19, // Lowest priority - don't interfere with user operations - overflow_policy: "truncate", // Don't create temp files for background git fetch }); if (!result.success) { diff --git a/src/types/ipc.ts b/src/types/ipc.ts index d9b7a7308..ece311231 100644 --- a/src/types/ipc.ts +++ b/src/types/ipc.ts @@ -213,7 +213,6 @@ export interface IPCApi { options?: { timeout_secs?: number; niceness?: number; - overflow_policy?: "truncate" | "tmpfile"; } ): Promise>; openTerminal(workspacePath: string): Promise;