diff --git a/mobile/src/components/RenameWorkspaceModal.tsx b/mobile/src/components/RenameWorkspaceModal.tsx index 1cf766041c..6bcd6ff550 100644 --- a/mobile/src/components/RenameWorkspaceModal.tsx +++ b/mobile/src/components/RenameWorkspaceModal.tsx @@ -253,7 +253,8 @@ export function RenameWorkspaceModal({ variant="caption" style={{ marginTop: spacing.sm, color: theme.colors.foregroundMuted }} > - Only lowercase letters, digits, underscore, and hyphen (1-64 characters) + Only lowercase letters, digits, underscore, hyphen, and forward slash (1-64 + characters, no leading/trailing/consecutive slashes) )} diff --git a/mobile/src/utils/workspaceValidation.test.ts b/mobile/src/utils/workspaceValidation.test.ts index ea06b2ae43..9d1689f12c 100644 --- a/mobile/src/utils/workspaceValidation.test.ts +++ b/mobile/src/utils/workspaceValidation.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "bun:test"; -import { validateWorkspaceName } from "./workspaceValidation"; +import { validateWorkspaceName, sanitizeWorkspaceNameForPath } from "./workspaceValidation"; describe("validateWorkspaceName", () => { describe("empty names", () => { @@ -55,6 +55,12 @@ describe("validateWorkspaceName", () => { expect(validateWorkspaceName("test-workspace-name")).toEqual({ valid: true }); }); + it("accepts forward slashes in branch-style names", () => { + expect(validateWorkspaceName("feature/my-branch")).toEqual({ valid: true }); + expect(validateWorkspaceName("fix/issue-123")).toEqual({ valid: true }); + expect(validateWorkspaceName("user/feature/deep")).toEqual({ valid: true }); + }); + it("accepts mixed valid characters", () => { expect(validateWorkspaceName("feature-branch_123")).toEqual({ valid: true }); expect(validateWorkspaceName("fix-001")).toEqual({ valid: true }); @@ -73,6 +79,18 @@ describe("validateWorkspaceName", () => { expect(result.error).toContain("lowercase letters"); }); + it("rejects leading slash", () => { + expect(validateWorkspaceName("/feature").valid).toBe(false); + }); + + it("rejects trailing slash", () => { + expect(validateWorkspaceName("feature/").valid).toBe(false); + }); + + it("rejects consecutive slashes", () => { + expect(validateWorkspaceName("feature//branch").valid).toBe(false); + }); + it("rejects special characters", () => { const invalidNames = [ "test!", @@ -92,7 +110,6 @@ describe("validateWorkspaceName", () => { "test}", "test|", "test\\", - "test/", "test?", "test<", "test>", @@ -126,3 +143,25 @@ describe("validateWorkspaceName", () => { }); }); }); + +describe("sanitizeWorkspaceNameForPath", () => { + it("returns name unchanged when no slashes", () => { + expect(sanitizeWorkspaceNameForPath("my-branch")).toBe("my-branch"); + }); + + it("replaces single slash with hyphen", () => { + expect(sanitizeWorkspaceNameForPath("feature/my-branch")).toBe("feature-my-branch"); + }); + + it("replaces multiple slashes in deep paths", () => { + expect(sanitizeWorkspaceNameForPath("user/feature/deep")).toBe("user-feature-deep"); + }); + + it("preserves existing consecutive hyphens", () => { + expect(sanitizeWorkspaceNameForPath("feature--branch")).toBe("feature--branch"); + }); + + it("does not collapse hyphens adjacent to replaced slash", () => { + expect(sanitizeWorkspaceNameForPath("feature/-branch")).toBe("feature--branch"); + }); +}); diff --git a/mobile/src/utils/workspaceValidation.ts b/mobile/src/utils/workspaceValidation.ts index 345d41cd99..74e8b59c60 100644 --- a/mobile/src/utils/workspaceValidation.ts +++ b/mobile/src/utils/workspaceValidation.ts @@ -1,8 +1,9 @@ /** * Validates workspace name format * - Must be 1-64 characters long - * - Can only contain: lowercase letters, digits, underscore, hyphen - * - Pattern: [a-z0-9_-]{1,64} + * - Can only contain: lowercase letters, digits, underscore, hyphen, forward slash + * - No leading, trailing, or consecutive slashes + * - Pattern: [a-z0-9_-]+(?:\/[a-z0-9_-]+)* (1-64 characters) */ export function validateWorkspaceName(name: string): { valid: boolean; error?: string } { if (!name || name.length === 0) { @@ -13,13 +14,22 @@ export function validateWorkspaceName(name: string): { valid: boolean; error?: s return { valid: false, error: "Workspace name cannot exceed 64 characters" }; } - const validPattern = /^[a-z0-9_-]+$/; + const validPattern = /^[a-z0-9_-]+(?:\/[a-z0-9_-]+)*$/; if (!validPattern.test(name)) { return { valid: false, - error: "Workspace name can only contain lowercase letters, digits, underscore, and hyphen", + error: + "Workspace names can only contain lowercase letters, numbers, hyphens, underscores, and forward slashes (no leading, trailing, or consecutive slashes)", }; } return { valid: true }; } + +/** + * Convert a workspace name to a filesystem-safe path component by replacing + * forward slashes with hyphens. + */ +export function sanitizeWorkspaceNameForPath(name: string): string { + return name.replace(/\//g, "-"); +} diff --git a/src/common/utils/validation/workspaceValidation.test.ts b/src/common/utils/validation/workspaceValidation.test.ts index 6ea6a4a7bc..caf840514c 100644 --- a/src/common/utils/validation/workspaceValidation.test.ts +++ b/src/common/utils/validation/workspaceValidation.test.ts @@ -1,4 +1,4 @@ -import { validateWorkspaceName } from "./workspaceValidation"; +import { validateWorkspaceName, sanitizeWorkspaceNameForPath } from "./workspaceValidation"; describe("validateWorkspaceName", () => { describe("valid names", () => { @@ -22,6 +22,12 @@ describe("validateWorkspaceName", () => { expect(validateWorkspaceName("feature-test-123").valid).toBe(true); }); + test("accepts forward slashes in branch-style names", () => { + expect(validateWorkspaceName("feature/my-branch").valid).toBe(true); + expect(validateWorkspaceName("fix/issue-123").valid).toBe(true); + expect(validateWorkspaceName("user/feature/deep").valid).toBe(true); + }); + test("accepts combinations", () => { expect(validateWorkspaceName("feature-branch_123").valid).toBe(true); expect(validateWorkspaceName("a1-b2_c3").valid).toBe(true); @@ -72,13 +78,45 @@ describe("validateWorkspaceName", () => { expect(validateWorkspaceName("branch%123").valid).toBe(false); expect(validateWorkspaceName("branch!123").valid).toBe(false); expect(validateWorkspaceName("branch.123").valid).toBe(false); - expect(validateWorkspaceName("branch/123").valid).toBe(false); expect(validateWorkspaceName("branch\\123").valid).toBe(false); }); - test("rejects names with slashes", () => { - expect(validateWorkspaceName("feature/branch").valid).toBe(false); + test("rejects leading slash", () => { + expect(validateWorkspaceName("/feature").valid).toBe(false); + }); + + test("rejects trailing slash", () => { + expect(validateWorkspaceName("feature/").valid).toBe(false); + }); + + test("rejects consecutive slashes", () => { + expect(validateWorkspaceName("feature//branch").valid).toBe(false); + }); + + test("rejects backslashes", () => { expect(validateWorkspaceName("path\\to\\branch").valid).toBe(false); }); }); }); + +describe("sanitizeWorkspaceNameForPath", () => { + test("returns name unchanged when no slashes", () => { + expect(sanitizeWorkspaceNameForPath("my-branch")).toBe("my-branch"); + }); + + test("replaces single slash with hyphen", () => { + expect(sanitizeWorkspaceNameForPath("feature/my-branch")).toBe("feature-my-branch"); + }); + + test("replaces multiple slashes in deep paths", () => { + expect(sanitizeWorkspaceNameForPath("user/feature/deep")).toBe("user-feature-deep"); + }); + + test("preserves existing consecutive hyphens", () => { + expect(sanitizeWorkspaceNameForPath("feature--branch")).toBe("feature--branch"); + }); + + test("does not collapse hyphens adjacent to replaced slash", () => { + expect(sanitizeWorkspaceNameForPath("feature/-branch")).toBe("feature--branch"); + }); +}); diff --git a/src/common/utils/validation/workspaceValidation.ts b/src/common/utils/validation/workspaceValidation.ts index a45cf6d883..3e76fd57c1 100644 --- a/src/common/utils/validation/workspaceValidation.ts +++ b/src/common/utils/validation/workspaceValidation.ts @@ -1,8 +1,9 @@ /** * Validates workspace name format * - Must be 1-64 characters long - * - Can only contain: lowercase letters, digits, underscore, hyphen - * - Pattern: [a-z0-9_-]{1,64} + * - Can only contain: lowercase letters, digits, underscore, hyphen, forward slash + * - No leading, trailing, or consecutive slashes + * - Pattern: [a-z0-9_-]+(?:\/[a-z0-9_-]+)* (1-64 characters) */ export function validateWorkspaceName(name: string): { valid: boolean; error?: string } { if (!name || name.length === 0) { @@ -13,16 +14,25 @@ export function validateWorkspaceName(name: string): { valid: boolean; error?: s return { valid: false, error: "Workspace name cannot exceed 64 characters" }; } - const validPattern = /^[a-z0-9_-]+$/; + const validPattern = /^[a-z0-9_-]+(?:\/[a-z0-9_-]+)*$/; if (!validPattern.test(name)) { return { valid: false, - // Workspace names become folder names, git branches, and session directories, - // so they need to be filesystem-safe across platforms. error: - "Workspace names can only contain lowercase letters, numbers, hyphens, and underscores", + "Workspace names can only contain lowercase letters, numbers, hyphens, underscores, and forward slashes (no leading, trailing, or consecutive slashes)", }; } return { valid: true }; } + +/** + * Convert a workspace name to a filesystem-safe path component by replacing + * forward slashes with hyphens. + * + * This allows git-style branch names like "feature/my-branch" to be used as + * workspace names while remaining safe for directory names and session paths. + */ +export function sanitizeWorkspaceNameForPath(name: string): string { + return name.replace(/\//g, "-"); +} diff --git a/src/node/config.ts b/src/node/config.ts index 41ccb6cd79..f1c8d18a80 100644 --- a/src/node/config.ts +++ b/src/node/config.ts @@ -39,6 +39,7 @@ import { isValidModelFormat, normalizeGatewayModel } from "@/common/utils/ai/mod import { ensurePrivateDirSync } from "@/node/utils/fs"; import { stripTrailingSlashes } from "@/node/utils/pathUtils"; import { getContainerName as getDockerContainerName } from "@/node/runtime/DockerRuntime"; +import { sanitizeWorkspaceNameForPath } from "@/common/utils/validation/workspaceValidation"; // Re-export project/provider types from dedicated schema/types files (for preload usage) export type { Workspace, ProjectConfig, ProjectsConfig, ProviderConfig, CanonicalProvidersConfig }; @@ -1060,7 +1061,7 @@ export class Config { // otherwise fall back to worktree-style path for legacy compatibility const projectName = this.getProjectName(projectPath); const workspacePath = - metadata.namedWorkspacePath ?? path.join(this.srcDir, projectName, metadata.name); + metadata.namedWorkspacePath ?? path.join(this.srcDir, projectName, path.isAbsolute(metadata.name) ? metadata.name : sanitizeWorkspaceNameForPath(metadata.name)); const workspaceEntry: Workspace = { path: workspacePath, id: metadata.id, diff --git a/src/node/runtime/CoderSSHRuntime.ts b/src/node/runtime/CoderSSHRuntime.ts index 462e6c8f31..bedc5a6c26 100644 --- a/src/node/runtime/CoderSSHRuntime.ts +++ b/src/node/runtime/CoderSSHRuntime.ts @@ -55,6 +55,7 @@ const CODER_NAME_REGEX = /^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$/; */ function toCoderCompatibleName(name: string): string { return name + .replace(/\//g, "-") // Replace slashes with hyphens (branch-style names) .replace(/_/g, "-") // Replace underscores with hyphens .replace(/^-+|-+$/g, "") // Remove leading/trailing hyphens .replace(/-{2,}/g, "-"); // Collapse multiple hyphens diff --git a/src/node/runtime/SSHRuntime.ts b/src/node/runtime/SSHRuntime.ts index 8a4263f956..9895ee3b5a 100644 --- a/src/node/runtime/SSHRuntime.ts +++ b/src/node/runtime/SSHRuntime.ts @@ -48,6 +48,7 @@ import { getOriginUrlForBundle } from "./gitBundleSync"; import { gitNoHooksPrefix } from "@/node/utils/gitNoHooksEnv"; import type { PtyHandle, PtySessionParams, SSHTransport } from "./transports"; import { streamToString, shescape } from "./streamUtils"; +import { sanitizeWorkspaceNameForPath } from "@/common/utils/validation/workspaceValidation"; /** Name of the shared bare repo directory under each project on the remote. */ const BASE_REPO_DIR = ".mux-base.git"; @@ -338,7 +339,13 @@ export class SSHRuntime extends RemoteRuntime { getWorkspacePath(projectPath: string, workspaceName: string): string { const projectName = getProjectName(projectPath); - return path.posix.join(this.config.srcBaseDir, projectName, workspaceName); + // In-place workspaces use absolute paths as workspace names — do not sanitize + // those since path.posix.join already handles them correctly and sanitizing + // would turn e.g. "/home/user/project" into "-home-user-project". + const safeName = path.posix.isAbsolute(workspaceName) + ? workspaceName + : sanitizeWorkspaceNameForPath(workspaceName); + return path.posix.join(this.config.srcBaseDir, projectName, safeName); } /** @@ -1147,6 +1154,27 @@ export class SSHRuntime extends RemoteRuntime { const oldPath = this.getWorkspacePath(projectPath, oldName); const newPath = this.getWorkspacePath(projectPath, newName); + // Short-circuit: if the sanitized paths are identical (e.g. renaming + // "feature-a" <-> "feature/a"), skip the directory move — only the + // persisted name changes, not the physical location. + if (oldPath === newPath) { + // Still rename the git branch to maintain the workspace/branch name sync. + // Best-effort: branch rename can fail if the workspace is in detached HEAD + // state or if the branch has drifted, matching the normal rename path. + try { + const expandedOldPath = expandTildeForSSH(oldPath); + const branchStream = await this.exec( + `git -C ${shescape.quote(expandedOldPath)} branch -m ${shescape.quote(oldName)} ${shescape.quote(newName)}`, + { cwd: this.config.srcBaseDir, timeout: 10, abortSignal } + ); + await branchStream.stdin.abort(); + await branchStream.exitCode; + } catch { + // Best-effort: branch rename can fail (detached HEAD, branch drift, etc.) + } + return { success: true, oldPath, newPath }; + } + try { const expandedOldPath = expandTildeForSSH(oldPath); const expandedNewPath = expandTildeForSSH(newPath); diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index a16a8cf531..c1907e3174 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -31,7 +31,7 @@ import { createRuntimeForWorkspace, resolveWorkspaceExecutionPath, } from "@/node/runtime/runtimeHelpers"; -import { validateWorkspaceName } from "@/common/utils/validation/workspaceValidation"; +import { validateWorkspaceName, sanitizeWorkspaceNameForPath } from "@/common/utils/validation/workspaceValidation"; import { ensurePrivateDir } from "@/node/utils/fs"; import { stripTrailingSlashes } from "@/node/utils/pathUtils"; import { getPlanFilePath, getLegacyPlanFilePath } from "@/common/utils/planStorage"; @@ -76,6 +76,9 @@ import { getSrcBaseDir, isSSHRuntime, isDockerRuntime, + isWorktreeRuntime, + isDevcontainerRuntime, + CODER_RUNTIME_PLACEHOLDER, } from "@/common/types/runtime"; import { isValidModelFormat, normalizeGatewayModel } from "@/common/utils/ai/models"; import { coerceThinkingLevel, type ThinkingLevel } from "@/common/types/thinking"; @@ -259,6 +262,71 @@ function buildWorkspaceTitleConversationContext( }; } +/** + * Check if a runtime config maps workspace names to filesystem directories. + * Worktree, SSH and devcontainer runtimes derive directories from the name; + * Docker and project-dir local runtimes do not. + */ +function usesPathBasedDirs(config: RuntimeConfig | undefined): boolean { + if (!config) return true; // legacy/undefined defaults to worktree + return isWorktreeRuntime(config) || isSSHRuntime(config) || isDevcontainerRuntime(config); +} + +/** + * Check if two runtime configs share the same directory namespace. + * Returns true when both use path-based dirs AND resolve to the same + * srcBaseDir (or both default to undefined / the project-level srcDir). + */ +function sharesPathNamespace( + a: RuntimeConfig | undefined, + b: RuntimeConfig | undefined, + defaultSrcDir?: string +): boolean { + if (!usesPathBasedDirs(a) || !usesPathBasedDirs(b)) return false; + // Resolve effective srcBaseDir: explicit config value or the project default. + // Normalize to handle trailing-slash differences. For local/worktree runtimes + // we also expand tilde so "~/mux/src" matches "/home/user/mux/src". + // For SSH runtimes tilde refers to the *remote* home directory, so local + // expansion would be incorrect — we only strip trailing slashes / normalize. + const aIsSSH = isSSHRuntime(a); + const bIsSSH = isSSHRuntime(b); + const bothSSH = aIsSSH && bIsSSH; + const normalizeSrcDir = (p: string | undefined): string | undefined => { + if (p == null) return p; + const expanded = bothSSH ? p : expandTilde(p); + return stripTrailingSlashes(path.normalize(expanded)); + }; + const aSrc = normalizeSrcDir(getSrcBaseDir(a) ?? defaultSrcDir); + const bSrc = normalizeSrcDir(getSrcBaseDir(b) ?? defaultSrcDir); + if (aSrc !== bSrc) return false; + // SSH runtimes on different hosts never share a filesystem even if + // srcBaseDir strings match. Treat the Coder placeholder as matching + // any Coder peer since it resolves to a real host at finalization time, + // but NOT as matching plain SSH runtimes on unrelated hosts. + if (aIsSSH !== bIsSSH) return false; + if (aIsSSH && bIsSSH) { + const aHost = a!.host; + const bHost = b!.host; + if (aHost !== bHost) { + // Only treat the placeholder as a wildcard when *both* runtimes + // are Coder-backed (have a `coder` config). This prevents a + // Coder runtime whose host hasn't been finalized yet from being + // considered to share a namespace with a plain SSH runtime. + const aIsCoder = "coder" in a! && a!.coder != null; + const bIsCoder = "coder" in b! && b!.coder != null; + const placeholderMatch = + aIsCoder && + bIsCoder && + (aHost === CODER_RUNTIME_PLACEHOLDER || + bHost === CODER_RUNTIME_PLACEHOLDER); + if (!placeholderMatch) { + return false; + } + } + } + return true; +} + /** * Generate a unique fork branch name from the parent workspace name. * Scans existing workspace names for the `{parentName}-fork-N` pattern @@ -1735,6 +1803,32 @@ export class WorkspaceService extends EventEmitter { return Err(errorMsg); } + // Slash-to-hyphen collision check (runs after srcBaseDir resolution so + // SSH paths like "~/mux" are already expanded to absolute paths). + // e.g. "feature/a" and "feature-a" both map to directory "feature-a". + // Only relevant for runtimes where workspace names map to filesystem + // directories. Also only compare against existing workspaces of the + // same kind, since Docker/local workspaces have separate namespaces. + if (usesPathBasedDirs(finalRuntimeConfig)) { + const sanitizedPath = sanitizeWorkspaceNameForPath(branchName); + const existingWorkspaces = + this.config.loadConfigOrDefault().projects.get(stripTrailingSlashes(projectPath)) + ?.workspaces ?? []; + const collision = existingWorkspaces.find( + (w) => + w.name !== branchName && + w.name != null && + !path.isAbsolute(w.name) && + sharesPathNamespace(finalRuntimeConfig, w.runtimeConfig, this.config.srcDir) && + sanitizeWorkspaceNameForPath(w.name) === sanitizedPath + ); + if (collision) { + return Err( + `Workspace name "${branchName}" conflicts with existing workspace "${collision.name}" (both resolve to the same directory)` + ); + } + } + const session = this.getOrCreateSession(workspaceId); this.initStateManager.startInit(workspaceId, projectPath); @@ -2287,6 +2381,30 @@ export class WorkspaceService extends EventEmitter { return Err(`Workspace with name "${newName}" already exists`); } + // Reject renames that would collide on disk after slash sanitization. + // Always check, not just for slash-containing names: "feature-a" collides with "feature/a". + // Only check within the same project since workspace directories are namespaced by project. + // Only compare against workspaces that share path-based directory mapping. + if (usesPathBasedDirs(oldMetadata.runtimeConfig)) { + const sanitizedPath = sanitizeWorkspaceNameForPath(newName); + const sameProjectWorkspaces = allWorkspaces.filter( + (ws) => ws.projectPath === oldMetadata.projectPath + ); + const pathCollision = sameProjectWorkspaces.find( + (ws) => + ws.id !== workspaceId && + ws.name != null && + !path.isAbsolute(ws.name) && + sharesPathNamespace(oldMetadata.runtimeConfig, ws.runtimeConfig, this.config.srcDir) && + sanitizeWorkspaceNameForPath(ws.name) === sanitizedPath + ); + if (pathCollision) { + return Err( + `Workspace name "${newName}" conflicts with existing workspace "${pathCollision.name}" (both resolve to the same directory)` + ); + } + } + const workspace = this.config.findWorkspace(workspaceId); if (!workspace) { return Err("Failed to find workspace in config"); @@ -3067,6 +3185,10 @@ export class WorkspaceService extends EventEmitter { // Fetch all metadata upfront for both branch name and title collision checks. const allMetadata = isAutoName ? await this.config.getAllWorkspaceMetadata() : []; let resolvedName: string; + let existingNamesWithSanitized: string[] = []; + // Track the effective parent name used for fork generation (may be + // normalized for legacy names with invalid characters). + let forkParentName = sourceMetadata.name; if (isAutoName) { const existingNamesSet = new Set( allMetadata.filter((m) => m.projectPath === foundProjectPath).map((m) => m.name) @@ -3085,7 +3207,15 @@ export class WorkspaceService extends EventEmitter { } const existingNames = [...existingNamesSet]; - resolvedName = generateForkBranchName(sourceMetadata.name, existingNames); + // Also include sanitized variants so auto-generated names avoid + // path collisions (e.g. "feature/a" and "feature-a" map to the same dir). + for (const name of existingNames) { + if (!path.isAbsolute(name)) { + existingNamesSet.add(sanitizeWorkspaceNameForPath(name)); + } + } + existingNamesWithSanitized = [...existingNamesSet]; + resolvedName = generateForkBranchName(forkParentName, existingNamesWithSanitized); if (!validateWorkspaceName(resolvedName).valid) { // Legacy workspace names can violate current naming rules (invalid @@ -3103,7 +3233,7 @@ export class WorkspaceService extends EventEmitter { let candidateParent = normalizedParent; while (candidateParent.length > 1) { - resolvedName = generateForkBranchName(candidateParent, existingNames); + resolvedName = generateForkBranchName(candidateParent, existingNamesWithSanitized); if (validateWorkspaceName(resolvedName).valid) { break; } @@ -3111,8 +3241,10 @@ export class WorkspaceService extends EventEmitter { } if (!validateWorkspaceName(resolvedName).valid) { - resolvedName = generateForkBranchName(candidateParent, existingNames); + resolvedName = generateForkBranchName(candidateParent, existingNamesWithSanitized); } + // Remember the normalized parent for collision retry loop + forkParentName = candidateParent; } } else { resolvedName = newName; @@ -3123,6 +3255,83 @@ export class WorkspaceService extends EventEmitter { return Err(resolvedNameValidation.error ?? "Invalid workspace name"); } + // Reject fork names that would collide on disk after slash sanitization. + // For auto-named forks we loop until no collision is found (multiple + // existing names can occupy consecutive suffixes). + // Skip for Docker/devcontainer runtimes which use hash-based container names. + if (usesPathBasedDirs(sourceRuntimeConfig)) { + const existingWorkspaces = + this.config.loadConfigOrDefault().projects.get(stripTrailingSlashes(foundProjectPath)) + ?.workspaces ?? []; + // Safety bound scales with existing workspace count so auto-forking + // always has enough room to skip past sanitized collisions. + const maxRetries = existingWorkspaces.length + 10; + let lastCollision: typeof existingWorkspaces[number] | undefined; + for (let attempt = 0; attempt < maxRetries; attempt++) { + const sanitizedPath = sanitizeWorkspaceNameForPath(resolvedName); + const collision = existingWorkspaces.find( + (w) => + w.name !== resolvedName && + w.name != null && + !path.isAbsolute(w.name) && + sharesPathNamespace(sourceRuntimeConfig, w.runtimeConfig, this.config.srcDir) && + sanitizeWorkspaceNameForPath(w.name) === sanitizedPath + ); + if (!collision) { + lastCollision = undefined; + break; + } + lastCollision = collision; + if (!isAutoName) { + return Err( + `Workspace name "${resolvedName}" conflicts with existing workspace "${collision.name}" (both resolve to the same directory)` + ); + } + // Auto-named fork: add collision name's sanitized form and retry + existingNamesWithSanitized = [ + ...existingNamesWithSanitized, + resolvedName, + sanitizedPath, + ]; + resolvedName = generateForkBranchName(forkParentName, existingNamesWithSanitized); + const retryValidation = validateWorkspaceName(resolvedName); + if (!retryValidation.valid) { + // Name too long after suffix bump — shorten the parent and retry + let shortened = forkParentName; + while (shortened.length > 1) { + shortened = shortened.slice(0, -1); + resolvedName = generateForkBranchName(shortened, existingNamesWithSanitized); + if (validateWorkspaceName(resolvedName).valid) { + forkParentName = shortened; + break; + } + } + if (!validateWorkspaceName(resolvedName).valid) { + return Err("Unable to generate a valid fork name after shortening"); + } + } + } + // After the loop, do a final collision check on the last-generated name. + // The loop generates a new candidate at the end of each iteration, so + // the final candidate was never verified inside the loop body. + if (lastCollision) { + const finalSanitized = sanitizeWorkspaceNameForPath(resolvedName); + const finalCollision = existingWorkspaces.find( + (w) => + w.name !== resolvedName && + w.name != null && + !path.isAbsolute(w.name) && + sharesPathNamespace(sourceRuntimeConfig, w.runtimeConfig, this.config.srcDir) && + sanitizeWorkspaceNameForPath(w.name) === finalSanitized + ); + if (finalCollision) { + return Err( + `Workspace name "${resolvedName}" conflicts with existing workspace "${finalCollision.name}" (both resolve to the same directory) after ${maxRetries} retries` + ); + } + } + } + const sourceRuntime = createRuntime(sourceRuntimeConfig, { projectPath: foundProjectPath, workspaceName: sourceMetadata.name, diff --git a/src/node/utils/runtime/helpers.ts b/src/node/utils/runtime/helpers.ts index f3559d6079..4c8e5d3047 100644 --- a/src/node/utils/runtime/helpers.ts +++ b/src/node/utils/runtime/helpers.ts @@ -243,9 +243,13 @@ export async function movePlanFile( // Resolve tildes to absolute paths - bash doesn't expand ~ inside quotes const resolvedOldPath = await runtime.resolvePath(oldPath); const resolvedNewPath = await runtime.resolvePath(newPath); + // Ensure destination directory exists (slash-containing workspace names + // create subdirectories, e.g. "feature/my-branch.md"). + const newDir = newPath.substring(0, newPath.lastIndexOf("/")); + const resolvedNewDir = await runtime.resolvePath(newDir); await execBuffered( runtime, - `mv ${shellQuote(resolvedOldPath)} ${shellQuote(resolvedNewPath)}`, + `mkdir -p ${shellQuote(resolvedNewDir)} && mv ${shellQuote(resolvedOldPath)} ${shellQuote(resolvedNewPath)}`, { cwd: "/tmp", timeout: 5, diff --git a/src/node/worktree/WorktreeManager.ts b/src/node/worktree/WorktreeManager.ts index a502501010..1eb5e99b13 100644 --- a/src/node/worktree/WorktreeManager.ts +++ b/src/node/worktree/WorktreeManager.ts @@ -17,6 +17,7 @@ import { toPosixPath } from "@/node/utils/paths"; import { log } from "@/node/services/log"; import { GIT_NO_HOOKS_ENV } from "@/node/utils/gitNoHooksEnv"; import { syncMuxignoreFiles } from "./muxignore"; +import { sanitizeWorkspaceNameForPath } from "@/common/utils/validation/workspaceValidation"; export class WorktreeManager { private readonly srcBaseDir: string; @@ -28,7 +29,13 @@ export class WorktreeManager { getWorkspacePath(projectPath: string, workspaceName: string): string { const projectName = getProjectName(projectPath); - return path.join(this.srcBaseDir, projectName, workspaceName); + // In-place workspaces use absolute paths as workspace names — do not sanitize + // those since path.join already handles them correctly and sanitizing would + // turn e.g. "/home/user/project" into "-home-user-project". + const safeName = path.isAbsolute(workspaceName) + ? workspaceName + : sanitizeWorkspaceNameForPath(workspaceName); + return path.join(this.srcBaseDir, projectName, safeName); } async createWorkspace(params: { @@ -244,6 +251,26 @@ export class WorktreeManager { const oldPath = this.getWorkspacePath(projectPath, oldName); const newPath = this.getWorkspacePath(projectPath, newName); + // Short-circuit: if the sanitized paths are identical (e.g. renaming + // "feature-a" <-> "feature/a"), skip the directory move — only the + // persisted name changes, not the physical location. + if (oldPath === newPath) { + // Still rename the git branch to maintain the workspace/branch name sync. + // Best-effort: branch rename can fail if the workspace is in detached HEAD + // state or if the branch has drifted, matching the normal rename path. + try { + using branchProc = execFileAsync( + "git", + ["-C", oldPath, "branch", "-m", oldName, newName], + noHooksEnv + ); + await branchProc.result; + } catch { + // Best-effort: branch rename can fail (detached HEAD, branch drift, etc.) + } + return { success: true, oldPath, newPath }; + } + try { // Move the worktree directory (updates git's internal worktree metadata) using moveProc = execFileAsync(