diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 12a5599c61..00fdb2b147 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -1,5 +1,4 @@ import { tool } from "ai"; -import * as path from "path"; import type { FileEditInsertToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; @@ -41,9 +40,8 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) }; } - const resolvedPath = path.isAbsolute(file_path) - ? file_path - : path.resolve(config.cwd, file_path); + // Use runtime's normalizePath method to resolve paths correctly for both local and SSH runtimes + const resolvedPath = config.runtime.normalizePath(file_path, config.cwd); // Check if file exists using runtime const exists = await fileExists(config.runtime, resolvedPath); diff --git a/src/services/tools/file_edit_operation.test.ts b/src/services/tools/file_edit_operation.test.ts index 927aae83be..ffb90a1afe 100644 --- a/src/services/tools/file_edit_operation.test.ts +++ b/src/services/tools/file_edit_operation.test.ts @@ -1,14 +1,15 @@ -import { describe, test, expect } from "@jest/globals"; +import { describe, test, expect, jest } from "@jest/globals"; import { executeFileEditOperation } from "./file_edit_operation"; import { WRITE_DENIED_PREFIX } from "@/types/tools"; import { createRuntime } from "@/runtime/runtimeFactory"; +import type { Runtime } from "@/runtime/Runtime"; const TEST_CWD = "/tmp"; -function createConfig() { +function createConfig(runtime?: Runtime) { return { cwd: TEST_CWD, - runtime: createRuntime({ type: "local", srcBaseDir: TEST_CWD }), + runtime: runtime ?? createRuntime({ type: "local", srcBaseDir: TEST_CWD }), tempDir: "/tmp", }; } @@ -26,4 +27,60 @@ describe("executeFileEditOperation", () => { expect(result.error.startsWith(WRITE_DENIED_PREFIX)).toBe(true); } }); + + test("should use runtime.normalizePath for path resolution, not Node's path.resolve", async () => { + // This test verifies that executeFileEditOperation uses runtime.normalizePath() + // instead of path.resolve() for resolving file paths. + // + // Why this matters: path.resolve() uses LOCAL filesystem semantics (Node.js path module), + // which normalizes paths differently than the remote filesystem expects. + // For example, path.resolve() on Windows uses backslashes, and path normalization + // can behave differently across platforms. + + const normalizePathCalls: Array<{ targetPath: string; basePath: string }> = []; + + const mockRuntime = { + stat: jest + .fn<() => Promise<{ size: number; modifiedTime: Date; isDirectory: boolean }>>() + .mockResolvedValue({ + size: 100, + modifiedTime: new Date(), + isDirectory: false, + }), + readFile: jest.fn<() => Promise>().mockResolvedValue(new Uint8Array()), + writeFile: jest.fn<() => Promise>().mockResolvedValue(undefined), + normalizePath: jest.fn<(targetPath: string, basePath: string) => string>( + (targetPath: string, basePath: string) => { + normalizePathCalls.push({ targetPath, basePath }); + // Mock SSH-style path normalization + if (targetPath.startsWith("/")) return targetPath; + return `${basePath}/${targetPath}`; + } + ), + } as unknown as Runtime; + + const testFilePath = "relative/path/to/file.txt"; + const testCwd = "/remote/workspace/dir"; + + await executeFileEditOperation({ + config: { + cwd: testCwd, + runtime: mockRuntime, + tempDir: "/tmp", + }, + filePath: testFilePath, + operation: () => ({ success: true, newContent: "test", metadata: {} }), + }); + + // Verify that runtime.normalizePath() was called for path resolution + const normalizeCallForFilePath = normalizePathCalls.find( + (call) => call.targetPath === testFilePath + ); + + expect(normalizeCallForFilePath).toBeDefined(); + + if (normalizeCallForFilePath) { + expect(normalizeCallForFilePath.basePath).toBe(testCwd); + } + }); }); diff --git a/src/services/tools/file_edit_operation.ts b/src/services/tools/file_edit_operation.ts index 14b922357f..5b16d08c6a 100644 --- a/src/services/tools/file_edit_operation.ts +++ b/src/services/tools/file_edit_operation.ts @@ -1,4 +1,3 @@ -import * as path from "path"; import type { FileEditDiffSuccessBase, FileEditErrorResult } from "@/types/tools"; import { WRITE_DENIED_PREFIX } from "@/types/tools"; import type { ToolConfiguration } from "@/utils/tools/tools"; @@ -45,7 +44,9 @@ export async function executeFileEditOperation({ }; } - const resolvedPath = path.isAbsolute(filePath) ? filePath : path.resolve(config.cwd, filePath); + // Use runtime's normalizePath method to resolve paths correctly for both local and SSH runtimes + // This ensures path resolution uses runtime-specific semantics instead of Node.js path module + const resolvedPath = config.runtime.normalizePath(filePath, config.cwd); // Check if file exists and get stats using runtime let fileStat; diff --git a/src/services/tools/file_read.ts b/src/services/tools/file_read.ts index 9bf9c6d1c7..34f172169f 100644 --- a/src/services/tools/file_read.ts +++ b/src/services/tools/file_read.ts @@ -1,5 +1,4 @@ import { tool } from "ai"; -import * as path from "path"; import type { FileReadToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; @@ -31,10 +30,8 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { }; } - // Resolve relative paths from configured working directory - const resolvedPath = path.isAbsolute(filePath) - ? filePath - : path.resolve(config.cwd, filePath); + // Use runtime's normalizePath method to resolve paths correctly for both local and SSH runtimes + const resolvedPath = config.runtime.normalizePath(filePath, config.cwd); // Check if file exists using runtime let fileStat; diff --git a/tests/ipcMain/runtimeFileEditing.test.ts b/tests/ipcMain/runtimeFileEditing.test.ts index c93dacdd0f..2143a574cd 100644 --- a/tests/ipcMain/runtimeFileEditing.test.ts +++ b/tests/ipcMain/runtimeFileEditing.test.ts @@ -484,6 +484,99 @@ describeIntegration("Runtime File Editing Tools", () => { }, type === "ssh" ? TEST_TIMEOUT_SSH_MS : TEST_TIMEOUT_LOCAL_MS ); + + test.concurrent( + "should handle relative paths correctly when editing files", + async () => { + const env = await createTestEnvironment(); + const tempGitRepo = await createTempGitRepo(); + + try { + // Setup provider + await setupProviders(env.mockIpcRenderer, { + anthropic: { + apiKey: getApiKey("ANTHROPIC_API_KEY"), + }, + }); + + // Create workspace + const branchName = generateBranchName("relative-path-test"); + const runtimeConfig = getRuntimeConfig(branchName); + const { workspaceId, cleanup } = await createWorkspaceHelper( + env, + tempGitRepo, + branchName, + runtimeConfig, + type === "ssh" + ); + + try { + const streamTimeout = + type === "ssh" ? STREAM_TIMEOUT_SSH_MS : STREAM_TIMEOUT_LOCAL_MS; + + // Create a file using AI with a relative path + const relativeTestFile = "subdir/relative_test.txt"; + const createEvents = await sendMessageAndWait( + env, + workspaceId, + `Create a file at path "${relativeTestFile}" with content: "Original content"`, + streamTimeout + ); + + // Verify file was created successfully + const createStreamEnd = createEvents.find( + (e) => "type" in e && e.type === "stream-end" + ); + expect(createStreamEnd).toBeDefined(); + expect((createStreamEnd as any).error).toBeUndefined(); + + // Now edit the file using a relative path + const editEvents = await sendMessageAndWait( + env, + workspaceId, + `Replace the text in ${relativeTestFile}: change "Original" to "Modified"`, + streamTimeout + ); + + // Verify edit was successful + const editStreamEnd = editEvents.find((e) => "type" in e && e.type === "stream-end"); + expect(editStreamEnd).toBeDefined(); + expect((editStreamEnd as any).error).toBeUndefined(); + + // Verify file_edit_replace_string tool was called + const toolCalls = editEvents.filter( + (e) => "type" in e && e.type === "tool-call-start" + ); + const editCall = toolCalls.find( + (e: any) => e.toolName === "file_edit_replace_string" + ); + expect(editCall).toBeDefined(); + + // Read the file to verify the edit was applied + const readEvents = await sendMessageAndWait( + env, + workspaceId, + `Read the file ${relativeTestFile} and tell me its content`, + streamTimeout + ); + + const responseText = extractTextFromEvents(readEvents); + // The file should contain "Modified" not "Original" + expect(responseText.toLowerCase()).toContain("modified"); + + // If this is SSH, the bug would cause the edit to fail because + // path.resolve() would resolve relative to the LOCAL filesystem + // instead of the REMOTE filesystem + } finally { + await cleanup(); + } + } finally { + await cleanupTestEnvironment(env); + await cleanupTempGitRepo(tempGitRepo); + } + }, + type === "ssh" ? TEST_TIMEOUT_SSH_MS : TEST_TIMEOUT_LOCAL_MS + ); } ); });