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..c3d8582ca 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.`, }; } @@ -135,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 a0c70b340..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) - const redundantPrefixValidation = validateNoRedundantPrefix( + // Validate and auto-correct redundant path prefix + const { correctedPath: validatedPath, warning: pathWarning } = validateAndCorrectPath( file_path, config.cwd, config.runtime ); - if (redundantPrefixValidation) { - return { - success: false, - error: redundantPrefixValidation.error, - }; - } + file_path = validatedPath; const pathValidation = validatePathInCwd(file_path, config.cwd, config.runtime); if (pathValidation) { @@ -81,7 +76,7 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) } } - return executeFileEditOperation({ + const result = await executeFileEditOperation({ config, filePath: file_path, abortSignal, @@ -119,6 +114,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..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) - const redundantPrefixValidation = validateNoRedundantPrefix( + // Validate and auto-correct redundant path prefix + const { correctedPath: validatedPath, warning: pathWarning } = validateAndCorrectPath( filePath, config.cwd, config.runtime ); - if (redundantPrefixValidation) { - return { - success: false, - error: redundantPrefixValidation.error, - }; - } + filePath = validatedPath; const pathValidation = validatePathInCwd(filePath, config.cwd, config.runtime); if (pathValidation) { @@ -139,6 +134,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.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"); } }); diff --git a/src/services/tools/file_read.ts b/src/services/tools/file_read.ts index c7bec3cbc..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) - const redundantPrefixValidation = validateNoRedundantPrefix( + // Validate and auto-correct redundant path prefix + const { correctedPath: validatedPath, warning: pathWarning } = validateAndCorrectPath( filePath, config.cwd, config.runtime ); - if (redundantPrefixValidation) { - return { - success: false, - error: redundantPrefixValidation.error, - }; - } + filePath = validatedPath; // Validate that the path is within the working directory const pathValidation = validatePathInCwd(filePath, config.cwd, config.runtime); @@ -172,6 +167,7 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { modifiedTime: fileStat.modifiedTime.toISOString(), lines_read: numberedLines.length, content, + ...(pathWarning && { warning: pathWarning }), }; } catch (error) { // Handle specific errors 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", }; 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 {