diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 3a6dd36cc..824ea2c0f 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -203,8 +203,10 @@ This project uses **Make** as the primary build orchestrator. See `Makefile` for - utils should be either pure functions or easily isolated (e.g. if they operate on the FS they accept a path). Testing them should not require complex mocks or setup. - **Integration tests:** + - **⚠️ IMPORTANT: Use `bun x jest` to run tests in the `tests/` folder** - Integration tests use Jest (not bun test), so you must run them with `bun x jest` or `TEST_INTEGRATION=1 bun x jest` - Run specific integration test: `TEST_INTEGRATION=1 bun x jest tests/ipcMain/sendMessage.test.ts -t "test name pattern"` - Run all integration tests: `TEST_INTEGRATION=1 bun x jest tests` (~35 seconds, runs 40 tests) + - Unit tests in `src/` use bun test: `bun test src/path/to/file.test.ts` - **⚠️ Running `tests/ipcMain` locally takes a very long time.** Prefer running specific test files or use `-t` to filter to specific tests. - **Performance**: Tests use `test.concurrent()` to run in parallel within each file - **NEVER bypass IPC in integration tests** - Integration tests must use the real IPC communication paths (e.g., `mockIpcRenderer.invoke()`) even when it's harder. Directly accessing services (HistoryService, PartialService, etc.) or manipulating config/state directly bypasses the integration layer and defeats the purpose of the test. diff --git a/src/runtime/LocalRuntime.test.ts b/src/runtime/LocalRuntime.test.ts new file mode 100644 index 000000000..a3cbdd053 --- /dev/null +++ b/src/runtime/LocalRuntime.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "bun:test"; +import * as os from "os"; +import * as path from "path"; +import { LocalRuntime } from "./LocalRuntime"; + +describe("LocalRuntime constructor", () => { + it("should expand tilde in srcBaseDir", () => { + const runtime = new LocalRuntime("~/workspace"); + const workspacePath = runtime.getWorkspacePath("/home/user/project", "branch"); + + // The workspace path should use the expanded home directory + const expected = path.join(os.homedir(), "workspace", "project", "branch"); + expect(workspacePath).toBe(expected); + }); + + it("should handle absolute paths without expansion", () => { + const runtime = new LocalRuntime("/absolute/path"); + const workspacePath = runtime.getWorkspacePath("/home/user/project", "branch"); + + const expected = path.join("/absolute/path", "project", "branch"); + expect(workspacePath).toBe(expected); + }); + + it("should handle bare tilde", () => { + const runtime = new LocalRuntime("~"); + const workspacePath = runtime.getWorkspacePath("/home/user/project", "branch"); + + const expected = path.join(os.homedir(), "project", "branch"); + expect(workspacePath).toBe(expected); + }); +}); + +describe("LocalRuntime.resolvePath", () => { + it("should expand tilde to home directory", async () => { + const runtime = new LocalRuntime("/tmp"); + const resolved = await runtime.resolvePath("~"); + expect(resolved).toBe(os.homedir()); + }); + + it("should expand tilde with path", async () => { + const runtime = new LocalRuntime("/tmp"); + // Use a path that likely exists (or use /tmp if ~ doesn't have subdirs) + const resolved = await runtime.resolvePath("~/.."); + const expected = path.dirname(os.homedir()); + expect(resolved).toBe(expected); + }); + + it("should resolve absolute paths", async () => { + const runtime = new LocalRuntime("/tmp"); + const resolved = await runtime.resolvePath("/tmp"); + expect(resolved).toBe("/tmp"); + }); + + it("should resolve non-existent paths without checking existence", async () => { + const runtime = new LocalRuntime("/tmp"); + const resolved = await runtime.resolvePath("/this/path/does/not/exist/12345"); + // Should resolve to absolute path without checking if it exists + expect(resolved).toBe("/this/path/does/not/exist/12345"); + }); + + it("should resolve relative paths from cwd", async () => { + const runtime = new LocalRuntime("/tmp"); + const resolved = await runtime.resolvePath("."); + // Should resolve to absolute path + expect(path.isAbsolute(resolved)).toBe(true); + }); +}); diff --git a/src/runtime/LocalRuntime.ts b/src/runtime/LocalRuntime.ts index 8edb8d5d8..8c54701bc 100644 --- a/src/runtime/LocalRuntime.ts +++ b/src/runtime/LocalRuntime.ts @@ -22,6 +22,7 @@ import { checkInitHookExists, getInitHookPath, createLineBufferedLoggers } from import { execAsync } from "../utils/disposableExec"; import { getProjectName } from "../utils/runtime/helpers"; import { getErrorMessage } from "../utils/errors"; +import { expandTilde } from "./tildeExpansion"; /** * Local runtime implementation that executes commands and file operations @@ -31,7 +32,8 @@ export class LocalRuntime implements Runtime { private readonly srcBaseDir: string; constructor(srcBaseDir: string) { - this.srcBaseDir = srcBaseDir; + // Expand tilde to actual home directory path for local file system operations + this.srcBaseDir = expandTilde(srcBaseDir); } async exec(command: string, options: ExecOptions): Promise { @@ -299,6 +301,14 @@ export class LocalRuntime implements Runtime { } } + resolvePath(filePath: string): Promise { + // Expand tilde to actual home directory path + const expanded = expandTilde(filePath); + + // Resolve to absolute path (handles relative paths like "./foo") + return Promise.resolve(path.resolve(expanded)); + } + normalizePath(targetPath: string, basePath: string): string { // For local runtime, use Node.js path resolution // Handle special case: current directory diff --git a/src/runtime/Runtime.ts b/src/runtime/Runtime.ts index 28f8fd9b4..18f11ade5 100644 --- a/src/runtime/Runtime.ts +++ b/src/runtime/Runtime.ts @@ -14,8 +14,8 @@ * * srcBaseDir (base directory for all workspaces): * - Where cmux stores ALL workspace directories - * - Local: ~/.cmux/src - * - SSH: /home/user/workspace (or custom remote path) + * - Local: ~/.cmux/src (tilde expanded to full path by LocalRuntime) + * - SSH: /home/user/workspace (must be absolute path, no tilde allowed) * * Workspace Path Computation: * {srcBaseDir}/{projectName}/{workspaceName} @@ -27,14 +27,14 @@ * Example: "feature-123" or "main" * * Full Example (Local): - * srcBaseDir: ~/.cmux/src + * srcBaseDir: ~/.cmux/src (expanded to /home/user/.cmux/src) * projectPath: /Users/me/git/my-project (local git repo) * projectName: my-project (extracted) * workspaceName: feature-123 - * → Workspace: ~/.cmux/src/my-project/feature-123 + * → Workspace: /home/user/.cmux/src/my-project/feature-123 * * Full Example (SSH): - * srcBaseDir: /home/user/workspace + * srcBaseDir: /home/user/workspace (absolute path required) * projectPath: /Users/me/git/my-project (local git repo) * projectName: my-project (extracted) * workspaceName: feature-123 @@ -195,6 +195,24 @@ export interface Runtime { */ stat(path: string): Promise; + /** + * Resolve a path to its absolute, canonical form (expanding tildes, resolving symlinks, etc.). + * This is used at workspace creation time to normalize srcBaseDir paths in config. + * + * @param path Path to resolve (may contain tildes or be relative) + * @returns Promise resolving to absolute path + * @throws RuntimeError if path cannot be resolved (e.g., doesn't exist, permission denied) + * + * @example + * // LocalRuntime + * await runtime.resolvePath("~/cmux") // => "/home/user/cmux" + * await runtime.resolvePath("./relative") // => "/current/dir/relative" + * + * // SSHRuntime + * await runtime.resolvePath("~/cmux") // => "/home/user/cmux" (via SSH shell expansion) + */ + resolvePath(path: string): Promise; + /** * Normalize a path for comparison purposes within this runtime's context. * Handles runtime-specific path semantics (local vs remote). diff --git a/src/runtime/SSHRuntime.test.ts b/src/runtime/SSHRuntime.test.ts new file mode 100644 index 000000000..4ee0f8ccc --- /dev/null +++ b/src/runtime/SSHRuntime.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "bun:test"; +import { SSHRuntime } from "./SSHRuntime"; + +describe("SSHRuntime constructor", () => { + it("should accept tilde in srcBaseDir", () => { + // Tildes are now allowed - they will be resolved via resolvePath() + expect(() => { + new SSHRuntime({ + host: "example.com", + srcBaseDir: "~/cmux", + }); + }).not.toThrow(); + }); + + it("should accept bare tilde in srcBaseDir", () => { + // Tildes are now allowed - they will be resolved via resolvePath() + expect(() => { + new SSHRuntime({ + host: "example.com", + srcBaseDir: "~", + }); + }).not.toThrow(); + }); + + it("should accept absolute paths in srcBaseDir", () => { + expect(() => { + new SSHRuntime({ + host: "example.com", + srcBaseDir: "/home/user/cmux", + }); + }).not.toThrow(); + }); +}); diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index 44ac8e336..04aa3726e 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -53,12 +53,18 @@ export interface SSHRuntimeConfig { * - Supports SSH config aliases, ProxyJump, ControlMaster, etc. * - No password prompts (assumes key-based auth or ssh-agent) * - Atomic file writes via temp + rename + * + * IMPORTANT: All SSH operations MUST include a timeout to prevent hangs from network issues. + * Timeouts should be either set literally for internal operations or forwarded from upstream + * for user-initiated operations. */ export class SSHRuntime implements Runtime { private readonly config: SSHRuntimeConfig; private readonly controlPath: string; constructor(config: SSHRuntimeConfig) { + // Note: srcBaseDir may contain tildes - they will be resolved via resolvePath() before use + // The WORKSPACE_CREATE IPC handler resolves paths before storing in config this.config = config; // Get deterministic controlPath from connection pool // Multiple SSHRuntime instances with same config share the same controlPath, @@ -315,6 +321,76 @@ export class SSHRuntime implements Runtime { isDirectory: fileType === "directory", }; } + async resolvePath(filePath: string): Promise { + // Use shell to expand tildes on remote system + // Bash will expand ~ automatically when we echo the unquoted variable + // This works with BusyBox (doesn't require GNU coreutils) + const command = `bash -c 'p=${shescape.quote(filePath)}; echo $p'`; + // Use 5 second timeout for path resolution (should be near-instant) + return this.execSSHCommand(command, 5000); + } + + /** + * Execute a simple SSH command and return stdout + * @param command - The command to execute on the remote host + * @param timeoutMs - Timeout in milliseconds (required to prevent network hangs) + * @private + */ + private async execSSHCommand(command: string, timeoutMs: number): Promise { + const sshArgs = this.buildSSHArgs(); + sshArgs.push(this.config.host, command); + + return new Promise((resolve, reject) => { + const proc = spawn("ssh", sshArgs); + let stdout = ""; + let stderr = ""; + let timedOut = false; + + // Set timeout to prevent hanging on network issues + const timer = setTimeout(() => { + timedOut = true; + proc.kill(); + reject( + new RuntimeErrorClass(`SSH command timed out after ${timeoutMs}ms: ${command}`, "network") + ); + }, timeoutMs); + + proc.stdout?.on("data", (data: Buffer) => { + stdout += data.toString(); + }); + + proc.stderr?.on("data", (data: Buffer) => { + stderr += data.toString(); + }); + + proc.on("close", (code) => { + clearTimeout(timer); + if (timedOut) return; // Already rejected + + if (code !== 0) { + reject(new RuntimeErrorClass(`SSH command failed: ${stderr.trim()}`, "network")); + return; + } + + const output = stdout.trim(); + resolve(output); + }); + + proc.on("error", (err) => { + clearTimeout(timer); + if (timedOut) return; // Already rejected + + reject( + new RuntimeErrorClass( + `Cannot execute SSH command: ${getErrorMessage(err)}`, + "network", + err instanceof Error ? err : undefined + ) + ); + }); + }); + } + normalizePath(targetPath: string, basePath: string): string { // For SSH, handle paths in a POSIX-like manner without accessing the remote filesystem const target = targetPath.trim(); diff --git a/src/runtime/tildeExpansion.test.ts b/src/runtime/tildeExpansion.test.ts new file mode 100644 index 000000000..25042eb9a --- /dev/null +++ b/src/runtime/tildeExpansion.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "bun:test"; +import * as os from "os"; +import * as path from "path"; +import { expandTilde } from "./tildeExpansion"; + +describe("expandTilde", () => { + it("should expand ~ to home directory", () => { + const result = expandTilde("~"); + expect(result).toBe(os.homedir()); + }); + + it("should expand ~/path to home directory + path", () => { + const result = expandTilde("~/workspace"); + expect(result).toBe(path.join(os.homedir(), "workspace")); + }); + + it("should leave absolute paths unchanged", () => { + const absolutePath = "/abs/path/to/dir"; + const result = expandTilde(absolutePath); + expect(result).toBe(absolutePath); + }); + + it("should leave relative paths unchanged", () => { + const relativePath = "relative/path"; + const result = expandTilde(relativePath); + expect(result).toBe(relativePath); + }); + + it("should handle nested paths correctly", () => { + const result = expandTilde("~/workspace/project/subdir"); + expect(result).toBe(path.join(os.homedir(), "workspace/project/subdir")); + }); +}); diff --git a/src/runtime/tildeExpansion.ts b/src/runtime/tildeExpansion.ts index be37c0810..e5084b6ca 100644 --- a/src/runtime/tildeExpansion.ts +++ b/src/runtime/tildeExpansion.ts @@ -1,11 +1,42 @@ /** - * Utilities for handling tilde path expansion in SSH commands + * Utilities for handling tilde path expansion * - * When running commands over SSH, tilde paths need special handling: + * For SSH commands, tilde paths need special handling: * - Quoted tildes won't expand: `cd '~'` fails, but `cd "$HOME"` works * - Must escape special shell characters when using $HOME expansion + * + * For local paths, tildes should be expanded to actual file system paths. */ +import * as os from "os"; +import * as path from "path"; + +/** + * Expand tilde to actual home directory path for local file system operations. + * + * Converts: + * - "~" → "/home/user" (actual home directory) + * - "~/path" → "/home/user/path" + * - "/abs/path" → "/abs/path" (unchanged) + * + * @param filePath - Path that may contain tilde prefix + * @returns Fully expanded absolute path + * + * @example + * expandTilde("~") // => "/home/user" + * expandTilde("~/workspace") // => "/home/user/workspace" + * expandTilde("/abs/path") // => "/abs/path" + */ +export function expandTilde(filePath: string): string { + if (filePath === "~") { + return os.homedir(); + } else if (filePath.startsWith("~/")) { + return path.join(os.homedir(), filePath.slice(2)); + } else { + return filePath; + } +} + /** * Expand tilde path to $HOME-based path for use in SSH commands. * diff --git a/src/services/gitService.test.ts b/src/services/gitService.test.ts index 237b68989..4f7cb665e 100644 --- a/src/services/gitService.test.ts +++ b/src/services/gitService.test.ts @@ -55,9 +55,6 @@ describe("removeWorktreeSafe", () => { const result = await createWorktree(mockConfig, repoPath, "test-branch", { trunkBranch: defaultBranch, }); - if (!result.success) { - console.error("createWorktree failed:", result.error); - } expect(result.success).toBe(true); const worktreePath = result.path!; diff --git a/src/services/ipcMain.ts b/src/services/ipcMain.ts index 3bfcdf2a3..5b5c37380 100644 --- a/src/services/ipcMain.ts +++ b/src/services/ipcMain.ts @@ -283,7 +283,31 @@ export class IpcMain { type: "local", srcBaseDir: this.config.srcDir, }; - const runtime = createRuntime(finalRuntimeConfig); + + // Create temporary runtime to resolve srcBaseDir path + // This allows tilde paths to work for both local and SSH runtimes + let runtime; + let resolvedSrcBaseDir: string; + try { + runtime = createRuntime(finalRuntimeConfig); + + // Resolve srcBaseDir to absolute path (expanding tildes, etc.) + resolvedSrcBaseDir = await runtime.resolvePath(finalRuntimeConfig.srcBaseDir); + + // If path was resolved to something different, recreate runtime with resolved path + if (resolvedSrcBaseDir !== finalRuntimeConfig.srcBaseDir) { + const resolvedRuntimeConfig: RuntimeConfig = { + ...finalRuntimeConfig, + srcBaseDir: resolvedSrcBaseDir, + }; + runtime = createRuntime(resolvedRuntimeConfig); + // Update finalRuntimeConfig to store resolved path in config + finalRuntimeConfig.srcBaseDir = resolvedSrcBaseDir; + } + } catch (error) { + const errorMsg = error instanceof Error ? error.message : String(error); + return { success: false, error: errorMsg }; + } // Create session BEFORE starting init so events can be forwarded const session = this.getOrCreateSession(workspaceId); diff --git a/src/services/tools/bash.test.ts b/src/services/tools/bash.test.ts index 40c287825..c860cfaa3 100644 --- a/src/services/tools/bash.test.ts +++ b/src/services/tools/bash.test.ts @@ -1173,39 +1173,45 @@ describe("SSH runtime redundant cd detection", () => { }; } - it("should add educational note when command starts with cd", async () => { - const remoteCwd = "~/workspace/project/branch"; + it("should reject redundant cd when command cds to working directory", async () => { + const remoteCwd = "/remote/workspace/project/branch"; using testEnv = createTestBashToolWithSSH(remoteCwd); const tool = testEnv.tool; const args: BashToolArgs = { - script: "cd ~/workspace/project/branch && echo test", + script: "cd /remote/workspace/project/branch && echo test", timeout_secs: 5, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - // Command should execute (not blocked) - // But should include a note about cd behavior - if (result.success && "note" in result) { - expect(result.note).toContain("bash command starts in"); - expect(result.note).toContain("do not persist"); + // Should reject the redundant cd + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Redundant cd to working directory"); + expect(result.error).toContain("no cd needed"); + expect(result.exitCode).toBe(-1); } }); - it("should not add note when command does not start with cd", async () => { - const remoteCwd = "~/workspace/project/branch"; + it("should allow cd to different directory", async () => { + const remoteCwd = "/remote/workspace/project/branch"; using testEnv = createTestBashToolWithSSH(remoteCwd); const tool = testEnv.tool; const args: BashToolArgs = { - script: "echo test", + script: "cd /tmp && echo test", timeout_secs: 5, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - // Should not have a note field - expect(result).not.toHaveProperty("note"); + // Should not be blocked (cd to a different directory is allowed) + // Note: Test runs locally so actual cd might fail, but we check it's not rejected + expect(result.exitCode).not.toBe(-1); // Not a rejection (-1) + if (!result.success) { + // If it failed, it should not be due to redundant cd detection + expect(result.error).not.toContain("Redundant cd"); + } }); }); diff --git a/src/services/tools/bash.ts b/src/services/tools/bash.ts index 2e6375624..5673175ee 100644 --- a/src/services/tools/bash.ts +++ b/src/services/tools/bash.ts @@ -76,11 +76,25 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { let displayTruncated = false; // Hit 16KB display limit let fileTruncated = false; // Hit 100KB file limit - // Detect if command starts with cd - we'll add an educational note for the agent - const scriptStartsWithCd = /^\s*cd\s/.test(script); - const cdNote = scriptStartsWithCd - ? `Note: Each bash command starts in ${config.cwd}. Directory changes (cd) do not persist between commands.` - : undefined; + // Detect redundant cd to working directory + // Match patterns like: "cd /path &&", "cd /path;", "cd '/path' &&", "cd \"/path\" &&" + const cdPattern = /^\s*cd\s+['"]?([^'";&|]+)['"]?\s*[;&|]/; + const match = cdPattern.exec(script); + if (match) { + const targetPath = match[1].trim(); + // Normalize paths for comparison using runtime's path resolution + const normalizedTarget = config.runtime.normalizePath(targetPath, config.cwd); + const normalizedCwd = config.runtime.normalizePath(".", config.cwd); + + if (normalizedTarget === normalizedCwd) { + return { + success: false, + error: `Redundant cd to working directory detected. The tool already runs in ${config.cwd} - no cd needed. Remove the 'cd ${targetPath}' prefix.`, + exitCode: -1, + wall_duration_ms: 0, + }; + } + } // Execute using runtime interface (works for both local and SSH) const execStream = await config.runtime.exec(script, { @@ -377,7 +391,6 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { output, exitCode: 0, wall_duration_ms, - ...(cdNote && { note: cdNote }), truncated: { reason: overflowReason ?? "unknown reason", totalLines: lines.length, @@ -465,7 +478,6 @@ File will be automatically cleaned up when stream ends.`; output: lines.join("\n"), exitCode: 0, wall_duration_ms, - ...(cdNote && { note: cdNote }), }); } else { resolveOnce({ diff --git a/src/services/tools/fileCommon.test.ts b/src/services/tools/fileCommon.test.ts index 835f9a514..a0f5f5822 100644 --- a/src/services/tools/fileCommon.test.ts +++ b/src/services/tools/fileCommon.test.ts @@ -1,6 +1,11 @@ import { describe, it, expect } from "bun:test"; import type { FileStat } from "@/runtime/Runtime"; -import { validatePathInCwd, validateFileSize, MAX_FILE_SIZE } from "./fileCommon"; +import { + validatePathInCwd, + validateFileSize, + validateNoRedundantPrefix, + MAX_FILE_SIZE, +} from "./fileCommon"; import { createRuntime } from "@/runtime/runtimeFactory"; describe("fileCommon", () => { @@ -131,4 +136,99 @@ describe("fileCommon", () => { expect(result).not.toBeNull(); }); }); + + describe("validateNoRedundantPrefix", () => { + const cwd = "/workspace/project"; + const runtime = createRuntime({ type: "local", srcBaseDir: cwd }); + + it("should allow relative paths", () => { + expect(validateNoRedundantPrefix("src/file.ts", cwd, runtime)).toBeNull(); + expect(validateNoRedundantPrefix("./src/file.ts", cwd, runtime)).toBeNull(); + expect(validateNoRedundantPrefix("file.ts", cwd, runtime)).toBeNull(); + }); + + it("should reject absolute paths that contain the cwd prefix", () => { + const result = validateNoRedundantPrefix("/workspace/project/src/file.ts", cwd, runtime); + expect(result).not.toBeNull(); + expect(result?.error).toContain("Redundant path prefix detected"); + expect(result?.error).toContain("Please use relative paths to save tokens"); + expect(result?.error).toContain("src/file.ts"); // Should suggest the relative path + }); + + it("should reject absolute paths at the cwd root", () => { + const result = validateNoRedundantPrefix("/workspace/project/file.ts", cwd, runtime); + expect(result).not.toBeNull(); + expect(result?.error).toContain("Redundant path prefix detected"); + expect(result?.error).toContain("file.ts"); // Should suggest the relative path + }); + + it("should allow absolute paths outside cwd (they will be caught by validatePathInCwd)", () => { + // This validation only catches redundant prefixes, not paths outside cwd + expect(validateNoRedundantPrefix("/etc/passwd", cwd, runtime)).toBeNull(); + expect(validateNoRedundantPrefix("/home/user/file.ts", cwd, runtime)).toBeNull(); + }); + + it("should handle paths with ..", () => { + // Relative paths with .. are fine for this check + expect(validateNoRedundantPrefix("../outside.ts", cwd, runtime)).toBeNull(); + expect(validateNoRedundantPrefix("src/../../outside.ts", cwd, runtime)).toBeNull(); + }); + + it("should work with cwd that has trailing slash", () => { + const cwdWithSlash = "/workspace/project/"; + const result = validateNoRedundantPrefix( + "/workspace/project/src/file.ts", + cwdWithSlash, + runtime + ); + expect(result).not.toBeNull(); + expect(result?.error).toContain("src/file.ts"); + }); + + it("should handle nested paths correctly", () => { + const result = validateNoRedundantPrefix( + "/workspace/project/src/components/Button/index.ts", + cwd, + runtime + ); + expect(result).not.toBeNull(); + expect(result?.error).toContain("src/components/Button/index.ts"); + }); + + it("should reject path that equals cwd exactly", () => { + const result = validateNoRedundantPrefix("/workspace/project", cwd, runtime); + expect(result).not.toBeNull(); + expect(result?.error).toContain("Redundant path prefix detected"); + expect(result?.error).toContain("."); // Should suggest current directory + }); + + it("should not match partial directory names", () => { + // /workspace/project2 should NOT match /workspace/project + expect(validateNoRedundantPrefix("/workspace/project2/file.ts", cwd, runtime)).toBeNull(); + expect(validateNoRedundantPrefix("/workspace/project-old/file.ts", cwd, runtime)).toBeNull(); + }); + + it("should work with SSH runtime", () => { + const sshRuntime = createRuntime({ + type: "ssh", + host: "user@localhost", + srcBaseDir: "/home/user/cmux", + identityFile: "/tmp/fake-key", + }); + const sshCwd = "/home/user/cmux/project/branch"; + + // Should reject absolute paths with redundant prefix on SSH too + const result = validateNoRedundantPrefix( + "/home/user/cmux/project/branch/src/file.ts", + sshCwd, + sshRuntime + ); + expect(result).not.toBeNull(); + expect(result?.error).toContain("Redundant path prefix detected"); + expect(result?.error).toContain("src/file.ts"); + + // Should allow relative paths on SSH + expect(validateNoRedundantPrefix("src/file.ts", sshCwd, sshRuntime)).toBeNull(); + }); + }); }); diff --git a/src/services/tools/fileCommon.ts b/src/services/tools/fileCommon.ts index e5f616337..7b726d33a 100644 --- a/src/services/tools/fileCommon.ts +++ b/src/services/tools/fileCommon.ts @@ -48,6 +48,54 @@ export function validateFileSize(stats: FileStat): { error: string } | null { return null; } +/** + * Validates that a file path doesn't contain redundant workspace prefix. + * Returns an error object if the path contains the cwd prefix, null if valid. + * This helps save tokens by encouraging relative paths. + * + * Works for both local and SSH runtimes by using runtime.normalizePath() + * for consistent path handling across different runtime types. + * + * @param filePath - The file path to validate + * @param cwd - The working directory + * @param runtime - The runtime to use for path normalization + * @returns Error object if redundant prefix found, null if valid + */ +export function validateNoRedundantPrefix( + filePath: string, + cwd: string, + runtime: Runtime +): { error: string } | null { + // Only check absolute paths (start with /) - relative paths are fine + // This works for both local and SSH since both use Unix-style paths + if (!filePath.startsWith("/")) { + return null; + } + + // Use runtime's normalizePath to ensure consistent handling across local and SSH + // Normalize the cwd to get canonical form (removes trailing slashes, etc.) + const normalizedCwd = runtime.normalizePath(".", cwd); + + // For absolute paths, we can't use normalizePath directly (it resolves relative paths) + // so just clean up trailing slashes manually + const normalizedPath = filePath.replace(/\/+$/, ""); + const cleanCwd = normalizedCwd.replace(/\/+$/, ""); + + // Check if the absolute path starts with the cwd + // Use startsWith + check for path separator to avoid partial matches + // e.g., /workspace/project should match /workspace/project/src but not /workspace/project2 + if (normalizedPath === cleanCwd || normalizedPath.startsWith(cleanCwd + "/")) { + // Calculate what the relative path would be + const relativePath = + normalizedPath === cleanCwd ? "." : normalizedPath.substring(cleanCwd.length + 1); + return { + error: `Redundant path prefix detected. The path '${filePath}' contains the workspace directory. Please use relative paths to save tokens: '${relativePath}'`, + }; + } + + return null; +} + /** * Validates that a file path is within the allowed working directory. * Returns an error object if the path is outside cwd, null if valid. diff --git a/src/services/tools/file_edit_insert.test.ts b/src/services/tools/file_edit_insert.test.ts index 562f50be1..2c429c777 100644 --- a/src/services/tools/file_edit_insert.test.ts +++ b/src/services/tools/file_edit_insert.test.ts @@ -55,7 +55,7 @@ describe("file_edit_insert tool", () => { using testEnv = createTestFileEditInsertTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileEditInsertToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path line_offset: 0, content: "INSERTED", }; @@ -78,7 +78,7 @@ describe("file_edit_insert tool", () => { using testEnv = createTestFileEditInsertTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileEditInsertToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path line_offset: 1, content: "INSERTED", }; @@ -101,7 +101,7 @@ describe("file_edit_insert tool", () => { using testEnv = createTestFileEditInsertTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileEditInsertToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path line_offset: 2, content: "INSERTED", }; @@ -124,7 +124,7 @@ describe("file_edit_insert tool", () => { using testEnv = createTestFileEditInsertTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileEditInsertToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path line_offset: 3, content: "INSERTED", }; @@ -147,7 +147,7 @@ describe("file_edit_insert tool", () => { using testEnv = createTestFileEditInsertTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileEditInsertToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path line_offset: 1, content: "INSERTED1\nINSERTED2", }; @@ -169,7 +169,7 @@ describe("file_edit_insert tool", () => { using testEnv = createTestFileEditInsertTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileEditInsertToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path line_offset: 0, content: "INSERTED", }; @@ -186,12 +186,10 @@ describe("file_edit_insert tool", () => { it("should fail when file does not exist and create is not set", async () => { // Setup - const nonExistentPath = path.join(testDir, "nonexistent.txt"); - using testEnv = createTestFileEditInsertTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileEditInsertToolArgs = { - file_path: nonExistentPath, + file_path: "nonexistent.txt", // Use relative path line_offset: 0, content: "INSERTED", }; @@ -209,15 +207,13 @@ describe("file_edit_insert tool", () => { it("should create file when create is true and file does not exist", async () => { // Setup - const nonExistentPath = path.join(testDir, "newfile.txt"); - const tool = createFileEditInsertTool({ cwd: testDir, runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }), runtimeTempDir: "/tmp", }); const args: FileEditInsertToolArgs = { - file_path: nonExistentPath, + file_path: "newfile.txt", // Use relative path line_offset: 0, content: "INSERTED", create: true, @@ -229,21 +225,19 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); - const fileContent = await fs.readFile(nonExistentPath, "utf-8"); + const fileContent = await fs.readFile(path.join(testDir, "newfile.txt"), "utf-8"); expect(fileContent).toBe("INSERTED\n"); }); it("should create parent directories when create is true", async () => { // Setup - const nestedPath = path.join(testDir, "nested", "dir", "newfile.txt"); - const tool = createFileEditInsertTool({ cwd: testDir, runtime: createRuntime({ type: "local", srcBaseDir: "/tmp" }), runtimeTempDir: "/tmp", }); const args: FileEditInsertToolArgs = { - file_path: nestedPath, + file_path: "nested/dir/newfile.txt", // Use relative path line_offset: 0, content: "INSERTED", create: true, @@ -255,7 +249,7 @@ describe("file_edit_insert tool", () => { // Assert expect(result.success).toBe(true); - const fileContent = await fs.readFile(nestedPath, "utf-8"); + const fileContent = await fs.readFile(path.join(testDir, "nested/dir/newfile.txt"), "utf-8"); expect(fileContent).toBe("INSERTED\n"); }); @@ -270,7 +264,7 @@ describe("file_edit_insert tool", () => { runtimeTempDir: "/tmp", }); const args: FileEditInsertToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path line_offset: 1, content: "INSERTED", create: true, @@ -294,7 +288,7 @@ describe("file_edit_insert tool", () => { using testEnv = createTestFileEditInsertTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileEditInsertToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path line_offset: -1, content: "INSERTED", }; @@ -317,7 +311,7 @@ describe("file_edit_insert tool", () => { using testEnv = createTestFileEditInsertTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileEditInsertToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path line_offset: 10, // File only has 2 lines content: "INSERTED", }; diff --git a/src/services/tools/file_edit_insert.ts b/src/services/tools/file_edit_insert.ts index 00fdb2b14..e65a376a9 100644 --- a/src/services/tools/file_edit_insert.ts +++ b/src/services/tools/file_edit_insert.ts @@ -2,7 +2,7 @@ import { tool } from "ai"; import type { FileEditInsertToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { validatePathInCwd } from "./fileCommon"; +import { validatePathInCwd, validateNoRedundantPrefix } from "./fileCommon"; import { WRITE_DENIED_PREFIX } from "@/types/tools"; import { executeFileEditOperation } from "./file_edit_operation"; import { RuntimeError } from "@/runtime/Runtime"; @@ -25,6 +25,19 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) create, }): Promise => { try { + // Validate no redundant path prefix (must come first to catch absolute paths) + const redundantPrefixValidation = validateNoRedundantPrefix( + file_path, + config.cwd, + config.runtime + ); + if (redundantPrefixValidation) { + return { + success: false, + error: redundantPrefixValidation.error, + }; + } + const pathValidation = validatePathInCwd(file_path, config.cwd, config.runtime); if (pathValidation) { return { diff --git a/src/services/tools/file_edit_operation.ts b/src/services/tools/file_edit_operation.ts index 5b16d08c6..5c06e3902 100644 --- a/src/services/tools/file_edit_operation.ts +++ b/src/services/tools/file_edit_operation.ts @@ -1,7 +1,12 @@ import type { FileEditDiffSuccessBase, FileEditErrorResult } from "@/types/tools"; import { WRITE_DENIED_PREFIX } from "@/types/tools"; import type { ToolConfiguration } from "@/utils/tools/tools"; -import { generateDiff, validateFileSize, validatePathInCwd } from "./fileCommon"; +import { + generateDiff, + validateFileSize, + validatePathInCwd, + validateNoRedundantPrefix, +} from "./fileCommon"; import { RuntimeError } from "@/runtime/Runtime"; import { readFileString, writeFileString } from "@/utils/runtime/helpers"; @@ -36,6 +41,19 @@ export async function executeFileEditOperation({ FileEditErrorResult | (FileEditDiffSuccessBase & TMetadata) > { try { + // Validate no redundant path prefix (must come first to catch absolute paths) + const redundantPrefixValidation = validateNoRedundantPrefix( + filePath, + config.cwd, + config.runtime + ); + if (redundantPrefixValidation) { + return { + success: false, + error: `${WRITE_DENIED_PREFIX} ${redundantPrefixValidation.error}`, + }; + } + const pathValidation = validatePathInCwd(filePath, config.cwd, config.runtime); if (pathValidation) { return { diff --git a/src/services/tools/file_edit_replace.test.ts b/src/services/tools/file_edit_replace.test.ts index 6f3d62217..da76ac87f 100644 --- a/src/services/tools/file_edit_replace.test.ts +++ b/src/services/tools/file_edit_replace.test.ts @@ -64,7 +64,7 @@ describe("file_edit_replace_string tool", () => { }); const payload: FileEditReplaceStringToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path old_string: "Hello world", new_string: "Hello universe", }; @@ -102,7 +102,7 @@ describe("file_edit_replace_lines tool", () => { }); const payload: FileEditReplaceLinesToolArgs = { - file_path: testFilePath, + file_path: "test.txt", // Use relative path start_line: 2, end_line: 3, new_lines: ["LINE2", "LINE3"], diff --git a/src/services/tools/file_read.test.ts b/src/services/tools/file_read.test.ts index 5bd855ade..bd061552e 100644 --- a/src/services/tools/file_read.test.ts +++ b/src/services/tools/file_read.test.ts @@ -55,7 +55,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path }; // Execute @@ -78,7 +78,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path offset: 3, // Start from line 3 }; @@ -101,7 +101,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path limit: 2, // Read only first 2 lines }; @@ -124,7 +124,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path offset: 2, // Start from line 2 limit: 2, // Read 2 lines }; @@ -148,7 +148,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path }; // Execute @@ -169,7 +169,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path }; // Execute @@ -185,12 +185,10 @@ describe("file_read tool", () => { it("should fail when file does not exist", async () => { // Setup - const nonExistentPath = path.join(testDir, "nonexistent.txt"); - using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: nonExistentPath, + filePath: "nonexistent.txt", // Use relative path }; // Execute @@ -211,7 +209,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path offset: 10, // Beyond file length }; @@ -234,7 +232,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path }; // Execute @@ -261,7 +259,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path }; // Execute @@ -285,7 +283,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path }; // Execute @@ -308,7 +306,7 @@ describe("file_read tool", () => { using testEnv = createTestFileReadTool({ cwd: testDir }); const tool = testEnv.tool; const args: FileReadToolArgs = { - filePath: testFilePath, + filePath: "test.txt", // Use relative path limit: 500, // Read only 500 lines }; @@ -322,6 +320,29 @@ describe("file_read tool", () => { } }); + it("should reject absolute paths containing the workspace directory", async () => { + // Setup + const content = "test content"; + await fs.writeFile(testFilePath, content); + + using testEnv = createTestFileReadTool({ cwd: testDir }); + const tool = testEnv.tool; + const args: FileReadToolArgs = { + filePath: testFilePath, // Absolute path containing cwd prefix + }; + + // Execute + const result = (await tool.execute!(args, mockToolCallOptions)) as FileReadToolResult; + + // Assert + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error).toContain("Redundant path prefix detected"); + expect(result.error).toContain("Please use relative paths to save tokens"); + expect(result.error).toContain("test.txt"); // Should suggest the relative path + } + }); + it("should reject reading files outside cwd using ..", async () => { // Setup - create a file in testDir const content = "secret content"; diff --git a/src/services/tools/file_read.ts b/src/services/tools/file_read.ts index 34f172169..bba7b8a46 100644 --- a/src/services/tools/file_read.ts +++ b/src/services/tools/file_read.ts @@ -2,7 +2,7 @@ import { tool } from "ai"; import type { FileReadToolResult } from "@/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions"; -import { validatePathInCwd, validateFileSize } from "./fileCommon"; +import { validatePathInCwd, validateFileSize, validateNoRedundantPrefix } from "./fileCommon"; import { RuntimeError } from "@/runtime/Runtime"; import { readFileString } from "@/utils/runtime/helpers"; @@ -21,6 +21,19 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { ): Promise => { // Note: abortSignal available but not used - file reads are fast and complete quickly try { + // Validate no redundant path prefix (must come first to catch absolute paths) + const redundantPrefixValidation = validateNoRedundantPrefix( + filePath, + config.cwd, + config.runtime + ); + if (redundantPrefixValidation) { + return { + success: false, + error: redundantPrefixValidation.error, + }; + } + // Validate that the path is within the working directory const pathValidation = validatePathInCwd(filePath, config.cwd, config.runtime); if (pathValidation) { diff --git a/src/utils/chatCommands.test.ts b/src/utils/chatCommands.test.ts index 304031dd1..306838a5f 100644 --- a/src/utils/chatCommands.test.ts +++ b/src/utils/chatCommands.test.ts @@ -18,7 +18,7 @@ describe("parseRuntimeString", () => { expect(result).toEqual({ type: "ssh", host: "user@host", - srcBaseDir: "~/cmux", + srcBaseDir: "/home/user/cmux", }); }); @@ -27,7 +27,7 @@ describe("parseRuntimeString", () => { expect(result).toEqual({ type: "ssh", host: "User@Host.Example.Com", - srcBaseDir: "~/cmux", + srcBaseDir: "/home/User/cmux", }); }); @@ -36,7 +36,7 @@ describe("parseRuntimeString", () => { expect(result).toEqual({ type: "ssh", host: "user@host", - srcBaseDir: "~/cmux", + srcBaseDir: "/home/user/cmux", }); }); @@ -47,19 +47,34 @@ describe("parseRuntimeString", () => { test("accepts SSH with hostname only (user will be inferred)", () => { const result = parseRuntimeString("ssh hostname", workspaceName); + // When no user is specified, uses current user (process.env.USER) + const expectedUser = process.env.USER ?? "user"; + const expectedHomeDir = expectedUser === "root" ? "/root" : `/home/${expectedUser}`; expect(result).toEqual({ type: "ssh", host: "hostname", - srcBaseDir: "~/cmux", + srcBaseDir: `${expectedHomeDir}/cmux`, }); }); test("accepts SSH with hostname.domain only", () => { const result = parseRuntimeString("ssh dev.example.com", workspaceName); + // When no user is specified, uses current user (process.env.USER) + const expectedUser = process.env.USER ?? "user"; + const expectedHomeDir = expectedUser === "root" ? "/root" : `/home/${expectedUser}`; expect(result).toEqual({ type: "ssh", host: "dev.example.com", - srcBaseDir: "~/cmux", + srcBaseDir: `${expectedHomeDir}/cmux`, + }); + }); + + test("uses /root for root user", () => { + const result = parseRuntimeString("ssh root@hostname", workspaceName); + expect(result).toEqual({ + type: "ssh", + host: "root@hostname", + srcBaseDir: "/root/cmux", }); }); diff --git a/src/utils/chatCommands.ts b/src/utils/chatCommands.ts index 3c19b4901..dbd9fa140 100644 --- a/src/utils/chatCommands.ts +++ b/src/utils/chatCommands.ts @@ -51,12 +51,20 @@ export function parseRuntimeString( throw new Error("SSH runtime requires host (e.g., 'ssh hostname' or 'ssh user@host')"); } + // Extract username from user@host format, or default to current user + const atIndex = hostPart.indexOf("@"); + const user = atIndex > 0 ? hostPart.substring(0, atIndex) : (process.env.USER ?? "user"); + + // Determine home directory path based on user + // root user's home is /root, all others are /home/ + const homeDir = user === "root" ? "/root" : `/home/${user}`; + // Accept both "hostname" and "user@hostname" formats // SSH will use current user or ~/.ssh/config if user not specified return { type: RUNTIME_MODE.SSH, host: hostPart, - srcBaseDir: "~/cmux", // Default remote base directory (NOT including workspace name) + srcBaseDir: `${homeDir}/cmux`, // Default remote base directory (NOT including workspace name) }; } diff --git a/tests/ipcMain/createWorkspace.test.ts b/tests/ipcMain/createWorkspace.test.ts index 44fcbb07a..892f70ec7 100644 --- a/tests/ipcMain/createWorkspace.test.ts +++ b/tests/ipcMain/createWorkspace.test.ts @@ -590,22 +590,26 @@ exit 1 ); test.concurrent( - "handles tilde (~/) paths correctly (SSH only)", + "resolves tilde paths in srcBaseDir to absolute paths (SSH only)", async () => { + if (!sshConfig) { + throw new Error("SSH server is required for SSH integration tests"); + } + const env = await createTestEnvironment(); const tempGitRepo = await createTempGitRepo(); try { - const branchName = generateBranchName("tilde-test"); + const branchName = generateBranchName("tilde-resolution-test"); const trunkBranch = await detectDefaultTrunkBranch(tempGitRepo); - // Use ~/workspace/... path instead of absolute path + // Use tilde path - should be accepted and resolved const tildeRuntimeConfig: RuntimeConfig = { type: "ssh", host: `testuser@localhost`, srcBaseDir: `~/workspace`, - identityFile: sshConfig!.privateKeyPath, - port: sshConfig!.port, + identityFile: sshConfig.privateKeyPath, + port: sshConfig.port, }; const { result, cleanup } = await createWorkspaceWithCleanup( @@ -616,17 +620,22 @@ exit 1 tildeRuntimeConfig ); + // Should succeed and resolve tilde to absolute path expect(result.success).toBe(true); if (!result.success) { - throw new Error(`Failed to create workspace with tilde path: ${result.error}`); + throw new Error(`Failed to create workspace: ${result.error}`); } - // Wait for init to complete - await new Promise((resolve) => setTimeout(resolve, getInitWaitTime())); + // Verify the stored runtimeConfig has resolved path (not tilde) + const projectsConfig = env.config.loadConfigOrDefault(); + const projectWorkspaces = + projectsConfig.projects.get(tempGitRepo)?.workspaces ?? []; + const workspace = projectWorkspaces.find((w) => w.name === branchName); - // Verify workspace exists - expect(result.metadata.id).toBeDefined(); - expect(result.metadata.namedWorkspacePath).toBeDefined(); + expect(workspace).toBeDefined(); + expect(workspace?.runtimeConfig?.srcBaseDir).toBeDefined(); + expect(workspace?.runtimeConfig?.srcBaseDir).toMatch(/^\/home\//); + expect(workspace?.runtimeConfig?.srcBaseDir).not.toContain("~"); await cleanup(); } finally { @@ -638,36 +647,28 @@ exit 1 ); test.concurrent( - "handles tilde paths with init hooks (SSH only)", + "resolves bare tilde in srcBaseDir to home directory (SSH only)", async () => { + if (!sshConfig) { + throw new Error("SSH server is required for SSH integration tests"); + } + const env = await createTestEnvironment(); const tempGitRepo = await createTempGitRepo(); try { - // Add init hook to repo - await createInitHook( - tempGitRepo, - `#!/bin/bash -echo "Init hook executed with tilde path" -` - ); - await commitChanges(tempGitRepo, "Add init hook for tilde test"); - - const branchName = generateBranchName("tilde-init-test"); + const branchName = generateBranchName("bare-tilde-resolution"); const trunkBranch = await detectDefaultTrunkBranch(tempGitRepo); - // Use ~/workspace/... path instead of absolute path + // Use bare tilde - should be accepted and resolved to home directory const tildeRuntimeConfig: RuntimeConfig = { type: "ssh", host: `testuser@localhost`, - srcBaseDir: `~/workspace`, - identityFile: sshConfig!.privateKeyPath, - port: sshConfig!.port, + srcBaseDir: `~`, + identityFile: sshConfig.privateKeyPath, + port: sshConfig.port, }; - // Capture init events to verify hook output - const initEvents = setupInitEventCapture(env); - const { result, cleanup } = await createWorkspaceWithCleanup( env, tempGitRepo, @@ -676,34 +677,22 @@ echo "Init hook executed with tilde path" tildeRuntimeConfig ); + // Should succeed and resolve tilde to home directory expect(result.success).toBe(true); if (!result.success) { - throw new Error( - `Failed to create workspace with tilde path + init hook: ${result.error}` - ); + throw new Error(`Failed to create workspace: ${result.error}`); } - // Wait for init to complete (including hook) - await new Promise((resolve) => setTimeout(resolve, getInitWaitTime())); - - // Verify init hook was executed - const outputEvents = filterEventsByType(initEvents, EVENT_TYPE_INIT_OUTPUT); - const outputLines = outputEvents.map((e) => { - const data = e.data as { line?: string }; - return data.line ?? ""; - }); - - // Debug: Print all output including errors - console.log("=== TILDE INIT HOOK OUTPUT ==="); - outputEvents.forEach((e) => { - const data = e.data as { line?: string; isError?: boolean }; - const prefix = data.isError ? "[ERROR]" : "[INFO] "; - console.log(prefix + (data.line ?? "")); - }); - console.log("=== END TILDE INIT HOOK OUTPUT ==="); + // Verify the stored runtimeConfig has resolved path (not tilde) + const projectsConfig = env.config.loadConfigOrDefault(); + const projectWorkspaces = + projectsConfig.projects.get(tempGitRepo)?.workspaces ?? []; + const workspace = projectWorkspaces.find((w) => w.name === branchName); - expect(outputLines.some((line) => line.includes("Running init hook"))).toBe(true); - expect(outputLines.some((line) => line.includes("Init hook executed"))).toBe(true); + expect(workspace).toBeDefined(); + expect(workspace?.runtimeConfig?.srcBaseDir).toBeDefined(); + expect(workspace?.runtimeConfig?.srcBaseDir).toMatch(/^\/home\//); + expect(workspace?.runtimeConfig?.srcBaseDir).not.toContain("~"); await cleanup(); } finally { @@ -820,10 +809,8 @@ echo "Init hook executed with tilde path" test.concurrent( "forwards origin remote instead of bundle path", async () => { - // Skip if SSH server not available if (!sshConfig) { - console.log("Skipping SSH-specific test: SSH server not available"); - return; + throw new Error("SSH server is required for SSH integration tests"); } const env = await createTestEnvironment(); @@ -848,7 +835,7 @@ echo "Init hook executed with tilde path" const runtimeConfig: RuntimeConfig = { type: "ssh", host: "testuser@localhost", - srcBaseDir: "~/workspace", + srcBaseDir: sshConfig.workdir, identityFile: sshConfig.privateKeyPath, port: sshConfig.port, };