Skip to content

Commit d27fdc9

Browse files
committed
🤖 Fix resolvePath() to separate path resolution from existence checking
- Remove existence checking from LocalRuntime.resolvePath() - Now just expands tildes and resolves to absolute path - No longer uses fsPromises.access() to verify path exists - Update SSHRuntime.resolvePath() to use readlink -m instead of realpath - readlink -m normalizes paths without requiring them to exist - Properly separates path resolution from validation - Extract SSH command spawning pattern into execSSHCommand() helper - Reduces duplication in SSHRuntime - Reusable for simple SSH commands that return stdout - Remove SSH integration tests for resolvePath() - Will rely on createWorkspace matrix tests for end-to-end verification - Keep only constructor unit tests that don't require SSH - Update LocalRuntime test to expect non-existent paths to resolve successfully - Revert unrelated gitService.test.ts debug logging This improves separation of concerns: resolvePath() now purely resolves paths without side effects, allowing callers to decide whether to validate existence. Generated with `cmux`
1 parent 2956db0 commit d27fdc9

File tree

5 files changed

+21
-107
lines changed

5 files changed

+21
-107
lines changed

src/runtime/LocalRuntime.test.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,11 @@ describe("LocalRuntime.resolvePath", () => {
5151
expect(resolved).toBe("/tmp");
5252
});
5353

54-
it("should reject non-existent paths", async () => {
54+
it("should resolve non-existent paths without checking existence", async () => {
5555
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-
);
56+
const resolved = await runtime.resolvePath("/this/path/does/not/exist/12345");
57+
// Should resolve to absolute path without checking if it exists
58+
expect(resolved).toBe("/this/path/does/not/exist/12345");
6059
});
6160

6261
it("should resolve relative paths from cwd", async () => {

src/runtime/LocalRuntime.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -306,19 +306,7 @@ export class LocalRuntime implements Runtime {
306306
const expanded = expandTilde(filePath);
307307

308308
// 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-
}
309+
return path.resolve(expanded);
322310
}
323311

324312
normalizePath(targetPath: string, basePath: string): string {

src/runtime/SSHRuntime.test.ts

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -31,63 +31,3 @@ describe("SSHRuntime constructor", () => {
3131
}).not.toThrow();
3232
});
3333
});
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(runtime.resolvePath("/this/path/does/not/exist/12345")).rejects.toThrow(
89-
/Failed to resolve path/
90-
);
91-
});
92-
});
93-
});

src/runtime/SSHRuntime.ts

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -318,10 +318,18 @@ export class SSHRuntime implements Runtime {
318318
};
319319
}
320320
async resolvePath(filePath: string): Promise<string> {
321-
// Use shell to expand tildes and resolve path on remote system
322-
// We use `realpath` which resolves symlinks and gives canonical path
323-
// Note: realpath also verifies the path exists
324-
const command = `realpath ${shescape.quote(filePath)}`;
321+
// Use shell to expand tildes and normalize path on remote system
322+
// Uses bash to expand ~ and readlink -m to normalize without checking existence
323+
// readlink -m canonicalizes the path (handles .., ., //) without requiring it to exist
324+
const command = `bash -c 'readlink -m ${shescape.quote(filePath)}'`;
325+
return this.execSSHCommand(command);
326+
}
327+
328+
/**
329+
* Execute a simple SSH command and return stdout
330+
* @private
331+
*/
332+
private async execSSHCommand(command: string): Promise<string> {
325333
const sshArgs = this.buildSSHArgs();
326334
sshArgs.push(this.config.host, command);
327335

@@ -340,33 +348,18 @@ export class SSHRuntime implements Runtime {
340348

341349
proc.on("close", (code) => {
342350
if (code !== 0) {
343-
reject(
344-
new RuntimeErrorClass(
345-
`Failed to resolve path '${filePath}' on remote host: ${stderr.trim()}`,
346-
"file_io"
347-
)
348-
);
349-
return;
350-
}
351-
352-
const resolved = stdout.trim();
353-
if (!resolved?.startsWith("/")) {
354-
reject(
355-
new RuntimeErrorClass(
356-
`Invalid resolved path '${resolved}' for '${filePath}' (expected absolute path)`,
357-
"file_io"
358-
)
359-
);
351+
reject(new RuntimeErrorClass(`SSH command failed: ${stderr.trim()}`, "network"));
360352
return;
361353
}
362354

363-
resolve(resolved);
355+
const output = stdout.trim();
356+
resolve(output);
364357
});
365358

366359
proc.on("error", (err) => {
367360
reject(
368361
new RuntimeErrorClass(
369-
`Cannot resolve path '${filePath}' on remote host: ${getErrorMessage(err)}`,
362+
`Cannot execute SSH command: ${getErrorMessage(err)}`,
370363
"network",
371364
err instanceof Error ? err : undefined
372365
)

src/services/gitService.test.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,6 @@ describe("removeWorktreeSafe", () => {
5555
const result = await createWorktree(mockConfig, repoPath, "test-branch", {
5656
trunkBranch: defaultBranch,
5757
});
58-
if (!result.success) {
59-
console.error("createWorktree failed:", result.error);
60-
}
6158
expect(result.success).toBe(true);
6259
const worktreePath = result.path!;
6360

@@ -91,9 +88,6 @@ describe("removeWorktreeSafe", () => {
9188
const result = await createWorktree(mockConfig, repoPath, "dirty-branch", {
9289
trunkBranch: defaultBranch,
9390
});
94-
if (!result.success) {
95-
console.error("createWorktree failed:", result.error);
96-
}
9791
expect(result.success).toBe(true);
9892
const worktreePath = result.path!;
9993

0 commit comments

Comments
 (0)