Skip to content

Commit 8fcafef

Browse files
committed
🤖 Enable redundant path prefix validation for SSH runtime
Previously, validateNoRedundantPrefix() skipped validation for SSH runtimes under the assumption that remote paths needed different handling. However, the validation is equally valuable for SSH - absolute paths like '/home/user/cmux/project/branch/src/file.ts' are just as redundant as '/workspace/project/src/file.ts' on local runtime. Changes: - Remove SSH runtime bypass in validateNoRedundantPrefix() - Use simple string matching instead of Node's path module (works for both) - Handle trailing slashes and partial directory name matches correctly - Add comprehensive tests for SSH runtime validation - Add edge case tests (path equals cwd, partial name matches) The validation now saves tokens on both local and SSH runtimes by encouraging agents to use relative paths consistently. _Generated with `cmux`_
1 parent c3c1bf0 commit 8fcafef

File tree

2 files changed

+59
-17
lines changed

2 files changed

+59
-17
lines changed

src/services/tools/fileCommon.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,5 +194,41 @@ describe("fileCommon", () => {
194194
expect(result).not.toBeNull();
195195
expect(result?.error).toContain("src/components/Button/index.ts");
196196
});
197+
198+
it("should reject path that equals cwd exactly", () => {
199+
const result = validateNoRedundantPrefix("/workspace/project", cwd, runtime);
200+
expect(result).not.toBeNull();
201+
expect(result?.error).toContain("Redundant path prefix detected");
202+
expect(result?.error).toContain("."); // Should suggest current directory
203+
});
204+
205+
it("should not match partial directory names", () => {
206+
// /workspace/project2 should NOT match /workspace/project
207+
expect(validateNoRedundantPrefix("/workspace/project2/file.ts", cwd, runtime)).toBeNull();
208+
expect(validateNoRedundantPrefix("/workspace/project-old/file.ts", cwd, runtime)).toBeNull();
209+
});
210+
211+
it("should work with SSH runtime", () => {
212+
const sshRuntime = createRuntime({
213+
type: "ssh",
214+
host: "user@localhost",
215+
srcBaseDir: "/home/user/cmux",
216+
identityFile: "/tmp/fake-key",
217+
});
218+
const sshCwd = "/home/user/cmux/project/branch";
219+
220+
// Should reject absolute paths with redundant prefix on SSH too
221+
const result = validateNoRedundantPrefix(
222+
"/home/user/cmux/project/branch/src/file.ts",
223+
sshCwd,
224+
sshRuntime
225+
);
226+
expect(result).not.toBeNull();
227+
expect(result?.error).toContain("Redundant path prefix detected");
228+
expect(result?.error).toContain("src/file.ts");
229+
230+
// Should allow relative paths on SSH
231+
expect(validateNoRedundantPrefix("src/file.ts", sshCwd, sshRuntime)).toBeNull();
232+
});
197233
});
198234
});

src/services/tools/fileCommon.ts

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,36 +53,42 @@ export function validateFileSize(stats: FileStat): { error: string } | null {
5353
* Returns an error object if the path contains the cwd prefix, null if valid.
5454
* This helps save tokens by encouraging relative paths.
5555
*
56+
* Works for both local and SSH runtimes by using simple string matching
57+
* for absolute paths instead of Node's path module (which only handles local paths).
58+
*
5659
* @param filePath - The file path to validate
5760
* @param cwd - The working directory
58-
* @param runtime - The runtime (skip check for SSH since paths are remote)
61+
* @param runtime - The runtime (unused, kept for consistency)
5962
* @returns Error object if redundant prefix found, null if valid
6063
*/
6164
export function validateNoRedundantPrefix(
6265
filePath: string,
6366
cwd: string,
6467
runtime: Runtime
6568
): { error: string } | null {
66-
// Skip for SSH runtimes - remote paths don't apply
67-
if (runtime instanceof SSHRuntime) {
69+
// Only check absolute paths (start with /) - relative paths are fine
70+
// This works for both local and SSH since both use Unix-style paths
71+
if (!filePath.startsWith("/")) {
6872
return null;
6973
}
7074

71-
// Check if the path contains the cwd as a prefix (indicating redundancy)
72-
// This catches cases like "/workspace/project/src/file.ts" when cwd is "/workspace/project"
73-
const normalizedPath = path.normalize(filePath);
74-
const normalizedCwd = path.normalize(cwd);
75+
// Normalize both paths: remove trailing slashes for consistent comparison
76+
const normalizedPath = filePath.replace(/\/+$/, "");
77+
const normalizedCwd = cwd.replace(/\/+$/, "");
7578

76-
// Only check absolute paths - relative paths are fine
77-
if (path.isAbsolute(normalizedPath)) {
78-
// Check if the absolute path starts with the cwd
79-
if (normalizedPath.startsWith(normalizedCwd)) {
80-
// Calculate what the relative path would be
81-
const relativePath = path.relative(normalizedCwd, normalizedPath);
82-
return {
83-
error: `Redundant path prefix detected. The path '${filePath}' contains the workspace directory. Please use relative paths to save tokens: '${relativePath}'`,
84-
};
85-
}
79+
// Check if the absolute path starts with the cwd
80+
// Use startsWith + check for path separator to avoid partial matches
81+
// e.g., /workspace/project should match /workspace/project/src but not /workspace/project2
82+
if (
83+
normalizedPath === normalizedCwd ||
84+
normalizedPath.startsWith(normalizedCwd + "/")
85+
) {
86+
// Calculate what the relative path would be
87+
const relativePath =
88+
normalizedPath === normalizedCwd ? "." : normalizedPath.substring(normalizedCwd.length + 1);
89+
return {
90+
error: `Redundant path prefix detected. The path '${filePath}' contains the workspace directory. Please use relative paths to save tokens: '${relativePath}'`,
91+
};
8692
}
8793

8894
return null;

0 commit comments

Comments
 (0)