Skip to content

Commit e66ec50

Browse files
committed
🤖 Add resolvePath() to Runtime interface for tilde expansion
- Add resolvePath() method to Runtime interface - LocalRuntime: Expands tildes using expandTilde() and verifies path exists - SSHRuntime: Uses remote 'realpath' command to expand and verify paths - WORKSPACE_CREATE: Resolves srcBaseDir before creating runtime and storing in config - Remove tilde validation from SSHRuntime constructor (now handled by resolvePath) - Both tilde (~) and non-tilde paths now work for workspace creation - Add comprehensive unit tests for both runtimes This enables storing resolved paths in config, ensuring consistency across local and SSH runtimes regardless of how the path was originally specified. Generated with `cmux`
1 parent 50ff033 commit e66ec50

File tree

7 files changed

+222
-14
lines changed

7 files changed

+222
-14
lines changed

src/runtime/LocalRuntime.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,41 @@ describe("LocalRuntime constructor", () => {
2929
expect(workspacePath).toBe(expected);
3030
});
3131
});
32+
33+
describe("LocalRuntime.resolvePath", () => {
34+
it("should expand tilde to home directory", async () => {
35+
const runtime = new LocalRuntime("/tmp");
36+
const resolved = await runtime.resolvePath("~");
37+
expect(resolved).toBe(os.homedir());
38+
});
39+
40+
it("should expand tilde with path", async () => {
41+
const runtime = new LocalRuntime("/tmp");
42+
// Use a path that likely exists (or use /tmp if ~ doesn't have subdirs)
43+
const resolved = await runtime.resolvePath("~/..");
44+
const expected = path.dirname(os.homedir());
45+
expect(resolved).toBe(expected);
46+
});
47+
48+
it("should resolve absolute paths", async () => {
49+
const runtime = new LocalRuntime("/tmp");
50+
const resolved = await runtime.resolvePath("/tmp");
51+
expect(resolved).toBe("/tmp");
52+
});
53+
54+
it("should reject non-existent paths", async () => {
55+
const runtime = new LocalRuntime("/tmp");
56+
// eslint-disable-next-line @typescript-eslint/await-thenable
57+
await expect(runtime.resolvePath("/this/path/does/not/exist/12345")).rejects.toThrow(
58+
/Cannot resolve path/
59+
);
60+
});
61+
62+
it("should resolve relative paths from cwd", async () => {
63+
const runtime = new LocalRuntime("/tmp");
64+
const resolved = await runtime.resolvePath(".");
65+
// Should resolve to absolute path
66+
expect(path.isAbsolute(resolved)).toBe(true);
67+
});
68+
});
69+

src/runtime/LocalRuntime.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,26 @@ export class LocalRuntime implements Runtime {
301301
}
302302
}
303303

304+
async resolvePath(filePath: string): Promise<string> {
305+
// Expand tilde to actual home directory path
306+
const expanded = expandTilde(filePath);
307+
308+
// Resolve to absolute path (handles relative paths like "./foo")
309+
const resolved = path.resolve(expanded);
310+
311+
// Verify path exists
312+
try {
313+
await fsPromises.access(resolved);
314+
return resolved;
315+
} catch (err) {
316+
throw new RuntimeErrorClass(
317+
`Cannot resolve path '${filePath}': ${getErrorMessage(err)}`,
318+
"file_io",
319+
err instanceof Error ? err : undefined
320+
);
321+
}
322+
}
323+
304324
normalizePath(targetPath: string, basePath: string): string {
305325
// For local runtime, use Node.js path resolution
306326
// Handle special case: current directory

src/runtime/Runtime.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,24 @@ export interface Runtime {
195195
*/
196196
stat(path: string): Promise<FileStat>;
197197

198+
/**
199+
* Resolve a path to its absolute, canonical form (expanding tildes, resolving symlinks, etc.).
200+
* This is used at workspace creation time to normalize srcBaseDir paths in config.
201+
*
202+
* @param path Path to resolve (may contain tildes or be relative)
203+
* @returns Promise resolving to absolute path
204+
* @throws RuntimeError if path cannot be resolved (e.g., doesn't exist, permission denied)
205+
*
206+
* @example
207+
* // LocalRuntime
208+
* await runtime.resolvePath("~/cmux") // => "/home/user/cmux"
209+
* await runtime.resolvePath("./relative") // => "/current/dir/relative"
210+
*
211+
* // SSHRuntime
212+
* await runtime.resolvePath("~/cmux") // => "/home/user/cmux" (via SSH shell expansion)
213+
*/
214+
resolvePath(path: string): Promise<string>;
215+
198216
/**
199217
* Normalize a path for comparison purposes within this runtime's context.
200218
* Handles runtime-specific path semantics (local vs remote).

src/runtime/SSHRuntime.test.ts

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,24 @@ import { describe, expect, it } from "bun:test";
22
import { SSHRuntime } from "./SSHRuntime";
33

44
describe("SSHRuntime constructor", () => {
5-
it("should reject tilde in srcBaseDir", () => {
5+
it("should accept tilde in srcBaseDir", () => {
6+
// Tildes are now allowed - they will be resolved via resolvePath()
67
expect(() => {
78
new SSHRuntime({
89
host: "example.com",
910
srcBaseDir: "~/cmux",
1011
});
11-
}).toThrow(/cannot start with tilde/);
12+
}).not.toThrow();
1213
});
1314

14-
it("should reject bare tilde in srcBaseDir", () => {
15+
it("should accept bare tilde in srcBaseDir", () => {
16+
// Tildes are now allowed - they will be resolved via resolvePath()
1517
expect(() => {
1618
new SSHRuntime({
1719
host: "example.com",
1820
srcBaseDir: "~",
1921
});
20-
}).toThrow(/cannot start with tilde/);
22+
}).not.toThrow();
2123
});
2224

2325
it("should accept absolute paths in srcBaseDir", () => {
@@ -29,3 +31,63 @@ describe("SSHRuntime constructor", () => {
2931
}).not.toThrow();
3032
});
3133
});
34+
35+
describe("SSHRuntime.resolvePath", () => {
36+
// Note: These tests require TEST_INTEGRATION=1 to run with actual SSH connection
37+
const isIntegrationTest = process.env.TEST_INTEGRATION === "1";
38+
const describeIfIntegration = isIntegrationTest ? describe : describe.skip;
39+
40+
describeIfIntegration("with SSH connection", () => {
41+
it("should expand tilde to home directory", async () => {
42+
const runtime = new SSHRuntime({
43+
host: process.env.SSH_HOST ?? "localhost",
44+
port: parseInt(process.env.SSH_PORT ?? "2222"),
45+
identityFile: process.env.SSH_IDENTITY_FILE,
46+
srcBaseDir: "~/test-workspace",
47+
});
48+
49+
const resolved = await runtime.resolvePath("~");
50+
// Should be an absolute path
51+
expect(resolved).toMatch(/^\/home\//);
52+
});
53+
54+
it("should expand tilde with path", async () => {
55+
const runtime = new SSHRuntime({
56+
host: process.env.SSH_HOST ?? "localhost",
57+
port: parseInt(process.env.SSH_PORT ?? "2222"),
58+
identityFile: process.env.SSH_IDENTITY_FILE,
59+
srcBaseDir: "~/test-workspace",
60+
});
61+
62+
const resolved = await runtime.resolvePath("~/..");
63+
// Should be parent of home directory
64+
expect(resolved).toBe("/home");
65+
});
66+
67+
it("should resolve absolute paths", async () => {
68+
const runtime = new SSHRuntime({
69+
host: process.env.SSH_HOST ?? "localhost",
70+
port: parseInt(process.env.SSH_PORT ?? "2222"),
71+
identityFile: process.env.SSH_IDENTITY_FILE,
72+
srcBaseDir: "/tmp",
73+
});
74+
75+
const resolved = await runtime.resolvePath("/tmp");
76+
expect(resolved).toBe("/tmp");
77+
});
78+
79+
it("should reject non-existent paths", async () => {
80+
const runtime = new SSHRuntime({
81+
host: process.env.SSH_HOST ?? "localhost",
82+
port: parseInt(process.env.SSH_PORT ?? "2222"),
83+
identityFile: process.env.SSH_IDENTITY_FILE,
84+
srcBaseDir: "/tmp",
85+
});
86+
87+
// eslint-disable-next-line @typescript-eslint/await-thenable
88+
await expect(
89+
runtime.resolvePath("/this/path/does/not/exist/12345")
90+
).rejects.toThrow(/Failed to resolve path/);
91+
});
92+
});
93+
});

src/runtime/SSHRuntime.ts

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,8 @@ export class SSHRuntime implements Runtime {
6060
private readonly controlPath: string;
6161

6262
constructor(config: SSHRuntimeConfig) {
63-
// Reject tilde paths - require explicit full paths for SSH
64-
// Rationale: Simplifies logic and avoids ambiguity about which user's home directory
65-
if (config.srcBaseDir.startsWith("~")) {
66-
throw new Error(
67-
`SSH runtime srcBaseDir cannot start with tilde. ` +
68-
`Use full path (e.g., /home/username/cmux instead of ~/cmux)`
69-
);
70-
}
71-
63+
// Note: srcBaseDir may contain tildes - they will be resolved via resolvePath() before use
64+
// The WORKSPACE_CREATE IPC handler resolves paths before storing in config
7265
this.config = config;
7366
// Generate unique control path for SSH connection multiplexing
7467
// This allows multiple SSH sessions to reuse a single TCP connection
@@ -325,6 +318,64 @@ export class SSHRuntime implements Runtime {
325318
isDirectory: fileType === "directory",
326319
};
327320
}
321+
async resolvePath(filePath: string): Promise<string> {
322+
// Use shell to expand tildes and resolve path on remote system
323+
// We use `realpath` which resolves symlinks and gives canonical path
324+
// Note: realpath also verifies the path exists
325+
const command = `realpath ${shescape.quote(filePath)}`;
326+
const sshArgs = this.buildSSHArgs();
327+
sshArgs.push(this.config.host, command);
328+
329+
return new Promise((resolve, reject) => {
330+
const proc = spawn("ssh", sshArgs);
331+
let stdout = "";
332+
let stderr = "";
333+
334+
proc.stdout?.on("data", (data: Buffer) => {
335+
stdout += data.toString();
336+
});
337+
338+
proc.stderr?.on("data", (data: Buffer) => {
339+
stderr += data.toString();
340+
});
341+
342+
proc.on("close", (code) => {
343+
if (code !== 0) {
344+
reject(
345+
new RuntimeErrorClass(
346+
`Failed to resolve path '${filePath}' on remote host: ${stderr.trim()}`,
347+
"file_io"
348+
)
349+
);
350+
return;
351+
}
352+
353+
const resolved = stdout.trim();
354+
if (!resolved?.startsWith("/")) {
355+
reject(
356+
new RuntimeErrorClass(
357+
`Invalid resolved path '${resolved}' for '${filePath}' (expected absolute path)`,
358+
"file_io"
359+
)
360+
);
361+
return;
362+
}
363+
364+
resolve(resolved);
365+
});
366+
367+
proc.on("error", (err) => {
368+
reject(
369+
new RuntimeErrorClass(
370+
`Cannot resolve path '${filePath}' on remote host: ${getErrorMessage(err)}`,
371+
"network",
372+
err instanceof Error ? err : undefined
373+
)
374+
);
375+
});
376+
});
377+
}
378+
328379
normalizePath(targetPath: string, basePath: string): string {
329380
// For SSH, handle paths in a POSIX-like manner without accessing the remote filesystem
330381
const target = targetPath.trim();

src/services/gitService.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ describe("removeWorktreeSafe", () => {
9191
const result = await createWorktree(mockConfig, repoPath, "dirty-branch", {
9292
trunkBranch: defaultBranch,
9393
});
94+
if (!result.success) {
95+
console.error("createWorktree failed:", result.error);
96+
}
9497
expect(result.success).toBe(true);
9598
const worktreePath = result.path!;
9699

src/services/ipcMain.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,10 +284,26 @@ export class IpcMain {
284284
srcBaseDir: this.config.srcDir,
285285
};
286286

287-
// Create runtime - catch validation errors (e.g., tilde paths in SSH)
287+
// Create temporary runtime to resolve srcBaseDir path
288+
// This allows tilde paths to work for both local and SSH runtimes
288289
let runtime;
290+
let resolvedSrcBaseDir: string;
289291
try {
290292
runtime = createRuntime(finalRuntimeConfig);
293+
294+
// Resolve srcBaseDir to absolute path (expanding tildes, etc.)
295+
resolvedSrcBaseDir = await runtime.resolvePath(finalRuntimeConfig.srcBaseDir);
296+
297+
// If path was resolved to something different, recreate runtime with resolved path
298+
if (resolvedSrcBaseDir !== finalRuntimeConfig.srcBaseDir) {
299+
const resolvedRuntimeConfig: RuntimeConfig = {
300+
...finalRuntimeConfig,
301+
srcBaseDir: resolvedSrcBaseDir,
302+
};
303+
runtime = createRuntime(resolvedRuntimeConfig);
304+
// Update finalRuntimeConfig to store resolved path in config
305+
finalRuntimeConfig.srcBaseDir = resolvedSrcBaseDir;
306+
}
291307
} catch (error) {
292308
const errorMsg = error instanceof Error ? error.message : String(error);
293309
return { success: false, error: errorMsg };

0 commit comments

Comments
 (0)