From dc0e9353029aa9bc2e6e30ba6440ca95faff6637 Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 3 Nov 2025 02:04:59 +0000 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=A4=96=20fix:=20auto-correct=20redund?= =?UTF-8?q?ant=20absolute=20paths=20with=20warning?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of rejecting absolute paths within the workspace, automatically convert them to relative paths and include a warning to the agent. **Problem:** - Oct 31 nightly run showed 55% of terminal-bench tasks (44/80) hit 'Redundant path prefix' errors - Agent wastes tool calls retrying with relative paths - Causes confusion and burns time approaching timeout - Particularly affects QEMU (80% fail), Git (83% fail), Security (75% fail) **Solution:** - validateNoRedundantPrefix now returns {correctedPath, warning} instead of {error} - Tools (file_read, file_edit_*) auto-correct the path and include warning in response - Agent still gets feedback to use relative paths, but operation succeeds - Preserves token-saving goal while removing friction **Expected impact:** - Reduce wasted tool calls in 44/80 tasks - Estimated +5-8 percentage point improvement in pass rate (48-51% vs current 43%) - Particularly helps QEMU/Git/Security task categories **Testing:** - Updated all tests to expect auto-correction instead of errors - All 26 fileCommon tests passing _Generated with `cmux`_ --- src/services/tools/fileCommon.test.ts | 33 ++++++++++++----------- src/services/tools/fileCommon.ts | 9 ++++--- src/services/tools/file_edit_insert.ts | 19 +++++++++---- src/services/tools/file_edit_operation.ts | 9 ++++--- src/services/tools/file_read.ts | 9 ++++--- 5 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/services/tools/fileCommon.test.ts b/src/services/tools/fileCommon.test.ts index a0f5f5822..a1bf20478 100644 --- a/src/services/tools/fileCommon.test.ts +++ b/src/services/tools/fileCommon.test.ts @@ -147,19 +147,20 @@ describe("fileCommon", () => { expect(validateNoRedundantPrefix("file.ts", cwd, runtime)).toBeNull(); }); - it("should reject absolute paths that contain the cwd prefix", () => { + it("should auto-correct absolute paths that contain the cwd prefix", () => { const result = validateNoRedundantPrefix("/workspace/project/src/file.ts", cwd, runtime); expect(result).not.toBeNull(); - expect(result?.error).toContain("Redundant path prefix detected"); - expect(result?.error).toContain("Please use relative paths to save tokens"); - expect(result?.error).toContain("src/file.ts"); // Should suggest the relative path + expect(result?.correctedPath).toBe("src/file.ts"); + expect(result?.warning).toContain("Using relative paths"); + expect(result?.warning).toContain("saves tokens"); + expect(result?.warning).toContain("auto-corrected"); }); - it("should reject absolute paths at the cwd root", () => { + it("should auto-correct absolute paths at the cwd root", () => { const result = validateNoRedundantPrefix("/workspace/project/file.ts", cwd, runtime); expect(result).not.toBeNull(); - expect(result?.error).toContain("Redundant path prefix detected"); - expect(result?.error).toContain("file.ts"); // Should suggest the relative path + expect(result?.correctedPath).toBe("file.ts"); + expect(result?.warning).toContain("auto-corrected"); }); it("should allow absolute paths outside cwd (they will be caught by validatePathInCwd)", () => { @@ -182,7 +183,8 @@ describe("fileCommon", () => { runtime ); expect(result).not.toBeNull(); - expect(result?.error).toContain("src/file.ts"); + expect(result?.correctedPath).toBe("src/file.ts"); + expect(result?.warning).toContain("auto-corrected"); }); it("should handle nested paths correctly", () => { @@ -192,14 +194,15 @@ describe("fileCommon", () => { runtime ); expect(result).not.toBeNull(); - expect(result?.error).toContain("src/components/Button/index.ts"); + expect(result?.correctedPath).toBe("src/components/Button/index.ts"); + expect(result?.warning).toContain("auto-corrected"); }); - it("should reject path that equals cwd exactly", () => { + it("should auto-correct path that equals cwd exactly", () => { const result = validateNoRedundantPrefix("/workspace/project", cwd, runtime); expect(result).not.toBeNull(); - expect(result?.error).toContain("Redundant path prefix detected"); - expect(result?.error).toContain("."); // Should suggest current directory + expect(result?.correctedPath).toBe("."); + expect(result?.warning).toContain("auto-corrected"); }); it("should not match partial directory names", () => { @@ -217,15 +220,15 @@ describe("fileCommon", () => { }); const sshCwd = "/home/user/cmux/project/branch"; - // Should reject absolute paths with redundant prefix on SSH too + // Should auto-correct absolute paths with redundant prefix on SSH too const result = validateNoRedundantPrefix( "/home/user/cmux/project/branch/src/file.ts", sshCwd, sshRuntime ); expect(result).not.toBeNull(); - expect(result?.error).toContain("Redundant path prefix detected"); - expect(result?.error).toContain("src/file.ts"); + expect(result?.correctedPath).toBe("src/file.ts"); + expect(result?.warning).toContain("auto-corrected"); // Should allow relative paths on SSH expect(validateNoRedundantPrefix("src/file.ts", sshCwd, sshRuntime)).toBeNull(); diff --git a/src/services/tools/fileCommon.ts b/src/services/tools/fileCommon.ts index c7a608f47..def462ccc 100644 --- a/src/services/tools/fileCommon.ts +++ b/src/services/tools/fileCommon.ts @@ -48,7 +48,7 @@ export function validateFileSize(stats: FileStat): { error: string } | null { /** * Validates that a file path doesn't contain redundant workspace prefix. - * Returns an error object if the path contains the cwd prefix, null if valid. + * If the path contains the cwd prefix, returns the corrected relative path and a warning. * This helps save tokens by encouraging relative paths. * * Works for both local and SSH runtimes by using runtime.normalizePath() @@ -57,13 +57,13 @@ export function validateFileSize(stats: FileStat): { error: string } | null { * @param filePath - The file path to validate * @param cwd - The working directory * @param runtime - The runtime to use for path normalization - * @returns Error object if redundant prefix found, null if valid + * @returns Object with corrected path and warning if redundant prefix found, null if valid */ export function validateNoRedundantPrefix( filePath: string, cwd: string, runtime: Runtime -): { error: string } | null { +): { correctedPath: string; warning: string } | null { // Only check absolute paths (start with /) - relative paths are fine // This works for both local and SSH since both use Unix-style paths if (!filePath.startsWith("/")) { @@ -87,7 +87,8 @@ export function validateNoRedundantPrefix( const relativePath = normalizedPath === cleanCwd ? "." : normalizedPath.substring(cleanCwd.length + 1); return { - error: `Redundant path prefix detected. The path '${filePath}' contains the workspace directory. Please use relative paths to save tokens: '${relativePath}'`, + correctedPath: relativePath, + warning: `Note: Using relative paths like '${relativePath}' instead of '${filePath}' saves tokens. The path has been auto-corrected for you.`, }; } diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index a0c70b340..9339a993f 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -24,16 +24,16 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) ): Promise => { try { // Validate no redundant path prefix (must come first to catch absolute paths) + // If redundant prefix found, auto-correct to relative path with warning + let pathWarning: string | undefined; const redundantPrefixValidation = validateNoRedundantPrefix( file_path, config.cwd, config.runtime ); if (redundantPrefixValidation) { - return { - success: false, - error: redundantPrefixValidation.error, - }; + file_path = redundantPrefixValidation.correctedPath; + pathWarning = redundantPrefixValidation.warning; } const pathValidation = validatePathInCwd(file_path, config.cwd, config.runtime); @@ -81,7 +81,7 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) } } - return executeFileEditOperation({ + const result = await executeFileEditOperation({ config, filePath: file_path, abortSignal, @@ -119,6 +119,15 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) }; }, }); + + // Add path warning if present + if (pathWarning && result.success) { + return { + ...result, + warning: pathWarning, + }; + } + return result; } catch (error) { if (error && typeof error === "object" && "code" in error && error.code === "EACCES") { return { diff --git a/src/services/tools/file_edit_operation.ts b/src/services/tools/file_edit_operation.ts index e84e23647..aab067449 100644 --- a/src/services/tools/file_edit_operation.ts +++ b/src/services/tools/file_edit_operation.ts @@ -44,16 +44,16 @@ export async function executeFileEditOperation({ > { try { // Validate no redundant path prefix (must come first to catch absolute paths) + // If redundant prefix found, auto-correct to relative path with warning + let pathWarning: string | undefined; const redundantPrefixValidation = validateNoRedundantPrefix( filePath, config.cwd, config.runtime ); if (redundantPrefixValidation) { - return { - success: false, - error: redundantPrefixValidation.error, - }; + filePath = redundantPrefixValidation.correctedPath; + pathWarning = redundantPrefixValidation.warning; } const pathValidation = validatePathInCwd(filePath, config.cwd, config.runtime); @@ -139,6 +139,7 @@ export async function executeFileEditOperation({ success: true, diff, ...operationResult.metadata, + ...(pathWarning && { warning: pathWarning }), }; } catch (error) { if (error && typeof error === "object" && "code" in error) { diff --git a/src/services/tools/file_read.ts b/src/services/tools/file_read.ts index c7bec3cbc..e34a1e3f7 100644 --- a/src/services/tools/file_read.ts +++ b/src/services/tools/file_read.ts @@ -23,16 +23,16 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { try { // Validate no redundant path prefix (must come first to catch absolute paths) + // If redundant prefix found, auto-correct to relative path with warning + let pathWarning: string | undefined; const redundantPrefixValidation = validateNoRedundantPrefix( filePath, config.cwd, config.runtime ); if (redundantPrefixValidation) { - return { - success: false, - error: redundantPrefixValidation.error, - }; + filePath = redundantPrefixValidation.correctedPath; + pathWarning = redundantPrefixValidation.warning; } // Validate that the path is within the working directory @@ -172,6 +172,7 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { modifiedTime: fileStat.modifiedTime.toISOString(), lines_read: numberedLines.length, content, + ...(pathWarning && { warning: pathWarning }), }; } catch (error) { // Handle specific errors From 8633e986184d45b38f62190560c020d4853afd05 Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 3 Nov 2025 02:24:23 +0000 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=A4=96=20fix:=20add=20warning=20field?= =?UTF-8?q?=20to=20tool=20result=20types?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add optional warning field to FileReadToolResult and FileEditDiffSuccessBase to support auto-correction warnings. Also update file_read test to expect auto-correction instead of rejection. --- src/types/tools.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/types/tools.ts b/src/types/tools.ts index a2ae4c7e4..b025b9219 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -52,6 +52,7 @@ export type FileReadToolResult = modifiedTime: string; lines_read: number; content: string; + warning?: string; } | { success: false; @@ -61,6 +62,7 @@ export type FileReadToolResult = export interface FileEditDiffSuccessBase { success: true; diff: string; + warning?: string; } export interface FileEditErrorResult { From 129ae6c10fe63f89fee627e710ad104d35bf9fc0 Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 3 Nov 2025 02:25:42 +0000 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=A4=96=20fix:=20update=20file=5Fread?= =?UTF-8?q?=20test=20for=20auto-correction=20behavior?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/services/tools/file_read.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/services/tools/file_read.test.ts b/src/services/tools/file_read.test.ts index b540fc7b7..3d48684f6 100644 --- a/src/services/tools/file_read.test.ts +++ b/src/services/tools/file_read.test.ts @@ -318,7 +318,7 @@ describe("file_read tool", () => { } }); - it("should reject absolute paths containing the workspace directory", async () => { + it("should auto-correct absolute paths containing the workspace directory", async () => { // Setup const content = "test content"; await fs.writeFile(testFilePath, content); @@ -332,12 +332,12 @@ describe("file_read tool", () => { // Execute const result = (await tool.execute!(args, mockToolCallOptions)) as FileReadToolResult; - // Assert - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant path prefix detected"); - expect(result.error).toContain("Please use relative paths to save tokens"); - expect(result.error).toContain("test.txt"); // Should suggest the relative path + // Assert - Should succeed with auto-correction warning + expect(result.success).toBe(true); + if (result.success) { + expect(result.warning).toContain("auto-corrected"); + expect(result.warning).toContain("test.txt"); // Should mention the relative path + expect(result.content).toContain("test content"); } }); From 8bf86d3265da3783d9c566edf59f826e73d2c2df Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 3 Nov 2025 02:38:20 +0000 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20extract=20path?= =?UTF-8?q?=20validation=20to=20shared=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminates duplication across file_read, file_edit_insert, and file_edit_operation by creating validateAndCorrectPath() wrapper in fileCommon.ts. --- src/services/tools/fileCommon.ts | 27 +++++++++++++++++++++++ src/services/tools/file_edit_insert.ts | 13 ++++------- src/services/tools/file_edit_operation.ts | 13 ++++------- src/services/tools/file_read.ts | 13 ++++------- 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/services/tools/fileCommon.ts b/src/services/tools/fileCommon.ts index def462ccc..2feb97da6 100644 --- a/src/services/tools/fileCommon.ts +++ b/src/services/tools/fileCommon.ts @@ -136,3 +136,30 @@ export function validatePathInCwd( return null; } + +/** + * Validates and auto-corrects redundant path prefixes in file paths. + * Returns the corrected path and an optional warning message. + * + * This is a convenience wrapper around validateNoRedundantPrefix that handles + * the common pattern of auto-correcting paths and returning warnings. + * + * @param filePath - The file path to validate (may be modified if redundant prefix found) + * @param cwd - The working directory + * @param runtime - The runtime to use for path normalization + * @returns Object with correctedPath and optional warning + */ +export function validateAndCorrectPath( + filePath: string, + cwd: string, + runtime: Runtime +): { correctedPath: string; warning?: string } { + const redundantPrefixValidation = validateNoRedundantPrefix(filePath, cwd, runtime); + if (redundantPrefixValidation) { + return { + correctedPath: redundantPrefixValidation.correctedPath, + warning: redundantPrefixValidation.warning, + }; + } + return { correctedPath: filePath }; +} diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 9339a993f..87d3e1a30 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -2,7 +2,7 @@ import { tool } from "ai"; import type { FileEditInsertToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { validatePathInCwd, validateNoRedundantPrefix } from "./fileCommon"; +import { validatePathInCwd, validateAndCorrectPath } from "./fileCommon"; import { EDIT_FAILED_NOTE_PREFIX, NOTE_READ_FILE_RETRY } from "@/types/tools"; import { executeFileEditOperation } from "./file_edit_operation"; import { RuntimeError } from "@/runtime/Runtime"; @@ -23,18 +23,13 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) { abortSignal } ): Promise => { try { - // Validate no redundant path prefix (must come first to catch absolute paths) - // If redundant prefix found, auto-correct to relative path with warning - let pathWarning: string | undefined; - const redundantPrefixValidation = validateNoRedundantPrefix( + // Validate and auto-correct redundant path prefix + const { correctedPath: validatedPath, warning: pathWarning } = validateAndCorrectPath( file_path, config.cwd, config.runtime ); - if (redundantPrefixValidation) { - file_path = redundantPrefixValidation.correctedPath; - pathWarning = redundantPrefixValidation.warning; - } + file_path = validatedPath; const pathValidation = validatePathInCwd(file_path, config.cwd, config.runtime); if (pathValidation) { diff --git a/src/services/tools/file_edit_operation.ts b/src/services/tools/file_edit_operation.ts index aab067449..7f085e2b3 100644 --- a/src/services/tools/file_edit_operation.ts +++ b/src/services/tools/file_edit_operation.ts @@ -4,7 +4,7 @@ import { generateDiff, validateFileSize, validatePathInCwd, - validateNoRedundantPrefix, + validateAndCorrectPath, } from "./fileCommon"; import { RuntimeError } from "@/runtime/Runtime"; import { readFileString, writeFileString } from "@/utils/runtime/helpers"; @@ -43,18 +43,13 @@ export async function executeFileEditOperation({ FileEditErrorResult | (FileEditDiffSuccessBase & TMetadata) > { try { - // Validate no redundant path prefix (must come first to catch absolute paths) - // If redundant prefix found, auto-correct to relative path with warning - let pathWarning: string | undefined; - const redundantPrefixValidation = validateNoRedundantPrefix( + // Validate and auto-correct redundant path prefix + const { correctedPath: validatedPath, warning: pathWarning } = validateAndCorrectPath( filePath, config.cwd, config.runtime ); - if (redundantPrefixValidation) { - filePath = redundantPrefixValidation.correctedPath; - pathWarning = redundantPrefixValidation.warning; - } + filePath = validatedPath; const pathValidation = validatePathInCwd(filePath, config.cwd, config.runtime); if (pathValidation) { diff --git a/src/services/tools/file_read.ts b/src/services/tools/file_read.ts index e34a1e3f7..0c0cb3d5c 100644 --- a/src/services/tools/file_read.ts +++ b/src/services/tools/file_read.ts @@ -2,7 +2,7 @@ import { tool } from "ai"; import type { FileReadToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { validatePathInCwd, validateFileSize, validateNoRedundantPrefix } from "./fileCommon"; +import { validatePathInCwd, validateFileSize, validateAndCorrectPath } from "./fileCommon"; import { RuntimeError } from "@/runtime/Runtime"; import { readFileString } from "@/utils/runtime/helpers"; @@ -22,18 +22,13 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { // Note: abortSignal available but not used - file reads are fast and complete quickly try { - // Validate no redundant path prefix (must come first to catch absolute paths) - // If redundant prefix found, auto-correct to relative path with warning - let pathWarning: string | undefined; - const redundantPrefixValidation = validateNoRedundantPrefix( + // Validate and auto-correct redundant path prefix + const { correctedPath: validatedPath, warning: pathWarning } = validateAndCorrectPath( filePath, config.cwd, config.runtime ); - if (redundantPrefixValidation) { - filePath = redundantPrefixValidation.correctedPath; - pathWarning = redundantPrefixValidation.warning; - } + filePath = validatedPath; // Validate that the path is within the working directory const pathValidation = validatePathInCwd(filePath, config.cwd, config.runtime); From 841a37b2112a9ecad03e512e31a9dfc54b45dcde Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 3 Nov 2025 02:41:34 +0000 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=A4=96=20fix:=20resolve=20telemetry?= =?UTF-8?q?=20VERSION=20type=20safety=20issue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/services/tools/fileCommon.ts | 4 ++-- src/telemetry/utils.ts | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/services/tools/fileCommon.ts b/src/services/tools/fileCommon.ts index 2feb97da6..c3d8582ca 100644 --- a/src/services/tools/fileCommon.ts +++ b/src/services/tools/fileCommon.ts @@ -140,10 +140,10 @@ export function validatePathInCwd( /** * Validates and auto-corrects redundant path prefixes in file paths. * Returns the corrected path and an optional warning message. - * + * * This is a convenience wrapper around validateNoRedundantPrefix that handles * the common pattern of auto-correcting paths and returning warnings. - * + * * @param filePath - The file path to validate (may be modified if redundant prefix found) * @param cwd - The working directory * @param runtime - The runtime to use for path normalization diff --git a/src/telemetry/utils.ts b/src/telemetry/utils.ts index 12864eced..be35a1d00 100644 --- a/src/telemetry/utils.ts +++ b/src/telemetry/utils.ts @@ -9,9 +9,8 @@ import { VERSION } from "../version"; * Get base telemetry properties included with all events */ export function getBaseTelemetryProperties(): BaseTelemetryProperties { - const gitDescribe: string = VERSION.git_describe; return { - version: gitDescribe, + version: String(VERSION.git_describe), platform: window.api?.platform || "unknown", electronVersion: window.api?.versions?.electron || "unknown", };