Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,11 @@ export class Config {

// Compute workspace path - this is only for legacy config migration
// New code should use Runtime.getWorkspacePath() directly
// Note: We manually sanitize here since this is config-level code that doesn't use Runtime
const { sanitizeBranchNameForDirectory } = require("./utils/workspace/directoryName");
const directoryName = sanitizeBranchNameForDirectory(metadata.name);
const projectName = this.getProjectName(projectPath);
const workspacePath = path.join(this.srcDir, projectName, metadata.name);
const workspacePath = path.join(this.srcDir, projectName, directoryName);
const workspaceEntry: Workspace = {
path: workspacePath,
id: metadata.id,
Expand Down
28 changes: 20 additions & 8 deletions src/runtime/LocalRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { execAsync, DisposableProcess } from "../utils/disposableExec";
import { getProjectName } from "../utils/runtime/helpers";
import { getErrorMessage } from "../utils/errors";
import { expandTilde } from "./tildeExpansion";
import { sanitizeBranchNameForDirectory } from "../utils/workspace/directoryName";

/**
* Local runtime implementation that executes commands and file operations
Expand Down Expand Up @@ -306,15 +307,18 @@ export class LocalRuntime implements Runtime {

getWorkspacePath(projectPath: string, workspaceName: string): string {
const projectName = getProjectName(projectPath);
return path.join(this.srcBaseDir, projectName, workspaceName);
// Always sanitize workspace name to ensure filesystem-safe directory names
// This is idempotent - sanitizing an already-sanitized name returns the same value
const directoryName = sanitizeBranchNameForDirectory(workspaceName);
return path.join(this.srcBaseDir, projectName, directoryName);
}

async createWorkspace(params: WorkspaceCreationParams): Promise<WorkspaceCreationResult> {
const { projectPath, branchName, trunkBranch, initLogger } = params;
const { projectPath, branchName, trunkBranch, directoryName, initLogger } = params;

try {
// Compute workspace path using the canonical method
const workspacePath = this.getWorkspacePath(projectPath, branchName);
// Compute workspace path (getWorkspacePath sanitizes the name automatically)
const workspacePath = this.getWorkspacePath(projectPath, directoryName);
initLogger.logStep("Creating git worktree...");

// Create parent directory if needed
Expand Down Expand Up @@ -451,11 +455,19 @@ export class LocalRuntime implements Runtime {
{ success: true; oldPath: string; newPath: string } | { success: false; error: string }
> {
// Note: _abortSignal ignored for local operations (fast, no need for cancellation)
// Compute workspace paths using canonical method
// Compute workspace paths (getWorkspacePath sanitizes the names automatically)
const oldPath = this.getWorkspacePath(projectPath, oldName);
const newPath = this.getWorkspacePath(projectPath, newName);

try {
// Create parent directory for new path if needed (for nested directory names)
const newParentDir = path.dirname(newPath);
try {
await fsPromises.access(newParentDir);
} catch {
await fsPromises.mkdir(newParentDir, { recursive: true });
}

// Use git worktree move to rename the worktree directory
// This updates git's internal worktree metadata correctly
using proc = execAsync(`git -C "${projectPath}" worktree move "${oldPath}" "${newPath}"`);
Expand All @@ -479,7 +491,7 @@ export class LocalRuntime implements Runtime {
// These are direct workspace directories (e.g., CLI/benchmark sessions), not git worktrees
const isInPlace = projectPath === workspaceName;

// Compute workspace path using the canonical method
// Compute workspace path (getWorkspacePath sanitizes the name automatically)
const deletedPath = this.getWorkspacePath(projectPath, workspaceName);

// Check if directory exists - if not, operation is idempotent
Expand Down Expand Up @@ -572,7 +584,7 @@ export class LocalRuntime implements Runtime {
async forkWorkspace(params: WorkspaceForkParams): Promise<WorkspaceForkResult> {
const { projectPath, sourceWorkspaceName, newWorkspaceName, initLogger } = params;

// Get source workspace path
// Get source workspace path (getWorkspacePath sanitizes the name automatically)
const sourceWorkspacePath = this.getWorkspacePath(projectPath, sourceWorkspaceName);

// Get current branch from source workspace
Expand All @@ -593,7 +605,7 @@ export class LocalRuntime implements Runtime {
projectPath,
branchName: newWorkspaceName,
trunkBranch: sourceBranch, // Fork from source branch instead of main/master
directoryName: newWorkspaceName,
directoryName: sanitizeBranchNameForDirectory(newWorkspaceName),
initLogger,
});

Expand Down
6 changes: 5 additions & 1 deletion src/runtime/Runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,11 @@ export interface WorkspaceCreationParams {
branchName: string;
/** Trunk branch to base new branches on */
trunkBranch: string;
/** Directory name to use for workspace (typically branch name) */
/**
* Directory name to use for workspace (sanitized branch name).
* This is the filesystem-safe version with slashes converted to dashes.
* ALWAYS computed via sanitizeBranchNameForDirectory(), never stored.
*/
directoryName: string;
/** Logger for streaming creation progress and init hook output */
initLogger: InitLogger;
Expand Down
16 changes: 10 additions & 6 deletions src/runtime/SSHRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -780,14 +780,18 @@ export class SSHRuntime implements Runtime {

getWorkspacePath(projectPath: string, workspaceName: string): string {
const projectName = getProjectName(projectPath);
return path.posix.join(this.config.srcBaseDir, projectName, workspaceName);
// Always sanitize workspace name to ensure filesystem-safe directory names
// This is idempotent - sanitizing an already-sanitized name returns the same value
const { sanitizeBranchNameForDirectory } = require("../utils/workspace/directoryName");
const directoryName = sanitizeBranchNameForDirectory(workspaceName);
return path.posix.join(this.config.srcBaseDir, projectName, directoryName);
}

async createWorkspace(params: WorkspaceCreationParams): Promise<WorkspaceCreationResult> {
try {
const { projectPath, branchName, initLogger, abortSignal } = params;
// Compute workspace path using canonical method
const workspacePath = this.getWorkspacePath(projectPath, branchName);
const { projectPath, branchName, directoryName, initLogger, abortSignal } = params;
// Compute workspace path (getWorkspacePath sanitizes the name automatically)
const workspacePath = this.getWorkspacePath(projectPath, directoryName);

// Prepare parent directory for git clone (fast - returns immediately)
// Note: git clone will create the workspace directory itself during initWorkspace,
Expand Down Expand Up @@ -922,7 +926,7 @@ export class SSHRuntime implements Runtime {
if (abortSignal?.aborted) {
return { success: false, error: "Rename operation aborted" };
}
// Compute workspace paths using canonical method
// Compute workspace paths (getWorkspacePath sanitizes the names automatically)
const oldPath = this.getWorkspacePath(projectPath, oldName);
const newPath = this.getWorkspacePath(projectPath, newName);

Expand Down Expand Up @@ -983,7 +987,7 @@ export class SSHRuntime implements Runtime {
return { success: false, error: "Delete operation aborted" };
}

// Compute workspace path using canonical method
// Compute workspace path (getWorkspacePath sanitizes the name automatically)
const deletedPath = this.getWorkspacePath(projectPath, workspaceName);

try {
Expand Down
1 change: 1 addition & 0 deletions src/services/agentSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ export class AgentSession {
const runtime = createRuntime(
metadata.runtimeConfig ?? { type: "local", srcBaseDir: this.config.srcDir }
);
// getWorkspacePath sanitizes the branch name automatically
return runtime.getWorkspacePath(metadata.projectPath, metadata.name);
})();
assert(
Expand Down
3 changes: 2 additions & 1 deletion src/services/aiService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,8 @@ export class AIService extends EventEmitter {
const isInPlace = metadata.projectPath === metadata.name;
const workspacePath = isInPlace
? metadata.projectPath
: runtime.getWorkspacePath(metadata.projectPath, metadata.name);
: // getWorkspacePath sanitizes the branch name automatically
runtime.getWorkspacePath(metadata.projectPath, metadata.name);

// Build system message from workspace metadata
const systemMessage = await buildSystemMessage(
Expand Down
88 changes: 82 additions & 6 deletions src/services/ipcMain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import type { SendMessageError } from "@/types/errors";
import type { SendMessageOptions, DeleteMessage } from "@/types/ipc";
import { Ok, Err } from "@/types/result";
import { validateWorkspaceName } from "@/utils/validation/workspaceValidation";
import {
sanitizeBranchNameForDirectory,
detectDirectoryNameConflict,
} from "@/utils/workspace/directoryName";
import type { WorkspaceMetadata } from "@/types/workspace";
import { createBashTool } from "@/services/tools/bash";
import type { BashToolResult } from "@/types/tools";
Expand Down Expand Up @@ -220,6 +224,23 @@ export class IpcMain {
return { success: false, error: validation.error };
}

// Check for directory name conflicts with existing workspaces
const config = this.config.loadConfigOrDefault();
const projectConfig = config.projects.get(projectPath);
if (projectConfig) {
const existingBranchNames = projectConfig.workspaces
.map((ws) => ws.name)
.filter((name): name is string => name !== undefined);
const conflict = detectDirectoryNameConflict(branchName, existingBranchNames);
if (conflict) {
const sanitizedName = sanitizeBranchNameForDirectory(branchName);
return {
success: false,
error: `Branch name "${branchName}" conflicts with existing workspace "${conflict}" (both use directory "${sanitizedName}")`,
};
}
}

if (typeof trunkBranch !== "string" || trunkBranch.trim().length === 0) {
return { success: false, error: "Trunk branch is required" };
}
Expand Down Expand Up @@ -284,11 +305,12 @@ export class IpcMain {
};

// Phase 1: Create workspace structure (FAST - returns immediately)
const directoryName = sanitizeBranchNameForDirectory(branchName);
const createResult = await runtime.createWorkspace({
projectPath,
branchName,
trunkBranch: normalizedTrunkBranch,
directoryName: branchName, // Use branch name as directory name
directoryName, // Sanitized directory name (slashes → dashes)
initLogger,
});

Expand Down Expand Up @@ -405,6 +427,48 @@ export class IpcMain {
return Ok({ newWorkspaceId: workspaceId });
}

// Find project path from config (needed for subsequent checks)
const workspace = this.config.findWorkspace(workspaceId);
if (!workspace) {
return Err("Failed to find workspace in config");
}
const { projectPath } = workspace;

// If the sanitized directory names are the same, it's also a no-op
// (e.g., renaming "feature-foo" to "feature/foo")
const oldDirName = sanitizeBranchNameForDirectory(oldName);
const newDirName = sanitizeBranchNameForDirectory(newName);
if (oldDirName === newDirName) {
// Still need to update the name in config even though directory stays the same
this.config.editConfig((config) => {
const projectConfig = config.projects.get(projectPath);
if (projectConfig) {
const workspaceEntry = projectConfig.workspaces.find((w) => w.id === workspaceId);
if (workspaceEntry) {
workspaceEntry.name = newName;
}
}
return config;
});

// Get updated metadata and emit event
const allMetadata = this.config.getAllWorkspaceMetadata();
const updatedMetadata = allMetadata.find((m) => m.id === workspaceId);
if (updatedMetadata) {
const session = this.sessions.get(workspaceId);
if (session) {
session.emitMetadata(updatedMetadata);
} else if (this.mainWindow) {
this.mainWindow.webContents.send(IPC_CHANNELS.WORKSPACE_METADATA, {
workspaceId,
metadata: updatedMetadata,
});
}
}

return Ok({ newWorkspaceId: workspaceId });
}

// Check if new name collides with existing workspace name or ID
const allWorkspaces = this.config.getAllWorkspaceMetadata();
const collision = allWorkspaces.find(
Expand All @@ -414,12 +478,23 @@ export class IpcMain {
return Err(`Workspace with name "${newName}" already exists`);
}

// Find project path from config
const workspace = this.config.findWorkspace(workspaceId);
if (!workspace) {
return Err("Failed to find workspace in config");
// Check for directory name conflicts with existing workspaces
const config = this.config.loadConfigOrDefault();
const projectConfig = config.projects.get(projectPath);
if (projectConfig) {
// Exclude the workspace being renamed from conflict detection
const existingBranchNames = projectConfig.workspaces
.filter((ws) => ws.id !== workspaceId)
.map((ws) => ws.name)
.filter((name): name is string => name !== undefined);
const conflict = detectDirectoryNameConflict(newName, existingBranchNames);
if (conflict) {
const sanitizedName = sanitizeBranchNameForDirectory(newName);
return Err(
`Branch name "${newName}" conflicts with existing workspace "${conflict}" (both use directory "${sanitizedName}")`
);
}
}
const { projectPath } = workspace;

// Create runtime instance for this workspace
// For local runtimes, workdir should be srcDir, not the individual workspace path
Expand Down Expand Up @@ -871,6 +946,7 @@ export class IpcMain {
srcBaseDir: this.config.srcDir,
};
const runtime = createRuntime(runtimeConfig);
// getWorkspacePath sanitizes the branch name automatically
const workspacePath = runtime.getWorkspacePath(metadata.projectPath, metadata.name);

// Create bash tool with workspace's cwd and secrets
Expand Down
14 changes: 10 additions & 4 deletions src/utils/validation/workspaceValidation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,19 @@ describe("validateWorkspaceName", () => {
expect(validateWorkspaceName("a1-b2_c3").valid).toBe(true);
});

test("accepts forward slashes", () => {
expect(validateWorkspaceName("feature/foo").valid).toBe(true);
expect(validateWorkspaceName("feature/branch").valid).toBe(true);
expect(validateWorkspaceName("docs/bash-timeout-ux").valid).toBe(true);
expect(validateWorkspaceName("bugfix/issue-123").valid).toBe(true);
});

test("accepts single character", () => {
expect(validateWorkspaceName("a").valid).toBe(true);
expect(validateWorkspaceName("1").valid).toBe(true);
expect(validateWorkspaceName("_").valid).toBe(true);
expect(validateWorkspaceName("-").valid).toBe(true);
expect(validateWorkspaceName("/").valid).toBe(true);
});

test("accepts 64 characters", () => {
Expand Down Expand Up @@ -72,12 +80,10 @@ 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 backslashes", () => {
expect(validateWorkspaceName("branch\\123").valid).toBe(false);
expect(validateWorkspaceName("path\\to\\branch").valid).toBe(false);
});
});
Expand Down
9 changes: 5 additions & 4 deletions src/utils/validation/workspaceValidation.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/**
* 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
* - Pattern: [a-z0-9_/-]{1,64}
*/
export function validateWorkspaceName(name: string): { valid: boolean; error?: string } {
if (!name || name.length === 0) {
Expand All @@ -13,11 +13,12 @@ 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_/-]+$/;
if (!validPattern.test(name)) {
return {
valid: false,
error: "Workspace name can only contain lowercase letters, digits, underscore, and hyphen",
error:
"Workspace name can only contain lowercase letters, digits, underscore, hyphen, and forward slash",
};
}

Expand Down
Loading
Loading