Skip to content

Commit 7adc518

Browse files
authored
Fix: Use runtime.normalizePath for file tool path resolution in SSH remotes (#438)
## Problem File editing tools were using Node.js `path.resolve()` to resolve relative paths, which uses **local** filesystem semantics. For SSH runtimes, this caused incorrect path resolution because paths should be resolved relative to the **remote** filesystem. ## Solution Replace `path.resolve()` with `runtime.normalizePath()` which handles path resolution correctly for both local and SSH runtimes. ## Changes - Fixed path resolution in `file_edit_operation.ts`, `file_edit_insert.ts`, and `file_read.ts` - Added unit test verifying `runtime.normalizePath()` is called - Added integration test for relative path handling in SSH runtimes - Removed unused `path` imports ## Testing - ✅ All existing tests pass (31 file tool tests) - ✅ New unit test verifies the fix - ✅ New integration test covers SSH relative paths
1 parent 2cac68f commit 7adc518

File tree

5 files changed

+160
-14
lines changed

5 files changed

+160
-14
lines changed

src/services/tools/file_edit_insert.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { tool } from "ai";
2-
import * as path from "path";
32
import type { FileEditInsertToolResult } from "@/types/tools";
43
import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools";
54
import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions";
@@ -41,9 +40,8 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration)
4140
};
4241
}
4342

44-
const resolvedPath = path.isAbsolute(file_path)
45-
? file_path
46-
: path.resolve(config.cwd, file_path);
43+
// Use runtime's normalizePath method to resolve paths correctly for both local and SSH runtimes
44+
const resolvedPath = config.runtime.normalizePath(file_path, config.cwd);
4745

4846
// Check if file exists using runtime
4947
const exists = await fileExists(config.runtime, resolvedPath);

src/services/tools/file_edit_operation.test.ts

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
1-
import { describe, test, expect } from "@jest/globals";
1+
import { describe, test, expect, jest } from "@jest/globals";
22
import { executeFileEditOperation } from "./file_edit_operation";
33
import { WRITE_DENIED_PREFIX } from "@/types/tools";
44
import { createRuntime } from "@/runtime/runtimeFactory";
5+
import type { Runtime } from "@/runtime/Runtime";
56

67
const TEST_CWD = "/tmp";
78

8-
function createConfig() {
9+
function createConfig(runtime?: Runtime) {
910
return {
1011
cwd: TEST_CWD,
11-
runtime: createRuntime({ type: "local", srcBaseDir: TEST_CWD }),
12+
runtime: runtime ?? createRuntime({ type: "local", srcBaseDir: TEST_CWD }),
1213
tempDir: "/tmp",
1314
};
1415
}
@@ -26,4 +27,60 @@ describe("executeFileEditOperation", () => {
2627
expect(result.error.startsWith(WRITE_DENIED_PREFIX)).toBe(true);
2728
}
2829
});
30+
31+
test("should use runtime.normalizePath for path resolution, not Node's path.resolve", async () => {
32+
// This test verifies that executeFileEditOperation uses runtime.normalizePath()
33+
// instead of path.resolve() for resolving file paths.
34+
//
35+
// Why this matters: path.resolve() uses LOCAL filesystem semantics (Node.js path module),
36+
// which normalizes paths differently than the remote filesystem expects.
37+
// For example, path.resolve() on Windows uses backslashes, and path normalization
38+
// can behave differently across platforms.
39+
40+
const normalizePathCalls: Array<{ targetPath: string; basePath: string }> = [];
41+
42+
const mockRuntime = {
43+
stat: jest
44+
.fn<() => Promise<{ size: number; modifiedTime: Date; isDirectory: boolean }>>()
45+
.mockResolvedValue({
46+
size: 100,
47+
modifiedTime: new Date(),
48+
isDirectory: false,
49+
}),
50+
readFile: jest.fn<() => Promise<Uint8Array>>().mockResolvedValue(new Uint8Array()),
51+
writeFile: jest.fn<() => Promise<void>>().mockResolvedValue(undefined),
52+
normalizePath: jest.fn<(targetPath: string, basePath: string) => string>(
53+
(targetPath: string, basePath: string) => {
54+
normalizePathCalls.push({ targetPath, basePath });
55+
// Mock SSH-style path normalization
56+
if (targetPath.startsWith("/")) return targetPath;
57+
return `${basePath}/${targetPath}`;
58+
}
59+
),
60+
} as unknown as Runtime;
61+
62+
const testFilePath = "relative/path/to/file.txt";
63+
const testCwd = "/remote/workspace/dir";
64+
65+
await executeFileEditOperation({
66+
config: {
67+
cwd: testCwd,
68+
runtime: mockRuntime,
69+
tempDir: "/tmp",
70+
},
71+
filePath: testFilePath,
72+
operation: () => ({ success: true, newContent: "test", metadata: {} }),
73+
});
74+
75+
// Verify that runtime.normalizePath() was called for path resolution
76+
const normalizeCallForFilePath = normalizePathCalls.find(
77+
(call) => call.targetPath === testFilePath
78+
);
79+
80+
expect(normalizeCallForFilePath).toBeDefined();
81+
82+
if (normalizeCallForFilePath) {
83+
expect(normalizeCallForFilePath.basePath).toBe(testCwd);
84+
}
85+
});
2986
});

src/services/tools/file_edit_operation.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import * as path from "path";
21
import type { FileEditDiffSuccessBase, FileEditErrorResult } from "@/types/tools";
32
import { WRITE_DENIED_PREFIX } from "@/types/tools";
43
import type { ToolConfiguration } from "@/utils/tools/tools";
@@ -45,7 +44,9 @@ export async function executeFileEditOperation<TMetadata>({
4544
};
4645
}
4746

48-
const resolvedPath = path.isAbsolute(filePath) ? filePath : path.resolve(config.cwd, filePath);
47+
// Use runtime's normalizePath method to resolve paths correctly for both local and SSH runtimes
48+
// This ensures path resolution uses runtime-specific semantics instead of Node.js path module
49+
const resolvedPath = config.runtime.normalizePath(filePath, config.cwd);
4950

5051
// Check if file exists and get stats using runtime
5152
let fileStat;

src/services/tools/file_read.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { tool } from "ai";
2-
import * as path from "path";
32
import type { FileReadToolResult } from "@/types/tools";
43
import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools";
54
import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions";
@@ -31,10 +30,8 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => {
3130
};
3231
}
3332

34-
// Resolve relative paths from configured working directory
35-
const resolvedPath = path.isAbsolute(filePath)
36-
? filePath
37-
: path.resolve(config.cwd, filePath);
33+
// Use runtime's normalizePath method to resolve paths correctly for both local and SSH runtimes
34+
const resolvedPath = config.runtime.normalizePath(filePath, config.cwd);
3835

3936
// Check if file exists using runtime
4037
let fileStat;

tests/ipcMain/runtimeFileEditing.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,99 @@ describeIntegration("Runtime File Editing Tools", () => {
484484
},
485485
type === "ssh" ? TEST_TIMEOUT_SSH_MS : TEST_TIMEOUT_LOCAL_MS
486486
);
487+
488+
test.concurrent(
489+
"should handle relative paths correctly when editing files",
490+
async () => {
491+
const env = await createTestEnvironment();
492+
const tempGitRepo = await createTempGitRepo();
493+
494+
try {
495+
// Setup provider
496+
await setupProviders(env.mockIpcRenderer, {
497+
anthropic: {
498+
apiKey: getApiKey("ANTHROPIC_API_KEY"),
499+
},
500+
});
501+
502+
// Create workspace
503+
const branchName = generateBranchName("relative-path-test");
504+
const runtimeConfig = getRuntimeConfig(branchName);
505+
const { workspaceId, cleanup } = await createWorkspaceHelper(
506+
env,
507+
tempGitRepo,
508+
branchName,
509+
runtimeConfig,
510+
type === "ssh"
511+
);
512+
513+
try {
514+
const streamTimeout =
515+
type === "ssh" ? STREAM_TIMEOUT_SSH_MS : STREAM_TIMEOUT_LOCAL_MS;
516+
517+
// Create a file using AI with a relative path
518+
const relativeTestFile = "subdir/relative_test.txt";
519+
const createEvents = await sendMessageAndWait(
520+
env,
521+
workspaceId,
522+
`Create a file at path "${relativeTestFile}" with content: "Original content"`,
523+
streamTimeout
524+
);
525+
526+
// Verify file was created successfully
527+
const createStreamEnd = createEvents.find(
528+
(e) => "type" in e && e.type === "stream-end"
529+
);
530+
expect(createStreamEnd).toBeDefined();
531+
expect((createStreamEnd as any).error).toBeUndefined();
532+
533+
// Now edit the file using a relative path
534+
const editEvents = await sendMessageAndWait(
535+
env,
536+
workspaceId,
537+
`Replace the text in ${relativeTestFile}: change "Original" to "Modified"`,
538+
streamTimeout
539+
);
540+
541+
// Verify edit was successful
542+
const editStreamEnd = editEvents.find((e) => "type" in e && e.type === "stream-end");
543+
expect(editStreamEnd).toBeDefined();
544+
expect((editStreamEnd as any).error).toBeUndefined();
545+
546+
// Verify file_edit_replace_string tool was called
547+
const toolCalls = editEvents.filter(
548+
(e) => "type" in e && e.type === "tool-call-start"
549+
);
550+
const editCall = toolCalls.find(
551+
(e: any) => e.toolName === "file_edit_replace_string"
552+
);
553+
expect(editCall).toBeDefined();
554+
555+
// Read the file to verify the edit was applied
556+
const readEvents = await sendMessageAndWait(
557+
env,
558+
workspaceId,
559+
`Read the file ${relativeTestFile} and tell me its content`,
560+
streamTimeout
561+
);
562+
563+
const responseText = extractTextFromEvents(readEvents);
564+
// The file should contain "Modified" not "Original"
565+
expect(responseText.toLowerCase()).toContain("modified");
566+
567+
// If this is SSH, the bug would cause the edit to fail because
568+
// path.resolve() would resolve relative to the LOCAL filesystem
569+
// instead of the REMOTE filesystem
570+
} finally {
571+
await cleanup();
572+
}
573+
} finally {
574+
await cleanupTestEnvironment(env);
575+
await cleanupTempGitRepo(tempGitRepo);
576+
}
577+
},
578+
type === "ssh" ? TEST_TIMEOUT_SSH_MS : TEST_TIMEOUT_LOCAL_MS
579+
);
487580
}
488581
);
489582
});

0 commit comments

Comments
 (0)